Stop a damaged save from being destroyed, and say so on screen

[dmccoystephenson]

Summary

Two failure modes around save files were addressed, both of which cost a player their whole run without a word on screen.

Save and load errors are now shown through the front-end contract (#142). They were reported with print(), which reaches nobody: stdout is not rendered at all by the pygame front-end or by either browser front-end, and on the console it is wiped by the lotsOfSpace() at the top of the next screen. showDialogue is used instead — an abstract primitive on BaseUserInterface, so all four front-ends were served by the one change rather than one of them.

A save that will not load is no longer overwritten by the fresh game that replaces it (#143). Two independent changes:

• Writes are made atomic. Each file is written to a temporary file beside the target and swapped in with os.replace (atomic on POSIX and Windows) once complete. Previously each path was opened with mode "w", which truncates it before a single byte is written — so a crash, a full disk or a kill mid-dump left a partial file and no intact copy anywhere, which is how a slot became unreadable to begin with.
• A slot that would not load is copied into a damaged-<date>-<time> subdirectory of itself before the fallback game is allowed to write. The whole slot is copied rather than only the file that failed, because restoring a run needs player.json, stats.json and timeService.json together. Under the Pyodide front-end the copy is flushed to IndexedDB, for the same reason delete_save_slot flushes.

Supporting details:

• The failed loads are collected and reported in one dialogue rather than one per file, so a slot with three bad files costs one acknowledgement.
• Each reason is trimmed to a single short line. jsonschema's ValidationError renders as a multi-paragraph dump of the whole instance and schema; its .message is the one-line reason, and no dialogue box could show the former.
• A failed save is reported on every action that fails, rather than once. A run that is no longer being written down is judged to be exactly the thing a player must not be allowed to miss.
• README.md's "Multiple Save Files" section was extended to describe the atomic write and what a damaged-... folder in a slot is.

One residual risk is recorded rather than hidden: os.replace over an existing file was not exercised under Pyodide in this environment. Emscripten's FS.rename unlinks an existing destination, so it is expected to behave, but it was verified by reading rather than by running.

Closes #142
Closes #143

Test plan

• python3 -m compileall -q src tests
• python3 -m pytest --cov=src --cov-report=term-missing --cov-report=xml:cov.xml — 781 passed, src/fishE.py at 99% (the only uncovered lines being the pre-existing if __name__ == "__main__" block)
• black and autoflake run over the changed files only
• Front-ends: no per-front-end code was needed. Every new message goes through BaseUserInterface.showDialogue, which UserInterface (console), PygameUserInterface, WebUserInterface and PyodideUserInterface (a subclass of WebUserInterface) all implement. The rendered text was printed and checked for length and line breaks; pygame's _wrapText/_splitLongWord handles the long backup path.
• New tests cover: the save-failure dialogue, no .tmp file surviving a save, a failed save leaving the previous one intact, the load-failure dialogue reaching the front-end and not stdout, several failures folded into one dialogue, a schema failure described in one short line, an over-long reason being trimmed, the whole slot being copied aside before the fallback game overwrites it, a clean slot not being copied aside, and a copy that itself fails saying so.
• tests/test_saveFileManager.py gained a regression test that the new nested damaged-... directory stays invisible to the save menu and does not disturb get_next_available_slot().

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

[dmccoystephenson]

Self-review

The full diff was read against this repo's own conventions (front-end parity through BaseUserInterface, the schema/*JsonReaderWriter contract, test-per-module mirroring, # @author headers on new files). No blocking defect was found. Findings are recorded below as path:line — finding, including the ones judged not worth acting on, so the reasoning is on the record rather than in someone's head.

Acted on

tests/test_fishE.py:583 — the comment read "remember the bytes of the whole slot before the game runs", but nothing was remembered in a variable and the assertion compares against a literal. Corrected in d06d1c1 to describe what the test actually sets up.

src/fishE.py:290 — _describeLoadFailure was added during implementation after it was noticed that jsonschema's ValidationError renders as a multi-paragraph dump of the whole instance and schema. Pasted verbatim into a dialogue, it would have been unreadable on every front-end, and unshowable on pygame without scrolling several screens. .message is used where present, the first line is taken, and 120 characters is the cap. Covered by test_a_schema_failure_is_described_in_one_short_line and test_describeLoadFailure_trims_a_reason_too_long_for_a_dialogue.

src/fishE.py:328 — a partially-copied backup was originally left on disk when shutil.copy2 failed midway. That is worse than no backup at all, because a folder named damaged-... holding two of three files looks like a rescued save and would be trusted as one. Any exception during the copy now discards the whole backup directory, and the dialogue says plainly that the slot will be overwritten.

Considered and deliberately not changed

src/fishE.py:377 — the three save files are each written atomically, but not atomically as a set: a failure between player.json and stats.json leaves a new player beside older stats. This is not a regression — the previous truncating writes had the same tearing plus a truncated file — and closing it properly means staging all three and swapping them together, which is a larger change than #143 asks for. Left for a follow-up.

src/fishE.py:404 — a failed save is reported on every action that fails, not once. This makes a permanently unwritable disk produce a dialogue per turn. That was judged correct rather than merely tolerable: the alternative is suppressing the second and later reports, which recreates the hidden-failure shape #142 exists to remove, and a run that is no longer being written down is not worth playing quietly.

src/fishE.py:311 — repeated damaged-<timestamp> directories can accumulate in one slot if the player quits before the fallback game's first save, leaving the slot damaged for the next session too. Each is a genuinely distinct point in time, the files are a few hundred bytes, and pruning them would mean deciding which copy of a player's lost run to throw away. Left alone.

src/fishE.py:301 — a stale player.json.tmp left by an earlier crash is copied into the backup along with everything else. That is intended: it may hold more of the lost run than anything else in the slot.

src/fishE.py:314 — os.path.dirname(get_save_path("player.json")) is a roundabout way to reach the slot directory, and it relies on get_save_path's side effect of creating that directory. SaveFileManager exposes no accessor for the slot path, and adding one to serve a single caller was judged not to earn its keep here.

src/fishE.py:387 — os.replace over an existing file was not exercised under Pyodide in this environment. Emscripten's FS.rename unlinks an existing destination, so the behaviour is expected to hold, but this was established by reading rather than by running, and it is the deployed front-end. Flagged rather than left silent.

Front-end parity

No per-front-end code was needed and none is missing. Every new message goes through BaseUserInterface.showDialogue, implemented independently by UserInterface (console, subclassed by ConsoleUserInterface), PygameUserInterface and WebUserInterface; PyodideUserInterface subclasses WebUserInterface and inherits it. The rendered text was printed and inspected: pygame's _wrapText/_splitLongWord breaks the long backup path, and the console and both browser front-ends render newlines as written.

Schema contract

No field was added, renamed or retyped on Player, Stats or TimeService, so schemas/player.json, schemas/stats.json and schemas/timeService.json need no change and none was made. What changed is only how the same JSON reaches disk.

Tests and docs

781 tests pass. src/fishE.py sits at 99%, the only uncovered lines being the pre-existing if __name__ == "__main__" block. The new nested damaged-... directory was checked against SaveFileManager.list_save_files and get_next_available_slot and is invisible to both, with a regression test added for that in tests/test_saveFileManager.py. README.md's "Multiple Save Files" section was extended; PLANNING.md says nothing about save durability and needed no edit.

This review comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).