From a0fd599fe674f1ce7b8f3cbf12eac34a78670492 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sat, 24 Oct 2026 02:34:01 -0400 Subject: [PATCH] UI: the macro step editor reads each step in place step_at copied the whole step list to look up one step, and the step list calls it for every row on every frame, so a macro at the 10,000 step limit (which a recording can reach, or a file can carry) took about ten seconds a frame. The lookup now walks the list without copying it: five frames of that macro take well under a second. A test opens a macro at the limit with a generous time bound. --- crates/ui-egui/src/macro_editor.rs | 33 +++++++++++++++++++++++-- crates/ui-egui/tests/it/macro_editor.rs | 17 +++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/macro_editor.rs b/crates/ui-egui/src/macro_editor.rs index 24034c76e..a225eee06 100644 --- a/crates/ui-egui/src/macro_editor.rs +++ b/crates/ui-egui/src/macro_editor.rs @@ -154,9 +154,38 @@ pub fn step_mut<'a>(steps: &'a mut Vec, path: &str) -> Option<&'a mut Step list_mut(steps, parent)?.get_mut(i) } +/// The step list a parent key names, read-only (see [`list_mut`]). +pub fn list_at<'a>(mut cur: &'a [Step], parent: &str) -> Option<&'a [Step]> { + let toks: Vec<&str> = parent.split('.').filter(|t| !t.is_empty()).collect(); + let mut i = 0; + while let Some(tok) = toks.get(i) { + let idx = tok.parse::().ok()?.checked_sub(1)?; + let branch = toks.get(i + 1).copied(); + let st = cur.get(idx)?; + cur = match (&st.kind, branch) { + (StepKind::If { then, .. }, Some("then")) => { + i += 2; + then + } + (StepKind::If { otherwise, .. }, Some("else")) => { + i += 2; + otherwise + } + (StepKind::Repeat { body, .. }, _) => { + i += 1; + body + } + _ => return None, + }; + } + Some(cur) +} + +/// A copy of the step at `path`. Reads the path in place: copying the whole list for every row +/// made a large macro's step list quadratic. pub fn step_at(steps: &[Step], path: &str) -> Option { - let mut copy = steps.to_vec(); - step_mut(&mut copy, path).map(|s| s.clone()) + let (parent, i) = split_path(path)?; + list_at(steps, parent)?.get(i).cloned() } /// The paths of the steps with a breakpoint. diff --git a/crates/ui-egui/tests/it/macro_editor.rs b/crates/ui-egui/tests/it/macro_editor.rs index 241c652b8..d08a8d641 100644 --- a/crates/ui-egui/tests/it/macro_editor.rs +++ b/crates/ui-egui/tests/it/macro_editor.rs @@ -530,3 +530,20 @@ fn the_editor_fits_a_narrow_window() { } } } + +#[test] +fn a_macro_at_the_step_limit_opens_without_freezing_the_editor() { + let mut h = harness(); + let steps: Vec<_> = (0..gridcraft_engine::model::MAX_MACRO_STEPS) + .map(|i| json!({"command": "cell.set", "params": {"cell": format!("A{}", i + 1), "input": "x"}})) + .collect(); + h.state_mut().run("macro.save", json!({"macro": {"name": "Big", "steps": steps}})).unwrap(); + h.state_mut().open_dialog("macroEditor", json!({"macro": "Big"})); + h.run_steps(2); + // Each row used to copy the whole list: five frames took about a minute. Now well under a + // second; the bound leaves room for a slow machine. + let t = std::time::Instant::now(); + h.run_steps(5); + assert!(t.elapsed() < std::time::Duration::from_secs(10), "five frames took {:?}", t.elapsed()); + assert!(h.query_by_label_contains("Step 1: ").is_some()); +}