diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 3b790ae..7210cba 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -46,6 +46,108 @@ Copy this block when adding a new issue: # Active issues +## #48 — A few specific scenery trees hover above terrain (per-GfxObj Z misplacement) + +**Status:** OPEN +**Severity:** LOW (cosmetic; ~3 trees per landblock, easy to ignore but obvious once spotted) +**Filed:** 2026-05-06 +**Component:** rendering / scenery placement / terrain Z sampling + +**Description:** In outdoor landblocks, a small subset of tree +scenery instances render visibly **floating above the terrain** +(trunk base ~0.5–1.5 m above the ground line). The vast majority +of scenery (other tree species, bushes, rocks) sits flush. The bug +is **per-GfxObj-id**: the same handful of species float wherever +they spawn; other species at the same (x, y) cell sit correctly. +Side-by-side with retail in the same area: retail places the same +species flush. User-confirmed via screenshot pair 2026-05-06. + +The user noted this is the only thing left wrong with terrain +rendering (canopy density / shape were *not* the issue — those +match retail when looked at carefully). The bug is purely vertical +offset on a few species. + +**Investigation 2026-05-06:** + +[`SceneryGenerator.cs:204`](src/AcDream.Core/World/SceneryGenerator.cs:204) +returns `LocalPosition.Z = obj.BaseLoc.Origin.Z` (just the +ObjectDesc's BaseLoc Z offset, no terrain). [`GameWindow.cs:4642`](src/AcDream.App/Rendering/GameWindow.cs:4642) +adds the terrain ground Z: + +```csharp +float groundZ = _physicsEngine.SampleTerrainZ(worldPx, worldPy) + ?? SampleTerrainZ(lb.Heightmap, _heightTable, localX, localY); +float finalZ = groundZ + spawn.LocalPosition.Z; +``` + +Both samplers claim to use the AC2D split-direction terrain mesh +formula. Player feet land flush, so player Z sampling is correct; +scenery for most species is also flush; only specific GfxObjs +float. + +**Three competing hypotheses (need one diagnostic to disambiguate):** + +1. **Per-GfxObj origin convention.** Most AC tree GfxObjs are + authored with local origin at the trunk base (mesh vertices + have `Z >= 0` measured up from the origin). A few species + may be authored with origin at bbox-center or visual top — + for those, `finalZ = groundZ + BaseLoc.Z` plants the *center* + at ground and the visible trunk floats by half its height. + Per-GfxObj-id ⇒ deterministic across instances ⇒ fits the + "same 3 species everywhere" pattern. + +2. **Physics-sampler vs bilinear-fallback Z mismatch on + NE↔SW-cut cells.** The physics path uses the AC2D + split-direction formula. The bilinear-fallback at + `GameWindow.cs:4643` uses naive bilinear over heightmap + corners — wrong on cells whose visible triangle slopes + the *other* way. If physics hasn't registered a landblock + yet when scenery hydrates (timing race), affected scenery + uses the bilinear sampler and lands on a different Z than + the visible terrain. Player Z is fine because player movement + always goes through the physics sampler. + +3. **Same close-degrade story as #47, applied to scenery.** Some + tree GfxObjs have `DIDDegrade` tables; slot 0 (close-detail) + and the base-LOD-3 mesh may have different mesh-local origins. + We currently draw the base GfxObj id directly for scenery (the + close-degrade resolver is scoped to humanoid setups only). + Retail draws slot 0 for nearby trees. If slot-0 has origin at + trunk-base while base-LOD-3 has origin at bbox-center, those + species float by exactly the offset between the two origins. + +**Cheapest first move:** add a one-shot scenery placement dump +gated by `ACDREAM_DUMP_SCENERY_Z=1` that logs, per spawn: + +``` +[scenery-z] gfxObj=0xXXXXXXXX setupOrGfx=… worldPos=(x,y,z) + BaseLoc.Z=… groundZ=… meshZRange=[zMin..zMax] + hasDIDDegrade=true/false degrades[0]=0xXX +``` + +User identifies one floating tree → grep that GfxObj id in the +log → look at meshZRange and `hasDIDDegrade`. That tells us +hypothesis 1 (zMin > 0 by the float amount), hypothesis 2 (matching +species correctly placed elsewhere → timing race), or hypothesis 3 +(`hasDIDDegrade=true` and slot 0 mesh has different zMin). One log +sample answers the question. + +**Files:** + +- [`src/AcDream.Core/World/SceneryGenerator.cs:204`](src/AcDream.Core/World/SceneryGenerator.cs:204) — BaseLoc.Z passthrough +- [`src/AcDream.App/Rendering/GameWindow.cs:4632-4655`](src/AcDream.App/Rendering/GameWindow.cs:4632) — groundZ resolution + finalZ assembly +- [`src/AcDream.Core/Physics/TerrainSurface.cs`](src/AcDream.Core/Physics/TerrainSurface.cs) — physics sampler (AC2D split-direction formula) +- `SampleTerrainZ` (private, in GameWindow.cs) — bilinear fallback +- [`src/AcDream.Core/Meshing/GfxObjDegradeResolver.cs`](src/AcDream.Core/Meshing/GfxObjDegradeResolver.cs) — close-degrade resolver if hypothesis 3 confirmed; would need scenery-scope expansion (drop the `IsIssue47HumanoidSetup` gate or add a scenery-aware variant) + +**Acceptance:** All scenery species rest flush on the visible +terrain mesh in side-by-side outdoor screenshots vs retail. No +regression on the species that already render correctly. + +**Handoff:** [docs/research/2026-05-06-issue-48-handoff.md](docs/research/2026-05-06-issue-48-handoff.md) + +--- + ## #39 — Run↔Walk cycle transition not visible on observed player remotes (acdream-as-observer) **Status:** OPEN diff --git a/docs/research/2026-05-06-issue-48-handoff.md b/docs/research/2026-05-06-issue-48-handoff.md new file mode 100644 index 0000000..f598e21 --- /dev/null +++ b/docs/research/2026-05-06-issue-48-handoff.md @@ -0,0 +1,303 @@ +# Issue #48 handoff — a few specific tree species hover above terrain + +**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). Read `CLAUDE.md` end-to- +end before touching code — the workflow ("grep named-retail first → +cross-reference → pseudocode → port → conformance test → integrate") +is mandatory. + +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`. This already works correctly +for the vast majority of scenery in every landblock we've tested. + +## The bug, in one paragraph + +A small subset of tree species — three specific GfxObjs that the +user spotted in Holtburg, possibly more elsewhere — render with +their trunk base **floating ~0.5–1.5 m above the visible terrain +mesh**, while every other scenery instance in the same landblock +sits flush. The bug is **per-GfxObj-id**, deterministic across +instances: those exact tree species float wherever they spawn, and +correctly-placed species at the same (x, y) cell coordinates do not. +Same dat as retail; retail places those same species flush. +Side-by-side screenshot pair confirmed 2026-05-06. + +This is the **only** outstanding terrain-rendering issue per the +user's report. Canopy density / shape are NOT the issue — those +match retail. The defect is purely vertical offset on a few species. + +## Acceptance criterion + +- All scenery species rest flush on the visible terrain mesh in + side-by-side outdoor screenshots vs retail. The user verifies + visually. +- **Don't break:** all the species that currently render flush + must continue to render flush. Player feet must continue to land + on the terrain (player Z snap is unaffected by this bug — fix + only the scenery path). + +## Confirmed facts (don't redo these) + +- [`SceneryGenerator.cs:204`](../../src/AcDream.Core/World/SceneryGenerator.cs:204) + returns `LocalPosition.Z = obj.BaseLoc.Origin.Z` (the ObjectDesc's + BaseLoc Z offset, with no terrain involvement). The intent is for + the renderer to add the ground Z later. +- [`GameWindow.cs:4640-4644`](../../src/AcDream.App/Rendering/GameWindow.cs:4640): + ```csharp + var worldPx = localX + lbOffset.X; + var worldPy = localY + lbOffset.Y; + float groundZ = _physicsEngine.SampleTerrainZ(worldPx, worldPy) + ?? SampleTerrainZ(lb.Heightmap, _heightTable, localX, localY); + float finalZ = groundZ + spawn.LocalPosition.Z; + ``` + Physics sampler first; bilinear fallback if physics hasn't + registered the landblock yet. +- Player feet land flush on terrain (the physics sampler is correct + for movement). Most species also land flush (so the placement + arithmetic is correct in aggregate). Only specific species float. +- This is **not** the same fix as #47 (humanoid bulky-body) — that + was DIDDegrade close-detail mesh selection. But hypothesis 3 + below considers whether scenery has the same problem. + +## Three competing hypotheses + +You need one log sample to disambiguate. Don't pre-commit to a +hypothesis until the diagnostic dump tells you which one it is. + +### H1 — Per-GfxObj origin convention + +Most AC tree GfxObjs are authored with mesh-local origin at the +trunk base: vertex `Z` values are `>= 0`, measured upward from the +ground-contact point. A few species may be authored with origin at +the **bbox center** or **visual top** instead. For those, our +`finalZ = groundZ + BaseLoc.Z` plants the *origin* at ground — +which means the visible trunk hovers by half-tree-height (center- +origin) or the whole tree-height (top-origin). + +**Predicts:** the offending GfxObj's `meshZRange.zMin` is +significantly greater than 0 by exactly the float amount. + +**Fix if H1:** at scenery hydration, when computing `finalZ`, +shift down by `meshZRange.zMin` so the lowest vertex lands at +`groundZ + BaseLoc.Z` instead of the origin landing there. +Concretely: `finalZ = groundZ + spawn.LocalPosition.Z - meshZMin` +where `meshZMin = min(vertex.Z)` across all the GfxObj's vertices. +Cache the per-GfxObj zMin in the existing scenery hydration cache +so we don't recompute. Verify retail does this — likely there's a +`ObjectDesc::PlaceOnGround` or `Frame::adjust_to_ground` retail +function. **Grep named-retail first.** + +### H2 — Physics-sampler vs bilinear-fallback Z mismatch + +The physics path uses the AC2D split-direction-aware terrain mesh +formula (matches the visible terrain mesh exactly). The bilinear +fallback at line 4643 uses naive bilinear over the four heightmap +corners — correct on horizontal cells but wrong on cells where +the visible triangle slopes the *other* way (NE↔SW cut vs SE↔NW +cut). If physics hasn't registered a landblock at the moment scenery +hydrates (timing race), the affected scenery uses the bilinear +sampler and lands on a different Z than the visible terrain. Player +Z is fine because player movement always queries the physics sampler. + +**Predicts:** the affected species would render correctly if you +delay scenery hydration until physics registration completes (or +if you make the bilinear fallback also use the split-direction +formula). The float amount should match the gap between the +bilinear and the AC2D split-direction Z at the same (x, y) — usually +small (<0.5 m), variable across instances of the same species. + +**Fix if H2:** make the bilinear fallback in `SampleTerrainZ` +(private in GameWindow.cs) use the same split-direction formula +the physics sampler uses. Or eliminate the fallback by making +scenery hydration await physics registration. Cite ACME's +`TerrainConformanceTests.cs` for the canonical split-direction +test — port it as a unit test on the bilinear fallback. + +### H3 — Close-degrade story applied to scenery + +Some tree GfxObjs have `DIDDegrade` tables (we confirmed this +pattern fixes humanoid bulky bodies in Issue #47). The base GfxObj +id (LOD-3, used by retail only as the entry point to `DIDDegrade`) +and `Degrades[0]` (close-detail) may have different mesh-local +origin conventions — e.g. base origin at bbox-center, slot-0 at +trunk-base. We currently draw the base directly for scenery (the +[`GfxObjDegradeResolver`](../../src/AcDream.Core/Meshing/GfxObjDegradeResolver.cs) +is scoped to humanoid setups via `IsIssue47HumanoidSetup`). + +**Predicts:** the offending GfxObj has `HasDIDDegrade` set, and +its `Degrades[0]` resolves to a different GfxObj whose `meshZRange` +starts at 0 — using slot 0 puts the trunk on the ground. + +**Fix if H3:** extend the `ACDREAM_RETAIL_CLOSE_DEGRADES` flag to +scenery (drop the humanoid-only gate, or add a parallel scenery- +aware path). Wire the resolver into the scenery hydration loop +right before mesh upload. The resolver's safe fallbacks mean it's +benign on scenery without `DIDDegrade` (most species). + +## Required first step — diagnostic dump + +Add a `ACDREAM_DUMP_SCENERY_Z=1` env-var-gated dump in the scenery +hydration loop (around [`GameWindow.cs:4646`](../../src/AcDream.App/Rendering/GameWindow.cs:4646)) +that logs, per spawn: + +``` +[scenery-z] gfx=0xXXXXXXXX setupOrGfx=... source=physics|bilinear + worldPos=(x,y,z) BaseLoc.Z=... groundZ=... + meshZRange=[zMin..zMax] meshZSpan=... + hasDIDDegrade=true|false [degrades[0]=0xYYYY zMin=...] +``` + +Where: + +- `gfx` is the GfxObjId actually being rendered +- `source` is `"physics"` if `_physicsEngine.SampleTerrainZ` returned + non-null, `"bilinear"` if we hit the fallback +- `meshZRange` comes from a one-time scan of the GfxObj's + `VertexArray.Vertices` — `min(v.Origin.Z)` and `max(v.Origin.Z)` +- `hasDIDDegrade` reads `gfxObj.Flags.HasFlag(GfxObjFlags.HasDIDDegrade)` +- if true, follow `gfxObj.DIDDegrade` and log slot 0's GfxObjId + zMin + +User identifies a floating tree (rotates camera, uses dev panel +or just visual identification), reads the log to find that gfx +id, and reports the line. The data instantly disambiguates: + +- H1: `meshZRange.zMin` ≈ float amount, `hasDIDDegrade=false` +- H2: matching gfx id elsewhere has `source=physics`, the offender + has `source=bilinear`, and `meshZRange.zMin` ≈ 0 +- H3: `hasDIDDegrade=true`, slot 0's `zMin` ≠ base's `zMin` + +## Files most likely to need edits + +- [`src/AcDream.App/Rendering/GameWindow.cs:4625-4655`](../../src/AcDream.App/Rendering/GameWindow.cs:4625) — + the scenery hydration loop. Diagnostic dump goes here. Final fix + (depending on hypothesis) probably also lands here. +- [`src/AcDream.Core/World/SceneryGenerator.cs:204`](../../src/AcDream.Core/World/SceneryGenerator.cs:204) — + if H1 needs the bbox computation passed through, extend the + generator output struct. +- [`src/AcDream.Core/Physics/TerrainSurface.cs`](../../src/AcDream.Core/Physics/TerrainSurface.cs) — + if H2, replace the bilinear fallback math here or in the private + `SampleTerrainZ` in GameWindow.cs. +- [`src/AcDream.Core/Meshing/GfxObjDegradeResolver.cs`](../../src/AcDream.Core/Meshing/GfxObjDegradeResolver.cs) — + if H3, extend its applicability to scenery (drop or augment the + humanoid-only gate currently in `GameWindow.IsIssue47HumanoidSetup`). +- [`docs/research/2026-05-06-issue-47-close-degrade-pseudocode.md`](2026-05-06-issue-47-close-degrade-pseudocode.md) — + cite if H3. + +## Workflow (per CLAUDE.md) + +1. **Step 0 — grep named-retail.** For H1, search: + ``` + grep -n "PlaceOnGround\|adjust_to_ground\|LandBlock::ObjectDesc\|BaseLoc\|ScaleObj\|FUN_005a6cc0" docs/research/named-retail/acclient_2013_pseudo_c.txt + ``` + The decomp likely has a function that adjusts placement Z by the + mesh's zMin or similar. If retail does this, the fix is to mirror + it; if retail doesn't, your hypothesis may need refinement. +2. **Cross-reference.** Check ACME's `WorldBuilder.Tests/ClientReference.cs` + and `WorldBuilder/Editors/Landscape/StaticObjectManager.cs` for + how scenery placement Z is computed there. Check ACViewer's + `Physics/Common/ObjectDesc.cs` for placement math. +3. **Pseudocode.** Add `docs/research/2026-05-XX-issue-48-fix-pseudocode.md` + with the algorithm in plain language before porting. +4. **Port faithfully.** Match retail line-by-line. +5. **Conformance test.** Unit test in `tests/AcDream.Core.Tests/` + covering the chosen hypothesis. For H1: assert that for a GfxObj + whose `meshZMin > 0`, the rendered final position has the lowest + vertex sitting at `groundZ + BaseLoc.Z`. For H2: port the AC2D + split-direction conformance check from ACME's + `TerrainConformanceTests.cs`. For H3: cover the scenery code path + in the existing `GfxObjDegradeResolverTests.cs` style. +6. **Visual verification.** User confirms in-client. + +## Test workflow (live verification) + +```powershell +Get-Process -Name AcDream.App -ErrorAction SilentlyContinue | Stop-Process -Force +Start-Sleep -Seconds 4 + +$env:ACDREAM_DUMP_SCENERY_Z = "1" # the diagnostic you'll add +$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_issue48.log" +``` + +User spawns at Holtburg, identifies a floating tree visually, +reports the GfxObjId. You grep the log for that id, look at the +diagnostic line, decide which hypothesis fits, implement, retest. + +## Constraints / don't-break + +- Player Z snap is correct — don't touch the physics sampler's + movement path. The fix is to scenery placement only. +- Most scenery species are correctly placed today. The fix must + not regress them. If H1: cap the `-meshZMin` shift so it only + applies to species where `zMin > epsilon` (don't shift down + species already authored with zMin = 0, in case authoring + variance produces tiny negative zMin from float precision). +- Don't widen the `ACDREAM_RETAIL_CLOSE_DEGRADES` scope to scenery + unless H3 is confirmed; doing so blindly might fix #48 but cause + unintended LOD selection on scenery at distance. +- Tests must stay green: `dotnet build AcDream.slnx -c Debug`, + `dotnet test AcDream.slnx`. There are 8 pre-existing motion test + failures in `AcDream.Core.Tests` that aren't yours — leave them. + +## When to stop and ask + +Per CLAUDE.md, ask only for: + +- Visual verification (user looking at the client) +- Genuine architectural disagreements (e.g. if all three hypotheses + prove wrong and a fourth is needed) +- Hard-to-reverse destructive actions + +Otherwise act. Don't ask "should I continue?". + +## References to consult + +- [`docs/research/2026-05-06-issue-47-close-degrade-pseudocode.md`](2026-05-06-issue-47-close-degrade-pseudocode.md) — + the close-degrade fix that landed for #47; precedent for H3 +- [`src/AcDream.Core/Meshing/GfxObjDegradeResolver.cs`](../../src/AcDream.Core/Meshing/GfxObjDegradeResolver.cs) — + ready-to-extend resolver if H3 fits +- [`src/AcDream.Core/World/SceneryGenerator.cs`](../../src/AcDream.Core/World/SceneryGenerator.cs) — + the placement math, fully decomp-cited +- `docs/research/named-retail/acclient_2013_pseudo_c.txt` — Sept + 2013 PDB pseudo-C, **grep this FIRST** +- ACME `WorldBuilder.Tests/ClientReference.cs` — decompiled retail + client C# port; canonical for terrain/scenery math +- ACME `WorldBuilder/Editors/Landscape/StaticObjectManager.cs` — + scenery rendering pipeline reference + +## Final note + +Don't pre-commit to a hypothesis. Add the diagnostic dump first, +get one log line for one offending tree from the user, then pick +the hypothesis the data points at and implement. The whole point +of the diagnostic-first move is that one sample disambiguates +three weeks of speculation into one straightforward port. Issue #47 +proved this pattern works. + +When the fix lands and the user has visually verified all scenery +sits flush: update [`docs/ISSUES.md`](../ISSUES.md) to mark #48 +DONE with the commit SHA, update memory if there's a durable +lesson, commit on the current branch with a co-authored message, +merge to main, push.