From bc0dacf974e7a9518091f6153704aec56294c2d6 Mon Sep 17 00:00:00 2001 From: Dan Albert Date: Mon, 13 Mar 2023 19:31:41 -0700 Subject: [PATCH] Fix saving of saves from other machines. Need to reset the persisted save paths on load, since the file that was loaded may have been moved since it was saved and the original location may no longer be accessible. Fixes https://github.com/dcs-liberation/dcs_liberation/issues/2756. --- game/persistence/savegamebundle.py | 2 +- game/persistence/savemanager.py | 20 +++++++++++++++++- tests/persistence/conftest.py | 2 ++ tests/persistence/test_savemanager.py | 29 ++++++++++++++++++++++++++- 4 files changed, 50 insertions(+), 3 deletions(-) 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()