diff --git a/game/persistence/savegamebundle.py b/game/persistence/savegamebundle.py index 3674b7e4..6a87dc5c 100644 --- a/game/persistence/savegamebundle.py +++ b/game/persistence/savegamebundle.py @@ -73,7 +73,7 @@ class SaveGameBundle: with ZipFile(self.bundle_path) as zip_bundle: with zip_bundle.open(name, "r") as save: game = pickle.load(save) - game.save_bundle = self + game.save_manager.set_loaded_from(self) return game def _update_bundle_member( diff --git a/game/persistence/savemanager.py b/game/persistence/savemanager.py index 3ebdf108..97f45a31 100644 --- a/game/persistence/savemanager.py +++ b/game/persistence/savemanager.py @@ -17,9 +17,14 @@ class SaveManager: def __init__(self, game: Game) -> None: self.game = game self.player_save_location: Path | None = None - self.autosave_path = self.default_save_directory() / "autosave.liberation.zip" self.last_saved_bundle: SaveGameBundle | None = None + @property + def autosave_path(self) -> Path: + # This is a property rather than a member because it's one less thing we need to + # update when loading a save that may have been copied from another machine. + return self.default_save_directory() / "autosave.liberation.zip" + @property def default_save_location(self) -> Path: if self.player_save_location is not None: @@ -46,6 +51,19 @@ class SaveManager: with self._save_bundle_context() as bundle: bundle.save_start_of_turn(self.game) + def set_loaded_from(self, bundle: SaveGameBundle) -> None: + """Reconfigures this save manager based on the loaded game. + + The SaveManager is persisted to Game, including details like the last saved path + and bundle details. This data is no longer valid if the save was moved manually + (such as from one machine to another), so it needs to be replaced with the load + location. + + This should only be called by SaveGameBundle after a game load. + """ + self.player_save_location = bundle.bundle_path + self.last_saved_bundle = bundle + @contextmanager def _save_bundle_context( self, override_destination: Path | None = None diff --git a/tests/persistence/conftest.py b/tests/persistence/conftest.py index f892c703..fb598496 100644 --- a/tests/persistence/conftest.py +++ b/tests/persistence/conftest.py @@ -4,11 +4,13 @@ from typing import cast import pytest from game import Game +from game.persistence import SaveManager class StubGame: def __init__(self) -> None: self.date = datetime.date.min + self.save_manager = SaveManager(cast(Game, self)) @pytest.fixture diff --git a/tests/persistence/test_savemanager.py b/tests/persistence/test_savemanager.py index 1ae89e22..190b2370 100644 --- a/tests/persistence/test_savemanager.py +++ b/tests/persistence/test_savemanager.py @@ -26,7 +26,7 @@ def setup_persistence_paths(tmp_path: Path) -> None: @pytest.fixture def save_manager(game: Game) -> SaveManager: - return SaveManager(game) + return game.save_manager def test_new_savemanager_saves_to_autosave(save_manager: SaveManager) -> None: @@ -150,3 +150,30 @@ def test_load_reads_correct_data(save_manager: SaveManager) -> None: assert SaveManager.load_last_turn(bundle_path).date == last_turn_date assert SaveManager.load_start_of_turn(bundle_path).date == start_of_turn_date assert SaveManager.load_player_save(bundle_path).date == player_date + + +def test_save_after_loading_foreign_save( + save_manager: SaveManager, tmp_path: Path +) -> None: + """Tests that we can save games that were copied from another machine. + + Regression test for https://github.com/dcs-liberation/dcs_liberation/issues/2756. + """ + # To simulate the situation from the bug, we save a game to a directory, move it out + # of that directory, delete the directory, then attempt to load and save the game. + # It should save to the new location. If it tries to save to the old location, it + # will fail because the directory does not exist. + + # Create the save on "the other machine"... + bad_directory = tmp_path / "other-machine" + bad_directory.mkdir() + bad_save_path = bad_directory / "foo.liberation.zip" + save_manager.save_player(override_destination=bad_save_path) + + good_save_path = tmp_path / "foo.liberation.zip" + bad_save_path.rename(good_save_path) + bad_directory.rmdir() + + game = SaveManager.load_player_save(good_save_path) + assert game.save_manager.player_save_location == good_save_path + game.save_manager.save_player()