From a5efb85ab7cf47a2b3b2fe75111752b94db201f4 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Thu, 14 Jun 2018 17:52:44 -0700 Subject: [PATCH 1/4] 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 | 99 +++++++++++++++++++++++++++++--------- Core/SaveState.h | 13 ++++- Qt/mainwindow.cpp | 4 +- UI/EmuScreen.cpp | 8 +-- UI/NativeApp.cpp | 4 +- UI/PauseScreen.cpp | 6 +-- Windows/MainWindowMenu.cpp | 4 +- 7 files changed, 101 insertions(+), 37 deletions(-) diff --git a/Core/SaveState.cpp b/Core/SaveState.cpp index 30889f8e71..7b3fd33138 100644 --- a/Core/SaveState.cpp +++ b/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 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() diff --git a/Core/SaveState.h b/Core/SaveState.h index 64c4dbe0c7..ec9efe7bb5 100644 --- a/Core/SaveState.h +++ b/Core/SaveState.h @@ -23,7 +23,12 @@ namespace SaveState { - typedef std::function Callback; + enum class Status { + FAILURE, + WARNING, + SUCCESS, + }; + typedef std::function 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(); }; diff --git a/Qt/mainwindow.cpp b/Qt/mainwindow.cpp index 904edc6095..620ebc77e9 100644 --- a/Qt/mainwindow.cpp +++ b/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"); diff --git a/UI/EmuScreen.cpp b/UI/EmuScreen.cpp index 35cf6a0e0e..e0d84e15f3 100644 --- a/UI/EmuScreen.cpp +++ b/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(); } diff --git a/UI/NativeApp.cpp b/UI/NativeApp.cpp index 97b42ffca1..6176680297 100644 --- a/UI/NativeApp.cpp +++ b/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); } }); } diff --git a/UI/PauseScreen.cpp b/UI/PauseScreen.cpp index b3622fc90f..f066103ba7 100644 --- a/UI/PauseScreen.cpp +++ b/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 { diff --git a/Windows/MainWindowMenu.cpp b/Windows/MainWindowMenu.cpp index caa5ae5511..051d9aa521 100644 --- a/Windows/MainWindowMenu.cpp +++ b/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); } From 006ef96b195a333af2b9deae4387baf65562f231 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Thu, 14 Jun 2018 17:54:13 -0700 Subject: [PATCH 2/4] SaveState: Add a setting to ignore warnings. In case you like to collect old and obscure bugs - everyone's got a hobby. --- Core/Config.cpp | 1 + Core/Config.h | 1 + Core/SaveState.cpp | 4 ++-- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Core/Config.cpp b/Core/Config.cpp index b458914d10..c3947bd22c 100644 --- a/Core/Config.cpp +++ b/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), diff --git a/Core/Config.h b/Core/Config.h index c7c7e02b13..978b15614f 100644 --- a/Core/Config.h +++ b/Core/Config.h @@ -129,6 +129,7 @@ public: bool bForceLagSync; bool bFuncReplacements; bool bHideSlowWarnings; + bool bHideStateWarnings; bool bPreloadFunctions; bool bSeparateSASThread; diff --git a/Core/SaveState.cpp b/Core/SaveState.cpp index 7b3fd33138..d665e33689 100644 --- a/Core/SaveState.cpp +++ b/Core/SaveState.cpp @@ -705,14 +705,14 @@ namespace SaveState callbackResult = Status::SUCCESS; hasLoadedState = true; - if (IsStale()) { + 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 (IsOldVersion()) { + } 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; From 16b11138b4e63c11465452b3ba49be36f4a4efc6 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Thu, 14 Jun 2018 18:25:52 -0700 Subject: [PATCH 3/4] SaveState: Use latest version if initial missing. This may be useful for debugging or if we decide on a buffer between versions. --- Common/ChunkFile.cpp | 8 +++++++- Common/ChunkFile.h | 4 ++-- Core/SaveState.cpp | 4 ++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Common/ChunkFile.cpp b/Common/ChunkFile.cpp index 3560c6eeb4..cbc8dc1808 100644 --- a/Common/ChunkFile.cpp +++ b/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; } diff --git a/Common/ChunkFile.h b/Common/ChunkFile.h index 6fc76ddb29..dba69b0da0 100644 --- a/Common/ChunkFile.h +++ b/Common/ChunkFile.h @@ -622,7 +622,7 @@ public: // Load file template template - 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); }; diff --git a/Core/SaveState.cpp b/Core/SaveState.cpp index d665e33689..7d4327307d 100644 --- a/Core/SaveState.cpp +++ b/Core/SaveState.cpp @@ -268,7 +268,6 @@ namespace SaveState p.Do(saveStateInitialGitVersion); } else { saveStateGeneration = 1; - saveStateInitialGitVersion = "v0.0.1"; } // Gotta do CoreTiming first since we'll restore into it. @@ -699,7 +698,8 @@ 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 = Status::SUCCESS; From 76ff2adda980c42d5e5bf733e47b18ad66c31699 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 16 Jun 2018 08:06:35 -0700 Subject: [PATCH 4/4] SaveState: Show only after 4 hours. This is 4 hours of the virtual PSP running the game continuously, perhaps from multiple PPSSPP play sessions using save states. --- Core/SaveState.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Core/SaveState.cpp b/Core/SaveState.cpp index 7d4327307d..7d336dd993 100644 --- a/Core/SaveState.cpp +++ b/Core/SaveState.cpp @@ -241,7 +241,9 @@ namespace SaveState static std::vector pending; static std::mutex mutex; static bool hasLoadedState = false; - static const int STALE_STATE_USES = 10; + 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 = ""; @@ -261,8 +263,10 @@ namespace SaveState 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); @@ -638,8 +642,7 @@ namespace SaveState bool IsStale() { if (saveStateGeneration >= STALE_STATE_USES) { - // Don't show it every time. - return saveStateGeneration % 5 == 0; + return CoreTiming::GetGlobalTimeUs() > STALE_STATE_TIME; } return false; }