Add multiple save file support with save slot management by Copilot · Pull Request #52 · Stephenson-Software/FishE

Copilot · 2026-02-05T09:54:07Z

Multiple Save Files Implementation - COMPLETE ✅

Test Coverage Summary

32 SaveFileManager tests covering all functionality
132 total tests in the project
Comprehensive coverage of edge cases, error scenarios, and integration workflows
All tests passing ✅

Security Summary

✅ No security vulnerabilities detected by CodeQL analysis.

Original prompt

This section details on the original issue you should resolve

<issue_title>Multiple save files</issue_title>
<issue_description>As a user, I would appreciate the ability to have multiple save files and be able to detect/manage these saves when the application starts.</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes Multiple save files #51

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Copilot

Pull request overview

This PR adds multiple save file support to the FishE game, allowing players to maintain independent save files in separate slots. The implementation introduces a SaveFileManager class to handle slot operations and updates the FishE initialization to display an interactive save selection menu at startup.

Changes:

New SaveFileManager class manages save slots with metadata display, creation, deletion, and gap-filling
FishE initialization updated to show save selection menu and route all save/load operations through selected slot paths
Added 15 comprehensive tests for SaveFileManager functionality and updated existing FishE test to mock save selection

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
src/saveFileManager.py	New class managing save slots in slot_N directories with metadata reading and slot operations
src/fishE.py	Added save selection menu, integrated SaveFileManager for all save/load operations, added file close calls
tests/test_saveFileManager.py	Comprehensive test suite covering slot management, metadata reading, edge cases, and error conditions
tests/test_fishE.py	Updated to mock SaveFileManager and _selectSaveFile method to avoid stdin interaction
README.md	Documented the new multiple save files feature with usage details
.gitignore	Changed from data/*.json to data/ to ignore entire data directory including slot subdirectories

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-15T01:31:26Z

src/fishE.py

+        playerSaveFile = open(self.saveFileManager.get_save_path("player.json"), "w")
        self.playerJsonReaderWriter.writePlayerToFile(self.player, playerSaveFile)
+        playerSaveFile.close()

-        timeServiceSaveFile = open("data/timeService.json", "w")
+        timeServiceSaveFile = open(
+            self.saveFileManager.get_save_path("timeService.json"), "w"
+        )
        self.timeServiceJsonReaderWriter.writeTimeServiceToFile(
            self.timeService, timeServiceSaveFile
        )
+        timeServiceSaveFile.close()

-        statsSaveFile = open("data/stats.json", "w")
+        statsSaveFile = open(self.saveFileManager.get_save_path("stats.json"), "w")
        self.statsJsonReaderWriter.writeStatsToFile(self.stats, statsSaveFile)
+        statsSaveFile.close()


File operations in save(), loadPlayer(), loadStats(), and loadTimeService() methods lack error handling. If any file operation fails (e.g., disk full, permission denied, corrupted file), the game will crash without a helpful error message. Consider adding try-except blocks to catch and handle IOError or OSError exceptions gracefully, possibly notifying the user of the issue.

Added comprehensive error handling with try-except blocks for IOError, OSError, and json.JSONDecodeError in all file operations. The game now gracefully handles disk full, permission denied, and corrupted file scenarios with user-friendly warnings. (6c58c97)

Copilot · 2026-02-15T01:31:26Z

src/fishE.py

+        playerSaveFile = open(self.saveFileManager.get_save_path("player.json"), "w")
        self.playerJsonReaderWriter.writePlayerToFile(self.player, playerSaveFile)
+        playerSaveFile.close()

-        timeServiceSaveFile = open("data/timeService.json", "w")
+        timeServiceSaveFile = open(
+            self.saveFileManager.get_save_path("timeService.json"), "w"
+        )
        self.timeServiceJsonReaderWriter.writeTimeServiceToFile(
            self.timeService, timeServiceSaveFile
        )
+        timeServiceSaveFile.close()

-        statsSaveFile = open("data/stats.json", "w")
+        statsSaveFile = open(self.saveFileManager.get_save_path("stats.json"), "w")
        self.statsJsonReaderWriter.writeStatsToFile(self.stats, statsSaveFile)
+        statsSaveFile.close()


While adding .close() calls is good, consider using context managers (with statements) for file operations to ensure files are closed even if an exception occurs. This pattern is already used in SaveFileManager (lines 51, 59) and would make the code more robust and consistent. For example: with open(self.saveFileManager.get_save_path("player.json"), "w") as playerSaveFile:

Updated all file operations to use context managers (with statements). This ensures files are properly closed even if exceptions occur, making the code more robust and consistent with SaveFileManager's existing patterns. (6c58c97)

Copilot · 2026-02-15T01:31:26Z

src/saveFileManager.py

+    def get_next_available_slot(self):
+        """Returns the next available save slot number"""
+        save_files = self.list_save_files()
+        if not save_files:
+            return 1
+
+        # Find gaps in slot numbers
+        existing_slots = sorted([save["slot"] for save in save_files])
+        for i in range(1, 100):
+            if i not in existing_slots:
+                return i
+        return len(existing_slots) + 1


If all 99 save slots are filled, get_next_available_slot() returns 100 (line 84), but list_save_files() only iterates through range(1, 100) which stops at 99 (line 22). This means slot 100 would be created but never listed. Consider either: 1) returning None when all slots are full and handling this in the UI, or 2) extending the range to match in both functions, or 3) adding a check to prevent creating slot 100.

Fixed by returning None when all 99 slots are full instead of 100. The UI now checks for None and displays "All save slots are full. Please delete a save first." Added test coverage for this edge case. (6c58c97)

src/fishE.py

Copilot · 2026-02-15T01:31:27Z

src/fishE.py

        if not os.path.exists("data"):
            os.makedirs("data")


The hardcoded "data" directory check is inconsistent with the SaveFileManager's configurable data_directory parameter. This creates a directory that might not match the SaveFileManager's configuration, potentially causing issues if a custom data directory is used. Consider using self.saveFileManager.data_directory instead of the hardcoded "data" string, or remove this check entirely since get_save_path() already creates the necessary directories (lines 99-100 in saveFileManager.py).

Fixed! Changed hardcoded "data" to use self.saveFileManager.data_directory for consistency. Since get_save_path() already creates necessary directories, the redundant check is now properly aligned with SaveFileManager's configuration. (6c58c97)

Copilot · 2026-02-15T01:31:27Z

src/fishE.py

+                self._deleteSaveFile(save_files)
+                # Recursively call to show updated menu
+                self._selectSaveFile()
+                return


The recursive call to _selectSaveFile() after deleting a save file could lead to a stack overflow if a user repeatedly deletes and cancels many times. Consider using a loop-based approach instead of recursion to handle menu refreshes, or at least add a depth limit.

Replaced recursion with a loop-based approach. The _selectSaveFile() method now uses while True at the top level, and _deleteSaveFile() returns True/False to control flow. This eliminates any risk of stack overflow regardless of user behavior. (6c58c97)

Copilot · 2026-02-15T01:31:27Z

src/saveFileManager.py

+        except Exception:
+            return None


The bare except clause catches all exceptions silently, making debugging difficult. While returning None is appropriate for corrupted save files, consider logging the exception or being more specific about which exceptions to catch (e.g., json.JSONDecodeError, IOError). This would help identify issues during development and provide better diagnostics.

Changed from bare except to catch specific exceptions: (json.JSONDecodeError, IOError, OSError). This makes debugging easier while still returning None for corrupted/inaccessible save files as intended. (6c58c97)

Copilot · 2026-02-15T01:31:27Z

src/saveFileManager.py

+        # Look for save slots (slot_1, slot_2, etc.)
+        for i in range(1, 100):  # Support up to 99 save slots
+            slot_name = f"slot_{i}"
+            slot_path = os.path.join(self.data_directory, slot_name)
+            if os.path.exists(slot_path):
+                metadata = self._read_save_metadata(slot_path)
+                if metadata:
+                    save_files.append(
+                        {
+                            "slot": i,
+                            "slot_name": slot_name,
+                            "path": slot_path,
+                            "metadata": metadata,
+                        }
+                    )


The loop iterating through range(1, 100) checks for the existence of 99 directories on every call to list_save_files(). This could be inefficient if called frequently. Consider using os.listdir() to only check directories that actually exist, filtering for those matching the "slot_N" pattern. This would scale better with the number of actual save files rather than the maximum possible slots.

Suggested change

# Look for save slots (slot_1, slot_2, etc.)

for i in range(1, 100): # Support up to 99 save slots

slot_name = f"slot_{i}"

slot_path = os.path.join(self.data_directory, slot_name)

if os.path.exists(slot_path):

metadata = self._read_save_metadata(slot_path)

if metadata:

save_files.append(

{

"slot": i,

"slot_name": slot_name,

"path": slot_path,

"metadata": metadata,

}

)

# Look for save slots (slot_1, slot_2, etc.) by inspecting existing directories

for entry in os.listdir(self.data_directory):

if not entry.startswith("slot_"):

continue

# Extract the numeric slot index from the directory name

_, _, suffix = entry.partition("_")

if not suffix.isdigit():

continue

slot_index = int(suffix)

if slot_index < 1 or slot_index >= 100:

# Preserve the upper bound of 99 save slots

continue

slot_name = entry

slot_path = os.path.join(self.data_directory, slot_name)

if not os.path.isdir(slot_path):

continue

metadata = self._read_save_metadata(slot_path)

if metadata:

save_files.append(

{

"slot": slot_index,

"slot_name": slot_name,

"path": slot_path,

"metadata": metadata,

}

)

Optimized using os.listdir() as suggested! Now only checks directories that actually exist and filters by the "slot_N" pattern. This scales with actual save files (typically 1-5) rather than always checking 99 slots. Much more efficient. (6c58c97)

Copilot · 2026-02-15T01:31:28Z

src/saveFileManager.py

+
+        # Create slot directory if it doesn't exist
+        if not os.path.exists(slot_path):
+            os.makedirs(slot_path)


Consider adding exist_ok=True parameter to os.makedirs() for robustness. While unlikely in single-player context, this prevents potential FileExistsError if the directory is created between the existence check and the makedirs call. This is a defensive programming practice that makes the code more resilient.

Suggested change

os.makedirs(slot_path)

os.makedirs(slot_path, exist_ok=True)

Added exist_ok=True to os.makedirs() for defensive programming. While unlikely in single-player context, this prevents potential FileExistsError race conditions. (6c58c97)

dmccoystephenson · 2026-02-15T01:39:37Z

@copilot address comments

…dd migration Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

dmccoystephenson · 2026-03-04T03:04:41Z

@copilot expand unit tests

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Copilot · 2026-03-04T03:08:34Z

@copilot expand unit tests

Expanded unit tests from 18 to 32 tests for SaveFileManager (+14 new tests). Total test suite grew from 118 to 132 tests. New coverage includes:

Edge Cases: Invalid slot names (slot_abc, slot_0, slot_100), boundary values, non-directory entries
Error Handling: OSError resilience, empty files, corrupted JSON handling
Migration: Partial migration (only player.json), existing slot_1 overwrite scenarios
Metadata: Partial data with defaults, missing optional files (timeService.json)
Integration: Full user workflows - create/delete/list operations in sequence

All 132 tests passing ✅ (39ec6fb)

Initial plan

8d92f12

Copilot AI assigned Copilot and dmccoystephenson Feb 5, 2026

Copilot started work on behalf of dmccoystephenson February 5, 2026 09:54 View session

Copilot AI and others added 4 commits February 5, 2026 09:58

Implement multiple save files with SaveFileManager

e5cb186

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Update .gitignore to exclude data directory

710045c

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Address code review feedback - improve imports and test structure

102ceac

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Add multiple save files documentation to README

1e5ad90

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Add support for multiple save files management~~ Add multiple save file support with save slot management Feb 5, 2026

Copilot AI requested a review from dmccoystephenson February 5, 2026 10:03

Copilot finished work on behalf of dmccoystephenson February 5, 2026 10:03

dmccoystephenson requested a review from Copilot February 15, 2026 01:24

Copilot started reviewing on behalf of dmccoystephenson February 15, 2026 01:25 View session

Copilot AI reviewed Feb 15, 2026

View reviewed changes

Copilot started work on behalf of dmccoystephenson February 15, 2026 01:39 View session

Address PR review comments - improve error handling, fix recursion, a…

6c58c97

…dd migration Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Copilot finished work on behalf of dmccoystephenson February 15, 2026 01:44

Copilot started work on behalf of dmccoystephenson March 4, 2026 03:05 View session

Expand unit tests with 14 additional test cases for SaveFileManager

39ec6fb

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>

Copilot finished work on behalf of dmccoystephenson March 4, 2026 03:09

dmccoystephenson marked this pull request as ready for review March 4, 2026 03:38

dmccoystephenson merged commit fed01f6 into main Mar 4, 2026
1 check passed

dmccoystephenson deleted the copilot/add-multiple-save-files-support branch March 4, 2026 04:55

Conversation

Copilot AI commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Multiple Save Files Implementation - COMPLETE ✅

Test Coverage Summary

Security Summary

Comments on the Issue (you are @copilot in this section)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

dmccoystephenson commented Feb 15, 2026

Uh oh!

dmccoystephenson commented Mar 4, 2026

Uh oh!

Copilot AI commented Mar 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Feb 5, 2026 •

edited

Loading