diff --git a/docs/research/2026-05-06-issue-49-handoff.md b/docs/research/2026-05-06-issue-49-handoff.md deleted file mode 100644 index ac67081d..00000000 --- a/docs/research/2026-05-06-issue-49-handoff.md +++ /dev/null @@ -1,306 +0,0 @@ -# Issue #49 handoff — scenery (X, Y) placement drifts from retail - -**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**: every -gameplay-affecting algorithm is ported faithfully from the named retail -decomp at `docs/research/named-retail/` (Sept 2013 EoR build PDB, -99.6% function-name recovery, full pseudo-C in -`acclient_2013_pseudo_c.txt`). Read `CLAUDE.md` end-to-end before -touching code — the workflow ("grep named-retail first → -cross-reference → pseudocode → port → conformance test → integrate") -is mandatory. **Especially relevant for this issue:** the cdb-attach -retail debugger workflow is documented in `CLAUDE.md` ("Retail debugger -toolchain") and `memory/project_retail_debugger.md`. You will need it. - -Outdoor scenery (trees, bushes, rocks, fences, small props) is -generated procedurally per landblock by [`SceneryGenerator`](../../src/AcDream.Core/World/SceneryGenerator.cs), -which is a faithful port of retail's `chunk_005A0000.c` placement -math. The generator returns `(GfxObjId, LocalPosition, Rotation, -Scale)` triples; the renderer combines `LocalPosition.X / .Y` with -the landblock origin and adds the terrain ground Z to get `finalZ`. - -`#48` fixed the Z (the bilinear fallback in -`GameWindow.SampleTerrainZ` had its diagonal triangle-pair arms -swapped). Trees now sit flush on terrain. **`#49` is the X/Y -counterpart: same scene, same character coords, in some places -acdream and retail place trees at different (X, Y).** - -## The bug, in one paragraph - -While verifying `#48` at Holtburg landblock `0xA9B30001` (acdream -character at local `(15.20, 12.54, 91.07)` with retail running side -by side at the same coords), the user spotted a tree placed in -**dramatically different (X, Y)** in acdream vs retail. In acdream, -the tree appears on the LEFT of the road, near a chess board / -picnic-bench area. In retail at the same character position, that -spot is empty; the tree (or a tree of the same species) sits FAR -across the road on the right (east) side. Side-by-side screenshot -pair captured 2026-05-06 (attached to the issue). User reports they -suspect this is widespread, not isolated to one tree. - -This is **not** a Z bug — every tree in the same screenshot has -its trunk meeting the visible terrain. It's also **not** the -LandBlockInfo Stab path — the chess board / bench themselves are -correctly placed (acdream and retail agree on those), so the -landblock origin and `lbOffset` math are right. The bug is in the -procedural scenery placement (`SceneryGenerator`). - -The user's frustration: the scenery math was previously audited -against the decomp by `eeee4c5 chore(scenery): audit -SceneryGenerator against decompiled acclient.exe — all MATCH`. The -audit confirmed the algorithm shape but did not prove runtime -agreement against the real retail client. That's the gap this -investigation closes. - -## Acceptance criterion - -- Side-by-side outdoor screenshot pair (acdream vs retail, same - character coords, same time of day) shows scenery positions - matching at multiple landblocks. The user verifies visually. -- Quantitative agreement: a cdb retail trace + acdream - `ACDREAM_DUMP_SCENERY_Z=1` log together show every spawn at the - same (gfxObjId, worldX, worldY, scale, heading) within float - precision for at least one landblock end-to-end. -- **Don't break:** all scenery that already places correctly must - continue to. The Z fix from `#48` must not regress (the new - `TerrainSurfaceTests.SampleZFromHeightmap_AgreesWithInstance_AcrossWholeLandblock` - conformance test must stay green). - -## Confirmed facts (don't redo these) - -- The Z fix from `#48` is in. `TerrainSurface.SampleZ` (instance, - used by physics / player) and `TerrainSurface.SampleZFromHeightmap` - (static, used by scenery hydration's bilinear fallback) share - `TerrainSurface.InterpolateZInTriangle`. Trees sit flush. -- LandBlockInfo Stab placement (chess boards, benches, building - decorations) is correct — landblock origin and `lbOffset` math - agree with retail. -- The `46544ef fix(scenery): drop non-retail extra-road-vertex - suppression` commit is **not** the cause. User has confirmed - that was a good fix. -- Same retail dat as we use; retail places trees flush AND in - a different XY for at least one species. So the dat content is - the same — the procedural placement algorithm differs. -- `ACDREAM_DUMP_SCENERY_Z=1` (gated by env var, kept committed - after `#48`) dumps every spawn's `(gfxObjId, worldX, worldY, - groundZ, partT, zRange, scale)` to launch.log. Reuse it for `#49`. - -## Competing hypotheses - -You need a cdb retail trace to disambiguate. Don't pre-commit until -the data tells you which one. - -### H1 — LCG noise constants drift - -`SceneryGenerator` uses an LCG-based hash of `(globalCellX, -globalCellY, j)` to produce the per-instance displacement noise. -The constants `1813693831`, `1360117743`, `1888038839`, `1109124029` -came from the decomp port. **Predicts:** an off-by-one constant -or a sign flip on the `dx` / `dy` derived from the noise; the -displacement vector relative to the cell center is wrong by a -constant or by a sign-flipped axis. - -**Fix if H1:** the cdb trace will show retail's -`(global_cell_x, global_cell_y, j) → (dx, dy)` triples. Diff against -ours; fix the math to match. - -### H2 — Cell-coordinate handedness / transposition - -`SceneryGenerator` computes `globalCellX = landblockX * 8 + cellX` -and similarly for Y. The `cellX` / `cellY` indexing within a -landblock might be reversed relative to retail. **Predicts:** all -trees in the same cell get a consistent (X, Y) shift; the shift -varies across cells but follows a transposition pattern (e.g. -swapping X and Y around the cell's NW corner). - -**Fix if H2:** swap the cell-indexing convention in -`SceneryGenerator`'s noise inputs to match retail. - -### H3 — `obj.Align != 0` path uses wrong reference - -Retail has a separate alignment path (`FUN_005a6f60`) that aligns -scenery to the landcell polygon's normal. `SceneryGenerator` -implements this via the `Align` flag — but the reference point -(cell center vs cell origin vs vertex) might differ. **Predicts:** -only species with `Align != 0` are misplaced; species with `Align -== 0` are placed correctly. - -**Fix if H3:** correct the reference-point math for the Align -branch. - -### H4 — Slope filter rejects different cells than retail - -The slope filter rejects cells where the terrain normal Z falls -outside `obj.MinSlope..obj.MaxSlope`. If our normal computation -disagrees with retail's, we reject some cells retail accepts (or -vice versa) — the spawn either disappears or moves to the next -eligible cell. **Predicts:** species-specific differences correlated -with sloped vs flat cells; ours has more or fewer instances per -landblock than retail. - -**Fix if H4:** port retail's slope filter math exactly. Note the -related `TerrainSurface.SampleSurface` produces the canonical -plane normal; that's likely the right source. - -### H5 — Region.SceneInfo lookup uses wrong scenery list - -The cell's `(terrain_type, road_type, scenery_type)` tuple selects -which scenery list applies. If our terrain-type extraction differs -from retail's, the wrong list is selected and we spawn a different -species (or none) at each cell. **Predicts:** species mismatches -not just position mismatches. - -**Fix if H5:** audit `Region.SceneInfo` traversal vs retail. - -## Required first step — cdb trace of retail's placement - -This is the gold-standard answer to "what does retail actually do at -runtime?" The cdb toolchain is already set up; verify with -`tools/pdb-extract/check_exe_pdb.py "C:/Turbine/Asheron's Call/acclient.exe"` -(expect `=== MATCH ===`). Then: - -1. Have the user launch retail and walk to Holtburg landblock - `0xA9B30001`, character at local `(~15, ~13)`. Same spot as the - `#48` verification. -2. Write a `.cdb` script with breakpoints on the placement - functions. Candidates from the named-retail decomp: - - `acclient!CLandBlock::get_land_scenes` (the outer driver) - - `acclient!FUN_005a6e60` (per-cell scenery placement, called - by `get_land_scenes`) - - `acclient!FUN_005a6cc0` (cell-noise / displacement) - - `acclient!CPhysicsObj::set_initial_frame` (final placement) -3. Each breakpoint action: log `gfxObjId` (or setup id) and the - final `(worldX, worldY)` going into `set_initial_frame`. Use - `gc` (go conditional) to avoid trapping the process. -4. Auto-detach via `qd` after 2000 hits to stop the trace. -5. Diff the retail trace against acdream's - `ACDREAM_DUMP_SCENERY_Z=1` log for the same landblock. The - spawn that's offset will be obvious; the offset pattern picks - the hypothesis. - -**Watchouts (per `project_retail_debugger.md`):** - -- `qd` / `q` / `qq` are FORBIDDEN inside bp actions — silently - ignored. Use `.detach`. -- Don't `Stop-Process -Force` cdb — kills the debuggee. -- Per-frame breakpoints with `.printf` lag retail to ACE timeout — - use counter-only actions for high-frequency targets. - -## Files most likely to need edits - -- [`src/AcDream.Core/World/SceneryGenerator.cs`](../../src/AcDream.Core/World/SceneryGenerator.cs) — - placement math (LCG noise, displacement, rotation, scale, slope - filter, Align branch). 99% of the fix lands here. -- [`src/AcDream.Core/Terrain/TerrainBlending.cs`](../../src/AcDream.Core/Terrain/TerrainBlending.cs) — - if H4 / H5, the terrain-type extraction or normal computation. -- [`src/AcDream.App/Rendering/GameWindow.cs:4546-4720`](../../src/AcDream.App/Rendering/GameWindow.cs:4546) — - scenery hydration loop. The diagnostic dump is here. -- New `docs/research/2026-05-XX-issue-49-fix-pseudocode.md` — - document the chosen hypothesis + retail-trace diff before - porting. - -## Test workflow (acdream side) - -```powershell -Get-Process -Name AcDream.App -ErrorAction SilentlyContinue | Stop-Process -Force -Start-Sleep -Seconds 4 - -$env:ACDREAM_DUMP_SCENERY_Z = "1" -$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" -dotnet run --project src\AcDream.App\AcDream.App.csproj --no-build -c Debug 2>&1 | - Tee-Object -FilePath "launch_issue49.log" -``` - -User spawns at Holtburg, types `/loc` to print position into the -log, identifies a misplaced tree visually, reports the spot. You -grep the log for entries near that spot, build a per-landblock -table, diff against the cdb retail trace. - -After the fix lands: rerun the test workflow, user verifies all -trees match retail at multiple landblocks (Holtburg, plus at least -one more region — Eastham / Glenden Wood). The -`SampleZFromHeightmap_AgreesWithInstance_AcrossWholeLandblock` test -must stay green throughout. - -## Constraints / don't-break - -- `#48` Z fix must stay correct. The conformance test pins both - sampler paths together — if you touch `TerrainSurface`, that test - must stay green. -- LandBlockInfo Stab placement (chess boards, benches, etc) is - already correct — don't touch the Stab code path. -- The `46544ef` road-vertex suppression drop was a good fix per - the user — don't reintroduce that suppression. -- The `eeee4c5` audit comment in `SceneryGenerator.cs` is now - partially wrong (it claimed "all MATCH" but a real bug exists). - Once the runtime trace lands, update the comment to reflect what - was actually verified vs only-shape-checked. -- 8 pre-existing motion / BSP step-up test failures are baseline - — leave them. New failures from your changes are not OK. - -## When to stop and ask - -Per CLAUDE.md, ask only for: - -- Visual verification (user looking at the client side-by-side - with retail). -- Genuine architectural disagreements (e.g. all five hypotheses - prove wrong and a sixth needs brainstorming). -- Hard-to-reverse destructive actions. - -Otherwise act. Don't ask "should I continue?". - -## References to consult - -- [`docs/research/2026-05-06-issue-48-fix-pseudocode.md`](2026-05-06-issue-48-fix-pseudocode.md) — - the Z fix pseudocode; precedent for the static / instance sampler unification pattern. -- [`docs/research/named-retail/acclient_2013_pseudo_c.txt`](named-retail/acclient_2013_pseudo_c.txt) — - Sept 2013 PDB pseudo-C; **grep this FIRST** for `CLandBlock::get_land_scenes`, - `FUN_005a6e60`, `FUN_005a6cc0`, `FUN_005a6f60`, - `CPhysicsObj::set_initial_frame`. -- [`docs/research/named-retail/symbols.json`](named-retail/symbols.json) — - raw symbol → address map for cdb breakpoints. -- ACME `WorldBuilder/Editors/Landscape/StaticObjectManager.cs` — - scenery rendering pipeline reference. -- `memory/project_retail_debugger.md` — the cdb attach workflow, - watchouts, what NOT to do. -- `memory/feedback_acviewer_as_oracle.md` — when a rendering bug - shows up in BOTH acdream AND ACViewer, it's in shared - dat-interpretation logic, not our renderer. If you have an ACViewer - install handy, a quick check there might short-circuit the cdb - workflow. - -## When the fix lands - -1. Update `docs/ISSUES.md` to mark `#49` DONE with the commit SHA. -2. Add a new line to `memory/project_phase_a1_state.md` (or - wherever scenery state lives) noting the runtime-traced - conformance. -3. Update the comment block at the top of `SceneryGenerator.cs` - from "audit confirmed shape" to "audit + cdb runtime trace - confirmed identity at landblock 0xA9B3 (and any others traced)". -4. Commit on the current branch with a co-authored message - (`Co-Authored-By: Claude Opus 4.7 (1M context) - `), merge to main, push. - -## Final note - -Don't pre-commit to a hypothesis. The cdb trace tells you the truth -in 30 minutes; speculation will burn hours. Issue `#48` proved the -diagnostic-first pattern works — `ACDREAM_DUMP_SCENERY_Z=1` plus -one log line picked the right hypothesis (H2 in `#48`'s case) -immediately. For `#49` the equivalent diagnostic is the cdb retail -trace. Use it.