Keep a damaged save slot visible, and stop it being overwritten

README.md

@@ -124,14 +124,18 @@ Sell your catch at the shop. The shop has a limited amount of money each day tha
 ### Multiple Save Files
 FishE supports multiple save files, allowing you to maintain different game progressions simultaneously. When you start the game, you'll see a save file manager that displays:
 
-- **Existing Saves**: each save slot is listed with a snapshot of its progress (Day, Money, and Fish count)
+- **Existing Saves**: each save slot is listed with a snapshot of its progress (Day, Money, and Fish count) — or, if the slot is damaged, with the fact that it can't be loaded (see below)
 - **Create New Save**: Start a fresh game in a new save slot
 - **Delete Save**: Remove unwanted save files
 - **Load**: Pick any existing save slot to continue your adventure
 
 Each save file is stored in its own slot (slot_1, slot_2, etc.) in the `data/` directory, ensuring your saves never conflict with each other. Set `FISHE_SAVE_DIR` to keep them somewhere else.
 
-Saves are written atomically — each file goes to a temporary file beside it and is swapped into place only once it's complete — so a crash or a power cut mid-save leaves the previous save intact rather than a half-written one. If a slot still turns out to be unreadable, the game tells you so on startup instead of starting quietly from scratch, and copies the whole slot into a `damaged-<date>-<time>` folder inside it before the new game overwrites anything. That folder is yours to inspect or restore by hand; deleting the slot from the menu deletes it too.
+Saves are written atomically — each file goes to a temporary file beside it and is swapped into place only once it's complete — so a crash or a power cut mid-save leaves the previous save intact rather than a half-written one.
+
+If a slot's `player.json` still turns out to be unreadable, the slot is listed as `Slot N (damaged)` rather than dropped from the menu, and marked unpickable with its reason the same way any unusable option is (see above) — so you can see it's there and can't pick it by mistake. Its number stays taken too, so **Create New Save** offers the next free slot instead of pointing a new game at the damaged one and overwriting the intact files beside it. Deleting it from the menu is how you reclaim the slot, and that row is marked `(damaged)` so you know which one you're clearing.
+
+Damage the menu can't see ahead of time — a save that parses but fails validation, or a bad `stats.json` or `timeService.json` — is caught on load instead: the game tells you so on startup rather than starting quietly from scratch, and copies the whole slot into a `damaged-<date>-<time>` folder inside it before the new game overwrites anything. That folder is yours to inspect or restore by hand; deleting the slot from the menu deletes it too.
 
 When you play in your own browser (the Pyodide front-end above), those same slots are written to your browser's IndexedDB instead of to disk — creating, saving and deleting a slot all take effect there, so your progress is waiting for you when you come back to the tab. They belong to that browser on that machine: clearing the site's data clears them, and they don't follow you to another browser or another device.
 

src/fishE.py

@@ -60,17 +60,24 @@ def __init__(self, interfaceType=INTERFACE_TYPE):
         # Show save file selection menu (uses the UI above)
         self._selectSaveFile()
 
-        # Load the chosen slot over the defaults if it has data
+        # Load the chosen slot over the defaults if it has data.
+        #
+        # Existence is the only condition: a file that is present but empty is a
+        # damaged save, not an absent one, and it has to reach the loader to be
+        # treated as such. Skipping the read on a zero-byte file (which an
+        # earlier truncating write could leave behind) meant nothing was
+        # appended to failedLoads, so the player was handed a starting character
+        # on the saved calendar with no warning and no copy kept.
         player_path = self.saveFileManager.get_save_path("player.json")
-        if os.path.exists(player_path) and os.path.getsize(player_path) > 0:
+        if os.path.exists(player_path):
             self.loadPlayer()
 
         stats_path = self.saveFileManager.get_save_path("stats.json")
-        if os.path.exists(stats_path) and os.path.getsize(stats_path) > 0:
+        if os.path.exists(stats_path):
             self.loadStats()
 
         time_path = self.saveFileManager.get_save_path("timeService.json")
-        if os.path.exists(time_path) and os.path.getsize(time_path) > 0:
+        if os.path.exists(time_path):
             self.loadTimeService()
 
         # A failed load leaves fresh objects in place of the player's run, and
@@ -154,8 +161,28 @@ def _selectSaveFile(self):
             # Build the option list, tracking what each option does in parallel.
             options = []
             actions = []  # (kind, arg) for the option at the same index
+            unavailable = {}  # {optionNumber: reason} for rows that can't be picked
             for save in save_files:
                 metadata = save["metadata"]
+                if metadata.get("unreadable"):
+                    # Shown rather than hidden, and unpickable rather than
+                    # loadable. Hiding it is what let the slot be handed back as
+                    # "Create New Save" and overwritten (see
+                    # SaveFileManager._unreadable_save_metadata); offering it as
+                    # a save would promise a run that cannot be read. Deleting
+                    # it is how the slot gets reclaimed, so the reason says so.
+                    # The action is only here to keep actions[] aligned with
+                    # options[] - showOptions will not return this number.
+                    # The label only identifies the slot; the blocker lives in
+                    # the reason, the same way every other unusable option in
+                    # the game is built. Spelling "damaged, cannot be loaded"
+                    # into the label as well reads as a stutter once a
+                    # front-end appends the reason to the row.
+                    options.append("Slot %d (damaged)" % save["slot"])
+                    actions.append(("damaged", save["slot"]))
+                    reason = "can't be read - delete it to reuse the slot"
+                    unavailable[len(options)] = reason
+                    continue
                 options.append(
                     "Load Slot %d (Day %d, $%d, %d fish)"
                     % (
@@ -178,7 +205,9 @@ def _selectSaveFile(self):
             actions.append(("quit", None))
 
             choice = int(
-                self.userInterface.showOptions("FishE - Save File Manager", options)
+                self.userInterface.showOptions(
+                    "FishE - Save File Manager", options, unavailable
+                )
             )
             kind, arg = actions[choice - 1]
 
@@ -190,10 +219,30 @@ def _selectSaveFile(self):
                 # loop to show the refreshed menu either way
             elif kind == "quit":
                 exit(0)
+            elif kind == "damaged":
+                # A conforming front-end refuses to return an unavailable
+                # option's number, so this should be unreachable. It is handled
+                # anyway because the alternative is falling out of this
+                # if-chain and silently re-rendering the same menu forever,
+                # which is an unexplained hang rather than a visible bug - and
+                # a new front-end is exactly the thing that would get this
+                # wrong (see the parity note on BaseUserInterface.showOptions).
+                self.userInterface.showDialogue(
+                    "Slot %d can't be loaded: its player.json could not be "
+                    "read.\n\nIt has been left alone rather than overwritten, "
+                    "so you can still copy the folder somewhere safe. To use "
+                    "the slot again, choose 'Delete a Save File'." % arg
+                )
 
     def _deleteSaveFile(self, save_files):
         """Delete a save file. Returns True if a file was deleted, False if cancelled."""
-        options = ["Delete Slot %d" % save["slot"] for save in save_files]
+        # A damaged slot is tagged here too: this menu is the only way to
+        # reclaim it, so the player has to be able to tell which row is the
+        # unreadable one they came here to clear.
+        options = []
+        for save in save_files:
+            damaged = " (damaged)" if save["metadata"].get("unreadable") else ""
+            options.append("Delete Slot %d%s" % (save["slot"], damaged))
         options.append("Cancel")
 
         choice = int(self.userInterface.showOptions("Delete a Save File", options))

src/saveFileManager.py

@@ -58,41 +58,87 @@ def list_save_files(self):
         return save_files
 
     def _read_save_metadata(self, slot_path):
-        """Read metadata from a save slot"""
+        """Read metadata from a save slot.
+
+        Returns None only when the slot holds no run at all (no player.json).
+        A player.json that will not parse comes back as the marker described in
+        _unreadable_save_metadata rather than as None, so a damaged slot stays
+        listed and stays claimed instead of disappearing."""
+        player_file = os.path.join(slot_path, "player.json")
+        time_file = os.path.join(slot_path, "timeService.json")
+
+        if not os.path.exists(player_file):
+            return None
+
+        metadata = {}
+
+        # player.json is the file that holds the run, so it is read strictly.
+        # Deliberately no "size > 0" guard: an empty file is a damaged file
+        # rather than an absent one, and skipping the read for it is what let a
+        # zero-byte save be offered in the menu as a real one.
+        try:
+            with open(player_file, "r") as f:
+                player_data = json.load(f)
+        except (json.JSONDecodeError, IOError, OSError) as error:
+            return self._unreadable_save_metadata(player_file, error)
+
+        if not isinstance(player_data, dict):
+            # Valid JSON that is not an object - a bare number, or some other
+            # file copied over the save - has no fields to read, and .get()
+            # would raise AttributeError here, taking the whole menu down with
+            # it rather than reporting one bad slot.
+            return self._unreadable_save_metadata(
+                player_file, ValueError("not a JSON object")
+            )
+
+        metadata["money"] = player_data.get("money", 0)
+        metadata["fishCount"] = player_data.get("fishCount", 0)
+        metadata["energy"] = player_data.get("energy", 100)
+
+        # The calendar is not the run: a slot whose timeService.json is missing
+        # or damaged still holds a loadable player, and FishE reports and
+        # preserves that damage on load. So this read is tolerant - leaving the
+        # fields out just falls the menu label back to its "Day 1" default.
         try:
-            player_file = os.path.join(slot_path, "player.json")
-            time_file = os.path.join(slot_path, "timeService.json")
-
-            if not os.path.exists(player_file):
-                return None
-
-            metadata = {}
-
-            # Read player data
-            if os.path.exists(player_file) and os.path.getsize(player_file) > 0:
-                with open(player_file, "r") as f:
-                    player_data = json.load(f)
-                    metadata["money"] = player_data.get("money", 0)
-                    metadata["fishCount"] = player_data.get("fishCount", 0)
-                    metadata["energy"] = player_data.get("energy", 100)
-
-            # Read time data
-            if os.path.exists(time_file) and os.path.getsize(time_file) > 0:
-                with open(time_file, "r") as f:
-                    time_data = json.load(f)
-                    metadata["day"] = time_data.get("day", 1)
-                    metadata["time"] = time_data.get("time", 0)
-
-            # Get last modified time
-            metadata["last_modified"] = datetime.fromtimestamp(
-                os.path.getmtime(player_file)
-            ).strftime("%Y-%m-%d %H:%M:%S")
-
-            return metadata
-        except (json.JSONDecodeError, IOError, OSError) as e:
-            # Return None for corrupted or inaccessible save files
+            with open(time_file, "r") as f:
+                time_data = json.load(f)
+            if isinstance(time_data, dict):
+                metadata["day"] = time_data.get("day", 1)
+                metadata["time"] = time_data.get("time", 0)
+        except (json.JSONDecodeError, IOError, OSError):
+            pass
+
+        metadata["last_modified"] = self._last_modified(player_file)
+
+        return metadata
+
+    def _last_modified(self, path):
+        """A file's modification time as a display string, or None if unknown."""
+        try:
+            return datetime.fromtimestamp(os.path.getmtime(path)).strftime(
+                "%Y-%m-%d %H:%M:%S"
+            )
+        except OSError:
             return None
 
+    def _unreadable_save_metadata(self, player_file, error):
+        """Metadata standing in for a slot whose player.json will not parse.
+
+        Returned instead of None because list_save_files() drops a slot with no
+        metadata, and get_next_available_slot() derives the taken slot numbers
+        from that same filtered list - so a damaged slot used to vanish from the
+        menu *and* be handed straight back as "Create New Save", pointing the
+        next save at the occupied directory and overwriting the intact
+        stats.json and timeService.json sitting beside the damaged file.
+
+        Callers key off "unreadable" to show the slot as present but unpickable
+        (see FishE._selectSaveFile)."""
+        return {
+            "unreadable": True,
+            "reason": str(error),
+            "last_modified": self._last_modified(player_file),
+        }
+
     def get_next_available_slot(self):
         """Returns the next available save slot number, or None if all slots are full"""
         save_files = self.list_save_files()