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

feat(L.1b): dual MotionCommand catalog — AceModern runtime + Retail2013 conformance

Browse source 
Splits MotionCommandResolver's single ACE-modern lookup into an
IMotionCommandCatalog seam with two implementations:

- AceModernCommandCatalog (runtime default): built cleanly from the
  DatReaderWriter MotionCommand enum (mirrors ACE + local DAT MotionTables)
  with a documented class-priority tiebreak. The old blind 0x016E-0x0197
  per-range override is DELETED — verified the ACE matrix (LifestoneRecall
  0x0153 to 0x10000153, MarketplaceRecall 0x0166, AllegianceHometownRecall
  0x0171, OffhandSlashHigh 0x0173) resolves correctly straight from the enum
  with no override. Stale shift-start comment corrected to SnowAngelState
  (0x43000115 to 0x43000118), not AllegianceHometownRecall.
- Retail2013CommandCatalog (conformance/reference): full verbatim extraction
  of command_ids[0x198] at 0x007c73e8 (acclient_2013_pseudo_c.txt:1017259-1017667),
  direct wire-low to full index lookup. 408 entries, anchors verified against
  source ([0x150]=0x10000150, [0x153]=0x09000153, [0x197]=0x10000197).

MotionCommandResolver.ReconstructFullCommand stays a static facade delegating
to an AceModern singleton — all ~10 runtime callers unchanged.

Removing the override exposed a pre-existing bug: CombatAnimationPlanner's
late-combat command block uses 2013 numbering, not ACE/DRW. Corrected the one
catalog test assertion pinned to the override's output (wire 0x0170 to
0x09000170 IssueSlashCommand, its true DRW identity) and filed the planner bug
as #159 rather than silently patching out-of-scope code.

Tests: +catalog matrices (ACE/2013), class-priority collisions, boundary
cases, real-DAT availability (gap-doc hit counts reproduced). Build + full
suite green (Core.Tests 1718, no regressions).

Spec: docs/superpowers/specs/2026-06-30-movement-wire-parity-design.md (1)
Research: docs/research/2026-06-26-ace-vs-2013-motion-command-gap.md

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-06-30 22:13:36 +02:00
parent 33ad231d18
commit 2c8620ea94
8 changed files with 612 additions and 65 deletions
Download patch file Download diff file Expand all files Collapse all files

19
docs/ISSUES.md
View file

@ -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