Add config-extensible allowlist for vanilla DLC/expansion bundle folders - #14
Merged
Merged
Conversation
FileMerger.IsVanillaDlcBundleFolder now has a two-arg overload that ORs the existing DLC1/DLC2/.../ep1/ep2/bob regex with an exact, case-insensitive match against a caller-supplied list of extra folder names, so a future DLC/expansion whose folder codename isn't in the built-in regex yet (e.g. CD Projekt Red's "Songs of the Past", announced 2026, no folder name public yet) can be recognized via config instead of a code change. Wired up via a new "AdditionalVanillaDlcFolderNames" App.config setting (comma-separated exact folder names, empty by default), read once at FileMerger. GetUnpackedFiles' call site and passed in - IsVanillaDlcBundleFolder itself stays a pure, static, AppState-free function, since touching AppState. Settings from code a test exercises can crash the whole dotnet test process (see WitcherScriptMerger.Core/CLAUDE.md and WitcherScriptMerger.Tests/ CLAUDE.md). Deliberately stays a strict allowlist, never existence-based auto-discovery: Vortex's "witcher3dlc" mod type deploys ordinary user mods into the identical GameDirectory\DLC\<modname>\content\... shape as real vanilla DLC content. Code review on this change caught a real, pre-existing bug in the base regex it ORs against: VanillaDlcBundleFolderPattern was end-anchored but matched against the full path with no start anchor, so it matched any folder name merely *ending* in a recognized substring (e.g. "ImmersiveDLC" or "Step1"), not just a folder name that IS one - exactly the kind of arbitrary mod-folder-name collision this allowlist's own "no auto-discovery" rule exists to prevent. Fixed by matching a fully "^...$"-anchored pattern against just the extracted folder-name segment instead, which also resolves a related inconsistency (regex checked the raw path, the allowlist checked a separator-trimmed segment). Verified: dotnet build/test/format all clean, 67 tests passing (16 new - extra-name matching, case-insensitivity, empty-list non-regression, the anchoring-bug regression, and a null-list guard). Also ran a disposable, non-committed scratch console app exercising the real two-arg overload against a real scratch filesystem tree, confirming a synthetic "SongsOfThePast" DLC folder is excluded without the config entry and included with it, while an "ImmersiveDLC"/"SomeVortexMod" folder is never matched either way - a full `merge` CLI run wasn't possible in this environment since QuickBMS/wcc_lite aren't available (bundle-content conflict detection itself requires QuickBMS during scanning, before this code even runs). AI-assisted: substantially produced with 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
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
FileMerger.IsVanillaDlcBundleFoldernow has a two-arg overload —IsVanillaDlcBundleFolder(string path, IEnumerable<string> additionalFolderNames)— that ORs the existingDLC[0-9]*/ep[0-9]/bobregex with an exact, case-insensitive match against a caller-supplied allowlist of extra folder names. The single-arg overload now forwards to it with an empty list, so all existing behavior/tests are unchanged.AdditionalVanillaDlcFolderNamesApp.config setting (comma-separated exact folder names, empty by default) added to bothWitcherScriptMerger/App.configandWitcherScriptMerger.Headless/App.config, so a future DLC/expansion whose folder codename isn't recognized yet (e.g. CD Projekt Red's "Songs of the Past", announced in 2026 with no public folder name at time of writing) can be picked up via config instead of a code change.FileMerger.GetUnpackedFiles' call site only —IsVanillaDlcBundleFolderitself never touchesAppState/AppState.Settings, staying a pure, static, directly unit-testable function. This matters here specifically:AppSettings's constructor callsEnvironment.Exit(1)when it can't find a config file next to the entry assembly, which would kill the entiredotnet testprocess if touched from code a test exercises (seeWitcherScriptMerger.Core/CLAUDE.mdandWitcherScriptMerger.Tests/CLAUDE.md).witcher3dlcmod type deploys ordinary user mods into<GameDir>\DLC\<modname>\content\..., the identical on-disk shape as real vanilla DLC content, so treating "any folder under DLC" as a vanilla merge baseline would risk silently merging a conflict against a mod's own bundle instead of vanilla's.A real bug found (and fixed) via code review
Running the
code-reviewskill on this diff caught a genuine, pre-existing bug in the base regex this new overload ORs against:VanillaDlcBundleFolderPattern("(DLC[0-9]*|ep[0-9]|bob)$") was matched against the full path with only an end anchor. Since .NET'sRegex.IsMatchhas no implicit start anchor, this matched any folder name merely ending in one of those substrings — not just a folder name that is one of them (optionally + digits). Confirmed:"ImmersiveDLC"(ends in"DLC") and"Step1"(ends in"ep1") both incorrectly matched. This is exactly the Vortex-mod-folder-name-collision risk this feature's own allowlist rule exists to prevent, so it was fixed as part of this PR: the regex is now"^(DLC[0-9]*|ep[0-9]|bob)$", matched against just the extracted folder-name segment (not the raw path) — which also resolves a related inconsistency where the regex checked the raw path while the allowlist checked an already separator-trimmed segment. Regression-tested viaIsVanillaDlcBundleFolder_FolderNameMerelyEndsInPattern_ReturnsFalse.Also fixed on review: allowlist config entries are now trimmed of stray trailing directory separators (e.g. a user typing
SongsOfThePast\), not just whitespace, matching the normalization already applied to the path side.Test plan
dotnet build WitcherScriptMerger.sln— succeeds, exactly the same 5 pre-existing warnings asmain, no new ones (confirmed via--no-incrementalfull rebuild).dotnet test WitcherScriptMerger.sln— all 67 tests pass (16 new: extra-name allowlist matching incl. a trailing path separator, empty-list non-regression against every existing regex case, case-insensitivity for extra names, an extra name absent from the list still returningfalse, anull-list guard, and the anchoring-bug regression above).dotnet format whitespace WitcherScriptMerger.sln --verify-no-changes— clean.mergeCLI run through actual bundle-content unpacking wasn't possible in this environment — noquickbms.exe/wcc_lite.exeavailable, and bundle-content conflicts are never even detected duringModFileIndex.BuildAsync's scan without a real QuickBMS install (QuickBms.GetBundleContentPathsgracefully returnsArray.Empty<string>()without it), so the vanilla-bundle-search code this PR touches is unreachable via the full pipeline without QuickBMS. Instead, ran a disposable, non-committed scratch console app (per the patternWitcherScriptMerger.Tests/CLAUDE.mddocuments for exactly this situation) that builds a real scratch filesystem tree (DLC\DLC1,DLC\SongsOfThePast,DLC\ImmersiveDLC,DLC\SomeVortexMod, each with acontent\bundles\*.bundlefile) and calls the real, shippedFileMerger.IsVanillaDlcBundleFoldertwo-arg overload viaFileMerger.GetUnpackedFiles' exact directory-discovery LINQ shape. Confirmed: without the config entry, the syntheticSongsOfThePastfolder is excluded; withAdditionalVanillaDlcFolderNames=SongsOfThePast, it's included as a vanilla-baseline candidate;ImmersiveDLC/SomeVortexModare never matched either way. This is a real exercise of the shipped code against realDirectory.GetDirectories()output, just stopping short of the QuickBMS-dependent unpack step.AI disclosure
This PR was substantially produced with Claude Code (Claude Sonnet 5), per
CONTRIBUTING.md's AI-assisted-development disclosure requirement. Commits carryCo-Authored-By/Claude-Sessiontrailers.https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah