From c5412aa7955abb10fe996e381b7dcfe56f288778 Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 7 May 2026 14:34:55 +0200 Subject: [PATCH] =?UTF-8?q?docs(research):=20#49=20handoff=20=E2=80=94=20s?= =?UTF-8?q?cenery=20(X,=20Y)=20drift=20investigation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-contained brief for a fresh agent: bug description with the 2026-05-06 screenshot evidence, what's confirmed (Z fix from #48 is in, Stab placement is correct, dat content matches), five competing hypotheses (LCG noise drift / cell-coord transposition / Align reference / slope filter / SceneInfo lookup), and the cdb retail trace as the gold-standard diagnostic to disambiguate. Same pattern as the #48 handoff — paste the doc into a new session as the prompt. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/research/2026-05-06-issue-49-handoff.md | 306 +++++++++++++++++++ 1 file changed, 306 insertions(+) create mode 100644 docs/research/2026-05-06-issue-49-handoff.md diff --git a/docs/research/2026-05-06-issue-49-handoff.md b/docs/research/2026-05-06-issue-49-handoff.md new file mode 100644 index 0000000..ac67081 --- /dev/null +++ b/docs/research/2026-05-06-issue-49-handoff.md @@ -0,0 +1,306 @@ +# 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.