make ModSettingsManager manage savedata fully

loader/include/Geode/loader/ModSettingsManager.hpp

@@ -4,12 +4,16 @@
 #include "SettingV3.hpp"
 
 namespace geode {
+    class Mod;
+    class SettingV3;
+
     class GEODE_DLL ModSettingsManager final {
     private:
         class Impl;
         std::unique_ptr<Impl> m_impl;
 
         friend class ::geode::SettingV3;
+        friend class ::geode::Mod;
 
         void markRestartRequired();
 
@@ -36,9 +40,23 @@ namespace geode {
          * The format of the savedata will be an object with the keys being 
          * setting IDs and then the values the values of the saved settings
          * @note If saving a setting fails, it will log a warning to the console
+         * @warning This will overwrite the whole `json` parameter - be sure to 
+         * pass the full settings savedata to `load()` so you can be sure that 
+         * unregistered custom settings' saved values don't disappear!
+         * @todo in v4: make this return the value instead lol
          */
         void save(matjson::Value& json);
 
+        // todo in 3.7.0: add this
+        /**
+         * Get the savedata for settings, aka the JSON object that contains all 
+         * the settings' saved states that was loaded up from disk and will be 
+         * saved to disk
+         * @warning Modifying this will modify the value of the settings - use 
+         * carefully!
+         */
+        // matjson::Value& getSaveData();
+
         Result<> registerCustomSettingType(std::string_view type, SettingGenerator generator);
         // todo in v4: remove this
         Result<> registerLegacyCustomSetting(std::string_view key, std::unique_ptr<SettingValue>&& ptr);

loader/src/loader/Mod.cpp

@@ -48,9 +48,8 @@ matjson::Value& Mod::getSaveContainer() {
     return m_impl->getSaveContainer();
 }
 
-matjson::Value& Mod::getSavedSettingsData() {
+// todo in 3.7.0: move Mod::getSavedSettingsData() back here from 
-    return m_impl->getSavedSettingsData();
+// ModSettingsManager.cpp and make it use ModSettingsManager::getSaveData()
-}
 
 bool Mod::isEnabled() const {
     return m_impl->isEnabled();

loader/src/loader/ModImpl.cpp

@@ -138,10 +138,6 @@ matjson::Value& Mod::Impl::getSaveContainer() {
     return m_saved;
 }
 
-matjson::Value& Mod::Impl::getSavedSettingsData() {
-    return m_savedSettingsData;
-}
-
 bool Mod::Impl::isEnabled() const {
     return m_enabled || this->isInternal();
 }
@@ -182,7 +178,6 @@ Result<> Mod::Impl::loadData() {
     auto settingPath = m_saveDirPath / "settings.json";
     if (std::filesystem::exists(settingPath)) {
         GEODE_UNWRAP_INTO(auto json, utils::file::readJson(settingPath));
-        m_savedSettingsData = json;
         auto load = m_settings->load(json);
         if (!load) {
             log::warn("Unable to load settings: {}", load.unwrapErr());
@@ -214,14 +209,8 @@ Result<> Mod::Impl::saveData() {
         return Ok();
     }
 
-    // Data saving should be fully fail-safe
+    // ModSettingsManager keeps track of the whole savedata
-    // If some settings weren't provided a custom settings handler (for example,
+    matjson::Value json;
-    // the mod was not loaded) then make sure to save their previous state in
-    // order to not lose data
-    if (!m_savedSettingsData.is_object()) {
-        m_savedSettingsData = matjson::Object();
-    }
-    matjson::Value json = m_savedSettingsData;
     m_settings->save(json);
 
     // saveData is expected to be synchronous, and always called from GD thread

loader/src/loader/ModImpl.hpp

@@ -52,10 +52,6 @@ namespace geode {
          * Setting values. This is behind unique_ptr for interior mutability
          */
         std::unique_ptr<ModSettingsManager> m_settings = nullptr;
-        /**
-         * Settings save data. Stored for efficient loading of custom settings
-         */
-        matjson::Value m_savedSettingsData = matjson::Object();
         /**
          * Whether the mod resources are loaded or not
          */
@@ -101,7 +97,6 @@ namespace geode {
         std::filesystem::path getBinaryPath() const;
 
         matjson::Value& getSaveContainer();
-        matjson::Value& getSavedSettingsData();
 
 #if defined(GEODE_EXPOSE_SECRET_INTERNALS_IN_HEADERS_DO_NOT_DEFINE_PLEASE)
         void setMetadata(ModMetadata const& metadata);

loader/src/loader/ModSettingsManager.cpp

@@ -89,7 +89,7 @@ public:
     // Stored so custom settings registered after the fact can be loaded
     // If the ability to unregister custom settings is ever added, remember to 
     // update this by calling saveSettingValueToSave
-    matjson::Object savedata;
+    matjson::Value savedata;
     bool restartRequired = false;
 
     void loadSettingValueFromSave(std::string const& key) {
@@ -232,13 +232,12 @@ void ModSettingsManager::save(matjson::Value& json) {
     for (auto& [key, _] : m_impl->settings) {
         m_impl->saveSettingValueToSave(key);
     }
-    // Doing this indirection instead of just `json = m_impl->savedata` because 
+    // Doing this since `ModSettingsManager` is expected to manage savedata fully
-    // we do NOT want to accidentally discard keys present in `json` but not in 
+    json = m_impl->savedata;
-    // `m_impl->savedata`
-    for (auto& [key, value] : m_impl->savedata) {
-        json[key] = value;
-    }
 }
+// matjson::Value& ModSettingsManager::getSaveData() {
+//     return m_impl->savedata;
+// }
 
 std::shared_ptr<SettingV3> ModSettingsManager::get(std::string_view key) {
     auto id = std::string(key);
@@ -275,3 +274,9 @@ std::optional<Setting> ModSettingsManager::getLegacyDefinition(std::string_view
 bool ModSettingsManager::restartRequired() const {
     return m_impl->restartRequired;
 }
+
+// todo in 3.7.0: move Mod::getSavedSettingsData() back to Mod.cpp and make it 
+// use ModSettingsManager::getSaveData()
+matjson::Value& Mod::getSavedSettingsData() {
+    return m_impl->m_settings->m_impl->savedata;
+}