Actions: Action Options and Set Options refuse a field that is not an index

The dialog's fields choose what OK does, as they choose its heading. A
`set` or `action` field that `ui.dialog.set` changed to null or to an
index past the last set or action is now an error: OK renames no set or
action, adds no set, and the dialog stays open. Before, an Action Options
dialog whose `action` was null renamed the set instead.

crates/ui-egui/src/dialogs/action_set_options.rs

@@ -3,7 +3,9 @@
 //!
 //! Fields: `set` (the index of the set renamed, or of the action's set; none for a new set),
 //! `action` (the index of the action renamed, in `set`; none for a set) and `name`. OK adds the set
-//! and makes it the current one, renames the set, or renames the action (`ui.action.rename`).
+//! and makes it the current one, renames the set, or renames the action (`ui.action.rename`). A
+//! `set` or `action` field that is not the index of a set or action is an error, OK then changes
+//! no set or action, and the dialog stays open.
 
 use serde_json::{Value, json};
 
@@ -61,19 +63,15 @@ fn confirm(app: &mut VectorcraftApp, d: &Dialog) -> Result<Value, String> {
     if name.is_empty() {
         return Ok(Value::Null);
     }
-    match (d.fields.get("set").and_then(Value::as_u64), d.fields.get("action").and_then(Value::as_u64)) {
-        (Some(set), Some(action)) => {
-            app.run(crate::panels::actions::RENAME_ACTION, json!({"set": set, "action": action, "name": name}))?;
-        }
-        (Some(set), None) => {
-            if let Some(s) = app.ui.action_sets.get_mut(set as usize) {
-                s.name = name;
-            }
-        }
-        (None, _) => {
-            app.ui.action_sets.push(crate::panels::actions::ActionSet::new(name, vec![]));
-            app.ui.action_set = Some(app.ui.action_sets.len() - 1);
-        }
+    // The fields the dialog holds choose what OK does, as they choose its heading.
+    if d.fields.contains_key("action") {
+        app.run(crate::panels::actions::RENAME_ACTION, json!({"set": d.fields.get("set"), "action": d.fields.get("action"), "name": name}))?;
+    } else if let Some(set) = d.fields.get("set") {
+        let s = set.as_u64().and_then(|i| usize::try_from(i).ok()).and_then(|i| app.ui.action_sets.get_mut(i)).ok_or("no such action set")?;
+        s.name = name;
+    } else {
+        app.ui.action_sets.push(crate::panels::actions::ActionSet::new(name, vec![]));
+        app.ui.action_set = Some(app.ui.action_sets.len() - 1);
     }
     app.ui.dialog = None;
     Ok(Value::Null)

crates/ui-egui/src/panels/actions.rs

@@ -610,6 +610,29 @@ mod tests {
         assert_eq!(names(&app), ["Push", "Nudge Again"], "a refused rename renames nothing");
     }
 
+    /// OK in Action Options or Set Options whose `set` or `action` field `ui.dialog.set` changed to
+    /// something that is not an index is an error: it renames no set or action, adds no set, and
+    /// keeps the dialog open.
+    #[test]
+    fn options_with_a_field_that_is_not_an_index_change_nothing() {
+        use crate::dialogs::action_set_options::KIND;
+        let mut app = app_with_sets();
+        for fields in [
+            json!({"set": 1, "action": null, "name": "X"}),
+            json!({"set": null, "action": 0, "name": "X"}),
+            json!({"set": 1, "action": 9, "name": "X"}),
+            json!({"set": null, "name": "X"}),
+            json!({"set": 9, "name": "X"}),
+        ] {
+            app.ui.dialog = Some(crate::state::Dialog::new(KIND, fields.clone()));
+            assert!(crate::dialogs::confirm(&mut app).is_err(), "{fields}");
+            assert!(app.ui.dialog.is_some(), "the dialog stays open: {fields}");
+            assert_eq!(set_names(&app), ["Default Actions", "Mine", "Other"], "{fields}");
+            let names = app.ui.action_sets[1].actions.iter().map(|a| a.name.as_str()).collect::<Vec<_>>();
+            assert_eq!(names, ["Nudge", "Nudge Again"], "{fields}");
+        }
+    }
+
     /// An app with the default set and the sets Mine (two actions) and Other.
     fn app_with_sets() -> VectorcraftApp {
         let mut app = VectorcraftApp::new(Session::new(), Default::default());