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

@@ -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.
  */

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 <typename... Args>
-    ConfigVar(const char* name, Args&&... arg)
-        : ConfigVarBase(name, GetConfigImpl<T>()), defaultValue(std::forward<Args>(arg)...),
+    ConfigVar(std::string name, Args&&... arg)
+        : ConfigVarBase(std::move(name), GetConfigImpl<T>()), defaultValue(std::forward<Args>(arg)...),
         value(), overrideValue() {}
 
+    ConfigVar(ConfigVar const&) = delete;
+
     /**
      * \brief Get the current value of the CVar.
      *

src/dusk/config.cpp

@@ -12,8 +12,9 @@
 #include <system_error>
 #include <string>
 
-#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<std::string_view, ConfigVarBase*> RegisteredConfigVars;
-static bool RegistrationDone = false;
+static absl::flat_hash_map<std::string, ConfigVarBase*> RegisteredConfigVars;
+static absl::flat_hash_map<std::string, nlohmann::json> UnregisteredConfigVars;
+static absl::flat_hash_map<std::string, std::string> 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 <typename T>
 static T sanitizeEnumValue(const ConfigVar<T>& cVar, T value) {
     if constexpr (std::is_enum_v<T>) {
@@ -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<void(ConfigVarBase&)> callb
         callback(*pair.second);
     }
 }
+
+void dusk::config::Shutdown() {
+    for (auto& pair : RegisteredConfigVars) {
+        pair.second->unmarkRegistered();
+    }
+
+    RegisteredConfigVars.clear();
+    UnregisteredConfigVars.clear();
+    UnregisteredConfigVarOverrides.clear();
+}

src/dusk/ui/settings.cpp

@@ -429,7 +429,7 @@ void add_speedrun_disabled_option(Pane& leftPane, Pane& rightPane, ConfigVar<boo
     config_bool_select(leftPane, rightPane, var, {
         .key = key,
         .helpText = helpText,
-        .isDisabled = [] { return getSettings().game.speedrunMode; },
+        .isDisabled = [] { return getSettings().game.speedrunMode.getValue(); },
     });
 }
 
@@ -463,7 +463,7 @@ SelectButton& config_int_select(Pane& leftPane, Pane& rightPane, ConfigVar<int>&
     std::function<bool()> isDisabled = {}, std::string suffix = "") {
     auto& button = leftPane.add_child<NumberButton>(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,
             {

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;