From 1a00a97ef3a038bf8f8e50b013f8f30843fd4986 Mon Sep 17 00:00:00 2001 From: morfidon <57798071+morfidon@users.noreply.github.com> Date: Wed, 11 Mar 2026 14:53:10 +0100 Subject: [PATCH] Clarify save semantics for manual saves and autosaves Split the save flow into explicit manual and auto save kinds and return a SaveResult instead of overloading gbValidSaveFile as the outcome of the last save attempt. Redesign backup handling so the persistent backup slot represents the last manual save, while autosave uses its own temporary restore copy and no longer overwrites the manual backup. Manual save UI now reports failure separately from preserved-save recovery, and autosave only reports success when the new save actually succeeds. --- Source/diablo.cpp | 8 +++--- Source/gamemenu.cpp | 19 ++++++++++-- Source/loadsave.cpp | 53 ++++++++++++++++++++++++---------- Source/loadsave.h | 13 ++++++++- Source/pfile.cpp | 70 ++++++++++++++++++++++++++++++++++++++------- Source/pfile.h | 3 +- 6 files changed, 131 insertions(+), 35 deletions(-) diff --git a/Source/diablo.cpp b/Source/diablo.cpp index 7ec96fdce..0e2c2f7cc 100644 --- a/Source/diablo.cpp +++ b/Source/diablo.cpp @@ -1981,7 +1981,6 @@ void RequestAutoSave(AutoSaveReason reason) QueueAutoSave(reason); } - void QueueAutoSave(AutoSaveReason reason) { if (gbIsMultiplayer) @@ -2003,11 +2002,12 @@ bool AttemptAutoSave(AutoSaveReason reason) const EventHandler saveProc = SetEventHandler(DisableInputEventHandler); const uint32_t currentTime = SDL_GetTicks(); - SaveGame(); + const SaveResult saveResult = SaveGame(SaveKind::Auto); const uint32_t afterSaveTime = SDL_GetTicks(); + const bool saveSucceeded = saveResult == SaveResult::Success; autoSaveCooldownUntil = afterSaveTime + AutoSaveCooldownMilliseconds; - if (gbValidSaveFile) { + if (saveSucceeded) { autoSaveNextTimerDueAt = afterSaveTime + GetAutoSaveIntervalMilliseconds(); if (reason != AutoSaveReason::Timer) { const int timeElapsed = static_cast(afterSaveTime - currentTime); @@ -2016,7 +2016,7 @@ bool AttemptAutoSave(AutoSaveReason reason) } } SetEventHandler(saveProc); - return gbValidSaveFile; + return saveSucceeded; } void InitKeymapActions() diff --git a/Source/gamemenu.cpp b/Source/gamemenu.cpp index 5fd82eecc..e331fec2b 100644 --- a/Source/gamemenu.cpp +++ b/Source/gamemenu.cpp @@ -40,6 +40,9 @@ bool isGameMenuOpen = false; namespace { +constexpr const char *SaveFailedPreservedMessage = N_("Save failed. The previous save is still available."); +constexpr const char *SaveFailedNoValidMessage = N_("Save failed. No valid save is available."); + // Forward-declare menu handlers, used by the global menu structs below. void GamemenuPrevious(bool bActivate); void GamemenuNewGame(bool bActivate); @@ -377,12 +380,22 @@ void gamemenu_save_game(bool /*bActivate*/) RedrawEverything(); DrawAndBlit(); const uint32_t currentTime = SDL_GetTicks(); - SaveGame(); + const SaveResult saveResult = SaveGame(SaveKind::Manual); ClrDiabloMsg(); - InitDiabloMsg(EMSG_GAME_SAVED, currentTime + 1000 - SDL_GetTicks()); + switch (saveResult) { + case SaveResult::Success: + InitDiabloMsg(EMSG_GAME_SAVED, currentTime + 1000 - SDL_GetTicks()); + break; + case SaveResult::FailedButPreviousSavePreserved: + InitDiabloMsg(_(SaveFailedPreservedMessage)); + break; + case SaveResult::FailedNoValidSave: + InitDiabloMsg(_(SaveFailedNoValidMessage)); + break; + } RedrawEverything(); NewCursor(CURSOR_HAND); - if (CornerStone.activated) { + if (saveResult == SaveResult::Success && CornerStone.activated) { CornerstoneSave(); if (!demo::IsRunning()) SaveOptions(); } diff --git a/Source/loadsave.cpp b/Source/loadsave.cpp index 8b9b6d6d7..98ad84ae6 100644 --- a/Source/loadsave.cpp +++ b/Source/loadsave.cpp @@ -54,8 +54,30 @@ namespace { constexpr size_t MaxMissilesForSaveGame = 125; constexpr size_t PlayerWalkPathSizeForSaveGame = 25; -uint8_t giNumberQuests; -uint8_t giNumberOfSmithPremiumItems; +uint8_t giNumberQuests; +uint8_t giNumberOfSmithPremiumItems; + +bool ActiveSaveContainsGame() +{ + if (gbIsMultiplayer) + return false; + + auto archive = OpenSaveArchive(gSaveNumber); + if (!archive) + return false; + + auto gameData = ReadArchive(*archive, "game"); + if (gameData == nullptr) + return false; + + return IsHeaderValid(LoadLE32(gameData.get())); +} + +SaveResult GetSaveFailureResult() +{ + gbValidSaveFile = ActiveSaveContainsGame(); + return gbValidSaveFile ? SaveResult::FailedButPreviousSavePreserved : SaveResult::FailedNoValidSave; +} template T SwapLE(T in) @@ -2680,7 +2702,7 @@ tl::expected LoadGame(bool firstflag) gbProcessPlayers = IsDiabloAlive(!firstflag); if (gbIsHellfireSaveGame != gbIsHellfire) { - SaveGame(); + SaveGame(SaveKind::Manual); } gbIsHellfireSaveGame = gbIsHellfire; @@ -2926,24 +2948,25 @@ void SaveGameData(SaveWriter &saveWriter) SaveLevelSeeds(saveWriter); } -void SaveGame() +SaveResult SaveGame(SaveKind kind) { - gbValidSaveFile = true; - #if defined(UNPACKED_SAVES) && defined(DVL_NO_FILESYSTEM) pfile_write_hero(/*writeGameData=*/true); sfile_write_stash(); + gbValidSaveFile = true; + return SaveResult::Success; #else - const bool gameSaved = pfile_write_game_with_backup(); - if (!gameSaved) { - gbValidSaveFile = false; - return; - } + const bool gameSaved = kind == SaveKind::Manual + ? pfile_write_manual_game_with_backup() + : pfile_write_auto_game(); + if (!gameSaved) + return GetSaveFailureResult(); - if (!pfile_write_stash_with_backup()) { - gbValidSaveFile = false; - return; - } + gbValidSaveFile = true; + if (!pfile_write_stash_with_backup()) + return GetSaveFailureResult(); + + return SaveResult::Success; #endif } diff --git a/Source/loadsave.h b/Source/loadsave.h index 0d139e143..0531edc5b 100644 --- a/Source/loadsave.h +++ b/Source/loadsave.h @@ -18,6 +18,17 @@ namespace devilution { extern DVL_API_FOR_TEST bool gbIsHellfireSaveGame; extern DVL_API_FOR_TEST uint8_t giNumberOfLevels; +enum class SaveKind : uint8_t { + Manual, + Auto, +}; + +enum class SaveResult : uint8_t { + Success, + FailedButPreviousSavePreserved, + FailedNoValidSave, +}; + void RemoveInvalidItem(Item &pItem); _item_indexes RemapItemIdxFromDiablo(_item_indexes i); _item_indexes RemapItemIdxToDiablo(_item_indexes i); @@ -40,7 +51,7 @@ tl::expected LoadGame(bool firstflag); void SaveHotkeys(SaveWriter &saveWriter, const Player &player); void SaveHeroItems(SaveWriter &saveWriter, Player &player); void SaveGameData(SaveWriter &saveWriter); -void SaveGame(); +SaveResult SaveGame(SaveKind kind); void SaveLevel(SaveWriter &saveWriter); tl::expected LoadLevel(); tl::expected ConvertLevels(SaveWriter &saveWriter); diff --git a/Source/pfile.cpp b/Source/pfile.cpp index d2b0e9985..a4865c252 100644 --- a/Source/pfile.cpp +++ b/Source/pfile.cpp @@ -171,6 +171,11 @@ SaveWriter GetStashWriter() } #if !(defined(UNPACKED_SAVES) && defined(DVL_NO_FILESYSTEM)) +bool SaveLocationExists(const std::string &location) +{ + return FileExists(location.c_str()) || DirectoryExists(location.c_str()); +} + void CopySaveLocation(const std::string &sourceLocation, const std::string &targetLocation) { #if defined(UNPACKED_SAVES) @@ -202,6 +207,22 @@ void RestoreSaveLocation(const std::string &targetLocation, const std::string &b CopyFileOverwrite(backupLocation.c_str(), targetLocation.c_str()); #endif } + +void DeleteSaveLocation(const std::string &location) +{ +#if defined(UNPACKED_SAVES) + if (!DirectoryExists(location.c_str())) + return; + + for (const std::filesystem::directory_entry &entry : std::filesystem::directory_iterator(location)) + RemoveFile(entry.path().string().c_str()); + + std::filesystem::remove(location); +#else + if (FileExists(location.c_str())) + RemoveFile(location.c_str()); +#endif +} #endif void Game2UiPlayer(const Player &player, _uiheroinfo *heroinfo, bool bHasSaveFile) @@ -644,25 +665,52 @@ void pfile_write_hero(bool writeGameData) } #if !(defined(UNPACKED_SAVES) && defined(DVL_NO_FILESYSTEM)) -bool pfile_write_game_with_backup() +bool SaveWrittenGameIsValid() +{ + auto archive = OpenSaveArchive(gSaveNumber); + return archive && ArchiveContainsGame(*archive); +} + +bool WriteGameAndRestoreOnFailure(const std::string &restoreLocation) +{ + const bool hasRestoreLocation = SaveLocationExists(restoreLocation); + + pfile_write_hero(/*writeGameData=*/true); + + if (SaveWrittenGameIsValid()) + return true; + + if (!hasRestoreLocation) + return false; + + RestoreSaveLocation(GetSavePath(gSaveNumber), restoreLocation); + return false; +} + +bool pfile_write_manual_game_with_backup() { const std::string backupPrefix = "backup_"; const std::string backupLocation = GetSavePath(gSaveNumber, backupPrefix); const std::string saveLocation = GetSavePath(gSaveNumber); - if (FileExists(saveLocation) || DirectoryExists(saveLocation.c_str())) + if (SaveLocationExists(saveLocation)) CopySaveLocation(saveLocation, backupLocation); - pfile_write_hero(/*writeGameData=*/true); + return WriteGameAndRestoreOnFailure(backupLocation); +} - auto archive = OpenSaveArchive(gSaveNumber); - const bool saveIsValid = archive && ArchiveContainsGame(*archive); - if (saveIsValid || !(FileExists(backupLocation) || DirectoryExists(backupLocation.c_str()))) - return saveIsValid; +bool pfile_write_auto_game() +{ + const std::string restorePrefix = "autosave_restore_"; + const std::string restoreLocation = GetSavePath(gSaveNumber, restorePrefix); + const std::string saveLocation = GetSavePath(gSaveNumber); - RestoreSaveLocation(saveLocation, backupLocation); + if (SaveLocationExists(saveLocation)) + CopySaveLocation(saveLocation, restoreLocation); - return false; + const bool saveIsValid = WriteGameAndRestoreOnFailure(restoreLocation); + DeleteSaveLocation(restoreLocation); + return saveIsValid; } bool pfile_write_stash_with_backup() @@ -674,7 +722,7 @@ bool pfile_write_stash_with_backup() const std::string backupLocation = GetStashSavePath(backupPrefix); const std::string stashLocation = GetStashSavePath(); - if (FileExists(stashLocation) || DirectoryExists(stashLocation.c_str())) + if (SaveLocationExists(stashLocation)) CopySaveLocation(stashLocation, backupLocation); SaveWriter stashWriter = GetStashWriter(); @@ -683,7 +731,7 @@ bool pfile_write_stash_with_backup() auto archive = OpenStashArchive(); const char *stashFileName = gbIsMultiplayer ? "mpstashitems" : "spstashitems"; const bool stashIsValid = archive && ReadArchive(*archive, stashFileName) != nullptr; - if (stashIsValid || !(FileExists(backupLocation) || DirectoryExists(backupLocation.c_str()))) { + if (stashIsValid || !SaveLocationExists(backupLocation)) { if (stashIsValid) Stash.dirty = false; return stashIsValid; diff --git a/Source/pfile.h b/Source/pfile.h index 6960cd857..307d20d5c 100644 --- a/Source/pfile.h +++ b/Source/pfile.h @@ -101,7 +101,8 @@ std::optional OpenStashArchive(); const char *pfile_get_password(); std::unique_ptr ReadArchive(SaveReader &archive, const char *pszName, size_t *pdwLen = nullptr); void pfile_write_hero(bool writeGameData = false); -bool pfile_write_game_with_backup(); +bool pfile_write_manual_game_with_backup(); +bool pfile_write_auto_game(); bool pfile_write_stash_with_backup(); #ifndef DISABLE_DEMOMODE