fix: internal stability and correctness fixes#51
Merged
Conversation
…andling, MCM mod name validation
There was a problem hiding this comment.
Pull request overview
This PR applies a set of internal correctness/safety hardening changes identified during an audit, primarily around Scaleform/GFx interactions, input scanning, and configuration integrity checks.
Changes:
- Prevent exceptions from escaping across the
__stdcallMenu Framework callback boundary; harden GFx list traversal and invocation success handling in MCM navigation. - Fix input scanning to stop after the first matching
keyCode(since the configured codes are unique in current usage). - Tighten configuration correctness with direct includes, an enum sentinel + compile-time sync check, and additional load/save-time warnings.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/MenuUI.cpp | Wraps RenderSettings in try/catch (...) to avoid exceptions crossing the DLL callback boundary. |
| src/MCMNavigator.cpp | Adds safer GFx element/object checks, verifies Invoke success, and scopes internal helpers to the TU. |
| src/InputHandler.cpp | Stops scanning configured buttons after the first keyCode match. |
| src/LongPressAction.h | Adds kCount sentinel to support compile-time sync checks. |
| src/Config.h | Adds <string> include and a static_assert tying kActionOptions to the enum size. |
| src/Config.cpp | Adds warnings for inconsistent MCM config and warns when saving over an unparsable INI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
00cdcb8 to
d196b55
Compare
This was referenced Jun 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
MenuUI — wrap
RenderSettingsbody intry/catch (...)to prevent undefined behaviour from exceptions crossing the__stdcallDLL boundary into SKSEMenuFramework.InputHandler — add
breakafter the first keyCode match inScanInputEvents; keyCodes are unique so continuing the loop served no purpose.MCMNavigator
Invokereturn values inSelectEntryByName; warn and returnfalseif either fails instead of silently succeeding.GetElementcalls with!entry.IsObject()in bothSelectEntryByNameandCollectEntryNames.IsModAlreadyOpen,CacheModListFromGFx,NavigateToTargetImpl,ReadModArraysIntoCache) into the anonymous namespace; they were previously atMCMNavigator::scope but undeclared in the header.ClearPapyrusPendingscope-guard to destructor-only (withNOLINTNEXTLINEfor the special-member-functions check).Config
#include <string>directly toConfig.h(was relying on transitive inclusion).kCountsentinel toLongPressActionenum and astatic_assertverifyingkActionOptionsstays in sync.LoadFilereturn code inSaveSettings; warn if the existing INI could not be parsed.sButtonStartAction=MCMorsButtonBackAction=MCMbut the corresponding mod name is not set.