From e0ed28f55ddd3bf3d9999a11acc30da038181ffb Mon Sep 17 00:00:00 2001 From: bflatastic Date: Sun, 4 Oct 2026 23:02:14 +0700 Subject: [PATCH] M3.9: closing a modified project asks to save it first; the modified mark follows undo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Quit and the window's close button, File ▸ New Project, Open Project, Open Recent, Close Project, the demo project and a Home template show Save / Don't Save / Cancel when the project has unsaved changes (Save asks for a path when untitled; cancelling that keeps the project open). File ▸ Revert asks to discard. Before, all of them dropped unsaved work without asking. - The window title shows the project's path and `*` while it is modified. - The modified mark follows undo: the session keeps the snapshot it last opened or saved (`Session::saved_project`), so undoing, jumping in the History panel or rolling back a batch to the saved state clears it. - An edit that changes nothing (a value set to what it already is) records no undo step and doesn't mark the project modified (`Project::same_content`, comparing untouched comps by pointer). - Agents' `engine.execute` / MCP calls are never asked; `ui.menu.invoke` shows the prompt like a click (automation ids `dialog.unsaved.save` / `dontSave` / `revert` / `cancel`), and the control channel's `app.quit {force: true}` skips it. - Docs: architecture, agents, control protocol, parity. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/commands/file.rs | 2 +- crates/engine/src/footage_check.rs | 4 +- crates/engine/src/lib.rs | 24 ++- crates/engine/src/tests_history.rs | 42 ++++ crates/project/src/lib.rs | 19 ++ crates/project/src/tests.rs | 20 ++ crates/ui-egui/src/control.rs | 6 +- crates/ui-egui/src/lib.rs | 11 + crates/ui-egui/src/menus.rs | 4 + crates/ui-egui/src/panels/dialogs.rs | 3 + crates/ui-egui/src/panels/home.rs | 2 + crates/ui-egui/src/panels/mod.rs | 1 + crates/ui-egui/src/panels/templates.rs | 4 + crates/ui-egui/src/panels/unsaved.rs | 271 +++++++++++++++++++++++++ docs/agents.md | 9 +- docs/architecture.md | 11 +- docs/control-protocol.md | 2 +- docs/parity.md | 11 + 18 files changed, 437 insertions(+), 9 deletions(-) create mode 100644 crates/ui-egui/src/panels/unsaved.rs diff --git a/crates/engine/src/commands/file.rs b/crates/engine/src/commands/file.rs index 1cd8f95c..28759286 100644 --- a/crates/engine/src/commands/file.rs +++ b/crates/engine/src/commands/file.rs @@ -59,7 +59,7 @@ fn save_to(s: &mut Session, path: &str) -> Result { let json = if crate::xml_project::is_xml_path(path) { crate::xml_project::to_xml(&json).map_err(EngineError::Other)? } else { json }; s.services.write_file(path, json.as_bytes()).map_err(|e| EngineError::Other(format!("cannot write {path}: {e}")))?; s.path = Some(path.to_string()); - s.saved_revision = s.revision; + s.mark_saved(); s.note_project_path(path); s.toast(format!("Saved {path}")); Ok(json!({"path": path, "bytes": json.len()})) diff --git a/crates/engine/src/footage_check.rs b/crates/engine/src/footage_check.rs index 79fee8d7..7bad53d0 100644 --- a/crates/engine/src/footage_check.rs +++ b/crates/engine/src/footage_check.rs @@ -112,7 +112,7 @@ fn apply(s: &mut Session, found: Vec<(ItemId, Found)>) -> Value { s.bump(); if !dirty { // Only the disk changed: the project isn't modified. - s.saved_revision = s.revision; + s.mark_saved(); } s.events.push(crate::Event::PurgeCaches); } @@ -148,7 +148,7 @@ mod tests { let there = dir.join("there.png"); std::fs::write(&there, b"x").unwrap(); let mut s = session_with(&[there.to_str().unwrap(), "/nonexistent/ec/gone.png"]); - s.saved_revision = s.revision; + s.mark_saved(); let r = s.execute("footage.check", json!({"wait": true})).unwrap(); assert_eq!(r, json!({"checked": 2, "missing": 1, "probed": 0})); let missing: Vec = s diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 26dc819a..427100f3 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -298,6 +298,9 @@ pub struct Session { pub history: History, pub revision: u64, pub saved_revision: u64, + /// The project as last opened or saved: the project is unmodified while it is this very + /// snapshot, so undoing back to the saved state clears the modified mark. + pub saved_project: Arc, pub path: Option, pub state: EditorState, pub services: Arc, @@ -417,8 +420,10 @@ pub type PluginLoader = fn(&[u8], &str) -> std::result::Result; impl Default for Session { fn default() -> Self { + let project = Arc::new(Project::default()); Session { - project: Arc::new(Project::default()), + saved_project: project.clone(), + project, history: History::default(), revision: 0, saved_revision: 0, @@ -541,6 +546,11 @@ impl Session { self.roto_pending.push(r); } } + // Nothing changed (a value set to what it already was): no undo step, not modified. + if p.same_content(&before) { + self.state = st; + return Ok(r); + } let same = merge.is_some() && merge.map(str::to_string) == self.history.merge_key; if !same { // Undone steps aren't lost: they stay in the History panel as a branch. @@ -558,6 +568,10 @@ impl Session { pub fn bump(&mut self) { self.revision += 1; + // Back at the saved state (undo, redo, a history jump or a rolled-back batch). + if Arc::ptr_eq(&self.project, &self.saved_project) { + self.saved_revision = self.revision; + } if self.disk_cache.is_some() { self.layer_cache.set_disk_salt(effectcraft_render::disk_cache::footage_salt(&self.project)); } @@ -611,6 +625,12 @@ impl Session { self.revision != self.saved_revision } + /// The project as it is now is the saved one (after opening or saving it). + pub fn mark_saved(&mut self) { + self.saved_project = self.project.clone(); + self.saved_revision = self.revision; + } + pub fn active_comp(&self) -> Option<&Comp> { self.project.comp(self.state.active_comp?) } @@ -840,7 +860,7 @@ impl Session { self.state = EditorState { snapping: true, view_layout: 1, ..Default::default() }; self.path = path; self.bump(); - self.saved_revision = self.revision; + self.mark_saved(); let first = self.project.comps().next().map(|(id, _)| *id); if let Some(c) = first { self.open_comp(c); diff --git a/crates/engine/src/tests_history.rs b/crates/engine/src/tests_history.rs index 967c29e3..d90ca7d0 100644 --- a/crates/engine/src/tests_history.rs +++ b/crates/engine/src/tests_history.rs @@ -133,3 +133,45 @@ fn undo_levels_trim_branches() { assert_eq!(s.history.branch_states(), 0); assert_eq!(list(&mut s).len(), 4); } + +#[test] +fn modified_mark_follows_undo_and_no_op_edits_record_nothing() { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "H", "width": 64, "height": 64, "duration": 2})).unwrap(); + solid(&mut s, "A"); + let path = std::env::temp_dir().join(format!("ec-dirty-{}.ecproj", std::process::id())); + s.execute("file.saveAs", json!({"path": path.to_string_lossy()})).unwrap(); + assert!(!s.is_dirty()); + // Setting a value to what it already is: no undo step, still saved. + let steps = s.history.undo.len(); + s.execute("prop.set", json!({"layer": "A", "path": "transform/opacity", "value": 100})).unwrap(); + assert_eq!(s.history.undo.len(), steps); + assert!(!s.is_dirty()); + // A real change, then undo back to the saved state: not modified; redo: modified again. + s.execute("prop.set", json!({"layer": "A", "path": "transform/opacity", "value": 50})).unwrap(); + assert!(s.is_dirty()); + s.execute("edit.undo", json!({})).unwrap(); + assert!(!s.is_dirty(), "undo back to the saved state"); + s.execute("edit.redo", json!({})).unwrap(); + assert!(s.is_dirty()); + // History jumps: the saved state is the parent of the current (changed) one. + let states = list(&mut s); + let cur = states.iter().find(|n| n["current"] == true).unwrap(); + let (changed, saved) = (cur["index"].as_u64().unwrap(), cur["parent"].as_u64().unwrap()); + s.execute("edit.history.goto", json!({"index": saved})).unwrap(); + assert!(!s.is_dirty(), "jumped to the saved state"); + s.execute("edit.history.goto", json!({"index": changed})).unwrap(); + assert!(s.is_dirty()); + s.execute("edit.undo", json!({})).unwrap(); + assert!(!s.is_dirty()); + let r = s.execute( + "engine.batch", + json!({"atomic": true, "steps": [ + {"command": "prop.set", "params": {"layer": "A", "path": "transform/opacity", "value": 10}}, + {"command": "no.suchCommand", "params": {}} + ]}), + ); + assert!(r.is_err()); + assert!(!s.is_dirty(), "rolled back"); + let _ = std::fs::remove_file(path); +} diff --git a/crates/project/src/lib.rs b/crates/project/src/lib.rs index 5e32c7a4..d318cc18 100644 --- a/crates/project/src/lib.rs +++ b/crates/project/src/lib.rs @@ -1047,6 +1047,25 @@ impl Project { } self.next_id = m; } + /// Whether `other` holds the same content (ids allocated but not used don't count). Shared + /// compositions compare by pointer first, so after a copy-on-write edit only the comps it + /// touched are compared field by field. + pub fn same_content(&self, other: &Project) -> bool { + let item_eq = |a: &Item, b: &Item| { + let kind = match (&a.kind, &b.kind) { + (ItemKind::Comp(x), ItemKind::Comp(y)) => Arc::ptr_eq(x, y) || x == y, + (x, y) => x == y, + }; + kind && a.id == b.id && a.name == b.name && a.label == b.label && a.comment == b.comment && a.parent == b.parent && a.proxy == b.proxy + }; + self.schema == other.schema + && self.settings == other.settings + && self.items.len() == other.items.len() + && self.items.iter().zip(&other.items).all(|((ka, a), (kb, b))| ka == kb && item_eq(a, b)) + && self.render_queue == other.render_queue + && self.render_templates == other.render_templates + && self.render_prefs == other.render_prefs + } pub fn to_json(&self) -> String { serde_json::to_string_pretty(self).unwrap_or_default() } diff --git a/crates/project/src/tests.rs b/crates/project/src/tests.rs index 290e7e50..9c200672 100644 --- a/crates/project/src/tests.rs +++ b/crates/project/src/tests.rs @@ -84,3 +84,23 @@ fn camera_and_light_layers_build() { let light = build::layer(&mut p, &comp, "Light 1", LayerSource::Light { kind: crate::LightKind::Spot }, (0, 0), None); assert!(light.props.prop("lightOptions/coneAngle").is_some()); } + +#[test] +fn same_content_ignores_unused_ids_and_sees_comp_edits() { + let (p, cid) = project_with_layer(); + let mut q = p.clone(); + assert!(q.same_content(&p)); + q.alloc(); + assert!(q.same_content(&p), "an allocated but unused id is not a change"); + // A comp copied on write but left equal is the same; a changed one is not. + let _ = q.comp_mut(cid).unwrap(); + assert!(q.same_content(&p)); + q.comp_mut(cid).unwrap().layers[0].name = "Renamed".into(); + assert!(!q.same_content(&p)); + let mut r = p.clone(); + r.item_mut(cid).unwrap().name = "Other".into(); + assert!(!r.same_content(&p)); + let mut r = p.clone(); + r.settings.bit_depth = crate::BitDepth::Bpc16; + assert!(!r.same_content(&p)); +} diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index b2beac44..617f0096 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -17,7 +17,7 @@ //! - `ui.playback {action: play|stop|toggle}` //! - `ui.screenshot {path?, panel?, id?}`: PNG of the window (or one panel / element) //! - `render.frame {comp?, time?, max_side?, path?, base64?}`: PNG of a comp frame rendered by the session -//! - `ui.resize {width, height}` / `ui.focus` / `app.quit` +//! - `ui.resize {width, height}` / `ui.focus` / `app.quit {force?}` (a modified project asks to save first unless `force`) //! //! The full reference with examples is `docs/control-protocol.md`. @@ -436,6 +436,10 @@ pub fn handle(app: &mut EffectcraftApp, ctx: &egui::Context, req: &ControlReques ok(Value::Null) } "app.quit" => { + // A modified project asks to save first unless `force: true`. + if p.get("force").and_then(Value::as_bool).unwrap_or(false) { + app.dialog_state.unsaved.quitting = true; + } ctx.send_viewport_cmd(egui::ViewportCommand::Close); ok(Value::Null) } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 3060eabe..8d6ff74e 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -77,6 +77,8 @@ pub enum Dialog { LayerStyles, /// Edit ▸ Templates ▸ Render Settings… / Output Module…. RenderTemplates, + /// Save changes before closing the project? (`panels::unsaved`). + UnsavedChanges, } /// Host hooks provided by the native app (file pickers etc.). @@ -192,6 +194,8 @@ pub struct EffectcraftApp { pub(crate) home_thumbs: std::collections::HashMap>, /// The (project path, saved revision) whose thumbnail was stored last. pub(crate) home_thumb_saved: Option<(String, u64)>, + /// The window title last sent (project name, `*` while modified). + window_title: String, /// Home ▸ Templates: thumbnail textures by template id (None = not renderable). pub(crate) template_thumbs: std::collections::HashMap>, /// Home ▸ Templates: the gallery, and the user template files it was listed from. @@ -244,6 +248,7 @@ impl EffectcraftApp { dock_rects: vec![], home_thumbs: Default::default(), home_thumb_saved: None, + window_title: String::new(), template_thumbs: Default::default(), template_list: None, } @@ -1056,6 +1061,12 @@ impl EffectcraftApp { } self.apply_prefs(&ctx); self.handle_events(&ctx); + panels::unsaved::on_close_requested(self, &ctx); + let title = panels::unsaved::window_title(self); + if title != self.window_title { + ctx.send_viewport_cmd(egui::ViewportCommand::Title(title.clone())); + self.window_title = title; + } self.tick_autosave(&ctx); prefs_live::frame(self, &ctx); if let Some(rx) = self.command_inbox.take() { diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 1dfb54d3..971b0734 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -192,6 +192,10 @@ fn no_params(p: &Value) -> bool { /// Execute a UI or engine command by id. pub fn invoke(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: Value) -> Result { let now = ctx.input(|i| i.time); + // Closing a modified project asks to save it first; the command runs once answered. + if crate::panels::unsaved::guard(app, id, ¶ms) { + return Ok(json!({"dialog": "unsavedChanges"})); + } // New Camera/Light and Camera/Light Settings without parameters open their dialogs. if crate::panels::dialogs_3d::route(app, id, ¶ms)? { return Ok(Value::Null); diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 8992b69a..8e81a859 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -44,6 +44,8 @@ pub struct DialogState { pub precompose: super::precomp::PrecomposeDraft, /// Layer Style dialog. pub layer_style: super::layer_styles_dialog::LayerStyleState, + /// The unsaved-changes prompt and the command it holds. + pub unsaved: super::unsaved::Pending, } pub fn open_new_comp(app: &mut EffectcraftApp) { @@ -130,6 +132,7 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context) { Dialog::TrackTarget => super::tracker::target_dialog(app, ctx, &t), Dialog::TrackApply => super::tracker::apply_dialog(app, ctx, &t), Dialog::RenderTemplates => super::rq_templates::show(app, ctx, &t), + Dialog::UnsavedChanges => super::unsaved::show(app, ctx, &t), } } diff --git a/crates/ui-egui/src/panels/home.rs b/crates/ui-egui/src/panels/home.rs index 102a6ed2..fc07fd36 100644 --- a/crates/ui-egui/src/panels/home.rs +++ b/crates/ui-egui/src/panels/home.rs @@ -349,6 +349,8 @@ fn run_actions(app: &mut EffectcraftApp, ctx: &egui::Context, actions: Vec<(&str let before = (app.session.path.clone(), app.session.revision); match crate::menus::invoke(app, ctx, id, params) { Err(e) => app.ui.status = e, + // Asking to save the open project first: Home stays until it is answered. + Ok(r) if r.get("dialog").is_some() => {} // Leave Home unless only the list changed or a file dialog was cancelled. Ok(_) => { let cancelled = matches!(id, "file.open" | "file.import") && (app.session.path.clone(), app.session.revision) == before; diff --git a/crates/ui-egui/src/panels/mod.rs b/crates/ui-egui/src/panels/mod.rs index cd8b8460..46557feb 100644 --- a/crates/ui-egui/src/panels/mod.rs +++ b/crates/ui-egui/src/panels/mod.rs @@ -46,6 +46,7 @@ pub mod templates; pub mod text_panels; pub mod timeline; pub mod tracker; +pub mod unsaved; pub mod viewer; pub mod viewer_overlays; pub mod viewer_text; diff --git a/crates/ui-egui/src/panels/templates.rs b/crates/ui-egui/src/panels/templates.rs index 26969a69..8baa76ed 100644 --- a/crates/ui-egui/src/panels/templates.rs +++ b/crates/ui-egui/src/panels/templates.rs @@ -158,6 +158,10 @@ pub fn home_tab(app: &mut EffectcraftApp, ui: &mut egui::Ui, area: Rect) { Err(e) => app.ui.status = e.to_string(), } } else if let Some(id) = open { + // A modified project asks to save first (the template opens once answered). + if super::unsaved::guard(app, "templates.create", &json!({"id": id})) { + return; + } match app.session.execute("templates.create", json!({"id": id})) { Ok(_) => app.ui.start_screen = false, Err(e) => app.ui.status = e.to_string(), diff --git a/crates/ui-egui/src/panels/unsaved.rs b/crates/ui-egui/src/panels/unsaved.rs new file mode 100644 index 00000000..a2ef7e4d --- /dev/null +++ b/crates/ui-egui/src/panels/unsaved.rs @@ -0,0 +1,271 @@ +//! The unsaved-changes prompt: commands that close the open project (Quit and closing the +//! window, File ▸ New Project / Open / Open Recent / Close Project / Revert, the demo project, a +//! Home template) first ask to save a modified project. Save saves (asking for a path when the +//! project is untitled) and continues; Don't Save continues; Cancel keeps the project open. +//! Revert only asks to discard. Agents running commands with `engine.execute` are never asked. +//! +//! Automation ids: `dialog.unsaved.save`, `dialog.unsaved.dontSave` (Revert: `dialog.unsaved.revert`), +//! `dialog.unsaved.cancel`. + +use egui::{Color32, vec2}; +use serde_json::{Value, json}; + +use crate::theme::Tokens; +use crate::{Dialog, EffectcraftApp}; + +/// Commands that replace or close the open project (Quit is asked when the window closes). +const CLOSING: &[&str] = &["file.newProject", "file.open", "file.openRecent", "file.closeProject", "file.revert", "file.openDemoProject", "templates.create"]; + +/// The command waiting on the prompt, and whether the user already answered it. +#[derive(Clone, Debug, Default)] +pub struct Pending { + pub command: Option<(String, Value)>, + /// The next guarded command runs without asking (the user answered Save or Don't Save). + pub answered: bool, + /// The window closes without asking (the user answered the Quit prompt). + pub quitting: bool, +} + +/// Whether `id` needs the prompt first. When it does, the prompt opens holding the command and +/// the caller must not run it (it runs once the user answers). +pub fn guard(app: &mut EffectcraftApp, id: &str, params: &Value) -> bool { + if !CLOSING.contains(&id) || std::mem::take(&mut app.dialog_state.unsaved.answered) { + return false; + } + ask(app, id, params) +} + +fn ask(app: &mut EffectcraftApp, id: &str, params: &Value) -> bool { + if !app.session.is_dirty() { + return false; + } + app.dialog_state.unsaved.command = Some((id.to_string(), params.clone())); + app.dialog = Some(Dialog::UnsavedChanges); + true +} + +/// The window's close button (and Quit, which closes it): keep the window open and ask. +pub fn on_close_requested(app: &mut EffectcraftApp, ctx: &egui::Context) { + if !ctx.input(|i| i.viewport().close_requested()) || app.dialog_state.unsaved.quitting { + return; + } + if ask(app, "app.quit", &json!({})) { + ctx.send_viewport_cmd(egui::ViewportCommand::CancelClose); + } +} + +/// The project's display name: its file name, or `Untitled Project.ecproj`. +pub fn project_name(app: &EffectcraftApp) -> String { + app.session + .path + .as_deref() + .and_then(|p| std::path::Path::new(p).file_name()) + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_else(|| "Untitled Project.ecproj".into()) +} + +/// The window title: the project's path (or name) and `*` while it has unsaved changes. +pub fn window_title(app: &EffectcraftApp) -> String { + let doc = app.session.path.clone().unwrap_or_else(|| project_name(app)); + format!("EffectCraft - {doc}{}", if app.session.is_dirty() { " *" } else { "" }) +} + +enum Answer { + Save, + Discard, + Cancel, +} + +pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { + let Some((id, _)) = app.dialog_state.unsaved.command.clone() else { + app.dialog = None; + return; + }; + let revert = id == "file.revert"; + let name = project_name(app); + let mut answer = None; + let title = if revert { "Revert Project" } else { "Unsaved Changes" }; + super::dialogs::modal(ctx, title, vec2(460.0, 170.0), t, |ui| { + let text = if revert { + format!("Revert “{name}” to the last saved version? Changes made since then will be lost.") + } else { + format!("Save changes to “{name}” before closing?") + }; + ui.label(egui::RichText::new(text).color(t.tab_text_active)); + if !revert { + ui.add_space(4.0); + ui.label(egui::RichText::new("Your changes will be lost if you don't save them.").color(t.text_dim).size(11.0)); + } + ui.add_space(18.0); + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + let (label, key) = if revert { (" Revert ", "dialog.unsaved.revert") } else { (" Save ", "dialog.unsaved.save") }; + let r = ui.add(egui::Button::new(egui::RichText::new(label).color(Color32::WHITE)).fill(t.accent)); + app.auto.add(key, r.rect, label.trim()); + if r.clicked() { + answer = Some(if revert { Answer::Discard } else { Answer::Save }); + } + let r = ui.button("Cancel"); + app.auto.add("dialog.unsaved.cancel", r.rect, "Cancel"); + if r.clicked() { + answer = Some(Answer::Cancel); + } + if !revert { + // Set apart from Save, as is customary. + ui.add_space(ui.available_width() - 90.0); + let r = ui.button("Don't Save"); + app.auto.add("dialog.unsaved.dontSave", r.rect, "Don't Save"); + if r.clicked() { + answer = Some(Answer::Discard); + } + } + }); + }); + if answer.is_none() && ctx.input(|i| i.key_pressed(egui::Key::Escape)) { + answer = Some(Answer::Cancel); + } + if let Some(a) = answer { + answer_with(app, ctx, a); + } +} + +fn answer_with(app: &mut EffectcraftApp, ctx: &egui::Context, a: Answer) { + app.dialog = None; + let Some((id, params)) = app.dialog_state.unsaved.command.take() else { return }; + match a { + Answer::Cancel => return, + Answer::Save => { + // An untitled project asks for a path; cancelling that dialog cancels the command. + if let Err(e) = crate::menus::invoke(app, ctx, "file.save", json!({})) { + app.ui.status = e; + return; + } + if app.session.is_dirty() { + return; + } + } + Answer::Discard => {} + } + if id == "app.quit" { + app.dialog_state.unsaved.quitting = true; + ctx.send_viewport_cmd(egui::ViewportCommand::Close); + return; + } + app.dialog_state.unsaved.answered = true; + let before = (app.session.path.clone(), app.session.revision); + if let Err(e) = crate::menus::invoke(app, ctx, &id, params) { + app.ui.status = e; + } + // Leave Home once something opened from it (not when its file dialog was cancelled). + if (app.session.path.clone(), app.session.revision) != before { + app.ui.start_screen = false; + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn app() -> (EffectcraftApp, egui::Context) { + let ctx = egui::Context::default(); + let app = EffectcraftApp::new(effectcraft_engine::Session::default()); + crate::theme::install(&ctx, &app.tokens); + (app, ctx) + } + + /// One frame of the dialogs, with `close` = the window's close button pressed; returns + /// whether the frame asked the window to stay open. + fn frame(app: &mut EffectcraftApp, ctx: &egui::Context, close: bool) -> bool { + let mut raw = egui::RawInput::default(); + if close { + raw.viewports.entry(egui::ViewportId::ROOT).or_default().events.push(egui::ViewportEvent::Close); + } + let mut out = ctx.run_ui(raw, |ui| { + app.auto.begin_frame(); + on_close_requested(app, ui.ctx()); + crate::panels::dialogs::show(app, ui.ctx()); + }); + out.textures_delta.clear(); + out.viewport_output.get(&egui::ViewportId::ROOT).is_some_and(|v| v.commands.contains(&egui::ViewportCommand::CancelClose)) + } + + fn modified(app: &mut EffectcraftApp) { + app.session.execute("comp.new", json!({"name": "Main", "width": 64, "height": 64})).unwrap(); + assert!(app.session.is_dirty()); + } + + #[test] + fn closing_a_modified_project_asks_and_cancel_keeps_it() { + let (mut app, ctx) = app(); + // An unmodified project closes without asking. + crate::menus::invoke(&mut app, &ctx, "file.newProject", json!({})).unwrap(); + assert!(app.dialog.is_none()); + modified(&mut app); + assert!(window_title(&app).ends_with("Untitled Project.ecproj *"), "{}", window_title(&app)); + let r = crate::menus::invoke(&mut app, &ctx, "file.newProject", json!({})).unwrap(); + assert_eq!(r, json!({"dialog": "unsavedChanges"})); + assert_eq!(app.dialog, Some(Dialog::UnsavedChanges)); + assert_eq!(app.session.project.comps().count(), 1, "nothing closed yet"); + frame(&mut app, &ctx, false); + for id in ["dialog.unsaved.save", "dialog.unsaved.dontSave", "dialog.unsaved.cancel"] { + assert!(app.auto.find(id).is_some(), "{id}"); + } + answer_with(&mut app, &ctx, Answer::Cancel); + assert!(app.dialog.is_none() && app.session.is_dirty()); + assert_eq!(app.session.project.comps().count(), 1); + // Don't Save runs the command. + crate::menus::invoke(&mut app, &ctx, "file.newProject", json!({})).unwrap(); + answer_with(&mut app, &ctx, Answer::Discard); + assert_eq!(app.session.project.comps().count(), 0); + assert!(!app.session.is_dirty() && app.dialog.is_none()); + assert!(!app.dialog_state.unsaved.answered, "the answer is used once"); + // Agents (`engine.execute`) are never asked. + modified(&mut app); + app.session.execute("file.closeProject", json!({})).unwrap(); + assert!(app.dialog.is_none() && app.session.project.comps().count() == 0); + } + + #[test] + fn save_saves_then_continues_and_revert_only_asks_to_discard() { + let (mut app, ctx) = app(); + let dir = std::env::temp_dir().join(format!("ec-unsaved-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("Kept.ecproj"); + modified(&mut app); + app.session.execute("file.saveAs", json!({"path": path.to_string_lossy()})).unwrap(); + assert!(!app.session.is_dirty()); + assert!(window_title(&app).ends_with("Kept.ecproj"), "{}", window_title(&app)); + // Revert: Revert / Cancel. + app.session.execute("layer.newNull", json!({})).unwrap(); + crate::menus::invoke(&mut app, &ctx, "file.revert", json!({})).unwrap(); + frame(&mut app, &ctx, false); + assert!(app.auto.find("dialog.unsaved.revert").is_some() && app.auto.find("dialog.unsaved.save").is_none()); + answer_with(&mut app, &ctx, Answer::Discard); + assert_eq!(app.session.active_comp().unwrap().layers.len(), 0, "reverted"); + // Save: the change is written, then the project closes. + app.session.execute("layer.newNull", json!({})).unwrap(); + crate::menus::invoke(&mut app, &ctx, "file.closeProject", json!({})).unwrap(); + answer_with(&mut app, &ctx, Answer::Save); + assert_eq!(app.session.project.comps().count(), 0, "closed"); + app.session.execute("file.open", json!({"path": path.to_string_lossy()})).unwrap(); + assert_eq!(app.session.active_comp().unwrap().layers.len(), 1, "the null was saved"); + let _ = std::fs::remove_dir_all(dir); + } + + #[test] + fn the_window_stays_open_until_the_quit_prompt_is_answered() { + let (mut app, ctx) = app(); + assert!(!frame(&mut app, &ctx, true), "an unmodified project quits at once"); + modified(&mut app); + assert!(frame(&mut app, &ctx, true), "the close is cancelled"); + assert_eq!(app.dialog, Some(Dialog::UnsavedChanges)); + answer_with(&mut app, &ctx, Answer::Discard); + assert!(app.dialog_state.unsaved.quitting); + assert!(!frame(&mut app, &ctx, true), "Don't Save lets the window close"); + // Undoing back to the saved state needs no prompt. + let (mut app, ctx) = self::app(); + modified(&mut app); + app.session.execute("edit.undo", json!({})).unwrap(); + assert!(!app.session.is_dirty()); + assert!(!frame(&mut app, &ctx, true)); + } +} diff --git a/docs/agents.md b/docs/agents.md index 7c293385..656cbb2d 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -134,7 +134,14 @@ build real projects through these interfaces; they are worked examples of everyt "control":"Title","value":"John Smith"}`); a command that targets an explicit `comp` runs even when the active comp would disable it (`essential.exportTemplate {"comp":"Lower Third", …}`). * **History**: Levels of Undo defaults to 32 (`prefs.set {"key":"general.undoLevels","value":99}`); - `history` lists branches and `history {"goto": id}` jumps between them. + `history` lists branches and `history {"goto": id}` jumps between them. A command that changes + nothing (a value set to what it already is) records no undo step. `get_state` reports `dirty` + (unsaved changes); it follows undo, so undoing back to the saved state clears it. +* **Unsaved changes**: commands run through MCP / `engine.execute` never ask to save. In the + desktop app, `ui.menu.invoke` of a command that closes a modified project (`file.open`, + `file.newProject`, `file.closeProject`, `file.revert`…) and the window's close button show the + Save / Don't Save / Cancel prompt (`ui_click` on `dialog.unsaved.save` / `dontSave` / `cancel`; + `dialog.unsaved.revert` for Revert); the control channel's `app.quit {"force": true}` skips it. ### Motion tracking diff --git a/docs/architecture.md b/docs/architecture.md index d58edc07..04c4d137 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -531,7 +531,16 @@ What keeps it fast: app; inline on wasm32). - **Undo is a pointer swap.** History stores `Arc` snapshots and comps are `Arc`s, so undo and redo of any edit are constant time. An edit copies the comps it changes (a one-layer - edit in a 5,000-layer comp copies that comp: ~8 ms). + edit in a 5,000-layer comp copies that comp: ~8 ms). An edit that changes nothing (a value + set to what it already was) records no step: `Project::same_content` compares the comps the + edit left shared by pointer, so the check costs about as much as the copy. +- **Modified follows undo.** The session keeps the snapshot it last opened or saved + (`Session::saved_project`); the project is unmodified whenever the current snapshot is that + one, so undoing (or jumping in the History panel, or rolling back a batch) to the saved state + clears the modified mark. The window title shows the project and `*` while it is modified, and + closing a modified project (Quit, the window's close button, New Project, Open, Open Recent, + Close Project, Revert, the demo project, a Home template) asks Save / Don't Save / Cancel first + (`ui-egui/src/panels/unsaved.rs`). Agents' `engine.execute` calls are never asked. Numbers (Apple Silicon, 14 cores, release build, best of 3–20 runs; the machine was shared with other builds, load average 150–220, so treat them as orders of magnitude). "Before" is the diff --git a/docs/control-protocol.md b/docs/control-protocol.md index be2e5e44..f789de9f 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -53,7 +53,7 @@ panel registers an element id; both are enforced by `crates/ui-egui/tests/ui_age | `ui.playback` | `{action: play\|stop\|toggle\|status}` | `{playing, time, audio, levelsDb, peaksDb}` (audio: an audio preview is running; L/R meter levels and held peaks in dBFS) | | `ui.resize` | `{width, height}` | Resizes the window (points). | | `ui.focus` | `{}` | Activates the window and takes keyboard focus. Agents normally avoid this, because the app is started without stealing focus. | -| `app.quit` | `{}` | Closes the app. | +| `app.quit` | `{force?}` | Closes the app. A modified project shows the Save / Don't Save / Cancel prompt first (automation ids `dialog.unsaved.save`, `dialog.unsaved.dontSave`, `dialog.unsaved.cancel`) unless `force: true`. | ## Input diff --git a/docs/parity.md b/docs/parity.md index 814e0538..630fb3d4 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -383,6 +383,17 @@ Browser in the web app (landed in M13.10: browser storage and File System Access wrong signatures and versions, fuel exhaustion, traps and bit-identical output; script `Socket` validates ports, and ScriptUI resource strings accept trailing array commas. +## Update: M3.9 unsaved changes + +Closing a modified project asks first, as After Effects does: Quit and the window's close +button, File ▸ New Project, Open Project, Open Recent, Close Project, the demo project and a Home +template show Save / Don't Save / Cancel (Save asks for a path when the project is untitled, and +cancelling that keeps the project open); File ▸ Revert asks to discard. The window title shows +the project's path and `*` while it has unsaved changes. The modified mark now follows undo +(undoing, jumping in the History panel or rolling back a batch to the saved state clears it), +and edits that change nothing record no undo step. Agents' `engine.execute` / MCP calls are never +asked; the control channel's `app.quit {force: true}` skips the prompt. + ## Update: M13.25 Home template gallery, Simulate Output ▸ My Custom RGB **Home ▸ Templates** (UI-7; File ▸ New ▸ New Project from Template…): a gallery of eight