docs(vfx #C.1.5b): handoff for next slice (issue #56 + EnvCell statics)

Self-contained handoff doc for the C.1.5b session. §1 is a copy-paste startup prompt for a fresh Claude Code session; §2+ is detailed context (commits shipped in C.1.5a, decision space for the #56 fix including precompute-per-spawn vs render-thread-side-table options, EnvCell synthetic-id scheme, walker-class placement options, file pointers, verification plan). Slice will resolve issue #56 first (per-part transform handling for static entities) before adding the EnvCell static-object DefaultScript walker, per the C.1.5a final reviewer's recommendation that resolving #56 unblocks slice 2's visual delight gate (the multi-emitter collapse symptom affects portals, chimneys, and fireplaces alike). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-11 18:22:01 +02:00 · 2026-05-11 18:22:01 +02:00 · ad261539d6
commit ad261539d6
parent 88bda12d98
1 changed files with 320 additions and 0 deletions
--- a/docs/plans/2026-05-12-phase-c1.5b-handoff.md
+++ 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<WorldEntity, uint>)`; `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<Setup>(...)` 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.