erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases 6 Packages Wiki Activity Actions

fix(vt): round 3 item 1 — drop-in .usd EnableLooting no longer clobbered

Browse source 
MossTankProfileStore's SideCarDocument.InventoryEnableLooting duplicated
the real "EnableLooting" .usd Settings row, and the side-car's own Apply()
ran AFTER the .usd was applied in LoadCurrent — so a drop-in profile with
looting on loaded with it silently off, clobbered by whatever the side-car
happened to hold (default false for a profile with no side-car at all).

Deleted the duplicate field; settings.Inventory.Loot.Enabled is now owned
exclusively by the .usd row. Added a reflection test proving no SideCarDocument
field (typed or with its Combat/Buff/Vitals/Inventory group prefix stripped)
collides with a VtankOptionCatalog name, and a drop-in regression test:
a real .usd fixture with EnableLooting=True and NO side-car now loads with
panel.LootEnabled == true.

Mutation: reverted MossTankProfileStore.cs to HEAD (keeping only the new
tests) and ran both new tests — SideCarDocumentHasNoFieldNamedForARealVtankSetting
failed with `["InventoryEnableLooting"]`, DropInUsdWithLootingEnabledAndNoSideCarLoadsLootingEnabled
failed with Expected True/Actual False — confirming both tests exercise the bug
before the fix.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-09-07 01:03:47 +02:00
parent 8a146aa8c4
commit b6d642c9b7
3 changed files with 102 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

14
src/AcDream.Plugins.MossTank/MossTankProfileStore.cs
View file

@ -625,7 +625,14 @@ internal sealed class MossTankProfileStore
public bool BuffTrainedSkillsOnly { get; set; } = true;
public bool VitalsEnabled { get; set; } = true;
public double InventoryScanIntervalSeconds { get; set; } = 0.25d;
public bool InventoryEnableLooting { get; set; }
// InventoryEnableLooting was deleted here (round 3, item 1): it
// duplicated the real "EnableLooting" .usd Settings row
// (VtankSettingsProfileSerializer "enablelooting" case), and this
// side-car's own Apply() ran AFTER the .usd was applied — so a
// drop-in profile with looting on loaded with it off, clobbered by
// whatever this field's own JSON default/stale value happened to be.
// settings.Inventory.Loot.Enabled is now owned exclusively by the
// .usd file; this side-car must never touch it again.
public string InventoryLootClassifierId { get; set; } = string.Empty;
public double InventoryLootScanIntervalSeconds { get; set; } = 0.25d;
public LootRuleDocument[] InventoryLootRules { get; set; } = [];
@ -658,7 +665,6 @@ internal sealed class MossTankProfileStore
BuffTrainedSkillsOnly = settings.Buffs.BuffTrainedSkillsOnly,
VitalsEnabled = settings.Vitals.Enabled,
InventoryScanIntervalSeconds = settings.Inventory.ScanIntervalSeconds,
InventoryEnableLooting = settings.Inventory.Loot.Enabled,
InventoryLootClassifierId = settings.Inventory.Loot.ExternalClassifierId,
InventoryLootScanIntervalSeconds = settings.Inventory.Loot.ScanIntervalSeconds,
InventoryLootRules = settings.Inventory.Loot.Rules
@ -724,7 +730,9 @@ internal sealed class MossTankProfileStore
settings.Inventory.ScanIntervalSeconds = Math.Clamp(
InventoryScanIntervalSeconds, 0.05d, 10d);
settings.Inventory.Loot.Enabled = InventoryEnableLooting;
// settings.Inventory.Loot.Enabled is NOT touched here — see the
// field-removal comment above. The .usd's own "EnableLooting"
// row (applied before this side-car, in LoadCurrent) owns it.
settings.Inventory.Loot.ExternalClassifierId = InventoryLootClassifierId?.Trim()
?? string.Empty;
settings.Inventory.Loot.ScanIntervalSeconds = Math.Clamp(

34
tests/AcDream.Plugins.MossTank.Tests/MossTankPanelTests.cs
View file

@ -102,6 +102,40 @@ public sealed class MossTankPanelTests
Assert.Equal(0.5d, panel.EvaluateExpression("uboptget['AttackDistance']").AsNumber(), precision: 7);
}
/// <summary>
/// Round 3 item 1 (BLOCKER): a drop-in <c>.usd</c> with
/// <c>EnableLooting=True</c> and NO side-car at all must load with
/// looting enabled. Before the fix, <c>SideCarDocument.Apply</c> ran
/// AFTER the <c>.usd</c> and unconditionally wrote its own
/// <c>InventoryEnableLooting</c> field (defaulting to <see langword="false"/>
/// for a profile that has never had a side-car written) over
/// <c>settings.Inventory.Loot.Enabled</c>, silently turning looting back
/// off.
/// </summary>
[Fact]
public void DropInUsdWithLootingEnabledAndNoSideCarLoadsLootingEnabled()
{
var storage = new MemoryStorage();
var automation = new FakeAutomation { Name = "Barris" };
string usdKey = VtankProfileDirectory.AutoCharacterFileName("Barris", string.Empty, "usd");
VtankDatabase database = VtankDefaultSettingsDatabase.Parse();
VtankTable settingsTable = database.Find("Settings")!;
int nameColumn = settingsTable.ColumnIndex("Setting");
int valueColumn = settingsTable.ColumnIndex("Value");
VtankRow row = settingsTable.Rows.First(candidate =>
candidate.Cells[nameColumn].AsString().Equals(
"EnableLooting", StringComparison.OrdinalIgnoreCase));
row.Cells[valueColumn] = VtankCell.Bool(true);
storage.Text[usdKey] = database.Render();
// No side-car key at all — a real drop-in, not something MossTank
// itself ever saved.
var panel = new MossTankPanel(new FakeHost(automation, storage));
Assert.True(panel.LootEnabled);
}
/// <summary>
/// Reproduces MossTankMetaProfileStore's pre-cutover by-character JSON
/// hash key (its own <c>Hash(string)</c> — 12-byte truncated SHA256,

57
tests/AcDream.Plugins.MossTank.Tests/MossTankProfileStoreSideCarTests.cs Normal file
View file

@ -0,0 +1,57 @@
using System.Reflection;
namespace AcDream.Plugins.MossTank.Tests;
/// <summary>
/// Round 3 item 1: proves <c>MossTankProfileStore.SideCarDocument</c> (the
/// private JSON side-car for state that has no VTank Settings-table name at
/// all) carries no field that ALSO has a real <c>.usd</c> catalog name —
/// any such overlap is a live duplication risk exactly like the
/// <c>InventoryEnableLooting</c>/<c>EnableLooting</c> bug this round fixed
/// (the side-car's own copy is always applied AFTER the <c>.usd</c> row, so
/// a stale or default side-car value silently clobbers whatever the real
/// profile file says).
/// </summary>
public sealed class MossTankProfileStoreSideCarTests
{
// The side-car's own property names are grouped by settings section
// (Combat*/Buff*/Vitals*/Inventory*) purely for readability — VTank's
// catalog names carry no such prefix, so each candidate is checked both
// as typed and with its known group prefix stripped.
private static readonly string[] GroupPrefixes = ["Combat", "Buff", "Vitals", "Inventory"];
[Fact]
public void SideCarDocumentHasNoFieldNamedForARealVtankSetting()
{
Type sideCar = typeof(MossTankProfileStore).GetNestedType(
"SideCarDocument", BindingFlags.NonPublic)
?? throw new InvalidOperationException(
"MossTankProfileStore.SideCarDocument was not found by reflection.");
var catalogNames = new HashSet<string>(
VtankOptionCatalog.Names, StringComparer.OrdinalIgnoreCase);
var overlaps = new List<string>();
foreach (PropertyInfo property in sideCar.GetProperties(
BindingFlags.Public | BindingFlags.Instance))
{
string name = property.Name;
if (catalogNames.Contains(name))
{
overlaps.Add(name);
continue;
}
foreach (string prefix in GroupPrefixes)
{
if (name.Length > prefix.Length
&& name.StartsWith(prefix, StringComparison.Ordinal)
&& catalogNames.Contains(name[prefix.Length..]))
{
overlaps.Add(name);
break;
}
}
}
Assert.Empty(overlaps);
}
}