fix(vt): J five nits — argument order, sort order, dedup key readers

Item J (slice-1 fix round), five sub-parts (item 12's sixth, the
VtankCellBuilder/VtankCellFactory note, required no change — both are
genuinely in use):

1. MobsInDist_Priority's regex-table entry gained an argument-order
   comment (count, distance, priority — cross-referenced against Meta.cs's
   runtime evaluation, CountMonstersByPriority(priority, distance) >=
   count) plus a new synthesized round-trip test
   (MobsInDistPriorityRoundTripsAllThreeNumbersDistinctly): this condition
   is never exercised by any committed real fixture (only its name
   appears, in the auto-completion header banner text), so nothing
   previously caught an accidental swap of any two of its three numeric
   fields.

2. VtankDatabase.Render() now explicitly sorts tables by name
   (StringComparer.Ordinal) before writing — VTank's own `y` class holds
   tables in a SortedDictionary, so a real .usd/.ast always emits
   table-name order. This port's own Tables is an insertion-ordered list,
   so every committed fixture happened to round-trip in order today
   purely because it was already sorted the last time real VTank wrote it
   (confirmed: defaultsettings.usd's own first five tables are already
   alphabetical). New RenderEmitsTablesInNameOrderRegardlessOfInsertionOrder
   adds three tables in deliberately reversed order to prove the sort,
   not just re-check an already-sorted fixture.

3. SaveMeta's NAV: block write order now comes from a List<(MetaAction,
   string)> populated in AssignEmbedTags's own traversal order, not
   Dictionary<MetaAction,string> enumeration — Dictionary enumeration
   order happens to match insertion order in the current runtime absent
   removals, but that is an implementation detail, never a documented BCL
   contract. embedTags stays a Dictionary purely for WriteAction's O(1)
   lookup; embedOrder is the sole source of write order.

4. "/vt nav save Foo.af" and "/vt meta save Foo.af" used to keep the
   ".af" suffix (only ".nav"/".met" were stripped from the argument),
   producing a doubled "exports/nav/Foo.af.af" / "exports/meta/Foo.af.af"
   export instead of "exports/nav/Foo.af" / "exports/meta/Foo.af" — .af
   is the only VTank-compatible storage format now, so both commands
   strip it too. NavCommandsImportAndExportExactVtankNavFiles/
   MetaCommandsImportAndExportExactVtankMetFiles renamed to
   NavCommandsImportLegacyAndExportAf/MetaCommandsImportLegacyAndExportAf
   (the writer is real metaf output now, not a byte-exact pass-through of
   the imported .nav/.met, so the old names overstated what they prove);
   new NavSaveAcceptsAnAfSuffixedNameWithoutDoublingIt/
   MetaSaveAcceptsAnAfSuffixedNameWithoutDoublingIt pin the fix.

5. VtankLootRequirementEvaluator's IntKeyExists/DoubleKeyExists were each
   an independently hand-maintained duplicate of IntValue/DoubleValue's
   own named-field key list — a key added to one switch and forgotten in
   the other would silently make BuffedInt/BuffedDouble's KeyExists gate
   treat a real, always-present field as "raw property bag only". New
   TryIntValue/TryDoubleValue are the single source of truth for both
   "what is this key's value" and "does it exist at all"; IntValue/
   DoubleValue and IntKeyExists/DoubleKeyExists are now both thin wrappers
   over them. The two double-side "virtual field" keys (VtankDoubleBase+12/+14,
   which always exist regardless of whether their remapped raw key is
   present) needed an explicit comment to preserve that exact semantic
   through the consolidation.

Full MossTank suite: 583 -> 585 (2 new tests from item 4; items 1-3 and 5
added/renamed tests without net new count beyond that). App.Tests
(Plugin|LaunchOptions filter): 84/84.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-09-06 23:37:01 +02:00
parent 8fd3999616
commit d631cc4fe6
7 changed files with 250 additions and 71 deletions

View file

@ -163,6 +163,15 @@ internal static class MetafSerializer
["ItemCountLE"] = new($@"^\s+(?<i>{I})\s+(?<s>{S})$", RegexOptions.Compiled),
["ItemCountGE"] = new($@"^\s+(?<i>{I})\s+(?<s>{S})$", RegexOptions.Compiled),
["MobsInDist_Name"] = new($@"^\s+(?<i>{I})\s+(?<d>{D})\s+(?<s>{S})$", RegexOptions.Compiled),
// Argument order is count, distance, priority — "i" (Number) is the
// threshold monster COUNT compared against, "d" (SecondaryNumber)
// is the DISTANCE band, and the trailing "i2" (TertiaryNumber) is
// the PRIORITY level; see Meta.cs's runtime evaluation:
// `CountMonstersByPriority(priority: TertiaryNumber,
// distance: SecondaryNumber) >= Number`. Not exercised by any
// committed real fixture (only appears in the auto-completion
// header banner text) — see MobsInDist_PriorityRoundTripsAllThreeNumbersDistinctly
// for the synthesized proof.
["MobsInDist_Priority"] = new($@"^\s+(?<i>{I})\s+(?<d>{D})\s+(?<i2>{I})$", RegexOptions.Compiled),
["NeedToBuff"] = EmptyArgs,
["NoMobsInDist"] = new($@"^\s+(?<d>{D})$", RegexOptions.Compiled),
@ -334,9 +343,17 @@ internal static class MetafSerializer
// plus a sanitized "__name" suffix, counter starting at 0) keeps
// the writer's byte-for-byte fidelity against metaf's own emission.
var embedTags = new Dictionary<MetaAction, string>(ReferenceEqualityComparer.Instance);
// A List, not just the Dictionary above (item J, slice-1 fix
// round): Dictionary<TKey,TValue> enumeration order happens to
// match insertion order in the current runtime absent removals,
// but that is an implementation detail, never a documented BCL
// contract — the NAV: block write order below must not depend on
// it. embedTags itself stays a Dictionary purely for WriteAction's
// O(1) action->tag lookup.
var embedOrder = new List<(MetaAction Action, string Tag)>();
int embedCounter = 0;
foreach (MetaRule rule in profile.Rules.Where(static rule => rule.Enabled))
AssignEmbedTags(rule.Action, embedTags, ref embedCounter);
AssignEmbedTags(rule.Action, embedTags, embedOrder, ref embedCounter);
foreach (IGrouping<string, MetaRule> stateGroup in profile.Rules
.Where(static rule => rule.Enabled)
@ -355,7 +372,7 @@ internal static class MetafSerializer
}
lines.Add("~~ }");
}
if (embedTags.Count > 0)
if (embedOrder.Count > 0)
{
// Meta.ExportToMetAF (metaf_monolithic.py:12784-12790): a blank
// line, this exact separator (no space either side of the "~~"
@ -364,7 +381,7 @@ internal static class MetafSerializer
lines.Add("~~========================= ONLY NAVS APPEAR BELOW THIS LINE =========================~~");
lines.Add(string.Empty);
}
foreach ((MetaAction action, string tag) in embedTags)
foreach ((MetaAction action, string tag) in embedOrder)
{
if (action.EmbeddedRoute is { } nav)
WriteNavBlock(lines, tag, nav);
@ -377,17 +394,20 @@ internal static class MetafSerializer
private static void AssignEmbedTags(
MetaAction action,
Dictionary<MetaAction, string> tags,
List<(MetaAction Action, string Tag)> order,
ref int counter)
{
if (action.Kind == MetaActionKind.LoadEmbeddedNavigationRoute)
{
string sanitized = NonTagCharacter.Replace(action.SecondaryText ?? string.Empty, "_");
string postfix = sanitized.Length > 0 ? $"__{sanitized}" : string.Empty;
tags[action] = $"nav{counter}{postfix}";
string tag = $"nav{counter}{postfix}";
tags[action] = tag;
order.Add((action, tag));
counter++;
}
foreach (MetaAction child in action.Children)
AssignEmbedTags(child, tags, ref counter);
AssignEmbedTags(child, tags, order, ref counter);
}
/// <summary>Internal (not private) so other one-shot import readers can share this no-op.</summary>

View file

@ -271,7 +271,13 @@ internal sealed partial class MossTankPanel
WriteVtank("Usage: /vt nav [save/load] [filename]");
return;
}
name = StripExtension(name, ".nav");
// Item J (slice-1 fix round): .af is the only VTank-compatible
// storage format now (Campaign VT slice 1 Part A); ".nav" is
// stripped for legacy-typed names, ".af" for names copy-pasted
// from the exports/nav/ directory or a real VTank profile folder —
// without both, "/vt nav save Foo.af" would have produced a
// doubled "Foo.af.af" export.
name = StripExtension(name, ".nav", ".af");
if (operation == "save")
{
_routeProfiles.Create(
@ -353,7 +359,9 @@ internal sealed partial class MossTankPanel
WriteVtank("Usage: /vt meta [save/load] [filename]");
return;
}
name = StripExtension(name, ".met", ".json");
// Item J (slice-1 fix round): .af is the only VTank-compatible
// storage format now — see the analogous nav-command comment above.
name = StripExtension(name, ".met", ".json", ".af");
if (operation == "save")
{
_metaProfiles.Create(

View file

@ -376,54 +376,103 @@ internal static class VtankLootRequirementEvaluator
: string.Empty,
};
/// <summary>
/// The single source of truth for both "what is this int-typed key's
/// value" (<see cref="IntValue"/>) and "does this item actually carry a
/// base value for this key at all" (<see cref="IntKeyExists"/>) — item J
/// (slice-1 fix round) consolidated these from two independently
/// hand-maintained key lists that had to be kept in exact sync by hand:
/// a named field added to one switch and forgotten in the other would
/// silently let <c>BuffedInt</c>'s KeyExists gate treat a real,
/// always-present field as "raw property bag only", which for a key the
/// item's bag never happens to carry would wrongly refuse a spell bonus
/// it should apply. Returns <see langword="false"/> only when
/// <paramref name="key"/> is not one of the named
/// <see cref="PluginInventoryItem"/> fields AND the raw property bag
/// does not carry it either.
/// </summary>
private static bool TryIntValue(
uint key,
in PluginInventoryItem item,
in PluginItemProperties properties,
out int value)
{
switch (key)
{
case 5: value = item.Burden; return true;
case 19: value = item.Value; return true;
case 105: value = checked((int)item.Workmanship); return true;
case 107: value = item.ItemCurrentMana; return true;
case 108: value = item.ItemMaximumMana; return true;
case 131: value = checked((int)item.MaterialType); return true;
case VtankIntBase + 0: value = checked((int)item.WeenieClassId); return true;
case VtankIntBase + 2: value = checked((int)item.ContainerObjectId); return true;
case VtankIntBase + 4: value = item.ItemsCapacity; return true;
case VtankIntBase + 5: value = item.ContainersCapacity; return true;
case VtankIntBase + 6: value = item.StackSize; return true;
case VtankIntBase + 7: value = item.MaximumStackSize; return true;
case VtankIntBase + 8: value = checked((int)item.SpellId); return true;
case VtankIntBase + 9: value = item.ContainerSlot; return true;
case VtankIntBase + 10: value = checked((int)item.WielderObjectId); return true;
case VtankIntBase + 11: value = checked((int)item.EquippedLocation); return true;
case VtankIntBase + 14: value = checked((int)item.ValidLocations); return true;
case VtankIntBase + 18: value = checked((int)item.Useability); return true;
case VtankIntBase + 23: value = checked((int)item.PublicFlags); return true;
case VtankIntBase + 31: value = item.CombatUse; return true;
case VtankIntBase + 32: value = item.WeaponSkill; return true;
case VtankIntBase + 33: value = item.DamageType; return true;
case VtankIntBase + 34: value = item.Damage; return true;
case VtankIntBase + 38: value = item.AppraisedSpellIds.Count; return true;
default:
value = 0;
return properties.Ints?.TryGetValue(key, out value) == true;
}
}
private static int IntValue(
uint key,
in PluginInventoryItem item,
in PluginItemProperties properties) => key switch
in PluginItemProperties properties) =>
TryIntValue(key, item, properties, out int value) ? value : 0;
/// <summary>See <see cref="TryDoubleValue"/> — the double-side equivalent of <see cref="TryIntValue"/>'s consolidation.</summary>
private static bool TryDoubleValue(
uint key,
in PluginInventoryItem item,
in PluginItemProperties properties,
out double value)
{
5 => item.Burden,
19 => item.Value,
105 => checked((int)item.Workmanship),
107 => item.ItemCurrentMana,
108 => item.ItemMaximumMana,
131 => checked((int)item.MaterialType),
VtankIntBase + 0 => checked((int)item.WeenieClassId),
VtankIntBase + 2 => checked((int)item.ContainerObjectId),
VtankIntBase + 4 => item.ItemsCapacity,
VtankIntBase + 5 => item.ContainersCapacity,
VtankIntBase + 6 => item.StackSize,
VtankIntBase + 7 => item.MaximumStackSize,
VtankIntBase + 8 => checked((int)item.SpellId),
VtankIntBase + 9 => item.ContainerSlot,
VtankIntBase + 10 => checked((int)item.WielderObjectId),
VtankIntBase + 11 => checked((int)item.EquippedLocation),
VtankIntBase + 14 => checked((int)item.ValidLocations),
VtankIntBase + 18 => checked((int)item.Useability),
VtankIntBase + 23 => checked((int)item.PublicFlags),
VtankIntBase + 31 => item.CombatUse,
VtankIntBase + 32 => item.WeaponSkill,
VtankIntBase + 33 => item.DamageType,
VtankIntBase + 34 => item.Damage,
VtankIntBase + 38 => item.AppraisedSpellIds.Count,
_ => properties.Ints?.TryGetValue(key, out int value) == true
? value
: 0,
};
switch (key)
{
case VtankDoubleBase + 9: value = item.Workmanship; return true;
case VtankDoubleBase + 11: value = item.DamageVariance; return true;
// These two are named/virtual fields (like the int side's
// VtankIntBase+N cases) that always "exist", regardless of
// whether the underlying remapped raw key (62/63) happens to
// be present in the property bag — matching the prior
// DoubleKeyExists switch's explicit "=> true" for both.
case VtankDoubleBase + 12:
TryRawFloat(properties, 62, out value);
return true;
case VtankDoubleBase + 14:
TryRawFloat(properties, 63, out value);
return true;
default:
return TryRawFloat(properties, key, out value);
}
}
private static double DoubleValue(
uint key,
in PluginInventoryItem item,
in PluginItemProperties properties) => key switch
{
VtankDoubleBase + 9 => item.Workmanship,
VtankDoubleBase + 11 => item.DamageVariance,
VtankDoubleBase + 12 => RawFloat(properties, 62),
VtankDoubleBase + 14 => RawFloat(properties, 63),
_ => RawFloat(properties, key),
};
in PluginItemProperties properties) =>
TryDoubleValue(key, item, properties, out double value) ? value : 0d;
private static double RawFloat(in PluginItemProperties properties, uint key) =>
properties.Floats?.TryGetValue(key, out double value) == true ? value : 0d;
private static bool TryRawFloat(in PluginItemProperties properties, uint key, out double value)
{
value = 0d;
return properties.Floats?.TryGetValue(key, out value) == true;
}
private static int BuffedInt(
uint key,
@ -472,35 +521,21 @@ internal static class VtankLootRequirementEvaluator
}
/// <summary>
/// Mirrors <see cref="IntValue"/>'s switch: every named-field key is a
/// concrete <see cref="PluginInventoryItem"/> property and always
/// "exists"; anything else falls through to the raw property-bag
/// lookup, where existence means the bag actually carries that key.
/// Now driven by <see cref="TryIntValue"/> (item J, slice-1 fix round)
/// instead of an independently hand-maintained duplicate of its key
/// list — see that method's doc for why the duplication was a hazard.
/// </summary>
private static bool IntKeyExists(
uint key,
in PluginInventoryItem item,
in PluginItemProperties properties) => key switch
{
5 or 19 or 105 or 107 or 108 or 131
or VtankIntBase + 0 or VtankIntBase + 2 or VtankIntBase + 4
or VtankIntBase + 5 or VtankIntBase + 6 or VtankIntBase + 7
or VtankIntBase + 8 or VtankIntBase + 9 or VtankIntBase + 10
or VtankIntBase + 11 or VtankIntBase + 14 or VtankIntBase + 18
or VtankIntBase + 23 or VtankIntBase + 31 or VtankIntBase + 32
or VtankIntBase + 33 or VtankIntBase + 34 or VtankIntBase + 38 => true,
_ => properties.Ints?.ContainsKey(key) == true,
};
in PluginItemProperties properties) =>
TryIntValue(key, item, properties, out _);
private static bool DoubleKeyExists(
uint key,
in PluginInventoryItem item,
in PluginItemProperties properties) => key switch
{
VtankDoubleBase + 9 or VtankDoubleBase + 11
or VtankDoubleBase + 12 or VtankDoubleBase + 14 => true,
_ => properties.Floats?.ContainsKey(key) == true,
};
in PluginItemProperties properties) =>
TryDoubleValue(key, item, properties, out _);
private static double MinimumDamage(in PluginInventoryItem item) =>
item.Damage - (item.DamageVariance * item.Damage);

View file

@ -1,4 +1,5 @@
using System.Globalization;
using System.Linq;
using System.Text;
namespace AcDream.Plugins.MossTank;
@ -251,11 +252,31 @@ internal sealed class VtankDatabase
return database;
}
/// <summary>
/// VTank's own <c>y</c> class holds its tables in a
/// <c>SortedDictionary&lt;string, bd&gt;</c> (item J, slice-1 fix
/// round), so a real <c>.usd</c>/<c>.ast</c> always emits its tables in
/// table-name order — confirmed against the committed
/// <c>defaultsettings.usd</c> fixture, whose first several tables
/// (AntiExtraBuffSpells, AssistItems, BuffedItems, ExtraBuffSpells,
/// GemFoodItems, …) are already alphabetical. This port's own
/// <see cref="Tables"/> is an insertion-ordered list rather than a
/// sorted structure, so every committed real fixture happens to
/// round-trip in order today purely because it was ALREADY sorted the
/// last time real VTank wrote it — this explicit sort on render makes
/// that invariant a property of the writer itself rather than an
/// accident of whatever order the source data happened to arrive in
/// (no observable behavior change for any fixture today; it only
/// matters the day a future code path adds or reorders a table).
/// </summary>
public string Render()
{
var sb = new StringBuilder();
VtankWriter.AppendLine(sb, Tables.Count.ToString(CultureInfo.InvariantCulture));
foreach ((string name, VtankTable table) in Tables)
var ordered = Tables
.OrderBy(static entry => entry.Name, StringComparer.Ordinal)
.ToArray();
VtankWriter.AppendLine(sb, ordered.Length.ToString(CultureInfo.InvariantCulture));
foreach ((string name, VtankTable table) in ordered)
{
VtankWriter.AppendLine(sb, name);
table.WriteTo(sb);