diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 7210cbaa..3b790aea 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -46,108 +46,6 @@ 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 deleted file mode 100644 index f598e21d..00000000 --- a/docs/research/2026-05-06-issue-48-handoff.md +++ /dev/null @@ -1,303 +0,0 @@ -# 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.