From 831f7d416b0124838790bb4559c3ce37637f7356 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 8 May 2026 14:54:12 +0200 Subject: [PATCH] docs(N.4): Week 4 handoff for the next agent Self-contained briefing for whoever picks up Week 4 (Tasks 22-28): the WbDrawDispatcher full draw loop + sky-pass preservation + visual verification + flag default-on + legacy-code deletion + plan finalization. Highlights two unresolved decisions that need a brainstorm checkpoint at the start of Week 4 (NOT 'just dispatch'): - Adjustment 4 plumbing: WorldEntity needs HiddenPartsMask + AnimPartChanges fields, OR EntitySpawnAdapter.OnCreate takes them as separate parameters. Decision before Task 22 writes code. - Surface-metadata side-table population strategy for Task 23. References the living-document plan + spec + 5 prior adjustments so a fresh agent has full context cold. Co-Authored-By: Claude Opus 4.6 --- .../2026-05-08-phase-n4-week4-handoff.md | 318 ++++++++++++++++++ 1 file changed, 318 insertions(+) create mode 100644 docs/research/2026-05-08-phase-n4-week4-handoff.md diff --git a/docs/research/2026-05-08-phase-n4-week4-handoff.md b/docs/research/2026-05-08-phase-n4-week4-handoff.md new file mode 100644 index 0000000..6ab30fe --- /dev/null +++ b/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` 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 ``)". + +## 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.