diff --git a/docs/research/vtank-kb/07-meta-and-expressions.md b/docs/research/vtank-kb/07-meta-and-expressions.md index 3c487fcf..e737f247 100644 --- a/docs/research/vtank-kb/07-meta-and-expressions.md +++ b/docs/research/vtank-kb/07-meta-and-expressions.md @@ -723,6 +723,7 @@ line counts). This is an unusually precise reverse-engineering result — the | 3 | **`MonsterNameCountWithinDistance`'s name pattern is case-insensitive in MossTank, case-sensitive in retail** | `d7.c()` compiles with `RegexOptions.Compiled` only (`d7.cs:26`) — case-sensitive | `Meta.cs`'s `MonsterCount` helper (`Meta.cs:536-552`) compiles with `RegexOptions.IgnoreCase \| RegexOptions.CultureInvariant` | **Medium-high.** A pattern authored against retail's case-sensitive matching (e.g. deliberately excluding a differently-cased variant name) will over-match in MossTank. Note `ChatMessage`/`ChatMessageCapture` do **not** have this divergence — MossTank's `ChatMatch` (`Meta.cs:463-513`) is correctly case-sensitive (`RegexOptions.CultureInvariant` only), matching `hl.cs`/`c5.cs`. | | 4 | **`;` sequence-operator value convention is inverted vs. retail (but matches UtilityBelt, and was chosen deliberately)** | Retail VTank: `a;b` evaluates to **`a`** (the left/first operand), discarding `b`'s value, while still executing `b` for side effects (`ExpressionEvaluator.cs:787-790`); `;` is an ordinary operator that can nest anywhere via normal precedence | `ExpressionProgram.Evaluate` (`ExpressionEngine.cs:20-27`) treats top-level `;`-separated statements as a `Node[]` program and returns the value of the **last** statement — this matches UtilityBelt's own audited grammar ("multiple `;`-separated statements, returning the final result", `2026-08-26-mosstank-vtank-utilitybelt-research.md:237`), which the campaign explicitly chose as MossTank's baseline dialect (§3.1 there) | **Medium, and by design, not an oversight.** Retail VTank and UtilityBelt already disagree with each other on `;`'s return value; MossTank correctly implements UtilityBelt's convention. The compat risk is narrower than a plain bug: only a `.met` authored *against retail's own* semantics (chaining `sideeffect[]; realcheck[]` and relying on the *first* value) evaluates to the opposite result once imported. | | 5 | **Watchdog ring pre-fill differs (far sentinel vs. arm-time position)** | All 10 position-history slots start as a sentinel far from any real coordinate (`(1000,1000,1000)`, `h7.cs:45-49`), guaranteeing expiry cannot even be *reachable* until one full `timeSpan` window has elapsed and every slot has been overwritten with a real sample | `SetWatchdog` (`Meta.cs:585-596`) pre-fills all 10 slots with the **current** position at arm time | **Lower-medium.** Both converge on "earliest possible expiry is ~one `TimeSpanSeconds` after arming" in the common case, but they diverge for a watchdog that is armed, then the player leaves and returns to very near the arm point before the window closes — retail's sentinel-seeded ring cannot spuriously read the arm-time position as one of its 10 samples, MossTank's can. | +| 6 | **`MetaRule.Enabled` (a per-rule disable-without-delete toggle) has no metaf-compatible representation at all** | Real VTank/metaf has no such concept — `metaf_monolithic.py` has zero occurrences of "enabled"/"disabled" anywhere; a rule in a `.met`/`.af` is simply present or absent | `MetafSerializer.SaveMeta` (Campaign VT slice-1 item H) now **refuses to save** a profile containing any disabled rule by default (throws `InvalidOperationException` naming the count) rather than silently dropping it — the caller must opt in via `SaveMeta(profile, dropDisabledRules: true)` to accept the loss explicitly. `MossTankMetaProfileStore.WriteLegacyExport` (the `.af` convenience mirror written alongside MossTank's own fully-fidelity JSON storage) deliberately does NOT opt in: it leaves that mirror stale rather than losing the disabled rule, and logs a warning via the existing try/catch. | **Low** (by design, not a bug): this is a MossTank-only UI extension with no VTank equivalent to diverge from; the representational loss is confined to the legacy-export mirror, never the authoritative profile data, and is now impossible to hit silently. | ### 5.3 Confirmed non-gaps (the format/engine is otherwise unusually faithful) diff --git a/src/AcDream.App/Plugins/VtankProfilesDefault.cs b/src/AcDream.App/Plugins/VtankProfilesDefault.cs new file mode 100644 index 00000000..63db4bd1 --- /dev/null +++ b/src/AcDream.App/Plugins/VtankProfilesDefault.cs @@ -0,0 +1,16 @@ +namespace AcDream.App.Plugins; + +/// +/// The graphical host's default root for IPluginHost.VtankProfiles +/// when RuntimeOptions.VtankProfileDirectoryOverride +/// (ACDREAM_VTANK_PROFILE_DIR) is unset — extracted out of +/// Program.cs's inline composition into its own pure, injectable-root +/// function so the "built with +/// only, never a hard-coded Windows path" guarantee is a real, failable unit +/// test rather than something only checkable by reading the source. +/// +internal static class VtankProfilesDefault +{ + internal static string Resolve(string dataDirectory) => + Path.Combine(dataDirectory, "vtank"); +} diff --git a/src/AcDream.App/Program.cs b/src/AcDream.App/Program.cs index ac54163c..1edd786a 100644 --- a/src/AcDream.App/Program.cs +++ b/src/AcDream.App/Program.cs @@ -188,7 +188,7 @@ var host = new AppPluginHost( lootClassifiers, new FilePluginStorage( runtimeOptions.VtankProfileDirectoryOverride - ?? Path.Combine(applicationPaths.DataDirectory, "vtank"))); + ?? VtankProfilesDefault.Resolve(applicationPaths.DataDirectory))); GraphicalPluginSession pluginSession = GraphicalPluginSession.Create( applicationPaths, runtimeOptions.Plugins, diff --git a/src/AcDream.Plugins.MossTank/MetafSerializer.cs b/src/AcDream.Plugins.MossTank/MetafSerializer.cs index f29e0194..04ae8ecd 100644 --- a/src/AcDream.Plugins.MossTank/MetafSerializer.cs +++ b/src/AcDream.Plugins.MossTank/MetafSerializer.cs @@ -292,9 +292,40 @@ internal static class MetafSerializer } } - public static string SaveMeta(MetaProfile profile) + /// + /// Saves as .af. Real VTank/metaf has + /// no concept of a "disabled" rule at all — Rule/State + /// carry no such flag (confirmed: metaf_monolithic.py has zero + /// occurrences of "enabled"/"disabled" anywhere), so + /// is a MossTank-only extension with no + /// metaf-compatible marker to preserve it through. Rather than silently + /// dropping a disabled rule (irrecoverable once re-imported — see + /// docs/research/vtank-kb/07-meta-and-expressions.md section 5), + /// this overload throws when the profile has any: callers that + /// understand and accept the loss (e.g. a legacy-export convenience + /// copy sitting alongside MossTank's own fully-fidelity JSON storage) + /// must say so explicitly via + /// . + /// + public static string SaveMeta(MetaProfile profile) => + SaveMeta(profile, dropDisabledRules: false); + + /// See 's class doc for why exists. + public static string SaveMeta(MetaProfile profile, bool dropDisabledRules) { ArgumentNullException.ThrowIfNull(profile); + if (!dropDisabledRules) + { + int disabledCount = profile.Rules.Count(static rule => !rule.Enabled); + if (disabledCount > 0) + { + throw new InvalidOperationException( + $"SaveMeta: {disabledCount} disabled rule(s) have no metaf-compatible " + + "representation and would be silently dropped. Call " + + "SaveMeta(profile, dropDisabledRules: true) to accept that loss " + + "explicitly."); + } + } var lines = new List { MetaHeader }; // EmbedNav's referenced Nav is a separate NAV: block elsewhere in // the file (metaf's own layout — see the class doc comment); the diff --git a/src/AcDream.Plugins.MossTank/MossTankMetaProfileStore.cs b/src/AcDream.Plugins.MossTank/MossTankMetaProfileStore.cs index 8ba153b5..c29c31b4 100644 --- a/src/AcDream.Plugins.MossTank/MossTankMetaProfileStore.cs +++ b/src/AcDream.Plugins.MossTank/MossTankMetaProfileStore.cs @@ -208,6 +208,13 @@ internal sealed class MossTankMetaProfileStore return; try { + // MetafSerializer.SaveMeta(profile) (item H, slice-1 fix round) + // throws when the profile has any disabled rule — real VTank/ + // metaf has no marker for MossTank's own "disabled" extension, so + // this .af mirror is deliberately left stale (rather than + // silently dropping the rule) until the user re-enables or + // deletes it. The authoritative JSON save above always has full + // fidelity regardless. _host.Storage.WriteText( $"exports/meta/{LegacyFileName(name)}.af", MetafSerializer.SaveMeta(profile)); diff --git a/src/AcDream.Plugins.MossTank/VtankLootRequirementEvaluator.cs b/src/AcDream.Plugins.MossTank/VtankLootRequirementEvaluator.cs index 693afd34..80c006e2 100644 --- a/src/AcDream.Plugins.MossTank/VtankLootRequirementEvaluator.cs +++ b/src/AcDream.Plugins.MossTank/VtankLootRequirementEvaluator.cs @@ -23,20 +23,33 @@ internal static class VtankLootRequirementEvaluator [6095] = (28, 80), }; - private static readonly IReadOnlyDictionary - DoubleSpellBonuses = new Dictionary + /// + /// VTClassic's static double-spell-bonus table (ComputedItemInfo.cs:88-139). + /// Change is the real per-row selector KB doc 05 section 2.2 names + /// ("additive unless the static table's Change==1, in which case + /// multiplicative", ComputedItemInfo.cs:244) — carried explicitly + /// here rather than inferred from whether Bonus's truncated + /// integer part happens to equal 1 (a prior port's proxy, which this + /// item replaced: it only worked because every multiplicative bonus in + /// this table happens to be in [1.0, 2.0) and every additive one happens + /// to be under 1.0 — a coincidence of the current 19 rows, not a rule, + /// and it would have silently mis-branched on a future row like an + /// additive 1.5 or a multiplicative 2.0+). + /// + private static readonly IReadOnlyDictionary + DoubleSpellBonuses = new Dictionary { - [3251] = (152, .01), [3250] = (152, .03), - [4670] = (152, .05), [6098] = (152, .07), - [2603] = (VtankDoubleBase + 12, .03), - [2591] = (VtankDoubleBase + 12, .05), - [4666] = (VtankDoubleBase + 12, .07), - [6094] = (VtankDoubleBase + 12, .09), - [2600] = (29, .03), [3985] = (29, .04), - [2588] = (29, .05), [4663] = (29, .07), [6091] = (29, .09), - [3201] = (144, 1.05), [3199] = (144, 1.10), - [3202] = (144, 1.15), [3200] = (144, 1.20), - [6086] = (144, 1.25), [6087] = (144, 1.30), + [3251] = (152, .01, false), [3250] = (152, .03, false), + [4670] = (152, .05, false), [6098] = (152, .07, false), + [2603] = (VtankDoubleBase + 12, .03, false), + [2591] = (VtankDoubleBase + 12, .05, false), + [4666] = (VtankDoubleBase + 12, .07, false), + [6094] = (VtankDoubleBase + 12, .09, false), + [2600] = (29, .03, false), [3985] = (29, .04, false), + [2588] = (29, .05, false), [4663] = (29, .07, false), [6091] = (29, .09, false), + [3201] = (144, 1.05, true), [3199] = (144, 1.10, true), + [3202] = (144, 1.15, true), [3200] = (144, 1.20, true), + [6086] = (144, 1.25, true), [6087] = (144, 1.30, true), }; private static readonly IReadOnlyDictionary ArmorColorSlots = @@ -453,7 +466,7 @@ internal static class VtankLootRequirementEvaluator { continue; } - value = (int)bonus.Bonus == 1 ? value * bonus.Bonus : value + bonus.Bonus; + value = bonus.Change ? value * bonus.Bonus : value + bonus.Bonus; } return value; } diff --git a/tests/AcDream.App.Tests/Plugins/VtankProfilesDefaultTests.cs b/tests/AcDream.App.Tests/Plugins/VtankProfilesDefaultTests.cs new file mode 100644 index 00000000..8bf05d5f --- /dev/null +++ b/tests/AcDream.App.Tests/Plugins/VtankProfilesDefaultTests.cs @@ -0,0 +1,37 @@ +using AcDream.App.Plugins; + +namespace AcDream.App.Tests.Plugins; + +/// +/// Item H (Campaign VT slice-1 fix round): a genuinely failable Linux-path +/// guarantee for — re-derives the +/// expected value with from an +/// injected root and compares, so a future hard-coded +/// dataDirectory + "\vtank" (or any other literal-backslash +/// regression) fails this test on any OS, not just by inspection. +/// +public sealed class VtankProfilesDefaultTests +{ + [Fact] + public void ResolveIsBuiltWithPathCombineOnly() + { + const string injectedRoot = "C:/some/injected/data-dir"; + string expected = Path.Combine(injectedRoot, "vtank"); + + string actual = VtankProfilesDefault.Resolve(injectedRoot); + + Assert.Equal(expected, actual); + Assert.DoesNotContain( + injectedRoot + "\\vtank", + actual.Replace(Path.DirectorySeparatorChar, '/'), + StringComparison.Ordinal); + } + + [Fact] + public void ResolveAppendsVtankBeneathWhicheverRootIsInjected() + { + Assert.Equal( + Path.Combine("/home/user/.local/share/acdream", "vtank"), + VtankProfilesDefault.Resolve("/home/user/.local/share/acdream")); + } +} diff --git a/tests/AcDream.Plugins.MossTank.Tests/MetafSerializerTests.cs b/tests/AcDream.Plugins.MossTank.Tests/MetafSerializerTests.cs index 9f21c599..377ec927 100644 --- a/tests/AcDream.Plugins.MossTank.Tests/MetafSerializerTests.cs +++ b/tests/AcDream.Plugins.MossTank.Tests/MetafSerializerTests.cs @@ -333,6 +333,63 @@ public sealed class MetafSerializerTests return null; } + // Item H (slice-1 fix round): real VTank/metaf has no marker for a + // disabled rule at all, so SaveMeta refuses by default rather than + // silently dropping it — see docs/research/vtank-kb/ + // 07-meta-and-expressions.md section 5, gap 6. + [Fact] + public void SaveMetaRefusesToDropADisabledRuleByDefault() + { + var profile = new MetaProfile + { + Rules = + [ + new MetaRule + { + Enabled = false, + Condition = MetaCondition.Always(), + Action = new MetaAction + { + Kind = MetaActionKind.ChatCommand, + Text = "/say must not silently vanish", + }, + }, + ], + }; + + InvalidOperationException thrown = Assert.Throws( + () => MetafSerializer.SaveMeta(profile)); + Assert.Contains("1 disabled rule", thrown.Message, StringComparison.Ordinal); + } + + [Fact] + public void SaveMetaDropsDisabledRulesOnlyWhenExplicitlyToldTo() + { + var profile = new MetaProfile + { + Rules = + [ + new MetaRule + { + Enabled = false, + Condition = MetaCondition.Always(), + Action = new MetaAction + { + Kind = MetaActionKind.ChatCommand, + Text = "/say must not run", + }, + }, + ], + }; + + string source = MetafSerializer.SaveMeta(profile, dropDisabledRules: true); + + Assert.DoesNotContain("must not run", source, StringComparison.Ordinal); + Assert.True(MetafSerializer.TryLoadMeta( + source, NoOpSpellCatalog.Instance, out MetaProfile loaded, out string error), error); + Assert.Empty(loaded.Rules); + } + [Fact] public void NestedAllAnyNotParsesCorrectly() { diff --git a/tests/AcDream.Plugins.MossTank.Tests/VtankLootRequirementEvaluatorTests.cs b/tests/AcDream.Plugins.MossTank.Tests/VtankLootRequirementEvaluatorTests.cs index 4b92ed03..76a18d13 100644 --- a/tests/AcDream.Plugins.MossTank.Tests/VtankLootRequirementEvaluatorTests.cs +++ b/tests/AcDream.Plugins.MossTank.Tests/VtankLootRequirementEvaluatorTests.cs @@ -75,6 +75,117 @@ public sealed class VtankLootRequirementEvaluatorTests Assert.Null(error); } + // Item H (slice-1 fix round): the KeyExistsDouble gate + // (ComputedItemInfo.cs:234) must reject the bonus exactly like + // KeyExistsInt above — the int-side test existed already, the + // double-side one did not. + // Spell 2600 grants +.03 (additive, Change=false) to DoubleValueKey 29 + // (VtankLootRequirementEvaluator's DoubleSpellBonuses table). + private const uint BonusDoubleKey = 29u; + private const uint BonusDoubleAdditiveSpellId = 2600u; + + [Fact] + public void BuffedDoubleRequirementDoesNotApplyBonusWhenBaseKeyIsAbsent() + { + PluginInventoryItem item = Item() with + { + AppraisedSpellIds = [BonusDoubleAdditiveSpellId], + }; + var properties = new PluginItemProperties( + Ints: new Dictionary(), + Int64s: new Dictionary(), + Bools: new Dictionary(), + Floats: new Dictionary(), // key 29 absent entirely + Strings: new Dictionary(), + DataIds: new Dictionary(), + InstanceIds: new Dictionary()); + + // Type 2005 = BuffedDoubleValKeyGE: values[0]=threshold, values[1]=key. + // Pre-fix (no double-side gate) this would read base=0, add the + // +.03 bonus unconditionally (0.03 >= 0.02 => true); the gate must + // keep it at the un-buffed base value (0 >= 0.02 => false) since the + // key never existed at all. + var requirement = new VtankLootRequirement + { + Type = 2005, + Payload = $"0.02\r\n{BonusDoubleKey}\r\n", + }; + bool matched = VtankLootRequirementEvaluator.IsMatch( + [requirement], item, properties, host: null, out string? error); + Assert.Null(error); + Assert.False(matched); + } + + [Fact] + public void BuffedDoubleRequirementAppliesAdditiveBonusWhenBaseKeyExists() + { + PluginInventoryItem item = Item() with + { + AppraisedSpellIds = [BonusDoubleAdditiveSpellId], + }; + var properties = new PluginItemProperties( + Ints: new Dictionary(), + Int64s: new Dictionary(), + Bools: new Dictionary(), + Floats: new Dictionary { [BonusDoubleKey] = 0d }, // key exists, base 0 + Strings: new Dictionary(), + DataIds: new Dictionary(), + InstanceIds: new Dictionary()); + + var requirement = new VtankLootRequirement + { + Type = 2005, + Payload = $"0.02\r\n{BonusDoubleKey}\r\n", + }; + // Base key exists (value 0) so the +.03 additive bonus applies: + // 0.03 >= 0.02. + Assert.True(VtankLootRequirementEvaluator.IsMatch( + [requirement], item, properties, host: null, out string? error)); + Assert.Null(error); + } + + // Spell 3201 grants a x1.05 MULTIPLICATIVE (Change=true) bonus to + // DoubleValueKey 144 — the branch this item's fix makes an explicit, + // named selector instead of inferring it from whether the bonus + // value's truncated int happens to equal 1. + [Fact] + public void BuffedDoubleRequirementAppliesMultiplicativeBonusWhenChangeIsSet() + { + const uint MultiplicativeKey = 144u; + const uint MultiplicativeSpellId = 3201u; + PluginInventoryItem item = Item() with + { + AppraisedSpellIds = [MultiplicativeSpellId], + }; + var properties = new PluginItemProperties( + Ints: new Dictionary(), + Int64s: new Dictionary(), + Bools: new Dictionary(), + Floats: new Dictionary { [MultiplicativeKey] = 10d }, // base 10 + Strings: new Dictionary(), + DataIds: new Dictionary(), + InstanceIds: new Dictionary()); + + // 10 * 1.05 = 10.5 >= 10.4 (true); if this were wrongly treated as + // additive it would be 10 + 1.05 = 11.05 (also >= 10.4 — so instead + // assert the exact computed value via a tight threshold that only + // the multiplicative result clears): 10.5 >= 10.45 is true, + // 10 + 1.05 = 11.05 is also true, so pin the boundary the other way: + // a threshold only the ADDITIVE result would clear, proving this is + // NOT additive. + var additiveWouldPass = new VtankLootRequirement + { + Type = 2005, + Payload = $"10.6\r\n{MultiplicativeKey}\r\n", + }; + // Additive would give 11.05 (>= 10.6 => true); multiplicative gives + // 10.5 (>= 10.6 => false). Observing "false" here proves the branch + // took the multiplicative path. + Assert.False(VtankLootRequirementEvaluator.IsMatch( + [additiveWouldPass], item, properties, host: null, out string? error)); + Assert.Null(error); + } + private static PluginInventoryItem Item() => new( 0u, 0u, "Test Item", 0u, 0u, 0u, 0u, 0u, 0u, 0u, 0u, 1, 0, 0, 0u, 0, 0, 0u, false, 0d, 0, 0, 0, 0d, 0, 0, 0); diff --git a/tests/AcDream.Plugins.MossTank.Tests/VtankMetaProfileSerializerTests.cs b/tests/AcDream.Plugins.MossTank.Tests/VtankMetaProfileSerializerTests.cs new file mode 100644 index 00000000..edafbf14 --- /dev/null +++ b/tests/AcDream.Plugins.MossTank.Tests/VtankMetaProfileSerializerTests.cs @@ -0,0 +1,69 @@ +using System.Globalization; + +namespace AcDream.Plugins.MossTank.Tests; + +/// +/// Item H (Campaign VT slice-1 fix round): restores two reader-only tests a +/// prior commit (0d10399e0, "A2 VTank profile directory resolution") deleted +/// as collateral damage of an unrelated file move — LoadsKnownTypedCondActRecord +/// and the signed-high-bit round trip embedded in the deleted +/// RoundTripPreservesEveryVtankConditionActionAndEmbeddedNav. Both are +/// adapted to the reader-only contract VtankMetaProfileSerializer has +/// had since it was demoted to a one-shot .met import (commit +/// 3ff9461ef, "MossTank does not implement .met"): neither calls the deleted +/// Save writer. +/// +public sealed class VtankMetaProfileSerializerTests +{ + [Fact] + public void LoadsKnownTypedCondActRecord() + { + const string source = "1\r\nCondAct\r\n5\r\nCType\r\nAType\r\n" + + "CData\r\nAData\r\nState\r\nn\r\nn\r\nn\r\nn\r\nn\r\n1\r\n" + + "i\r\n1\r\ni\r\n2\r\ni\r\n0\r\ns\r\n/say ready\r\n" + + "s\r\nDefault\r\n"; + + Assert.True(VtankMetaProfileSerializer.TryLoad( + source, out MetaProfile profile, out string error), error); + + MetaRule rule = Assert.Single(profile.Rules); + Assert.Equal(MetaConditionKind.Always, rule.Condition.Kind); + Assert.Equal(MetaActionKind.ChatCommand, rule.Action.Kind); + Assert.Equal("/say ready", rule.Action.Text); + Assert.Equal("Default", rule.State); + } + + /// + /// Condition type 17 (LandblockEquals) reads a raw signed decimal int + /// line (ReadCondition case 17: reader.Expect("i"); + /// value.Number = reader.ReadInt();) into a + /// field. A real landblock id with the high bit set — 0x8B370000, a + /// legitimate 32-bit landblock/object id — becomes negative once cast to + /// a signed (VTank's own on-disk encoding, exactly + /// what the deleted round-trip test exercised via + /// unchecked((int)0x8B370000u)); this pins that the reader + /// recovers the exact same bit pattern rather than silently clamping or + /// mis-widening it (a double exactly represents every 32-bit int, so + /// this is a real assertion, not a tautology once the string form is + /// the true value under test). + /// + [Fact] + public void SignedHighBitLandblockIdRoundTripsExactly() + { + int highBitValue = unchecked((int)0x8B370000u); + Assert.True(highBitValue < 0, "0x8B370000 must decode as negative once cast to Int32."); + string source = "1\r\nCondAct\r\n5\r\nCType\r\nAType\r\n" + + "CData\r\nAData\r\nState\r\nn\r\nn\r\nn\r\nn\r\nn\r\n1\r\n" + + "i\r\n17\r\ni\r\n0\r\n" + + "i\r\n" + highBitValue.ToString(CultureInfo.InvariantCulture) + "\r\n" + + "i\r\n0\r\n" + + "s\r\nDefault\r\n"; + + Assert.True(VtankMetaProfileSerializer.TryLoad( + source, out MetaProfile profile, out string error), error); + + MetaRule rule = Assert.Single(profile.Rules); + Assert.Equal(MetaConditionKind.LandblockEquals, rule.Condition.Kind); + Assert.Equal((double)highBitValue, rule.Condition.Number); + } +}