diff --git a/crates/ui-egui/src/dialogs/action_set_options.rs b/crates/ui-egui/src/dialogs/action_set_options.rs index ae10b18b4..568ea41c0 100644 --- a/crates/ui-egui/src/dialogs/action_set_options.rs +++ b/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 { 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) diff --git a/crates/ui-egui/src/panels/actions.rs b/crates/ui-egui/src/panels/actions.rs index 61f86a99e..109d4d824 100644 --- a/crates/ui-egui/src/panels/actions.rs +++ b/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::>(); + 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());