diff --git a/src/Config.cpp b/src/Config.cpp index a054b85..9ecbb12 100644 --- a/src/Config.cpp +++ b/src/Config.cpp @@ -18,18 +18,11 @@ namespace return duration; } if (!std::isfinite(raw)) { - logger::warn("fHoldDuration is non-finite — using default {:.1f}", HoldFast::kDefaultHoldDuration); + logger::warn("fHoldDuration is non-finite — using default {:.1f}", duration); return duration; } - if (raw <= 0.0F) { - logger::warn("fHoldDuration ({:.2f}) must be positive — using default {:.1f}", raw, HoldFast::kDefaultHoldDuration); - return duration; - } - if (raw < HoldFast::kMinHoldDuration) { - logger::warn("fHoldDuration ({:.2f}) is below minimum {:.1f} — using default {:.1f}", raw, HoldFast::kMinHoldDuration, HoldFast::kDefaultHoldDuration); - return duration; - } - logger::warn("fHoldDuration ({:.2f}) exceeds maximum {:.1f} — capping", raw, HoldFast::kMaxHoldDuration); + logger::warn("fHoldDuration ({:.2f}) out of range [{:.1f}, {:.1f}] — using {:.1f}", + raw, HoldFast::kMinHoldDuration, HoldFast::kMaxHoldDuration, duration); return duration; } @@ -73,25 +66,22 @@ HoldFast::Config::Settings HoldFast::Config::LoadSettings() constexpr auto kValidActions = "Map, System, Quests, Stats, Inventory, Magic, Favorites/Favourites, TweenMenu, Wait, NewSave, QuickSave, Bestiary, CharacterSheet, MCM, None"; - settings.startAction = hasStart ? ParseAction(rawStart) : LongPressAction::kNone; - if (hasStart && settings.startAction == LongPressAction::kNone) { - std::string lower{ HoldFast::TrimWhitespace(rawStart) }; - for (auto& c : lower) { - c = static_cast(std::tolower(static_cast(c))); + const auto warnIfUnrecognised = [kValidActions](const char* raw, const char* key, LongPressAction action) { + if (action != LongPressAction::kNone) { + return; } - if (lower != "none") { - logger::warn("sButtonStartAction='{}' is not a recognised action (valid: {}) — disabling button", rawStart, kValidActions); + if (!HoldFast::CaseInsensitiveEqual(HoldFast::TrimWhitespace(raw), HoldFast::kNoneName)) { + logger::warn("{}='{}' is not a recognised action (valid: {}) — disabling button", key, raw, kValidActions); } - } + }; + + settings.startAction = hasStart ? ParseAction(rawStart) : LongPressAction::kNone; settings.backAction = hasBack ? ParseAction(rawBack) : LongPressAction::kNone; - if (hasBack && settings.backAction == LongPressAction::kNone) { - std::string lower{ HoldFast::TrimWhitespace(rawBack) }; - for (auto& c : lower) { - c = static_cast(std::tolower(static_cast(c))); - } - if (lower != "none") { - logger::warn("sButtonBackAction='{}' is not a recognised action (valid: {}) — disabling button", rawBack, kValidActions); - } + if (hasStart) { + warnIfUnrecognised(rawStart, "sButtonStartAction", settings.startAction); + } + if (hasBack) { + warnIfUnrecognised(rawBack, "sButtonBackAction", settings.backAction); } settings.startMCMModName = GetMCMTarget(ini, "sButtonStartMCMModName"); @@ -100,11 +90,11 @@ HoldFast::Config::Settings HoldFast::Config::LoadSettings() settings.backMCMQuickexit = ini.GetBoolValue("General", "bButtonBackMCMQuickexit", true); if (settings.startAction == LongPressAction::kMCM && - HoldFast::CaseInsensitiveEqual(settings.startMCMModName, "None")) { + HoldFast::CaseInsensitiveEqual(settings.startMCMModName, HoldFast::kNoneName)) { logger::warn("sButtonStartAction=MCM but sButtonStartMCMModName is not set — Start button will open MCM without navigating to a specific mod"); } if (settings.backAction == LongPressAction::kMCM && - HoldFast::CaseInsensitiveEqual(settings.backMCMModName, "None")) { + HoldFast::CaseInsensitiveEqual(settings.backMCMModName, HoldFast::kNoneName)) { logger::warn("sButtonBackAction=MCM but sButtonBackMCMModName is not set — Back button will open MCM without navigating to a specific mod"); } diff --git a/src/InputHandler.cpp b/src/InputHandler.cpp index e80d850..35eebcc 100644 --- a/src/InputHandler.cpp +++ b/src/InputHandler.cpp @@ -527,7 +527,7 @@ void InputHandler::InvokeScaleformTab(JournalTab tab) return; } - if (_pendingMCMModName.empty() || _pendingMCMModName == "None") { + if (_pendingMCMModName.empty() || _pendingMCMModName == HoldFast::kNoneName) { return; } const auto* taskIface = SKSE::GetTaskInterface(); diff --git a/src/LongPressAction.h b/src/LongPressAction.h index 5a77230..cefc567 100644 --- a/src/LongPressAction.h +++ b/src/LongPressAction.h @@ -2,12 +2,14 @@ #include #include +#include namespace HoldFast { - inline constexpr float kMinHoldDuration = 0.1F; - inline constexpr float kDefaultHoldDuration = 0.5F; - inline constexpr float kMaxHoldDuration = 5.0F; + inline constexpr float kMinHoldDuration = 0.1F; + inline constexpr float kDefaultHoldDuration = 0.5F; + inline constexpr float kMaxHoldDuration = 5.0F; + inline constexpr std::string_view kNoneName = "None"; } enum class LongPressAction diff --git a/src/MCMNavigator.cpp b/src/MCMNavigator.cpp index 73f9208..fc957ed 100644 --- a/src/MCMNavigator.cpp +++ b/src/MCMNavigator.cpp @@ -1,5 +1,6 @@ #include "PCH.h" +#include "LongPressAction.h" #include "MCMNavigator.h" #include "Utils.h" @@ -22,7 +23,7 @@ namespace MCMNavigator { return std::ranges::lexicographical_compare( a, b, - [](unsigned char x, unsigned char y) { return std::tolower(x) < std::tolower(y); }); + [](unsigned char x, unsigned char y) { return HoldFast::AsciiToLower(x) < HoldFast::AsciiToLower(y); }); } std::string_view StripModNamePrefix(std::string_view name) @@ -294,7 +295,7 @@ namespace MCMNavigator void NavigateToTargetImpl(const std::string& modName) { - if (modName.empty() || modName == "None") { + if (modName.empty() || modName == HoldFast::kNoneName) { return; } diff --git a/src/MenuUI.cpp b/src/MenuUI.cpp index 89194ef..30c1ab1 100644 --- a/src/MenuUI.cpp +++ b/src/MenuUI.cpp @@ -142,7 +142,7 @@ namespace { MCMNavigator::EnsureCachePopulated(); - const char* preview = modName.empty() || modName == "None" ? "None" : modName.c_str(); + const char* preview = modName.empty() || modName == HoldFast::kNoneName ? "None" : modName.c_str(); if (!ImGuiMCP::BeginCombo(modLabel, preview)) { return; @@ -150,9 +150,9 @@ namespace const auto cachedMods = MCMNavigator::GetCachedModNames(); - const bool noneSelected = modName.empty() || modName == "None"; + const bool noneSelected = modName.empty() || modName == HoldFast::kNoneName; if (ImGuiMCP::Selectable("None", noneSelected)) { - modName = "None"; + modName = HoldFast::kNoneName; changed = true; } diff --git a/src/Utils.h b/src/Utils.h index 6bf45fb..d306df2 100644 --- a/src/Utils.h +++ b/src/Utils.h @@ -2,7 +2,6 @@ #include #include -#include #include #include @@ -18,11 +17,16 @@ namespace HoldFast return s.substr(first, s.find_last_not_of(" \t\r\n") - first + 1); } + [[nodiscard]] inline constexpr unsigned char AsciiToLower(unsigned char c) noexcept + { + return (c >= 'A' && c <= 'Z') ? static_cast(c + ('a' - 'A')) : c; + } + [[nodiscard]] inline bool CaseInsensitiveEqual(std::string_view a, std::string_view b) { return std::ranges::equal( a, b, - [](unsigned char x, unsigned char y) { return std::tolower(x) == std::tolower(y); }); + [](unsigned char x, unsigned char y) { return AsciiToLower(x) == AsciiToLower(y); }); } [[nodiscard]] inline float ClampHoldDuration(float value, float defaultVal, float minVal, float maxVal) diff --git a/test/PluginTests.cpp b/test/PluginTests.cpp index 8475e56..d881681 100644 --- a/test/PluginTests.cpp +++ b/test/PluginTests.cpp @@ -9,6 +9,34 @@ using HoldFast::TrimWhitespace; using HoldFast::Config::ActionName; using HoldFast::Config::ParseAction; +TEST_CASE("AsciiToLower lowercases A-Z only", "[utils]") +{ + using HoldFast::AsciiToLower; + + CHECK(AsciiToLower('A') == 'a'); + CHECK(AsciiToLower('Z') == 'z'); + CHECK(AsciiToLower('M') == 'm'); + CHECK(AsciiToLower('a') == 'a'); + CHECK(AsciiToLower('z') == 'z'); + CHECK(AsciiToLower('0') == '0'); + CHECK(AsciiToLower('!') == '!'); + CHECK(AsciiToLower(static_cast(0xFF)) == static_cast(0xFF)); +} + +TEST_CASE("CaseInsensitiveEqual matches ASCII case-insensitively", "[utils]") +{ + using HoldFast::CaseInsensitiveEqual; + + CHECK(CaseInsensitiveEqual("None", "none")); + CHECK(CaseInsensitiveEqual("NONE", "None")); + CHECK(CaseInsensitiveEqual("MCM", "mcm")); + CHECK(CaseInsensitiveEqual("Map", "MAP")); + CHECK(CaseInsensitiveEqual("", "")); + CHECK_FALSE(CaseInsensitiveEqual("Map", "Mcm")); + CHECK_FALSE(CaseInsensitiveEqual("None", "")); + CHECK_FALSE(CaseInsensitiveEqual("abc", "abcd")); +} + TEST_CASE("TrimWhitespace removes leading and trailing whitespace", "[utils]") { CHECK(TrimWhitespace(" hello ") == "hello");