From 3792912ad14f90993fb8885d8efe24dd80a4a09e Mon Sep 17 00:00:00 2001 From: PJB3005 Date: Fri, 29 May 2026 04:38:24 +0200 Subject: [PATCH] Allow CVars to be registered late, improve usability for dynamically registered CVars. Now we store the raw JSON value in memory for unregistered CVars. Intended to be used for mod CVars, as we obviously can't statically define all of those. CVar names are now stored as an std::string, so the lifetime is easy to manage when dynamically registered. CVars cannot be moved/copied anymore. We had some code that was accidentally relying on this, and I fixed that. --- include/dusk/config.hpp | 9 ++-- include/dusk/config_var.hpp | 17 +++++-- src/dusk/config.cpp | 94 ++++++++++++++++++++++++++++++------- src/dusk/ui/settings.cpp | 10 ++-- src/m_Do/m_Do_main.cpp | 14 ++---- 5 files changed, 101 insertions(+), 43 deletions(-) diff --git a/include/dusk/config.hpp b/include/dusk/config.hpp index 382c4c24c6..5994ec6375 100644 --- a/include/dusk/config.hpp +++ b/include/dusk/config.hpp @@ -89,17 +89,16 @@ public: */ void Register(ConfigVarBase& configVar); -/** - * \brief Indicate that all registrations have happened and everything should lock in. - */ -void FinishRegistration(); - /** * \brief Load config from the standard user preferences location. */ void LoadFromUserPreferences(); void LoadFromFileName(const char* path); +void LoadArgOverride(std::string_view name, std::string_view value); + +void Shutdown(); + /** * \brief Save the config to file. */ diff --git a/include/dusk/config_var.hpp b/include/dusk/config_var.hpp index 0bae27bfd3..86cb32a1cd 100644 --- a/include/dusk/config_var.hpp +++ b/include/dusk/config_var.hpp @@ -68,7 +68,7 @@ protected: /** * The name of this CVar, used in the configuration file. */ - const char* name; + std::string name; /** * Whether this CVar has been registered with the global managing logic. @@ -86,8 +86,10 @@ protected: */ const ConfigImplBase* impl; - ConfigVarBase(const char* name, const ConfigImplBase* impl); - virtual ~ConfigVarBase() = default; + // The configuration system stores a direct pointer to the ConfigVar instance. + // It is not legal to move or copy it. + ConfigVarBase(const ConfigVarBase&) = delete; + ConfigVarBase(std::string name, const ConfigImplBase* impl); /** * Check that the CVar is registered, aborting if this is not the case. @@ -98,6 +100,8 @@ protected: } public: + virtual ~ConfigVarBase(); + /** * Get the name of this CVar, used in the configuration file. */ @@ -120,6 +124,7 @@ public: * This is necessary to make it legal to access. */ void markRegistered(); + void unmarkRegistered(); /** * Clear a speedrun-mode override if one is active on this CVar. @@ -185,10 +190,12 @@ public: * @param arg Arguments to forward to construct the default value. */ template - ConfigVar(const char* name, Args&&... arg) - : ConfigVarBase(name, GetConfigImpl()), defaultValue(std::forward(arg)...), + ConfigVar(std::string name, Args&&... arg) + : ConfigVarBase(std::move(name), GetConfigImpl()), defaultValue(std::forward(arg)...), value(), overrideValue() {} + ConfigVar(ConfigVar const&) = delete; + /** * \brief Get the current value of the CVar. * diff --git a/src/dusk/config.cpp b/src/dusk/config.cpp index aed43089c4..55a6527389 100644 --- a/src/dusk/config.cpp +++ b/src/dusk/config.cpp @@ -12,8 +12,9 @@ #include #include -#include "dusk/main.h" #include "dusk/action_bindings.h" +#include "dusk/logging.h" +#include "dusk/main.h" using namespace dusk::config; @@ -23,8 +24,9 @@ using json = nlohmann::json; aurora::Module DuskConfigLog("dusk::config"); -static absl::flat_hash_map RegisteredConfigVars; -static bool RegistrationDone = false; +static absl::flat_hash_map RegisteredConfigVars; +static absl::flat_hash_map UnregisteredConfigVars; +static absl::flat_hash_map UnregisteredConfigVarOverrides; static std::filesystem::path GetConfigJsonPath() { return dusk::ConfigPath / ConfigFileName; @@ -46,17 +48,23 @@ static void ReplaceFile(const std::filesystem::path& source, const std::filesyst } } -ConfigVarBase::ConfigVarBase(const char* name, const ConfigImplBase* impl) : name(name), registered(false), layer(ConfigVarLayer::Default), impl(impl) { +ConfigVarBase::ConfigVarBase(std::string name, const ConfigImplBase* impl) : name(std::move(name)), registered(false), layer(ConfigVarLayer::Default), impl(impl) { } const char* ConfigVarBase::getName() const noexcept { - return name; + return name.c_str(); } const ConfigImplBase* ConfigVarBase::getImpl() const noexcept { return impl; } +ConfigVarBase::~ConfigVarBase() { + if (registered) { + DuskLog.fatal("CVar '{}' was destroyed while still registered!", name); + } +} + template static T sanitizeEnumValue(const ConfigVar& cVar, T value) { if constexpr (std::is_enum_v) { @@ -200,17 +208,37 @@ namespace dusk::config { } void dusk::config::Register(ConfigVarBase& configVar) { - const auto& name = configVar.getName(); - if (RegistrationDone) { - DuskConfigLog.fatal("Tried to register CVar {} after registrations closed!", name); - } - + const std::string_view name = configVar.getName(); if (RegisteredConfigVars.contains(name)) { DuskConfigLog.fatal("Tried to register CVar {} twice!", name); } RegisteredConfigVars[name] = &configVar; configVar.markRegistered(); + + const auto unregPair = UnregisteredConfigVars.find(name); + if (unregPair != UnregisteredConfigVars.end()) { + const auto value = std::move(unregPair->second); + UnregisteredConfigVars.erase(name); + + try { + configVar.getImpl()->loadFromJson(configVar, value); + } catch (std::exception& e) { + DuskConfigLog.error("Failed to load key '{}' from config value: {}", name, e.what()); + } + } + + const auto overridePair = UnregisteredConfigVarOverrides.find(name); + if (overridePair != UnregisteredConfigVarOverrides.end()) { + const auto value = std::move(overridePair->second); + UnregisteredConfigVars.erase(name); + + try { + configVar.getImpl()->loadFromArg(configVar, value); + } catch (std::exception& e) { + DuskConfigLog.error("Failed to load key '{}' from override arg: {}", name, e.what()); + } + } } void ConfigVarBase::markRegistered() { @@ -220,8 +248,11 @@ void ConfigVarBase::markRegistered() { registered = true; } -void dusk::config::FinishRegistration() { - RegistrationDone = true; +void ConfigVarBase::unmarkRegistered() { + if (!registered) + abort(); + + registered = false; } void dusk::config::LoadFromUserPreferences() { @@ -242,11 +273,16 @@ static void LoadFromPath(const char* path) { return; } + UnregisteredConfigVars.clear(); + for (const auto& el : j.items()) { const auto& key = el.key(); auto configVar = RegisteredConfigVars.find(key); if (configVar == RegisteredConfigVars.end()) { - DuskConfigLog.error("Unknown key '{}' found in config!", key); + DuskConfigLog.debug( + "Unknown key '{}' found in config! If this gets registered later, that's acceptable!", + key); + UnregisteredConfigVars.emplace(key, el.value()); continue; } @@ -259,10 +295,6 @@ static void LoadFromPath(const char* path) { } void dusk::config::LoadFromFileName(const char* path) { - if (!RegistrationDone) { - DuskConfigLog.fatal("Registration not finished yet!"); - } - DuskConfigLog.info("Loading config from '{}'", path); try { @@ -280,6 +312,20 @@ void dusk::config::LoadFromFileName(const char* path) { } } +void dusk::config::LoadArgOverride(std::string_view name, std::string_view value) { + const auto cVar = GetConfigVar(name); + if (!cVar) { + UnregisteredConfigVarOverrides.emplace(name, name); + return; + } + + try { + cVar->getImpl()->loadFromArg(*cVar, value); + } catch (const std::exception& e) { + DuskLog.fatal("Unable to parse: '{}': {}", value, e.what()); + } +} + void dusk::config::Save() { const auto configJsonPath = GetConfigJsonPath(); if (configJsonPath.empty()) { @@ -300,6 +346,10 @@ void dusk::config::Save() { } } + for (const auto& pair : UnregisteredConfigVars) { + j[pair.first] = pair.second; + } + try { const auto tempConfigJsonPath = GetTempConfigJsonPath(configJsonPath); io::FileStream::WriteAllText(tempConfigJsonPath, j.dump(4)); @@ -330,3 +380,13 @@ void dusk::config::EnumerateRegistered(std::function callb callback(*pair.second); } } + +void dusk::config::Shutdown() { + for (auto& pair : RegisteredConfigVars) { + pair.second->unmarkRegistered(); + } + + RegisteredConfigVars.clear(); + UnregisteredConfigVars.clear(); + UnregisteredConfigVarOverrides.clear(); +} diff --git a/src/dusk/ui/settings.cpp b/src/dusk/ui/settings.cpp index 99dee7ace7..444b910dd5 100644 --- a/src/dusk/ui/settings.cpp +++ b/src/dusk/ui/settings.cpp @@ -429,7 +429,7 @@ void add_speedrun_disabled_option(Pane& leftPane, Pane& rightPane, ConfigVar& std::function isDisabled = {}, std::string suffix = "") { auto& button = leftPane.add_child(NumberButton::Props{ .key = std::move(key), - .getValue = [&var] { return var; }, + .getValue = [&var] { return var.getValue(); }, .setValue = [&var, min, max](int value) { var.setValue(std::clamp(value, min, max)); @@ -1047,7 +1047,7 @@ SettingsWindow::SettingsWindow(bool prelaunch) : mPrelaunch(prelaunch) { leftPane.add_section("Tools"); addOption("Turbo Key", getSettings().game.enableTurboKeybind, "Hold Tab to increase game speed by up to 4x.", - [] { return getSettings().game.speedrunMode; }); + [] { return getSettings().game.speedrunMode.getValue(); }); addOption("Reset Key (" + Rml::String{hotkeys::DO_RESET} + ")", getSettings().game.enableResetKeybind, "Press " + Rml::String{hotkeys::DO_RESET} + " to reset the game."); @@ -1155,7 +1155,7 @@ SettingsWindow::SettingsWindow(bool prelaunch) : mPrelaunch(prelaunch) { getSettings().game.damageMultiplier.setValue(value); config::Save(); }, - .isDisabled = [] { return getSettings().game.speedrunMode; }, + .isDisabled = [] { return getSettings().game.speedrunMode.getValue(); }, .isModified = [] { return getSettings().game.damageMultiplier.getValue() != @@ -1446,7 +1446,7 @@ SettingsWindow::SettingsWindow(bool prelaunch) : mPrelaunch(prelaunch) { } } }, - .isDisabled = [] { return getSettings().game.speedrunMode; }, + .isDisabled = [] { return getSettings().game.speedrunMode.getValue(); }, }); config_bool_select(leftPane, rightPane, getSettings().game.showInputViewer, { diff --git a/src/m_Do/m_Do_main.cpp b/src/m_Do/m_Do_main.cpp index a813368a63..542b64584b 100644 --- a/src/m_Do/m_Do_main.cpp +++ b/src/m_Do/m_Do_main.cpp @@ -416,16 +416,7 @@ static void ApplyCVarOverrides(const cxxopts::OptionValue& option) { const auto name = std::string_view(cvarArg).substr(0, sep); const auto value = std::string_view(cvarArg).substr(sep + 1); - const auto cVar = dusk::config::GetConfigVar(name); - if (!cVar) { - DuskLog.fatal("Unknown --cvar name: '{}'", name); - } - - try { - cVar->getImpl()->loadFromArg(*cVar, value); - } catch (const std::exception& e) { - DuskLog.fatal("Unable to parse: '{}': {}", value, e.what()); - } + dusk::config::LoadArgOverride(name, value); } } @@ -504,7 +495,6 @@ int game_main(int argc, char* argv[]) { mainCalled = true; dusk::registerSettings(); - dusk::config::FinishRegistration(); cxxopts::ParseResult parsed_arg_options; @@ -796,6 +786,8 @@ int game_main(int argc, char* argv[]) { dusk::discord::shutdown(); #endif dusk::ui::shutdown(); + + dusk::config::Shutdown(); aurora_shutdown(); return 0;