From 271f6dc602c81084d54ad7422aef991115b4b77b Mon Sep 17 00:00:00 2001 From: codepuncher Date: Tue, 16 Jun 2026 20:48:30 +0100 Subject: [PATCH 1/4] fix(menu-ui): wrap RenderSettings in try/catch to prevent UB across DLL boundary --- src/MenuUI.cpp | 68 ++++++++++++++++++++++++++------------------------ 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/src/MenuUI.cpp b/src/MenuUI.cpp index 551d19c..89194ef 100644 --- a/src/MenuUI.cpp +++ b/src/MenuUI.cpp @@ -175,40 +175,44 @@ namespace void __stdcall RenderSettings() { - auto& state = GetMenuState(); - bool changed = false; - changed |= ImGuiMCP::SliderFloat("Hold duration", &state.stagedSettings.holdDuration, InputHandler::kMinHoldDuration, InputHandler::kMaxHoldDuration, "%.2fs"); - changed |= DrawActionCombo("Start long-press action", state.stagedSettings.startAction); - if (state.stagedSettings.startAction == InputHandler::LongPressAction::kMCM) { - DrawMCMTargetInputs( - "Start MCM mod name", - state.stagedSettings.startMCMModName, - changed); - changed |= ImGuiMCP::Checkbox("Close journal after leaving MCM mod page##start", &state.stagedSettings.startMCMQuickexit); - } - changed |= DrawActionCombo("Back long-press action", state.stagedSettings.backAction); - if (state.stagedSettings.backAction == InputHandler::LongPressAction::kMCM) { - DrawMCMTargetInputs( - "Back MCM mod name", - state.stagedSettings.backMCMModName, - changed); - changed |= ImGuiMCP::Checkbox("Close journal after leaving MCM mod page##back", &state.stagedSettings.backMCMQuickexit); - } + try { + auto& state = GetMenuState(); + bool changed = false; + changed |= ImGuiMCP::SliderFloat("Hold duration", &state.stagedSettings.holdDuration, InputHandler::kMinHoldDuration, InputHandler::kMaxHoldDuration, "%.2fs"); + changed |= DrawActionCombo("Start long-press action", state.stagedSettings.startAction); + if (state.stagedSettings.startAction == InputHandler::LongPressAction::kMCM) { + DrawMCMTargetInputs( + "Start MCM mod name", + state.stagedSettings.startMCMModName, + changed); + changed |= ImGuiMCP::Checkbox("Close journal after leaving MCM mod page##start", &state.stagedSettings.startMCMQuickexit); + } + changed |= DrawActionCombo("Back long-press action", state.stagedSettings.backAction); + if (state.stagedSettings.backAction == InputHandler::LongPressAction::kMCM) { + DrawMCMTargetInputs( + "Back MCM mod name", + state.stagedSettings.backMCMModName, + changed); + changed |= ImGuiMCP::Checkbox("Close journal after leaving MCM mod page##back", &state.stagedSettings.backMCMQuickexit); + } - if (changed) { - state.hasPendingChanges = true; - } + if (changed) { + state.hasPendingChanges = true; + } - if (ImGuiMCP::Button("Save to config")) { - SavePendingChanges(); - } - ImGuiMCP::SameLine(); - if (ImGuiMCP::Button("Reload from config")) { - ReloadFromConfig(true); - } - ImGuiMCP::SameLine(); - if (ImGuiMCP::Button("Reset to defaults")) { - ResetToDefaults(); + if (ImGuiMCP::Button("Save to config")) { + SavePendingChanges(); + } + ImGuiMCP::SameLine(); + if (ImGuiMCP::Button("Reload from config")) { + ReloadFromConfig(true); + } + ImGuiMCP::SameLine(); + if (ImGuiMCP::Button("Reset to defaults")) { + ResetToDefaults(); + } + } catch (...) { + logger::error("RenderSettings: unhandled exception — skipping UI frame"); } } From 404f8d6af407eb7c7b599552ce252edb8bef7cb3 Mon Sep 17 00:00:00 2001 From: codepuncher Date: Tue, 16 Jun 2026 20:48:46 +0100 Subject: [PATCH 2/4] fix(input): add break after keyCode match in ScanInputEvents --- src/InputHandler.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/InputHandler.cpp b/src/InputHandler.cpp index f9bc5b9..e80d850 100644 --- a/src/InputHandler.cpp +++ b/src/InputHandler.cpp @@ -181,6 +181,7 @@ bool InputHandler::ScanInputEvents(RE::InputEvent* const* a_events) if (ProcessButton(btn, bs)) { shouldBlock = true; } + break; } } From c3c95fe50bea7b53620a34b89efc20250e017b2d Mon Sep 17 00:00:00 2001 From: codepuncher Date: Tue, 16 Jun 2026 20:50:10 +0100 Subject: [PATCH 3/4] fix(mcm): check Invoke return values and guard GetElement with IsObject --- src/MCMNavigator.cpp | 191 ++++++++++++++++++++++--------------------- 1 file changed, 98 insertions(+), 93 deletions(-) diff --git a/src/MCMNavigator.cpp b/src/MCMNavigator.cpp index abf834b..73f9208 100644 --- a/src/MCMNavigator.cpp +++ b/src/MCMNavigator.cpp @@ -128,7 +128,9 @@ namespace MCMNavigator names.reserve(length); for (std::uint32_t i = 0; i < length; i++) { RE::GFxValue entry; - entryList.GetElement(i, &entry); + if (!entryList.GetElement(i, &entry) || !entry.IsObject()) { + continue; + } RE::GFxValue nameVal; entry.GetMember(memberName, &nameVal); if (nameVal.IsString()) { @@ -167,7 +169,9 @@ namespace MCMNavigator int index = -1; for (std::uint32_t i = 0; i < length; i++) { RE::GFxValue entry; - entryList.GetElement(i, &entry); + if (!entryList.GetElement(i, &entry) || !entry.IsObject()) { + continue; + } RE::GFxValue nameVal; entry.GetMember(varName, &nameVal); if (!nameVal.IsString()) { @@ -185,8 +189,13 @@ namespace MCMNavigator } std::array args{ static_cast(index), 0.0 }; - listObj.Invoke("doSetSelectedIndex", nullptr, args.data(), 2); - listObj.Invoke("onItemPress", nullptr, args.data(), 2); + const bool selectedOk = listObj.Invoke("doSetSelectedIndex", nullptr, args.data(), 2); + const bool pressOk = listObj.Invoke("onItemPress", nullptr, args.data(), 2); + if (!selectedOk || !pressOk) { + logger::warn("MCMNavigator: Invoke failed on '{}' (doSetSelectedIndex={}, onItemPress={})", + targetName, selectedOk, pressOk); + return false; + } logger::debug("MCMNavigator: selected '{}' at index {} in {}", targetName, index, listPath); return true; } @@ -252,6 +261,90 @@ namespace MCMNavigator } g_lock = false; } + + bool IsModAlreadyOpen(std::string_view modName) + { + if (!IsAnyModOpen()) { + return false; + } + auto* view = GetJournalView(); + if (!view) { + return false; + } + RE::GFxValue titleText; + const std::string titlePath = std::string{ kModListPanel } + "_titleText"; + view->GetVariable(&titleText, titlePath.c_str()); + return titleText.IsString() && HoldFast::CaseInsensitiveEqual(modName, titleText.GetString()); + } + + void CacheModListFromGFx() + { + auto* view = GetJournalView(); + if (!view || !IsMCMOpen()) { + return; + } + auto names = CollectEntryNames(view, std::string{ kModList }, "text"); + std::ranges::sort(names, CaseInsensitiveLess); + if (names.empty()) { + return; + } + std::scoped_lock lock(g_cacheMutex); + g_modCache = std::move(names); + } + + void NavigateToTargetImpl(const std::string& modName) + { + if (modName.empty() || modName == "None") { + return; + } + + if (g_lock.exchange(true)) { + logger::debug("MCMNavigator: navigation already in flight — skipping"); + return; + } + + if (IsAnyModOpen() && !IsModAlreadyOpen(modName)) { + logger::debug("MCMNavigator: a different mod is open — transitioning to mod list"); + TransitionToModList(); + } + + g_target.modName = modName; + g_target.deadline = std::chrono::steady_clock::now() + kNavTimeout; + + if (IsModAlreadyOpen(modName)) { + g_lock = false; + } else if (!DelayCallForUI(OpenMod, kModRetryFrames)) { + logger::warn("MCMNavigator: task interface unavailable — navigation cancelled"); + g_lock = false; + } + } + + void ReadModArraysIntoCache(const RE::BSTSmartPointer& namesArr) + { + std::vector modNames; + modNames.reserve(namesArr->size()); + + for (RE::BSScript::Array::size_type i = 0; i < namesArr->size(); ++i) { + const auto& nameElem = (*namesArr)[i]; + if (!nameElem.IsString()) { + continue; + } + const auto modName = nameElem.GetString(); + if (modName.empty()) { + continue; + } + modNames.emplace_back(StripModNamePrefix(modName)); + } + + if (modNames.empty()) { + return; + } + + std::ranges::sort(modNames, CaseInsensitiveLess); + std::scoped_lock lock(g_cacheMutex); + g_modCache = std::move(modNames); + logger::info("MCMNavigator: cached {} mods from SKI_ConfigManager", g_modCache.size()); + } } bool IsMCMOpen() @@ -278,36 +371,6 @@ namespace MCMNavigator return state.IsNumber() && state.GetNumber() == 2.0; } - bool IsModAlreadyOpen(std::string_view modName) - { - if (!IsAnyModOpen()) { - return false; - } - auto* view = GetJournalView(); - if (!view) { - return false; - } - RE::GFxValue titleText; - const std::string titlePath = std::string{ kModListPanel } + "_titleText"; - view->GetVariable(&titleText, titlePath.c_str()); - return titleText.IsString() && HoldFast::CaseInsensitiveEqual(modName, titleText.GetString()); - } - - void CacheModListFromGFx() - { - auto* view = GetJournalView(); - if (!view || !IsMCMOpen()) { - return; - } - auto names = CollectEntryNames(view, std::string{ kModList }, "text"); - std::ranges::sort(names, CaseInsensitiveLess); - if (names.empty()) { - return; - } - std::scoped_lock lock(g_cacheMutex); - g_modCache = std::move(names); - } - void TryCacheFromOpenMCM() { if (!IsMCMOpen()) { @@ -352,33 +415,6 @@ namespace MCMNavigator return g_modCache; } - void NavigateToTargetImpl(const std::string& modName) - { - if (modName.empty() || modName == "None") { - return; - } - - if (g_lock.exchange(true)) { - logger::debug("MCMNavigator: navigation already in flight — skipping"); - return; - } - - if (IsAnyModOpen() && !IsModAlreadyOpen(modName)) { - logger::debug("MCMNavigator: a different mod is open — transitioning to mod list"); - TransitionToModList(); - } - - g_target.modName = modName; - g_target.deadline = std::chrono::steady_clock::now() + kNavTimeout; - - if (IsModAlreadyOpen(modName)) { - g_lock = false; - } else if (!DelayCallForUI(OpenMod, kModRetryFrames)) { - logger::warn("MCMNavigator: task interface unavailable — navigation cancelled"); - g_lock = false; - } - } - void NavigateToTarget(const std::string& modName) noexcept { try { @@ -388,42 +424,11 @@ namespace MCMNavigator } } - void ReadModArraysIntoCache(const RE::BSTSmartPointer& namesArr) - { - std::vector modNames; - modNames.reserve(namesArr->size()); - - for (RE::BSScript::Array::size_type i = 0; i < namesArr->size(); ++i) { - const auto& nameElem = (*namesArr)[i]; - if (!nameElem.IsString()) { - continue; - } - const auto modName = nameElem.GetString(); - if (modName.empty()) { - continue; - } - modNames.emplace_back(StripModNamePrefix(modName)); - } - - if (modNames.empty()) { - return; - } - - std::ranges::sort(modNames, CaseInsensitiveLess); - std::scoped_lock lock(g_cacheMutex); - g_modCache = std::move(modNames); - logger::info("MCMNavigator: cached {} mods from SKI_ConfigManager", g_modCache.size()); - } - void CacheModListFromPapyrus() { + // NOLINTNEXTLINE(cppcoreguidelines-special-member-functions) const struct ClearPapyrusPending { - ClearPapyrusPending() = default; - ClearPapyrusPending(const ClearPapyrusPending&) = default; - ClearPapyrusPending(ClearPapyrusPending&&) = default; - ClearPapyrusPending& operator=(const ClearPapyrusPending&) = default; - ClearPapyrusPending& operator=(ClearPapyrusPending&&) = default; ~ClearPapyrusPending() { g_papyrusPending = false; } } clearPapyrusPending; From d196b55034d4e58777e68bba2aca49be2f02286c Mon Sep 17 00:00:00 2001 From: codepuncher Date: Tue, 16 Jun 2026 20:50:54 +0100 Subject: [PATCH 4/4] fix(config): add missing include, kCount assert, SaveSettings error handling, MCM mod name validation --- src/Config.cpp | 14 +++++++++++++- src/Config.h | 3 +++ src/LongPressAction.h | 1 + 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/Config.cpp b/src/Config.cpp index fce8934..a054b85 100644 --- a/src/Config.cpp +++ b/src/Config.cpp @@ -99,6 +99,15 @@ HoldFast::Config::Settings HoldFast::Config::LoadSettings() settings.startMCMQuickexit = ini.GetBoolValue("General", "bButtonStartMCMQuickexit", true); settings.backMCMQuickexit = ini.GetBoolValue("General", "bButtonBackMCMQuickexit", true); + if (settings.startAction == LongPressAction::kMCM && + HoldFast::CaseInsensitiveEqual(settings.startMCMModName, "None")) { + 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")) { + logger::warn("sButtonBackAction=MCM but sButtonBackMCMModName is not set — Back button will open MCM without navigating to a specific mod"); + } + return settings; } @@ -106,7 +115,10 @@ bool HoldFast::Config::SaveSettings(const Settings& settings) { CSimpleIniA ini; ini.SetSpaces(false); - ini.LoadFile(kIniPath); + const auto loadRc = ini.LoadFile(kIniPath); + if (loadRc < SI_OK && loadRc != SI_FILE) { + logger::warn("SaveSettings: failed to parse existing HoldFast.ini (rc={}) — existing content may be lost", static_cast(loadRc)); + } const auto startActionName = std::string{ ActionName(settings.startAction) }; const auto backActionName = std::string{ ActionName(settings.backAction) }; diff --git a/src/Config.h b/src/Config.h index d582fe1..29dd76d 100644 --- a/src/Config.h +++ b/src/Config.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -44,6 +45,8 @@ namespace HoldFast::Config { "MCM", LongPressAction::kMCM }, { "None", LongPressAction::kNone }, } }; + static_assert(kActionOptions.size() == static_cast(LongPressAction::kCount), + "kActionOptions is out of sync with LongPressAction enum — add the missing entry"); [[nodiscard]] Settings LoadSettings(); [[nodiscard]] bool SaveSettings(const Settings& settings); diff --git a/src/LongPressAction.h b/src/LongPressAction.h index 6dcf53f..5a77230 100644 --- a/src/LongPressAction.h +++ b/src/LongPressAction.h @@ -27,6 +27,7 @@ enum class LongPressAction kBestiary, kCharacterSheet, kMCM, + kCount, }; struct ButtonConfig