Merge pull request #11181 from unknownbrackets/savestate

SaveState: Show warning on old / long use state

Common/ChunkFile.cpp

@@ -225,7 +225,7 @@ CChunkFileReader::Error CChunkFileReader::GetFileTitle(const std::string &filena
 	return LoadFileHeader(pFile, header, title);
 }
 
-CChunkFileReader::Error CChunkFileReader::LoadFile(const std::string &filename, const char *gitVersion, u8 *&_buffer, size_t &sz, std::string *failureReason) {
+CChunkFileReader::Error CChunkFileReader::LoadFile(const std::string &filename, std::string *gitVersion, u8 *&_buffer, size_t &sz, std::string *failureReason) {
 	if (!File::Exists(filename)) {
 		*failureReason = "LoadStateDoesntExist";
 		ERROR_LOG(SAVESTATE, "ChunkReader: File doesn't exist");
@@ -264,6 +264,12 @@ CChunkFileReader::Error CChunkFileReader::LoadFile(const std::string &filename,
 		delete [] buffer;
 	}
 
+	if (header.GitVersion[31]) {
+		*gitVersion = std::string(header.GitVersion, 32);
+	} else {
+		*gitVersion = header.GitVersion;
+	}
+
 	return ERROR_NONE;
 }
 

Common/ChunkFile.h

@@ -622,7 +622,7 @@ public:
 
 	// Load file template
 	template<class T>
-	static Error Load(const std::string &filename, const char *gitVersion, T& _class, std::string *failureReason)
+	static Error Load(const std::string &filename, std::string *gitVersion, T& _class, std::string *failureReason)
 	{
 		*failureReason = "LoadStateWrongVersion";
 
@@ -700,7 +700,7 @@ private:
 		REVISION_CURRENT = REVISION_TITLE,
 	};
 
-	static Error LoadFile(const std::string &filename, const char *gitVersion, u8 *&buffer, size_t &sz, std::string *failureReason);
+	static Error LoadFile(const std::string &filename, std::string *gitVersion, u8 *&buffer, size_t &sz, std::string *failureReason);
 	static Error SaveFile(const std::string &filename, const std::string &title, const char *gitVersion, u8 *buffer, size_t sz);
 	static Error LoadFileHeader(File::IOFile &pFile, SChunkHeader &header, std::string *title);
 };

Core/Config.cpp

@@ -416,6 +416,7 @@ static ConfigSetting cpuSettings[] = {
 	ConfigSetting("FastMemoryAccess", &g_Config.bFastMemory, true, true, true),
 	ReportedConfigSetting("FuncReplacements", &g_Config.bFuncReplacements, true, true, true),
 	ConfigSetting("HideSlowWarnings", &g_Config.bHideSlowWarnings, false, true, false),
+	ConfigSetting("HideStateWarnings", &g_Config.bHideStateWarnings, false, true, true),
 	ConfigSetting("PreloadFunctions", &g_Config.bPreloadFunctions, false, true, true),
 	ReportedConfigSetting("CPUSpeed", &g_Config.iLockedCPUSpeed, 0, true, true),
 

Core/Config.h

@@ -129,6 +129,7 @@ public:
 	bool bForceLagSync;
 	bool bFuncReplacements;
 	bool bHideSlowWarnings;
+	bool bHideStateWarnings;
 	bool bPreloadFunctions;
 
 	bool bSeparateSASThread;

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,11 @@ namespace SaveState
 	static std::vector<Operation> pending;
 	static std::mutex mutex;
 	static bool hasLoadedState = false;
+	static const int STALE_STATE_USES = 2;
+	// 4 hours of total gameplay since the virtual PSP started the game.
+	static const u64 STALE_STATE_TIME = 4 * 3600 * 1000;
+	static int saveStateGeneration = 0;
+	static std::string saveStateInitialGitVersion = "";
 
 	// TODO: Should this be configurable?
 	static const int REWIND_NUM_STATES = 20;
@@ -252,10 +258,22 @@ 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) {
+			// This only increments on save, of course.
+			++saveStateGeneration;
+			p.Do(saveStateGeneration);
+			// This saves the first git version to create this save state (or generation of save states.)
+			if (saveStateInitialGitVersion.empty())
+				saveStateInitialGitVersion = PPSSPP_GIT_VERSION;
+			p.Do(saveStateInitialGitVersion);
+		} else {
+			saveStateGeneration = 1;
+		}
+
 		// Gotta do CoreTiming first since we'll restore into it.
 		CoreTiming::DoState(p);
 
@@ -419,7 +437,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 +470,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) {
+			auto renameCallback = [=](Status status, const std::string &message, void *data) {
-				if (status) {
+				if (status != Status::FAILURE) {
 					if (g_Config.bEnableStateUndo) {
 						DeleteIfExists(fnUndo);
 						RenameIfExists(fn, fnUndo);
@@ -476,7 +494,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 +636,29 @@ namespace SaveState
 	}
 #endif
 
-	bool HasLoadedState()
+	bool HasLoadedState() {
-	{
 		return hasLoadedState;
 	}
 
+	bool IsStale() {
+		if (saveStateGeneration >= STALE_STATE_USES) {
+			return CoreTiming::GetGlobalTimeUs() > STALE_STATE_TIME;
+		}
+		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 +683,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;
@@ -664,11 +701,26 @@ namespace SaveState
 			{
 			case SAVESTATE_LOAD:
 				INFO_LOG(SAVESTATE, "Loading state from %s", op.filename.c_str());
-				result = CChunkFileReader::Load(op.filename, PPSSPP_GIT_VERSION, state, &reason);
+				// Use the state's latest version as a guess for saveStateInitialGitVersion.
+				result = CChunkFileReader::Load(op.filename, &saveStateInitialGitVersion, state, &reason);
 				if (result == CChunkFileReader::ERROR_NONE) {
 					callbackMessage = sc->T("Loaded State");
-					callbackResult = true;
+					callbackResult = Status::SUCCESS;
 					hasLoadedState = true;
+
+					if (!g_Config.bHideStateWarnings && 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 (!g_Config.bHideStateWarnings && 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 +736,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 +755,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 +771,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;
+				tempResult = CChunkFileReader::Verify(state) == CChunkFileReader::ERROR_NONE;
-				if (callbackResult) {
+				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 +793,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);
+				tempResult = TakeGameScreenshot(op.filename.c_str(), ScreenshotFormat::JPG, SCREENSHOT_DISPLAY);
-				if (!callbackResult) {
+				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 +844,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

@@ -157,10 +157,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) {
+static void AfterStateBoot(SaveState::Status status, const std::string &message, void *ignored) {
-	AfterSaveStateAction(success, message, ignored);
+	AfterSaveStateAction(status, message, ignored);
 	Core_EnableStepping(false);
 	host->UpdateDisassembly();
 }

UI/NativeApp.cpp

@@ -592,9 +592,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);
 	}