diff --git a/.cursor/plans/guarded_shutdown_savepoints_2026-07-13.plan.md b/.cursor/plans/guarded_shutdown_savepoints_2026-07-13.plan.md new file mode 100644 index 0000000..949b5d6 --- /dev/null +++ b/.cursor/plans/guarded_shutdown_savepoints_2026-07-13.plan.md @@ -0,0 +1,62 @@ +# Guarded Shutdown And History Savepoints + +Date: 2026-07-13 +Issue: BS-PR-709 / Gitea #55 +Milestone: M7 - Production Readiness + +## Goal + +Route every editor-exit surface through one non-blocking dirty-document decision and make scene +dirtiness follow the authored undo timeline. Closing the native window, choosing File > Quit, and +requesting an editor exit programmatically must never silently discard dirty scene tabs. + +## Decisions + +- The editor disables Bevy's automatic close-and-exit systems and owns primary-window close + requests. A guarded-shutdown resource is the single authority that may emit `AppExit`. +- The final dirty-state recheck and `AppExit` authorization run in a dedicated schedule after + Bevy's `Last`; session clean-marker persistence runs after that finalizer. +- Native window close, File > Quit, and internal editor quit requests enqueue the same shutdown + intent. Repeated requests while confirmation or Save As is pending are coalesced. +- Clean sessions exit immediately. Dirty sessions use the shared `NativeDialogBroker` and a single + Save All / Discard / Cancel decision. Cancel preserves all tabs and live editor state; Discard + exits without rewriting authored files; Save All exits only after every dirty tab is saved. +- Untitled tabs and other asynchronous Save As work keep shutdown pending until their dialog result + is applied on the main thread. Cancellation or any failed write returns the editor to an idle, + dirty, recoverable state and does not emit `AppExit`. +- Each scene tab retains a canonical authored-content checkpoint established only by a successful + load or save. The checkpoint is keyed by stable actor identity, canonical component order, stable + parent identity, and scene composition rather than transient Bevy entity numbers. +- Push marks the active document dirty. Undo and redo serialize the authoritative authored + projection and compare it with that tab's checkpoint, so saving at nonzero history depth, + returning to that point, branching, tab switches, and direct non-history mutations remain exact. + +## Implementation + +1. Add per-tab canonical clean checkpoints to scene I/O and reconcile `SceneIo::dirty` after push, + undo, redo, save, load, and document switches without persisting Entity-ID-based history stacks. +2. Add focused history tests for undo-to-clean, redo-away-from-clean, save at nonzero depth, + branching before/after the clean point, and tab isolation. +3. Add a guarded-shutdown plugin/resource that intercepts `WindowCloseRequested`, accepts menu and + programmatic requests, coordinates non-blocking confirmation/save completion, and emits the only + final `AppExit` for the full editor. +4. Make Save All report complete, pending, cancelled, or failed explicitly. Resume pending shutdown + after untitled Save As completion and preserve the originating tab plus dirty state on failure. +5. Add File > Quit and route Switch Project's exit half through the same guard without spawning a + replacement process until the dirty-document decision succeeds. +6. Update ADR 0023, native-dialog/session-recovery documentation, editor architecture guidance, + root README controls/checklist, and production-readiness evidence. +7. Run formatting, strict Clippy, workspace tests, and focused headless state-machine tests. Keep + packaged tests deferred by project-owner direction. +8. Launch the exact editor commit under Hyprland. Exercise Cancel, Discard, Save All, clean close, + File > Quit, and the compositor close button; verify the editor remains responsive during native + dialogs, no state is lost on cancel/failure, and no process or warning remains after exit. + +## Acceptance + +- Native close, File > Quit, and programmatic editor exit share one guarded implementation. +- One or many dirty tabs cannot be lost without explicit Discard; failed or cancelled saves do not + exit and preserve the session. +- Undoing exactly to the saved state clears the dirty marker; redo or a divergent edit restores it. +- Saving at nonzero history depth establishes a new clean point without deleting useful history. +- Headless state-machine tests and exact-commit native Linux QA pass with clean logs. diff --git a/README.md b/README.md index 342ca7e..184f7dc 100644 --- a/README.md +++ b/README.md @@ -57,8 +57,9 @@ cargo run -p editor --bin project_launcher --features dev ``` The installed **Blacksite Editor** desktop entry also exposes **Open Project Browser** from its -desktop action menu. In the editor, **File > Switch Project...** performs a clean shutdown and -opens the same browser; choosing a project starts a fresh editor process with that root. +desktop action menu. In the editor, **File > Switch Project...** uses the guarded Save All / Discard / +Cancel shutdown path before opening the same browser; choosing a project starts a fresh editor +process with that root. ### Hot reload (gameplay iteration) @@ -136,7 +137,7 @@ deep-stale variants. | Click empty viewport / `Esc` | Deselect | | `Delete` / `Backspace` | Delete selection | | `Ctrl+D` | Duplicate selection | -| `Ctrl+Z` / `Ctrl+Shift+Z` / `Ctrl+Y` | Undo / redo | +| `Ctrl+Z` / `Ctrl+Shift+Z` / `Ctrl+Y` | Undo / redo; returning exactly to the last loaded or saved authored state clears that scene tab's dirty marker | | `F2` in Hierarchy | Rename selection | | `W` / `E` / `R` | Translate / rotate / scale gizmo; multi-selection uses one grouped gizmo and undo step | | `X` | Toggle world/local gizmo orientation | @@ -165,7 +166,8 @@ deep-stale variants. | Drag actor between rows / onto Scene Root (Hierarchy, Manual sort) | Reorder siblings / unparent to the root | | Hierarchy lock | Excludes the actor from selection, gizmos, multi-drag, structural drops, and mutating context actions | | Hierarchy context | Group selection, create authored local children below linked prefab roots, remove/reparent generated members through same-layer structural overrides, or unparent | -| File menu | New, non-blocking native Open/Save As, **Open Sample** for the five-area regression pack, recovery restore/keep-copy/discard when available, Import Assets, Export Selection, Save Selection As Prefab (including linked-root variants), Recent Scenes | +| File menu | New, non-blocking native Open/Save As, **Open Sample** for the five-area regression pack, recovery restore/keep-copy/discard when available, Import Assets, Export Selection, Save Selection As Prefab (including linked-root variants), Recent Scenes, Switch Project, and Quit | +| Window close / **File > Quit** | Clean sessions exit immediately; dirty scene tabs use one non-blocking **Save All / Discard / Cancel** decision, and cancel or failed saves keep the editor open | | Status strip / Asset Details source-state chip | Inspect compact clean, modified, untracked, conflicted, read-only, and optional ownership status; hover for the source path and provider details | | Authored File Not Saved dialog | Resolve an external edit, read-only target, or ownership lock with Reload, Compare Metadata, Save As, or Cancel; the editor never offers force overwrite | | Main toolbar, right side | Switch or close independent scene tabs, create an untitled tab, and manage loaded/locked composed subscenes | @@ -410,6 +412,7 @@ crates/ - [x] Transactional physics placement with real Avian gravity/colliders, paused Edit-mode physics, prerequisite diagnostics, isolated non-selected bodies, exact cancel, and grouped transform undo ([ADR 0041](docs/adr/0041-transactional-editor-physics-placement.md), [workflow guide](docs/editor/physics-placement.md), [Gitea #25](https://git.spacetrainclubhouse.com/Falling-Metal-Interactive/Blacksite/issues/25)) - [x] Collider authoring health shared by the inspector, Collider viewport, Diagnostics panel, and physics placement, with scaled shape overlays, cooked mesh bounds, missing/stale/invalid/oversized findings, and undoable dimension-preserving shape switching ([collider guide](docs/editor/collider-authoring.md), [Gitea #26](https://git.spacetrainclubhouse.com/Falling-Metal-Interactive/Blacksite/issues/26)) - [x] Non-blocking native file/folder/confirmation broker across scene, asset, prefab, composition, collaboration, and Project Browser workflows ([ADR 0038](docs/adr/0038-non-blocking-native-dialog-broker.md), [workflow guide](docs/editor/native-dialogs.md), [Gitea #52](https://git.spacetrainclubhouse.com/Falling-Metal-Interactive/Blacksite/issues/52)) +- [x] Guarded native/menu/programmatic editor shutdown with asynchronous multi-tab Save All / Discard / Cancel and canonical per-tab clean checkpoints for exact undo/redo dirtiness ([ADR 0042](docs/adr/0042-guarded-editor-shutdown-and-document-savepoints.md), [Gitea #55](https://git.spacetrainclubhouse.com/Falling-Metal-Interactive/Blacksite/issues/55)) - [x] Asset import, static mesh/prefab placement, texture assignment, and selection export - [x] PIE player-only snapshot/restore (authored `LevelObject` edits persist on stop) - [x] Unified viewport render-to-texture target + Play session bootstrap diff --git a/crates/editor/src/history/mod.rs b/crates/editor/src/history/mod.rs index 03ff134..06a9007 100644 --- a/crates/editor/src/history/mod.rs +++ b/crates/editor/src/history/mod.rs @@ -1631,7 +1631,7 @@ pub fn apply_command_undo(world: &mut World) { undo_command(world, &mut command); world.resource_mut::().push_redo(command); world.resource_mut::().set_redo_status(label); - mark_dirty(world); + crate::scene_io::reconcile_active_dirty_with_checkpoint(world); } pub fn apply_command_redo(world: &mut World) { @@ -1652,7 +1652,7 @@ pub fn apply_command_redo(world: &mut World) { redo_command(world, &mut command); world.resource_mut::().push_undo(command); world.resource_mut::().set_undo_status(label); - mark_dirty(world); + crate::scene_io::reconcile_active_dirty_with_checkpoint(world); } fn prepare_prefab_source_history( diff --git a/crates/editor/src/lib.rs b/crates/editor/src/lib.rs index c3ca58c..8453b0a 100644 --- a/crates/editor/src/lib.rs +++ b/crates/editor/src/lib.rs @@ -30,6 +30,7 @@ pub use project::project_io; pub use project::samples; pub use project::session; pub use project::settings_ui; +pub use project::shutdown; pub use scene::scene_io; pub use scene::scene_schema; pub use scene::scene_view; @@ -76,6 +77,7 @@ use play::PlaySessionPlugin; use project::collaboration::CollaborationPlugin; use project::native_dialog::NativeDialogPlugin; use project::samples::SampleCatalogPlugin; +use project::shutdown::ShutdownPlugin; use project_io::ProjectIoPlugin; use render_view::RenderViewPlugin; use scene_io::SceneIoPlugin; @@ -102,6 +104,7 @@ impl PluginGroup for EditorPluginGroup { .add(ProjectIoPlugin) .add(SampleCatalogPlugin) .add(NativeDialogPlugin) + .add(ShutdownPlugin) .add(scene_schema::SceneSchemaPlugin) .add(net_editor::NetEditorPlugin) .add(AssetDbPlugin) @@ -143,7 +146,7 @@ impl PluginGroup for EditorPluginGroup { /// Shared Bevy app wiring for the in-process editor (game sim + egui shell). pub fn configure_editor_app(app: &mut App) { - app.add_plugins(launch::default_plugins("Bevy FPS Editor")) + app.add_plugins(launch::editor_plugins("Bevy FPS Editor")) .insert_resource(GameInputEnabled(false)) .insert_resource(GameInputFocused(false)) .insert_resource(SimEnabled(false)) diff --git a/crates/editor/src/project/mod.rs b/crates/editor/src/project/mod.rs index 07ca68e..c322980 100644 --- a/crates/editor/src/project/mod.rs +++ b/crates/editor/src/project/mod.rs @@ -8,3 +8,4 @@ pub mod project_io; pub mod samples; pub mod session; pub mod settings_ui; +pub mod shutdown; diff --git a/crates/editor/src/project/session.rs b/crates/editor/src/project/session.rs index d3dd400..f3d3dc6 100644 --- a/crates/editor/src/project/session.rs +++ b/crates/editor/src/project/session.rs @@ -11,6 +11,7 @@ use crate::project_io::{ProjectWorkspace, UserPreferences}; use crate::scene::recovery::{atomic_write, default_state_root}; use crate::scene_io::{SceneIo, SceneIoRequest}; use crate::settings_ui::ProjectSettingsPanel; +use crate::shutdown::ShutdownCoordinator; use crate::ui::{BrushDiagnosticsPanel, DiagnosticsPanel}; use crate::viewport::rendering_diagnostics::RenderingDiagnosticsPanel; use crate::viewport::CameraBookmarks; @@ -109,8 +110,8 @@ impl Plugin for EditorSessionPlugin { .add_systems(Startup, initialize_session) .add_systems(Update, persist_running_session) .add_systems( - Last, - persist_clean_session_on_exit.after(bevy::window::ExitSystems), + crate::shutdown::EditorShutdownFinalize, + persist_clean_session_on_exit.after(crate::shutdown::finalize_authorized_shutdown), ); } } @@ -311,6 +312,7 @@ fn persist_running_session(world: &mut World) { #[allow(clippy::too_many_arguments)] fn persist_clean_session_on_exit( mut exits: MessageReader, + shutdown: Option>, scene_io: Res, workspace: Res, prefs: Res, @@ -323,6 +325,10 @@ fn persist_clean_session_on_exit( if exits.read().next().is_none() { return; } + if !shutdown.is_some_and(|shutdown| shutdown.authorized_exit_sent()) { + warn!("Ignoring an unauthorized AppExit for clean-session persistence"); + return; + } let Some(path) = session_path() else { return; }; diff --git a/crates/editor/src/project/shutdown.rs b/crates/editor/src/project/shutdown.rs new file mode 100644 index 0000000..0ad9428 --- /dev/null +++ b/crates/editor/src/project/shutdown.rs @@ -0,0 +1,752 @@ +//! Guarded editor shutdown and the handoff to scene persistence. + +use bevy::app::{AppExit, MainScheduleOrder}; +use bevy::ecs::schedule::ScheduleLabel; +use bevy::prelude::*; +use bevy::window::{PrimaryWindow, WindowCloseRequested}; + +use crate::native_dialog::NativeDialogBroker; +use crate::scene_io::SceneIo; + +const SAVE_ALL_LABEL: &str = "Save All"; +const DISCARD_LABEL: &str = "Discard"; +const CANCEL_LABEL: &str = "Cancel"; + +/// The user or editor surface that initiated a guarded shutdown. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ShutdownSource { + NativeWindow, + FileMenu, + Programmatic, + SwitchProject, +} + +/// Observable state of the single editor shutdown flow. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum ShutdownIntent { + #[default] + Idle, + WaitingForBroker { + source: ShutdownSource, + }, + Confirming { + source: ShutdownSource, + }, + Saving { + source: ShutdownSource, + }, + Authorized { + source: ShutdownSource, + require_clean_scenes: bool, + }, + ExitSent { + source: ShutdownSource, + }, +} + +impl ShutdownIntent { + pub fn source(self) -> Option { + match self { + Self::Idle => None, + Self::WaitingForBroker { source } + | Self::Confirming { source } + | Self::Saving { source } + | Self::Authorized { source, .. } + | Self::ExitSent { source } => Some(source), + } + } +} + +/// Coordinates all editor exit requests so only an authorized flow emits [`AppExit`]. +#[derive(Resource, Debug, Default)] +pub struct ShutdownCoordinator { + intent: ShutdownIntent, + coalesced_requests: u32, + last_blocker: Option, +} + +impl ShutdownCoordinator { + pub fn intent(&self) -> ShutdownIntent { + self.intent + } + + pub fn coalesced_requests(&self) -> u32 { + self.coalesced_requests + } + + pub fn last_blocker(&self) -> Option<&str> { + self.last_blocker.as_deref() + } + + pub fn authorized_exit_sent(&self) -> bool { + matches!(self.intent, ShutdownIntent::ExitSent { .. }) + } + + /// Queue a shutdown if no shutdown flow is already active. + /// + /// Repeated window, menu, or programmatic requests are coalesced until the + /// current flow is cancelled, fails to save, or sends the authorized exit. + pub fn request(&mut self, source: ShutdownSource) -> bool { + if self.intent != ShutdownIntent::Idle { + self.coalesced_requests = self.coalesced_requests.saturating_add(1); + return false; + } + self.intent = ShutdownIntent::WaitingForBroker { source }; + self.coalesced_requests = 0; + self.last_blocker = None; + true + } + + #[cfg(test)] + pub(crate) fn set_intent_for_test(&mut self, intent: ShutdownIntent) { + self.intent = intent; + } +} + +/// Emitted once when the guarded dialog chooses `Save All`. +/// +/// Scene persistence owns the actual save sequence and must finish it with +/// [`complete_shutdown_save`]. +#[derive(Message, Debug, Clone, Copy, PartialEq, Eq)] +pub struct ShutdownSaveAllRequested { + pub source: ShutdownSource, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ShutdownSaveOutcome { + Saved, + Cancelled, + Failed(String), +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ShutdownDecision { + SaveAll, + Discard, + Cancel, +} + +/// Runs after Bevy's `Last` schedule so no editor authoring system can mutate +/// a document after the final dirty-state check in the same frame. +#[derive(ScheduleLabel, Debug, Clone, PartialEq, Eq, Hash)] +pub struct EditorShutdownFinalize; + +/// Queue a guarded shutdown from an exclusive-world UI or workflow callback. +pub fn request_editor_shutdown(world: &mut World, source: ShutdownSource) -> bool { + world + .get_resource_mut::() + .is_some_and(|mut coordinator| coordinator.request(source)) +} + +/// Queue a project-browser handoff through the same dirty-document guard as editor exit. +pub fn request_project_switch(world: &mut World) -> bool { + request_editor_shutdown(world, ShutdownSource::SwitchProject) +} + +/// Resolve the dirty-scene confirmation through the shared coordinator. +pub fn resolve_shutdown_decision(world: &mut World, decision: ShutdownDecision) -> bool { + let (save_request, status) = { + let Some(mut coordinator) = world.get_resource_mut::() else { + return false; + }; + let ShutdownIntent::Confirming { source } = coordinator.intent else { + return false; + }; + match decision { + ShutdownDecision::SaveAll => { + coordinator.intent = ShutdownIntent::Saving { source }; + ( + Some(source), + Some("Saving all modified scene tabs before shutdown"), + ) + } + ShutdownDecision::Discard => { + coordinator.intent = ShutdownIntent::Authorized { + source, + require_clean_scenes: false, + }; + (None, None) + } + ShutdownDecision::Cancel => { + coordinator.intent = ShutdownIntent::Idle; + ( + None, + Some("Shutdown cancelled; unsaved scene tabs retained"), + ) + } + } + }; + + if let Some(status) = status { + if let Some(mut scene_io) = world.get_resource_mut::() { + scene_io.set_status(status); + } + } + if let Some(source) = save_request { + world.write_message(ShutdownSaveAllRequested { source }); + } + true +} + +/// Finish the save-all handoff. A reported success is authorized only after +/// every scene tab is observably clean. +pub fn complete_shutdown_save(world: &mut World, outcome: ShutdownSaveOutcome) -> bool { + let scenes_are_clean = !scene_has_unsaved_changes(world); + let Some(mut coordinator) = world.get_resource_mut::() else { + return false; + }; + let ShutdownIntent::Saving { source } = coordinator.intent else { + return false; + }; + + match outcome { + ShutdownSaveOutcome::Saved if scenes_are_clean => { + coordinator.intent = ShutdownIntent::Authorized { + source, + require_clean_scenes: true, + }; + coordinator.last_blocker = None; + true + } + ShutdownSaveOutcome::Saved => { + coordinator.intent = ShutdownIntent::Idle; + coordinator.last_blocker = + Some("Save All completed while unsaved scene tabs remain".into()); + false + } + ShutdownSaveOutcome::Cancelled => { + coordinator.intent = ShutdownIntent::Idle; + coordinator.last_blocker = None; + false + } + ShutdownSaveOutcome::Failed(error) => { + coordinator.intent = ShutdownIntent::Idle; + coordinator.last_blocker = Some(error); + false + } + } +} + +pub struct ShutdownPlugin; + +impl Plugin for ShutdownPlugin { + fn build(&self, app: &mut App) { + app.init_schedule(EditorShutdownFinalize); + app.world_mut() + .resource_mut::() + .insert_after(Last, EditorShutdownFinalize); + app.init_resource::() + .add_message::() + .add_systems( + Update, + (capture_native_close_requests, drive_shutdown).chain(), + ) + .add_systems(EditorShutdownFinalize, finalize_authorized_shutdown); + } +} + +fn capture_native_close_requests( + mut commands: Commands, + mut close_requests: MessageReader, + primary_window: Query>, + mut coordinator: ResMut, +) { + let Ok(primary_window) = primary_window.single() else { + return; + }; + for request in close_requests.read() { + if request.window == primary_window { + coordinator.request(ShutdownSource::NativeWindow); + } else { + commands.entity(request.window).despawn(); + } + } +} + +fn drive_shutdown(world: &mut World) { + let intent = world.resource::().intent; + match intent { + ShutdownIntent::Idle + | ShutdownIntent::Confirming { .. } + | ShutdownIntent::Saving { .. } + | ShutdownIntent::ExitSent { .. } => {} + ShutdownIntent::WaitingForBroker { source } => { + let unsaved_count = unsaved_scene_count(world); + if unsaved_count == 0 { + world.resource_mut::().intent = ShutdownIntent::Authorized { + source, + require_clean_scenes: true, + }; + return; + } + if world.resource::().is_pending() { + return; + } + + let action = if source == ShutdownSource::SwitchProject { + "switching projects" + } else { + "quitting" + }; + let description = if unsaved_count == 1 { + format!("One scene tab has unsaved changes. Save it before {action}?") + } else { + format!( + "{unsaved_count} scene tabs have unsaved changes. Save all before {action}?" + ) + }; + let title = if source == ShutdownSource::SwitchProject { + "Switch Project" + } else { + "Quit Blacksite Editor" + }; + let request = world.resource::().request( + move || { + rfd::MessageDialog::new() + .set_title(title) + .set_description(description) + .set_level(rfd::MessageLevel::Warning) + .set_buttons(rfd::MessageButtons::YesNoCancelCustom( + SAVE_ALL_LABEL.into(), + DISCARD_LABEL.into(), + CANCEL_LABEL.into(), + )) + .show() + }, + |world, result| { + resolve_shutdown_decision(world, decision_from_dialog(result)); + }, + ); + if request.is_ok() { + world.resource_mut::().intent = + ShutdownIntent::Confirming { source }; + } + } + ShutdownIntent::Authorized { .. } => {} + } +} + +/// Final dirty-state recheck after editor UI and authoring schedules have completed. +pub(crate) fn finalize_authorized_shutdown(world: &mut World) { + let (source, require_clean_scenes) = { + let coordinator = world.resource::(); + let ShutdownIntent::Authorized { + source, + require_clean_scenes, + } = coordinator.intent + else { + return; + }; + (source, require_clean_scenes) + }; + if require_clean_scenes && scene_has_unsaved_changes(world) { + world.resource_mut::().intent = + ShutdownIntent::WaitingForBroker { source }; + return; + } + if source == ShutdownSource::SwitchProject { + if let Err(error) = crate::launcher::spawn_project_launcher_process() { + let message = format!("Switch project failed: {error}"); + { + let mut coordinator = world.resource_mut::(); + coordinator.intent = ShutdownIntent::Idle; + coordinator.last_blocker = Some(message.clone()); + } + if let Some(mut scene_io) = world.get_resource_mut::() { + scene_io.set_status(message); + } + return; + } + } + world.resource_mut::().intent = ShutdownIntent::ExitSent { source }; + debug!(?source, "Editor shutdown authorized"); + world.write_message(AppExit::Success); +} + +fn scene_has_unsaved_changes(world: &World) -> bool { + world + .get_resource::() + .is_some_and(SceneIo::has_unsaved_tabs) +} + +fn unsaved_scene_count(world: &World) -> usize { + let Some(scene_io) = world.get_resource::() else { + return 0; + }; + let tab_count = scene_io.tabs.iter().filter(|tab| tab.dirty).count(); + if scene_io.dirty + && scene_io + .tabs + .get(scene_io.active_tab) + .is_none_or(|tab| !tab.dirty) + { + tab_count + 1 + } else { + tab_count + } +} + +fn decision_from_dialog(result: rfd::MessageDialogResult) -> ShutdownDecision { + match result { + rfd::MessageDialogResult::Custom(label) if label == SAVE_ALL_LABEL => { + ShutdownDecision::SaveAll + } + rfd::MessageDialogResult::Custom(label) if label == DISCARD_LABEL => { + ShutdownDecision::Discard + } + rfd::MessageDialogResult::Yes => ShutdownDecision::SaveAll, + rfd::MessageDialogResult::No => ShutdownDecision::Discard, + _ => ShutdownDecision::Cancel, + } +} + +#[cfg(test)] +mod tests { + use std::sync::mpsc; + + use bevy::ecs::message::MessageCursor; + + use super::*; + use crate::native_dialog::NativeDialogPlugin; + + fn confirming_world() -> World { + let mut world = World::new(); + world.init_resource::>(); + world.insert_resource(ShutdownCoordinator { + intent: ShutdownIntent::Confirming { + source: ShutdownSource::FileMenu, + }, + ..default() + }); + world + } + + #[test] + fn repeated_requests_are_coalesced_behind_the_first_source() { + let mut coordinator = ShutdownCoordinator::default(); + + assert!(coordinator.request(ShutdownSource::NativeWindow)); + assert!(!coordinator.request(ShutdownSource::FileMenu)); + assert_eq!(coordinator.coalesced_requests(), 1); + assert_eq!( + coordinator.intent(), + ShutdownIntent::WaitingForBroker { + source: ShutdownSource::NativeWindow + } + ); + } + + #[test] + fn cancel_returns_to_idle_and_discard_authorizes_exit() { + let mut cancel_world = confirming_world(); + assert!(resolve_shutdown_decision( + &mut cancel_world, + ShutdownDecision::Cancel + )); + assert_eq!( + cancel_world.resource::().intent(), + ShutdownIntent::Idle + ); + + let mut discard_world = confirming_world(); + assert!(resolve_shutdown_decision( + &mut discard_world, + ShutdownDecision::Discard + )); + assert_eq!( + discard_world.resource::().intent(), + ShutdownIntent::Authorized { + source: ShutdownSource::FileMenu, + require_clean_scenes: false, + } + ); + } + + #[test] + fn dirty_cancel_preserves_the_session_and_discard_emits_one_exit() { + let mut app = App::new(); + app.add_message::() + .init_resource::() + .init_resource::(); + app.world_mut().resource_mut::().mark_dirty(); + app.world_mut() + .resource_mut::() + .set_intent_for_test(ShutdownIntent::Confirming { + source: ShutdownSource::Programmatic, + }); + + assert!(resolve_shutdown_decision( + app.world_mut(), + ShutdownDecision::Cancel + )); + drive_shutdown(app.world_mut()); + finalize_authorized_shutdown(app.world_mut()); + assert_eq!(app.should_exit(), None); + assert!(app.world().resource::().has_unsaved_tabs()); + + app.world_mut() + .resource_mut::() + .set_intent_for_test(ShutdownIntent::Confirming { + source: ShutdownSource::Programmatic, + }); + assert!(resolve_shutdown_decision( + app.world_mut(), + ShutdownDecision::Discard + )); + drive_shutdown(app.world_mut()); + finalize_authorized_shutdown(app.world_mut()); + assert_eq!(app.should_exit(), Some(AppExit::Success)); + assert!(app.world().resource::().has_unsaved_tabs()); + assert!(app + .world() + .resource::() + .authorized_exit_sent()); + } + + #[test] + fn file_menu_and_programmatic_clean_exit_use_the_same_coordinator_path() { + for source in [ShutdownSource::FileMenu, ShutdownSource::Programmatic] { + let mut app = App::new(); + app.add_message::() + .init_resource::() + .init_resource::() + .init_resource::(); + + assert!(request_editor_shutdown(app.world_mut(), source)); + drive_shutdown(app.world_mut()); + finalize_authorized_shutdown(app.world_mut()); + + assert_eq!(app.should_exit(), Some(AppExit::Success)); + assert_eq!( + app.world().resource::().intent(), + ShutdownIntent::ExitSent { source } + ); + } + } + + #[test] + fn save_all_emits_one_persistence_handoff() { + let mut world = confirming_world(); + let mut cursor = MessageCursor::::default(); + + assert!(resolve_shutdown_decision( + &mut world, + ShutdownDecision::SaveAll + )); + assert!(!resolve_shutdown_decision( + &mut world, + ShutdownDecision::SaveAll + )); + + let messages = world.resource::>(); + let requests: Vec<_> = cursor.read(messages).copied().collect(); + assert_eq!( + requests, + vec![ShutdownSaveAllRequested { + source: ShutdownSource::FileMenu + }] + ); + } + + #[test] + fn clean_native_close_emits_the_authorized_exit() { + let mut app = App::new(); + app.add_message::() + .add_plugins((NativeDialogPlugin, ShutdownPlugin)) + .init_resource::(); + let window = app.world_mut().spawn(PrimaryWindow).id(); + app.world_mut() + .write_message(WindowCloseRequested { window }); + + app.update(); + + assert_eq!(app.should_exit(), Some(AppExit::Success)); + assert_eq!( + app.world().resource::().intent(), + ShutdownIntent::ExitSent { + source: ShutdownSource::NativeWindow + } + ); + } + + #[test] + fn secondary_window_close_does_not_request_editor_shutdown() { + let mut app = App::new(); + app.add_message::() + .add_plugins((NativeDialogPlugin, ShutdownPlugin)) + .init_resource::(); + app.world_mut().spawn(PrimaryWindow); + let secondary = app.world_mut().spawn_empty().id(); + app.world_mut() + .write_message(WindowCloseRequested { window: secondary }); + + app.update(); + + assert_eq!(app.should_exit(), None); + assert_eq!( + app.world().resource::().intent(), + ShutdownIntent::Idle + ); + assert!(app.world().get_entity(secondary).is_err()); + } + + #[test] + fn occupied_dialog_broker_keeps_shutdown_waiting() { + let mut app = App::new(); + app.add_message::() + .add_plugins((NativeDialogPlugin, ShutdownPlugin)) + .init_resource::(); + app.world_mut().resource_mut::().mark_dirty(); + let (release_sender, release_receiver) = mpsc::channel(); + app.world() + .resource::() + .request( + move || release_receiver.recv().expect("release sender dropped"), + |_, _: ()| {}, + ) + .unwrap(); + assert!(request_editor_shutdown( + app.world_mut(), + ShutdownSource::Programmatic + )); + + app.update(); + + assert_eq!( + app.world().resource::().intent(), + ShutdownIntent::WaitingForBroker { + source: ShutdownSource::Programmatic + } + ); + release_sender.send(()).unwrap(); + } + + #[test] + fn successful_save_requires_every_scene_tab_to_be_clean() { + let mut world = World::new(); + world.init_resource::(); + world.insert_resource(ShutdownCoordinator { + intent: ShutdownIntent::Saving { + source: ShutdownSource::Programmatic, + }, + ..default() + }); + + assert!(complete_shutdown_save( + &mut world, + ShutdownSaveOutcome::Saved + )); + assert_eq!( + world.resource::().intent(), + ShutdownIntent::Authorized { + source: ShutdownSource::Programmatic, + require_clean_scenes: true, + } + ); + + world.resource_mut::().mark_dirty(); + world.resource_mut::().intent = ShutdownIntent::Saving { + source: ShutdownSource::Programmatic, + }; + assert!(!complete_shutdown_save( + &mut world, + ShutdownSaveOutcome::Saved + )); + assert_eq!( + world.resource::().intent(), + ShutdownIntent::Idle + ); + assert!(world + .resource::() + .last_blocker() + .is_some()); + } + + #[test] + fn a_new_edit_after_save_reopens_the_guard_instead_of_exiting() { + let mut world = World::new(); + world.init_resource::>(); + world.init_resource::(); + world.insert_resource(ShutdownCoordinator { + intent: ShutdownIntent::Authorized { + source: ShutdownSource::Programmatic, + require_clean_scenes: true, + }, + ..default() + }); + world.resource_mut::().mark_dirty(); + + finalize_authorized_shutdown(&mut world); + + assert_eq!( + world.resource::().intent(), + ShutdownIntent::WaitingForBroker { + source: ShutdownSource::Programmatic + } + ); + let messages = world.resource::>(); + assert_eq!( + MessageCursor::::default().read(messages).count(), + 0 + ); + } + + #[test] + fn last_schedule_edit_is_seen_before_final_exit_authorization() { + let mut app = App::new(); + app.add_message::() + .add_plugins((NativeDialogPlugin, ShutdownPlugin)) + .init_resource::() + .add_systems(Last, |mut scene_io: ResMut| { + scene_io.mark_dirty(); + }); + assert!(request_editor_shutdown( + app.world_mut(), + ShutdownSource::Programmatic + )); + + app.update(); + + assert_eq!(app.should_exit(), None); + assert!(app.world().resource::().has_unsaved_tabs()); + assert_eq!( + app.world().resource::().intent(), + ShutdownIntent::WaitingForBroker { + source: ShutdownSource::Programmatic + } + ); + } + + #[test] + fn custom_dialog_labels_map_to_shutdown_decisions() { + assert_eq!( + decision_from_dialog(rfd::MessageDialogResult::Custom(SAVE_ALL_LABEL.into())), + ShutdownDecision::SaveAll + ); + assert_eq!( + decision_from_dialog(rfd::MessageDialogResult::Custom(DISCARD_LABEL.into())), + ShutdownDecision::Discard + ); + assert_eq!( + decision_from_dialog(rfd::MessageDialogResult::Custom(CANCEL_LABEL.into())), + ShutdownDecision::Cancel + ); + } + + #[test] + fn only_the_final_coordinator_state_authorizes_clean_session_persistence() { + let mut coordinator = ShutdownCoordinator::default(); + assert!(!coordinator.authorized_exit_sent()); + + coordinator.intent = ShutdownIntent::Authorized { + source: ShutdownSource::Programmatic, + require_clean_scenes: true, + }; + assert!(!coordinator.authorized_exit_sent()); + + coordinator.intent = ShutdownIntent::ExitSent { + source: ShutdownSource::Programmatic, + }; + assert!(coordinator.authorized_exit_sent()); + } +} diff --git a/crates/editor/src/scene/scene_io.rs b/crates/editor/src/scene/scene_io.rs index b3fb3df..d37e4d7 100644 --- a/crates/editor/src/scene/scene_io.rs +++ b/crates/editor/src/scene/scene_io.rs @@ -7,7 +7,10 @@ use bevy::prelude::*; use bevy::window::PrimaryWindow; use bevy::world_serialization::serde::WorldDeserializer; use bevy::world_serialization::{DynamicWorld, DynamicWorldBuilder, WorldFilter, WorldInstance}; -use scene::{document::SceneDocument, strip_schema_version, validate_level_text}; +use scene::{ + document::{SceneDocument, SceneEntity}, + strip_schema_version, validate_level_text, +}; use serde::de::DeserializeSeed; use shared::{ infer_actor_kind, validate_actor, ActorId, ActorKind, ActorName, ActorValidationError, @@ -26,6 +29,9 @@ use crate::history::{clear_level_objects, snapshot_entity, EditorHistory}; use crate::native_dialog::NativeDialogBroker; use crate::project::collaboration::{publish_authored_file, FileSnapshot, FileWriteIntent}; use crate::project::samples::SampleCatalog; +use crate::project::shutdown::{ + complete_shutdown_save, request_project_switch, ShutdownSaveAllRequested, ShutdownSaveOutcome, +}; use crate::scene::recovery::{ default_state_root, discard_recovery_snapshots, latest_recovery_snapshot, write_recovery_snapshot, @@ -63,6 +69,7 @@ pub struct SceneTab { pub path: Option, pub dirty: bool, snapshot: String, + clean_snapshot: String, recovery_snapshot: Option, disk_snapshot: Option, } @@ -127,6 +134,7 @@ impl Default for SceneIo { path: active_path, dirty: false, snapshot: String::new(), + clean_snapshot: String::new(), recovery_snapshot: None, disk_snapshot: None, }], @@ -224,6 +232,25 @@ struct RecoveryClock { elapsed_secs: f32, } +#[derive(Resource, Debug, Default)] +struct SceneSaveAllTransaction { + start_requested: bool, + active: Option, +} + +impl SceneSaveAllTransaction { + fn is_active(&self) -> bool { + self.start_requested || self.active.is_some() + } +} + +#[derive(Debug)] +struct SaveAllRun { + original_tab_id: u64, + pending_tab_ids: VecDeque, + awaiting_destination: Option, +} + pub struct SceneIoPlugin; impl Plugin for SceneIoPlugin { @@ -231,10 +258,13 @@ impl Plugin for SceneIoPlugin { app.init_resource::() .init_resource::() .init_resource::() + .init_resource::() .add_systems( Update, ( + receive_shutdown_save_all_requests, process_scene_io_requests, + drive_save_all_transaction, tick_scene_recovery, update_window_title, ) @@ -244,6 +274,15 @@ impl Plugin for SceneIoPlugin { } } +fn receive_shutdown_save_all_requests( + mut requests: MessageReader, + mut transaction: ResMut, +) { + if requests.read().next().is_some() { + transaction.start_requested = true; + } +} + fn load_startup_scene(world: &mut World) { let active_path = world.resource::().active_path.clone(); let default_path = PathBuf::from( @@ -266,8 +305,9 @@ fn load_startup_scene(world: &mut World) { remember_path(world, path.clone()); world.resource_mut::().clear(); refresh_recovery_notice(world, &path); - if let Err(error) = capture_active_tab(world) { - warn!("Failed to capture startup scene tab: {error}"); + if let Err(error) = establish_active_clean_checkpoint(world) { + world.resource_mut::().mark_dirty(); + warn!("Failed to establish the startup scene checkpoint: {error}"); } let recovery = world.resource::().recovery_snapshot.clone(); let status = recovery.map_or_else( @@ -283,6 +323,10 @@ fn load_startup_scene(world: &mut World) { world.resource_mut::().set_status(status); } Err(err) => { + if let Err(error) = establish_active_clean_checkpoint(world) { + world.resource_mut::().mark_dirty(); + warn!("Failed to establish the generated scene checkpoint: {error}"); + } world.resource_mut::().set_status(format!( "Startup scene load failed: {err}; using generated starter arena" )); @@ -295,10 +339,12 @@ fn process_scene_io_requests(world: &mut World) { return; }; - if matches!(request, SceneIoRequest::SwitchProject) - && world.resource::().has_unsaved_tabs() + if world.resource::().is_active() + && scene_request_conflicts_with_save_all(&request) { - request_switch_project_confirmation(world); + world + .resource_mut::() + .set_status("Scene operation unavailable while Save All is in progress"); return; } @@ -310,7 +356,13 @@ fn process_scene_io_requests(world: &mut World) { SceneIoRequest::ImportAssets => import_with_dialog(world), SceneIoRequest::ExportSelection => export_selection_with_dialog(world), SceneIoRequest::SaveSelectionAsPrefab => save_selection_as_prefab(world), - SceneIoRequest::SwitchProject => switch_project(world), + SceneIoRequest::SwitchProject => { + if request_project_switch(world) { + "Preparing to switch projects".to_string() + } else { + "A guarded editor exit is already in progress".to_string() + } + } SceneIoRequest::RestoreRecovery => restore_recovery(world), SceneIoRequest::SaveRecoveryCopyAs => save_recovery_copy_with_dialog(world), SceneIoRequest::DiscardRecovery => discard_recovery(world), @@ -325,51 +377,8 @@ fn process_scene_io_requests(world: &mut World) { world.resource_mut::().set_status(status); } -fn switch_project(world: &mut World) -> String { - match crate::launcher::spawn_project_launcher_process() { - Ok(()) => { - world.write_message(AppExit::Success); - "Opening Blacksite Project Browser".to_string() - } - Err(error) => format!("Switch project failed: {error}"), - } -} - -fn request_switch_project_confirmation(world: &mut World) { - let result = world.resource::().request( - || { - rfd::MessageDialog::new() - .set_title("Unsaved Changes") - .set_description("The scene has unsaved changes. Save before continuing?") - .set_level(rfd::MessageLevel::Warning) - .set_buttons(rfd::MessageButtons::YesNoCancel) - .show() - }, - |world, decision| match decision { - rfd::MessageDialogResult::Yes => { - let status = save_all_tabs(world); - if status == "Saved all modified scene tabs" { - let status = switch_project(world); - world.resource_mut::().set_status(status); - } else { - world.resource_mut::().set_status(format!( - "Project switch paused until every scene is saved: {status}" - )); - } - } - rfd::MessageDialogResult::No => { - let status = switch_project(world); - world.resource_mut::().set_status(status); - } - _ => world - .resource_mut::() - .set_status("Project switch cancelled"), - }, - ); - world.resource_mut::().set_status(match result { - Ok(()) => "Waiting for unsaved-scene confirmation".into(), - Err(error) => format!("Project switch unavailable: {error}"), - }); +fn scene_request_conflicts_with_save_all(request: &SceneIoRequest) -> bool { + !matches!(request, SceneIoRequest::SwitchProject) } fn open_recent(world: &mut World, index: usize) -> String { @@ -461,6 +470,7 @@ fn open_path(world: &mut World, path: PathBuf) -> String { path: Some(path.clone()), dirty: false, snapshot: String::new(), + clean_snapshot: String::new(), recovery_snapshot: None, disk_snapshot: Some(disk_snapshot), }); @@ -472,8 +482,9 @@ fn open_path(world: &mut World, path: PathBuf) -> String { remember_path(world, path.clone()); world.resource_mut::().clear(); refresh_recovery_notice(world, &path); - if let Err(error) = capture_active_tab(world) { - warn!("Failed to capture opened scene tab: {error}"); + if let Err(error) = establish_active_clean_checkpoint(world) { + world.resource_mut::().mark_dirty(); + warn!("Failed to establish the opened scene checkpoint: {error}"); } format!("Loading {}", path.display()) } @@ -498,6 +509,7 @@ fn new_scene_tab(world: &mut World) -> String { path: None, dirty: false, snapshot: String::new(), + clean_snapshot: String::new(), recovery_snapshot: None, disk_snapshot: None, }); @@ -507,7 +519,8 @@ fn new_scene_tab(world: &mut World) -> String { io.mark_clean(); } world.resource_mut::().clear(); - if let Err(error) = capture_active_tab(world) { + if let Err(error) = establish_active_clean_checkpoint(world) { + world.resource_mut::().mark_dirty(); return format!("New scene failed: {error}"); } "Created a new empty scene tab".to_string() @@ -539,6 +552,101 @@ fn capture_active_tab(world: &mut World) -> Result<(), String> { Ok(()) } +const CHILD_OF_COMPONENT: &str = "bevy_ecs::hierarchy::ChildOf"; + +#[derive(serde::Serialize)] +struct CanonicalSceneCheckpoint { + schema_version: u32, + composition: Option, + resource_types: Vec, + entities: Vec, +} + +fn canonical_scene_checkpoint(text: &str) -> Result { + let document = SceneDocument::from_ron_text(text)?; + let mut resource_types = document.resource_types; + resource_types.sort(); + resource_types.dedup(); + + let mut entities = document.entities; + for entity in &mut entities { + entity + .components + .retain(|component| component.type_name != CHILD_OF_COMPONENT); + entity.components.sort_by(|left, right| { + left.type_name + .cmp(&right.type_name) + .then_with(|| left.ron.cmp(&right.ron)) + }); + } + entities.sort_by(|left, right| { + left.document_id + .cmp(&right.document_id) + .then_with(|| left.actor_id.cmp(&right.actor_id)) + }); + + ron::ser::to_string(&CanonicalSceneCheckpoint { + schema_version: document.schema_version, + composition: document.composition, + resource_types, + entities, + }) + .map_err(|error| format!("could not encode clean scene checkpoint: {error}")) +} + +fn establish_active_clean_checkpoint(world: &mut World) -> Result<(), String> { + capture_active_tab(world)?; + let (active_tab, snapshot) = { + let io = world.resource::(); + let tab = io + .tabs + .get(io.active_tab) + .ok_or_else(|| format!("active scene tab {} is missing", io.active_tab))?; + (io.active_tab, tab.snapshot.clone()) + }; + let clean_snapshot = canonical_scene_checkpoint(&snapshot)?; + { + let mut io = world.resource_mut::(); + let tab = io + .tabs + .get_mut(active_tab) + .ok_or_else(|| format!("active scene tab {active_tab} is missing"))?; + tab.clean_snapshot = clean_snapshot; + } + world.resource_mut::().mark_clean(); + Ok(()) +} + +pub(crate) fn reconcile_active_dirty_with_checkpoint(world: &mut World) { + let clean_snapshot = { + let Some(io) = world.get_resource::() else { + return; + }; + io.tabs + .get(io.active_tab) + .map(|tab| tab.clean_snapshot.clone()) + .unwrap_or_default() + }; + if clean_snapshot.is_empty() { + world.resource_mut::().mark_dirty(); + return; + } + + let current = serialize_active_scene(world).and_then(|text| canonical_scene_checkpoint(&text)); + match current { + Ok(current) if current == clean_snapshot => { + world.resource_mut::().mark_clean(); + } + Ok(_) => { + world.resource_mut::().mark_dirty(); + } + Err(error) => { + warn!("Could not reconcile scene dirty state with its clean checkpoint: {error}"); + world.resource_mut::().mark_dirty(); + } + } +} + fn switch_scene_tab(world: &mut World, index: usize) -> String { let current = world.resource::().active_tab; if index == current { @@ -603,6 +711,7 @@ fn finish_close_scene_tab(world: &mut World) -> String { path: None, dirty: false, snapshot: String::new(), + clean_snapshot: String::new(), recovery_snapshot: None, disk_snapshot: None, }]; @@ -612,7 +721,10 @@ fn finish_close_scene_tab(world: &mut World) -> String { io.mark_clean(); } world.resource_mut::().clear(); - let _ = capture_active_tab(world); + if let Err(error) = establish_active_clean_checkpoint(world) { + world.resource_mut::().mark_dirty(); + return format!("Closed scene, but the empty scene checkpoint failed: {error}"); + } return "Closed scene; created an empty scene tab".to_string(); } @@ -695,46 +807,296 @@ fn request_close_scene_confirmation(world: &mut World) { } } -fn save_all_tabs(world: &mut World) -> String { - let original_id = world.resource::().tabs[world.resource::().active_tab].id; - let dirty_ids: Vec = world +fn drive_save_all_transaction(world: &mut World) { + let should_start = { + let mut transaction = world.resource_mut::(); + if transaction.active.is_some() { + transaction.start_requested = false; + false + } else { + std::mem::take(&mut transaction.start_requested) + } + }; + if !should_start { + return; + } + + let Some(original_tab_id) = world + .resource::() + .tabs + .get(world.resource::().active_tab) + .map(|tab| tab.id) + else { + finish_save_all_without_run( + world, + ShutdownSaveOutcome::Failed("the active scene tab is missing".into()), + ); + return; + }; + world.resource_mut::().active = Some(SaveAllRun { + original_tab_id, + pending_tab_ids: VecDeque::new(), + awaiting_destination: None, + }); + + if let Err(error) = capture_active_tab(world) { + finish_save_all_transaction( + world, + ShutdownSaveOutcome::Failed(format!( + "could not preserve the active scene before Save All: {error}" + )), + ); + return; + } + queue_current_dirty_tabs(world); + advance_save_all_transaction(world); +} + +fn queue_current_dirty_tabs(world: &mut World) { + let dirty_ids: VecDeque = world .resource::() .tabs .iter() .filter(|tab| tab.dirty) .map(|tab| tab.id) .collect(); - for id in dirty_ids { - let Some(index) = world + if let Some(run) = world + .resource_mut::() + .active + .as_mut() + { + run.pending_tab_ids = dirty_ids; + } +} + +fn advance_save_all_transaction(world: &mut World) { + loop { + let Some((awaiting_destination, next_tab_id)) = world + .resource::() + .active + .as_ref() + .map(|run| { + ( + run.awaiting_destination, + run.pending_tab_ids.front().copied(), + ) + }) + else { + return; + }; + if awaiting_destination.is_some() { + return; + } + + let Some(tab_id) = next_tab_id else { + if let Err(error) = capture_active_tab(world) { + finish_save_all_transaction( + world, + ShutdownSaveOutcome::Failed(format!( + "could not verify scene state after Save All: {error}" + )), + ); + return; + } + queue_current_dirty_tabs(world); + let still_dirty = world + .resource::() + .active + .as_ref() + .is_some_and(|run| !run.pending_tab_ids.is_empty()); + if still_dirty { + continue; + } + finish_save_all_transaction(world, ShutdownSaveOutcome::Saved); + return; + }; + + let Some((index, dirty, path, label)) = world .resource::() .tabs .iter() - .position(|tab| tab.id == id) + .enumerate() + .find(|(_, tab)| tab.id == tab_id) + .map(|(index, tab)| (index, tab.dirty, tab.path.clone(), tab.label())) else { - continue; + finish_save_all_transaction( + world, + ShutdownSaveOutcome::Failed(format!( + "scene tab {tab_id} disappeared during Save All" + )), + ); + return; }; - if index != world.resource::().active_tab { - let status = switch_scene_tab(world, index); - if status.starts_with("Scene switch failed") { - return format!("Save failed: {status}"); + if !dirty { + pop_save_all_tab(world, tab_id); + continue; + } + if let Err(error) = activate_scene_tab_index(world, index, tab_id) { + finish_save_all_transaction(world, ShutdownSaveOutcome::Failed(error)); + return; + } + + if path.is_some() { + match save_active_existing(world) { + Ok(_) => { + pop_save_all_tab(world, tab_id); + continue; + } + Err(error) => { + finish_save_all_transaction(world, ShutdownSaveOutcome::Failed(error)); + return; + } } } - let status = save_active_or_prompt(world); - if status.starts_with("Save failed") || status.ends_with("cancelled") { - return status; + + if let Some(run) = world + .resource_mut::() + .active + .as_mut() + { + run.awaiting_destination = Some(tab_id); + } + let request = world.resource::().request( + || { + rfd::FileDialog::new() + .set_directory("assets/levels") + .add_filter("Bevy scene", &["scn.ron", "ron"]) + .set_file_name("editor_scene.scn.ron") + .save_file() + }, + move |world, path| finish_save_all_destination(world, tab_id, path), + ); + match request { + Ok(()) => world + .resource_mut::() + .set_status(format!("Save All: choose a destination for {label}")), + Err(error) => finish_save_all_transaction( + world, + ShutdownSaveOutcome::Failed(format!( + "could not choose a destination for {label}: {error}" + )), + ), + } + return; + } +} + +fn finish_save_all_destination(world: &mut World, tab_id: u64, path: Option) { + let awaiting = world + .resource::() + .active + .as_ref() + .and_then(|run| run.awaiting_destination); + if awaiting != Some(tab_id) { + finish_save_all_transaction( + world, + ShutdownSaveOutcome::Failed(format!( + "Save All received a stale destination for scene tab {tab_id}" + )), + ); + return; + } + + let Some(path) = path else { + finish_save_all_transaction(world, ShutdownSaveOutcome::Cancelled); + return; + }; + let result = activate_scene_tab_by_id(world, tab_id) + .and_then(|_| save_active_as(world, path).map(|_| ())); + match result { + Ok(()) => { + if let Some(run) = world + .resource_mut::() + .active + .as_mut() + { + run.awaiting_destination = None; + } + pop_save_all_tab(world, tab_id); + advance_save_all_transaction(world); + } + Err(error) => { + finish_save_all_transaction(world, ShutdownSaveOutcome::Failed(error)); } } - if let Some(index) = world +} + +fn pop_save_all_tab(world: &mut World, expected_tab_id: u64) { + let mut transaction = world.resource_mut::(); + let Some(run) = transaction.active.as_mut() else { + return; + }; + if run.pending_tab_ids.front() == Some(&expected_tab_id) { + run.pending_tab_ids.pop_front(); + } +} + +fn activate_scene_tab_by_id(world: &mut World, tab_id: u64) -> Result<(), String> { + let index = world .resource::() .tabs .iter() - .position(|tab| tab.id == original_id) - { - if index != world.resource::().active_tab { - let _ = switch_scene_tab(world, index); - } + .position(|tab| tab.id == tab_id) + .ok_or_else(|| format!("scene tab {tab_id} disappeared during Save All"))?; + activate_scene_tab_index(world, index, tab_id) +} + +fn activate_scene_tab_index( + world: &mut World, + index: usize, + expected_tab_id: u64, +) -> Result<(), String> { + if world.resource::().active_tab == index { + return Ok(()); } - "Saved all modified scene tabs".to_string() + let status = switch_scene_tab(world, index); + let active_id = world + .resource::() + .tabs + .get(world.resource::().active_tab) + .map(|tab| tab.id); + if active_id == Some(expected_tab_id) { + Ok(()) + } else { + Err(format!( + "could not activate scene tab {expected_tab_id}: {status}" + )) + } +} + +fn finish_save_all_transaction(world: &mut World, outcome: ShutdownSaveOutcome) { + let Some(run) = world + .resource_mut::() + .active + .take() + else { + finish_save_all_without_run(world, outcome); + return; + }; + let outcome = match activate_scene_tab_by_id(world, run.original_tab_id) { + Ok(()) => outcome, + Err(restore_error) => { + let detail = match outcome { + ShutdownSaveOutcome::Saved => "all saves completed".to_string(), + ShutdownSaveOutcome::Cancelled => "saving was cancelled".to_string(), + ShutdownSaveOutcome::Failed(error) => error, + }; + ShutdownSaveOutcome::Failed(format!( + "{detail}; could not restore the original scene tab: {restore_error}" + )) + } + }; + finish_save_all_without_run(world, outcome); +} + +fn finish_save_all_without_run(world: &mut World, outcome: ShutdownSaveOutcome) { + let status = match &outcome { + ShutdownSaveOutcome::Saved => "Saved all modified scene tabs".to_string(), + ShutdownSaveOutcome::Cancelled => "Save All cancelled; editor remains open".to_string(), + ShutdownSaveOutcome::Failed(error) => format!("Save All failed: {error}"), + }; + complete_shutdown_save(world, outcome); + world.resource_mut::().set_status(status); } fn save_selection_as_prefab(world: &mut World) -> String { @@ -924,19 +1286,32 @@ pub fn save_active_or_prompt_world(world: &mut World) -> String { } fn save_active_or_prompt(world: &mut World) -> String { - let path = world.resource::().active_path.clone(); - match path { - Some(path) => match save_level(world, &path, SceneWriteContext::Active) { - Ok(count) => { - world.resource_mut::().mark_clean(); - remember_path(world, path.clone()); - retire_scene_recovery(world, &path); - format!("Saved {count} level entities to {}", path.display()) - } - Err(err) => format!("Save failed: {err}"), - }, - None => save_with_dialog(world), + if world.resource::().active_path.is_none() { + return save_with_dialog(world); } + match save_active_existing(world) { + Ok((count, path)) => format!("Saved {count} level entities to {}", path.display()), + Err(error) => format!("Save failed: {error}"), + } +} + +fn save_active_existing(world: &mut World) -> Result<(usize, PathBuf), String> { + let path = world + .resource::() + .active_path + .clone() + .ok_or_else(|| "the active scene has no destination".to_string())?; + let count = save_level(world, &path, SceneWriteContext::Active)?; + remember_path(world, path.clone()); + retire_scene_recovery(world, &path); + establish_active_clean_checkpoint(world).map_err(|error| { + world.resource_mut::().mark_dirty(); + format!( + "save completed, but the clean checkpoint failed for {}: {error}", + path.display() + ) + })?; + Ok((count, path)) } fn save_with_dialog(world: &mut World) -> String { @@ -963,23 +1338,57 @@ fn finish_save_with_dialog(world: &mut World, path: Option) -> String { let Some(path) = path else { return "Save cancelled".to_string(); }; - let expected = match FileSnapshot::capture(&path) { - Ok(expected) => expected, - Err(error) => return format!("Save failed: {error}"), - }; - - match save_level(world, &path, SceneWriteContext::ActiveSaveAs { expected }) { - Ok(count) => { - world.resource_mut::().active_path = Some(path.clone()); - world.resource_mut::().mark_clean(); - remember_path(world, path.clone()); - retire_scene_recovery(world, &path); - format!("Saved {count} level entities to {}", path.display()) - } - Err(err) => format!("Save failed: {err}"), + match save_active_as(world, path) { + Ok((count, path)) => format!("Saved {count} level entities to {}", path.display()), + Err(error) => format!("Save failed: {error}"), } } +fn save_active_as(world: &mut World, path: PathBuf) -> Result<(usize, PathBuf), String> { + ensure_save_as_destination_is_distinct(world, &path)?; + let expected = FileSnapshot::capture(&path)?; + let count = save_level(world, &path, SceneWriteContext::ActiveSaveAs { expected })?; + world.resource_mut::().active_path = Some(path.clone()); + remember_path(world, path.clone()); + retire_scene_recovery(world, &path); + establish_active_clean_checkpoint(world).map_err(|error| { + world.resource_mut::().mark_dirty(); + format!( + "save completed, but the clean checkpoint failed for {}: {error}", + path.display() + ) + })?; + Ok((count, path)) +} + +fn ensure_save_as_destination_is_distinct(world: &World, path: &Path) -> Result<(), String> { + let io = world.resource::(); + let active_id = io + .tabs + .get(io.active_tab) + .map(|tab| tab.id) + .ok_or_else(|| "the active scene tab is missing".to_string())?; + let configured_root = world + .get_resource::() + .map(|workspace| PathBuf::from(&workspace.root)) + .unwrap_or_else(|| PathBuf::from(".")); + let project_root = configured_root.canonicalize().unwrap_or(configured_root); + if let Some(owner) = io.tabs.iter().find(|tab| { + tab.id != active_id + && tab + .path + .as_deref() + .is_some_and(|candidate| scene_paths_share_identity(&project_root, candidate, path)) + }) { + return Err(format!( + "{} is already open in scene tab {}", + path.display(), + owner.label() + )); + } + Ok(()) +} + fn open_with_dialog(world: &mut World) -> String { let request = world.resource::().request( || { @@ -1875,12 +2284,13 @@ pub(crate) fn reload_scene_after_file_conflict( io.recovery_snapshot = None; let active_tab = io.active_tab; io.tabs[active_tab].disk_snapshot = Some(disk_snapshot); - io.mark_clean(); } world.resource_mut::().clear(); remember_path(world, path.to_path_buf()); refresh_recovery_notice(world, path); - capture_active_tab(world)?; + establish_active_clean_checkpoint(world).inspect_err(|_| { + world.resource_mut::().mark_dirty(); + })?; Ok(format!("Reloaded {} from disk", path.display())) } @@ -1906,10 +2316,12 @@ pub(crate) fn adopt_scene_conflict_save_as( io.active_path = Some(path.clone()); let active_tab = io.active_tab; io.tabs[active_tab].disk_snapshot = Some(disk_snapshot); - io.mark_clean(); } remember_path(world, path.clone()); retire_scene_recovery(world, &path); + establish_active_clean_checkpoint(world).inspect_err(|_| { + world.resource_mut::().mark_dirty(); + })?; Ok(format!( "Saved {entity_count} level entities to {}", path.display() @@ -2294,6 +2706,302 @@ mod tests { } } + fn checkpoint_test_app() -> App { + let mut app = App::new(); + app.register_type::() + .register_type::() + .register_type::() + .register_type::() + .register_type::() + .register_type::() + .register_type::() + .register_type::() + .register_type::(); + app.init_resource::() + .init_resource::() + .init_resource::(); + app.insert_resource(SceneComposition { + scene_id: "checkpoint-test-scene".to_string(), + subscenes: Vec::new(), + }); + app + } + + fn spawn_checkpoint_actor(app: &mut App, actor_id: &str) -> Entity { + app.world_mut() + .spawn(( + LevelObject, + ActorId::new(actor_id), + ActorKind::Empty, + Name::new("Checkpoint Actor"), + Transform::IDENTITY, + HierarchySiblingIndex(0), + EditorVisibility::default(), + )) + .id() + } + + fn arm_shutdown_save(world: &mut World) { + world.init_resource::(); + world + .resource_mut::() + .set_intent_for_test(crate::shutdown::ShutdownIntent::Saving { + source: crate::shutdown::ShutdownSource::Programmatic, + }); + } + + fn prepare_saved_dirty_checkpoint_scene(app: &mut App, root: &Path) -> PathBuf { + let path = root.join("assets/levels/save-all.scn.ron"); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + app.insert_resource(crate::project_io::ProjectWorkspace { + root: root.display().to_string(), + ..default() + }); + spawn_checkpoint_actor(app, "save-all-actor"); + let text = serialize_active_scene(app.world_mut()).unwrap(); + std::fs::write(&path, text).unwrap(); + let disk_snapshot = FileSnapshot::capture(&path).unwrap(); + { + let mut io = app.world_mut().resource_mut::(); + io.active_path = Some(path.clone()); + io.tabs[0].path = Some(path.clone()); + io.tabs[0].disk_snapshot = Some(disk_snapshot); + io.mark_dirty(); + } + app.world_mut().init_resource::(); + app.world_mut() + .resource_mut::() + .start_requested = true; + arm_shutdown_save(app.world_mut()); + path + } + + #[test] + fn save_all_saved_scene_finishes_clean_without_a_dialog() { + let root = + std::env::temp_dir().join(format!("blacksite-save-all-saved-{}", uuid::Uuid::new_v4())); + let mut app = checkpoint_test_app(); + let original_id = app.world().resource::().tabs[0].id; + let path = prepare_saved_dirty_checkpoint_scene(&mut app, &root); + + drive_save_all_transaction(app.world_mut()); + + let io = app.world().resource::(); + assert!(!io.dirty); + assert!(!io.tabs[0].dirty); + assert_eq!(io.tabs[io.active_tab].id, original_id); + assert_eq!(io.status, "Saved all modified scene tabs"); + assert!(app + .world() + .resource::() + .active + .is_none()); + assert_eq!( + app.world() + .resource::() + .intent(), + crate::shutdown::ShutdownIntent::Authorized { + source: crate::shutdown::ShutdownSource::Programmatic, + require_clean_scenes: true, + } + ); + assert!(path.exists()); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn save_all_two_saved_tabs_uses_stable_ids_and_restores_original_tab() { + let root = std::env::temp_dir().join(format!( + "blacksite-save-all-two-tabs-{}", + uuid::Uuid::new_v4() + )); + let levels = root.join("assets/levels"); + std::fs::create_dir_all(&levels).unwrap(); + let root = root.canonicalize().unwrap(); + let first_path = levels.join("first.scn.ron"); + let second_path = levels.join("second.scn.ron"); + + let mut app = App::new(); + app.add_plugins(MinimalPlugins) + .add_plugins((AssetPlugin::default(), WorldSerializationPlugin)) + .add_plugins(shared::SharedTypesPlugin); + app.init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .insert_resource(settings::ProjectSettings::default()) + .insert_resource(crate::project_io::ProjectWorkspace { + root: root.display().to_string(), + ..default() + }) + .insert_resource(SceneComposition { + scene_id: "save-all-two-tabs".into(), + subscenes: Vec::new(), + }); + let text = serialize_active_scene(app.world_mut()).unwrap(); + std::fs::write(&first_path, &text).unwrap(); + std::fs::write(&second_path, &text).unwrap(); + let clean_snapshot = canonical_scene_checkpoint(&text).unwrap(); + let first_id = 41; + let second_id = 93; + { + let mut io = app.world_mut().resource_mut::(); + io.active_path = Some(first_path.clone()); + io.dirty = true; + io.tabs = vec![ + SceneTab { + id: first_id, + path: Some(first_path.clone()), + dirty: true, + snapshot: text.clone(), + clean_snapshot: clean_snapshot.clone(), + recovery_snapshot: None, + disk_snapshot: Some(FileSnapshot::capture(&first_path).unwrap()), + }, + SceneTab { + id: second_id, + path: Some(second_path.clone()), + dirty: true, + snapshot: text, + clean_snapshot, + recovery_snapshot: None, + disk_snapshot: Some(FileSnapshot::capture(&second_path).unwrap()), + }, + ]; + io.active_tab = 0; + } + app.world_mut() + .resource_mut::() + .start_requested = true; + arm_shutdown_save(app.world_mut()); + + drive_save_all_transaction(app.world_mut()); + + let io = app.world().resource::(); + assert_eq!( + io.tabs.iter().map(|tab| tab.id).collect::>(), + [first_id, second_id] + ); + assert_eq!(io.tabs[io.active_tab].id, first_id); + assert!(io.tabs.iter().all(|tab| !tab.dirty)); + assert!(!io.dirty); + assert_eq!(io.status, "Saved all modified scene tabs"); + assert!(app + .world() + .resource::() + .active + .is_none()); + assert_eq!( + app.world() + .resource::() + .intent(), + crate::shutdown::ShutdownIntent::Authorized { + source: crate::shutdown::ShutdownSource::Programmatic, + require_clean_scenes: true, + } + ); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn save_all_external_revision_failure_keeps_scene_dirty_and_disk_untouched() { + let root = std::env::temp_dir().join(format!( + "blacksite-save-all-conflict-{}", + uuid::Uuid::new_v4() + )); + let mut app = checkpoint_test_app(); + let original_id = app.world().resource::().tabs[0].id; + let path = prepare_saved_dirty_checkpoint_scene(&mut app, &root); + std::fs::write(&path, b"external revision").unwrap(); + + drive_save_all_transaction(app.world_mut()); + + let io = app.world().resource::(); + assert!(io.dirty); + assert!(io.tabs[0].dirty); + assert_eq!(io.tabs[io.active_tab].id, original_id); + assert!(io.status.contains("changed outside Blacksite")); + assert_eq!(std::fs::read(&path).unwrap(), b"external revision"); + assert!(app + .world() + .resource::() + .active + .is_none()); + let shutdown = app + .world() + .resource::(); + assert_eq!(shutdown.intent(), crate::shutdown::ShutdownIntent::Idle); + assert!(shutdown + .last_blocker() + .is_some_and(|blocker| blocker.contains("changed outside Blacksite"))); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn cancelling_untitled_save_all_preserves_dirty_tab_and_ends_transaction() { + let mut world = World::new(); + world.init_resource::(); + world.init_resource::(); + arm_shutdown_save(&mut world); + world.resource_mut::().mark_dirty(); + world.resource_mut::().active = Some(SaveAllRun { + original_tab_id: 1, + pending_tab_ids: VecDeque::from([1]), + awaiting_destination: Some(1), + }); + + finish_save_all_destination(&mut world, 1, None); + + assert!(world.resource::().dirty); + assert!(world.resource::().tabs[0].dirty); + assert!(world.resource::().active.is_none()); + assert_eq!( + world.resource::().status, + "Save All cancelled; editor remains open" + ); + assert_eq!( + world + .resource::() + .intent(), + crate::shutdown::ShutdownIntent::Idle + ); + } + + #[test] + fn save_as_rejects_a_destination_owned_by_another_tab() { + let path = PathBuf::from("assets/levels/already-open.scn.ron"); + let mut world = World::new(); + world.init_resource::(); + world.resource_mut::().tabs.push(SceneTab { + id: 2, + path: Some(path.clone()), + dirty: false, + snapshot: "snapshot".into(), + clean_snapshot: "snapshot".into(), + recovery_snapshot: None, + disk_snapshot: None, + }); + + let error = ensure_save_as_destination_is_distinct(&world, &path).unwrap_err(); + + assert!(error.contains("already open")); + assert!(error.contains("already-open.scn.ron")); + } + + #[test] + fn save_all_blocks_scene_io_that_can_replace_transaction_tabs() { + assert!(scene_request_conflicts_with_save_all(&SceneIoRequest::New)); + assert!(scene_request_conflicts_with_save_all( + &SceneIoRequest::SwitchTab(1) + )); + assert!(scene_request_conflicts_with_save_all( + &SceneIoRequest::CloseTab(0) + )); + assert!(!scene_request_conflicts_with_save_all( + &SceneIoRequest::SwitchProject + )); + } + #[test] fn registry_registered_extension_is_included_in_scene_persistence() { let mut app = App::new(); @@ -2373,6 +3081,7 @@ mod tests { path: Some(PathBuf::from("assets/levels/lighting.scn.ron")), dirty: true, snapshot: "dirty inactive tab".to_string(), + clean_snapshot: String::new(), recovery_snapshot: None, disk_snapshot: None, }); @@ -2389,6 +3098,154 @@ mod tests { ); } + #[test] + fn undo_to_clean_checkpoint_clears_dirty_and_redo_marks_dirty() { + let mut app = checkpoint_test_app(); + let entity = spawn_checkpoint_actor(&mut app, "checkpoint-actor"); + let world = app.world_mut(); + establish_active_clean_checkpoint(world).unwrap(); + + crate::history::set_transform_with_history( + world, + entity, + Transform::IDENTITY, + Transform::from_xyz(3.0, 1.0, -2.0), + ); + assert!(world.resource::().dirty); + + crate::history::apply_command_undo(world); + assert!(!world.resource::().dirty); + + crate::history::apply_command_redo(world); + assert!(world.resource::().dirty); + } + + #[test] + fn clean_checkpoint_at_nonzero_history_depth_survives_redo_but_not_a_branch() { + let mut app = checkpoint_test_app(); + let entity = spawn_checkpoint_actor(&mut app, "checkpoint-actor"); + let world = app.world_mut(); + establish_active_clean_checkpoint(world).unwrap(); + + let saved = Transform::from_xyz(2.0, 0.0, 0.0); + crate::history::set_transform_with_history(world, entity, Transform::IDENTITY, saved); + assert_eq!(world.resource::().undo_depth(), 1); + establish_active_clean_checkpoint(world).unwrap(); + assert!(!world.resource::().dirty); + + crate::history::apply_command_undo(world); + assert!(world.resource::().dirty); + crate::history::apply_command_redo(world); + assert!(!world.resource::().dirty); + + crate::history::apply_command_undo(world); + let branched = Transform::from_xyz(-4.0, 0.0, 0.0); + crate::history::set_transform_with_history(world, entity, Transform::IDENTITY, branched); + assert_eq!(world.resource::().undo_depth(), 1); + assert!(!world.resource::().can_redo()); + assert!(world.resource::().dirty); + + crate::history::apply_command_undo(world); + assert!( + world.resource::().dirty, + "the unreachable saved checkpoint must not be inferred from history depth" + ); + } + + #[test] + fn undo_does_not_clear_an_independent_direct_authored_edit() { + let mut app = checkpoint_test_app(); + let entity = spawn_checkpoint_actor(&mut app, "checkpoint-actor"); + let world = app.world_mut(); + establish_active_clean_checkpoint(world).unwrap(); + + crate::history::set_transform_with_history( + world, + entity, + Transform::IDENTITY, + Transform::from_xyz(1.0, 0.0, 0.0), + ); + world.entity_mut(entity).insert(Name::new("Direct edit")); + world.resource_mut::().mark_dirty(); + + crate::history::apply_command_undo(world); + + assert_eq!( + world.get::(entity).map(Name::as_str), + Some("Direct edit") + ); + assert!(world.resource::().dirty); + } + + #[test] + fn delete_undo_matches_checkpoint_across_runtime_entity_key_changes() { + let mut app = checkpoint_test_app(); + let entity = spawn_checkpoint_actor(&mut app, "checkpoint-actor"); + let original_bits = entity.to_bits(); + let world = app.world_mut(); + establish_active_clean_checkpoint(world).unwrap(); + + crate::history::delete_entities_with_history(world, &[entity]); + assert!(world.resource::().dirty); + crate::history::apply_command_undo(world); + + let restored = world + .query::<(Entity, &ActorId)>() + .iter(world) + .find_map(|(entity, actor_id)| (actor_id.0 == "checkpoint-actor").then_some(entity)) + .expect("undo should restore the deleted actor"); + assert_ne!(restored.to_bits(), original_bits); + assert!(!world.resource::().dirty); + } + + #[test] + fn clean_checkpoints_are_independent_per_scene_tab() { + let mut app = checkpoint_test_app(); + let entity = spawn_checkpoint_actor(&mut app, "checkpoint-actor"); + let world = app.world_mut(); + establish_active_clean_checkpoint(world).unwrap(); + let first_checkpoint = world.resource::().tabs[0].clean_snapshot.clone(); + + world + .entity_mut(entity) + .insert(Transform::from_xyz(5.0, 0.0, 0.0)); + let second_snapshot = serialize_active_scene(world).unwrap(); + let second_checkpoint = canonical_scene_checkpoint(&second_snapshot).unwrap(); + { + let mut io = world.resource_mut::(); + io.tabs.push(SceneTab { + id: 2, + path: None, + dirty: false, + snapshot: second_snapshot, + clean_snapshot: second_checkpoint, + recovery_snapshot: None, + disk_snapshot: None, + }); + io.active_tab = 1; + io.dirty = false; + } + + world + .entity_mut(entity) + .insert(Transform::from_xyz(7.0, 0.0, 0.0)); + reconcile_active_dirty_with_checkpoint(world); + assert!(world.resource::().tabs[1].dirty); + assert!(!world.resource::().tabs[0].dirty); + + world.entity_mut(entity).insert(Transform::IDENTITY); + { + let mut io = world.resource_mut::(); + io.active_tab = 0; + io.dirty = true; + } + reconcile_active_dirty_with_checkpoint(world); + let io = world.resource::(); + assert!(!io.tabs[0].dirty); + assert!(io.tabs[1].dirty); + assert_eq!(io.tabs[0].clean_snapshot, first_checkpoint); + } + #[test] fn active_scene_write_refuses_an_external_revision() { let root = std::env::temp_dir().join(format!( diff --git a/crates/editor/src/ui/menu.rs b/crates/editor/src/ui/menu.rs index efa409d..8724d7c 100644 --- a/crates/editor/src/ui/menu.rs +++ b/crates/editor/src/ui/menu.rs @@ -9,6 +9,7 @@ use crate::history::{apply_command_redo, apply_command_undo, EditorHistory}; use crate::project::samples::{SampleCatalog, SampleCatalogEntry}; use crate::scene_io::{SceneIo, SceneIoRequest}; use crate::settings_ui::{open_project_settings_panel, ProjectSettingsPanel}; +use crate::shutdown::{request_editor_shutdown, ShutdownSource}; use crate::state::{EditorMode, PlayPaused}; use super::diagnostics::{BrushDiagnosticsPanel, DiagnosticsPanel}; @@ -148,6 +149,11 @@ pub fn top_menu_bar( } }); } + ui.separator(); + if menu_item(ui, "Quit", None, true).clicked() { + request_editor_shutdown(world, ShutdownSource::FileMenu); + ui.close(); + } }); ui.menu_button("Edit", |ui| { diff --git a/crates/game/src/launch.rs b/crates/game/src/launch.rs index 432e455..84d2224 100644 --- a/crates/game/src/launch.rs +++ b/crates/game/src/launch.rs @@ -6,7 +6,7 @@ use bevy::app::PluginGroupBuilder; use bevy::asset::AssetPlugin; use bevy::log::{LogPlugin, DEFAULT_FILTER}; use bevy::prelude::*; -use bevy::window::{CompositeAlphaMode, PresentMode, WindowMode, WindowPlugin}; +use bevy::window::{CompositeAlphaMode, ExitCondition, PresentMode, WindowMode, WindowPlugin}; /// Runtime switch for the HDR camera pass. /// @@ -40,6 +40,27 @@ fn find_assets_directory(start: Option) -> Option { } pub fn default_plugins(title: impl Into) -> PluginGroupBuilder { + configured_plugins(WindowPlugin { + primary_window: Some(primary_window(title)), + ..default() + }) +} + +/// Default plugins for the editor, whose shutdown coordinator owns window-close handling. +pub fn editor_plugins(title: impl Into) -> PluginGroupBuilder { + configured_plugins(editor_window_plugin(title)) +} + +fn editor_window_plugin(title: impl Into) -> WindowPlugin { + WindowPlugin { + primary_window: Some(primary_window(title)), + exit_condition: ExitCondition::DontExit, + close_when_requested: false, + ..default() + } +} + +fn configured_plugins(window_plugin: WindowPlugin) -> PluginGroupBuilder { DefaultPlugins .set(LogPlugin { // Blacksite intentionally replaces Bevy's `.scn.ron` loader with a @@ -48,10 +69,7 @@ pub fn default_plugins(title: impl Into) -> PluginGroupBuilder { filter: format!("{DEFAULT_FILTER}bevy_asset::server::loaders=error"), ..default() }) - .set(WindowPlugin { - primary_window: Some(primary_window(title)), - ..default() - }) + .set(window_plugin) .set(AssetPlugin { file_path: resolve_assets_directory(), ..default() @@ -86,8 +104,8 @@ pub fn hdr_enabled_from_env() -> bool { #[cfg(test)] mod tests { - use super::{primary_window, resolve_assets_directory}; - use bevy::window::CompositeAlphaMode; + use super::{editor_window_plugin, primary_window, resolve_assets_directory}; + use bevy::window::{CompositeAlphaMode, ExitCondition}; #[test] fn primary_window_is_opaque() { @@ -97,6 +115,14 @@ mod tests { assert_eq!(window.composite_alpha_mode, CompositeAlphaMode::Opaque); } + #[test] + fn editor_window_defers_close_and_exit_to_the_editor() { + let plugin = editor_window_plugin("test"); + + assert!(!plugin.close_when_requested); + assert!(matches!(plugin.exit_condition, ExitCondition::DontExit)); + } + #[test] fn resolve_assets_directory_finds_workspace_assets() { let cwd = std::env::current_dir().expect("cwd"); diff --git a/docs/README.md b/docs/README.md index c430e71..4a5291b 100644 --- a/docs/README.md +++ b/docs/README.md @@ -56,6 +56,7 @@ Immutable-style log of significant decisions. Add a new numbered ADR when changi | [0039](adr/0039-inline-height-grid-terrain-foundation.md) | Inline authored height grids with deterministic runtime-only chunk hydration | | [0040](adr/0040-terrain-material-layer-weights.md) | Four-channel terrain material layers, compact normalized weights, and raster transport | | [0041](adr/0041-transactional-editor-physics-placement.md) | Paused editor physics ownership and transactional gravity placement | +| [0042](adr/0042-guarded-editor-shutdown-and-document-savepoints.md) | Guarded native editor exit and canonical per-document clean checkpoints | ## Editor framework @@ -119,6 +120,7 @@ Detailed milestone and feature plans live in [`.cursor/plans/`](../.cursor/plans | `terrain_material_layers_*.plan.md` | Terrain shared-material layers, normalized weights, blended hydration, and modal painting | | `operator_invariants_completion_*.plan.md` | Production operator dispatch, interruption, rollback, cleanup, and undo/redo acceptance | | `editor_sample_regression_pack_*.plan.md` | Five-area sample manifest, editor catalog, deterministic validation, and native regression acceptance | +| `guarded_shutdown_savepoints_*.plan.md` | Native close coordination, asynchronous Save All, and canonical history clean points | ## Crate responsibilities (quick reference) diff --git a/docs/adr/0023-transactional-scene-persistence-and-recovery.md b/docs/adr/0023-transactional-scene-persistence-and-recovery.md index 6cc689f..c30a1e2 100644 --- a/docs/adr/0023-transactional-scene-persistence-and-recovery.md +++ b/docs/adr/0023-transactional-scene-persistence-and-recovery.md @@ -34,12 +34,19 @@ assets, mark a scene clean, enter source control, or silently replace authored w or Discard removes that scene's recovery generations. Save Recovery Copy As writes the snapshot to a separate transactional scene file without changing the active scene or recovery lifecycle. - A successful manual save retires recovery generations for that scene. +- A successful load or manual save establishes that tab's canonical authored-content checkpoint. + Undo and redo compare the current stable authored projection with this checkpoint to derive dirty + state. Failed writes, cancelled Save As, and recovery restores do not advance it. +- Native editor exit is authorized only after all dirty scene tabs save successfully or the user + explicitly discards them. See [ADR 0042](0042-guarded-editor-shutdown-and-document-savepoints.md). ## Consequences - Interrupted manual writes retain the previous complete destination on filesystems with atomic same-directory rename semantics. - Recovery is machine-local and does not pollute builds or source control. +- Undoing exactly to the last loaded or saved authored state clears the dirty marker without + weakening recovery after direct edits or failed saves. - Autosave still performs authored serialization and hydration rebuild work. Performance instrumentation and background snapshotting remain follow-up work under the production roadmap. - Unsaved new scenes do not yet have a stable recovery identity; they require a later session-ID diff --git a/docs/adr/0024-versioned-editor-session-state.md b/docs/adr/0024-versioned-editor-session-state.md index a5642bf..3b1640f 100644 --- a/docs/adr/0024-versioned-editor-session-state.md +++ b/docs/adr/0024-versioned-editor-session-state.md @@ -20,8 +20,10 @@ untrusted serialized ECS state. - Session schema v1 allowlists metadata only: project root, active scene path, saved dock-layout text, hierarchy expansion paths, non-destructive panel visibility, and finite camera bookmark transforms. -- Write the session transactionally. Startup immediately writes `clean_shutdown: false`; normal - `AppExit` writes the final document with `clean_shutdown: true`. +- Write the session transactionally. Startup immediately writes `clean_shutdown: false`; only an + editor-shutdown-coordinator-authorized `AppExit` writes the final document with + `clean_shutdown: true`. Direct `AppExit`, forced termination, and process failure retain the + abnormal marker. See [ADR 0042](0042-guarded-editor-shutdown-and-document-savepoints.md). - A clean prior session may restore its active scene and non-destructive UI state automatically. An abnormal prior session starts from the normal safe scene and requires explicit confirmation before opening the prior scene. diff --git a/docs/adr/0042-guarded-editor-shutdown-and-document-savepoints.md b/docs/adr/0042-guarded-editor-shutdown-and-document-savepoints.md new file mode 100644 index 0000000..15a7a8b --- /dev/null +++ b/docs/adr/0042-guarded-editor-shutdown-and-document-savepoints.md @@ -0,0 +1,57 @@ +# ADR 0042: Guarded Editor Shutdown And Document Savepoints + +## Status + +Accepted + +## Context + +Bevy's default native-window policy closes a requested window and exits after the final window is +gone. That policy bypasses Blacksite's scene-tab and project-switch confirmations, so a compositor +close request could silently discard dirty authored work. Reading `WindowCloseRequested` in another +system is insufficient because Bevy's default close system has its own message cursor and still +despawns the window. + +Scene dirtiness was also independent of editor history. Every undo or redo marked the active scene +dirty, even when undo restored the exact state that had last loaded or saved. Undo depth alone is +not a valid savepoint: editor commands contain runtime entity IDs and are intentionally cleared when +tabs switch, while saves may establish a clean point in the middle of a timeline. + +## Decision + +- The full editor disables Bevy's automatic close-on-request behavior. The game and Project Browser + keep their existing native-window policy. +- One editor shutdown coordinator owns native window close, **File > Quit**, project switching, and + programmatic editor-exit requests. It coalesces duplicate requests and is the only normal path + authorized to emit `AppExit`. +- Final authorization runs in a dedicated main schedule after Bevy's `Last` schedule. The guard + rechecks dirty scene state after every editor authoring system, then clean-session persistence + runs after that finalizer in the same post-`Last` schedule. +- A clean editor exits immediately. Dirty scene tabs enter one non-blocking native decision with + explicit **Save All**, **Discard**, and **Cancel** actions. Save completion, including an untitled + tab's Save As picker, is applied on the main thread. The coordinator exits only after every dirty + tab saves or the user explicitly discards; cancel or failure retains the live session. +- Clean-session persistence accepts only an exit authorized by the coordinator. Crashes, forced + termination, and unguarded `AppExit` retain the abnormal-session marker. +- Every scene tab owns a canonical authored-content checkpoint established by a successful load or + save. The projection uses stable actor IDs, stable parent actor IDs, sorted component identities, + and scene composition; it excludes transient Bevy entity numbers and runtime hydration. +- A committed edit marks the active tab dirty. Successful undo or redo compares the current + canonical authored projection with that tab's checkpoint. Equality clears the dirty marker; + divergence sets it. Failed saves and recovery restores do not advance the checkpoint. +- Editor command stacks remain active-document runtime state and are still cleared on tab switches. + Checkpoint ownership belongs to the document tab, so dirty truth survives those clears. + +## Consequences + +- Native close can no longer silently discard authored scene tabs, and the editor remains responsive + while confirmation or Save As dialogs are open. +- A normal exit requires an explicit editor-owned authorization. New code must request shutdown + through the coordinator instead of writing `AppExit` directly. +- Undoing to the last saved authored state clears title and tab dirty indicators, including after a + save at nonzero history depth. Redo or a divergent branch marks the document dirty again. +- Reconciliation serializes the authored projection after undo and redo. This is more work than a + depth comparison, but it is bounded to explicit history navigation and remains correct across + direct non-history repairs, entity respawns, and tab switches. +- Operating-system process kill and power loss cannot be confirmed; recovery and abnormal-session + handling remain the safety boundary for those cases. diff --git a/docs/editor/README.md b/docs/editor/README.md index d322b8a..c63b7cb 100644 --- a/docs/editor/README.md +++ b/docs/editor/README.md @@ -111,6 +111,10 @@ Docs for the in-process egui editor (`crates/editor/`). Update this index when a components on load and component transactions, and does not prohibit compatible composition such as renderer plus light. Component validation owns requirements and geometry-source conflicts. - **Scene persistence and recovery** use transactional same-directory writes for authored scenes and bounded user-local recovery snapshots for dirty saved scenes. Recovery runs every 120 seconds by default, keeps five generations, never marks the scene clean, and appears as a status/File-menu restore or discard action when newer than the authored file. Restore leaves the original scene dirty until manually saved. See [ADR 0023](../adr/0023-transactional-scene-persistence-and-recovery.md). +- **Guarded shutdown and clean savepoints** route native close, **File > Quit**, project switching, + and programmatic exit through one non-blocking Save All / Discard / Cancel coordinator. Each tab's + dirty marker is reconciled against its canonical last-loaded-or-saved authored projection after + undo/redo. See [ADR 0042](../adr/0042-guarded-editor-shutdown-and-document-savepoints.md). - **Collaborative file safety** records exact content revisions for loaded scenes, prefab source operations, editable materials, and Project Settings, then rechecks them immediately before atomic publication. External changes, create races, read-only files, and optional ownership locks keep the existing bytes intact and open Reload/Compare Metadata/Save As/Cancel recovery. Git status is asynchronous, observational, and absent without noise outside a repository. See [collaborative-file-safety.md](collaborative-file-safety.md) and [ADR 0037](../adr/0037-collaborative-authored-file-safety.md). - **Scene visualizers** expose actor root icons, colliders, lights, player spawns, gameplay markers, volumes, prefab/model anchors, Project Sun, and runtime player/camera markers without changing saved scene data. Spot lights show inner/outer cones and cameras show near/far frusta. Viewport options independently gate visualizer categories and include actor icon and transform-gizmo size sliders. - **Clean game-view overlay** (`G`) hides editor chrome, grid, selection outlines, transform gizmos, actor root icons, visualizers, and selectable proxies without changing camera ownership; `Ctrl+G` toggles grid. diff --git a/docs/editor/architecture.md b/docs/editor/architecture.md index 10d50fc..ca3d3c6 100644 --- a/docs/editor/architecture.md +++ b/docs/editor/architecture.md @@ -87,11 +87,20 @@ or Cancel. Generated artifacts remain under their subsystem-owned regeneration p [collaborative-file-safety.md](collaborative-file-safety.md) and [ADR 0037](../adr/0037-collaborative-authored-file-safety.md). +`project/shutdown.rs` owns native close, **File > Quit**, project switching, and programmatic editor +exit. The editor launch configuration leaves native close requests open; the coordinator coalesces +them, retries the shared native-dialog broker when busy, and authorizes `AppExit` only after clean +state, successful Save All, or explicit Discard. Scene I/O owns the asynchronous per-tab save +transaction and reports completion back to the coordinator. Final authorization and clean-session +persistence run in an editor schedule inserted after Bevy's `Last`, so material drops, brush/gizmo +finalizers, and every other authoring system finish before the last dirty-state check. See +[ADR 0042](../adr/0042-guarded-editor-shutdown-and-document-savepoints.md). + `project/session.rs` owns the independent versioned restart document under the user state directory. It snapshots allowlisted paths, panel visibility, dock/hierarchy metadata, and camera -bookmarks, writes a running marker at startup, and records clean `AppExit`. Abnormal sessions do -not auto-open the prior scene; the explicit safe-resume prompt is the only path back. See -[ADR 0024](../adr/0024-versioned-editor-session-state.md). +bookmarks, writes a running marker at startup, and records clean state only for a coordinator- +authorized `AppExit`. Abnormal sessions do not auto-open the prior scene; the explicit safe-resume +prompt is the only path back. See [ADR 0024](../adr/0024-versioned-editor-session-state.md). `project/diagnostics_bundle.rs` exports a separate, transactional support report from an explicit metadata allowlist. It summarizes build/renderer identity, project paths, dirty state, aggregate @@ -200,7 +209,7 @@ the Edit-to-Play boundary restore the complete runtime snapshot. See 1. User edits entities with `LevelObject` + reflectable components from `shared`. 2. `EditorOnly` entities (cameras, helpers) are filtered from hierarchy and save; visualizer proxies can be picked but resolve back to source entities. 3. Player placement is stored as `PlayerSpawn`; the runtime `Player` is never serialized. -4. `SceneIo` writes native Bevy dynamic scenes under `assets/levels/`, routed through `scene::document::SceneDocument`. The active tab is materialized in the ECS world; inactive tabs retain normalized authored snapshots and independent dirty/recovery state. `SceneComposition` resources reference validated project-relative subscenes by stable IDs, and runtime-only `ComposedSceneMember` ownership prevents child actors from being flattened into the owner save. See [multi-scene-composition.md](multi-scene-composition.md) and ADR 0026. +4. `SceneIo` writes native Bevy dynamic scenes under `assets/levels/`, routed through `scene::document::SceneDocument`. The active tab is materialized in the ECS world; inactive tabs retain normalized authored snapshots, canonical clean checkpoints, and independent dirty/recovery state. Undo/redo reconciles stable authored content rather than transient ECS entity IDs. `SceneComposition` resources reference validated project-relative subscenes by stable IDs, and runtime-only `ComposedSceneMember` ownership prevents child actors from being flattened into the owner save. See [multi-scene-composition.md](multi-scene-composition.md), ADR 0026, and [ADR 0042](../adr/0042-guarded-editor-shutdown-and-document-savepoints.md). 5. Hydration systems in `game` spawn meshes, colliders, lights, static mesh renderer parts, and dedicated skinned-model hierarchies at runtime. Level-object roots are initialized with Bevy visibility hierarchy components before generated children are attached, so authoring diff --git a/docs/editor/multi-scene-composition.md b/docs/editor/multi-scene-composition.md index 28d6908..59015fe 100644 --- a/docs/editor/multi-scene-composition.md +++ b/docs/editor/multi-scene-composition.md @@ -6,6 +6,11 @@ save/discard/cancel prompt when needed. **File > Open Scene...** opens another t discarding the active document. Save and Save As affect the active tab, while project switching offers to save every modified tab. +Each tab also retains a canonical checkpoint of the last successfully loaded or saved authored +projection. History commands remain active-tab runtime state, but undo and redo reconcile against +that document checkpoint. Returning exactly to the checkpoint clears only that tab's dirty marker; +redo, a divergent edit, failed save, or restored recovery remains dirty. + ## Composition workflow The **Composition** menu beside the tabs edits the active scene's persisted `SceneComposition`: diff --git a/docs/editor/native-dialogs.md b/docs/editor/native-dialogs.md index 00b542d..10acb35 100644 --- a/docs/editor/native-dialogs.md +++ b/docs/editor/native-dialogs.md @@ -16,6 +16,13 @@ loop or editor rendering. - Collaborative Save As performs the same revision and destination checks after selection. - Close or project switch never treats an opened Save As picker as a completed save. When a save remains pending, the transition pauses and can be retried after saving. +- Native window close, **File > Quit**, project switch, and programmatic editor exit share the + guarded-shutdown coordinator. Dirty scene tabs present exact **Save All**, **Discard**, and + **Cancel** actions. Save All remains pending through each untitled tab's Save As result and exits + only after every dirty tab is clean; cancellation or failure preserves the live session. +- A close request received while another native dialog owns the broker remains queued. The editor + rechecks dirty state and retries acquisition after that workflow completes instead of dropping + the request or opening a competing dialog. ## Covered Surfaces @@ -24,6 +31,8 @@ subscene selection, collaborative conflict copies, Project Browser folder select confirmations. New workflows must use the broker instead of calling `rfd` from an egui/Bevy system. See [ADR 0038](../adr/0038-non-blocking-native-dialog-broker.md). +Guarded exit ownership and document savepoints are defined by +[ADR 0042](../adr/0042-guarded-editor-shutdown-and-document-savepoints.md). Live native-Wayland acceptance is recorded in the [native-dialog responsiveness evaluation](evaluations/native-dialog-responsiveness/). diff --git a/docs/editor/session-recovery.md b/docs/editor/session-recovery.md index 97ab61f..ced819d 100644 --- a/docs/editor/session-recovery.md +++ b/docs/editor/session-recovery.md @@ -13,10 +13,14 @@ or credentials. ## Restart behavior -- **Clean shutdown:** the last authored scene and non-destructive UI state are restored. +- **Clean shutdown:** after the guarded exit path saves all dirty scene tabs or receives explicit + Discard confirmation, the last authored scene and non-destructive UI state are restored. - **Abnormal shutdown:** the editor opens its normal safe scene and displays **Recover Editor Session**. **Resume Last Scene** opens only the previous authored scene. **Continue Safe** keeps the startup scene. Modal tools and dirty preview state are never restored. +- Native window close, **File > Quit**, project switching, and programmatic editor exit use one + non-blocking Save All / Discard / Cancel workflow. Cancel and failed saves keep the session open. + Direct `AppExit`, process kill, and crashes are not recorded as clean shutdowns. - Dirty scene data remains governed by the independent scene recovery workflow in [ADR 0023](../adr/0023-transactional-scene-persistence-and-recovery.md).