Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions WitcherScriptMerger.Core/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,37 @@ ever going to remain, the interface indirection was deleted as premature abstrac
own private `DiffPlexMergeEngine` field directly — there's no engine-selection step at
startup in either host anymore.

## Vortex-fork parity fixes (`mods.settings` "VK=" lines, DLC-bundle-folder matching)

Two small parity gaps versus a separate, Vortex-integrated fork of this project
(`IDCs/WitcherScriptMerger`, the fork Vortex's real `game-witcher3` extension actually
drives) were found by direct comparison and fixed here:

- **`LoadOrder/CustomLoadOrder.ProcessLine`** now recognizes and ignores a `VK=`
(VortexKey) line instead of falling into the catch-all "unrecognized value" branch.
Vortex's own mod-management integration writes this key into `mods.settings`; without
this, `ProcessLine` returning `false` aborts `Refresh()`'s entire parse loop
(`IsValid` stays `false`, `Mods` stays empty) the moment a Vortex-managed
`mods.settings` is read. Deliberately a narrow, explicit `VK=` check rather than a
generic "tolerate any unrecognized key" change — the catch-all warning is intentional
malformed-file detection, and broadening it to accept-all would remove that
protection for a genuinely broken file. If another mod manager introduces another key
this parser doesn't know, it fails the same way `VK=` used to, by design; fix it the
same way, one recognized key at a time, rather than widening acceptance generically.
- **`Inventory/FileMerger.IsVanillaDlcBundleFolder`** (backing `GetUnpackedFiles`'s
vanilla-bundle lookup) now matches `"bob"` (Blood & Wine's internal DLC folder
codename) in addition to `DLC[0-9]*`/`ep[0-9]`, and matches case-insensitively. The
original regex had no `bob` alternative at all — Blood & Wine bundle-content
conflicts never matched against a vanilla bundle — and was case-sensitive, which
matters because real on-disk folder names vary in casing across different game/mod
installs regardless of platform. Exposed as a public static pure function (mirroring
`DiffPlexMergeEngine.BuildMerge`'s own public/static shape) specifically so it's
directly unit-testable.

Both are regression-tested in `WitcherScriptMerger.Tests`
(`LoadOrder/CustomLoadOrderTests.cs`, `Inventory/FileMergerTests.cs`) — see
`WitcherScriptMerger.Tests/CLAUDE.md`.

## CLI & MCP orchestration (`Cli/`, `Mcp/`)

`Cli/MergeOperations.cs` is the scan-then-merge sequence shared by both hosts' `merge`
Expand Down
19 changes: 18 additions & 1 deletion WitcherScriptMerger.Core/Inventory/FileMerger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,23 @@ public class MergeReportData
bool _bundleChanged;
List<Merge> _pendingBundleMerges = new List<Merge>();

static readonly Regex VanillaDlcBundleFolderPattern =
new Regex(@"(DLC[0-9]*|ep[0-9]|bob)$", RegexOptions.IgnoreCase | RegexOptions.Compiled);

// Matches a DLC-folder name that has its own "bundles" subfolder to search for a
// vanilla bundle: base-game expansions (DLC1, DLC2, ...), the "ep1"/"ep2" xpac
// folders, and "bob" - Blood & Wine's internal folder codename (see this
// project's CLAUDE.md, "Vortex-fork parity fixes" section, for the fork
// comparison this was found against). IgnoreCase because real on-disk folder
// names vary in casing across different game/mod installs - e.g. a repacked or
// differently-sourced "Bob"/"BOB" folder - and .NET's Regex is case-sensitive by
// default regardless of the underlying filesystem, so the prior case-sensitive
// match could silently miss a real vanilla DLC folder on any platform, not just
// a case-sensitive one. Public and static (no instance state involved)
// specifically so it's directly unit testable, matching
// DiffPlexMergeEngine.BuildMerge's own reasoning for the same shape.
public static bool IsVanillaDlcBundleFolder(string path) => VanillaDlcBundleFolderPattern.IsMatch(path);

#endregion

public FileMerger(MergeInventory inventory)
Expand Down Expand Up @@ -714,7 +731,7 @@ bool GetUnpackedFiles(string contentRelativePath, ref MergeSource source1, ref M
.Concat(
Directory.Exists(Paths.DlcDirectory)
? Directory.GetDirectories(Paths.DlcDirectory)
.Where(path => new Regex("DLC[0-9]*$").IsMatch(path) || new Regex("ep[0-9]$").IsMatch(path))
.Where(IsVanillaDlcBundleFolder)
.Select(path => Path.Combine(path, Paths.BundleBase, "bundles"))
: Enumerable.Empty<string>()
)
Expand Down
9 changes: 9 additions & 0 deletions WitcherScriptMerger.Core/LoadOrder/CustomLoadOrder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,15 @@ bool ProcessLine(string line, int lineNum, ref ModLoadSetting setting)
if (!ProcessPriorityLine(line, lineNum, setting))
return false;
}
else if (line.StartsWith("VK="))
{
// VortexKey - written into mods.settings by Vortex's own mod-management
// integration (see this project's CLAUDE.md, "Vortex-fork parity fixes"
// section, for the fork comparison this was found against); this parser
// has no use for it, but it's a legitimate line, not a malformed file, so
// it's recognized and ignored rather than falling into the catch-all
// warning below and aborting the whole parse.
}
else if (!string.IsNullOrWhiteSpace(line) && !line.StartsWith(";"))
{
ShowWarningForMalformedFile($"Unrecognized value on line {lineNum}:\n\n{line}");
Expand Down
6 changes: 6 additions & 0 deletions WitcherScriptMerger.Tests/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ does.
`MergeHeadless_EncodingMismatch_...` fixture reproducing the `baseEffect.ws`-style false
conflict that motivated it.
- `Tools/HasherTests.cs` — `Hasher`'s xxHash32 output, including synthetic edge cases.
- `Inventory/FileMergerTests.cs` — `FileMerger.IsVanillaDlcBundleFolder`: known vanilla
DLC-folder names, case-insensitivity, and non-matches (including anchoring) — see
Core's `CLAUDE.md`'s "Vortex-fork parity fixes" section.
- `LoadOrder/CustomLoadOrderTests.cs` — `CustomLoadOrder.ProcessLine`'s tolerance for
`mods.settings` "VK=" (VortexKey) lines, via reflection — see Core's `CLAUDE.md`'s
"Vortex-fork parity fixes" section.
- `Tools/KDiff3CrossCheckTests.cs` — an auto-solvable-only A/B check of
`DiffPlexMergeEngine` against a real `KDiff3.exe` binary, when a developer happens to
have one locally (WSM no longer bundles or requires KDiff3 itself — see
Expand Down
56 changes: 56 additions & 0 deletions WitcherScriptMerger.Tests/Inventory/FileMergerTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
using WitcherScriptMerger.Inventory;
using Xunit;

namespace WitcherScriptMerger.Tests.Inventory
{
// Regression coverage for FileMerger.IsVanillaDlcBundleFolder - the DLC-folder-name
// filter GetUnpackedFiles uses to find a matching vanilla bundle. A different fork of
// this project (github.com/IDCs/WitcherScriptMerger) found and fixed two real gaps
// here that this repo had inherited unmodified from upstream: no "bob" (Blood & Wine's
// internal folder codename) alternative, and a case-sensitive match that only ever
// worked by luck of Windows' case-insensitive filesystem - see
// FileMerger.cs's own comment on VanillaDlcBundleFolderPattern and Core's CLAUDE.md.
public class FileMergerTests
{
[Theory]
[InlineData(@"C:\Witcher3\DLC\DLC1")]
[InlineData(@"C:\Witcher3\DLC\DLC13")]
[InlineData(@"C:\Witcher3\DLC\DLC")]
[InlineData(@"C:\Witcher3\DLC\ep1")]
[InlineData(@"C:\Witcher3\DLC\ep2")]
[InlineData(@"C:\Witcher3\DLC\bob")]
public void IsVanillaDlcBundleFolder_KnownVanillaDlcFolders_ReturnsTrue(string path)
{
Assert.True(FileMerger.IsVanillaDlcBundleFolder(path));
}

[Theory]
[InlineData(@"C:\Witcher3\DLC\dlc1")]
[InlineData(@"C:\Witcher3\DLC\Dlc1")]
[InlineData(@"C:\Witcher3\DLC\EP1")]
[InlineData(@"C:\Witcher3\DLC\Ep1")]
[InlineData(@"C:\Witcher3\DLC\BOB")]
[InlineData(@"C:\Witcher3\DLC\Bob")]
public void IsVanillaDlcBundleFolder_DifferentCasing_StillMatches(string path)
{
// The prior implementation used a case-sensitive regex - since .NET's Regex is
// case-sensitive by default regardless of platform, it could silently miss a real
// vanilla DLC folder whose on-disk casing simply differs (e.g. a
// differently-sourced or repacked install), on any platform.
Assert.True(FileMerger.IsVanillaDlcBundleFolder(path));
}

[Theory]
[InlineData(@"C:\Witcher3\DLC\some_other_mod")]
[InlineData(@"C:\Witcher3\DLC\bobsleigh")]
[InlineData(@"C:\Witcher3\DLC\episode1")]
[InlineData(@"")]
public void IsVanillaDlcBundleFolder_NonVanillaFolders_ReturnsFalse(string path)
{
// "bobsleigh"/"episode1" specifically confirm the pattern is anchored to the end
// of the path (via "$") rather than matching "bob"/"ep" + a digit as a bare
// substring anywhere earlier in a longer, unrelated folder name.
Assert.False(FileMerger.IsVanillaDlcBundleFolder(path));
}
}
}
63 changes: 63 additions & 0 deletions WitcherScriptMerger.Tests/LoadOrder/CustomLoadOrderTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
using System.Reflection;
using System.Runtime.CompilerServices;
using WitcherScriptMerger.LoadOrder;
using Xunit;

namespace WitcherScriptMerger.Tests.LoadOrder
{
// Regression coverage for CustomLoadOrder.ProcessLine's handling of "VK=" lines. A
// different fork of this project (github.com/IDCs/WitcherScriptMerger) found that
// Vortex writes "VK=" (VortexKey) lines into mods.settings, which this parser had no
// tolerance for - falling into the catch-all "unrecognized value" branch and aborting
// the entire parse (IsValid stays false, no load order is usable) - see
// CustomLoadOrder.cs's own comment on the "VK=" branch and Core's CLAUDE.md.
//
// ProcessLine is a private instance method invoked via reflection rather than exposed
// publicly - unlike FileMerger.IsVanillaDlcBundleFolder (pure string/regex logic with
// no other coupling), ProcessLine is inherently stateful across a multi-line parse
// (accumulates a ModLoadSetting via `ref`) and CustomLoadOrder's constructor reads a
// real, fixed path under the current user's Documents folder - reflection avoids
// either widening ProcessLine's visibility or refactoring CustomLoadOrder's file-path
// coupling just for this test. The instance itself is created via
// RuntimeHelpers.GetUninitializedObject rather than `new CustomLoadOrder()`, skipping
// the constructor (and its Refresh() call) entirely - ProcessLine touches no instance
// state beyond the `ref` setting parameter, so it needs no initialized instance, and
// skipping construction avoids depending on the test-running machine's real
// mods.settings file, which - unlike the "file doesn't exist" case Refresh() no-ops
// on safely - could be present and locked by a running game/Vortex process on a
// developer machine with a live install, throwing IOException for a reason unrelated
// to what this test actually covers.
public class CustomLoadOrderTests
{
[Fact]
public void ProcessLine_VortexKeyLine_IsRecognizedAndIgnored()
{
var loadOrder = (CustomLoadOrder)RuntimeHelpers.GetUninitializedObject(typeof(CustomLoadOrder));
var processLine = typeof(CustomLoadOrder).GetMethod("ProcessLine", BindingFlags.NonPublic | BindingFlags.Instance);

ModLoadSetting setting = null;
object[] args = { "VK=1a2b3c4d", 1, setting };
var result = (bool)processLine.Invoke(loadOrder, args);

// A malformed line returns false and aborts the whole parse (see Refresh()) -
// true here confirms "VK=..." is treated as a recognized, ignorable line, not
// as "unrecognized value" like it would have been before this fix.
Assert.True(result);
}

[Fact]
public void ProcessLine_TrulyUnrecognizedLine_StillFails()
{
// Confirms the VK= fix didn't accidentally widen ProcessLine to silently accept
// everything - a genuinely malformed line must still fail the parse.
var loadOrder = (CustomLoadOrder)RuntimeHelpers.GetUninitializedObject(typeof(CustomLoadOrder));
var processLine = typeof(CustomLoadOrder).GetMethod("ProcessLine", BindingFlags.NonPublic | BindingFlags.Instance);

ModLoadSetting setting = null;
object[] args = { "SomethingElse=1a2b3c4d", 1, setting };
var result = (bool)processLine.Invoke(loadOrder, args);

Assert.False(result);
}
}
}
Loading