From 0ba32f7add97daf1c5ea52cf77b75e81d862676f Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sat, 26 Apr 2025 10:33:01 +0200 Subject: [PATCH 1/4] Android: Detect when native code should flush unsaved data --- .../dolphinemu/dolphinemu/utils/ActivityTracker.kt | 11 +++++++++++ Source/Android/jni/ActivityTracker.cpp | 9 +++++++++ Source/Core/UICommon/UICommon.cpp | 5 +++++ Source/Core/UICommon/UICommon.h | 2 ++ 4 files changed, 27 insertions(+) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/utils/ActivityTracker.kt b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/utils/ActivityTracker.kt index 8bcd1bdecb8..62de1f41614 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/utils/ActivityTracker.kt +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/utils/ActivityTracker.kt @@ -56,10 +56,21 @@ class ActivityTracker : ActivityLifecycleCallbacks { override fun onActivitySaveInstanceState(activity: Activity, bundle: Bundle) {} + override fun onActivityPostSaveInstanceState(activity: Activity, bundle: Bundle) { + if (DirectoryInitialization.areDolphinDirectoriesReady() && + !activity.isChangingConfigurations + ) { + flushUnsavedData() + } + } + override fun onActivityDestroyed(activity: Activity) {} companion object { @JvmStatic external fun setBackgroundExecutionAllowedNative(allowed: Boolean) + + @JvmStatic + external fun flushUnsavedData() } } diff --git a/Source/Android/jni/ActivityTracker.cpp b/Source/Android/jni/ActivityTracker.cpp index b8718905769..20fc7df79ad 100644 --- a/Source/Android/jni/ActivityTracker.cpp +++ b/Source/Android/jni/ActivityTracker.cpp @@ -5,6 +5,8 @@ #include "Common/Logging/Log.h" #include "Core/AchievementManager.h" +#include "UICommon/UICommon.h" +#include "jni/Host.h" extern "C" { @@ -18,4 +20,11 @@ Java_org_dolphinemu_dolphinemu_utils_ActivityTracker_setBackgroundExecutionAllow INFO_LOG_FMT(CORE, "SetBackgroundExecutionAllowed {}", allowed); AchievementManager::GetInstance().SetBackgroundExecutionAllowed(allowed); } + +JNIEXPORT void JNICALL +Java_org_dolphinemu_dolphinemu_utils_ActivityTracker_flushUnsavedData(JNIEnv*, jclass) +{ + HostThreadLock guard; + UICommon::FlushUnsavedData(); +} } diff --git a/Source/Core/UICommon/UICommon.cpp b/Source/Core/UICommon/UICommon.cpp index 60077bfcd53..8fe882b9ad1 100644 --- a/Source/Core/UICommon/UICommon.cpp +++ b/Source/Core/UICommon/UICommon.cpp @@ -162,6 +162,11 @@ void Shutdown() Config::Shutdown(); } +void FlushUnsavedData() +{ + INFO_LOG_FMT(CORE, "Flushing unsaved data..."); +} + void InitControllers(const WindowSystemInfo& wsi) { if (g_controller_interface.IsInit()) diff --git a/Source/Core/UICommon/UICommon.h b/Source/Core/UICommon/UICommon.h index f23a0005c7c..e5862838165 100644 --- a/Source/Core/UICommon/UICommon.h +++ b/Source/Core/UICommon/UICommon.h @@ -14,6 +14,8 @@ namespace UICommon void Init(); void Shutdown(); +void FlushUnsavedData(); + void InitControllers(const WindowSystemInfo& wsi); void ShutdownControllers(); From b0652925faa54b9bfd5a17cfed8870a2b1f2759b Mon Sep 17 00:00:00 2001 From: Jordan Woyak Date: Sat, 26 Apr 2025 17:18:56 -0500 Subject: [PATCH 2/4] Core: Add a HookableEvent for UICommon::FlushUnsavedData. --- Source/Core/UICommon/UICommon.cpp | 7 +++++++ Source/Core/UICommon/UICommon.h | 3 +++ 2 files changed, 10 insertions(+) diff --git a/Source/Core/UICommon/UICommon.cpp b/Source/Core/UICommon/UICommon.cpp index 8fe882b9ad1..7d3835398fb 100644 --- a/Source/Core/UICommon/UICommon.cpp +++ b/Source/Core/UICommon/UICommon.cpp @@ -65,6 +65,7 @@ namespace UICommon { static Config::ConfigChangedCallbackID s_config_changed_callback_id; +static Common::HookableEvent<> s_flush_unsaved_data_event_hook; static void CreateDumpPath(std::string path) { @@ -162,9 +163,15 @@ void Shutdown() Config::Shutdown(); } +[[nodiscard]] Common::EventHook AddFlushUnsavedDataCallback(std::function callback) +{ + return s_flush_unsaved_data_event_hook.Register(std::move(callback)); +} + void FlushUnsavedData() { INFO_LOG_FMT(CORE, "Flushing unsaved data..."); + s_flush_unsaved_data_event_hook.Trigger(); } void InitControllers(const WindowSystemInfo& wsi) diff --git a/Source/Core/UICommon/UICommon.h b/Source/Core/UICommon/UICommon.h index e5862838165..991f584089d 100644 --- a/Source/Core/UICommon/UICommon.h +++ b/Source/Core/UICommon/UICommon.h @@ -6,6 +6,7 @@ #include #include "Common/CommonTypes.h" +#include "Common/HookableEvent.h" struct WindowSystemInfo; @@ -14,6 +15,8 @@ namespace UICommon void Init(); void Shutdown(); +// Triggered from the Host-thread on Android before a potential process termination. +[[nodiscard]] Common::EventHook AddFlushUnsavedDataCallback(std::function callback); void FlushUnsavedData(); void InitControllers(const WindowSystemInfo& wsi); From 1d9e475123346c0e83289e788bdfad9f3345b512 Mon Sep 17 00:00:00 2001 From: Jordan Woyak Date: Sun, 19 Oct 2025 20:31:22 -0500 Subject: [PATCH 3/4] Common: Add TransferableSharedMutex class and unit tests. --- Source/Core/Common/CMakeLists.txt | 1 + Source/Core/Common/TransferableSharedMutex.h | 92 ++++++++++++++ Source/Core/DolphinLib.props | 1 + Source/UnitTests/Common/MutexTest.cpp | 122 +++++++++++++++++++ 4 files changed, 216 insertions(+) create mode 100644 Source/Core/Common/TransferableSharedMutex.h diff --git a/Source/Core/Common/CMakeLists.txt b/Source/Core/Common/CMakeLists.txt index bed44a540e9..17adc83b8e6 100644 --- a/Source/Core/Common/CMakeLists.txt +++ b/Source/Core/Common/CMakeLists.txt @@ -151,6 +151,7 @@ add_library(common Timer.h TimeUtil.cpp TimeUtil.h + TransferableSharedMutex.h TraversalClient.cpp TraversalClient.h TraversalProto.h diff --git a/Source/Core/Common/TransferableSharedMutex.h b/Source/Core/Common/TransferableSharedMutex.h new file mode 100644 index 00000000000..f735046ff04 --- /dev/null +++ b/Source/Core/Common/TransferableSharedMutex.h @@ -0,0 +1,92 @@ +// Copyright 2025 Dolphin Emulator Project +// SPDX-License-Identifier: GPL-2.0-or-later + +#pragma once + +#include +#include +#include + +namespace Common +{ +// Behaves like `std::shared_mutex` but locks and unlocks may come from different threads. +class TransferableSharedMutex +{ +public: + void lock() + { + while (true) + { + CounterType old_value{}; + if (m_counter.compare_exchange_strong(old_value, EXCLUSIVE_LOCK_VALUE, + std::memory_order_acquire, std::memory_order_relaxed)) + { + return; + } + + // lock() or lock_shared() is already held. + // Wait for an unlock notification and try again. + m_counter.wait(old_value, std::memory_order_relaxed); + } + } + + bool try_lock() + { + CounterType old_value{}; + return m_counter.compare_exchange_weak(old_value, EXCLUSIVE_LOCK_VALUE, + std::memory_order_acquire, std::memory_order_relaxed); + } + + void unlock() + { + m_counter.store(0, std::memory_order_release); + m_counter.notify_all(); // Notify potentially multiple wait()ers in lock_shared(). + } + + void lock_shared() + { + while (true) + { + auto old_value = m_counter.load(std::memory_order_relaxed); + while (old_value < LAST_SHARED_LOCK_VALUE) + { + if (m_counter.compare_exchange_strong(old_value, old_value + 1, std::memory_order_acquire, + std::memory_order_relaxed)) + { + return; + } + } + + // Something has gone very wrong if m_counter is nearly saturated with shared_lock(). + assert(old_value != LAST_SHARED_LOCK_VALUE); + + // lock() is already held. + // Wait for an unlock notification and try again. + m_counter.wait(old_value, std::memory_order_relaxed); + } + } + + bool try_lock_shared() + { + auto old_value = m_counter.load(std::memory_order_relaxed); + return (old_value < LAST_SHARED_LOCK_VALUE) && + m_counter.compare_exchange_weak(old_value, old_value + 1, std::memory_order_acquire, + std::memory_order_relaxed); + } + + void unlock_shared() + { + if (m_counter.fetch_sub(1, std::memory_order_release) == 1) + m_counter.notify_one(); // Notify one of the wait()ers in lock(). + } + +private: + using CounterType = std::uintptr_t; + + static constexpr auto EXCLUSIVE_LOCK_VALUE = CounterType(-1); + static constexpr auto LAST_SHARED_LOCK_VALUE = EXCLUSIVE_LOCK_VALUE - 1; + + std::atomic m_counter{}; +}; + +} // namespace Common diff --git a/Source/Core/DolphinLib.props b/Source/Core/DolphinLib.props index 30ff8d1c7ea..d030f8821b7 100644 --- a/Source/Core/DolphinLib.props +++ b/Source/Core/DolphinLib.props @@ -171,6 +171,7 @@ + diff --git a/Source/UnitTests/Common/MutexTest.cpp b/Source/UnitTests/Common/MutexTest.cpp index 222724ad502..eb3bba6d150 100644 --- a/Source/UnitTests/Common/MutexTest.cpp +++ b/Source/UnitTests/Common/MutexTest.cpp @@ -6,9 +6,11 @@ #include #include #include +#include #include #include "Common/Mutex.h" +#include "Common/TransferableSharedMutex.h" template static void DoAtomicMutexTests(const char mutex_name[]) @@ -100,3 +102,123 @@ TEST(Mutex, AtomicMutex) DoAtomicMutexTests("AtomicMutex"); DoAtomicMutexTests("SpinMutex"); } + +TEST(Mutex, TransferableSharedMutex) +{ + Common::TransferableSharedMutex work_mutex; + + bool worker_done = false; + + static constexpr auto SLEEP_TIME = std::chrono::microseconds{1}; + + // lock() on main thread, unlock() on worker thread. + std::thread thread{[&, lk = std::unique_lock{work_mutex}] { + std::this_thread::sleep_for(SLEEP_TIME); + worker_done = true; + }}; + + // lock() waits for the thread to unlock(). + { + std::lock_guard lk{work_mutex}; + EXPECT_TRUE(worker_done); + } + + thread.join(); + + // Prevent below workers from incrementing `done_count`. + Common::TransferableSharedMutex done_mutex; + std::unique_lock done_lk{done_mutex}; + + // try_*() fails when holding an exclusive lock. + EXPECT_FALSE(done_mutex.try_lock()); + EXPECT_FALSE(done_mutex.try_lock_shared()); + + static constexpr int THREAD_COUNT = 4; + static constexpr int REPEAT_COUNT = 100; + static constexpr int TOTAL_ITERATIONS = THREAD_COUNT * REPEAT_COUNT; + + std::atomic work_count = 0; + std::atomic done_count = 0; + int additional_work_count = 0; + + std::atomic try_lock_fail_count = 0; + std::atomic try_lock_shared_fail_count = 0; + + std::vector threads(THREAD_COUNT); + for (auto& t : threads) + { + // lock_shared() multiple times on main thread. + t = std::thread{[&, work_lk = std::shared_lock{work_mutex}]() mutable { + std::this_thread::sleep_for(SLEEP_TIME); + + // try_lock() fails after lock_shared(). + EXPECT_FALSE(work_mutex.try_lock()); + + // Main thread already holds done_mutex. + EXPECT_FALSE(done_mutex.try_lock()); + EXPECT_FALSE(done_mutex.try_lock_shared()); + + ++work_count; + + // Signal work is done. + work_lk.unlock(); + + // lock_shared() blocks until main thread unlock()s. + { + std::shared_lock lk{done_mutex}; + ++done_count; + } + + // Contesting all of [try_]lock[_shared] doesn't explode. + for (int i = 0; i != REPEAT_COUNT; ++i) + { + while (!work_mutex.try_lock()) + { + try_lock_fail_count.fetch_add(1, std::memory_order_relaxed); + } + work_mutex.unlock(); + + while (!work_mutex.try_lock_shared()) + { + try_lock_shared_fail_count.fetch_add(1, std::memory_order_relaxed); + } + work_mutex.unlock_shared(); + { + std::lock_guard lk{work_mutex}; + ++additional_work_count; + } + std::shared_lock lk{work_mutex}; + } + }}; + } + + // lock() waits for threads to unlock_shared(). + { + std::lock_guard lk{work_mutex}; + EXPECT_EQ(work_count.load(std::memory_order_relaxed), THREAD_COUNT); + } + + std::this_thread::sleep_for(SLEEP_TIME); + + // The threads are still blocking on done_mutex. + EXPECT_EQ(done_count, 0); + + done_lk.unlock(); + std::ranges::for_each(threads, &std::thread::join); + + // The threads finished. + EXPECT_EQ(done_count, THREAD_COUNT); + + EXPECT_EQ(additional_work_count, TOTAL_ITERATIONS); + + GTEST_LOG_(INFO) << "try_lock() failure %: " + << (try_lock_fail_count * 100.0 / (TOTAL_ITERATIONS + try_lock_fail_count)); + + GTEST_LOG_(INFO) << "try_lock_shared() failure %: " + << (try_lock_shared_fail_count * 100.0 / + (TOTAL_ITERATIONS + try_lock_shared_fail_count)); + + // Things are still sane after contesting in worker threads. + done_lk.lock(); + std::lock_guard lk{work_mutex}; +} From 2322437f96bc53f1edcd9100daedb5b54c6e0fc4 Mon Sep 17 00:00:00 2001 From: Jordan Woyak Date: Sun, 2 Nov 2025 22:21:15 -0600 Subject: [PATCH 4/4] State: Simplify interthread communication and cleanups. Save/Load calls are now always non-blocking for the caller, but appropriately block the CPU thread as needed. --- .../dolphinemu/dolphinemu/NativeLibrary.kt | 6 +- .../activities/EmulationActivity.kt | 14 +- .../dolphinemu/fragments/EmulationFragment.kt | 2 +- Source/Android/jni/MainAndroid.cpp | 10 +- Source/Core/Core/State.cpp | 651 +++++++++--------- Source/Core/Core/State.h | 19 +- 6 files changed, 328 insertions(+), 374 deletions(-) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.kt b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.kt index 0b6490b2908..0edc0f91f9d 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.kt +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.kt @@ -233,19 +233,17 @@ object NativeLibrary { * Saves a game state to the slot number. * * @param slot The slot location to save state to. - * @param wait If false, returns as early as possible. If true, returns once the savestate has been written to disk. */ @JvmStatic - external fun SaveState(slot: Int, wait: Boolean) + external fun SaveState(slot: Int) /** * Saves a game state to the specified path. * * @param path The path to save state to. - * @param wait If false, returns as early as possible. If true, returns once the savestate has been written to disk. */ @JvmStatic - external fun SaveStateAs(path: String, wait: Boolean) + external fun SaveStateAs(path: String) /** * Loads a game state from the slot number. diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/activities/EmulationActivity.kt b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/activities/EmulationActivity.kt index 1b23118de39..8a1d8addbdf 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/activities/EmulationActivity.kt +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/activities/EmulationActivity.kt @@ -494,16 +494,16 @@ class EmulationActivity : AppCompatActivity(), ThemeProvider { } MENU_ACTION_TAKE_SCREENSHOT -> NativeLibrary.SaveScreenShot() - MENU_ACTION_QUICK_SAVE -> NativeLibrary.SaveState(9, false) + MENU_ACTION_QUICK_SAVE -> NativeLibrary.SaveState(9) MENU_ACTION_QUICK_LOAD -> NativeLibrary.LoadState(9) MENU_ACTION_SAVE_ROOT -> showSubMenu(SaveOrLoad.SAVE) MENU_ACTION_LOAD_ROOT -> showSubMenu(SaveOrLoad.LOAD) - MENU_ACTION_SAVE_SLOT1 -> NativeLibrary.SaveState(0, false) - MENU_ACTION_SAVE_SLOT2 -> NativeLibrary.SaveState(1, false) - MENU_ACTION_SAVE_SLOT3 -> NativeLibrary.SaveState(2, false) - MENU_ACTION_SAVE_SLOT4 -> NativeLibrary.SaveState(3, false) - MENU_ACTION_SAVE_SLOT5 -> NativeLibrary.SaveState(4, false) - MENU_ACTION_SAVE_SLOT6 -> NativeLibrary.SaveState(5, false) + MENU_ACTION_SAVE_SLOT1 -> NativeLibrary.SaveState(0) + MENU_ACTION_SAVE_SLOT2 -> NativeLibrary.SaveState(1) + MENU_ACTION_SAVE_SLOT3 -> NativeLibrary.SaveState(2) + MENU_ACTION_SAVE_SLOT4 -> NativeLibrary.SaveState(3) + MENU_ACTION_SAVE_SLOT5 -> NativeLibrary.SaveState(4) + MENU_ACTION_SAVE_SLOT6 -> NativeLibrary.SaveState(5) MENU_ACTION_LOAD_SLOT1 -> NativeLibrary.LoadState(0) MENU_ACTION_LOAD_SLOT2 -> NativeLibrary.LoadState(1) MENU_ACTION_LOAD_SLOT3 -> NativeLibrary.LoadState(2) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.kt b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.kt index 9cf87c3401a..e4698ba43c8 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.kt +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.kt @@ -232,7 +232,7 @@ class EmulationFragment : Fragment(), SurfaceHolder.Callback { } } - fun saveTemporaryState() = NativeLibrary.SaveStateAs(temporaryStateFilePath, true) + fun saveTemporaryState() = NativeLibrary.SaveStateAs(temporaryStateFilePath) private val temporaryStateFilePath: String get() = "${requireContext().filesDir}${File.separator}temp.sav" diff --git a/Source/Android/jni/MainAndroid.cpp b/Source/Android/jni/MainAndroid.cpp index 7c78f49045b..71128334034 100644 --- a/Source/Android/jni/MainAndroid.cpp +++ b/Source/Android/jni/MainAndroid.cpp @@ -310,19 +310,17 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_eglBindAPI(J } JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SaveState(JNIEnv*, jclass, - jint slot, - jboolean wait) + jint slot) { HostThreadLock guard; - State::Save(Core::System::GetInstance(), slot, wait); + State::Save(Core::System::GetInstance(), slot); } JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SaveStateAs(JNIEnv* env, jclass, - jstring path, - jboolean wait) + jstring path) { HostThreadLock guard; - State::SaveAs(Core::System::GetInstance(), GetJString(env, path), wait); + State::SaveAs(Core::System::GetInstance(), GetJString(env, path)); } JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_LoadState(JNIEnv*, jclass, diff --git a/Source/Core/Core/State.cpp b/Source/Core/Core/State.cpp index ffd811bd6c7..36974a9bd59 100644 --- a/Source/Core/Core/State.cpp +++ b/Source/Core/Core/State.cpp @@ -4,12 +4,11 @@ #include "Core/State.h" #include -#include #include #include #include -#include #include +#include #include #include #include @@ -20,15 +19,17 @@ #include #include +#include "Common/Buffer.h" #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" #include "Common/Contains.h" -#include "Common/Event.h" #include "Common/FileUtil.h" #include "Common/IOFile.h" +#include "Common/Logging/Log.h" #include "Common/MsgHandler.h" #include "Common/Thread.h" #include "Common/TimeUtil.h" +#include "Common/TransferableSharedMutex.h" #include "Common/Version.h" #include "Common/WorkQueueThread.h" @@ -40,12 +41,13 @@ #include "Core/HW/HW.h" #include "Core/HW/Memmap.h" #include "Core/HW/Wiimote.h" -#include "Core/Host.h" #include "Core/Movie.h" #include "Core/NetPlayProto.h" #include "Core/PowerPC/PowerPC.h" #include "Core/System.h" +#include "UICommon/UICommon.h" + #include "VideoCommon/FrameDumpFFMpeg.h" #include "VideoCommon/OnScreenDisplay.h" #include "VideoCommon/VideoBackendBase.h" @@ -66,33 +68,31 @@ static unsigned char __LZO_MMODEL out[OUT_LEN]; static AfterLoadCallbackFunc s_on_after_load_callback; +static Common::EventHook s_flush_unsaved_data_hook; + // Temporary undo state buffer static Common::UniqueBuffer s_undo_load_buffer; -static std::mutex s_undo_load_buffer_mutex; -static std::mutex s_load_or_save_in_progress_mutex; +// Used to estimate buffer size for the next save. +static u32 s_last_state_size = 0; -struct CompressAndDumpState_args +// Shared locks are acquired for each state save task. +// Tasks generally transition from: Calling thread -> CPU thread -> Compress/Write thread. +// Holding an "exclusive" lock will: +// 1. Ensure all previous save tasks have been completely written to the file systen. +// 2. Prevent new tasks from starting. +static Common::TransferableSharedMutex s_state_saves_in_progress; + +struct CompressAndDumpStateArgs { Common::UniqueBuffer buffer; std::string filename; - std::shared_ptr state_write_done_event; + std::shared_lock task_lock; }; -// Protects against simultaneous reads and writes to the final savestate location from multiple -// threads. -static std::mutex s_save_thread_mutex; - // Queue for compressing and writing savestates to disk. -static Common::WorkQueueThread s_save_thread; - -// Keeps track of savestate writes that are currently happening, so we don't load a state while -// another one is still saving. This is particularly important so if you save to a slot and then -// immediately load from the same one, you don't accidentally load the state that's still at that -// file path before the write is done. -static std::mutex s_state_writes_in_queue_mutex; -static size_t s_state_writes_in_queue; -static std::condition_variable s_state_write_queue_is_empty; +// Only the CPU thread manipulates this worker. +static Common::WorkQueueThreadSP s_compress_and_dump_thread; // Don't forget to increase this after doing changes on the savestate system constexpr u32 STATE_VERSION = 175; // Last changed in PR 13751 @@ -121,20 +121,18 @@ static const std::map> s_old_versions = {38, {"4.0-4963", "4.0-5267"}}, {39, {"4.0-5279", "4.0-5525"}}, {40, {"4.0-5531", "4.0-5809"}}, {41, {"4.0-5811", "4.0-5923"}}, {42, {"4.0-5925", "4.0-5946"}}}; -enum -{ - STATE_NONE = 0, - STATE_SAVE = 1, - STATE_LOAD = 2, -}; +static constexpr bool s_use_compression = true; -static bool s_use_compression = true; - -void EnableCompression(bool compression) +// Acquired for tasks that will write state save data to the filesystem. +// This allows for later waiting on completion of said tasks when necessary. +// We want to maintain a proper order of async operations, e.g. Save, Save, GetInfoString. +[[nodiscard]] static auto GetStateSaveTaskLock() { - s_use_compression = compression; + return std::shared_lock{s_state_saves_in_progress}; } +static bool ReadHeader(const std::string& filename, StateHeader& header); + static void DoState(Core::System& system, PointerWrap& p) { bool is_wii = system.IsWii() || system.IsMIOS(); @@ -201,68 +199,82 @@ static void DoState(Core::System& system, PointerWrap& p) #endif // USE_RETRO_ACHIEVEMENTS } -void LoadFromBuffer(Core::System& system, Common::UniqueBuffer& buffer) +static bool CheckIfStateLoadIsAllowed(Core::System& system) { + if (!Core::IsRunningOrStarting(system)) + return false; + if (NetPlay::IsNetPlayRunning()) { OSD::AddMessage("Loading savestates is disabled in Netplay to prevent desyncs"); - return; + return false; } if (AchievementManager::GetInstance().IsHardcoreModeActive()) { OSD::AddMessage("Loading savestates is disabled in RetroAchievements hardcore mode"); - return; + return false; } - Core::RunOnCPUThread( - system, - [&] { - u8* ptr = buffer.data(); - PointerWrap p(&ptr, buffer.size(), PointerWrap::Mode::Read); - DoState(system, p); - }, - true); + return true; } -void SaveToBuffer(Core::System& system, Common::UniqueBuffer& buffer) +static bool LoadFromBuffer(Core::System& system, std::span buffer) { - Core::RunOnCPUThread( - system, - [&] { - u8* ptr = nullptr; - PointerWrap p_measure(&ptr, 0, PointerWrap::Mode::Measure); - DoState(system, p_measure); + u8* ptr = buffer.data(); + PointerWrap p(&ptr, buffer.size(), PointerWrap::Mode::Read); + DoState(system, p); + return p.IsReadMode(); +} - const size_t new_buffer_size = ptr - (u8*)(nullptr); - if (new_buffer_size > buffer.size()) - buffer.reset(new_buffer_size); +// Returns the required size, or 0 on failure. +static std::size_t SaveToBuffer(Core::System& system, Common::UniqueBuffer& buffer) +{ + // Attempt to save to our provided buffer as-is. + // If buffer isn't large enough, PointerWrap transitions to MeasureMode, + // and then we have our measurement for a second attempt. + u8* ptr = buffer.data(); + PointerWrap pointer_wrap(&ptr, buffer.size(), PointerWrap::Mode::Write); + DoState(system, pointer_wrap); + const auto measured_size = pointer_wrap.GetOffsetFromPreviousPosition(buffer.data()); - ptr = buffer.data(); - PointerWrap p(&ptr, buffer.size(), PointerWrap::Mode::Write); - DoState(system, p); - }, - true); + if (pointer_wrap.IsWriteMode()) + { + s_last_state_size = measured_size; + return measured_size; + } + + if (measured_size > buffer.size()) + { + DEBUG_LOG_FMT(CORE, "SaveToBuffer: Growing buffer from size {} to measured size {}", + buffer.size(), measured_size); + buffer.reset(measured_size); + return SaveToBuffer(system, buffer); + } + + // Buffer was large enough but we still failed for some other reason. + return 0; } namespace { struct SlotWithTimestamp { + // 1-based indexing. int slot; double timestamp; }; } // namespace -// returns first slot number not in the vector, or -1 if all are in the vector -static int GetEmptySlot(const std::vector& used_slots) +// Returns first slot number (1-based indexing) not in the vector. +static std::optional GetEmptySlot(const std::vector& used_slots) { - for (int i = 1; i <= (int)NUM_STATES; i++) + for (int i = 1; i <= int(NUM_STATES); ++i) { if (!Common::Contains(used_slots, i, &SlotWithTimestamp::slot)) return i; } - return -1; + return std::nullopt; } // Arbitrarily chosen value (38 years) that is subtracted in GetSystemTimeAsDouble() @@ -289,40 +301,41 @@ static std::string SystemTimeAsDoubleToString(double time) return fmt::format(std::locale{""}, "{:%x %X}", *local_time); } -static std::string MakeStateFilename(int number); +static std::string MakeStateFilename(int number) +{ + return fmt::format("{}{}.s{:02d}", File::GetUserPath(D_STATESAVES_IDX), + SConfig::GetInstance().GetGameID(), number); +} static std::vector GetUsedSlotsWithTimestamp() { std::vector result; StateHeader header; - for (int i = 1; i <= (int)NUM_STATES; i++) + for (int i = 1; i <= int(NUM_STATES); ++i) { std::string filename = MakeStateFilename(i); - if (File::Exists(filename)) - { - if (ReadHeader(filename, header)) - { - result.emplace_back(SlotWithTimestamp{.slot = i, .timestamp = header.legacy_header.time}); - } - } + if (!File::Exists(filename) || !ReadHeader(filename, header)) + continue; + + result.emplace_back(SlotWithTimestamp{.slot = i, .timestamp = header.legacy_header.time}); } return result; } -static void CompressBufferToFile(const u8* raw_buffer, u64 size, File::IOFile& f) +static void CompressBufferToFile(std::span raw_buffer, File::IOFile& f) { u64 total_bytes_compressed = 0; while (true) { - const u64 bytes_left_to_compress = size - total_bytes_compressed; + const u64 bytes_left_to_compress = raw_buffer.size() - total_bytes_compressed; const int bytes_to_compress = static_cast(std::min(static_cast(LZ4_MAX_INPUT_SIZE), bytes_left_to_compress)); Common::UniqueBuffer compressed_buffer(LZ4_compressBound(bytes_to_compress)); const int compressed_len = LZ4_compress_default( - reinterpret_cast(raw_buffer) + total_bytes_compressed, compressed_buffer.get(), - bytes_to_compress, int(compressed_buffer.size())); + reinterpret_cast(raw_buffer.data()) + total_bytes_compressed, + compressed_buffer.get(), bytes_to_compress, int(compressed_buffer.size())); if (compressed_len == 0) { @@ -335,7 +348,7 @@ static void CompressBufferToFile(const u8* raw_buffer, u64 size, File::IOFile& f f.WriteBytes(compressed_buffer.get(), compressed_len); total_bytes_compressed += bytes_to_compress; - if (total_bytes_compressed == size) + if (total_bytes_compressed == raw_buffer.size()) break; } } @@ -374,17 +387,17 @@ static void WriteHeadersToFile(size_t uncompressed_size, File::IOFile& f) // If StateExtendedHeader is amended to include more than the base, add WriteBytes() calls here. } -static void CompressAndDumpState(Core::System& system, CompressAndDumpState_args& save_args) +static void CompressAndDumpState(Core::System& system, const CompressAndDumpStateArgs& save_args) { - const u8* const buffer_data = save_args.buffer.data(); - const size_t buffer_size = save_args.buffer.size(); + const auto& buffer = save_args.buffer; const std::string& filename = save_args.filename; // Find free temporary filename. - // TODO: The file exists check and the actual opening of the file should be atomic, we don't have - // functions for that. + + // TODO: The file exists check and the actual opening of the file should be atomic. + // This is only an issue for multiple instances of dolphin operating on the same user folder. std::string temp_filename; - size_t temp_counter = static_cast(Common::CurrentThreadId()); + auto temp_counter = static_cast(Common::CurrentThreadId()); do { temp_filename = fmt::format("{}{}.tmp", filename, temp_counter); @@ -398,12 +411,12 @@ static void CompressAndDumpState(Core::System& system, CompressAndDumpState_args return; } - WriteHeadersToFile(buffer_size, f); + WriteHeadersToFile(buffer.size(), f); if (s_use_compression) - CompressBufferToFile(buffer_data, buffer_size, f); + CompressBufferToFile(buffer, f); else - f.WriteBytes(buffer_data, buffer_size); + f.WriteBytes(buffer.data(), buffer.size()); if (!f.IsGood()) Core::DisplayMessage("Failed to write state file", 2000); @@ -412,111 +425,82 @@ static void CompressAndDumpState(Core::System& system, CompressAndDumpState_args const std::string last_state_dtmname = last_state_filename + ".dtm"; const std::string dtmname = filename + ".dtm"; + // Backup existing state (overwriting an existing backup, if any). + if (File::Exists(filename)) { - std::lock_guard lk(s_save_thread_mutex); + if (File::Exists(last_state_filename)) + File::Delete((last_state_filename)); + if (File::Exists(last_state_dtmname)) + File::Delete((last_state_dtmname)); - // Backup existing state (overwriting an existing backup, if any). - if (File::Exists(filename)) + if (!File::Rename(filename, last_state_filename)) { - if (File::Exists(last_state_filename)) - File::Delete((last_state_filename)); - if (File::Exists(last_state_dtmname)) - File::Delete((last_state_dtmname)); - - if (!File::Rename(filename, last_state_filename)) - { - Core::DisplayMessage("Failed to move previous state to state undo backup", 1000); - } - else if (File::Exists(dtmname)) - { - if (!File::Rename(dtmname, last_state_dtmname)) - Core::DisplayMessage("Failed to move previous state's dtm to state undo backup", 1000); - } + Core::DisplayMessage("Failed to move previous state to state undo backup", 1000); } - - auto& movie = system.GetMovie(); - if ((movie.IsMovieActive()) && !movie.IsJustStartingRecordingInputFromSaveState()) - movie.SaveRecording(dtmname); - else if (!movie.IsMovieActive()) - File::Delete(dtmname); - - // Move written state to final location. - // TODO: This should also be atomic. This is possible on all systems, but needs a special - // implementation of IOFile on Windows. - if (!f.Close()) - Core::DisplayMessage("Failed to close state file", 2000); - - if (!File::Rename(temp_filename, filename)) + else if (File::Exists(dtmname)) { - Core::DisplayMessage("Failed to rename state file", 2000); - } - else - { - const std::filesystem::path temp_path(filename); - Core::DisplayMessage(fmt::format("Saved State to {}", temp_path.filename().string()), 2000); + if (!File::Rename(dtmname, last_state_dtmname)) + Core::DisplayMessage("Failed to move previous state's dtm to state undo backup", 1000); } } + + auto& movie = system.GetMovie(); + if ((movie.IsMovieActive()) && !movie.IsJustStartingRecordingInputFromSaveState()) + movie.SaveRecording(dtmname); + else if (!movie.IsMovieActive()) + File::Delete(dtmname); + + // Move written state to final location. + // TODO: This should also be atomic. This is possible on all systems, but needs a special + // implementation of IOFile on Windows. + if (!f.Close()) + Core::DisplayMessage("Failed to close state file", 2000); + + if (!File::Rename(temp_filename, filename)) + { + Core::DisplayMessage("Failed to rename state file", 2000); + } + else + { + const std::filesystem::path temp_path(filename); + Core::DisplayMessage(fmt::format("Saved State to {}", temp_path.filename().string()), 2000); + } } -void SaveAs(Core::System& system, const std::string& filename, bool wait) +static void SaveAsFromCore(Core::System& system, std::string filename) { - std::unique_lock lk(s_load_or_save_in_progress_mutex, std::try_to_lock); - if (!lk) - return; + // Try with a buffer a bit larger than the previous state. + // This will often avoid the "Measure" step. + const auto buffer_size_estimate = std::size_t(s_last_state_size) * 110 / 100; + Common::UniqueBuffer buffer{buffer_size_estimate}; + if (const auto actual_size = SaveToBuffer(system, buffer)) + { + // Adjust the oversized buffer down to the actual size. + buffer.assign(buffer.extract().first, actual_size); + + CompressAndDumpStateArgs dump_args{ + .buffer = std::move(buffer), + .filename = std::move(filename), + .task_lock = GetStateSaveTaskLock(), + }; + Core::DisplayMessage("Saving State...", 1000); + s_compress_and_dump_thread.EmplaceItem(std::move(dump_args)); + } + else + { + Core::DisplayMessage("Unable to save: Internal DoState Error", 4000); + } +} + +void SaveAs(Core::System& system, std::string filename) +{ Core::RunOnCPUThread( system, - [&] { - { - std::lock_guard lk_(s_state_writes_in_queue_mutex); - ++s_state_writes_in_queue; - } - - // Measure the size of the buffer. - u8* ptr = nullptr; - PointerWrap p_measure(&ptr, 0, PointerWrap::Mode::Measure); - DoState(system, p_measure); - const size_t buffer_size = ptr - (u8*)(nullptr); - - // Then actually do the write. - Common::UniqueBuffer current_buffer(buffer_size); - ptr = current_buffer.data(); - PointerWrap p(&ptr, buffer_size, PointerWrap::Mode::Write); - DoState(system, p); - - if (p.IsWriteMode()) - { - Core::DisplayMessage("Saving State...", 1000); - - std::shared_ptr sync_event; - - CompressAndDumpState_args save_args; - save_args.buffer = std::move(current_buffer); - save_args.filename = filename; - if (wait) - { - sync_event = std::make_shared(); - save_args.state_write_done_event = sync_event; - } - - s_save_thread.EmplaceItem(std::move(save_args)); - - if (sync_event) - sync_event->Wait(); - } - else - { - // someone aborted the save by changing the mode? - { - // Note: The worker thread takes care of this in the other branch. - std::lock_guard lk_(s_state_writes_in_queue_mutex); - if (--s_state_writes_in_queue == 0) - s_state_write_queue_is_empty.notify_all(); - } - Core::DisplayMessage("Unable to save: Internal DoState Error", 4000); - } + [&system, filename = std::move(filename), lock = GetStateSaveTaskLock()]() mutable { + SaveAsFromCore(system, std::move(filename)); }, - true); + false); } static bool GetVersionFromLZO(StateHeader& header, File::IOFile& f) @@ -616,18 +600,17 @@ static bool ReadStateHeaderFromFile(StateHeader& header, File::IOFile& f, return true; } -bool ReadHeader(const std::string& filename, StateHeader& header) +static bool ReadHeader(const std::string& filename, StateHeader& header) { - // ensure that the savestate write thread isn't moving around states while we do this - std::lock_guard lk(s_save_thread_mutex); - File::IOFile f(filename, "rb"); - bool get_version_header = false; + constexpr bool get_version_header = false; return ReadStateHeaderFromFile(header, f, get_version_header); } std::string GetInfoStringOfSlot(int slot, bool translate) { + std::lock_guard lk{s_state_saves_in_progress}; + std::string filename = MakeStateFilename(slot); if (!File::Exists(filename)) return translate ? Common::GetStringT("Empty") : "Empty"; @@ -641,6 +624,8 @@ std::string GetInfoStringOfSlot(int slot, bool translate) u64 GetUnixTimeOfSlot(int slot) { + std::lock_guard lk{s_state_saves_in_progress}; + State::StateHeader header; if (!ReadHeader(MakeStateFilename(slot), header)) return 0; @@ -657,7 +642,7 @@ static bool DecompressLZ4(Common::UniqueBuffer& raw_buffer, u64 size, File:: u64 total_bytes_read = 0; while (true) { - s32 compressed_data_len; + s32 compressed_data_len = 0; if (!f.ReadArray(&compressed_data_len, 1)) { PanicAlertFmt("Could not read state data length"); @@ -677,8 +662,8 @@ static bool DecompressLZ4(Common::UniqueBuffer& raw_buffer, u64 size, File:: return false; } - u32 max_decompress_size = - static_cast(std::min((u64)LZ4_MAX_INPUT_SIZE, size - total_bytes_read)); + const auto max_decompress_size = + static_cast(std::min((u64)LZ4_MAX_INPUT_SIZE, size - total_bytes_read)); int bytes_read = LZ4_decompress_safe( compressed_data.get(), reinterpret_cast(raw_buffer.data()) + total_bytes_read, @@ -697,7 +682,8 @@ static bool DecompressLZ4(Common::UniqueBuffer& raw_buffer, u64 size, File:: { return true; } - else if (total_bytes_read > size) + + if (total_bytes_read > size) { PanicAlertFmtT("Internal LZ4 Error - payload size mismatch ({0} / {1}))", total_bytes_read, size); @@ -711,7 +697,7 @@ static bool ValidateHeaders(const StateHeader& header) bool success = true; // Game ID - if (strncmp(SConfig::GetInstance().GetGameID().c_str(), header.legacy_header.game_id, 6)) + if (strncmp(SConfig::GetInstance().GetGameID().c_str(), header.legacy_header.game_id, 6) != 0) { Core::DisplayMessage(fmt::format("State belongs to a different game (ID {})", std::string_view{header.legacy_header.game_id, @@ -720,8 +706,8 @@ static bool ValidateHeaders(const StateHeader& header) return false; } - // Check both the state version and the revision string - std::string current_str = Common::GetScmRevStr(); + // Check the state version. + // FYI: We don't require an exact revision string match. std::string loaded_str = header.version_string; const u32 loaded_version = header.version_header.version_cookie - COOKIE_BASE; @@ -756,22 +742,7 @@ static bool ValidateHeaders(const StateHeader& header) static void LoadFileStateData(const std::string& filename, Common::UniqueBuffer& ret_data) { File::IOFile f; - - { - // If a state is currently saving, wait for that to end or time out. - std::unique_lock lk(s_state_writes_in_queue_mutex); - if (s_state_writes_in_queue != 0) - { - if (!s_state_write_queue_is_empty.wait_for(lk, std::chrono::seconds(3), - [] { return s_state_writes_in_queue == 0; })) - { - Core::DisplayMessage( - "A previous state saving operation is still in progress, cancelling load.", 2000); - return; - } - } - f.Open(filename, "rb"); - } + f.Open(filename, "rb"); StateHeader header; if (!ReadStateHeaderFromFile(header, f) || !ValidateHeaders(header)) @@ -835,87 +806,79 @@ static void LoadFileStateData(const std::string& filename, Common::UniqueBuffer< ret_data.swap(buffer); } -void LoadAs(Core::System& system, const std::string& filename) +static void LoadAsFromCore(Core::System& system, std::string filename) { - if (!Core::IsRunningOrStarting(system)) - return; + // Ensure all data has reached the filesystem before trying to use it. + s_compress_and_dump_thread.WaitForCompletion(); - if (NetPlay::IsNetPlayRunning()) + // Save temp buffer for undo load state + auto& movie = system.GetMovie(); + if (!movie.IsJustStartingRecordingInputFromSaveState()) { - OSD::AddMessage("Loading savestates is disabled in Netplay to prevent desyncs"); - return; + SaveToBuffer(system, s_undo_load_buffer); + const std::string dtmpath = File::GetUserPath(D_STATESAVES_IDX) + "undo.dtm"; + if (movie.IsMovieActive()) + movie.SaveRecording(dtmpath); + else if (File::Exists(dtmpath)) + File::Delete(dtmpath); } - if (AchievementManager::GetInstance().IsHardcoreModeActive()) + bool was_file_read = false; + bool loaded_successfully = false; + + // brackets here are so buffer gets freed ASAP { - OSD::AddMessage("Loading savestates is disabled in RetroAchievements hardcore mode"); - return; + Common::UniqueBuffer buffer; + LoadFileStateData(filename, buffer); + + if (!buffer.empty()) + { + was_file_read = true; + loaded_successfully = LoadFromBuffer(system, buffer); + } } - std::unique_lock lk(s_load_or_save_in_progress_mutex, std::try_to_lock); - if (!lk) + if (was_file_read) + { + if (loaded_successfully) + { + std::filesystem::path temp_filename(std::move(filename)); + Core::DisplayMessage(fmt::format("Loaded State from {}", temp_filename.filename().string()), + 2000); + if (File::Exists(filename + ".dtm")) + { + movie.LoadInput(filename + ".dtm"); + } + else if (!movie.IsJustStartingRecordingInputFromSaveState() && + !movie.IsJustStartingPlayingInputFromSaveState()) + { + movie.EndPlayInput(false); + } + } + else + { + Core::DisplayMessage("The savestate could not be loaded", OSD::Duration::NORMAL); + + // since we could be in an inconsistent state now (and might crash or whatever), undo. + UndoLoadState(system); + } + } + + if (s_on_after_load_callback) + s_on_after_load_callback(); +} + +void LoadAs(Core::System& system, std::string filename) +{ + if (!CheckIfStateLoadIsAllowed(system)) return; Core::RunOnCPUThread( system, - [&] { - // Save temp buffer for undo load state - auto& movie = system.GetMovie(); - if (!movie.IsJustStartingRecordingInputFromSaveState()) - { - std::lock_guard lk2(s_undo_load_buffer_mutex); - SaveToBuffer(system, s_undo_load_buffer); - const std::string dtmpath = File::GetUserPath(D_STATESAVES_IDX) + "undo.dtm"; - if (movie.IsMovieActive()) - movie.SaveRecording(dtmpath); - else if (File::Exists(dtmpath)) - File::Delete(dtmpath); - } - - bool loaded = false; - bool loadedSuccessfully = false; - - // brackets here are so buffer gets freed ASAP - { - Common::UniqueBuffer buffer; - LoadFileStateData(filename, buffer); - - if (!buffer.empty()) - { - u8* ptr = buffer.data(); - PointerWrap p(&ptr, buffer.size(), PointerWrap::Mode::Read); - DoState(system, p); - loaded = true; - loadedSuccessfully = p.IsReadMode(); - } - } - - if (loaded) - { - if (loadedSuccessfully) - { - std::filesystem::path tempfilename(filename); - Core::DisplayMessage( - fmt::format("Loaded State from {}", tempfilename.filename().string()), 2000); - if (File::Exists(filename + ".dtm")) - movie.LoadInput(filename + ".dtm"); - else if (!movie.IsJustStartingRecordingInputFromSaveState() && - !movie.IsJustStartingPlayingInputFromSaveState()) - movie.EndPlayInput(false); - } - else - { - Core::DisplayMessage("The savestate could not be loaded", OSD::Duration::NORMAL); - - // since we could be in an inconsistent state now (and might crash or whatever), undo. - UndoLoadState(system); - } - } - - if (s_on_after_load_callback) - s_on_after_load_callback(); + [&system, filename = std::move(filename)]() mutable { + LoadAsFromCore(system, std::move(filename)); }, - true); + false); } void SetOnAfterLoadCallback(AfterLoadCallbackFunc callback) @@ -925,37 +888,25 @@ void SetOnAfterLoadCallback(AfterLoadCallbackFunc callback) void Init(Core::System& system) { - s_save_thread.Reset("Savestate Worker", [&system](CompressAndDumpState_args args) { - CompressAndDumpState(system, args); + s_compress_and_dump_thread.Reset("Savestate Worker", + std::bind_front(&CompressAndDumpState, std::ref(system))); - { - std::lock_guard lk(s_state_writes_in_queue_mutex); - if (--s_state_writes_in_queue == 0) - s_state_write_queue_is_empty.notify_all(); - } - - if (args.state_write_done_event) - args.state_write_done_event->Set(); + s_flush_unsaved_data_hook = UICommon::AddFlushUnsavedDataCallback([] { + // Holding the lock for any amount of time means there are no pending state save tasks. + std::lock_guard lk{s_state_saves_in_progress}; }); } void Shutdown() { - s_save_thread.Shutdown(); - - std::lock_guard lk(s_undo_load_buffer_mutex); + s_compress_and_dump_thread.Shutdown(); s_undo_load_buffer.reset(); + s_flush_unsaved_data_hook.reset(); } -static std::string MakeStateFilename(int number) +void Save(Core::System& system, int slot) { - return fmt::format("{}{}.s{:02d}", File::GetUserPath(D_STATESAVES_IDX), - SConfig::GetInstance().GetGameID(), number); -} - -void Save(Core::System& system, int slot, bool wait) -{ - SaveAs(system, MakeStateFilename(slot), wait); + SaveAs(system, MakeStateFilename(slot)); } void Load(Core::System& system, int slot) @@ -965,68 +916,86 @@ void Load(Core::System& system, int slot) void LoadLastSaved(Core::System& system, int i) { - if (i <= 0) - { - Core::DisplayMessage("State doesn't exist", 2000); + if (!CheckIfStateLoadIsAllowed(system)) return; - } - std::vector used_slots = GetUsedSlotsWithTimestamp(); - if (static_cast(i) > used_slots.size()) - { - Core::DisplayMessage("State doesn't exist", 2000); - return; - } + Core::RunOnCPUThread( + system, + [&system, i] { + // Data must reach the filesystem for up to date "UsedSlots". + s_compress_and_dump_thread.WaitForCompletion(); - std::ranges::stable_sort(used_slots, {}, &SlotWithTimestamp::timestamp); - Load(system, (used_slots.end() - i)->slot); + std::vector used_slots = GetUsedSlotsWithTimestamp(); + if (std::size_t(i) > used_slots.size()) + { + Core::DisplayMessage("State doesn't exist", 2000); + return; + } + + std::ranges::stable_sort(used_slots, std::ranges::greater{}, &SlotWithTimestamp::timestamp); + LoadAsFromCore(system, MakeStateFilename(used_slots[i].slot)); + }, + false); } -// must wait for state to be written because it must know if all slots are taken void SaveFirstSaved(Core::System& system) { - std::vector used_slots = GetUsedSlotsWithTimestamp(); - if (used_slots.size() < NUM_STATES) - { - // save to an empty slot - Save(system, GetEmptySlot(used_slots), true); - return; - } + Core::RunOnCPUThread( + system, + [&system, lock = GetStateSaveTaskLock()] { + // Data must reach the filesystem for up to date "UsedSlots". + s_compress_and_dump_thread.WaitForCompletion(); - // overwrite the oldest state - std::ranges::stable_sort(used_slots, {}, &SlotWithTimestamp::timestamp); - Save(system, used_slots.front().slot, true); + std::vector used_slots = GetUsedSlotsWithTimestamp(); + auto slot = GetEmptySlot(used_slots); + if (!slot.has_value()) + { + // overwrite the oldest state + std::ranges::stable_sort(used_slots, {}, &SlotWithTimestamp::timestamp); + slot = used_slots.front().slot; + } + + SaveAsFromCore(system, MakeStateFilename(*slot)); + }, + false); } // Load the last state before loading the state void UndoLoadState(Core::System& system) { - std::lock_guard lk(s_undo_load_buffer_mutex); - if (!s_undo_load_buffer.empty()) - { - auto& movie = system.GetMovie(); - if (movie.IsMovieActive()) - { - const std::string dtmpath = File::GetUserPath(D_STATESAVES_IDX) + "undo.dtm"; - if (File::Exists(dtmpath)) - { - LoadFromBuffer(system, s_undo_load_buffer); - movie.LoadInput(dtmpath); - } - else - { - PanicAlertFmtT("No undo.dtm found, aborting undo load state to prevent movie desyncs"); - } - } - else - { - LoadFromBuffer(system, s_undo_load_buffer); - } - } - else - { - PanicAlertFmtT("There is nothing to undo!"); - } + if (!CheckIfStateLoadIsAllowed(system)) + return; + + Core::RunOnCPUThread( + system, + [&system] { + if (s_undo_load_buffer.empty()) + { + PanicAlertFmtT("There is nothing to undo!"); + return; + } + + auto& movie = system.GetMovie(); + if (movie.IsMovieActive()) + { + // Note: Only the CPU thread writes to "undo.dtm". + const std::string dtmpath = File::GetUserPath(D_STATESAVES_IDX) + "undo.dtm"; + if (File::Exists(dtmpath)) + { + LoadFromBuffer(system, s_undo_load_buffer); + movie.LoadInput(dtmpath); + } + else + { + PanicAlertFmtT("No undo.dtm found, aborting undo load state to prevent movie desyncs"); + } + } + else + { + LoadFromBuffer(system, s_undo_load_buffer); + } + }, + false); } // Load the state that the last save state overwritten on diff --git a/Source/Core/Core/State.h b/Source/Core/Core/State.h index 4b100b214c3..1c4eeb05547 100644 --- a/Source/Core/Core/State.h +++ b/Source/Core/Core/State.h @@ -10,7 +10,6 @@ #include #include -#include "Common/Buffer.h" #include "Common/CommonTypes.h" namespace Core @@ -81,13 +80,8 @@ struct StateExtendedHeader }; void Init(Core::System& system); - void Shutdown(); -void EnableCompression(bool compression); - -bool ReadHeader(const std::string& filename, StateHeader& header); - // Returns a string containing information of the savestate in the given slot // which can be presented to the user for identification purposes std::string GetInfoStringOfSlot(int slot, bool translate = true); @@ -97,17 +91,12 @@ u64 GetUnixTimeOfSlot(int slot); // These don't happen instantly - they get scheduled as events. // ...But only if we're not in the main CPU thread. -// If we're in the main CPU thread then they run immediately instead -// because some things (like Lua) need them to run immediately. -// Slots from 0-99. -void Save(Core::System& system, int slot, bool wait = false); +// If we're in the main CPU thread then they run immediately instead. +void Save(Core::System& system, int slot); void Load(Core::System& system, int slot); -void SaveAs(Core::System& system, const std::string& filename, bool wait = false); -void LoadAs(Core::System& system, const std::string& filename); - -void SaveToBuffer(Core::System& system, Common::UniqueBuffer& buffer); -void LoadFromBuffer(Core::System& system, Common::UniqueBuffer& buffer); +void SaveAs(Core::System& system, std::string filename); +void LoadAs(Core::System& system, std::string filename); void LoadLastSaved(Core::System& system, int i = 1); void SaveFirstSaved(Core::System& system);