Merge branch 'claude/quirky-jepsen-fd60f1' — N.4 Week 4 handoff doc

docs/research/2026-05-08-phase-n4-week4-handoff.md

@@ -0,0 +1,318 @@
+# Phase N.4 Week 4 handoff — full draw dispatcher + visual verification + ship
+
+**Use this whole document as the prompt** when handing off to a fresh
+agent. Everything they need to pick up cold is below.
+
+---
+
+## Background you'll need
+
+You're working in `acdream`, a from-scratch C# .NET 10 reimplementation
+of Asheron's Call's retail client. The project's house rule (in
+`CLAUDE.md`) is **the code is modern, the behavior is retail**.
+
+acdream is in the middle of Phase N.4 — the rendering pipeline
+foundation migration to WorldBuilder's `ObjectMeshManager` +
+`TextureAtlasManager`. **Three of the four planned weeks have shipped
+this session (2026-05-08)**:
+
+- Week 1 (commits up through `c49c6ed`): foundation types — feature
+  flag, surface metadata side-table, mesh-extraction + setup-flatten
+  conformance tests, `WbMeshAdapter` constructed against the real WB
+  pipeline.
+- Week 2 (commits up through `36f7a60`): streaming integration —
+  `LandblockSpawnAdapter` routes atlas-tier (procedural / `ServerGuid==0`)
+  GfxObjs to WB's ref-count lifecycle. `WbMeshAdapter.Tick()` drains
+  the WB pipeline's main-thread queues per frame (fixes a real memory
+  leak).
+- Week 3 (commits up through `d30fcb2`): per-instance tier hookup —
+  `AnimatedEntityState` holds per-server-spawned-entity overrides;
+  `EntitySpawnAdapter` routes server-spawned entities through the
+  existing `TextureCache.GetOrUploadWithPaletteOverride` decode path.
+
+**Current state at `main`:** build green, **947 tests pass**, 8
+pre-existing failures only (unchanged from pre-N.4 main). Default-off
+behavior is byte-identical to pre-N.4 main; flag-on (`ACDREAM_USE_WB_FOUNDATION=1`)
+runs both rendering pipelines in parallel — WB silently prepares
+content, but nothing is yet drawn through it.
+
+**Read first:**
+
+- [docs/superpowers/plans/2026-05-08-phase-n4-rendering-foundation.md](../superpowers/plans/2026-05-08-phase-n4-rendering-foundation.md) —
+  the **living-document** plan. Top of file has a Progress table
+  showing Tasks 1-21 ✅ shipped with commit SHAs. Adjustments 1-5
+  document architectural surprises caught during execution. **Read the
+  Adjustments before writing any Task 22 code** — they explain why the
+  current architecture is what it is.
+- [docs/superpowers/specs/2026-05-08-phase-n4-rendering-foundation-design.md](../superpowers/specs/2026-05-08-phase-n4-rendering-foundation-design.md) —
+  the design spec. Architecture / two-tier split / animation handling /
+  data-flow diagrams. Strategic source of truth for "how the pieces
+  fit together."
+- [CLAUDE.md](../../CLAUDE.md) — project-wide rules. The "Currently in
+  flight" section near the top points at the plan.
+
+## What Week 4 is
+
+Seven tasks (22-28). **Task 22 alone is the biggest single task in the
+entire 28-task plan** — it's the moment we flip from "WB is silently
+preparing content" to "WB is drawing content to your screen."
+
+The remaining six tasks are smaller: surface-metadata side-table
+population, sky-pass preservation check, micro-tests round-out, visual
+verification at 5 named locations, flag default-on, delete legacy
+code, finalize plan + memory + ISSUES.
+
+**Task 22 also unlocks the Adjustment 3 mitigation.** Right now
+flag-on has a real FPS regression because both rendering pipelines run
+in parallel (legacy renderer still does atlas-tier upload + draw,
+even though WB is also building atlas state). When Task 22 lands the
+dispatcher AND wires the legacy-renderer short-circuit for atlas-tier
+content, that double-work disappears.
+
+## Two unresolved decisions before Task 22 starts
+
+These need a brainstorm checkpoint at the start of Week 4, NOT a
+"just dispatch":
+
+1. **Adjustment 4 plumbing.** `WorldEntity` doesn't carry
+   `HiddenPartsMask` or `AnimPartChanges` — those live on the
+   network-layer spawn record and don't make it to the render-side
+   entity. Two options:
+   - **A**: add `HiddenPartsMask` + `AnimPartChanges` fields to
+     `WorldEntity`, populate at spawn time. Cleaner long-term; small
+     network→render plumbing change.
+   - **B**: thread them as separate parameters into
+     `EntitySpawnAdapter.OnCreate(entity, hiddenMask, animPartChanges)`.
+     Sidesteps the `WorldEntity` change but couples the spawn-handler
+     to the adapter API.
+
+   Decide before writing Task 22 because the dispatcher reads from
+   `AnimatedEntityState` which currently holds defaults (empty mask +
+   empty override map). Without this resolved, hidden parts won't
+   actually be hidden flag-on.
+
+2. **Surface-metadata side-table population strategy** (Task 23). The
+   spec proposes: when `WbMeshAdapter.IncrementRefCount(id)` is first
+   called for a GfxObj, walk its sub-meshes via `GfxObjMesh.Build`,
+   write each `(gfxObjId, surfaceIdx) → AcSurfaceMetadata` entry into
+   the side-table. The `_metadataPopulated: HashSet<ulong>` field
+   tracks which ids have been processed.
+
+   **But:** if the same GfxObj gets its ref count drop to zero and
+   then re-incremented (LRU eviction + reload), do we re-populate?
+   The metadata is invariant per-GfxObj (surface flags don't change
+   with eviction), so probably no — the `HashSet` is fine. But
+   verify before implementing.
+
+## Watchouts (lessons from Weeks 1-3)
+
+These are real, observed gotchas. Read each before going deeper.
+
+- **The renderer is tier-blind by design (Adjustment 2).** Don't try
+  to put routing decisions in `InstancedMeshRenderer` or any mesh
+  uploader. Routing belongs at the **spawn-callback layer**:
+  `LandblockSpawnAdapter` for atlas-tier, `EntitySpawnAdapter` for
+  per-instance. Task 22's dispatcher reads from those adapters'
+  per-entity state at draw time; it doesn't make tier decisions.
+
+- **Flag-off must stay byte-identical to pre-N.4.** Every Task-22 code
+  path must have a `WbFoundationFlag.IsEnabled` gate. Default-off path
+  is what users see; we can't regress it.
+
+- **WB's pipeline does work even when you're not draining its results.**
+  Adjustment 3: `IncrementRefCount` triggers background mesh prep,
+  texture decode, atlas allocation. `WbMeshAdapter.Tick()` already
+  drains the upload queue per frame. The remaining FPS cost is
+  pure dual-pipeline cost (legacy + WB doing the same upload work).
+  Task 22's short-circuit fixes this.
+
+- **`MeshRef.SurfaceOverrides`** is the per-surface texture-swap data
+  carried by spawned entities. `GfxObjSubMesh.SurfaceId` is what gets
+  swapped. Task 22's draw loop must consult both: the entity's
+  `MeshRef.SurfaceOverrides` for explicit swaps, and otherwise the
+  mesh's built-in `SurfaceId`.
+
+- **Conformance tests catch divergences early.** Per N.1's rotation
+  bug: write the conformance test BEFORE the substitution. The
+  matrix-composition test (`(entityWorld) × (animation) × (restPose)`)
+  is the load-bearing one for Task 22 — pin it before integrating.
+
+- **`WbMeshAdapter.Tick()` is required.** It's already wired into
+  `GameWindow.OnRender`. Task 22's dispatcher needs the upload queue
+  drained BEFORE it tries to draw, so order in OnRender is:
+  `_wbMeshAdapter?.Tick()` → `_wbDrawDispatcher?.Draw(...)` → other
+  draw work.
+
+- **Name retail decomp first; Phase N.4 doesn't change that rule.**
+  Task 22's matrix composition uses standard graphics math — no AC-
+  specific algorithms — so the "grep `named-retail/` first" workflow
+  doesn't apply to the matrix code itself. But for any AC-specific
+  question that surfaces during integration (e.g., "does retail
+  render hidden parts as zero-alpha or skip them entirely?"), grep
+  `docs/research/named-retail/acclient_2013_pseudo_c.txt` first.
+
+## Acceptance criteria for Week 4
+
+From the plan:
+
+- [ ] All conformance tests pass (Tasks 3, 4, 20 — already shipped;
+      verify still green after Task 22 lands).
+- [ ] All component micro-tests pass (Tasks 11, 17, 18, 19, 22 —
+      Task 22 adds matrix-composition tests).
+- [ ] All existing tests still pass. 8 pre-existing failures don't
+      count.
+- [ ] Build green throughout.
+- [ ] Visual verification at 5 named locations passes:
+  1. Holtburg outdoor — terrain props, scenery, buildings, NPCs,
+     characters all render correctly.
+  2. Drudge Hideout (or comparable) — EnvCell + interior lighting +
+     animated creatures.
+  3. Foundry — heavy NPC traffic + customized appearances.
+  4. A character with extreme palette overrides.
+  5. Long roam (5+ minutes) — GPU memory stabilizes (LRU eviction
+     fires).
+- [ ] Memory budget enforcement actually verified (Task 13 was
+      deferred to here; Task 22 makes it testable because GL resources
+      finally get allocated for LRU to evict).
+- [ ] Sky pass renders identically (load-bearing — sky's
+      `Translucent+ClipMap` cloud sheet, raw-`Additive` fog skip,
+      `Luminosity` keyframe handling all flow through the side-table
+      via `AcSurfaceMetadata`).
+- [ ] Flag flipped to default-on at the end (Task 26).
+- [ ] Legacy code paths deleted (Task 27).
+- [ ] Roadmap + memory + ISSUES updated (Task 28).
+
+## Tasks 22-28 — quick map
+
+Full detail is in the plan. Brief here:
+
+- **22 — `WbDrawDispatcher` full draw loop.** ~1-2 days. Atlas-tier
+  + per-instance-tier draw with matrix composition. Reads from
+  `WbMeshAdapter.GetRenderData(id)` for atlas content; reads from
+  `EntitySpawnAdapter.GetState(serverGuid)` for per-instance state;
+  composes per-part `(entity × animation × rest-pose)` matrices;
+  pushes uniforms; issues GL draws. **Also wires the legacy-
+  renderer short-circuit** for atlas-tier content (the Adjustment 3
+  fix).
+- **23 — Surface-metadata side-table population.** ~half day. Hook
+  into `WbMeshAdapter.IncrementRefCount` so that on first registration
+  of a GfxObj, the side-table gets populated with one
+  `AcSurfaceMetadata` per surfaceIdx (using `GfxObjMesh.Build`'s
+  metadata as the source of truth).
+- **24 — Sky-pass preservation check.** ~half day. Verify the sky
+  pass's `NeedsUvRepeat` / `DisableFog` / `Luminosity` flow through
+  the side-table to `SkyRenderer` correctly. Likely no code change;
+  smoke-test sky rendering with flag on, weather/day-night cycle.
+- **25 — Component micro-tests round-out.** Audit existing tests
+  against the spec's Testing section. Probably nothing to add since
+  Tasks 11/17/18/19/22 already cover the listed micro-tests.
+- **26 — Visual verification + flag default-on.** Human-in-the-loop
+  walk through the 5 named locations. If clean, flip
+  `WbFoundationFlag.IsEnabled` from `== "1"` to `!= "0"` so flag-on
+  becomes the default.
+- **27 — Delete legacy code paths.** Remove the now-unused legacy
+  upload code in `StaticMeshRenderer` + `InstancedMeshRenderer`.
+  N.6 fully replaces these files anyway.
+- **28 — Update roadmap + memory + ISSUES + finalize plan.** Mark
+  N.4 shipped in the roadmap's Live ✓ table. File any cosmetic
+  deltas as ISSUES. Add a memory note if a durable lesson emerged.
+  Flip the plan's status header from "Living document — work in
+  progress" to "Final state — phase shipped (merge `<sha>`)".
+
+## Where to start
+
+1. **Read the three "Read first" docs above end-to-end.** Especially
+   the Adjustments section in the plan — those are the architectural
+   constraints Task 22 must respect.
+2. **Decide Adjustment 4 plumbing** (option A vs B from above). This
+   is a small brainstorm checkpoint, not a multi-question
+   `superpowers:brainstorming` skill invocation. Document the choice
+   inline in the plan as Adjustment 6.
+3. **Don't create a new worktree.** The existing branch
+   `claude/quirky-jepsen-fd60f1` and worktree
+   `.claude/worktrees/quirky-jepsen-fd60f1` are clean and ready.
+   Submodule already initialized. Build green.
+4. **Use `superpowers:subagent-driven-development`** to execute Week 4
+   task-by-task. Pattern from Weeks 1-3: dispatch one subagent per
+   task (or batch of related tasks), use Sonnet for implementation,
+   merge to main per logical chunk, update the plan's Progress table
+   as commits land.
+5. **Pause for visual verification at Task 26.** This is a human-in-
+   the-loop step — needs you to walk the 5 named locations.
+
+## Open questions a fresh agent might hit
+
+- **Q: Why did Adjustment 5 mark Task 20 (per-instance decode
+  conformance) as "structural"?** Because both old and new paths call
+  the same `TextureCache.GetOrUploadWithPaletteOverride` function. We
+  preserved the decode logic exactly; the seam is at the call site,
+  not at the algorithm. Byte-equality is automatic.
+
+- **Q: Can I delete `InstancedMeshRenderer`?** Not in N.4. The plan
+  marks it as "becomes a thin adapter in N.4, fully replaced in N.6."
+  Task 27 deletes the legacy upload paths inside it but keeps the
+  file as a draw-orchestration adapter until N.6.
+
+- **Q: What's the memory budget check actually checking?** GPU memory
+  stabilizes during long roam. WB's `_maxGpuMemory = 1 GB` triggers
+  LRU eviction once the cache exceeds that. We verify by walking
+  for 5+ minutes at radius 7 (49 landblocks visible at any time) and
+  confirming GPU memory in the title bar plateaus rather than
+  growing unboundedly.
+
+- **Q: What happens if Task 22 takes longer than expected?** The
+  living-document convention says document Adjustments inline. If
+  Task 22 needs to split (e.g., atlas-tier draw lands first, per-
+  instance tier in a follow-on commit), that's fine — just update
+  the Progress table and add an Adjustment explaining the split.
+
+## Useful greps and commands
+
+- `dotnet build --verbosity quiet 2>&1 | tail -3` — quick build check.
+- `dotnet test --verbosity quiet 2>&1 | tail -3` — full test suite.
+- `git -C C:\Users\erikn\source\repos\acdream log --oneline -10` —
+  recent main commits.
+- `grep -rn "WbFoundationFlag.IsEnabled" src/` — every place we gate
+  on the flag (audit before flipping default-on in Task 26).
+- `grep -rn "_wbMeshAdapter\|_wbSpawnAdapter\|_wbEntitySpawnAdapter" src/` —
+  every WB adapter wiring point.
+
+## Smoke-test launch (PowerShell)
+
+```powershell
+# Kill any stale processes first
+Get-Process -Name AcDream.App -ErrorAction SilentlyContinue | Stop-Process -Force
+Start-Sleep -Seconds 4
+
+# Flag-on at radius 7 — Week 4 dev environment
+$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_USE_WB_FOUNDATION = "1"
+$env:ACDREAM_STREAM_RADIUS = "7"
+dotnet run --project src\AcDream.App\AcDream.App.csproj --no-build -c Debug 2>&1 |
+    Tee-Object -FilePath "n4-week4-smoke.log"
+```
+
+(Drop the `ACDREAM_USE_WB_FOUNDATION` line for flag-off comparison.)
+
+## Adjustments index — quick reference
+
+For full text, see the plan document (each is a `### Adjustment N`
+subsection under Task 6's old position, in chronological order):
+
+1. **`DefaultDatReaderWriter` discovery** (2026-05-08) — no
+   dat-reader bridge needed; WB ships a usable concrete
+   implementation.
+2. **Renderer is tier-blind** (2026-05-08) — routing belongs at
+   spawn callbacks, not in the renderer.
+3. **FPS regression = dual-pipeline cost** (2026-05-08) — both
+   pipelines run in parallel until Task 22's short-circuit lands.
+4. **`WorldEntity` lacks HiddenParts/AnimPartChange fields**
+   (2026-05-08) — plumbing deferred; Task 22 needs to resolve
+   (option A: add fields; option B: thread as separate args).
+5. **Task 20 is structural** (2026-05-08) — same function called
+   both paths, byte-equality automatic, no test file needed.