From 1542e299e3ccd60026c5d0a80115be1a4e8e31c1 Mon Sep 17 00:00:00 2001 From: HJfod <60038575+HJfod@users.noreply.github.com> Date: Tue, 21 Jan 2025 23:42:05 +0200 Subject: [PATCH] separate enable-if messages from evaluation to fix negation --- loader/src/loader/SettingV3.cpp | 142 ++++++++++++++++++++------------ 1 file changed, 90 insertions(+), 52 deletions(-) diff --git a/loader/src/loader/SettingV3.cpp b/loader/src/loader/SettingV3.cpp index 72dbf3494..00ef688b5 100644 --- a/loader/src/loader/SettingV3.cpp +++ b/loader/src/loader/SettingV3.cpp @@ -14,26 +14,34 @@ using namespace geode::prelude; namespace enable_if_parsing { struct Component { virtual ~Component() = default; - virtual Result<> check() const = 0; - virtual Result<> eval(std::string const& defaultModID) const = 0; + virtual Result<> checkSemantics() const = 0; + virtual bool shouldEnableSetting(std::string const& defaultModID) const = 0; + virtual std::string shouldEnableReason(std::string const& defaultModID) const = 0; }; struct RequireModLoaded final : public Component { std::string modID; RequireModLoaded(std::string const& modID) : modID(modID) {} - Result<> check() const override { + Result<> checkSemantics() const override { return Ok(); } - Result<> eval(std::string const& defaultModID) const override { + bool shouldEnableSetting(std::string const& defaultModID) const override { if (Loader::get()->getLoadedMod(modID)) { - return Ok(); + return true; } auto modName = modID; if (auto mod = Loader::get()->getInstalledMod(modID)) { modName = mod->getName(); } - return Err("Enable the mod {}", modName); + return false; + } + std::string shouldEnableReason(std::string const&) const override { + auto modName = modID; + if (auto mod = Loader::get()->getInstalledMod(modID)) { + modName = mod->getName(); + } + return fmt::format("Enable the mod {}", modName); } }; struct RequireSettingEnabled final : public Component { @@ -42,7 +50,7 @@ namespace enable_if_parsing { RequireSettingEnabled(std::string const& modID, std::string const& settingID) : modID(modID), settingID(settingID) {} - Result<> check() const override { + Result<> checkSemantics() const override { if (auto mod = Loader::get()->getInstalledMod(modID)) { if (!mod->hasSetting(settingID)) { return Err("Mod '{}' does not have setting '{}'", mod->getName(), settingID); @@ -53,27 +61,31 @@ namespace enable_if_parsing { } return Ok(); } - Result<> eval(std::string const& defaultModID) const override { + bool shouldEnableSetting(std::string const& defaultModID) const override { if (auto mod = Loader::get()->getLoadedMod(modID)) { if (mod->getSettingValue(settingID)) { - return Ok(); + return true; } - // This is an if-check just in case, even though check() should already - // make sure that getSettingV3 is guaranteed to return true + return false; + } + return false; + } + std::string shouldEnableReason(std::string const& defaultModID) const override { + if (auto mod = Loader::get()->getLoadedMod(modID)) { auto name = settingID; if (auto sett = mod->getSetting(settingID)) { name = sett->getDisplayName(); } if (modID == defaultModID) { - return Err("Enable the setting '{}'", name); + return fmt::format("Enable the setting '{}'", name); } - return Err("Enable the setting '{}' from the mod {}", name, mod->getName()); + return fmt::format("Enable the setting '{}' from the mod {}", name, mod->getName()); } auto modName = modID; if (auto mod = Loader::get()->getInstalledMod(modID)) { modName = mod->getName(); } - return Err("Enable the mod {}", modName); + return fmt::format("Enable the mod {}", modName); } }; struct RequireSavedValueEnabled final : public Component { @@ -82,24 +94,34 @@ namespace enable_if_parsing { RequireSavedValueEnabled(std::string const& modID, std::string const& savedValue) : modID(modID), savedValue(savedValue) {} - Result<> check() const override { + Result<> checkSemantics() const override { return Ok(); } - Result<> eval(std::string const& defaultModID) const override { + bool shouldEnableSetting(std::string const& defaultModID) const override { if (auto mod = Loader::get()->getLoadedMod(modID)) { if (mod->getSavedValue(savedValue)) { - return Ok(); + return true; } + return false; + } + auto modName = modID; + if (auto mod = Loader::get()->getInstalledMod(modID)) { + modName = mod->getName(); + } + return false; + } + std::string shouldEnableReason(std::string const& defaultModID) const override { + if (auto mod = Loader::get()->getLoadedMod(modID)) { if (modID == defaultModID) { - return Err("Enable the value '{}'", savedValue); + return fmt::format("Enable the value '{}'", savedValue); } - return Err("Enable the value '{}' from the mod {}", savedValue, mod->getName()); + return fmt::format("Enable the value '{}' from the mod {}", savedValue, mod->getName()); } auto modName = modID; if (auto mod = Loader::get()->getInstalledMod(modID)) { modName = mod->getName(); } - return Err("Enable the mod {}", modName); + return fmt::format("Enable the mod {}", modName); } }; struct RequireNot final : public Component { @@ -107,19 +129,19 @@ namespace enable_if_parsing { RequireNot(std::unique_ptr&& component) : component(std::move(component)) {} - Result<> check() const override { - return component->check(); + Result<> checkSemantics() const override { + return component->checkSemantics(); } - Result<> eval(std::string const& defaultModID) const override { - if (auto res = component->eval(defaultModID)) { - // Surely this will never break! - auto str = res.unwrapErr(); - string::replaceIP(str, "Enable", "___TEMP"); - string::replaceIP(str, "Disable", "Enable"); - string::replaceIP(str, "___TEMP", "Disable"); - return Err(str); - } - return Ok(); + bool shouldEnableSetting(std::string const& defaultModID) const override { + return !component->shouldEnableSetting(defaultModID); + } + std::string shouldEnableReason(std::string const& defaultModID) const override { + // Surely this will never break! + auto str = component->shouldEnableReason(defaultModID); + string::replaceIP(str, "Enable", "___TEMP"); + string::replaceIP(str, "Disable", "Enable"); + string::replaceIP(str, "___TEMP", "Disable"); + return str; } }; struct RequireAll final : public Component { @@ -127,19 +149,31 @@ namespace enable_if_parsing { RequireAll(std::vector>&& components) : components(std::move(components)) {} - Result<> check() const override { + Result<> checkSemantics() const override { for (auto& comp : components) { - GEODE_UNWRAP(comp->check()); + GEODE_UNWRAP(comp->checkSemantics()); } return Ok(); } - Result<> eval(std::string const& defaultModID) const override { + bool shouldEnableSetting(std::string const& defaultModID) const override { // Only print out whatever the first erroring condition is to not shit out // "Please enable X and Y and Z and Ö and Å and" for (auto& comp : components) { - GEODE_UNWRAP(comp->eval(defaultModID)); + if (!comp->shouldEnableSetting(defaultModID)) { + return false; + } } - return Ok(); + return true; + } + std::string shouldEnableReason(std::string const& defaultModID) const override { + for (auto& comp : components) { + // Yes this does require double evaluation which is cringe + // shouldEnableSetting() is guaranteed to be side-effect-free though + if (!comp->shouldEnableSetting(defaultModID)) { + return comp->shouldEnableReason(defaultModID); + } + } + return "If you see this, Geode is broken"; } }; struct RequireSome final : public Component { @@ -147,28 +181,36 @@ namespace enable_if_parsing { RequireSome(std::vector>&& components) : components(std::move(components)) {} - Result<> check() const override { + Result<> checkSemantics() const override { for (auto& comp : components) { - GEODE_UNWRAP(comp->check()); + GEODE_UNWRAP(comp->checkSemantics()); } return Ok(); } - Result<> eval(std::string const& defaultModID) const override { + bool shouldEnableSetting(std::string const& defaultModID) const override { + for (auto& comp : components) { + if (comp->shouldEnableSetting(defaultModID)) { + return true; + } + } + return components.empty(); + } + std::string shouldEnableReason(std::string const& defaultModID) const override { std::optional err; for (auto& comp : components) { - auto res = comp->eval(defaultModID); + auto res = comp->shouldEnableSetting(defaultModID); if (res) { - return Ok(); + return "If you see this, Geode is broken"; } // Only show first condition that isn't met if (!err.has_value()) { - err = res.unwrapErr(); + err = comp->shouldEnableReason(defaultModID); } } if (err.has_value()) { - return Err(*err); + return *err; } - return Ok(); + return "If you see this, Geode is broken"; } }; @@ -505,7 +547,7 @@ void SettingV3::parseEnableIf(JsonExpectedValue& json) { json.has("enable-if") .mustBe("a valid \"enable-if\" scheme", [this](std::string const& str) -> Result<> { GEODE_UNWRAP_INTO(auto tree, enable_if_parsing::Parser::parse(str, m_impl->modID)); - GEODE_UNWRAP(tree->check()); + GEODE_UNWRAP(tree->checkSemantics()); m_impl->enableIfTree = std::move(tree); return Ok(); }) @@ -548,7 +590,7 @@ std::optional SettingV3::getEnableIf() const { } bool SettingV3::shouldEnable() const { if (m_impl->enableIfTree) { - return m_impl->enableIfTree->eval(m_impl->modID).isOk(); + return m_impl->enableIfTree->shouldEnableSetting(m_impl->modID); } return true; } @@ -559,11 +601,7 @@ std::optional SettingV3::getEnableIfDescription() const { if (!m_impl->enableIfTree) { return std::nullopt; } - auto res = m_impl->enableIfTree->eval(m_impl->modID); - if (res) { - return std::nullopt; - } - return res.unwrapErr(); + return m_impl->enableIfTree->shouldEnableReason(m_impl->modID); } bool SettingV3::requiresRestart() const { return m_impl->requiresRestart;