SaveState: Show warning on old / long use state.

Using save states instead of in game saves causes bugs in games, and
preserves bugs from bad settings and old PPSSPP versions.

This tells users when they might be affected.

Core/SaveState.cpp

@@ -23,6 +23,7 @@
 #include "base/timeutil.h"
 #include "i18n/i18n.h"
 #include "thread/threadutil.h"
+#include "util/text/parsers.h"
 
 #include "Common/FileUtil.h"
 #include "Common/ChunkFile.h"
@@ -240,6 +241,9 @@ namespace SaveState
 	static std::vector<Operation> pending;
 	static std::mutex mutex;
 	static bool hasLoadedState = false;
+	static const int STALE_STATE_USES = 10;
+	static int saveStateGeneration = 0;
+	static std::string saveStateInitialGitVersion = "";
 
 	// TODO: Should this be configurable?
 	static const int REWIND_NUM_STATES = 20;
@@ -252,10 +256,21 @@ namespace SaveState
 
 	void SaveStart::DoState(PointerWrap &p)
 	{
-		auto s = p.Section("SaveStart", 1);
+		auto s = p.Section("SaveStart", 1, 2);
 		if (!s)
 			return;
 
+		if (s >= 2) {
+			++saveStateGeneration;
+			p.Do(saveStateGeneration);
+			if (saveStateInitialGitVersion.empty())
+				saveStateInitialGitVersion = PPSSPP_GIT_VERSION;
+			p.Do(saveStateInitialGitVersion);
+		} else {
+			saveStateGeneration = 1;
+			saveStateInitialGitVersion = "v0.0.1";
+		}
+
 		// Gotta do CoreTiming first since we'll restore into it.
 		CoreTiming::DoState(p);
 
@@ -419,7 +434,7 @@ namespace SaveState
 		} else {
 			I18NCategory *sy = GetI18NCategory("System");
 			if (callback)
-				callback(false, sy->T("Failed to load state. Error in the file system."), cbUserData);
+				callback(Status::FAILURE, sy->T("Failed to load state. Error in the file system."), cbUserData);
 		}
 	}
 
@@ -452,8 +467,8 @@ namespace SaveState
 		std::string fnUndo = GenerateSaveSlotFilename(gameFilename, slot, UNDO_STATE_EXTENSION);
 		std::string shotUndo = GenerateSaveSlotFilename(gameFilename, slot, UNDO_SCREENSHOT_EXTENSION);
 		if (!fn.empty()) {
-			auto renameCallback = [=](bool status, const std::string &message, void *data) {
-				if (status) {
+			auto renameCallback = [=](Status status, const std::string &message, void *data) {
+				if (status != Status::FAILURE) {
 					if (g_Config.bEnableStateUndo) {
 						DeleteIfExists(fnUndo);
 						RenameIfExists(fn, fnUndo);
@@ -476,7 +491,7 @@ namespace SaveState
 		} else {
 			I18NCategory *sy = GetI18NCategory("System");
 			if (callback)
-				callback(false, sy->T("Failed to save state. Error in the file system."), cbUserData);
+				callback(Status::FAILURE, sy->T("Failed to save state. Error in the file system."), cbUserData);
 		}
 	}
 
@@ -618,11 +633,30 @@ namespace SaveState
 	}
 #endif
 
-	bool HasLoadedState()
-	{
+	bool HasLoadedState() {
 		return hasLoadedState;
 	}
 
+	bool IsStale() {
+		if (saveStateGeneration >= STALE_STATE_USES) {
+			// Don't show it every time.
+			return saveStateGeneration % 5 == 0;
+		}
+		return false;
+	}
+
+	bool IsOldVersion() {
+		if (saveStateInitialGitVersion.empty())
+			return false;
+
+		Version state(saveStateInitialGitVersion);
+		Version gitVer(PPSSPP_GIT_VERSION);
+		if (!state.IsValid() || !gitVer.IsValid())
+			return false;
+
+		return state < gitVer;
+	}
+
 	void Process()
 	{
 #ifndef MOBILE_DEVICE
@@ -647,7 +681,8 @@ namespace SaveState
 		{
 			Operation &op = operations[i];
 			CChunkFileReader::Error result;
-			bool callbackResult;
+			Status callbackResult;
+			bool tempResult;
 			std::string callbackMessage;
 			std::string reason;
 			std::string title;
@@ -667,8 +702,22 @@ namespace SaveState
 				result = CChunkFileReader::Load(op.filename, PPSSPP_GIT_VERSION, state, &reason);
 				if (result == CChunkFileReader::ERROR_NONE) {
 					callbackMessage = sc->T("Loaded State");
-					callbackResult = true;
+					callbackResult = Status::SUCCESS;
 					hasLoadedState = true;
+
+					if (IsStale()) {
+						// For anyone wondering why (too long to put on the screen in an osm):
+						// Using save states instead of saves simulates many hour play sessions.
+						// Sometimes this exposes game bugs that were rarely seen on real devices,
+						// because few people played on a real PSP for 10 hours straight.
+						callbackMessage = sc->T("Loaded.  Save in game, restart, and load for less bugs.");
+						callbackResult = Status::WARNING;
+					} else if (IsOldVersion()) {
+						// Save states also preserve bugs from old PPSSPP versions, so warn.
+						callbackMessage = sc->T("Loaded.  Save in game, restart, and load for less bugs.");
+						callbackResult = Status::WARNING;
+					}
+
 #ifndef MOBILE_DEVICE
 					if (g_Config.bSaveLoadResetsAVdumping) {
 						if (g_Config.bDumpFrames) {
@@ -684,10 +733,10 @@ namespace SaveState
 					HandleFailure();
 					callbackMessage = i18nLoadFailure;
 					ERROR_LOG(SAVESTATE, "Load state failure: %s", reason.c_str());
-					callbackResult = false;
+					callbackResult = Status::FAILURE;
 				} else {
 					callbackMessage = sc->T(reason.c_str(), i18nLoadFailure);
-					callbackResult = false;
+					callbackResult = Status::FAILURE;
 				}
 				break;
 
@@ -703,7 +752,7 @@ namespace SaveState
 				result = CChunkFileReader::Save(op.filename, title, PPSSPP_GIT_VERSION, state);
 				if (result == CChunkFileReader::ERROR_NONE) {
 					callbackMessage = sc->T("Saved State");
-					callbackResult = true;
+					callbackResult = Status::SUCCESS;
 #ifndef MOBILE_DEVICE
 					if (g_Config.bSaveLoadResetsAVdumping) {
 						if (g_Config.bDumpFrames) {
@@ -719,16 +768,17 @@ namespace SaveState
 					HandleFailure();
 					callbackMessage = i18nSaveFailure;
 					ERROR_LOG(SAVESTATE, "Save state failure: %s", reason.c_str());
-					callbackResult = false;
+					callbackResult = Status::FAILURE;
 				} else {
 					callbackMessage = i18nSaveFailure;
-					callbackResult = false;
+					callbackResult = Status::FAILURE;
 				}
 				break;
 
 			case SAVESTATE_VERIFY:
-				callbackResult = CChunkFileReader::Verify(state) == CChunkFileReader::ERROR_NONE;
-				if (callbackResult) {
+				tempResult = CChunkFileReader::Verify(state) == CChunkFileReader::ERROR_NONE;
+				callbackResult = tempResult ? Status::SUCCESS : Status::FAILURE;
+				if (tempResult) {
 					INFO_LOG(SAVESTATE, "Verified save state system");
 				} else {
 					ERROR_LOG(SAVESTATE, "Save state system verification failed");
@@ -740,35 +790,36 @@ namespace SaveState
 				result = rewindStates.Restore();
 				if (result == CChunkFileReader::ERROR_NONE) {
 					callbackMessage = sc->T("Loaded State");
-					callbackResult = true;
+					callbackResult = Status::SUCCESS;
 					hasLoadedState = true;
 				} else if (result == CChunkFileReader::ERROR_BROKEN_STATE) {
 					// Cripes.  Good news is, we might have more.  Let's try those too, better than a reset.
 					if (HandleFailure()) {
 						// Well, we did rewind, even if too much...
 						callbackMessage = sc->T("Loaded State");
-						callbackResult = true;
+						callbackResult = Status::SUCCESS;
 						hasLoadedState = true;
 					} else {
 						callbackMessage = i18nLoadFailure;
-						callbackResult = false;
+						callbackResult = Status::FAILURE;
 					}
 				} else {
 					callbackMessage = i18nLoadFailure;
-					callbackResult = false;
+					callbackResult = Status::FAILURE;
 				}
 				break;
 
 			case SAVESTATE_SAVE_SCREENSHOT:
-				callbackResult = TakeGameScreenshot(op.filename.c_str(), ScreenshotFormat::JPG, SCREENSHOT_DISPLAY);
-				if (!callbackResult) {
+				tempResult = TakeGameScreenshot(op.filename.c_str(), ScreenshotFormat::JPG, SCREENSHOT_DISPLAY);
+				callbackResult = tempResult ? Status::SUCCESS : Status::FAILURE;
+				if (!tempResult) {
 					ERROR_LOG(SAVESTATE, "Failed to take a screenshot for the savestate! %s", op.filename.c_str());
 				}
 				break;
 
 			default:
 				ERROR_LOG(SAVESTATE, "Savestate failure: unknown operation type %d", op.type);
-				callbackResult = false;
+				callbackResult = Status::FAILURE;
 				break;
 			}
 
@@ -790,6 +841,8 @@ namespace SaveState
 		rewindStates.Clear();
 
 		hasLoadedState = false;
+		saveStateGeneration = 0;
+		saveStateInitialGitVersion.clear();
 	}
 
 	void Shutdown()

Core/SaveState.h

@@ -23,7 +23,12 @@
 
 namespace SaveState
 {
-	typedef std::function<void(bool status, const std::string &message, void *cbUserData)> Callback;
+	enum class Status {
+		FAILURE,
+		WARNING,
+		SUCCESS,
+	};
+	typedef std::function<void(Status status, const std::string &message, void *cbUserData)> Callback;
 
 	static const int NUM_SLOTS = 5;
 	static const char *STATE_EXTENSION = "ppst";
@@ -79,6 +84,12 @@ namespace SaveState
 	// Returns true if a savestate has been used during this session.
 	bool HasLoadedState();
 
+	// Returns true if the state has been reused instead of real saves many times.
+	bool IsStale();
+
+	// Returns true if state is from an older PPSSPP version.
+	bool IsOldVersion();
+
 	// Check if there's any save stating needing to be done.  Normally called once per frame.
 	void Process();
 };

Qt/mainwindow.cpp

@@ -163,10 +163,10 @@ void MainWindow::closeAct()
 	SetGameTitle("");
 }
 
-void SaveStateActionFinished(bool result, const std::string &message, void *userdata)
+void SaveStateActionFinished(SaveState::Status status, const std::string &message, void *userdata)
 {
 	// TODO: Improve messaging?
-	if (!result)
+	if (status == SaveState::Status::FAILURE)
 	{
 		QMessageBox msgBox;
 		msgBox.setWindowTitle("Load Save State");

UI/EmuScreen.cpp

@@ -346,14 +346,14 @@ void EmuScreen::dialogFinished(const Screen *dialog, DialogResult result) {
 	RecreateViews();
 }
 
-static void AfterSaveStateAction(bool success, const std::string &message, void *) {
+static void AfterSaveStateAction(SaveState::Status status, const std::string &message, void *) {
 	if (!message.empty()) {
-		osm.Show(message, 2.0);
+		osm.Show(message, status == SaveState::Status::SUCCESS ? 2.0 : 5.0);
 	}
 }
 
-static void AfterStateBoot(bool success, const std::string &message, void *ignored) {
-	AfterSaveStateAction(success, message, ignored);
+static void AfterStateBoot(SaveState::Status status, const std::string &message, void *ignored) {
+	AfterSaveStateAction(status, message, ignored);
 	Core_EnableStepping(false);
 	host->UpdateDisassembly();
 }

UI/NativeApp.cpp

@@ -590,9 +590,9 @@ void NativeInit(int argc, const char *argv[], const char *savegame_dir, const ch
 #endif
 
 	if (!boot_filename.empty() && stateToLoad != NULL) {
-		SaveState::Load(stateToLoad, [](bool status, const std::string &message, void *) {
+		SaveState::Load(stateToLoad, [](SaveState::Status status, const std::string &message, void *) {
 			if (!message.empty()) {
-				osm.Show(message, 2.0);
+				osm.Show(message, status == SaveState::Status::SUCCESS ? 2.0 : 5.0);
 			}
 		});
 	}

UI/PauseScreen.cpp

@@ -238,9 +238,9 @@ void SaveSlotView::Draw(UIContext &dc) {
 	UI::LinearLayout::Draw(dc);
 }
 
-static void AfterSaveStateAction(bool status, const std::string &message, void *) {
+static void AfterSaveStateAction(SaveState::Status status, const std::string &message, void *) {
 	if (!message.empty()) {
-		osm.Show(message, 2.0);
+		osm.Show(message, status == SaveState::Status::SUCCESS ? 2.0 : 5.0);
 	}
 }
 
@@ -373,7 +373,7 @@ void GamePauseScreen::dialogFinished(const Screen *dialog, DialogResult dr) {
 		ScreenshotViewScreen *s = (ScreenshotViewScreen *)dialog;
 		int slot = s->GetSlot();
 		g_Config.iCurrentStateSlot = slot;
-		SaveState::LoadSlot(gamePath_, slot, SaveState::Callback(), 0);
+		SaveState::LoadSlot(gamePath_, slot, &AfterSaveStateAction);
 
 		finishNextFrame_ = true;
 	} else {

Windows/MainWindowMenu.cpp

@@ -470,9 +470,9 @@ namespace MainWindow {
 		g_Config.iInternalScreenRotation = rotation;
 	}
 
-	static void SaveStateActionFinished(bool result, const std::string &message, void *userdata) {
+	static void SaveStateActionFinished(SaveState::Status status, const std::string &message, void *userdata) {
 		if (!message.empty()) {
-			osm.Show(message, 2.0);
+			osm.Show(message, status == SaveState::Status::SUCCESS ? 2.0 : 5.0);
 		}
 		PostMessage(MainWindow::GetHWND(), WM_USER_SAVESTATE_FINISH, 0, 0);
 	}