Fix Vortex/DLC compatibility gaps found via IDCs/WitcherScriptMerger comparison - #11
Merged
Merged
Conversation
…Cs/WitcherScriptMerger A different fork (github.com/IDCs/WitcherScriptMerger, the fork Vortex's real game-witcher3 extension actually downloads and drives) fixed two real gaps we'd inherited unmodified from upstream: 1. CustomLoadOrder.ProcessLine had no tolerance for "VK=" (VortexKey) lines that Vortex writes into mods.settings - a VK= line hit the catch-all "unrecognized value" branch and aborted parsing the entire file (IsValid stays false, no load order usable). Now recognized and ignored, like a comment line. 2. FileMerger's vanilla-bundle DLC-folder filter had no "bob" (Blood & Wine's internal folder codename) alternative, so conflicts inside B&W bundle content went undetected, and was case-sensitive - only ever worked by luck of Windows' case-insensitive filesystem. Now matches "bob" too, case-insensitively; more relevant for WitcherScriptMerger.Headless running on case-sensitive Linux filesystems. Extracted the DLC-folder match into FileMerger.IsVanillaDlcBundleFolder (public static, no instance state) specifically so it's directly unit testable, mirroring DiffPlexMergeEngine.BuildMerge's own reasoning for the same shape. CustomLoadOrder.ProcessLine stays private (stateful across a multi-line parse, and the class reads a real file path in its constructor) - tested via reflection instead, since constructing a CustomLoadOrder() is safe when mods.settings doesn't exist (Refresh() no-ops) and touches neither AppState.Settings nor the filesystem beyond that existence check. Verified: dotnet build clean (5 pre-existing warnings, 0 new), dotnet test 49/49 passing (18 new), dotnet format whitespace --verify-no-changes clean. AI-assisted (Claude Code) per this repo's CONTRIBUTING.md disclosure convention. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah
- Add a "Vortex-fork parity fixes" section to WitcherScriptMerger.Core/CLAUDE.md documenting the VK= and bob/case-insensitive-DLC gaps, and point both fix comments and the Tests/CLAUDE.md coverage list at it instead of a dangling "see CLAUDE.md" reference that didn't resolve to any actual content. - Correct FileMerger.cs's IsVanillaDlcBundleFolder comment (and the matching test comment): the prior case-sensitive regex bug was never actually caused by Windows' case-insensitive filesystem - .NET's Regex is case-sensitive on any platform - the real risk is real-world casing variance across installs. - CustomLoadOrderTests.cs now builds its CustomLoadOrder via RuntimeHelpers.GetUninitializedObject instead of the real constructor, so the test no longer depends on the state of a developer's real Documents\The Witcher 3\mods.settings file (previously safe only when that file was absent; a present-and-locked file would throw IOException unrelated to what the test covers). ProcessLine touches no instance state beyond its ref parameter, so skipping construction is safe. Reviewed-by: code-review skill (workflow-backed, medium effort) 🤖 Generated with Claude Code (https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah
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
1 participant
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.
Summary
Comparison against
IDCs/WitcherScriptMerger— a dormant but heavily-downloaded forkthat Vortex's real
game-witcher3extension actually drives — surfaced two concreteparity gaps this fork had inherited unmodified from upstream:
CustomLoadOrder.ProcessLinehad no tolerance forVK=(VortexKey) lines thatVortex's own mod-management integration writes into
mods.settings. Any such linefell into the catch-all "unrecognized value" branch, which aborts
Refresh()'sentire parse loop (
IsValidstaysfalse,Modsstays empty) — a Vortex-managedload order was silently unusable. Now recognized and ignored, narrowly (this repo's
mods.settingsparser deliberately keeps its catch-all warning for genuinelymalformed lines rather than broadening acceptance generically).
FileMerger's DLC-bundle-folder regex (GetUnpackedFiles's vanilla-bundlelookup) had no
"bob"alternative (Blood & Wine's internal DLC folder codename) —Blood & Wine bundle-content conflicts never matched against a vanilla bundle — and
was case-sensitive, which risks missing a real vanilla DLC folder whose on-disk
casing simply differs across installs. Fixed and exposed as a public static pure
function,
FileMerger.IsVanillaDlcBundleFolder, matchingDiffPlexMergeEngine.BuildMerge's existing public/static shape for testability.Both are documented in
WitcherScriptMerger.Core/CLAUDE.md's new "Vortex-fork parityfixes" section and regression-tested in
WitcherScriptMerger.Tests.This PR also incorporates fixes from a workflow-backed code-review pass (max effort,
findings addressed in the second commit): corrected an inaccurate causal claim in the
case-insensitivity rationale comment (.NET's
Regexis case-sensitive regardless ofplatform — the filesystem was never the mechanism), fixed dangling "see CLAUDE.md"
references that didn't resolve to any actual content, updated
WitcherScriptMerger.Tests/CLAUDE.md's coverage inventory, and madeCustomLoadOrderTests.csbuild itsCustomLoadOrderviaRuntimeHelpers.GetUninitializedObjectinstead of the real constructor, so the test nolonger depends on the state of a developer's real
mods.settingsfile. One reviewfinding (generalizing
ProcessLineto tolerate any unrecognized key, not justVK=) wasdeliberately left as noted-but-not-implemented — it's a legitimate design idea but wider
in scope than the two specific gaps this PR targets, and the existing catch-all warning
is intentional malformed-file protection this repo doesn't want to weaken.
How verified
dotnet build WitcherScriptMerger.sln— succeeds, 0 warnings beyond the 5 pre-existingones on
main.dotnet test WitcherScriptMerger.sln— 49/49 passing (2 new tests:FileMergerTests,CustomLoadOrderTests).dotnet format whitespace WitcherScriptMerger.sln --verify-no-changes— clean..csfiles.AI disclosure
This PR was developed with Claude Code (Anthropic), including a workflow-backed
multi-agent code-review pass whose findings are incorporated in the second commit.
🤖 Generated with Claude Code
https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah