diff --git a/loader/include/Geode/loader/Mod.hpp b/loader/include/Geode/loader/Mod.hpp index 6b2d32a89..7195df7a4 100644 --- a/loader/include/Geode/loader/Mod.hpp +++ b/loader/include/Geode/loader/Mod.hpp @@ -82,6 +82,8 @@ namespace geode { friend void GEODE_CALL ::geode_implicit_load(Mod*); void settingReact(geode::Function fn); + + matjson::Value& getSaveContainerTemp(); public: // no copying Mod(Mod const&) = delete; @@ -242,6 +244,7 @@ namespace geode { } matjson::Value& getSaveContainer(); + matjson::Value const& getSaveContainerConst() const; matjson::Value& getSavedSettingsData(); /** @@ -286,7 +289,7 @@ namespace geode { template T getSavedValue(std::string_view key) { - auto& saved = this->getSaveContainer(); + auto& saved = this->getSaveContainerConst(); if (auto res = saved.get(key).andThen([](auto&& v) { return v.template as(); }); res.isOk()) { @@ -297,13 +300,15 @@ namespace geode { template T getSavedValue(std::string_view key, T const& defaultValue) { - auto& saved = this->getSaveContainer(); + auto& saved = this->getSaveContainerConst(); if (auto res = saved.get(key).andThen([](auto&& v) { return v.template as(); }); res.isOk()) { return res.unwrap(); } - saved[key] = matjson::Value(defaultValue); + + auto savedMutable = this->getSaveContainerTemp(); + savedMutable[key] = matjson::Value(defaultValue); return defaultValue; } @@ -316,8 +321,22 @@ namespace geode { */ template T setSavedValue(std::string_view key, T const& value) { - auto& saved = this->getSaveContainer(); auto old = this->getSavedValue(key); + + // optimization: if the value is the same, don't write to the save container + // constexpr checks needed to avoid compile errors for types that don't support operator== + if constexpr (std::ranges::range) { + using Elem = std::ranges::range_value_t; + if constexpr (requires(Elem const& a, Elem const& b) { { a == b } -> std::convertible_to; }) { + if (old == value) return old; + } + } + else if constexpr (requires(T const& a, T const& b) { { a == b } -> std::convertible_to; }) { + if (old == value) return old; + } + + // value changed, write to save container + auto& saved = this->getSaveContainer(); saved[key] = value; return old; } diff --git a/loader/include/Geode/loader/ModSettingsManager.hpp b/loader/include/Geode/loader/ModSettingsManager.hpp index f7eeb2adf..581e7c924 100644 --- a/loader/include/Geode/loader/ModSettingsManager.hpp +++ b/loader/include/Geode/loader/ModSettingsManager.hpp @@ -16,6 +16,8 @@ namespace geode { friend class ::geode::Mod; void markRestartRequired(); + void queueSave(); + void saveFinished(); public: static ModSettingsManager* from(Mod* mod); @@ -67,5 +69,7 @@ namespace geode { * for this mod, they are also reloaded for the dependant mods */ void addDependant(Mod* mod); + + bool shouldSave() const; }; } diff --git a/loader/src/loader/Mod.cpp b/loader/src/loader/Mod.cpp index 46305b689..2c2bdc194 100644 --- a/loader/src/loader/Mod.cpp +++ b/loader/src/loader/Mod.cpp @@ -45,6 +45,14 @@ matjson::Value& Mod::getSaveContainer() { return m_impl->getSaveContainer(); } +matjson::Value& Mod::getSaveContainerTemp() { + return m_impl->getSaveContainerTemp(); +} + +matjson::Value const& Mod::getSaveContainerConst() const { + return m_impl->getSaveContainerConst(); +} + matjson::Value& Mod::getSavedSettingsData() { return m_impl->m_settings->getSaveData(); } diff --git a/loader/src/loader/ModImpl.cpp b/loader/src/loader/ModImpl.cpp index 31b9951a7..42f4451ae 100644 --- a/loader/src/loader/ModImpl.cpp +++ b/loader/src/loader/ModImpl.cpp @@ -165,6 +165,24 @@ VersionInfo Mod::Impl::getVersion() const { } matjson::Value& Mod::Impl::getSaveContainer() { + // saved value taken, so we need to save it every time + // since we dont know what the caller will do with the container + m_saveRequestState = SaveRequestState::SaveUntilExit; + + return m_saved; +} + +matjson::Value& Mod::Impl::getSaveContainerTemp() { + // saved value dirty - caller promises to get rid + // of its ref before next save + if(m_saveRequestState == SaveRequestState::Clean) { + m_saveRequestState = SaveRequestState::SaveOnce; + } + + return m_saved; +} + +matjson::Value const& Mod::Impl::getSaveContainerConst() const { return m_saved; } @@ -180,7 +198,7 @@ bool Mod::Impl::needsEarlyLoad(std::vector& checked) const { checked.push_back(m_self); if (this->getMetadata().needsEarlyLoad()) return true; for (auto& dep : m_dependants) { - if(std::find(checked.begin(), checked.end(), dep) != checked.end()) continue; + if (std::find(checked.begin(), checked.end(), dep) != checked.end()) continue; if (dep->m_impl->needsEarlyLoad(checked)) return true; } return false; @@ -209,11 +227,18 @@ Result<> Mod::Impl::loadData() { // Check if settings exist auto settingPath = m_saveDirPath / "settings.json"; if (std::filesystem::exists(settingPath)) { - GEODE_UNWRAP_INTO(auto json, utils::file::readJson(settingPath)); - auto load = m_settings->load(json); - if (!load) { - log::warn("Unable to load settings: {}", load.unwrapErr()); + if (auto json = utils::file::readJson(settingPath)) { + auto load = m_settings->load(json.unwrap()); + if (!load) { + log::warn("Unable to load settings: {}", load.unwrapErr()); + } + } else { + // this used to early return but skipping saved values is not great behavior here imo + m_settings->queueSave(); + log::warn("Unable to load settings: {}", json.unwrapErr()); } + } else { + m_settings->queueSave(); } // Saved values @@ -243,18 +268,36 @@ Result<> Mod::Impl::saveData() { } // ModSettingsManager keeps track of the whole savedata - matjson::Value json = m_settings->save(); + if (m_settings->shouldSave()) { + log::debug("Saving settings for mod {}", m_metadata.getID()); - // saveData is expected to be synchronous, and always called from GD thread - ModStateEvent(ModEventType::DataSaved, std::move(m_self)).send(); + matjson::Value json = m_settings->save(); - auto res = utils::file::writeStringSafe(m_saveDirPath / "settings.json", json.dump()); - if (!res) { - log::error("Unable to save settings: {}", res.unwrapErr()); + // saveData is expected to be synchronous, and always called from GD thread + ModStateEvent(ModEventType::DataSaved, std::move(m_self)).send(); + + auto res = utils::file::writeStringSafe(m_saveDirPath / "settings.json", json.dump()); + if (!res) { + log::error("Unable to save settings: {}", res.unwrapErr()); + } else { + m_settings->saveFinished(); + } + } else { + // duplicated line to retain old expectations of saveData being called after json dump but before file write + ModStateEvent(ModEventType::DataSaved, std::move(m_self)).send(); } - auto res2 = utils::file::writeStringSafe(m_saveDirPath / "saved.json", m_saved.dump()); - if (!res2) { - log::error("Unable to save values: {}", res2.unwrapErr()); + + if (m_saveRequestState != SaveRequestState::Clean) { + log::debug("Saving values for mod {}", m_metadata.getID()); + + auto res2 = utils::file::writeStringSafe(m_saveDirPath / "saved.json", m_saved.dump()); + if (!res2) { + log::error("Unable to save values: {}", res2.unwrapErr()); + } + + if(m_saveRequestState == SaveRequestState::SaveOnce) { + m_saveRequestState = SaveRequestState::Clean; + } } return Ok(); diff --git a/loader/src/loader/ModImpl.hpp b/loader/src/loader/ModImpl.hpp index 0c39b41be..a3790d639 100644 --- a/loader/src/loader/ModImpl.hpp +++ b/loader/src/loader/ModImpl.hpp @@ -8,6 +8,12 @@ #include namespace geode { + enum class SaveRequestState { + Clean, + SaveOnce, + SaveUntilExit, + }; + class Mod::Impl { public: Mod* m_self; @@ -49,6 +55,11 @@ namespace geode { * Saved values */ matjson::Value m_saved = matjson::Value(); + /** + * Whether the saved values need to be saved to disk one time + * (container dirty) + */ + SaveRequestState m_saveRequestState = SaveRequestState::Clean; /** * Setting values. This is behind unique_ptr for interior mutability */ @@ -105,6 +116,8 @@ namespace geode { bool isEphemeral() const; matjson::Value& getSaveContainer(); + matjson::Value& getSaveContainerTemp(); + matjson::Value const& getSaveContainerConst() const; #if defined(GEODE_EXPOSE_SECRET_INTERNALS_IN_HEADERS_DO_NOT_DEFINE_PLEASE) void setMetadata(ModMetadata const& metadata); diff --git a/loader/src/loader/ModSettingsManager.cpp b/loader/src/loader/ModSettingsManager.cpp index d76d48fce..4da4e2ced 100644 --- a/loader/src/loader/ModSettingsManager.cpp +++ b/loader/src/loader/ModSettingsManager.cpp @@ -188,6 +188,7 @@ class ModSettingsManager::Impl final { // update this by calling saveSettingValueToSave matjson::Value savedata; bool restartRequired = false; + SaveRequestState saveRequestState = SaveRequestState::Clean; bool loadSettingValueFromSave(std::string const& key) { if (this->savedata.contains(key) && this->settings.contains(key)) { @@ -205,6 +206,12 @@ class ModSettingsManager::Impl final { return true; } else { + if (!this->savedata.contains(key)) { + log::error("Unable to load setting '{}' for mod {} (not found in savedata)", key, this->modID); + if(saveRequestState == SaveRequestState::Clean) { + saveRequestState = SaveRequestState::SaveOnce; + } + } return false; } } @@ -237,7 +244,9 @@ class ModSettingsManager::Impl final { } if (auto v3 = (*gen)(key, modID, setting.json)) { setting.v3 = v3.unwrap(); - this->loadSettingValueFromSave(key); + + // the loading is unnecessary, m_saveData is not initialized yet + // this->loadSettingValueFromSave(key); } else { log::error( @@ -280,6 +289,22 @@ void ModSettingsManager::markRestartRequired() { m_impl->restartRequired = true; } +void ModSettingsManager::queueSave() { + if(m_impl->saveRequestState == SaveRequestState::Clean) { + m_impl->saveRequestState = SaveRequestState::SaveOnce; + } +} + +void ModSettingsManager::saveFinished() { + if(m_impl->saveRequestState == SaveRequestState::SaveOnce) { + m_impl->saveRequestState = SaveRequestState::Clean; + } +} + +bool ModSettingsManager::shouldSave() const { + return m_impl->saveRequestState != SaveRequestState::Clean; +} + Result<> ModSettingsManager::registerCustomSettingType(std::string_view type, SettingGenerator generator) { GEODE_UNWRAP(SharedSettingTypesPool::get().add(m_impl->modID, type, std::move(generator))); m_impl->createSettings(); @@ -310,6 +335,14 @@ Result<> ModSettingsManager::load(matjson::Value const& json) { } } } + + for (auto const& [key, _] : m_impl->settings) { + if (!json.contains(key)) { + log::error("Unable to load setting '{}' for mod {} (not found in savedata)", key, m_impl->modID); + this->queueSave(); + break; + } + } } return Ok(); } @@ -323,6 +356,8 @@ matjson::Value ModSettingsManager::save() { } matjson::Value& ModSettingsManager::getSaveData() { + m_impl->saveRequestState = SaveRequestState::SaveUntilExit; + return m_impl->savedata; } diff --git a/loader/src/loader/SettingV3.cpp b/loader/src/loader/SettingV3.cpp index 6c4953935..c445855aa 100644 --- a/loader/src/loader/SettingV3.cpp +++ b/loader/src/loader/SettingV3.cpp @@ -575,8 +575,11 @@ Mod* SettingV3::getMod() const { void SettingV3::markChanged() { auto manager = ModSettingsManager::from(this->getMod()); - if (m_impl->requiresRestart) { - manager->markRestartRequired(); + if (manager) { + if (m_impl->requiresRestart) { + manager->markRestartRequired(); + } + manager->queueSave(); } SettingChangedEventV3(this->getModID(), this->getKey()).send(shared_from_this()); } diff --git a/loader/src/ui/mods/settings/ModSettingsPopup.cpp b/loader/src/ui/mods/settings/ModSettingsPopup.cpp index fdbaf22f6..dc0a8a504 100644 --- a/loader/src/ui/mods/settings/ModSettingsPopup.cpp +++ b/loader/src/ui/mods/settings/ModSettingsPopup.cpp @@ -65,7 +65,16 @@ bool ModSettingsPopup::init(Mod* mod, bool forceDisableTheme) { void ModSettingsPopup::updateState(SettingNode* invoker) { BaseSettingsPopup::updateState(invoker); - m_restartBtn->setVisible(ModSettingsManager::from(m_mod)->restartRequired()); + + auto manager = ModSettingsManager::from(m_mod); + m_restartBtn->setVisible(manager->restartRequired()); + + // frame delay for debounce (avoids repeating save for every changed setting) + Loader::get()->queueInMainThread([mod = m_mod, manager] { + if (manager->shouldSave()) { + (void) mod->saveData(); + } + }); } void ModSettingsPopup::onOpenSaveDirectory(CCObject*) {