diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 87c7b2da..7210cbaa 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -46,112 +46,9 @@ Copy this block when adding a new issue: # Active issues -## #49 — Scenery (X, Y) placement drifts from retail at some landblocks +## #48 — A few specific scenery trees hover above terrain (per-GfxObj Z misplacement) **Status:** OPEN -**Severity:** MEDIUM (visible misplacement; species-specific or per-cell, not a global offset) -**Filed:** 2026-05-06 -**Component:** scenery placement / `SceneryGenerator` - -**Description:** While verifying the `#48` Z fix at Holtburg -landblock `0xA9B30001`, the user spotted a scenery tree placed at -the **wrong (X, Y)** in acdream relative to retail at the same -character coords. Specifically: a large tree that retail places far -across the road on the right (east) side appears in acdream on the -left (west) side, near a chess board / picnic-bench area. Side-by- -side screenshot pair captured 2026-05-06. - -This is **not** a Z bug — every tree in the same screenshot has its -trunk meeting the visible terrain (the `#48` `SampleTerrainZ` fix is -working). It's also **not** the LandBlockInfo Stab path — the chess -board / bench themselves are correctly placed, so the landblock -origin and `lbOffset` math are right. - -**Hypotheses (need cdb retail trace to disambiguate):** - -1. The displacement-noise math in `SceneryGenerator` differs from - retail's `chunk_005A0000` LCG by a constant or a sign flip. Audit - `eeee4c5` claimed "all MATCH" against the decomp, but a runtime - trace would prove or disprove. -2. Coordinate-system handedness: cell-local `(lx, ly)` in our path - may map to retail's `(ly, lx)` somewhere, rotating tree XY 90° - around the cell's NW corner. -3. The `obj.Align != 0` path in retail (`FUN_005a6f60`, aligns the - object to the landcell polygon's normal) may use a different - reference point than ours, drifting placement on sloped cells. -4. Slope filter could reject a cell retail accepts (or vice versa), - pushing trees into adjacent cells. -5. Region-table / `SceneInfo` lookup might select a different - scenery list for the cell type. - -**Investigation plan (gold-standard, per `project_retail_debugger.md`):** - -1. Run the existing `ACDREAM_DUMP_SCENERY_Z=1` diagnostic to capture - acdream's full per-spawn (gfx, world XY, scale, partT) for - landblock `0xA9B3FFFF`. -2. Attach cdb to a live retail client at the same Holtburg spot - (`tools/pdb-extract/check_exe_pdb.py` confirms PDB pairs with - v11.4186). Set a breakpoint on `CLandBlock::get_land_scenes` (or - the inner `chunk_005A0000` placement function); capture every - `(gfxObjId, worldX, worldY, scale, heading)` retail emits for - the same landblock. -3. Diff the two tables. The spawn that's offset will be obvious; - the offset pattern (one tree, all trees, one species, constant - delta, etc.) determines which hypothesis above is correct. - -**Files:** - -- [`src/AcDream.Core/World/SceneryGenerator.cs`](src/AcDream.Core/World/SceneryGenerator.cs) — placement math (LCG noise, displacement, rotation, scale, slope filter) -- `acclient!CLandBlock::get_land_scenes` (`docs/research/named-retail/acclient_2013_pseudo_c.txt`) — retail entry point -- `chunk_005A0000.c` — referenced retail source per `SceneryGenerator.cs` comments -- [`docs/research/named-retail/symbols.json`](docs/research/named-retail/symbols.json) — for cdb breakpoints - -**Acceptance:** Side-by-side outdoor screenshot pair (acdream vs -retail, same character coords, same time of day) shows scenery -positions matching at multiple landblocks. The cdb trace + diagnostic -diff documents quantitative agreement (zero offset within float -precision) on at least one landblock end-to-end. - -**Out of scope here (kept under `#48`):** Z floating. That's fixed. - ---- - -## #48 — [DONE 2026-05-06 · a469395] A few specific scenery trees hover above terrain (per-GfxObj Z misplacement) - -**Resolution:** Hypothesis 2 (physics-sampler vs bilinear-fallback Z -mismatch). The bilinear fallback in `GameWindow.SampleTerrainZ` had -its two diagonal arms swapped — used the SEtoNW triangle test on -SWtoNE cells and vice versa. Every scenery hydration in our -diagnostic ran through the bilinear path (`source=bilinear` in all -`[scenery-z]` log lines) because physics hadn't yet built a -`TerrainSurface` for the streaming-in landblock — so on sloped -cells, scenery sat at a different Z than the visible terrain mesh -by up to ~1.5 m. The bug was latent since `ff325ab` (2026-04-17) -which upgraded the fallback from naive 4-corner bilinear to -triangle-aware barycentric, but with the diagonal-pair tests -swapped. `TerrainSurface.SampleZ` (used by the physics path / player -Z) was always correct, so player feet stayed flush — the two paths -just disagreed and only scenery noticed. - -Fix: extracted the canonical triangle-pick math into -`TerrainSurface.InterpolateZInTriangle` (private static); added -`TerrainSurface.SampleZFromHeightmap` (public static) that reads -heights directly from the landblock byte array using the same -canonical math; redirected `GameWindow.SampleTerrainZ` to delegate -to it. New conformance test -`SampleZFromHeightmap_AgreesWithInstance_AcrossWholeLandblock` pins -both sampler paths together at 1500 sample points across both -diagonals, so future drift gets caught. User visually confirmed -2026-05-06. - -The diagnostic dump (`ACDREAM_DUMP_SCENERY_Z=1`, -`GameWindow.cs:4661`) is kept committed — it's gated by env var, -zero cost when off, and is the right starting point for `#49` -(scenery X/Y placement) too. - -Pseudocode: [`docs/research/2026-05-06-issue-48-fix-pseudocode.md`](docs/research/2026-05-06-issue-48-fix-pseudocode.md). - -**Status:** DONE **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 @@ -746,13 +643,11 @@ primary motion + collision resolution. - Verification: side-by-side vs legacy default in 2-client setup, identical visible behavior. -## #38 — [DONE 2026-05-06 · (this commit)] Chase camera + player feel "30 fps" since L.5 physics-tick gate +## #38 — Chase camera + player feel "30 fps" since L.5 physics-tick gate -**Status:** DONE +**Status:** OPEN **Severity:** MEDIUM (gameplay-feel regression; not a correctness bug) **Filed:** 2026-05-01 -**Closed:** 2026-05-06 -**Commit:** `(this commit)` **Component:** rendering / physics / camera **Description:** User reports that running around in third-person / @@ -809,8 +704,7 @@ collision fixes.) **Acceptance:** - Chase-camera run-around at 60+ FPS feels as smooth as render rate - suggests (no perceptual stepping) — user visually confirmed - 2026-05-06. + suggests (no perceptual stepping) - Network outbound (MoveToState / AutonomousPosition cadence + values) unchanged from current behavior - Collision behavior unchanged (the L.5 wedge / steep-roof scenarios @@ -1466,11 +1360,10 @@ If hypothesis (a) is correct, this issue effectively rolls into **#28** — the --- -## #47 — [DONE 2026-05-06 · 0bd9b96] Humanoid Setup 0x02000001 renders bulky / lacks shape detail vs retail +## #47 — [DONE 2026-05-06] Humanoid Setup 0x02000001 renders bulky / lacks shape detail vs retail -**Status:** DONE +**Status:** DONE (commit pending) **Closed:** 2026-05-06 -**Commit:** `0bd9b96` **Severity:** MEDIUM (cosmetic — characters readable but visibly different from retail) **Filed:** 2026-05-06 **Component:** rendering / mesh / character animation @@ -1489,10 +1382,10 @@ Concrete swaps the resolver now performs: - Heritage variants: `0x010004BF → 0x010017A8`, `0x010004BD → 0x010017A7`, `0x010004B7 → 0x0100179A`, etc. -Fix landed as `GfxObjDegradeResolver`, default-on and scoped to humanoid -setups (34-part with ≥8 null-sentinel attachment slots). Set -`ACDREAM_RETAIL_CLOSE_DEGRADES=0` only for diagnostic before/after -comparisons. User confirmed visually 2026-05-06. +Fix landed as `GfxObjDegradeResolver`, gated behind +`ACDREAM_RETAIL_CLOSE_DEGRADES=1` and scoped to humanoid setups +(34-part with ≥8 null-sentinel attachment slots). User confirmed +visually 2026-05-06. Files: `src/AcDream.Core/Meshing/GfxObjDegradeResolver.cs`, `src/AcDream.App/Rendering/GameWindow.cs` (wiring), 5 unit tests in diff --git a/docs/research/2026-05-06-issue-38-render-interp-pseudocode.md b/docs/research/2026-05-06-issue-38-render-interp-pseudocode.md deleted file mode 100644 index 11c9b97f..00000000 --- a/docs/research/2026-05-06-issue-38-render-interp-pseudocode.md +++ /dev/null @@ -1,98 +0,0 @@ -# Issue #38 render interpolation pseudocode - -## Problem - -Phase L.5 correctly restored retail's `CPhysicsObj::update_object` -physics gate: the body only integrates when accumulated frame time reaches -`PhysicsBody.MinQuantum` (1/30 second). That keeps collision behavior aligned -with retail, but the renderer currently reads `_body.Position` directly. - -At 60+ FPS that means several rendered frames can show the same physics -position, then jump to the next 30 Hz position. Chase camera makes the stepping -obvious because both the player mesh and camera target follow that discrete -physics sample. - -## Evidence - -- Named retail: `CPhysicsObj::update_object` at `0x00515d10` - (`docs/research/named-retail/acclient_2013_pseudo_c.txt:283950`) skips or - consumes time outside the valid quantum window and calls - `CPhysicsObj::UpdateObjectInternal` only for accepted physics quanta. -- ACE mirrors the same constants in `PhysicsGlobals`: `MinQuantum = 1/30`, - `MaxQuantum = 0.1`, `HugeQuantum = 2.0`. -- ACE's `InterpolationManager` / `PositionManager` prove client-style - smoothing is a known AC-family concept, but their queue chases network target - positions. Issue #38 is narrower: local render-only interpolation between the - last two authoritative local physics ticks. -- Glenn Fiedler's "Fix Your Timestep!" describes the canonical fixed-timestep - accumulator plus render interpolation pattern: - https://gafferongames.com/post/fix_your_timestep/ - -## Intentional divergence - -Retail did not need a separate high-FPS render interpolation layer because the -2013 client effectively presented near the same cadence as its physics gate. -acdream renders at modern display rates, so we preserve retail's 30 Hz physics -truth but draw a blended visual position between the last two physics truths. - -## Pseudocode - -State stored by `PlayerMovementController`: - -```text -physicsAccum: leftover render time not yet consumed by physics -prevPhysicsPos: body position at the start of the most recently completed tick -currPhysicsPos: body position at the end of the most recently completed tick -``` - -On spawn, teleport, or any authoritative SetPosition: - -```text -body.Position = newPosition -prevPhysicsPos = newPosition -currPhysicsPos = newPosition -physicsAccum = 0 -``` - -Each Update(dt): - -```text -physicsAccum += dt - -if physicsAccum > HugeQuantum: - physicsAccum = 0 - prevPhysicsPos = body.Position - currPhysicsPos = body.Position - return/render body.Position - -if physicsAccum >= MinQuantum: - oldTickEnd = currPhysicsPos - preIntegratePos = body.Position - - tickDt = min(physicsAccum, MaxQuantum) - calc_acceleration() - UpdatePhysicsInternal(tickDt) - - postIntegratePos = body.Position - resolve collision from preIntegratePos to postIntegratePos - body.Position = resolved physics position - update contact/cell/velocity exactly as before - - prevPhysicsPos = oldTickEnd - currPhysicsPos = body.Position - physicsAccum -= tickDt - -alpha = clamp(physicsAccum / MinQuantum, 0, 1) -renderPosition = lerp(prevPhysicsPos, currPhysicsPos, alpha) -``` - -Important constraints: - -- Never write `renderPosition` back to `_body.Position`. -- `MovementResult.Position` remains the authoritative physics/collision/network - position. -- Add `MovementResult.RenderPosition` for mesh and camera drawing only. -- Outbound `MoveToState` / `AutonomousPosition` keep using - `MovementResult.Position`. -- If no physics tick has completed yet, `prevPhysicsPos == currPhysicsPos`, so - interpolation is a no-op. diff --git a/docs/research/2026-05-06-issue-48-fix-pseudocode.md b/docs/research/2026-05-06-issue-48-fix-pseudocode.md deleted file mode 100644 index c613711d..00000000 --- a/docs/research/2026-05-06-issue-48-fix-pseudocode.md +++ /dev/null @@ -1,86 +0,0 @@ -# Issue #48 fix — bilinear fallback triangle-pair test was swapped - -## Diagnosis (from the diagnostic dump) - -ACDREAM_DUMP_SCENERY_Z=1 in landblock 0xA9B3 around the user's spawn -showed: - -- Every scenery line had `source=bilinear` (physics engine had not - registered the landblock at hydration time — typical streaming race). -- The trees flagged as "floating" had multi-part setups whose lowest - vertex extended into the ground per our calc (`partWorldZMin = - groundZ - 3.825`) — i.e., per the diagnostic the tree should sit - flush. The retail client at the same world coords does sit flush. -- Player Z at the same world coords (~91 m) came from the physics - sampler (`TerrainSurface.SampleZ`), which is correct. - -That meant the bilinear fallback in `GameWindow.SampleTerrainZ` was -producing a different ground Z than the visible terrain mesh and the -player physics path on sloped cells, so trees were placed at a -ground-Z that didn't match the terrain the player was walking on. - -## Root cause - -Two terrain Z samplers exist: - -1. `AcDream.Core.Physics.TerrainSurface.SampleZ` (instance) — used by the - physics engine for player + entity ground-snap. Triangle-aware, - matches the visible terrain mesh. -2. `AcDream.App.Rendering.GameWindow.SampleTerrainZ` (private bilinear - fallback) — used by scenery hydration when physics has not yet built - a `TerrainSurface` for a streaming-in landblock. - -Both choose a per-cell **diagonal** with the AC2D `FSplitNESW` formula -(constants `0x0CCAC033`, `0x421BE3BD`, `0x6C1AC587`, `0x519B8F25`). -Both then choose one of the cell's two **triangles** based on the -fractional position within the cell. **The fallback's two arms were -swapped** relative to the chosen diagonal: - -| Diagonal | Correct dividing test | Correct triangles | Bilinear-fallback test (BUGGY) | -|---|---|---|---| -| `SWtoNE` (BL→TR, line y=x) | `tx > ty` | {BL,BR,TR} below / {BL,TR,TL} above | `s + t <= 1` (wrong — that's the SEtoNW test) | -| `SEtoNW` (BR→TL, line x+y=1) | `tx + ty <= 1` | {BL,BR,TL} below / {BR,TR,TL} above | `s >= t` (wrong — that's the SWtoNE test) | - -On sloped cells the wrong triangle's plane gives a Z that disagrees -with the rendered terrain by up to ~1.5 m. Flat cells happen to mask -the bug because all four corners share one Z. - -## Fix - -1. Extract the correct triangle-picker math from `TerrainSurface.SampleZ` - (instance) into a new public static method - `TerrainSurface.SampleZFromHeightmap(byte[] heights, float[] heightTable, - uint landblockX, uint landblockY, float localX, float localY)`. - Same algorithm, but reads the four corner heights directly from the - landblock's raw heightmap byte array instead of the pre-resolved - instance cache. One source of truth for the triangle math. -2. Replace `GameWindow.SampleTerrainZ` body with a call to that static. -3. Conformance test in `tests/AcDream.Core.Tests/Physics/TerrainSurfaceTests.cs` - exercising a sloped heightmap on both diagonals, asserting that the - new static and the existing instance method return the same Z at the - same `(localX, localY)` (especially at points near the cell diagonal - where the previous bug manifested). - -## Why this is the right fix - -- Retail (`docs/research/named-retail/acclient_2013_pseudo_c.txt`) places - scenery via `CLandBlock::get_land_scenes` (0x00530460) → - `Plane::set_height(plane, &pos)` (0x0052f050), which projects the - position onto the terrain triangle's plane. The split direction comes - from the same `FSplitNESW` formula. Our `TerrainSurface.SampleZ` is a - faithful port of that algorithm and is already used by physics; the - bilinear fallback should be identical. -- The bug is purely in the fallback path. Player Z is unaffected. -- No retail behavior changes; only acdream consistency. -- "Don't break" constraints from the handoff are satisfied: - - Player Z snap untouched (different code path). - - Species that already render flush still do (their Z was correct - on cells where bilinear and physics agreed; now it's correct - everywhere). - -## Pre-existing bugs out of scope - -The user reports separate (X, Y) misplacement at other locations. -That's a different bug — likely in `SceneryGenerator`'s placement -math or one of the terrain-mesh / region tables — and outside the -scope of this fix. File as a follow-up issue. diff --git a/src/AcDream.App/Input/PlayerMovementController.cs b/src/AcDream.App/Input/PlayerMovementController.cs index 1bc88b8b..a57ccd97 100644 --- a/src/AcDream.App/Input/PlayerMovementController.cs +++ b/src/AcDream.App/Input/PlayerMovementController.cs @@ -41,7 +41,6 @@ public readonly record struct MovementInput( /// public readonly record struct MovementResult( Vector3 Position, - Vector3 RenderPosition, uint CellId, bool IsOnGround, bool MotionStateChanged, @@ -129,7 +128,6 @@ public sealed class PlayerMovementController public float Yaw { get; set; } public Vector3 Position => _body.Position; - public Vector3 RenderPosition => ComputeRenderPosition(); public uint CellId { get; private set; } /// @@ -215,8 +213,6 @@ public sealed class PlayerMovementController // ACE: PhysicsObj.UpdateObject (Physics.cs). // Named-retail: CPhysicsObj::update_object (acclient_2013_pseudo_c.txt:283950). private float _physicsAccum; - private Vector3 _prevPhysicsPos; - private Vector3 _currPhysicsPos; public PlayerMovementController(PhysicsEngine physics) { @@ -291,8 +287,6 @@ public sealed class PlayerMovementController public void SetPosition(Vector3 pos, uint cellId) { _body.Position = pos; - _prevPhysicsPos = pos; - _currPhysicsPos = pos; CellId = cellId; // Treat as grounded after a server-side position snap. @@ -301,13 +295,6 @@ public sealed class PlayerMovementController // Reset physics clock so any subsequent update_object calls start fresh. _body.LastUpdateTime = 0.0; - _physicsAccum = 0f; - } - - private Vector3 ComputeRenderPosition() - { - float alpha = Math.Clamp(_physicsAccum / PhysicsBody.MinQuantum, 0f, 1f); - return Vector3.Lerp(_prevPhysicsPos, _currPhysicsPos, alpha); } public MovementResult Update(float dt, MovementInput input) @@ -319,7 +306,6 @@ public sealed class PlayerMovementController { return new MovementResult( Position: Position, - RenderPosition: RenderPosition, CellId: CellId, IsOnGround: _body.OnWalkable, MotionStateChanged: false, @@ -538,16 +524,12 @@ public sealed class PlayerMovementController // accumulated dt) when the threshold is reached. See _physicsAccum // declaration for the full retail trace evidence. var preIntegratePos = _body.Position; - bool physicsTickRan = false; - Vector3 oldTickEndPos = _currPhysicsPos; _physicsAccum += dt; if (_physicsAccum > PhysicsBody.HugeQuantum) { // Stale frame (debugger break, GC pause). Discard accumulated dt. _physicsAccum = 0f; - _prevPhysicsPos = _body.Position; - _currPhysicsPos = _body.Position; } else if (_physicsAccum >= PhysicsBody.MinQuantum) { @@ -557,7 +539,6 @@ public sealed class PlayerMovementController _body.calc_acceleration(); _body.UpdatePhysicsInternal(tickDt); _physicsAccum -= tickDt; - physicsTickRan = true; } // Else: dt below MinQuantum threshold — skip integration. Position // and velocity remain unchanged; Resolve below runs as a zero-distance @@ -610,11 +591,6 @@ public sealed class PlayerMovementController // Apply resolved position. _body.Position = resolveResult.Position; - if (physicsTickRan) - { - _prevPhysicsPos = oldTickEndPos; - _currPhysicsPos = _body.Position; - } // L.3a (2026-04-30): retail wall-bounce / velocity reflection. // @@ -898,7 +874,6 @@ public sealed class PlayerMovementController return new MovementResult( Position: Position, - RenderPosition: RenderPosition, CellId: CellId, IsOnGround: _body.OnWalkable, MotionStateChanged: changed, diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index b00345d8..21e72ead 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -173,24 +173,15 @@ public sealed class GameWindow : IDisposable private static readonly int s_hidePartIndex = int.TryParse(Environment.GetEnvironmentVariable("ACDREAM_HIDE_PART"), out var hp) ? hp : -1; - // Issue #47 — use retail's close-detail GfxObj selection on + // Issue #47 — opt in to retail's close-detail GfxObj selection on // humanoid setups. When enabled, every per-part GfxObj id (after // server AnimPartChanges are applied) is replaced with Degrades[0] // from its DIDDegrade table when present. See GfxObjDegradeResolver - // for the full retail-decomp citation. Default-on after visual - // confirmation; set ACDREAM_RETAIL_CLOSE_DEGRADES=0 only for - // diagnostic before/after comparisons. + // for the full retail-decomp citation. Off by default while the fix + // bakes; flip to default-on once we've confirmed no scenery/setup + // regressions. private static readonly bool s_retailCloseDegrades = - !string.Equals(Environment.GetEnvironmentVariable("ACDREAM_RETAIL_CLOSE_DEGRADES"), "0", StringComparison.Ordinal); - - // Issue #48 diagnostic — dump per-scenery-spawn placement evidence - // (rendered gfx id, sample source physics-vs-bilinear, ground/baseLoc/finalZ, - // mesh vertex Z range, DIDDegrade slot 0). One log line per spawn lets - // the user identify a floating tree by its world coordinates and tell - // whether the cause is BaseLoc.Z addition (H1), bilinear-fallback drift - // (H2), or DIDDegrade selection (H3). Diagnostic-first per CLAUDE.md. - private static readonly bool s_dumpSceneryZ = - string.Equals(Environment.GetEnvironmentVariable("ACDREAM_DUMP_SCENERY_Z"), "1", StringComparison.Ordinal); + string.Equals(Environment.GetEnvironmentVariable("ACDREAM_RETAIL_CLOSE_DEGRADES"), "1", StringComparison.Ordinal); /// /// Issue #47 humanoid-setup detector. Matches Aluvian Male @@ -2532,28 +2523,62 @@ public sealed class GameWindow : IDisposable } /// - /// Triangle-aware terrain Z sample directly from a landblock's raw - /// heightmap. Used as the bilinear fallback in scenery hydration when - /// physics hasn't built a TerrainSurface for the landblock yet - /// (streaming race). Delegates to - /// - /// so this fallback and the player-physics path stay in lock-step on - /// sloped cells. - /// - /// - /// Issue #48: the previous in-place implementation here had its two - /// diagonal arms swapped (SWtoNE cells used the SEtoNW triangle test - /// and vice versa), so scenery on hilly terrain sat at a different Z - /// than the visible terrain mesh — a multi-meter offset in some - /// cells, the user-reported "floating trees" symptom. - /// + /// Bilinear sample of the landblock heightmap at (x, y) in landblock-local + /// world units. Matches the x-major indexing convention of LandblockMesh. /// - private static float SampleTerrainZ(DatReaderWriter.DBObjs.LandBlock block, float[] heightTable, float localX, float localY) + private float SampleTerrainZ(DatReaderWriter.DBObjs.LandBlock block, float[] heightTable, float worldX, float worldY) { + // Exact port of WorldBuilder TerrainUtils.GetHeight (line 59-108). + // Barycentric interpolation over the cell's triangle pair, respecting + // the cell's split direction (SWtoNE vs SEtoNW). + const float CellSize = 24f; + + uint cellX = (uint)(worldX / CellSize); + uint cellY = (uint)(worldY / CellSize); + if (cellX >= 8) cellX = 7; + if (cellY >= 8) cellY = 7; + uint landblockX = (block.Id >> 24) & 0xFFu; uint landblockY = (block.Id >> 16) & 0xFFu; - return AcDream.Core.Physics.TerrainSurface.SampleZFromHeightmap( - block.Height, heightTable, landblockX, landblockY, localX, localY); + var splitDirection = AcDream.Core.Terrain.TerrainBlending.CalculateSplitDirection( + landblockX, cellX, landblockY, cellY); + + // 4 cell corners (heightmap x-major: Height[x*9 + y]) + float h0 = heightTable[block.Height[cellX * 9 + cellY]]; // BL + float h1 = heightTable[block.Height[(cellX + 1) * 9 + cellY]]; // BR + float h2 = heightTable[block.Height[(cellX + 1) * 9 + (cellY + 1)]]; // TR + float h3 = heightTable[block.Height[cellX * 9 + (cellY + 1)]]; // TL + + float lx = worldX - cellX * CellSize; + float ly = worldY - cellY * CellSize; + float s = lx / CellSize; + float t = ly / CellSize; + + if (splitDirection == AcDream.Core.Terrain.CellSplitDirection.SWtoNE) + { + if (s + t <= 1f) + { + return h0 * (1f - s - t) + h1 * s + h3 * t; + } + else + { + float u = s + t - 1f; + float v = 1f - s; + float w = 1f - u - v; + return h1 * w + h2 * u + h3 * v; + } + } + else // SEtoNW + { + if (s >= t) + { + return h0 * (1f - s) + h1 * (s - t) + h2 * t; + } + else + { + return h0 * (1f - t) + h2 * s + h3 * (t - s); + } + } } private void OnLiveEntityDeleted(AcDream.Core.Net.Messages.DeleteObject.Parsed delete) @@ -4614,76 +4639,10 @@ public sealed class GameWindow : IDisposable // fall back to the local bilinear sample. var worldPx = localX + lbOffset.X; var worldPy = localY + lbOffset.Y; - float? maybePhysicsZ = _physicsEngine.SampleTerrainZ(worldPx, worldPy); - float groundZ = maybePhysicsZ + float groundZ = _physicsEngine.SampleTerrainZ(worldPx, worldPy) ?? SampleTerrainZ(lb.Heightmap, _heightTable, localX, localY); float finalZ = groundZ + spawn.LocalPosition.Z; - // Issue #48 diagnostic. One log line per (spawn, rendered-mesh) - // disambiguates H1 (BaseLoc.Z / mesh-zMin per-species), H2 - // (physics-vs-bilinear sampler drift), and H3 (DIDDegrade slot 0). - // User identifies a floating tree visually, finds the matching - // line by world coords + gfx id, the data picks the hypothesis. - if (s_dumpSceneryZ) - { - string source = maybePhysicsZ.HasValue ? "physics" : "bilinear"; - foreach (var mr in meshRefs) - { - var dgfx = _dats.Get(mr.GfxObjId); - if (dgfx is null) continue; - - float zMin = float.PositiveInfinity, zMax = float.NegativeInfinity; - foreach (var v in dgfx.VertexArray.Vertices.Values) - { - if (v.Origin.Z < zMin) zMin = v.Origin.Z; - if (v.Origin.Z > zMax) zMax = v.Origin.Z; - } - if (float.IsPositiveInfinity(zMin)) { zMin = 0f; zMax = 0f; } - - // Per-part transform offset inside the setup (post-spawn-scale). - // For setup spawns this is Setup.PlacementFrames[Default].Frames[i] * - // spawn.Scale. For single-GfxObj spawns it's identity * spawn.Scale. - var partT = mr.PartTransform.Translation; - - bool hasDD = dgfx.Flags.HasFlag(DatReaderWriter.Enums.GfxObjFlags.HasDIDDegrade); - string ddInfo = string.Empty; - if (hasDD && dgfx.DIDDegrade != 0) - { - var ddi = _dats.Get(dgfx.DIDDegrade); - if (ddi is not null && ddi.Degrades.Count > 0) - { - uint slot0Id = (uint)ddi.Degrades[0].Id; - float slot0Min = 0f; - var slot0Gfx = _dats.Get(slot0Id); - if (slot0Gfx is not null && slot0Gfx.VertexArray.Vertices.Count > 0) - { - slot0Min = float.PositiveInfinity; - foreach (var v in slot0Gfx.VertexArray.Vertices.Values) - if (v.Origin.Z < slot0Min) slot0Min = v.Origin.Z; - if (float.IsPositiveInfinity(slot0Min)) slot0Min = 0f; - } - ddInfo = $" deg[0]=0x{slot0Id:X8} deg[0]ZMin={slot0Min:F3}"; - } - } - - // partWorldZMin = the lowest vertex of this part in world space. - // = finalZ (setup origin in world Z) + partT.Z (part offset) + zMin (mesh-local lowest vertex) - // If everything is right and the lowest part of the tree should - // touch the ground, we expect partWorldZMin <= groundZ for at - // least one part of a multi-part setup. - float partWorldZMin = finalZ + partT.Z + zMin; - - Console.WriteLine( - $"[scenery-z] lb=0x{lb.LandblockId:X8} root=0x{spawn.ObjectId:X8} gfx=0x{mr.GfxObjId:X8}" + - $" source={source}" + - $" world=({worldPx:F2},{worldPy:F2}) localXY=({localX:F2},{localY:F2})" + - $" groundZ={groundZ:F3} BaseLoc.Z={spawn.LocalPosition.Z:F3} finalZ={finalZ:F3}" + - $" partT=({partT.X:F2},{partT.Y:F2},{partT.Z:F3}) spawnScale={spawn.Scale:F3}" + - $" zRange=[{zMin:F3}..{zMax:F3}] partWorldZMin={partWorldZMin:F3} delta={partWorldZMin - groundZ:F3}" + - $" hasDIDDegrade={hasDD}{ddInfo}"); - } - } - var hydrated = new AcDream.Core.World.WorldEntity { Id = sceneryIdBase + localIndex++, @@ -5769,7 +5728,7 @@ public sealed class GameWindow : IDisposable // the physics-resolved location each frame. if (_entitiesByServerGuid.TryGetValue(_playerServerGuid, out var pe)) { - pe.Position = result.RenderPosition; + pe.Position = result.Position; pe.Rotation = System.Numerics.Quaternion.CreateFromAxisAngle( System.Numerics.Vector3.UnitZ, _playerController.Yaw - MathF.PI / 2f); @@ -5791,7 +5750,7 @@ public sealed class GameWindow : IDisposable // position never changes. With the pin: player visibly // rises above the camera, matching retail "you can see // yourself jump" feedback. - _chaseCamera.Update(result.RenderPosition, _playerController.Yaw, + _chaseCamera.Update(result.Position, _playerController.Yaw, isOnGround: result.IsOnGround, dt: (float)dt); diff --git a/src/AcDream.Core/Physics/TerrainSurface.cs b/src/AcDream.Core/Physics/TerrainSurface.cs index fe511881..caa54935 100644 --- a/src/AcDream.Core/Physics/TerrainSurface.cs +++ b/src/AcDream.Core/Physics/TerrainSurface.cs @@ -143,89 +143,23 @@ public sealed class TerrainSurface // and ACE's LandblockStruct.ConstructPolygons. bool splitSWtoNE = IsSplitSWtoNE(_landblockX, (uint)cx, _landblockY, (uint)cy); - return InterpolateZInTriangle(hBL, hBR, hTR, hTL, tx, ty, splitSWtoNE); - } - - /// - /// Sample terrain Z directly from a landblock's raw heightmap. Same - /// algorithm as (instance), but reads the four - /// corner heights through heightTable[heights[x*9+y]] on the fly - /// instead of from the pre-resolved instance cache. Use this when a - /// hasn't been built yet for a landblock — - /// e.g. scenery hydration during streaming, before physics has registered - /// the landblock. Both paths produce the same Z, so scenery sits flush - /// with the visible terrain mesh and with the player physics path. - /// - /// - /// Issue #48 root cause: the previous bilinear fallback in - /// GameWindow.SampleTerrainZ had its two diagonal arms swapped - /// (used the SEtoNW triangle test for SWtoNE cells and vice versa), - /// so on sloped cells scenery sat at a different Z than the visible - /// terrain by up to ~1.5 m. Routing the fallback through this static - /// helper guarantees both samplers stay in lock-step. - /// - /// - public static float SampleZFromHeightmap( - byte[] heights, float[] heightTable, - uint landblockX, uint landblockY, - float localX, float localY) - { - ArgumentNullException.ThrowIfNull(heights); - ArgumentNullException.ThrowIfNull(heightTable); - if (heights.Length < 81) - throw new ArgumentException("heights must have 81 entries", nameof(heights)); - if (heightTable.Length < 256) - throw new ArgumentException("heightTable must have 256 entries", nameof(heightTable)); - - float fx = Math.Clamp(localX / CellSize, 0f, CellsPerSide - 0.001f); - float fy = Math.Clamp(localY / CellSize, 0f, CellsPerSide - 0.001f); - int cx = (int)fx; - int cy = (int)fy; - cx = Math.Clamp(cx, 0, CellsPerSide - 1); - cy = Math.Clamp(cy, 0, CellsPerSide - 1); - - float tx = fx - cx; - float ty = fy - cy; - - // x-major heightmap indexing matches TerrainSurface's pre-resolution - // (heights[x * 9 + y]) and ACE LandblockStruct. - float hBL = heightTable[heights[cx * HeightmapSide + cy ]]; - float hBR = heightTable[heights[(cx+1) * HeightmapSide + cy ]]; - float hTR = heightTable[heights[(cx+1) * HeightmapSide + (cy+1)]]; - float hTL = heightTable[heights[cx * HeightmapSide + (cy+1)]]; - - bool splitSWtoNE = IsSplitSWtoNE(landblockX, (uint)cx, landblockY, (uint)cy); - return InterpolateZInTriangle(hBL, hBR, hTR, hTL, tx, ty, splitSWtoNE); - } - - /// - /// Pick the cell's triangle for the chosen diagonal and barycentric- - /// interpolate Z. Single source of truth shared by both - /// (instance, pre-resolved cache) and - /// (static, raw heightmap). - /// Triangle layout matches ACE LandblockStruct.ConstructPolygons: - /// SWtoNE cells split BL→TR (line y=x), SEtoNW cells split BR→TL - /// (line x+y=1). - /// - private static float InterpolateZInTriangle( - float hBL, float hBR, float hTR, float hTL, - float tx, float ty, bool splitSWtoNE) - { if (splitSWtoNE) { // Diagonal BL(0,0) → TR(1,1) — line y = x. // Triangles: {BL,BR,TR} below (tx > ty), {BL,TR,TL} above. if (tx > ty) - return hBL + (hBR - hBL) * tx + (hTR - hBR) * ty; // BL+BR+TR - return hBL + (hTR - hTL) * tx + (hTL - hBL) * ty; // BL+TR+TL + return hBL + (hBR - hBL) * tx + (hTR - hBR) * ty; // BL+BR+TR triangle + else + return hBL + (hTR - hTL) * tx + (hTL - hBL) * ty; // BL+TR+TL triangle } else { // Diagonal BR(1,0) → TL(0,1) — line x + y = 1. // Triangles: {BL,BR,TL} below (tx+ty <= 1), {BR,TR,TL} above. if (tx + ty <= 1f) - return hBL + (hBR - hBL) * tx + (hTL - hBL) * ty; // BL+BR+TL - return hTR + (hTL - hTR) * (1f - tx) + (hBR - hTR) * (1f - ty); // BR+TR+TL + return hBL + (hBR - hBL) * tx + (hTL - hBL) * ty; // BL+BR+TL triangle + else + return hTR + (hTL - hTR) * (1f - tx) + (hBR - hTR) * (1f - ty); // BR+TR+TL triangle } } diff --git a/tests/AcDream.Core.Tests/Input/PlayerMovementControllerTests.cs b/tests/AcDream.Core.Tests/Input/PlayerMovementControllerTests.cs index 8a09b79f..fe1e859a 100644 --- a/tests/AcDream.Core.Tests/Input/PlayerMovementControllerTests.cs +++ b/tests/AcDream.Core.Tests/Input/PlayerMovementControllerTests.cs @@ -49,81 +49,6 @@ public class PlayerMovementControllerTests Assert.True(result.Position.X > 96f + 2f, $"X={result.Position.X} should have moved forward"); } - [Fact] - public void Update_SubQuantumFrame_InterpolatesRenderPositionWithoutAdvancingPhysicsPosition() - { - var engine = MakeFlatEngine(); - var controller = new PlayerMovementController(engine); - var start = new Vector3(96f, 96f, 50f); - controller.SetPosition(start, 0x0001); - controller.Yaw = 0f; - - var firstTick = controller.Update(PhysicsBody.MinQuantum, new MovementInput(Forward: true)); - Assert.True(firstTick.Position.X > start.X, "Physics tick should advance the authoritative body position"); - Assert.Equal(start.X, firstTick.RenderPosition.X, precision: 4); - - var halfFrame = controller.Update(PhysicsBody.MinQuantum * 0.5f, new MovementInput(Forward: true)); - - Assert.Equal(firstTick.Position.X, halfFrame.Position.X, precision: 4); - Assert.True(halfFrame.RenderPosition.X > start.X, "Render position should move between physics ticks"); - Assert.True(halfFrame.RenderPosition.X < firstTick.Position.X, - $"Render X={halfFrame.RenderPosition.X} should stay between {start.X} and {firstTick.Position.X}"); - - float expectedMidpoint = start.X + ((firstTick.Position.X - start.X) * 0.5f); - Assert.Equal(expectedMidpoint, halfFrame.RenderPosition.X, precision: 3); - } - - [Fact] - public void SetPosition_ResnapsRenderInterpolationEndpoints() - { - var engine = MakeFlatEngine(); - var controller = new PlayerMovementController(engine); - controller.SetPosition(new Vector3(96f, 96f, 50f), 0x0001); - controller.Yaw = 0f; - - controller.Update(PhysicsBody.MinQuantum, new MovementInput(Forward: true)); - controller.Update(PhysicsBody.MinQuantum * 0.5f, new MovementInput(Forward: true)); - - var snapped = new Vector3(120f, 80f, 50f); - controller.SetPosition(snapped, 0x0001); - var result = controller.Update(PhysicsBody.MinQuantum * 0.5f, new MovementInput()); - - Assert.Equal(snapped, result.Position); - Assert.Equal(snapped, result.RenderPosition); - } - - [Fact] - public void Update_HugeQuantumDiscard_ResnapsRenderInterpolationEndpoints() - { - var engine = MakeFlatEngine(); - var controller = new PlayerMovementController(engine); - controller.SetPosition(new Vector3(96f, 96f, 50f), 0x0001); - controller.Yaw = 0f; - - var moved = controller.Update(PhysicsBody.MinQuantum, new MovementInput(Forward: true)); - var stale = controller.Update(PhysicsBody.HugeQuantum + 0.1f, new MovementInput(Forward: true)); - - Assert.Equal(moved.Position.X, stale.Position.X, precision: 4); - Assert.Equal(stale.Position, stale.RenderPosition); - } - - [Fact] - public void Update_LeftoverAboveMinQuantum_ClampsRenderAlphaToCurrentPhysicsPosition() - { - var engine = MakeFlatEngine(); - var controller = new PlayerMovementController(engine); - controller.SetPosition(new Vector3(96f, 96f, 50f), 0x0001); - controller.Yaw = 0f; - - var result = controller.Update( - PhysicsBody.MaxQuantum + PhysicsBody.MinQuantum, - new MovementInput(Forward: true)); - - Assert.Equal(result.Position.X, result.RenderPosition.X, precision: 4); - Assert.Equal(result.Position.Y, result.RenderPosition.Y, precision: 4); - Assert.Equal(result.Position.Z, result.RenderPosition.Z, precision: 4); - } - [Fact] public void Update_RunForward_MoveFasterThanWalk() { diff --git a/tests/AcDream.Core.Tests/Physics/TerrainSurfaceTests.cs b/tests/AcDream.Core.Tests/Physics/TerrainSurfaceTests.cs index c26b214b..7ceda8fd 100644 --- a/tests/AcDream.Core.Tests/Physics/TerrainSurfaceTests.cs +++ b/tests/AcDream.Core.Tests/Physics/TerrainSurfaceTests.cs @@ -67,64 +67,6 @@ public class TerrainSurfaceTests Assert.Equal(42f, surface.SampleZ(300f, 300f)); } - [Fact] - public void SampleZFromHeightmap_AgreesWithInstance_AcrossWholeLandblock() - { - // Issue #48 conformance: the static SampleZFromHeightmap (bilinear - // fallback used at scenery hydration before physics registers a - // landblock) must produce the same Z as the instance SampleZ - // (player physics path) at every (x, y). The previous fallback in - // GameWindow had its diagonal arms swapped — this test pins both - // paths to one source of truth. - // - // Heightmap with distinct per-(x,y) values so every triangle plane - // is genuinely different from the others; flat / planar heightmaps - // would mask a triangle-pick bug because all four corners would - // give the same interpolated Z. - var heights = new byte[81]; - for (int x = 0; x < 9; x++) - for (int y = 0; y < 9; y++) - heights[x * 9 + y] = (byte)((x * 17 + y * 13) % 256); - - var hTable = LinearHeightTable(); - - // Pick a landblock where IsSplitSWtoNE(...) returns BOTH true and - // false across the 64 cells — Holtburg coords (0xA9, 0xB3) work. - const uint lbX = 0xA9, lbY = 0xB3; - var instance = new TerrainSurface(heights, hTable, lbX, lbY); - - // Sample on a fine grid (~1500 points) covering all 64 cells and - // crossing every cell's diagonal boundary. A triangle-pick bug - // would show up as a >0.5 m Z mismatch on the diagonal-spanning - // cells (the corner heights vary by ~10 bytes = 10 Z each cell). - for (float lx = 0.5f; lx < 192f; lx += 5f) - for (float ly = 0.5f; ly < 192f; ly += 5f) - { - float instanceZ = instance.SampleZ(lx, ly); - float staticZ = TerrainSurface.SampleZFromHeightmap( - heights, hTable, lbX, lbY, lx, ly); - Assert.True( - Math.Abs(instanceZ - staticZ) < 0.0001f, - $"Z mismatch at ({lx:F1},{ly:F1}) lb=(0x{lbX:X},0x{lbY:X}): instance={instanceZ:F4} static={staticZ:F4}"); - } - } - - [Fact] - public void SampleZFromHeightmap_RejectsBadInputs() - { - var goodHeights = new byte[81]; - var goodTable = LinearHeightTable(); - - Assert.Throws(() => - TerrainSurface.SampleZFromHeightmap(null!, goodTable, 0, 0, 0f, 0f)); - Assert.Throws(() => - TerrainSurface.SampleZFromHeightmap(goodHeights, null!, 0, 0, 0f, 0f)); - Assert.Throws(() => - TerrainSurface.SampleZFromHeightmap(new byte[80], goodTable, 0, 0, 0f, 0f)); - Assert.Throws(() => - TerrainSurface.SampleZFromHeightmap(goodHeights, new float[255], 0, 0, 0f, 0f)); - } - [Fact] public void SampleSurfacePolygon_ReturnsContainingTriangleVertices() {