diff --git a/crates/ui-egui/src/menus/frontend.rs b/crates/ui-egui/src/menus/frontend.rs index 047c576a5..4503acfe9 100644 --- a/crates/ui-egui/src/menus/frontend.rs +++ b/crates/ui-egui/src/menus/frontend.rs @@ -320,6 +320,7 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu Value::Null } "window.saveWorkspace" => { + crate::workspace_state::ensure_positions_ready(&app.ui)?; let name = app.ui.workspace.clone(); app.ui.saved_workspaces.insert(name.clone(), app.ui.dock.clone()); app.store_saved_workspaces()?; @@ -336,6 +337,7 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu ); return Ok(Value::Null); }; + crate::workspace_state::ensure_positions_ready(&app.ui)?; app.ui.saved_workspaces.insert(name.clone(), app.ui.dock.clone()); app.ui.workspace = name.clone(); app.store_saved_workspaces()?; @@ -359,6 +361,7 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu }; let builtin = crate::dock::WORKSPACES.contains(&name.as_str()); if p.get("delete").and_then(Value::as_bool) == Some(true) { + crate::workspace_state::ensure_positions_ready(&app.ui)?; if app.ui.saved_workspaces.remove(&name).is_none() && !builtin { return Err(format!("no workspace `{name}`")); } @@ -373,6 +376,7 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu if builtin { return Err(format!("`{name}` is a built-in workspace: save it as a new workspace instead")); } + crate::workspace_state::ensure_positions_ready(&app.ui)?; let dock = app.ui.saved_workspaces.remove(&name).ok_or_else(|| format!("no workspace `{name}`"))?; app.ui.saved_workspaces.insert(new.to_string(), dock); if app.ui.workspace == name { diff --git a/crates/ui-egui/src/workspace_state.rs b/crates/ui-egui/src/workspace_state.rs index 4807e4e40..de2e99ce7 100644 --- a/crates/ui-egui/src/workspace_state.rs +++ b/crates/ui-egui/src/workspace_state.rs @@ -197,9 +197,20 @@ impl WorkspaceStore { let path = config.dir().map_or_else(|| FILE.to_string(), |dir| dir.join(FILE).display().to_string()); return Err(format!("{path} couldn't be loaded at startup, so it is left untouched; fix or move it and restart")); } + ensure_positions_ready(ui)?; self.save(Some(config), ui).map(|_| ()) } } +/// Explicit saves must wait until migrated native-window positions refer to their content. +/// Check before capturing a named layout too, so a later background save cannot persist +/// a snapshot made with the old outer-window positions. +pub(crate) fn ensure_positions_ready(ui: &UiState) -> Result<(), String> { + if !ui.native_outer.is_empty() { + return Err("workspace window positions are still being restored; try saving again in a moment".into()); + } + Ok(()) +} + #[cfg(test)] mod tests; diff --git a/crates/ui-egui/src/workspace_state/tests.rs b/crates/ui-egui/src/workspace_state/tests.rs index e7b6d9aae..aab2814ba 100644 --- a/crates/ui-egui/src/workspace_state/tests.rs +++ b/crates/ui-egui/src/workspace_state/tests.rs @@ -65,6 +65,29 @@ fn corrupt_future_or_unknown_workspace_keeps_defaults_and_original_file() { /// saved as "My Layout"; then Layer closed. const V1: &str = include_str!("../../tests/fixtures/workspaces-v1.json"); +#[test] +fn explicit_save_reports_pending_migration_and_can_be_retried() { + let config = MemoryConfig::default(); + config.write(FILE, V1).unwrap(); + let mut ui = UiState::default(); + let mut store = WorkspaceStore::load(Some(&config), &mut ui); + assert!(!ui.native_outer.is_empty()); + assert!(!store.save(Some(&config), &ui).unwrap(), "background saves defer quietly"); + let error = store.save_now(Some(&config), &ui).unwrap_err(); + assert!(error.contains("positions") && error.contains("try saving again"), "{error}"); + assert_eq!(config.read(FILE).as_deref(), Some(V1)); + assert_eq!(config.read(BACKUP), None); + assert!(store.has_changes(&ui)); + + crate::dock_ui::native_outer_to_content(&mut ui, None, None, 3.0); + store.save_now(Some(&config), &ui).unwrap(); + assert_eq!(config.read(BACKUP).as_deref(), Some(V1)); + let mut restored = UiState::default(); + let loaded = WorkspaceStore::load(Some(&config), &mut restored); + assert_eq!(loaded.error, None); + assert_eq!(Snapshot::capture(&restored), Snapshot::capture(&ui)); +} + #[test] fn version_one_file_loads_with_every_panel_in_place() { let config = MemoryConfig::default(); diff --git a/crates/ui-egui/tests/it/ui_workspace_focus.rs b/crates/ui-egui/tests/it/ui_workspace_focus.rs index 9bd5374bd..af8ecd9ad 100644 --- a/crates/ui-egui/tests/it/ui_workspace_focus.rs +++ b/crates/ui-egui/tests/it/ui_workspace_focus.rs @@ -16,6 +16,29 @@ fn tab_id(p: PanelKind) -> Id { craft_ui::docking::DockArea::::hit_id(effectcraft_ui_egui::dock_ui::area_id(), &craft_ui::docking::Hit::Tab(p)) } +#[test] +fn workspace_commands_wait_for_migration_without_changing_named_layouts() { + let store = Arc::new(MemoryConfig::default()); + let original = include_str!("../fixtures/workspaces-v1.json"); + store.write("workspaces.json", original).unwrap(); + let mut app = EffectcraftApp::new(Session { config: Some(store.clone()), ..Default::default() }); + assert!(!app.ui.native_outer.is_empty()); + let ctx = egui::Context::default(); + let before = (app.ui.workspace.clone(), app.ui.saved_workspaces.clone(), app.ui.dock.clone()); + for (id, params) in [ + ("window.saveWorkspace", json!({})), + ("window.saveWorkspaceAs", json!({"name": "Too soon"})), + ("window.editWorkspaces", json!({"name": "My Layout", "delete": true})), + ("window.editWorkspaces", json!({"name": "My Layout", "rename": "Too soon"})), + ] { + let error = effectcraft_ui_egui::menus::invoke(&mut app, &ctx, id, params).unwrap_err(); + assert!(error.contains("positions"), "{id}: {error}"); + assert_eq!((app.ui.workspace.clone(), app.ui.saved_workspaces.clone(), app.ui.dock.clone()), before); + assert_eq!(store.read("workspaces.json").as_deref(), Some(original)); + assert_eq!(store.read("workspaces.old.json"), None); + } +} + #[test] fn named_and_current_workspaces_survive_frontend_restart() { let store = Arc::new(MemoryConfig::default());