From d0b7d9d027dab601b16173c92d074b9fecb548d0 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Thu, 5 Jun 2025 20:36:31 +1000 Subject: [PATCH] Host: Get rid of base settings interface indirection --- src/duckstation-mini/mini_host.cpp | 54 ++++++++++++------------ src/duckstation-qt/qthost.cpp | 50 +++++++++++----------- src/duckstation-regtest/regtest_host.cpp | 31 +++++++------- src/util/ini_settings_interface.cpp | 13 +++++- src/util/ini_settings_interface.h | 5 ++- 5 files changed, 83 insertions(+), 70 deletions(-) diff --git a/src/duckstation-mini/mini_host.cpp b/src/duckstation-mini/mini_host.cpp index f6af1d94a..c73e97a03 100644 --- a/src/duckstation-mini/mini_host.cpp +++ b/src/duckstation-mini/mini_host.cpp @@ -97,7 +97,7 @@ static void SavePlatformWindowGeometry(s32 x, s32 y, s32 width, s32 height); struct SDLHostState { // UI thread state - ALIGN_TO_CACHE_LINE std::unique_ptr base_settings_interface; + ALIGN_TO_CACHE_LINE INISettingsInterface base_settings_interface; bool batch_mode = false; bool start_fullscreen_ui_fullscreen = false; bool was_paused_by_focus_loss = false; @@ -265,44 +265,44 @@ bool MiniHost::InitializeConfig() std::string settings_path = Path::Combine(EmuFolders::DataRoot, "settings.ini"); const bool settings_exists = FileSystem::FileExists(settings_path.c_str()); INFO_LOG("Loading config from {}.", settings_path); - s_state.base_settings_interface = std::make_unique(std::move(settings_path)); - Host::Internal::SetBaseSettingsLayer(s_state.base_settings_interface.get()); + s_state.base_settings_interface.SetPath(std::move(settings_path)); + Host::Internal::SetBaseSettingsLayer(&s_state.base_settings_interface); u32 settings_version; - if (!settings_exists || !s_state.base_settings_interface->Load() || - !s_state.base_settings_interface->GetUIntValue("Main", "SettingsVersion", &settings_version) || + if (!settings_exists || !s_state.base_settings_interface.Load() || + !s_state.base_settings_interface.GetUIntValue("Main", "SettingsVersion", &settings_version) || settings_version != SETTINGS_VERSION) { - if (s_state.base_settings_interface->ContainsValue("Main", "SettingsVersion")) + if (s_state.base_settings_interface.ContainsValue("Main", "SettingsVersion")) { // NOTE: No point translating this, because there's no config loaded, so no language loaded. Host::ReportErrorAsync("Error", fmt::format("Settings version {} does not match expected version {}, resetting.", settings_version, SETTINGS_VERSION)); } - s_state.base_settings_interface->SetUIntValue("Main", "SettingsVersion", SETTINGS_VERSION); - SetDefaultSettings(*s_state.base_settings_interface, true, true); + s_state.base_settings_interface.SetUIntValue("Main", "SettingsVersion", SETTINGS_VERSION); + SetDefaultSettings(s_state.base_settings_interface, true, true); // Make sure we can actually save the config, and the user doesn't have some permission issue. Error error; - if (!s_state.base_settings_interface->Save(&error)) + if (!s_state.base_settings_interface.Save(&error)) { Host::ReportFatalError( "Error", fmt::format( "Failed to save configuration to\n\n{}\n\nThe error was: {}\n\nPlease ensure this directory is writable. You " "can also try portable mode by creating portable.txt in the same directory you installed DuckStation into.", - s_state.base_settings_interface->GetPath(), error.GetDescription())); + s_state.base_settings_interface.GetPath(), error.GetDescription())); return false; } } - EmuFolders::LoadConfig(*s_state.base_settings_interface.get()); + EmuFolders::LoadConfig(s_state.base_settings_interface); EmuFolders::EnsureFoldersExist(); // We need to create the console window early, otherwise it appears in front of the main window. - if (!Log::IsConsoleOutputEnabled() && s_state.base_settings_interface->GetBoolValue("Logging", "LogToConsole", false)) - Log::SetConsoleOutputParams(true, s_state.base_settings_interface->GetBoolValue("Logging", "LogTimestamps", true)); + if (!Log::IsConsoleOutputEnabled() && s_state.base_settings_interface.GetBoolValue("Logging", "LogToConsole", false)) + Log::SetConsoleOutputParams(true, s_state.base_settings_interface.GetBoolValue("Logging", "LogTimestamps", true)); // imgui setup, make sure it doesn't bug out ImGuiManager::SetFontPathAndRange(std::string(), {0x0020, 0x00FF, 0x2022, 0x2022, 0, 0}); @@ -444,7 +444,7 @@ void Host::CommitBaseSettingChanges() { auto lock = Host::GetSettingsLock(); Error error; - if (!MiniHost::s_state.base_settings_interface->Save(&error)) + if (!MiniHost::s_state.base_settings_interface.Save(&error)) ERROR_LOG("Failed to save settings: {}", error.GetDescription()); } @@ -730,12 +730,12 @@ void Host::DestroyAuxiliaryRenderWindow(AuxiliaryRenderWindowHandle handle, s32* bool MiniHost::GetSavedPlatformWindowGeometry(s32* x, s32* y, s32* width, s32* height) { - auto lock = Host::GetSettingsLock(); + const auto lock = Host::GetSettingsLock(); - bool result = s_state.base_settings_interface->GetIntValue("SimpleHost", "WindowX", x); - result = result && s_state.base_settings_interface->GetIntValue("SimpleHost", "WindowY", y); - result = result && s_state.base_settings_interface->GetIntValue("SimpleHost", "WindowWidth", width); - result = result && s_state.base_settings_interface->GetIntValue("SimpleHost", "WindowHeight", height); + bool result = s_state.base_settings_interface.GetIntValue("SimpleHost", "WindowX", x); + result = result && s_state.base_settings_interface.GetIntValue("SimpleHost", "WindowY", y); + result = result && s_state.base_settings_interface.GetIntValue("SimpleHost", "WindowWidth", width); + result = result && s_state.base_settings_interface.GetIntValue("SimpleHost", "WindowHeight", height); return result; } @@ -744,12 +744,11 @@ void MiniHost::SavePlatformWindowGeometry(s32 x, s32 y, s32 width, s32 height) if (Host::IsFullscreen()) return; - auto lock = Host::GetSettingsLock(); - s_state.base_settings_interface->SetIntValue("SimpleHost", "WindowX", x); - s_state.base_settings_interface->SetIntValue("SimpleHost", "WindowY", y); - s_state.base_settings_interface->SetIntValue("SimpleHost", "WindowWidth", width); - s_state.base_settings_interface->SetIntValue("SimpleHost", "WindowHeight", height); - s_state.base_settings_interface->Save(); + const auto lock = Host::GetSettingsLock(); + s_state.base_settings_interface.SetIntValue("SimpleHost", "WindowX", x); + s_state.base_settings_interface.SetIntValue("SimpleHost", "WindowY", y); + s_state.base_settings_interface.SetIntValue("SimpleHost", "WindowWidth", width); + s_state.base_settings_interface.SetIntValue("SimpleHost", "WindowHeight", height); } void MiniHost::UIThreadMainLoop() @@ -1275,7 +1274,7 @@ void Host::RequestResetSettings(bool system, bool controller) auto lock = Host::GetSettingsLock(); { - SettingsInterface& si = *s_state.base_settings_interface.get(); + SettingsInterface& si = s_state.base_settings_interface; if (system) { @@ -1846,7 +1845,8 @@ int main(int argc, char* argv[]) // Ensure log is flushed. Log::SetFileOutputParams(false, nullptr); - s_state.base_settings_interface.reset(); + if (s_state.base_settings_interface.IsDirty()) + s_state.base_settings_interface.Save(); SDL_QuitSubSystem(SDL_INIT_VIDEO | SDL_INIT_EVENTS); diff --git a/src/duckstation-qt/qthost.cpp b/src/duckstation-qt/qthost.cpp index e2a6dab96..35c1d2059 100644 --- a/src/duckstation-qt/qthost.cpp +++ b/src/duckstation-qt/qthost.cpp @@ -115,7 +115,7 @@ static bool ParseCommandLineParametersAndInitializeConfig(QApplication& app, std::shared_ptr& boot_params); } // namespace QtHost -static std::unique_ptr s_base_settings_interface; +static INISettingsInterface s_base_settings_interface; static std::unique_ptr s_settings_save_timer; static bool s_batch_mode = false; static bool s_nogui_mode = false; @@ -255,7 +255,7 @@ QString QtHost::GetResourcesBasePath() INISettingsInterface* QtHost::GetBaseSettingsInterface() { - return s_base_settings_interface.get(); + return &s_base_settings_interface; } bool QtHost::SaveGameSettings(SettingsInterface* sif, bool delete_if_empty) @@ -482,37 +482,37 @@ bool QtHost::InitializeConfig() std::string settings_path = Path::Combine(EmuFolders::DataRoot, "settings.ini"); const bool settings_exists = FileSystem::FileExists(settings_path.c_str()); INFO_LOG("Loading config from {}.", settings_path); - s_base_settings_interface = std::make_unique(std::move(settings_path)); - Host::Internal::SetBaseSettingsLayer(s_base_settings_interface.get()); + s_base_settings_interface.SetPath(std::move(settings_path)); + Host::Internal::SetBaseSettingsLayer(&s_base_settings_interface); uint settings_version; - if (!settings_exists || !s_base_settings_interface->Load() || - !s_base_settings_interface->GetUIntValue("Main", "SettingsVersion", &settings_version) || + if (!settings_exists || !s_base_settings_interface.Load() || + !s_base_settings_interface.GetUIntValue("Main", "SettingsVersion", &settings_version) || settings_version != SETTINGS_VERSION) { - if (s_base_settings_interface->ContainsValue("Main", "SettingsVersion")) + if (s_base_settings_interface.ContainsValue("Main", "SettingsVersion")) { // NOTE: No point translating this, because there's no config loaded, so no language loaded. Host::ReportErrorAsync("Error", fmt::format("Settings version {} does not match expected version {}, resetting.", settings_version, SETTINGS_VERSION)); } - s_base_settings_interface->SetUIntValue("Main", "SettingsVersion", SETTINGS_VERSION); - SetDefaultSettings(*s_base_settings_interface, true, true); + s_base_settings_interface.SetUIntValue("Main", "SettingsVersion", SETTINGS_VERSION); + SetDefaultSettings(s_base_settings_interface, true, true); // Flag for running the setup wizard if this is our first run. We want to run it next time if they don't finish it. - s_base_settings_interface->SetBoolValue("Main", "SetupWizardIncomplete", true); + s_base_settings_interface.SetBoolValue("Main", "SetupWizardIncomplete", true); // Make sure we can actually save the config, and the user doesn't have some permission issue. Error error; - if (!s_base_settings_interface->Save(&error)) + if (!s_base_settings_interface.Save(&error)) { QMessageBox::critical( nullptr, QStringLiteral("DuckStation"), QStringLiteral( "Failed to save configuration to\n\n%1\n\nThe error was: %2\n\nPlease ensure this directory is writable. You " "can also try portable mode by creating portable.txt in the same directory you installed DuckStation into.") - .arg(QString::fromStdString(s_base_settings_interface->GetPath())) + .arg(QString::fromStdString(s_base_settings_interface.GetPath())) .arg(QString::fromStdString(error.GetDescription()))); return false; } @@ -520,15 +520,15 @@ bool QtHost::InitializeConfig() // Setup wizard was incomplete last time? s_run_setup_wizard = - s_run_setup_wizard || s_base_settings_interface->GetBoolValue("Main", "SetupWizardIncomplete", false); + s_run_setup_wizard || s_base_settings_interface.GetBoolValue("Main", "SetupWizardIncomplete", false); - EmuFolders::LoadConfig(*s_base_settings_interface.get()); + EmuFolders::LoadConfig(s_base_settings_interface); EmuFolders::EnsureFoldersExist(); MigrateSettings(); // We need to create the console window early, otherwise it appears in front of the main window. - if (!Log::IsConsoleOutputEnabled() && s_base_settings_interface->GetBoolValue("Logging", "LogToConsole", false)) - Log::SetConsoleOutputParams(true, s_base_settings_interface->GetBoolValue("Logging", "LogTimestamps", true)); + if (!Log::IsConsoleOutputEnabled() && s_base_settings_interface.GetBoolValue("Logging", "LogToConsole", false)) + Log::SetConsoleOutputParams(true, s_base_settings_interface.GetBoolValue("Logging", "LogTimestamps", true)); UpdateApplicationLanguage(nullptr); return true; @@ -645,7 +645,7 @@ void EmuThread::setDefaultSettings(bool system /* = true */, bool controller /* { auto lock = Host::GetSettingsLock(); - QtHost::SetDefaultSettings(*s_base_settings_interface, system, controller); + QtHost::SetDefaultSettings(s_base_settings_interface, system, controller); QtHost::QueueSettingsSave(); } @@ -675,15 +675,15 @@ void QtHost::SetDefaultSettings(SettingsInterface& si, bool system, bool control void QtHost::MigrateSettings() { SmallString value; - if (s_base_settings_interface->GetStringValue("Display", "SyncMode", &value)) + if (s_base_settings_interface.GetStringValue("Display", "SyncMode", &value)) { - s_base_settings_interface->SetBoolValue("Display", "VSync", (value == "VSync" || value == "VSyncRelaxed")); - s_base_settings_interface->SetBoolValue( + s_base_settings_interface.SetBoolValue("Display", "VSync", (value == "VSync" || value == "VSyncRelaxed")); + s_base_settings_interface.SetBoolValue( "Display", "OptimalFramePacing", - (value == "VRR" || s_base_settings_interface->GetBoolValue("Display", "DisplayAllFrames", false))); - s_base_settings_interface->DeleteValue("Display", "SyncMode"); - s_base_settings_interface->DeleteValue("Display", "DisplayAllFrames"); - s_base_settings_interface->Save(); + (value == "VRR" || s_base_settings_interface.GetBoolValue("Display", "DisplayAllFrames", false))); + s_base_settings_interface.DeleteValue("Display", "SyncMode"); + s_base_settings_interface.DeleteValue("Display", "DisplayAllFrames"); + s_base_settings_interface.Save(); } } @@ -2553,7 +2553,7 @@ void QtHost::SaveSettings() { Error error; auto lock = Host::GetSettingsLock(); - if (!s_base_settings_interface->Save(&error)) + if (!s_base_settings_interface.Save(&error)) ERROR_LOG("Failed to save settings: {}", error.GetDescription()); } diff --git a/src/duckstation-regtest/regtest_host.cpp b/src/duckstation-regtest/regtest_host.cpp index 603a9e360..01d38095c 100644 --- a/src/duckstation-regtest/regtest_host.cpp +++ b/src/duckstation-regtest/regtest_host.cpp @@ -70,7 +70,7 @@ static RegTestHostState s_state; } // namespace RegTestHost -static std::unique_ptr s_base_settings_interface; +static MemorySettingsInterface s_base_settings_interface; static Threading::Thread s_gpu_thread; static u32 s_frames_to_run = 60 * 60; @@ -114,11 +114,10 @@ bool RegTestHost::InitializeConfig() { SetFolders(); - s_base_settings_interface = std::make_unique(); - Host::Internal::SetBaseSettingsLayer(s_base_settings_interface.get()); + Host::Internal::SetBaseSettingsLayer(&s_base_settings_interface); // default settings for runner - SettingsInterface& si = *s_base_settings_interface.get(); + SettingsInterface& si = s_base_settings_interface; g_settings.Load(si, si); g_settings.Save(si, false); si.SetStringValue("GPU", "Renderer", Settings::GetRendererName(GPURenderer::Software)); @@ -140,7 +139,7 @@ bool RegTestHost::InitializeConfig() for (u32 i = 0; i < static_cast(InputSourceType::Count); i++) si.SetBoolValue("InputSources", InputManager::InputSourceToString(static_cast(i)), false); - EmuFolders::LoadConfig(*s_base_settings_interface.get()); + EmuFolders::LoadConfig(s_base_settings_interface); EmuFolders::EnsureFoldersExist(); return true; @@ -846,13 +845,13 @@ bool RegTestHost::ParseCommandLineParameters(int argc, char* argv[], std::option } Log::SetLogLevel(level.value()); - s_base_settings_interface->SetStringValue("Logging", "LogLevel", Settings::GetLogLevelName(level.value())); + s_base_settings_interface.SetStringValue("Logging", "LogLevel", Settings::GetLogLevelName(level.value())); continue; } else if (CHECK_ARG("-console")) { Log::SetConsoleOutputParams(true); - s_base_settings_interface->SetBoolValue("Logging", "LogToConsole", true); + s_base_settings_interface.SetBoolValue("Logging", "LogToConsole", true); continue; } else if (CHECK_ARG_PARAM("-renderer")) @@ -864,7 +863,7 @@ bool RegTestHost::ParseCommandLineParameters(int argc, char* argv[], std::option return false; } - s_base_settings_interface->SetStringValue("GPU", "Renderer", Settings::GetRendererName(renderer.value())); + s_base_settings_interface.SetStringValue("GPU", "Renderer", Settings::GetRendererName(renderer.value())); continue; } else if (CHECK_ARG_PARAM("-upscale")) @@ -877,7 +876,7 @@ bool RegTestHost::ParseCommandLineParameters(int argc, char* argv[], std::option } INFO_LOG("Setting upscale to {}.", upscale); - s_base_settings_interface->SetIntValue("GPU", "ResolutionScale", static_cast(upscale)); + s_base_settings_interface.SetIntValue("GPU", "ResolutionScale", static_cast(upscale)); continue; } else if (CHECK_ARG_PARAM("-cpu")) @@ -890,21 +889,21 @@ bool RegTestHost::ParseCommandLineParameters(int argc, char* argv[], std::option } INFO_LOG("Setting CPU execution mode to {}.", Settings::GetCPUExecutionModeName(cpu.value())); - s_base_settings_interface->SetStringValue("CPU", "ExecutionMode", + s_base_settings_interface.SetStringValue("CPU", "ExecutionMode", Settings::GetCPUExecutionModeName(cpu.value())); continue; } else if (CHECK_ARG("-pgxp")) { INFO_LOG("Enabling PGXP."); - s_base_settings_interface->SetBoolValue("GPU", "PGXPEnable", true); + s_base_settings_interface.SetBoolValue("GPU", "PGXPEnable", true); continue; } else if (CHECK_ARG("-pgxp-cpu")) { INFO_LOG("Enabling PGXP CPU mode."); - s_base_settings_interface->SetBoolValue("GPU", "PGXPEnable", true); - s_base_settings_interface->SetBoolValue("GPU", "PGXPCPU", true); + s_base_settings_interface.SetBoolValue("GPU", "PGXPEnable", true); + s_base_settings_interface.SetBoolValue("GPU", "PGXPCPU", true); continue; } else if (CHECK_ARG("--")) @@ -948,9 +947,9 @@ bool RegTestHost::SetNewDataRoot(const std::string& filename) // Switch to file logging. INFO_LOG("Dumping frames to '{}'...", dump_directory); EmuFolders::DataRoot = std::move(dump_directory); - s_base_settings_interface->SetBoolValue("Logging", "LogToFile", true); - s_base_settings_interface->SetStringValue("Logging", "LogLevel", Settings::GetLogLevelName(Log::Level::Dev)); - Settings::UpdateLogConfig(*s_base_settings_interface); + s_base_settings_interface.SetBoolValue("Logging", "LogToFile", true); + s_base_settings_interface.SetStringValue("Logging", "LogLevel", Settings::GetLogLevelName(Log::Level::Dev)); + Settings::UpdateLogConfig(s_base_settings_interface); } return true; diff --git a/src/util/ini_settings_interface.cpp b/src/util/ini_settings_interface.cpp index ebfe98a39..329d75961 100644 --- a/src/util/ini_settings_interface.cpp +++ b/src/util/ini_settings_interface.cpp @@ -19,7 +19,11 @@ LOG_CHANNEL(Settings); // we only allow one ini to be parsed at any point in time. static std::mutex s_ini_load_save_mutex; -INISettingsInterface::INISettingsInterface(std::string filename) : m_path(std::move(filename)), m_ini(true, true) +INISettingsInterface::INISettingsInterface() : m_ini(true, true) +{ +} + +INISettingsInterface::INISettingsInterface(std::string path) : m_path(std::move(path)), m_ini(true, true) { } @@ -29,6 +33,12 @@ INISettingsInterface::~INISettingsInterface() Save(); } +void INISettingsInterface::SetPath(std::string path) +{ + m_dirty |= (path != m_path); + m_path = std::move(path); +} + bool INISettingsInterface::Load(Error* error /* = nullptr */) { if (m_path.empty()) @@ -47,6 +57,7 @@ bool INISettingsInterface::Load(Error* error /* = nullptr */) Error::SetStringFmt(error, "INI LoadFile() failed: {}", static_cast(err)); } + m_dirty = false; return (err == SI_OK); } diff --git a/src/util/ini_settings_interface.h b/src/util/ini_settings_interface.h index 2ec493d84..565b9fd68 100644 --- a/src/util/ini_settings_interface.h +++ b/src/util/ini_settings_interface.h @@ -13,10 +13,13 @@ class INISettingsInterface final : public SettingsInterface { public: + INISettingsInterface(); INISettingsInterface(std::string path); ~INISettingsInterface() override; - const std::string& GetPath() const { return m_path; } + ALWAYS_INLINE bool IsDirty() const { return m_dirty; } + ALWAYS_INLINE const std::string& GetPath() const { return m_path; } + void SetPath(std::string path); bool Load(Error* error = nullptr); bool Load(std::string new_path, Error* error = nullptr);