diff --git a/crates/desktop/src/cartridge.rs b/crates/desktop/src/cartridge.rs index 7f74b73..09a33fc 100644 --- a/crates/desktop/src/cartridge.rs +++ b/crates/desktop/src/cartridge.rs @@ -329,7 +329,7 @@ pub fn execute(options: Options, writer: &mut impl Write) -> Result<(), Box io::Result<()> { + if matches!( + diagnostic.downcast_ref(), + Some(gba_core::snapshot::SnapshotError::RestoreFailed(_)) + ) { + return writeln!( + writer, + "Save not written: machine state is undefined after a failed state restore." + ); + } + match save.persist(memory) { + Ok(Some(saved)) => { + writeln!(writer, "Save persisted after diagnostic.")?; + if let Some(backup) = saved.backup { + writeln!(writer, "Previous save backup: {backup:?}")?; + } + Ok(()) + } + Ok(None) => writeln!(writer, "Save unchanged; no file written."), + Err(error) => writeln!(writer, "Save not written after diagnostic: {error}"), + } +} + #[cfg(test)] mod tests; diff --git a/crates/desktop/src/cartridge/save_file/tests.rs b/crates/desktop/src/cartridge/save_file/tests.rs index 7a25205..40e8f33 100644 --- a/crates/desktop/src/cartridge/save_file/tests.rs +++ b/crates/desktop/src/cartridge/save_file/tests.rs @@ -199,3 +199,56 @@ fn reject_rom_aliases_symlinks_directories_and_readonly_images() { assert_eq!(fs::read(&f.save).unwrap(), image); fs::set_permissions(&f.save, fs::Permissions::from_mode(0o600)).unwrap(); } + +fn after_diagnostic( + save: &mut SaveFile, + m: &Memory, + error: &(dyn std::error::Error + 'static), +) -> String { + let mut output = Vec::new(); + crate::cartridge::persist_save_after_diagnostic(save, m, error, &mut output).unwrap(); + String::from_utf8(output).unwrap() +} + +#[test] +fn diagnostics_persist_completed_programs_and_report_refusals() { + let diagnostic = io::Error::other("unsupported hardware"); + let f = Fixture::new(); + let original = f.image(255); + let mut m = memory(); + let mut save = f.open().unwrap(); + save.initialize(&mut m).unwrap(); + assert!(after_diagnostic(&mut save, &m, &diagnostic).contains("Save unchanged")); + program(&mut m, 0x12); + let text = after_diagnostic(&mut save, &m, &diagnostic); + assert!(text.contains("Save persisted after diagnostic."), "{text}"); + assert!(text.contains("Previous save backup"), "{text}"); + let written = fs::read(&f.save).unwrap(); + assert_eq!(written[0], 0x12); + assert_eq!(written[1..], original[1..]); + + // An incomplete command is reported, never written, and never replaces the diagnostic. + start_program(&mut m, 0x02); + let text = after_diagnostic(&mut save, &m, &diagnostic); + assert!(text.contains("Save not written after diagnostic"), "{text}"); + assert!(text.contains("incomplete"), "{text}"); + assert_eq!(fs::read(&f.save).unwrap(), written); +} + +#[test] +fn failed_state_restores_never_write_the_save() { + use gba_core::snapshot::SnapshotError; + let f = Fixture::new(); + let original = f.image(255); + let mut m = memory(); + let mut save = f.open().unwrap(); + save.initialize(&mut m).unwrap(); + program(&mut m, 0x12); + let error: Box = Box::new(SnapshotError::RestoreFailed(Box::new( + SnapshotError::Truncated, + ))); + let text = after_diagnostic(&mut save, &m, error.as_ref()); + assert!(text.contains("failed state restore"), "{text}"); + assert_eq!(fs::read(&f.save).unwrap(), original); + assert_eq!(fs::read_dir(&f.dir).unwrap().count(), 3); // ROM, image, owned lock only. +} diff --git a/crates/desktop/src/desktop/rom.rs b/crates/desktop/src/desktop/rom.rs index dc7c777..b87731d 100644 --- a/crates/desktop/src/desktop/rom.rs +++ b/crates/desktop/src/desktop/rom.rs @@ -121,7 +121,7 @@ pub fn run( )?; writeln!( writer, - "Input releases and audio mutes on focus loss. Save files require --save-file and clean exit; state file: {state_path:?}. Startup frames are not suppressed." + "Input releases and audio mutes on focus loss. Save files require --save-file and persist on exit; state file: {state_path:?}. Startup frames are not suppressed." )?; writeln!( writer, @@ -256,8 +256,19 @@ pub fn run( if let Some(output) = audio.as_ref() { output.report(writer)?; } - result?; writer.flush()?; + if let Err(diagnostic) = result { + // A player's completed in-game saves outlive the diagnostic that ended the session. + if let Some(save) = save_file { + crate::cartridge::persist_save_after_diagnostic( + save, + session.memory_mut(), + diagnostic.as_ref(), + writer, + )?; + } + return Err(diagnostic); + } if let Some(save) = save_file { crate::cartridge::persist_save(save, session.memory_mut(), writer)?; } diff --git a/crates/desktop/src/main.rs b/crates/desktop/src/main.rs index ada2c52..6d80747 100644 --- a/crates/desktop/src/main.rs +++ b/crates/desktop/src/main.rs @@ -141,7 +141,7 @@ fn run() -> Result<(), Box> { --audio Enable macOS output at 1x (default: muted)\n\ --rtc Select cartridge RTC (UTC startup, host elapsed time)\n\ --save-type flash64|flash128 Select nominal Macronix Flash emulation\n\ - --save-file PATH Load raw Flash image; persist only on clean exit\n\ + --save-file PATH Load raw Flash image; persist on clean exit or window diagnostic\n\ --state-file PATH ROM window save state; F5 saves, F9 loads (default: ROM path + .state)\n\ --test-suite PATH.json Run bounded ROM assertions; write a JSON report\n\ --cpu-demo Run the terminal-only ARM/Thumb instruction demo\n\ diff --git a/docs/hardware/saves.md b/docs/hardware/saves.md index 6eb999b..9d59fab 100644 --- a/docs/hardware/saves.md +++ b/docs/hardware/saves.md @@ -129,7 +129,10 @@ The frontend persists changed bytes only on a clean exit: - Reaching a terminal step limit or entering terminal STOP. An incomplete Flash command/operation refuses persistence and returns an error, even if no bytes changed yet. -CPU, DMA, rendering, window, or reported audio errors do not persist the session image. +A ROM window that ends on a CPU, DMA, rendering, window, or reported audio error also persists changed bytes, so completed in-game saves outlive the diagnostic. +The diagnostic remains the session's error. A refused or failed write there is reported as a line of output instead of replacing it. +A failed save-state restore leaves save memory undefined and never writes the file. +Terminal runs keep the stricter rule: a diagnostic does not persist the session image. There is no autosave, crash recovery, or RTC persistence. [Save states](save-states.md) are a separate, whole-machine feature and do not replace the game's own save. The emulator cannot detect a game's multi-command save transaction. Exiting between completed commands can still save incomplete game data. A crash or forced termination loses changes since startup and can leave a stale lock. @@ -159,7 +162,8 @@ Persistence currently requires a Unix host. Darwin arm64 is validated; Linux and Original tests cover all program byte values in both banks, erase scope, exact delays, polling, cancellation, and bit transitions. ARM/Thumb tests cover store timing, completion reads, failed-step rollback, block preflight, HALT, and STOP. File tests cover size checks, unchanged images, locks, external changes, aliases, read-only paths, and retained backups. -Original ROM process tests program, restart, reload through an IWRAM byte reader, and verify no persistence after diagnostics. +Original ROM process tests program, restart, reload through an IWRAM byte reader, and verify no persistence after terminal diagnostics. +File tests also cover window-diagnostic persistence, its reported refusals, and the failed-restore exclusion. A native-window test persists on frame-limit exit and reloads in another process. No game code, user save, firmware, or copyrighted assets are included in the tests.