Opus final review of B.4c flagged that Task 4's handoff doc invented implementation details that don't exist in the code: 1. IsDoorSpawn claimed to check "spawn.WeenieObj.WeenieType == 8 OR IsDoorName(spawn.Name)" — the actual code is just IsDoorName(spawn.Name) delegating to "name == "Door"". No WeenieType lookup exists. 2. A "_doorSequencers" per-door dict was referenced in three places — that dict doesn't exist. The actual code reuses the existing _animatedEntities[entity.Id] dict (same one that holds creatures + the player), with Animation = null! per the existing pattern at line 7885. 3. The UM dispatch path was described as a new B.4c-added branch with pseudocode — that's wrong. B.4c does NOT add a new dispatch path; OnLiveMotionUpdated's existing TryGetValue against _animatedEntities handles doors automatically once Task 1's spawn-time branch registers them. The only UM-dispatch B.4c contribution is the [door-cycle] diagnostic line, gated on IsDoorName. Corrects sections "At world load (spawn time)", "When the door opens", "Per-frame mesh rebuild", and "Door types covered" to reflect the actual shipped code. cmd→motion mapping (cmd=0x000C → open, cmd=0x000B → close) left as-is — it was correct. No code change. Verified by re-reading GameWindow.cs IsDoorSpawn / IsDoorName helpers, the Task 1 spawn-time branch body, and the TickAnimations sequencer dispatch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
346 lines
16 KiB
Markdown
346 lines
16 KiB
Markdown
# Phase B.4c shipped — handoff (visual-verified 2026-05-13)
|
|
|
|
**Date:** 2026-05-13.
|
|
**Branch:** `claude/phase-b4c-door-anim` (ready to merge to main; do NOT merge here — controller handles that after code review).
|
|
**Predecessors:**
|
|
- [docs/research/2026-05-13-b4b-shipped-handoff.md](2026-05-13-b4b-shipped-handoff.md) — B.4b shipped handoff; interaction was the upstream dependency (Use message, SetState handling, collision exemption, double-click detection — all shipped there).
|
|
- [docs/superpowers/specs/2026-05-13-phase-b4c-design.md](../superpowers/specs/2026-05-13-phase-b4c-design.md) — B.4c design spec.
|
|
- [docs/superpowers/plans/2026-05-13-phase-b4c-plan.md](../superpowers/plans/2026-05-13-phase-b4c-plan.md) — B.4c implementation plan (4 tasks).
|
|
|
|
---
|
|
|
|
## TL;DR
|
|
|
|
Phase B.4c **shipped end-to-end and is visual-verified 2026-05-13.** The M1
|
|
demo target *"open the inn door"* now has **full visual feedback** — the door
|
|
swings open when double-clicked and swings closed again when ACE toggles it
|
|
back. 4 implementation commits implement and fix door-specific spawn-time
|
|
`AnimationSequencer` registration + `UpdateMotion` routing + stance-value
|
|
correctness.
|
|
|
|
The plan estimated "2 tasks, door spawn-time registration + UM diagnostic."
|
|
Visual testing surfaced **two bonus discoveries** beyond the plan:
|
|
|
|
1. The plan's `NonCombatStance` constant was wrong: `0x80000001` (from
|
|
creature motion table conventions) should be `0x8000003D` (from AC's
|
|
`MotionStance.NonCombat = 0x0000003D`). Wrong constant → wrong
|
|
`HasCycle` lookup → `SetCycle` never fires → sequencer empty →
|
|
per-frame part rebuild collapses to entity origin → doors render halfway
|
|
underground.
|
|
2. The `AnimationSequencer`'s link→cycle boundary transition produces a
|
|
brief one-frame flash through the prior pose at the end of the door-swing
|
|
animation. Not B.4c-specific — it is the sequencer's general link+cycle
|
|
queue mechanics. Deferred as issue #61.
|
|
|
|
Issue #58 (door swing animation) is closed. Issues #61 + #62 (cycle-boundary
|
|
flash; PARTSDIAG null-guard) are filed as M1-deferred polish.
|
|
|
|
---
|
|
|
|
## What shipped on this branch
|
|
|
|
| # | Commit | Subject | Task |
|
|
|---|---|---|---|
|
|
| 1 | `9053860` | `feat(B.4c): door spawn-time AnimationSequencer with state-seeded initial cycle` | Task 1 |
|
|
| 2 | `b89f004` | `feat(B.4c): [door-cycle] diagnostic in OnLiveMotionUpdated` | Task 2 |
|
|
| 3 | `8a9b15e` | `refactor(B.4c): share IsDoorName predicate + durable comment + use UM locals` | Task 2 review |
|
|
| 4 | `454d88e` | `fix(B.4c): correct NonCombat stance value (0x3D, not 0x01) + read spawn.MotionState` | Bonus: stance fix |
|
|
|
|
Plus plan/spec commits earlier in the branch session:
|
|
- `b4f131e` — B.4c design spec.
|
|
- `6ae38f7` — B.4c implementation plan (4 tasks).
|
|
|
|
**Build:** clean. **Tests:** existing test suite passes; no new unit tests added
|
|
(the door-cycle registration path runs in-process with a live GameWindow; pure
|
|
unit tests would require a MotionTable + AnimationSequencer integration harness).
|
|
|
|
---
|
|
|
|
## What the code does end-to-end
|
|
|
|
When the world loads, any entity whose name contains "Door" (checked via the
|
|
shared `GameWindow.IsDoorName(string)` helper, committed as part of Task 2
|
|
review) is registered in the **door-animation side-track** at spawn time. This
|
|
happens inside `GameWindow.OnLiveEntitySpawnedLocked`, which branches on
|
|
`IsDoorSpawn(spawn)` before reaching the standard creature/player paths.
|
|
|
|
### At world load (spawn time)
|
|
|
|
1. `IsDoorSpawn(spawn)` — delegates to `IsDoorName(spawn.Name)`, which
|
|
returns `name == "Door"`. Detection by server-sent name string only.
|
|
Cheap, exact, no WeenieType lookup. If a future ACE localizes "Door"
|
|
or sends a different name, those entities silently won't animate —
|
|
acceptable per B.4c's "doors only at English Holtburg" scope.
|
|
|
|
2. **Initial state seed** — the door's `PhysicsState` from `spawn` carries the
|
|
open/closed bit. The code reads `spawn.PhysicsState` (or
|
|
`spawn.MotionState?.Stance` as a fallback for unusual doors with explicit
|
|
stance data) to determine whether to seed the sequencer with the `Off`
|
|
(closed) or `On` (open) cycle.
|
|
|
|
3. **AnimationSequencer registration** — a fresh `AnimationSequencer` is
|
|
created for the door entity's `MotionTableId` (from `spawn`). Then:
|
|
```csharp
|
|
var style = 0x80000000u | (uint)MotionStance.NonCombat; // = 0x8000003D
|
|
var cycleCmd = isOpen ? MotionCommand.On : MotionCommand.Off;
|
|
sequencer.SetCycle(style, (uint)cycleCmd, speed: 0f);
|
|
```
|
|
The fully-initialized `AnimatedEntity` (with the seeded `Sequencer`) is
|
|
registered into the existing `_animatedEntities` dict keyed by `entity.Id`
|
|
— same dict that holds creatures and the player. `Animation = null!`
|
|
(the null-forgiving suppression matches an existing pattern at
|
|
`GameWindow.cs:7885` for sequencer-driven entities where the legacy
|
|
`Animation` field is unused). At the first per-frame `Advance(dt)`
|
|
call from `TickAnimations`, the sequencer produces the correct
|
|
rest-pose frames for the door's current state.
|
|
|
|
4. **Log evidence at spawn:**
|
|
```
|
|
[door-anim] registered guid=0x7A9B403A entityId=0x000F4291 mtable=0x09000202 initialStyle=0x8000003D initialCycle=0x4000000C
|
|
```
|
|
`0x4000000C` = `MotionCommand.Off` with the upper flag bits — the door is
|
|
closed at spawn, matching the initial world state.
|
|
|
|
### When the door opens (UpdateMotion arrives)
|
|
|
|
ACE broadcasts `UpdateMotion (0xF74D)` with `stance=0x003D` (NonCombat) and
|
|
wire `cmd=0x000C` (which `MotionCommandResolver.ReconstructFullCommand`
|
|
maps to full motion `0x4000000B` = `MotionCommand.On` = door open).
|
|
|
|
B.4c does NOT add a new dispatch path here — the existing
|
|
`OnLiveMotionUpdated` handler already routes via the `_animatedEntities`
|
|
dict + per-entity `Sequencer`, the same code path creatures use. The
|
|
only B.4c contribution at UM dispatch is the new `[door-cycle]`
|
|
diagnostic gated on `IsDoorName(doorInfo.Name)`. Before B.4c, doors
|
|
silently dropped at the `_animatedEntities.TryGetValue` check at
|
|
`GameWindow.cs:3036` because doors weren't registered; B.4c's Task 1
|
|
spawn-time branch fixed that.
|
|
|
|
The sequencer transitions from the `Off` cycle (static closed pose) through
|
|
the door-swing link animation to the `On` cycle (static open pose).
|
|
|
|
**Log evidence:**
|
|
```
|
|
UM guid=0x7A9B403A mt=0x00 stance=0x003D cmd=0x000C spd=0.00 | seq now style=0x8000003D motion=0x4000000B
|
|
[door-cycle] guid=0x7A9B403A stance=0x003D cmd=0x000C
|
|
```
|
|
The `[door-cycle]` line is the new B.4c diagnostic (gated on
|
|
`ACDREAM_PROBE_BUILDING=1`). The `seq now motion=0x4000000B` shows the
|
|
sequencer's current motion state after the `SetCycle` call.
|
|
|
|
### SetState chain (from B.4b + L.2g, unchanged)
|
|
|
|
Simultaneously with `UpdateMotion`, ACE also sends `SetState (0xF74B)`:
|
|
```
|
|
[setstate] guid=0x7A9B... state=0x0001000C
|
|
```
|
|
This is the B.4b / L.2g chain: `ShadowObjectRegistry.UpdatePhysicsState` flips
|
|
the door's cached state, `CollisionExemption.ShouldSkip` exempts on ETHEREAL-alone,
|
|
and the player can walk through. B.4c is additive — it only adds the animation
|
|
layer; it does not touch the collision path.
|
|
|
|
### When the door closes
|
|
|
|
ACE toggles on the next Use: `UpdateMotion` with `cmd=0x000B` (Off = close).
|
|
The sequencer transitions from the `On` cycle (open pose) through the door-swing
|
|
link animation (reversed) to the `Off` cycle (closed pose).
|
|
|
|
**Log evidence:**
|
|
```
|
|
UM guid=0x7A9B403A mt=... cmd=0x000B ... motion=0x4000000C
|
|
[door-cycle] guid=0x7A9B... cmd=0x000B
|
|
[setstate] guid=0x7A9B... state=0x00010008
|
|
```
|
|
|
|
### Per-frame mesh rebuild
|
|
|
|
The door sequencer integrates into `GameWindow.TickAnimations` via the same
|
|
`_animatedEntities` dict that holds creatures. Each frame, `ae.Sequencer.Advance(dt)`
|
|
is called and the resulting per-part transforms drive the same `MeshRefs` rebuild
|
|
that creature entities use (sequencer branch at `GameWindow.cs:7497`; doors
|
|
never enter the legacy slerp `else` branch). This is the reason the stance-value
|
|
bug produced underground doors: with the wrong style key (`0x80000001`)
|
|
`HasCycle` returned false, the sequencer was empty at spawn, `Advance` returned
|
|
identity frames, and the per-frame part-matrix rebuild received `Vector3.Zero /
|
|
Quaternion.Identity` for every part — collapsing them all to the entity origin.
|
|
|
|
---
|
|
|
|
## The two bonus discoveries
|
|
|
|
### 1. NonCombatStance constant was wrong: 0x01 vs 0x3D (`454d88e`) — THE render blocker
|
|
|
|
**Root cause:** The B.4c design spec specified the initial-cycle style key as:
|
|
```csharp
|
|
uint style = 0x80000000u | (uint)MotionStance.NonCombat; // spec said 0x80000001
|
|
```
|
|
The spec's comment was wrong. `MotionStance.NonCombat` in acdream (and retail)
|
|
is `0x0000003D`, not `0x00000001`. The value `0x01` is a creature-specific
|
|
variant. The style key for the door's cycle lookup must be `0x8000003D`.
|
|
|
|
With the wrong style key:
|
|
- `sequencer.HasCycle(0x80000001, MotionCommand.Off)` → false.
|
|
- `SetCycle(0x80000001, ...)` enqueued a cycle that was never reachable.
|
|
- On first `Advance(dt)`, the sequencer returned 0 part-frames.
|
|
- The per-frame mesh rebuild at `GameWindow.cs:7691` iterated 0 frames, leaving
|
|
every door part at the entity root origin (which is the door's structural
|
|
pivot, typically near the hinge). For inn doors this pivot is at roughly
|
|
floor level, so all the door's mesh parts collapsed to that single point,
|
|
rendering as a thin sliver partway underground.
|
|
|
|
**Fix:** Corrected the constant. Additionally, added a defensive read of
|
|
`spawn.MotionState?.Stance` as the source of the stance value where available,
|
|
so unusual doors with explicit motion state (possible in custom ACE content) use
|
|
their actual stance rather than the hardcoded NonCombat assumption:
|
|
|
|
```csharp
|
|
var stance = spawn.MotionState?.Stance ?? MotionStance.NonCombat;
|
|
uint style = 0x80000000u | (uint)stance;
|
|
```
|
|
|
|
**Verification:** After this fix, the `[door-anim]` log line showed
|
|
`initialStyle=0x8000003D` (correct), and doors appeared at the correct floor
|
|
level and height at world load.
|
|
|
|
### 2. AnimationSequencer link→cycle boundary flash (deferred as #61)
|
|
|
|
**Observed:** User reports "weird flapping at end of animation when it opens.
|
|
It is like it flaps back to closed quickly then open. Like really quickly."
|
|
Both open and close animations exhibit this flash.
|
|
|
|
**Root cause hypothesis:** `AnimationSequencer.SetCycle` enqueues a transition
|
|
link (the actual swing animation) followed by the target cycle (the door's
|
|
rest pose — likely a single-frame static "open" or "closed" pose). At the link→
|
|
cycle boundary, the sequencer evaluates the cycle's frame 0 before the cycle
|
|
settles into its natural rest position. If the link's last frame and the
|
|
cycle's frame 0 don't match exactly (which is common for one-shot door motions
|
|
versus the continuous idle cycles the sequencer was designed for), the renderer
|
|
sees one frame of the "wrong" pose at the link boundary.
|
|
|
|
**Why not B.4c-specific:** This is the sequencer's general link+cycle queue
|
|
boundary semantics. Any entity that uses a one-shot `SetCycle` transition
|
|
(rather than a continuous idle cycle) will exhibit this if the link/cycle
|
|
boundary frames diverge. The door case just makes it visible because the
|
|
swing duration is short (1-2 seconds) and the user is watching closely.
|
|
|
|
**Deferred:** Filed as issue #61. Workaround: the flash is brief (~1 frame,
|
|
~16ms at 60 FPS) and does not affect the door's usability. M1 is met without
|
|
this fix.
|
|
|
|
---
|
|
|
|
## Open notes / follow-ups
|
|
|
|
### #61 — AnimationSequencer link→cycle frame-0 flash (filed this session)
|
|
|
|
See Bonus discovery #2 above. Deferred as M1-deferred polish. Low severity.
|
|
Acceptance: door swing animations play cleanly with no intermediate closed/open
|
|
pose flash at the link→cycle transition.
|
|
|
|
### #62 — PARTSDIAG null-guard for sequencer-driven entities (filed this session)
|
|
|
|
The PARTSDIAG block at `GameWindow.cs:7657` reads `ae.Animation.PartFrames`
|
|
without a null-guard. B.4c introduced `Animation = null!` for sequencer-driven
|
|
door entities. Today this is safe (doors never enter `_remoteDeadReckon` because
|
|
ACE never sends UpdatePosition for them). Deferred as low-severity latent crash.
|
|
One-line fix when addressed.
|
|
|
|
### Chests, levers, traps
|
|
|
|
The `IsDoorName` / `IsDoorSpawn` predicate correctly gates on door entities only.
|
|
Other interactable non-creature entities (chests, levers, traps) will still
|
|
silently drop their `UpdateMotion` commands — they are not covered by B.4c and
|
|
no issue has been filed for them yet. When those animations become relevant
|
|
(M2/M3 inventory + dungeon content), the same spawn-time registration pattern
|
|
can be extended: broaden the detection predicate beyond `name == "Door"` and
|
|
register additional entity types in the existing `_animatedEntities` dict via
|
|
the same sibling branch.
|
|
|
|
### Door toggle behavior
|
|
|
|
Unchanged from B.4b. ACE doors toggle on each Use: first double-click opens,
|
|
subsequent double-click closes. Both transitions now play the correct swing
|
|
animation (open swing on open, close swing on close).
|
|
|
|
---
|
|
|
|
## Next session
|
|
|
|
**M1 demo progress as of this branch:**
|
|
- "Walk through Holtburg without getting stuck" — Phase L.2 in progress (outdoor collision works; `CBuildingObj` interior still deferred to L.2d).
|
|
- "Open the inn door" — **DONE with full visual feedback** (B.4b interaction + B.4c animation, this branch). Door swings open AND closed.
|
|
- "Click an NPC" — pick + Use wiring exists (from B.4b); depends on ACE NPC handler responding to Use correctly.
|
|
- "Pick up an item" — `BuildPickUp` + F-key wiring not yet in `OnInputAction`. Post-B.4b/B.4c deferred.
|
|
|
|
**Recommended next steps (in M1 critical-path order):**
|
|
|
|
1. **"Click an NPC" verification spike** — B.4b's WorldPicker + Use messaging
|
|
is already wired. The question is whether ACE NPCs respond to Use and what
|
|
they broadcast back. A quick spike: stand near an NPC in Holtburg,
|
|
double-click, check what ACE sends back. If ACE sends recognizable response
|
|
messages, wire them; if it is silent, investigate ACE's NPC handler
|
|
configuration for testaccount.
|
|
|
|
2. **Phase B.5 — Ground item pickup (F key)** — `SelectionPickUp` input action
|
|
+ F-key binding exist but `OnInputAction` has no case. `BuildUse` is the
|
|
same wire format as `BuildPickUp`. Adding the `SelectionPickUp` case to
|
|
the switch and routing to `InteractRequests.BuildPickUp` is a one-commit
|
|
addition.
|
|
|
|
3. **Triage chronic open-issue list** — #2 (lightning), #4 (sky horizon-glow),
|
|
#28 (aurora), #29 (cloud thinness), #37 (humanoid coat), #41
|
|
(remote-motion blips) have been open since April/early-May. Link each to
|
|
a future phase or downgrade. ~1 hour.
|
|
|
|
4. **#61 fix (cycle-boundary flash)** — low-severity M1 polish. If the user
|
|
finds the flash distracting during the M1 demo record, address before
|
|
milestone wrap; otherwise defer to M2 animation quality pass.
|
|
|
|
---
|
|
|
|
## Reproducibility
|
|
|
|
Same launch recipe as B.4b. For reproducing the visual test:
|
|
|
|
```powershell
|
|
$env:ACDREAM_DAT_DIR = "$env:USERPROFILE\Documents\Asheron's Call"
|
|
$env:ACDREAM_LIVE = "1"
|
|
$env:ACDREAM_TEST_HOST = "127.0.0.1"
|
|
$env:ACDREAM_TEST_PORT = "9000"
|
|
$env:ACDREAM_TEST_USER = "testaccount"
|
|
$env:ACDREAM_TEST_PASS = "testpassword"
|
|
$env:ACDREAM_DEVTOOLS = "1"
|
|
$env:ACDREAM_PROBE_BUILDING = "1"
|
|
$env:ACDREAM_PROBE_RESOLVE = "1"
|
|
dotnet run --project src\AcDream.App\AcDream.App.csproj -c Debug 2>&1 |
|
|
Tee-Object -FilePath "launch-b4c.log"
|
|
```
|
|
|
|
Walk to the Holtburg inn doorway. Watch the `[door-anim]` lines appear in the
|
|
log as each door entity spawns (verifies correct style=0x8000003D and initial
|
|
cycle). Double-left-click a closed door. Watch the swing animation. Walk
|
|
through. Wait ~30s (ACE auto-close). Watch the close animation.
|
|
|
|
After closing the client, grep for:
|
|
|
|
```powershell
|
|
Select-String -Path launch-b4c.log -Pattern "door-anim|door-cycle|setstate"
|
|
```
|
|
|
|
Expected:
|
|
- `[door-anim] registered guid=... initialStyle=0x8000003D initialCycle=0x4000000C` — correct style + Off initial cycle for each closed door.
|
|
- `[door-cycle] guid=... stance=0x003D cmd=0x000C` — open UpdateMotion processed.
|
|
- `[setstate] guid=... state=0x0001000C` — ACE collision-flip processed (from B.4b / L.2g).
|
|
- `[door-cycle] guid=... cmd=0x000B` — close UpdateMotion processed.
|
|
- `[setstate] guid=... state=0x00010008` — ACE close collision-flip processed.
|
|
|
|
---
|
|
|
|
## Worktree state at handoff
|
|
|
|
- Branch `claude/phase-b4c-door-anim`.
|
|
- 6 commits ahead of `3e08e10` (the B.4b+L.2g merge from this morning):
|
|
2 docs/spec/plan commits + 4 implementation commits.
|
|
- Controller should run a code review, then merge to main.
|
|
- Do NOT rebase or squash — each commit tells a diagnostic story that the
|
|
next phase's debugging may need.
|