From 6d757e0061516fd28624c728506c58855238d7a9 Mon Sep 17 00:00:00 2001 From: Nigel Ihza Firdausy <106657905+Ni-zav@users.noreply.github.com> Date: Sun, 11 Oct 2026 21:23:02 +0700 Subject: [PATCH] fix(script): keep undo groups atomic beyond history limit (#810) --- crates/script/src/runtime.rs | 54 ++++++++++++++++++++++++------------ crates/script/src/tests.rs | 22 +++++++++++++++ 2 files changed, 59 insertions(+), 17 deletions(-) diff --git a/crates/script/src/runtime.rs b/crates/script/src/runtime.rs index 712999ee..91c95ce7 100644 --- a/crates/script/src/runtime.rs +++ b/crates/script/src/runtime.rs @@ -8,6 +8,7 @@ use std::sync::Arc; use boa_engine::{Context, JsNativeError, JsResult, JsString, JsValue, NativeFunction, Source, js_string}; use effectcraft_engine::project::Project; use effectcraft_engine::{ScriptRequest, Session}; +use effectcraft_engine::history::History; use serde_json::{Value as J, json}; const PRELUDE: &str = include_str!("prelude.js"); @@ -63,8 +64,9 @@ impl Outcome { pub(crate) struct Active { pub(crate) session: Session, pub(crate) output: Vec, - /// Open undo groups: (name, project snapshot when the outermost group began). - pub(crate) groups: Vec<(String, Arc)>, + /// Open groups: name, project snapshot, original bounded history and saved-project + /// identity. The history snapshot preserves earlier steps if long groups hit the limit. + pub(crate) groups: Vec<(String, Arc, History, Arc)>, /// `app.scheduleTask` queue: (id, code, repeat). pub(crate) tasks: Vec<(u32, String, bool)>, pub(crate) next_task: u32, @@ -238,12 +240,18 @@ fn native_undo(_: &JsValue, args: &[JsValue], ctx: &mut Context) -> JsResult { let snap = a.session.project.clone(); - a.groups.push((if name.is_empty() || name == "undefined" { "Script".into() } else { name }, snap)); + a.groups.push(( + if name.is_empty() || name == "undefined" { "Script".into() } else { name }, + snap, + a.session.history.clone(), + a.session.saved_project.clone(), + )); } _ => { if a.groups.len() == 1 { - let (name, snap) = a.groups.pop().unwrap_or_default(); - collapse(&mut a.session, &name, &snap); + if let Some((name, snap, prior, saved)) = a.groups.pop() { + collapse(&mut a.session, &name, &snap, &prior, &saved); + } } else { a.groups.pop(); } @@ -255,17 +263,29 @@ fn native_undo(_: &JsValue, args: &[JsValue], ctx: &mut Context) -> JsResult) { +/// Fold every command in a group into a single bounded-history entry, even if the +/// oldest intermediate undo state was trimmed before the group ended. +pub(crate) fn collapse( + s: &mut Session, + name: &str, + snap: &Arc, + prior: &History, + saved: &Arc, +) { let h = &mut s.history; - let Some(start) = h.undo.iter().rposition(|(_, p)| Arc::ptr_eq(p, snap)) else { - // Nothing was recorded (or history was reset by New / Open Project). - return; - }; - let first = h.undo[start].1.clone(); - h.undo.truncate(start); - h.undo.push((name.to_string(), first)); - h.merge_key = None; + if let Some(start) = h.undo.iter().rposition(|(_, p)| Arc::ptr_eq(p, snap)) { + // The group is short enough that its opening snapshot is still in history. + let first = h.undo[start].1.clone(); + h.undo.truncate(start); + h.undo.push((name.to_string(), first)); + h.merge_key = None; + } else if !Arc::ptr_eq(&s.project, snap) && Arc::ptr_eq(&s.saved_project, saved) && !h.undo.is_empty() { + // The group exceeded Levels of Undo. Restore the pre-group history (including + // older steps and redo branches) and record one step for the *whole* group. + // A New / Open Project changes saved_project; never resurrect the old history. + *h = prior.clone(); + h.record(name, snap.clone(), s.prefs.general.undo_levels.max(1) as usize); + } } /// `__task("schedule", code, repeat)` → id; `__task("cancel", id)`. @@ -543,8 +563,8 @@ impl Drop for Restore<'_> { /// and take the output so far. pub(crate) fn settle() -> Vec { with_active(|a| { - if let Some((name, snap)) = a.groups.first().cloned() { - collapse(&mut a.session, &name, &snap); + if let Some((name, snap, prior, saved)) = a.groups.first().cloned() { + collapse(&mut a.session, &name, &snap, &prior, &saved); a.groups.clear(); } a.session.sanitize_state(); diff --git a/crates/script/src/tests.rs b/crates/script/src/tests.rs index b50a7bfb..b5819c11 100644 --- a/crates/script/src/tests.rs +++ b/crates/script/src/tests.rs @@ -730,3 +730,25 @@ fn import_options_sequence_picks_one_still_or_the_run() { assert_eq!(kind_of(&s, "shot_[0001-0003].png"), FootageKind::Sequence); let _ = std::fs::remove_dir_all(&dir); } + +#[test] +fn long_script_undo_group_counts_as_one_level() { + let mut s = session(); + s.prefs.general.undo_levels = 3; + ok(&mut s, "var c = app.project.items.addComp('Long Undo', 100, 100, 1, 1, 24); c.layers.addNull();"); + assert_eq!(s.history.undo.len(), 2); + // More edits than the history cap must still be one undoable group. + ok( + &mut s, + "var c = app.project.item(1); app.beginUndoGroup('Forty layers'); for (var i = 0; i < 40; i++) c.layers.addNull(); app.endUndoGroup();", + ); + assert_eq!(s.project.comp(ItemId(1)).unwrap().layers.len(), 41); + assert_eq!(s.history.undo.len(), 3); + assert_eq!(s.history.undo[2].0, "Forty layers"); + s.undo(); + assert_eq!(s.project.comp(ItemId(1)).unwrap().layers.len(), 1); + // Entries preceding the group remain available instead of being trimmed by + // the forty intermediate operations. + s.undo(); + assert_eq!(s.project.comp(ItemId(1)).unwrap().layers.len(), 0); +}