diff --git a/docs/plans/2026-05-12-phase-c1.5b-handoff.md b/docs/plans/2026-05-12-phase-c1.5b-handoff.md new file mode 100644 index 0000000..b314516 --- /dev/null +++ b/docs/plans/2026-05-12-phase-c1.5b-handoff.md @@ -0,0 +1,320 @@ +# Phase C.1.5b handoff — issue #56 + EnvCell statics + animation-hook verification + +**Created:** 2026-05-12, immediately after Phase C.1.5a merged to `main` (commit `88bda12`). +**Audience:** the fresh-session Claude (or human) picking up C.1.5b. +**Predecessor:** [C.1.5a portal PES wiring](../superpowers/specs/2026-05-12-phase-c1.5a-portals-design.md) — slice 1, shipped. + +--- + +## §1 Startup prompt (copy this into a fresh session) + +Everything below this fence is the prompt to paste into a new Claude Code session. The detailed context the session needs lives in §2+ of this same file. + +``` +Pick up Phase C.1.5b — issue #56 (multi-emitter per-part collapse) first, +then EnvCell static-object DefaultScript dispatch + animation-hook +particle path verification. + +## Context + +Phase C.1.5a (portal PES wiring) merged to main 2026-05-12 (merge commit +88bda12). The PhysicsScriptRunner now fires Setup.DefaultScript on every +server-spawned WorldEntity via the new EntityScriptActivator. Visual +verification at the Holtburg Town network portal confirmed the mechanism +works end-to-end (10-hook portal script fires correctly, color + +persistence + orientation match retail), but exposed a pre-existing C.1 +limitation now tracked as ISSUE #56: ParticleHookSink ignores +CreateParticleHook.PartIndex, so all 10 of the portal's emitters +collapse to one root position → compressed, partly-ground-buried swirl. + +The C.1.5a final cross-task reviewer recommended #56 be resolved FIRST +in this slice, before the EnvCell static-object walker, because slice +2's natural visual gate (Holtburg inn interior fireplace, cottage +chimney) uses the same multi-emitter pattern — without #56 fixed, +slice 2 ships with the same visual gap. + +## Read first (in order) + +1. docs/plans/2026-05-12-phase-c1.5b-handoff.md (this file's §2+) +2. docs/ISSUES.md #56 (the per-part collapse problem with reproducible + identifiers from the C.1.5a verification session) +3. docs/superpowers/specs/2026-05-12-phase-c1.5a-portals-design.md §10 + (slice 2 preview written during C.1.5a brainstorming) +4. docs/plans/2026-04-27-phase-c1-pes-particles.md lines 285–295 (the + original C.1.5 scope source) + +## Two slices in this session + +### Slice A — issue #56 fix (per-part transform handling for static entities) + +For static entities (portals, EnvCell statics, building decorations — +no animation), precompute the per-part offset from +Setup.PlacementFrames[Resting] at spawn time and surface those offsets +to the ParticleHookSink so SpawnFromHook can apply them. The handoff +doc §3 has the suggested architecture + decision space. + +Acceptance: relaunch + walk to the Holtburg Town network portal. The +10 emitters should distribute across the portal Setup's parts instead +of collapsing — swirl extends vertically through the arch with +retail-like shape, not buried in the ground. + +### Slice B — EnvCell static-object DefaultScript dispatch + animation-hook verification + +Walk EnvCell.StaticObjects for newly-loaded landblocks; for each +StaticObject whose Setup has a non-zero DefaultScript, fire the +activator with a synthetic entity ID (suggested scheme: hash of +(landblockId, cellIndex, staticIndex) with a high-bit marker so it +doesn't collide with server guids — see handoff §4). Then verify the +animation-hook particle path (already shipped in C.1; just needs +visual confirmation): cast a spell on +Acdream and compare to retail. + +Acceptance: Holtburg inn fireplace flames, cottage chimney smoke, and +a spell-cast particle effect on +Acdream all match retail. + +## What this is NOT + +- Not a renderer change. particle.frag stays as-is; bindless migration + waits for N.6 slice 2 after this slice lands. +- Not a perf phase. The N.6 baseline at radius=4 still holds; the + per-part precompute cost is bounded by N parts × M emitters per + spawned entity (small). +- Not adding new emitter types. Use the existing PES emitter data. +- Not touching the animated-entity path. For animated entities (NPCs, + monsters), per-part transforms vary per frame and would need a + per-tick refresh similar to UpdateEntityAnchor. Defer to a future + phase; C.1.5b stays scoped to static entities only. + +## Suggested workflow + +1. Read the handoff doc + the four referenced docs above. +2. Invoke superpowers:brainstorming to settle: + - For slice A: precompute-per-part-at-spawn vs render-thread-side-table + approach (handoff §3 has the tradeoff analysis). + - For slice B: the synthetic-entity-id scheme; whether the EnvCell + walker piggybacks LandblockSpawnAdapter or gets its own class. + - Visual verification locations. +3. After brainstorm: spec at + docs/superpowers/specs/2026-05-13-phase-c1.5b-design.md (one spec + for both slices since they share the activator and tests), then plan + at docs/superpowers/plans/2026-05-13-phase-c1.5b.md, then execute + via superpowers:subagent-driven-development. + +## Open issues from C.1.5a worth knowing + +- #56 — multi-emitter per-part collapse. This slice's headline. +- #55 — meshMissing diagnostic spam at radius=4 standstill. LOW + severity, not blocking; only touch if you're already in the + dispatcher for unrelated reasons. +- Cold-path timing observation (C.1.5a Task 2 review): the activator + fires DefaultScript before pending-bucket entities are merged into + a loaded landblock. Mirrors existing _wbEntitySpawnAdapter pattern; + not a regression; defer. + +## Three doc-drift items from C.1.5a (trivial — fold into the new spec) + +1. C.1.5a spec §4 says "fifth (optional) parameter" — actually fourth. +2. C.1.5a spec §4 says "~50 lines" — file ships at 93 lines. +3. GpuWorldState.AddEntitiesToExistingLandblock (A.5 Far→Near + promotion path) does not fire the activator. No-op today because + promotion-tier entities are atlas-tier and the activator's + ServerGuid==0 guard would skip them anyway, but worth a code + comment explaining why the call is intentionally omitted there + (parallel to existing comments at the RemoveEntitiesFromLandblock + block in the same file). + +Start by reading the handoff doc, then ask me what slice-A/slice-B +boundary feels right and what visual verification locations I want +to target. +``` + +--- + +## §2 What shipped in C.1.5a (so you don't re-do it) + +### Commits on `main` (oldest to newest under merge `88bda12`) + +| SHA | Title | +|---|---| +| `06d7fbd` | docs(vfx): Phase C.1.5a — portal PES wiring design spec | +| `ed5335b` | docs(vfx #C.1.5a): implementation plan + spec wiring-location fixes | +| `003c502` | feat(vfx #C.1.5a): add EntityScriptActivator (no wiring yet) | +| `e0529b0` | test(vfx #C.1.5a): real-emitter verification in OnRemove test + unused using | +| `44d8502` | feat(vfx #C.1.5a): wire EntityScriptActivator into GpuWorldState lifecycle | +| `65d833d` | feat(vfx #C.1.5a): construct EntityScriptActivator in GameWindow | +| `849690c` | refactor(vfx #C.1.5a): reuse SequencerFactory's capturedDats in resolver | +| `334f0c6` | fix(vfx #C.1.5a): seed entity rotation in activator so hook offset rotates | +| `9009318` | docs(vfx #C.1.5a): ship Phase C.1.5a + file issue #56 for per-part collapse | +| `88bda12` | Merge branch 'claude/lucid-burnell-aab524' — Phase C.1.5a | + +### New files + +- [`src/AcDream.App/Rendering/Vfx/EntityScriptActivator.cs`](../../src/AcDream.App/Rendering/Vfx/EntityScriptActivator.cs) — 93 lines including doc comments. Constructor `(PhysicsScriptRunner, ParticleHookSink, Func)`; `OnCreate(WorldEntity)` resolves the entity's `Setup.DefaultScript.DataId`, seeds `_particleSink.SetEntityRotation(entity.ServerGuid, entity.Rotation)`, and calls `_scriptRunner.Play(scriptId, entity.ServerGuid, entity.Position)`; `OnRemove(uint serverGuid)` calls `_scriptRunner.StopAllForEntity(serverGuid)` + `_particleSink.StopAllForEntity(serverGuid, fadeOut: false)`. +- [`tests/AcDream.Core.Tests/Rendering/Vfx/EntityScriptActivatorTests.cs`](../../tests/AcDream.Core.Tests/Rendering/Vfx/EntityScriptActivatorTests.cs) — 4 xUnit `[Fact]` tests with mutation-check teeth verified during the C.1.5a code-quality reviews. + +### Modified files + +- [`src/AcDream.App/Streaming/GpuWorldState.cs`](../../src/AcDream.App/Streaming/GpuWorldState.cs) — fourth optional ctor parameter `EntityScriptActivator? entityScriptActivator = null`, field `_entityScriptActivator`, and two `?.OnCreate(entity)` / `?.OnRemove(serverGuid)` calls immediately after the matching `_wbEntitySpawnAdapter?.OnCreate` / `?.OnRemove` calls in `AppendLiveEntity` and `RemoveEntityByServerGuid`. +- [`src/AcDream.App/Rendering/GameWindow.cs`](../../src/AcDream.App/Rendering/GameWindow.cs) — new field declaration alongside `_wbEntitySpawnAdapter` and inline construction of the activator + resolver lambda inside the existing `OnLoad` block (~line 1620), passed to `GpuWorldState` as a named argument. + +### What's working + +- Server-spawned entities (`ServerGuid != 0`) with `Setup.DefaultScript.DataId != 0` fire that script through `PhysicsScriptRunner.Play` on enter-world. +- Multi-hook scripts dispatch all their hooks in order (timed by `StartTime` offsets — more retail-faithful than WB's "all at once" collection). +- `CreateParticleHook.Offset.Origin` rotates correctly from entity-local to world frame via the activator's `SetEntityRotation` seed. +- Despawn cleanly stops all scripts + emitters for the entity. +- 4 unit tests cover all three branches plus the rotation-seed correctness. +- Visual verification at the Holtburg Town network portal passed for the mechanism: 10-hook portal script fires correctly with matching color, persistence, orientation, multi-emitter dispatch. + +## §3 Issue #56 decision space (slice A) + +### The problem + +`ParticleHookSink.SpawnFromHook` computes: + +```csharp +var rotation = _rotationByEntity.TryGetValue(entityId, out var rot) ? rot : Quaternion.Identity; +var anchor = worldPos + Vector3.Transform(offset, rotation); +``` + +…where `worldPos` is `entity.Position` and `offset` is `cph.Offset.Origin`. The `CreateParticleHook.PartIndex` field is recorded into the per-handle tracking dict but never applied to the anchor. Retail's intended geometry is: + +``` +anchor = entityWorldPose × partLocalTransform[partIndex] × hookOffsetInPartLocal +``` + +Without the part transform multiplication, every emitter in a multi-emitter script lands at the same root position. Visible symptom: the Holtburg portal's 10 emitters compress to one point and the swirl appears partially buried because the offset's local-up direction goes off in world axes instead of the part's local axes. + +### Where part transforms come from + +For STATIC entities (no animation), per-part transforms come from `Setup.PlacementFrames[Resting].Frames[partIndex]` — see how `ObjectMeshManager.CollectParts` walks them in `references/WorldBuilder` (worktree-relative path; submodule must be initialized to read): + +- For each `i` in `0..setup.Parts.Count`, the per-part transform is `Matrix4x4.CreateScale(setup.DefaultScale[i]) * Matrix4x4.CreateFromQuaternion(placementFrame.Frames[i].Orientation) * Matrix4x4.CreateTranslation(placementFrame.Frames[i].Origin)`. +- `DefaultScale` only applies when `SetupFlags.HasDefaultScale` is set. +- Fall back to `PlacementFrames[Default]` if `Resting` isn't present. + +For ANIMATED entities (NPCs, monsters, the player), per-part transforms vary per animation frame and live in `AnimatedEntityState` / the animation tick. **Out of scope for C.1.5b.** + +### Approach options + +**Option A — precompute per-spawn, pass at activator-call time.** + +`EntityScriptActivator` reads the Setup's `PlacementFrames[Resting]` once per spawn, builds a `Matrix4x4[] partTransforms` array, and passes it to a new sink method `_particleSink.SetEntityPartTransforms(entityId, partTransforms)` before calling `_scriptRunner.Play(...)`. `ParticleHookSink.SpawnFromHook` then reads `_partTransformsByEntity` to apply per-hook: + +```csharp +var partXf = _partTransformsByEntity.TryGetValue(entityId, out var pts) && partIndex < pts.Length + ? pts[partIndex] : Matrix4x4.Identity; +var anchor = worldPos + Vector3.Transform(Vector3.Transform(offset, partXf), rotation); +``` + +Pros: clean ownership (activator owns the lifecycle of part transforms keyed by entityId), matches existing sink-state patterns (`_rotationByEntity`, `_renderPassByEntity`), small code surface, fully testable. + +Cons: stores per-entity array (matrix per part) — bounded but allocates. Doesn't compose with the animated-entity case (which would need per-tick refresh). + +**Option B — render-thread side-table populated by the dispatcher.** + +The `WbDrawDispatcher` already computes per-part world transforms each frame. Surface them via a side-table the sink queries. Per-frame. + +Pros: free composition with animated entities (the dispatcher transforms whether the entity is animated or not). + +Cons: render-thread / sink-thread coordination concern, bigger architectural surface, the dispatcher would need a new responsibility (publish part transforms) outside its draw-loop hot path. Risk of touching the modern bindless dispatcher's perf budget that N.5/N.5b worked to lock in. + +**Option C — sink-side dat lookup on demand.** + +`ParticleHookSink` calls `_dats.Get(...)` on the hook fire to look up the part transform. Pros: zero state on activator. Cons: introduces dat coupling into the sink (currently dat-free), per-hook-fire dat lookup is a hidden allocation, doesn't compose with animated entities either, and we'd be reading the same Setup multiple times for the same entity. + +### Recommended approach + +**Option A.** It's the smallest surface, matches the existing sink-state pattern, doesn't expand any other layer's responsibilities, and the "doesn't compose with animated entities" downside is intentional — animated entities are explicitly out of scope and will get their own treatment later, possibly via Option B at that time. + +### Test approach + +Mirror the C.1.5a `OnCreate_SetsEntityRotationForHookOffsetTransform` test: construct an entity whose Setup has 2 parts (root at origin + part 1 lifted at (0, 0, 1)), fire a CreateParticleHook with `PartIndex=1` and `Offset.Origin=(0, 0, 0)`, assert the spawned particle's world position is `(0, 0, 1)` (the part's offset, not the root). Add a mutation check: delete the `SetEntityPartTransforms` line and confirm the test fails. + +## §4 EnvCell static-object dispatch decision space (slice B) + +### The problem + +`EnvCell.StaticObjects` are interior decoration objects inside dungeon / building cells. Each StaticObject has a Setup reference and a placement frame. They have NO `ServerGuid` — they're dat-hydrated, not server-spawned. + +Our `EntityScriptActivator.OnCreate` early-returns when `entity.ServerGuid == 0` (atlas-tier guard). So as-is, the activator won't fire DefaultScript for EnvCell statics. + +### Two architectural questions + +**Q1 — synthetic entity ID for tracking + cleanup.** + +`PhysicsScriptRunner` keys active scripts by `(scriptId, entityId)`. `ParticleHookSink` keys per-entity emitter handles by `entityId`. EnvCell statics need a stable, unique 32-bit ID for these tables that won't collide with server guids (and won't collide between two EnvCell statics in different cells). + +Suggested scheme: + +``` +uint syntheticId = 0xC0000000u + | ((landblockId & 0x0000FF00u) << 16) // landblock X byte → bits 24-31 minus high marker + | ((landblockId & 0xFF000000u) >> 8) // landblock Y byte → bits 16-23 + | ((cellIndex & 0x0000FFFFu) << 0); // bits 0-15: cell index within landblock +``` + +…leaving 4 bits for the static-object index within the cell. Adjust bit layout for the actual `(LandblockId, CellIndex, StaticIndex)` distribution. The `0xC0_______u` marker is **above** server guid range and **above** the anonymous-emitter range (`0x80_______u`) used by `ParticleHookSink._anonymousEmitterSerial`, so no collision. + +Sanity check: `WorldEntity.ServerGuid` is `uint`; the `(scriptId, entityId)` dedupe key in the runner only needs uniqueness, not semantic meaning. Either scheme works as long as it's collision-free. + +**Q2 — which adapter walks EnvCell.StaticObjects?** + +Three options: + +- **Option α — piggyback `LandblockSpawnAdapter`.** That adapter already walks `landblock.Entities` for atlas-tier mesh-ref counting. Extending it to also walk `EnvCell.StaticObjects` and fire DefaultScript via the activator keeps the per-landblock-load flow in one place. Cons: blurs the adapter's single responsibility. + +- **Option β — new `EnvCellStaticActivator` class.** Mirror `EntityScriptActivator`'s shape but key by synthetic-id, walking each loaded landblock's EnvCells on load and firing per-static-object. Cons: more code; slight duplication of the activator pattern. + +- **Option γ — `EntityScriptActivator` learns a "static-object" entry point.** Add `OnEnvCellStaticCreate(LoadedLandblock landblock, int cellIndex, int staticIndex, Setup setup, Vector3 worldPos, Quaternion worldRot)` to the existing activator. Compute the synthetic ID inside. Cons: signature creep on the activator. + +Recommended: **Option β.** Keeps the existing activator's `WorldEntity`-shaped contract pure; the new class has a clean per-static-object contract; both share `_scriptRunner` and `_particleSink` instances so no architectural duplication, just two thin orchestrators. + +### Lifecycle + +EnvCell statics live as long as their parent landblock is loaded. On landblock unload, the new activator should stop all scripts for all its synthetic IDs from that landblock. Mirror `LandblockSpawnAdapter`'s `OnLandblockLoaded` / `OnLandblockUnloaded` lifecycle. + +## §5 Animation-hook verification (slice B's quick half) + +Already shipped in C.1: `MotionInterpreter` fires per-keyframe hooks through `IAnimationHookSink` → `ParticleHookSink`. We just haven't verified visually in the current codebase state. + +Procedure: +1. Cast a spell on `+Acdream` (the test character likely has at least one spell + components configured — check or grant if needed). +2. Watch the cast-anim particle effect (sparkles, glyphs, etc.) — does it match retail's casting animation? +3. Optional: trigger an emote with a particle hook (the `\dance` / `\drink` emotes are good candidates if they have particle data). + +If broken, file an issue with the symptom. If working, mark slice B complete on verification. + +## §6 Verification locations + +All in or near Holtburg, within ~30s of `+Acdream`'s spawn: + +- **#56 fix re-verify** — the Town network portal used in C.1.5a. Same procedure as C.1.5a's Task 4 (see [the C.1.5a spec §8](../superpowers/specs/2026-05-12-phase-c1.5a-portals-design.md)). +- **EnvCell chimney** — any cottage / inn within the Holtburg outer perimeter with a smoking chimney in retail. Confirm via dual-client. +- **EnvCell fireplace** — Holtburg Inn interior. Walk inside and stand near the fireplace. Confirm flame particles match retail. +- **Animation-hook verify** — cast a spell standing somewhere safe (outside any aggro range). Compare to retail. + +## §7 File pointers for slice 2 + +- Particle pipeline (Core): [`src/AcDream.Core/Vfx/ParticleSystem.cs`](../../src/AcDream.Core/Vfx/ParticleSystem.cs), [`ParticleHookSink.cs`](../../src/AcDream.Core/Vfx/ParticleHookSink.cs), [`PhysicsScriptRunner.cs`](../../src/AcDream.Core/Vfx/PhysicsScriptRunner.cs). +- Activator (App): [`src/AcDream.App/Rendering/Vfx/EntityScriptActivator.cs`](../../src/AcDream.App/Rendering/Vfx/EntityScriptActivator.cs). +- Streaming bridge (App): [`src/AcDream.App/Streaming/GpuWorldState.cs`](../../src/AcDream.App/Streaming/GpuWorldState.cs), [`LandblockSpawnAdapter.cs`](../../src/AcDream.App/Rendering/Wb/LandblockSpawnAdapter.cs). +- Renderer: [`src/AcDream.App/Rendering/ParticleRenderer.cs`](../../src/AcDream.App/Rendering/ParticleRenderer.cs) — **don't touch** in C.1.5b; bindless migration is N.6 slice 2. +- EnvCell loader: search for `LoadedCell` / `EnvCell.StaticObjects` in `src/AcDream.App/Streaming/` and `src/AcDream.Core/World/`. +- C.1.5a tests as a reference: [`tests/AcDream.Core.Tests/Rendering/Vfx/EntityScriptActivatorTests.cs`](../../tests/AcDream.Core.Tests/Rendering/Vfx/EntityScriptActivatorTests.cs). + +## §8 Open questions to surface during brainstorming + +- Slice A: does the C.1.5a final reviewer's "static-only fix is self-contained" claim hold up? (Section §3 Option A says yes; brainstorming should verify by checking `EntityScriptActivator`'s spawn path doesn't depend on animation state.) +- Slice B: which Setup field actually lives on `EnvCell.StaticObjects` — is it a `SetupId` reference or an inline Setup? Different shape changes the synthetic-ID hash input. +- Slice B: are EnvCell statics ALSO subject to the cold-path timing observation from C.1.5a Task 2 review (firing before the cell is rendered)? + +## §9 Worktree cleanup reminder (one-time, from outside the worktree) + +The C.1.5a worktree directory at `C:/Users/erikn/source/repos/acdream/.claude/worktrees/lucid-burnell-aab524` was not auto-removed because the controller session held a file lock. After this session ends, from any other directory: + +```powershell +git -C "C:/Users/erikn/source/repos/acdream" worktree remove --force ` + "C:/Users/erikn/source/repos/acdream/.claude/worktrees/lucid-burnell-aab524" +``` + +The branch `claude/lucid-burnell-aab524` was successfully deleted; only the worktree directory needs manual cleanup.