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

@@ -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<int>(afterSaveTime - currentTime);
@@ -2016,7 +2016,7 @@ bool AttemptAutoSave(AutoSaveReason reason)
 		}
 	}
 	SetEventHandler(saveProc);
-	return gbValidSaveFile;
+	return saveSucceeded;
 }
 
 void InitKeymapActions()

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();
 	}

Source/loadsave.cpp

@@ -54,8 +54,30 @@ namespace {
 constexpr size_t MaxMissilesForSaveGame = 125;
 constexpr size_t PlayerWalkPathSizeForSaveGame = 25;
 
-uint8_t giNumberQuests;
+uint8_t giNumberQuests;
-uint8_t giNumberOfSmithPremiumItems;
+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 <class T>
 T SwapLE(T in)
@@ -2680,7 +2702,7 @@ tl::expected<void, std::string> 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();
+	const bool gameSaved = kind == SaveKind::Manual
-	if (!gameSaved) {
+	    ? pfile_write_manual_game_with_backup()
-		gbValidSaveFile = false;
+	    : pfile_write_auto_game();
-		return;
+	if (!gameSaved)
-	}
+		return GetSaveFailureResult();
 
-	if (!pfile_write_stash_with_backup()) {
+	gbValidSaveFile = true;
-		gbValidSaveFile = false;
+	if (!pfile_write_stash_with_backup())
-		return;
+		return GetSaveFailureResult();
-	}
+
+	return SaveResult::Success;
 #endif
 }
 

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<void, std::string> 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<void, std::string> LoadLevel();
 tl::expected<void, std::string> ConvertLevels(SaveWriter &saveWriter);

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);
+bool pfile_write_auto_game()
-	const bool saveIsValid = archive && ArchiveContainsGame(*archive);
+{
-	if (saveIsValid || !(FileExists(backupLocation) || DirectoryExists(backupLocation.c_str())))
+	const std::string restorePrefix = "autosave_restore_";
-		return saveIsValid;
+	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;

Source/pfile.h

@@ -101,7 +101,8 @@ std::optional<SaveReader> OpenStashArchive();
 const char *pfile_get_password();
 std::unique_ptr<std::byte[]> 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