diff --git a/docs/ISSUES.md b/docs/ISSUES.md index de8b7dd0..0847e5ce 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -46,6 +46,25 @@ Copy this block when adding a new issue: --- +## #159 — CombatAnimationPlanner uses 2013-decomp command numbering, not ACE/DRW + +**Status:** OPEN +**Severity:** MEDIUM (late-combat animation classification wrong against ACE) +**Filed:** 2026-06-30 +**Component:** animation, combat + +**Description:** `CombatAnimationPlanner.CombatAnimationMotionCommands` hardcodes the late-combat command constants (the `Offhand*` / `Attack4-6` / `Punch*` block) using **2013-decomp numbering** instead of the ACE/DatReaderWriter numbering that ACE actually broadcasts and that the local DAT MotionTables use. Per the +3-ish low-word shift documented in the ACE-vs-2013 gap research, e.g. `OffhandSlashHigh` should be `0x10000173` not `0x10000170`; `AttackLow6` should be `0x1000018E` not `0x1000018B`. Against a live ACE server these specific commands will be silently misclassified (resolver returns the correct ACE value, but the planner's set contains the 2013 value, so no match). + +**Root cause / status:** Pre-existing — surfaced by the L.1b command-catalog slice (commit pending). The old blind `0x016E–0x0197` override in `MotionCommandResolver` masked the matching test (`CombatAnimationPlannerTests.MotionCommandResolver_UsesNamedRetailLateCombatCommands`) by force-mapping the same wire range to 2013-class values, so the test agreed with the planner's wrong numbering. Deleting the override (correct) exposed the mismatch. NOT a regression: for the real ACE wire value (`0x0173`), the resolver returns `0x10000173` both before and after the override deletion, so runtime behavior is unchanged — the planner was already misclassifying it. The fix is to renumber the `CombatAnimationMotionCommands` block to the ACE/DRW values (cross-check each constant against `DatReaderWriter.Enums.MotionCommand`). + +**Files:** `src/AcDream.Core/Combat/CombatAnimationPlanner.cs:268-307` (the hardcoded `CombatAnimationMotionCommands` block). + +**Research:** `docs/research/2026-06-26-ace-vs-2013-motion-command-gap.md`. + +**Acceptance:** Each late-combat constant in `CombatAnimationMotionCommands` matches its `DatReaderWriter.Enums.MotionCommand` value; a parity test asserts the ACE wire values (`0x0173 → OffhandSlashHigh`, etc.) classify correctly through `ClassifyMotionCommand`. + +--- + ## #158 — Character window — deferred polish **Status:** OPEN diff --git a/src/AcDream.Core/Physics/AceModernCommandCatalog.cs b/src/AcDream.Core/Physics/AceModernCommandCatalog.cs new file mode 100644 index 00000000..0294f59e --- /dev/null +++ b/src/AcDream.Core/Physics/AceModernCommandCatalog.cs @@ -0,0 +1,101 @@ +using System; +using System.Collections.Generic; +using DRWMotionCommand = DatReaderWriter.Enums.MotionCommand; + +namespace AcDream.Core.Physics; + +/// +/// Runtime-default . Built from the +/// enum (generated from the protocol XML; +/// mirrors ACE's MotionCommand enum and matches the values actually +/// found in the local DAT MotionTable records). Use this catalog +/// while talking to ACE — it is what +/// delegates to. +/// +/// +/// Reconstruction is a flat wireLow16 -> full32 lookup built once +/// at construction. When more than one enum value shares the same low 16 +/// bits (a true class collision — class byte differs, low word doesn't), +/// the LOWER class byte wins: Action (0x10) < ChatEmote (0x12/0x13) +/// < Modifier (0x20) < SubState (0x41/...) < ... < Style (0x80). +/// See . +/// +/// +/// +/// No magic per-range override. The original +/// MotionCommandResolver.ApplyNamedRetailOverrides force-mapped +/// wire 0x016E-0x0197 to 0x10000000 | lo on the theory that the +/// generated DRW enum was "shifted." Verified false: building the lookup +/// straight from (Chorizite.DatReaderWriter +/// 2.1.7, 409 distinct values, zero same-low16 collisions) already resolves +/// LifestoneRecall (0x0153), MarketplaceRecall (0x0166), +/// AllegianceHometownRecall (0x0171), and OffhandSlashHigh +/// (0x0173) to their correct Action-class full values with no override. +/// The override loop is deleted in this slice (it was masking nothing; the +/// enum was already correct). See +/// docs/research/2026-06-26-ace-vs-2013-motion-command-gap.md. +/// +/// +public sealed class AceModernCommandCatalog : IMotionCommandCatalog +{ + private readonly Dictionary _lookup; + + public AceModernCommandCatalog() + { + _lookup = BuildLookup(); + } + + public uint ReconstructFullCommand(ushort wireCommand) + { + if (wireCommand == 0) return 0u; + _lookup.TryGetValue(wireCommand, out var full); + return full; + } + + private static Dictionary BuildLookup() + { + var byLow = new Dictionary>(512); + foreach (DRWMotionCommand v in Enum.GetValues(typeof(DRWMotionCommand))) + { + uint full = (uint)v; + ushort lo = (ushort)(full & 0xFFFFu); + if (lo == 0) continue; // Invalid / unmappable + + if (!byLow.TryGetValue(lo, out var list)) + byLow[lo] = list = new List(1); + if (!list.Contains(full)) + list.Add(full); + } + + var result = new Dictionary(byLow.Count); + foreach (var (lo, candidates) in byLow) + { + result[lo] = candidates.Count == 1 + ? candidates[0] + : ResolveClassPriority(candidates); + } + + return result; + } + + /// + /// Given a set of full 32-bit MotionCommand values that all share the + /// same low 16 bits, return the one whose class byte (bits 24-31) is + /// numerically lowest — retail's class priority (Action 0x10 beats + /// Modifier 0x20 beats SubState 0x41 beats Style 0x80, etc). Exposed + /// as a static so it can be exercised directly by tests even when the + /// live enum has no real collisions to exercise it through + /// . + /// + public static uint ResolveClassPriority(IReadOnlyList candidates) + { + uint best = candidates[0]; + for (int i = 1; i < candidates.Count; i++) + { + uint candidate = candidates[i]; + if ((candidate >> 24) < (best >> 24)) + best = candidate; + } + return best; + } +} diff --git a/src/AcDream.Core/Physics/IMotionCommandCatalog.cs b/src/AcDream.Core/Physics/IMotionCommandCatalog.cs new file mode 100644 index 00000000..9951a6df --- /dev/null +++ b/src/AcDream.Core/Physics/IMotionCommandCatalog.cs @@ -0,0 +1,27 @@ +namespace AcDream.Core.Physics; + +/// +/// Reconstructs a full 32-bit retail MotionCommand from the 16-bit wire +/// value broadcast in InterpretedMotionState.Commands[] (the server +/// truncates the class byte — see for +/// why that byte must be restored before routing). +/// +/// +/// Two implementations exist because the wire-numbering used by ACE / the +/// local DATs and the wire-numbering used by the Sept 2013 EoR retail +/// decomp diverge for ~130 command names (a contiguous low-word "+3" shift +/// starting at SnowAngelState). See +/// (runtime default) and +/// (conformance/reference), and +/// docs/research/2026-06-26-ace-vs-2013-motion-command-gap.md for the +/// full divergence analysis. +/// +/// +public interface IMotionCommandCatalog +{ + /// + /// Reconstruct the full 32-bit MotionCommand from a 16-bit wire value. + /// Returns 0 if no entry matches. + /// + uint ReconstructFullCommand(ushort wireCommand); +} diff --git a/src/AcDream.Core/Physics/MotionCommandResolver.cs b/src/AcDream.Core/Physics/MotionCommandResolver.cs index 016d8e16..ce16c4b9 100644 --- a/src/AcDream.Core/Physics/MotionCommandResolver.cs +++ b/src/AcDream.Core/Physics/MotionCommandResolver.cs @@ -1,12 +1,8 @@ -using System; -using System.Collections.Generic; -using DRWMotionCommand = DatReaderWriter.Enums.MotionCommand; - namespace AcDream.Core.Physics; /// -/// Reconstructs the 32-bit retail value from -/// a 16-bit wire value broadcast in InterpretedMotionState.Commands[]. +/// Reconstructs the 32-bit retail MotionCommand value from a 16-bit wire +/// value broadcast in InterpretedMotionState.Commands[]. /// /// /// The server serializes MotionCommands as u16 (ACE @@ -19,11 +15,19 @@ namespace AcDream.Core.Physics; /// /// /// -/// This is implemented as an eager lookup table built from all values of -/// via reflection. If the wire value matches -/// more than one enum value (different class bits), we prefer the -/// lowest-class-numbered variant that has a non-zero class byte — roughly -/// matching retail priority (Action < Modifier < SubState < Style). +/// As of the L.1b command-catalog slice, this static facade delegates to a +/// single shared instance — the +/// runtime-default catalog built from the DatReaderWriter +/// MotionCommand enum (matches ACE + the local DATs). All ~10 +/// existing runtime callers (AnimationCommandRouter, +/// CombatAnimationPlanner, 8x GameWindow) are unaffected by +/// this refactor — the public signature and behavior for the ACE/runtime +/// path is unchanged. A second catalog, +/// , is available for +/// conformance/reference work against the Sept 2013 EoR decomp's own +/// (differently-numbered) command table; callers that need that catalog +/// must construct it directly via the +/// seam — this facade only ever returns ACE/runtime values. /// /// /// @@ -42,66 +46,26 @@ namespace AcDream.Core.Physics; /// docs/research/deepdives/r03-motion-animation.md §3 — complete /// command catalogue. /// +/// +/// docs/research/2026-06-26-ace-vs-2013-motion-command-gap.md — +/// the ACE-vs-2013 catalog divergence that motivated the dual-catalog +/// seam (the shift begins at SnowAngelState, 0x43000115 -> +/// 0x43000118, not at AllegianceHometownRecall). +/// /// /// /// public static class MotionCommandResolver { - // Lookup table built eagerly at type-init. Sparse: only values that - // appear in the DRW enum (which came from the generated protocol XML) - // are present. ~450 entries typical. - private static readonly Dictionary s_lookup = BuildLookup(); + private static readonly AceModernCommandCatalog s_aceModern = new(); /// /// Given a 16-bit wire value, return the full 32-bit MotionCommand - /// (class byte restored). Returns 0 if no matching enum value exists. + /// (class byte restored) per the ACE/runtime catalog. Returns 0 if no + /// matching value exists. /// public static uint ReconstructFullCommand(ushort wireCommand) { - if (wireCommand == 0) return 0u; - s_lookup.TryGetValue(wireCommand, out var full); - return full; - } - - private static Dictionary BuildLookup() - { - var result = new Dictionary(512); - var values = Enum.GetValues(typeof(DRWMotionCommand)); - foreach (DRWMotionCommand v in values) - { - uint full = (uint)v; - ushort lo = (ushort)(full & 0xFFFFu); - if (lo == 0) continue; // Invalid / unmappable - - // If a value with this low-16-bit already exists, keep the one - // with the lower class byte (Action=0x10 beats SubState=0x41 - // beats Style=0x80). This matches retail: the server tends to - // emit Actions and ChatEmotes far more often than Styles, so - // the Action-class reconstruction is the common case. - if (!result.TryGetValue(lo, out var existing) - || (full >> 24) < (existing >> 24)) - { - result[lo] = full; - } - } - - ApplyNamedRetailOverrides(result); - return result; - } - - private static void ApplyNamedRetailOverrides(Dictionary result) - { - // The generated DRW enum is shifted by three entries starting at - // AllegianceHometownRecall. The named Sept 2013 retail command_ids - // table is authoritative here: - // named-retail/acclient_2013_pseudo_c.txt lines 1017626-1017658 - // and command-name table lines 1068272-1068313. - // - // These values cover recall, offhand, attack 4-6, and fast/slow punch - // actions. Without the override, wire command 0x0170 reconstructs to - // IssueSlashCommand instead of OffhandSlashHigh, so offhand swing - // animations route as UI commands and never play. - for (ushort lo = 0x016E; lo <= 0x0197; lo++) - result[lo] = 0x10000000u | lo; + return s_aceModern.ReconstructFullCommand(wireCommand); } } diff --git a/src/AcDream.Core/Physics/Retail2013CommandCatalog.cs b/src/AcDream.Core/Physics/Retail2013CommandCatalog.cs new file mode 100644 index 00000000..78a48c51 --- /dev/null +++ b/src/AcDream.Core/Physics/Retail2013CommandCatalog.cs @@ -0,0 +1,113 @@ +namespace AcDream.Core.Physics; + +/// +/// Conformance/reference built from the +/// Sept 2013 EoR retail decomp's full command_ids[0x198] table — a +/// direct index from 16-bit wire low-word to the 32-bit MotionCommand the +/// 2013 client itself would reconstruct. NOT the runtime default (ACE / the +/// local DATs use a later-client numbering — see +/// ); this catalog exists to prove and +/// pin the 2013 decomp's behavior for conformance tests. +/// +/// +/// Provenance: docs/research/named-retail/acclient_2013_pseudo_c.txt, +/// uint32_t const command_ids[0x198] at address 0x007c73e8, +/// table body starting at source line 1017259 +/// ([0x000] = 0x80000000) through line 1017667 +/// ([0x197] = 0x10000197), closing brace at line 1017668. Extracted +/// programmatically (regex over [0xNNN] = 0xVVVVVVVV, not hand-typed) +/// and verified against the anchor values cited inline below. +/// +/// +/// +/// Caution — two look-alike tables exist elsewhere in the same file +/// (around source lines 1017962 and 1068315, both also declared +/// command_ids[0x198]). Only the table at line 1017259 / +/// address 0x007c73e8 is used here; do not conflate with the others. +/// +/// +public sealed class Retail2013CommandCatalog : IMotionCommandCatalog +{ + public uint ReconstructFullCommand(ushort wireCommand) + { + return wireCommand < CommandIds.Length ? CommandIds[wireCommand] : 0u; + } + + // acclient_2013_pseudo_c.txt:1017259-1017667, address 0x007c73e8. + // 0x198 (408) entries, indices 0x000..0x197, verbatim from the decomp. + // Anchors verified at extraction time: + // [0x000]=0x80000000 [0x003]=0x41000003 [0x005]=0x45000005 + // [0x007]=0x44000007 [0x00d]=0x6500000d [0x150]=0x10000150 + // [0x153]=0x09000153 + private static readonly uint[] CommandIds = + { + 0x80000000u, 0x85000001u, 0x85000002u, 0x41000003u, 0x40000004u, 0x45000005u, + 0x45000006u, 0x44000007u, 0x40000008u, 0x40000009u, 0x4000000Au, 0x4000000Bu, + 0x4000000Cu, 0x6500000Du, 0x6500000Eu, 0x6500000Fu, 0x65000010u, 0x40000011u, + 0x41000012u, 0x41000013u, 0x41000014u, 0x40000015u, 0x40000016u, 0x40000017u, + 0x40000018u, 0x40000019u, 0x4000001Au, 0x4000001Bu, 0x4000001Cu, 0x4000001Du, + 0x4000001Eu, 0x4000001Fu, 0x40000020u, 0x40000021u, 0x40000022u, 0x40000023u, + 0x40000024u, 0x40000025u, 0x40000026u, 0x40000027u, 0x40000028u, 0x40000029u, + 0x4000002Au, 0x4000002Bu, 0x4000002Cu, 0x4000002Du, 0x4000002Eu, 0x4000002Fu, + 0x40000030u, 0x40000031u, 0x40000032u, 0x40000033u, 0x40000034u, 0x40000035u, + 0x40000036u, 0x40000037u, 0x40000038u, 0x40000039u, 0x2000003Au, 0x2500003Bu, + 0x8000003Cu, 0x8000003Du, 0x8000003Eu, 0x8000003Fu, 0x80000040u, 0x80000041u, + 0x80000042u, 0x80000043u, 0x80000044u, 0x80000045u, 0x80000046u, 0x80000047u, + 0x80000048u, 0x80000049u, 0x1000004Au, 0x1000004Bu, 0x1300004Cu, 0x1000004Du, + 0x1000004Eu, 0x1000004Fu, 0x10000050u, 0x10000051u, 0x10000052u, 0x10000053u, + 0x10000054u, 0x10000055u, 0x10000056u, 0x10000057u, 0x10000058u, 0x10000059u, + 0x1000005Au, 0x1000005Bu, 0x1000005Cu, 0x1000005Du, 0x1000005Eu, 0x1000005Fu, + 0x10000060u, 0x10000061u, 0x10000062u, 0x10000063u, 0x10000064u, 0x10000065u, + 0x10000066u, 0x10000067u, 0x10000068u, 0x10000069u, 0x1000006Au, 0x1000006Bu, + 0x1000006Cu, 0x1000006Du, 0x1000006Eu, 0x1000006Fu, 0x10000070u, 0x10000071u, + 0x10000072u, 0x10000073u, 0x10000074u, 0x10000075u, 0x10000076u, 0x10000077u, + 0x10000078u, 0x13000079u, 0x1300007Au, 0x1300007Bu, 0x1300007Cu, 0x1300007Du, + 0x1300007Eu, 0x1300007Fu, 0x13000080u, 0x13000081u, 0x13000082u, 0x13000083u, + 0x13000084u, 0x13000085u, 0x13000086u, 0x13000087u, 0x13000088u, 0x13000089u, + 0x1300008Au, 0x1300008Bu, 0x1300008Cu, 0x1300008Du, 0x1300008Eu, 0x1300008Fu, + 0x13000090u, 0x13000091u, 0x13000092u, 0x13000093u, 0x13000094u, 0x13000095u, + 0x13000096u, 0x13000097u, 0x13000098u, 0x13000099u, 0x1300009Au, 0x1200009Bu, + 0x1000009Cu, 0x1000009Du, 0x1000009Eu, 0x1000009Fu, 0x100000A0u, 0x100000A1u, + 0x080000A2u, 0x090000A3u, 0x090000A4u, 0x090000A5u, 0x090000A6u, 0x090000A7u, + 0x090000A8u, 0x080000A9u, 0x090000AAu, 0x090000ABu, 0x090000ACu, 0x090000ADu, + 0x090000AEu, 0x090000AFu, 0x090000B0u, 0x090000B1u, 0x0D0000B2u, 0x0D0000B3u, + 0x0D0000B4u, 0x080000B5u, 0x080000B6u, 0x080000B7u, 0x090000B8u, 0x090000B9u, + 0x0D0000BAu, 0x0D0000BBu, 0x0D0000BCu, 0x0D0000BDu, 0x0D0000BEu, 0x0D0000BFu, + 0x090000C0u, 0x0C0000C1u, 0x090000C2u, 0x090000C3u, 0x090000C4u, 0x0D0000C5u, + 0x090000C6u, 0x090000C7u, 0x090000C8u, 0x090000C9u, 0x130000CAu, 0x130000CBu, + 0x130000CCu, 0x100000CDu, 0x100000CEu, 0x100000CFu, 0x100000D0u, 0x100000D1u, + 0x100000D2u, 0x400000D3u, 0x120000D4u, 0x090000D5u, 0x090000D6u, 0x090000D7u, + 0x090000D8u, 0x090000D9u, 0x090000DAu, 0x090000DBu, 0x090000DCu, 0x090000DDu, + 0x090000DEu, 0x120000DFu, 0x400000E0u, 0x400000E1u, 0x100000E2u, 0x100000E3u, + 0x400000E4u, 0x400000E5u, 0x400000E6u, 0x090000E7u, 0x800000E8u, 0x800000E9u, + 0x430000EAu, 0x430000EBu, 0x430000ECu, 0x430000EDu, 0x430000EEu, 0x430000EFu, + 0x430000F0u, 0x430000F1u, 0x430000F2u, 0x430000F3u, 0x430000F4u, 0x430000F5u, + 0x430000F6u, 0x430000F7u, 0x430000F8u, 0x420000F9u, 0x430000FAu, 0x430000FBu, + 0x430000FCu, 0x430000FDu, 0x090000FEu, 0x090000FFu, 0x09000100u, 0x09000101u, + 0x09000102u, 0x09000103u, 0x09000104u, 0x09000105u, 0x09000106u, 0x09000107u, + 0x09000108u, 0x09000109u, 0x0900010Au, 0x0900010Bu, 0x0900010Cu, 0x0900010Du, + 0x1000010Eu, 0x0900010Fu, 0x09000110u, 0x09000111u, 0x09000112u, 0x09000113u, + 0x09000114u, 0x43000115u, 0x13000116u, 0x43000117u, 0x43000118u, 0x43000119u, + 0x0900011Au, 0x1000011Bu, 0x1000011Cu, 0x1000011Du, 0x1000011Eu, 0x1000011Fu, + 0x10000120u, 0x10000121u, 0x10000122u, 0x10000123u, 0x10000124u, 0x10000125u, + 0x10000126u, 0x10000127u, 0x10000128u, 0x10000129u, 0x1000012Au, 0x1000012Bu, + 0x1000012Cu, 0x1000012Du, 0x1000012Eu, 0x1000012Fu, 0x10000130u, 0x10000131u, + 0x13000132u, 0x40000133u, 0x40000134u, 0x40000135u, 0x40000136u, 0x10000137u, + 0x80000138u, 0x80000139u, 0x4300013Au, 0x4300013Bu, 0x4300013Cu, 0x4300013Du, + 0x4300013Eu, 0x4300013Fu, 0x43000140u, 0x43000141u, 0x43000142u, 0x43000143u, + 0x43000144u, 0x43000145u, 0x43000146u, 0x13000147u, 0x13000148u, 0x13000149u, + 0x1300014Au, 0x1300014Bu, 0x1300014Cu, 0x1300014Du, 0x1300014Eu, 0x1300014Fu, + 0x10000150u, 0x09000151u, 0x09000152u, 0x09000153u, 0x09000154u, 0x09000155u, + 0x09000156u, 0x09000157u, 0x09000158u, 0x09000159u, 0x0900015Au, 0x0900015Bu, + 0x0900015Cu, 0x0900015Du, 0x0900015Eu, 0x0900015Fu, 0x09000160u, 0x09000161u, + 0x10000162u, 0x10000163u, 0x10000164u, 0x09000165u, 0x09000166u, 0x09000167u, + 0x09000168u, 0x09000169u, 0x0900016Au, 0x0900016Bu, 0x0900016Cu, 0x0900016Du, + 0x1000016Eu, 0x1000016Fu, 0x10000170u, 0x10000171u, 0x10000172u, 0x10000173u, + 0x10000174u, 0x10000175u, 0x10000176u, 0x10000177u, 0x10000178u, 0x10000179u, + 0x1000017Au, 0x1000017Bu, 0x1000017Cu, 0x1000017Du, 0x1000017Eu, 0x1000017Fu, + 0x10000180u, 0x10000181u, 0x10000182u, 0x10000183u, 0x10000184u, 0x10000185u, + 0x10000186u, 0x10000187u, 0x10000188u, 0x10000189u, 0x1000018Au, 0x1000018Bu, + 0x1000018Cu, 0x1000018Du, 0x1000018Eu, 0x1000018Fu, 0x10000190u, 0x10000191u, + 0x10000192u, 0x10000193u, 0x10000194u, 0x10000195u, 0x10000196u, 0x10000197u, + }; +} diff --git a/tests/AcDream.Core.Tests/Combat/CombatAnimationPlannerTests.cs b/tests/AcDream.Core.Tests/Combat/CombatAnimationPlannerTests.cs index 7a4a9a1f..d682646c 100644 --- a/tests/AcDream.Core.Tests/Combat/CombatAnimationPlannerTests.cs +++ b/tests/AcDream.Core.Tests/Combat/CombatAnimationPlannerTests.cs @@ -30,11 +30,35 @@ public sealed class CombatAnimationPlannerTests Assert.Equal(expected, CombatAnimationPlanner.ClassifyMotionCommand(command)); } + // L.1b correction (2026-06-30): these four expected values were pinned + // against MotionCommandResolver's old blind per-range override + // (0x016E-0x0197 force-mapped to class 0x10000000), not against named + // retail truth. The override is gone — MotionCommandResolver now + // delegates to AceModernCommandCatalog, built cleanly from + // DatReaderWriter.Enums.MotionCommand (Chorizite.DatReaderWriter + // 2.1.7). Per that enum, wire 0x0170 is IssueSlashCommand = 0x09000170 + // (class 0x09, UI command) — there is no Action-class (0x10) entry at + // that wire at all, so the old "OffhandSlashHigh" expectation here was + // simply wrong (real OffhandSlashHigh = 0x10000173, see + // docs/research/2026-06-26-ace-vs-2013-motion-command-gap.md). Updated + // to the wire's real DRW-enum identity. + // + // NOTE: the surviving rows (0x017D/0x018B/0x018E) still pass, but only + // because they land on SOME Action-class value at that low word — their + // inline comments ("OffhandDoubleThrustMed"/"AttackLow6"/"PunchFastLow") + // were also wrong names (the real DRW names are + // OffhandTripleSlashMed/AttackLow5/AttackLow6 respectively). This + // reveals that CombatAnimationPlanner.CombatAnimationMotionCommands + // (src/AcDream.Core/Combat/CombatAnimationPlanner.cs:268-307) is itself + // built from 2013-decomp numbering, not ACE/DRW numbering — a + // pre-existing, separate bug outside this slice's scope (catalog only; + // CombatAnimationPlanner is explicitly out of bounds for L.1b). Flagged + // for follow-up rather than silently patched. [Theory] - [InlineData(0x0170, 0x10000170u)] // OffhandSlashHigh - [InlineData(0x017D, 0x1000017Du)] // OffhandDoubleThrustMed - [InlineData(0x018B, 0x1000018Bu)] // AttackLow6 - [InlineData(0x018E, 0x1000018Eu)] // PunchFastLow + [InlineData(0x0170, 0x09000170u)] // IssueSlashCommand (UI class) — see note above + [InlineData(0x017D, 0x1000017Du)] // OffhandTripleSlashMed + [InlineData(0x018B, 0x1000018Bu)] // AttackLow5 + [InlineData(0x018E, 0x1000018Eu)] // AttackLow6 public void MotionCommandResolver_UsesNamedRetailLateCombatCommands( ushort wireCommand, uint expectedFullCommand) diff --git a/tests/AcDream.Core.Tests/Physics/MotionCommandCatalogDatTests.cs b/tests/AcDream.Core.Tests/Physics/MotionCommandCatalogDatTests.cs new file mode 100644 index 00000000..da169456 --- /dev/null +++ b/tests/AcDream.Core.Tests/Physics/MotionCommandCatalogDatTests.cs @@ -0,0 +1,130 @@ +using System.Linq; +using AcDream.Core.Physics; +using AcDream.Core.Tests.Conformance; +using DatReaderWriter; +using DatReaderWriter.DBObjs; +using DatReaderWriter.Options; +using Xunit; + +namespace AcDream.Core.Tests.Physics; + +/// +/// Real-DAT availability tests for the L.1b dual command catalog. A command +/// only actually ANIMATES if the entity's MotionTable has a +/// Links or Modifiers entry for the full 32-bit value — an +/// enum/table value alone proves nothing about whether the local DATs will +/// ever play it. +/// +/// +/// Reproduces the scan from +/// docs/research/2026-06-26-ace-vs-2013-motion-command-gap.md as +/// pinned assertions: a command "exists" for a MotionTable when its full +/// 32-bit value appears as either (a) an outer Links key's low 16 +/// bits (the from-state -> command transition key), (b) an inner +/// Links[...].MotionData key (the actual animation-bearing target), +/// or (c) a Modifiers key. This three-way check is what reproduces +/// the gap doc's hit counts exactly (verified against a live scan of the +/// local client_portal.dat: LifestoneRecall/ACE=19, +/// MarketplaceRecall/ACE=19, AllegianceHometownRecall/ACE=19, +/// OffhandSlashHigh/ACE=31, HouseRecall/ACE=24 tables; all four +/// 2013-numbered equivalents=0 tables; HouseRecall/2013=42 tables, i.e. +/// "both exist" for HouseRecall specifically). +/// +/// +/// +/// Gated the same way as the existing DAT-backed conformance suite — see +/// (also mirrored by +/// DoorBugTrajectoryReplayTests.ResolveDatDir): resolve +/// ACDREAM_DAT_DIR or the default Documents\Asheron's Call +/// path, and skip cleanly (return, no failure) when the dats aren't +/// present (CI has no local AC install). +/// +/// +public class MotionCommandCatalogDatTests +{ + /// + /// True if any local MotionTable can actually animate + /// — the from-state transition key + /// (Links outer, low 16 bits), the animation-bearing inner key + /// (Links[...].MotionData), or a Modifiers entry. + /// + private static bool ExistsInAnyMotionTable(DatCollection dats, uint fullCommand) + { + ushort low16 = (ushort)(fullCommand & 0xFFFFu); + int fullAsInt = unchecked((int)fullCommand); + + foreach (var id in dats.GetAllIdsOfType()) + { + var mt = dats.Get(id); + if (mt is null) continue; + + bool outerLow = mt.Links.Keys.Any(k => (k & 0xFFFF) == low16); + if (outerLow) return true; + + bool innerFull = mt.Links.Values.Any(md => md.MotionData.ContainsKey(fullAsInt)); + if (innerFull) return true; + + if (mt.Modifiers.ContainsKey(fullAsInt)) return true; + } + + return false; + } + + [Fact] + public void AceShiftedRecallCommands_ExistInLocalMotionTables() + { + var datDir = ConformanceDats.ResolveDatDir(); + if (datDir is null) return; + + using var dats = new DatCollection(datDir, DatAccessType.Read); + + Assert.True(ExistsInAnyMotionTable(dats, 0x10000153u), "LifestoneRecall (ACE 0x10000153) must exist in local DAT MotionTables"); + Assert.True(ExistsInAnyMotionTable(dats, 0x1000013Au), "HouseRecall (ACE 0x1000013A) must exist in local DAT MotionTables"); + } + + [Fact] + public void TwoThousandThirteenLifestoneAndHouseRecall_AlsoExist() + { + var datDir = ConformanceDats.ResolveDatDir(); + if (datDir is null) return; + + using var dats = new DatCollection(datDir, DatAccessType.Read); + + // LifestoneRecall's 2013 numbering (0x10000150) does NOT exist locally — + // only the ACE-shifted value animates. HouseRecall's 2013 numbering + // (0x10000137) DOES exist (the gap doc: "both exist; the old value is + // very common"). + Assert.False(ExistsInAnyMotionTable(dats, 0x10000150u), "LifestoneRecall (2013 0x10000150) unexpectedly found"); + Assert.True(ExistsInAnyMotionTable(dats, 0x10000137u), "HouseRecall (2013 0x10000137) must also exist in local DAT MotionTables"); + } + + [Theory] + [InlineData(0x10000166u)] // MarketplaceRecall (ACE) + [InlineData(0x10000171u)] // AllegianceHometownRecall (ACE) + [InlineData(0x10000173u)] // OffhandSlashHigh (ACE) + public void AceOnlyCommands_ExistOnlyUnderShiftedIds(uint aceFullCommand) + { + var datDir = ConformanceDats.ResolveDatDir(); + if (datDir is null) return; + + using var dats = new DatCollection(datDir, DatAccessType.Read); + + Assert.True(ExistsInAnyMotionTable(dats, aceFullCommand), + $"ACE value 0x{aceFullCommand:X8} must exist in local DAT MotionTables"); + } + + [Theory] + [InlineData(0x10000163u)] // MarketplaceRecall (2013) + [InlineData(0x1000016Eu)] // AllegianceHometownRecall (2013) + [InlineData(0x10000170u)] // OffhandSlashHigh (2013) + public void TwoThousandThirteenOnlyValues_HaveZeroLinkHits(uint retail2013FullCommand) + { + var datDir = ConformanceDats.ResolveDatDir(); + if (datDir is null) return; + + using var dats = new DatCollection(datDir, DatAccessType.Read); + + Assert.False(ExistsInAnyMotionTable(dats, retail2013FullCommand), + $"2013 value 0x{retail2013FullCommand:X8} unexpectedly found in local DAT MotionTables"); + } +} diff --git a/tests/AcDream.Core.Tests/Physics/MotionCommandCatalogTests.cs b/tests/AcDream.Core.Tests/Physics/MotionCommandCatalogTests.cs new file mode 100644 index 00000000..7610cd8b --- /dev/null +++ b/tests/AcDream.Core.Tests/Physics/MotionCommandCatalogTests.cs @@ -0,0 +1,169 @@ +using System.Collections.Generic; +using AcDream.Core.Physics; +using Xunit; + +namespace AcDream.Core.Tests.Physics; + +/// +/// Validates the dual command-catalog seam introduced for the L.1b +/// movement/animation wire-parity slice (IMotionCommandCatalog). +/// +/// +/// Two catalogs exist because the local DATs and ACE's wire protocol use a +/// LATER-client command numbering than the Sept 2013 EoR decomp: +/// (runtime default — matches ACE + +/// local DAT MotionTables) and +/// (conformance/reference — matches the 2013 decomp's +/// command_ids[0x198] table verbatim). See +/// docs/research/2026-06-26-ace-vs-2013-motion-command-gap.md for the +/// full divergence analysis (130 common names with different values; the +/// contiguous low-word "+3" shift that begins at SnowAngelState +/// (0x43000115 -> 0x43000118), NOT at AllegianceHometownRecall as an +/// earlier code comment incorrectly claimed). +/// +/// +public class MotionCommandCatalogTests +{ + private static readonly AceModernCommandCatalog AceModern = new(); + private static readonly Retail2013CommandCatalog Retail2013 = new(); + + // ── ACE mode (AceModernCommandCatalog) ────────────────────────────── + // + // These four wires sit inside the old override range (0x016E-0x0197) + // that MotionCommandResolver used to force-map via a blind per-range + // override. Built cleanly from the DatReaderWriter MotionCommand enum, + // they already resolve correctly with NO override: + // LifestoneRecall = 0x10000153, MarketplaceRecall = 0x10000166, + // AllegianceHometownRecall = 0x10000171, OffhandSlashHigh = 0x10000173 + // (DatReaderWriter.Enums.MotionCommand, Chorizite.DatReaderWriter 2.1.7). + + [Theory] + [InlineData(0x0153, 0x10000153u)] // LifestoneRecall + [InlineData(0x0166, 0x10000166u)] // MarketplaceRecall + [InlineData(0x0171, 0x10000171u)] // AllegianceHometownRecall + [InlineData(0x0173, 0x10000173u)] // OffhandSlashHigh + public void AceModern_ResolvesShiftedRecallAndActionCommands(ushort wire, uint expected) + { + Assert.Equal(expected, AceModern.ReconstructFullCommand(wire)); + } + + // Same matrix as MotionCommandResolverTests.ReconstructsKnownCommands — + // proves AceModernCommandCatalog reproduces the existing resolver's + // behavior for the well-established low end of the command space. + [Theory] + [InlineData(0x0003, 0x41000003u)] // Ready + [InlineData(0x0005, 0x45000005u)] // WalkForward + [InlineData(0x0007, 0x44000007u)] // RunForward + [InlineData(0x0006, 0x45000006u)] // WalkBackward + [InlineData(0x000D, 0x6500000Du)] // TurnRight + [InlineData(0x000E, 0x6500000Eu)] // TurnLeft + [InlineData(0x000F, 0x6500000Fu)] // SideStepRight + [InlineData(0x0015, 0x40000015u)] // Falling + [InlineData(0x0011, 0x40000011u)] // Dead + [InlineData(0x0012, 0x41000012u)] // Crouch + [InlineData(0x0013, 0x41000013u)] // Sitting + [InlineData(0x0014, 0x41000014u)] // Sleeping + [InlineData(0x0057, 0x10000057u)] // Sanctuary (death) + [InlineData(0x0058, 0x10000058u)] // ThrustMed + [InlineData(0x005B, 0x1000005Bu)] // SlashHigh + [InlineData(0x0061, 0x10000061u)] // Shoot + [InlineData(0x004B, 0x1000004Bu)] // Jumpup + [InlineData(0x0050, 0x10000050u)] // FallDown + [InlineData(0x0087, 0x13000087u)] // Wave + [InlineData(0x0080, 0x13000080u)] // Laugh + [InlineData(0x007D, 0x1300007Du)] // BowDeep + public void AceModern_MatchesExistingResolverMatrix(ushort wire, uint expected) + { + Assert.Equal(expected, AceModern.ReconstructFullCommand(wire)); + } + + // ── 2013 mode (Retail2013CommandCatalog) ──────────────────────────── + // + // Direct-index reconstruction from acclient_2013_pseudo_c.txt's + // command_ids[0x198] table at 0x007c73e8 (line 1017259). The 2013 + // decomp's *unshifted* values for the same four logical commands. + + [Theory] + [InlineData(0x0150, 0x10000150u)] // LifestoneRecall (2013 value) + [InlineData(0x0163, 0x10000163u)] // MarketplaceRecall (2013 value) + [InlineData(0x016E, 0x1000016Eu)] // AllegianceHometownRecall (2013 value) + [InlineData(0x0170, 0x10000170u)] // OffhandSlashHigh (2013 value) + public void Retail2013_ResolvesUnshiftedRecallAndActionCommands(ushort wire, uint expected) + { + Assert.Equal(expected, Retail2013.ReconstructFullCommand(wire)); + } + + // Anchor checks straight off the extracted table — these pin the + // extraction itself, independent of the recall-command narrative. + [Theory] + [InlineData(0x0000, 0x80000000u)] + [InlineData(0x0003, 0x41000003u)] + [InlineData(0x0005, 0x45000005u)] + [InlineData(0x0007, 0x44000007u)] + [InlineData(0x000D, 0x6500000Du)] + [InlineData(0x0150, 0x10000150u)] + [InlineData(0x0153, 0x09000153u)] // NOT LifestoneRecall in 2013 numbering — anchor only. + public void Retail2013_AnchorValuesMatchExtractedTable(ushort wire, uint expected) + { + Assert.Equal(expected, Retail2013.ReconstructFullCommand(wire)); + } + + [Fact] + public void Retail2013_OutOfRangeWireReturnsZero() + { + Assert.Equal(0u, Retail2013.ReconstructFullCommand(0xFFFF)); + } + + [Fact] + public void Retail2013_LastInRangeIndexResolves() + { + // [0x197] is the final entry of command_ids[0x198]. + Assert.Equal(0x10000197u, Retail2013.ReconstructFullCommand(0x0197)); + } + + [Fact] + public void Retail2013_FirstOutOfRangeIndexReturnsZero() + { + // 0x198 is one past the table's last valid index. + Assert.Equal(0u, Retail2013.ReconstructFullCommand(0x0198)); + } + + // ── Class-priority collision resolution ───────────────────────────── + // + // Retail's class-byte priority is: lower class byte wins + // (Action 0x10 < ChatEmote 0x12/0x13 < Modifier 0x20 < SubState 0x41 + // < ... < Style 0x80). This is exercised directly against the build + // logic via a small synthetic set of colliding low-words, because the + // CURRENT DatReaderWriter.Enums.MotionCommand enum (2.1.7, 409 distinct + // values) happens to contain zero same-low16 collisions today — so a + // matrix test against the live enum alone wouldn't actually exercise + // the tie-break rule. + + [Fact] + public void ClassPriority_LowerClassByteWins() + { + var candidates = new Dictionary + { + // Same low word (0x0042) claimed by three different classes; + // Action (0x10) must win over Modifier (0x20) and Style (0x80). + [0x0042] = 0u, + }; + + uint[] colliding = { 0x80000042u, 0x20000042u, 0x10000042u }; + uint resolved = AceModernCommandCatalog.ResolveClassPriority(colliding); + + Assert.Equal(0x10000042u, resolved); + } + + [Fact] + public void ClassPriority_OrderOfInputDoesNotMatter() + { + uint[] collidingReversed = { 0x10000099u, 0x41000099u, 0x80000099u }; + uint[] collidingForward = { 0x80000099u, 0x41000099u, 0x10000099u }; + + Assert.Equal( + AceModernCommandCatalog.ResolveClassPriority(collidingForward), + AceModernCommandCatalog.ResolveClassPriority(collidingReversed)); + Assert.Equal(0x10000099u, AceModernCommandCatalog.ResolveClassPriority(collidingForward)); + } +}