From 1ede87a1355d8ba4882cc9b4ca12f2eca3eb1cd3 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 8 May 2026 12:05:04 +0200 Subject: [PATCH] =?UTF-8?q?docs:=20flag=20N.2=20blocker=20=E2=80=94=20WB?= =?UTF-8?q?=20terrain=20split=20formula=20diverges=20from=20retail?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit during N.3 follow-up uncovered that WB's TerrainUtils CalculateSplitDirection uses a different math expression than retail's FSplitNESW (the AC2D-cited polynomial 0x0CCAC033 etc that our visual terrain mesh and physics already share). Substituting TerrainSurface.SampleZ with WB's GetHeight in isolation would re-introduce the triangle-Z hover bug from earlier work — physics and visual mesh would pick different diagonals on disputed cells. Updates: - ISSUE #51 documents the divergence with file references and the research that's needed when N.5 picks this up. - Roadmap N.2 entry flags the dependency on N.5 and the reasoning ("not low-risk after all"). N.1's conformance proved slope-filtering equivalence (boolean walkable verdict), not formula equivalence. The lesson is captured in memory (feedback_wb_migration_formulas.md, not in-repo). Co-Authored-By: Claude Opus 4.6 --- docs/ISSUES.md | 57 ++++++++++++++++++++++++++++++++ docs/plans/2026-04-11-roadmap.md | 21 +++++++++--- 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 464590f..d3fd991 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -46,6 +46,63 @@ Copy this block when adding a new issue: # Active issues +## #51 — WB's terrain-split formula diverges from retail's `FSplitNESW` + +**Status:** OPEN +**Severity:** MEDIUM (blocks isolated N.2; affects sequencing of N-phase migration) +**Filed:** 2026-05-08 +**Component:** terrain math / Phase N (WorldBuilder rendering migration) + +**Description:** WB's `TerrainUtils.CalculateSplitDirection` +([references/WorldBuilder/WorldBuilder.Shared/Modules/Landscape/Lib/TerrainUtils.cs:44](references/WorldBuilder/WorldBuilder.Shared/Modules/Landscape/Lib/TerrainUtils.cs:44)) +uses a different math expression from retail's `FSplitNESW` +(documented in CLAUDE.md as **the** real AC terrain split formula, +constants `0x0CCAC033` / `0x421BE3BD` / `0x6C1AC587` / `0x519B8F25`). +Ours is a degree-2 polynomial in (x,y); WB's is linear in (x,y). +They cannot be algebraically equivalent and disagree on a meaningful +fraction of cells. + +**Concrete impact:** On any cell where the formulas pick different +diagonals, the same world position (X, Y) maps to different terrain +heights — up to ~2m for a sloped cell with one elevated corner. If a +caller mixes "WB-formula path" and "AC2D-formula path" for the same +cell, the player physics floats above or sinks below the visible +ground. This is the bug class fixed in +[src/AcDream.Core/Physics/TerrainSurface.cs:113-120](src/AcDream.Core/Physics/TerrainSurface.cs:113) +(diagonal-direction inversion). + +**Files implicated:** +- `src/AcDream.Core/Physics/TerrainSurface.cs` — uses AC2D formula via + `IsSplitSWtoNE` +- `src/AcDream.Core/World/TerrainBlending.cs` — visual mesh, also AC2D +- `references/WorldBuilder/WorldBuilder.Shared/Modules/Landscape/Lib/TerrainUtils.cs:44` + — WB's diverging formula +- `references/WorldBuilder/Chorizite.OpenGLSDLBackend/Lib/TerrainGeometryGenerator.cs` + — WB's render mesh (presumably also uses WB's formula in lockstep) + +**Sequencing implication:** Phase N.2 (terrain math helpers +substitution) cannot be shipped in isolation — it must land alongside +N.5 (visual terrain renderer migration), at which point both physics +and visual mesh switch to WB's formula together. Roadmap N.2 entry +flags this dependency. + +**Research needed (when N.5 picks this up):** +1. Quantify divergence: run WB's `CalculateSplitDirection` and our + `IsSplitSWtoNE` across all (lbX, lbY, cellX, cellY) tuples for a + representative landblock set; record disagreement rate. +2. Confirm WB's `TerrainGeometryGenerator` uses WB's formula in its + render mesh — if so, switching everything to WB's formula keeps + visual + physics synced. (Highly likely.) +3. Decide whether ANY retail-conformance test (e.g., physics matching + server-authoritative Z within tolerance) is invalidated by the + formula change. + +**Acceptance:** Resolved when N.5 lands and both physics + visual +mesh use WB's split formula, OR when we decide to keep the AC2D +formula and patch WB's renderer in our fork. + +--- + ## #50 — Road-edge tree at 0xA9B1 visible in acdream but not retail **Status:** OPEN diff --git a/docs/plans/2026-04-11-roadmap.md b/docs/plans/2026-04-11-roadmap.md index 45edf2c..010333c 100644 --- a/docs/plans/2026-04-11-roadmap.md +++ b/docs/plans/2026-04-11-roadmap.md @@ -574,10 +574,23 @@ for our deletions/additions; merge upstream `master` periodically. formula was ~180° off from retail's actual `Frame::set_heading` atan2 round-trip). One known cosmetic difference filed in ISSUES.md (road-edge tree at landblock 0xA9B1). -- **N.2 — Terrain math helpers.** Refactor `TerrainSurface.SampleZ` / - `SampleNormal` / `SampleSurface` to call WB's `TerrainUtils.GetHeight` - / `GetNormal` internally. ~1-2 days. Smallest remaining N phase, low - risk after N.1's conformance proof on GetNormal. +- **N.2 — Terrain math helpers.** ⚠️ **Blocked on N.5 — do not attempt + in isolation.** Originally scoped as a 1-2 day low-risk substitution + of `TerrainSurface.SampleZ` / `SampleSurface` / `SampleSurfacePolygon` + with WB's `TerrainUtils.GetHeight` / `GetNormal`. Audit during N.3 + follow-up uncovered that **WB's `CalculateSplitDirection` uses a + different formula than retail's `FSplitNESW`** (the AC2D-cited + polynomial `0x0CCAC033` / `0x421BE3BD` / `0x6C1AC587` / `0x519B8F25` + that our visual terrain mesh and physics already share). The + formulas pick different cell-diagonals on disputed cells, producing + up to ~2m Z divergence at the same world position. Substituting + physics-side alone would un-sync physics from the still-ours visual + mesh — exactly the triangle-Z hover bug class. N.1's conformance + test proved WB's `GetNormal` is good enough for slope-filtering + (boolean walkable check) but NOT that WB's height formula matches + retail. Resolution: fold this work into **N.5** when the visual + mesh switches to WB's renderer in lockstep with physics. Until + then, leave `TerrainSurface` alone. See ISSUE #51. - **✓ SHIPPED — N.3 — Texture decoding.** Shipped 2026-05-08. `SurfaceDecoder` now delegates INDEX16 / P8 / A8R8G8B8 / R8G8B8 / A8 to WB's `TextureHelpers.Fill*`. The A8 divergence (our old code did R=G=B=A=val