fix(desktop): persist the save when a ROM window ends on a diagnostic
A core, DMA, rendering, window, or audio error returned before the save was written, so every completed in-game save since launch was lost. The window now writes changed bytes on that path too. The diagnostic stays the session error; a refused or failed write is reported as output. A failed state restore leaves save memory undefined and is never written. Terminal runs keep the strict rule. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HhfHfHEuzv2AUTd6erW1gU
This commit is contained in:
parent
fd73788735
commit
66817c919d
5 changed files with 105 additions and 6 deletions
|
|
@ -329,7 +329,7 @@ pub fn execute(options: Options, writer: &mut impl Write) -> Result<(), Box<dyn
|
||||||
.map(|path| save_file::SaveFile::open(path, &options.path, options.save_device))
|
.map(|path| save_file::SaveFile::open(path, &options.path, options.save_device))
|
||||||
.transpose()?;
|
.transpose()?;
|
||||||
if let Some(save) = save_file.as_ref() {
|
if let Some(save) = save_file.as_ref() {
|
||||||
writeln!(writer, "Save file: {}; writes only on clean exit, with unique backups when replacing existing data.", if save.loaded() { "loaded" } else { "new erased image" })?;
|
writeln!(writer, "Save file: {}; writes on clean exit or a ROM window diagnostic, with unique backups when replacing existing data.", if save.loaded() { "loaded" } else { "new erased image" })?;
|
||||||
} else if options.save_device != SaveDevice::None {
|
} else if options.save_device != SaveDevice::None {
|
||||||
writeln!(
|
writeln!(
|
||||||
writer,
|
writer,
|
||||||
|
|
@ -400,5 +400,36 @@ pub(crate) fn persist_save(
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Keep completed in-game saves when a window session ends on a diagnostic.
|
||||||
|
/// The diagnostic stays the session's error, so a refused or failed write is only reported.
|
||||||
|
/// A failed state restore leaves save memory undefined and is never written.
|
||||||
|
pub(crate) fn persist_save_after_diagnostic(
|
||||||
|
save: &mut save_file::SaveFile,
|
||||||
|
memory: &gba_core::memory::Memory,
|
||||||
|
diagnostic: &(dyn Error + 'static),
|
||||||
|
writer: &mut impl Write,
|
||||||
|
) -> 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)]
|
#[cfg(test)]
|
||||||
mod tests;
|
mod tests;
|
||||||
|
|
|
||||||
|
|
@ -199,3 +199,56 @@ fn reject_rom_aliases_symlinks_directories_and_readonly_images() {
|
||||||
assert_eq!(fs::read(&f.save).unwrap(), image);
|
assert_eq!(fs::read(&f.save).unwrap(), image);
|
||||||
fs::set_permissions(&f.save, fs::Permissions::from_mode(0o600)).unwrap();
|
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<dyn std::error::Error> = 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.
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -121,7 +121,7 @@ pub fn run(
|
||||||
)?;
|
)?;
|
||||||
writeln!(
|
writeln!(
|
||||||
writer,
|
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!(
|
writeln!(
|
||||||
writer,
|
writer,
|
||||||
|
|
@ -256,8 +256,19 @@ pub fn run(
|
||||||
if let Some(output) = audio.as_ref() {
|
if let Some(output) = audio.as_ref() {
|
||||||
output.report(writer)?;
|
output.report(writer)?;
|
||||||
}
|
}
|
||||||
result?;
|
|
||||||
writer.flush()?;
|
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 {
|
if let Some(save) = save_file {
|
||||||
crate::cartridge::persist_save(save, session.memory_mut(), writer)?;
|
crate::cartridge::persist_save(save, session.memory_mut(), writer)?;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -141,7 +141,7 @@ fn run() -> Result<(), Box<dyn Error>> {
|
||||||
--audio Enable macOS output at 1x (default: muted)\n\
|
--audio Enable macOS output at 1x (default: muted)\n\
|
||||||
--rtc Select cartridge RTC (UTC startup, host elapsed time)\n\
|
--rtc Select cartridge RTC (UTC startup, host elapsed time)\n\
|
||||||
--save-type flash64|flash128 Select nominal Macronix Flash emulation\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\
|
--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\
|
--test-suite PATH.json Run bounded ROM assertions; write a JSON report\n\
|
||||||
--cpu-demo Run the terminal-only ARM/Thumb instruction demo\n\
|
--cpu-demo Run the terminal-only ARM/Thumb instruction demo\n\
|
||||||
|
|
|
||||||
|
|
@ -129,7 +129,10 @@ The frontend persists changed bytes only on a clean exit:
|
||||||
- Reaching a terminal step limit or entering terminal STOP.
|
- 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.
|
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.
|
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.
|
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.
|
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.
|
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.
|
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.
|
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.
|
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.
|
No game code, user save, firmware, or copyrighted assets are included in the tests.
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue