acdream/docs/superpowers/specs/2026-05-08-phase-n1-scenery-via-wb-helpers-design.md
Erik 8a06fce7a5 spec(rendering): Phase N WorldBuilder migration design + N.1 scenery
Adds two design docs and a roadmap entry for the strategic shift
from "port retail rendering algorithms ourselves" to "depend on a
fork of Chorizite/WorldBuilder for rendering + dat-handling."

- docs/superpowers/specs/2026-05-08-phase-n-worldbuilder-migration-design.md
  — parent design: integration model (fork + submodule), 10 sub-phases
  (N.0 setup through N.10 GL consolidation), strangler-fig phasing
  with per-phase feature flags, retail-decomp boundary clarified for
  what WB does NOT cover (network, physics, animation, motion, UI,
  plugin, audio, chat).

- docs/superpowers/specs/2026-05-08-phase-n1-scenery-via-wb-helpers-design.md
  — N.1 detailed design: replace IsOnRoad / DisplaceObject /
  slope-normal calc / rotation / scale inside SceneryGenerator.Generate()
  with calls to WB's SceneryHelpers + TerrainUtils. Keep data flow,
  ScenerySpawn shape, and renderer integration. Add small LandBlock →
  TerrainEntry[] adapter. Feature flag ACDREAM_USE_WB_SCENERY=1.

- docs/plans/2026-04-11-roadmap.md — Phase N entry added between
  Phase M and Phase J. Lists all 10 sub-phases with effort estimates.

Fork already created at https://github.com/eriknihlen/WorldBuilder.
N.0 setup (replace references/WorldBuilder/ snapshot with submodule,
add project references, build green) is the next implementation step.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-08 08:47:23 +02:00

191 lines
8.2 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Phase N.1 — Scenery via WorldBuilder Helpers: Design
**Date:** 2026-05-08
**Parent design:** [`2026-05-08-phase-n-worldbuilder-migration-design.md`](2026-05-08-phase-n-worldbuilder-migration-design.md)
**Status:** Design complete, awaiting plan generation.
## Goal
Replace the algorithm guts of `SceneryGenerator.Generate()` with calls
to WorldBuilder's stateless `SceneryHelpers` and `TerrainUtils`. Keep
our data flow, our `ScenerySpawn` shape, and our renderer integration
unchanged.
## Why scenery first
1. **Active bug source.** Issues #48, #49 are scenery-related; the
investigation in this session uncovered another (the road-edge tree
at `0xA9B1`) we couldn't easily root-cause despite our code looking
identical to WB's.
2. **Smallest coherent slice.** Scenery placement uses only stateless
helpers from WB (Displace, OnRoad, GetNormal, CheckSlope, RotateObj,
ScaleObj). No need to take WB's `SceneryRenderManager`, no need for
editor-shaped data flow.
3. **Proves the integration pattern.** Phase N.0 wires up the
submodule + project references. N.1 uses them with a tiny surface
area. If something is wrong with the dependency model, we discover
it cheaply.
## Architecture
### What changes
`src/AcDream.Core/World/SceneryGenerator.cs`:
- Remove our private `IsOnRoad(LandBlock, float, float)` helper.
- Remove our private `DisplaceObject(ObjectDesc, uint, uint, uint)` helper.
- Remove the `RoadHalfWidth` constant.
- Replace inline algorithm calls with WB equivalents (see table below).
New file `src/AcDream.Core/World/WbSceneryAdapter.cs` (or similar
location — TBD during implementation):
- Helper `BuildTerrainEntries(LandBlock block) → TerrainEntry[]`
converting our `DatReaderWriter.DBObjs.LandBlock` (the dat type) into
the `TerrainEntry[]` shape WB's `TerrainUtils` expects (9×9 grid,
Type/Scenery/Road/Height fields per vertex).
- Helper for `RegionInfo` if needed (small wrapper over our
`Region` dat).
### Algorithm-call substitution table
| Today (ours) | Phase N.1 (WB) |
|---|---|
| `IsRoadVertex(raw)` (kept; small util) | unchanged — small predicate, no benefit to swap |
| `IsOnRoad(block, lx, ly)` | `TerrainUtils.OnRoad(new Vector3(lx, ly, 0), terrainEntries)` |
| `DisplaceObject(obj, gx, gy, j)` | `SceneryHelpers.Displace(obj, gx, gy, j)` |
| Slope normal: `TerrainSurface.SampleNormalZFromHeightmap(...)` | `TerrainUtils.GetNormal(region, terrainEntries, lbX, lbY, lbOffset).Z` |
| Slope check: `nz < obj.MinSlope \|\| nz > obj.MaxSlope` | `SceneryHelpers.CheckSlope(obj, normal.Z)` (returns bool) |
| Rotation logic (`AFrame::set_heading` reproduction) | `SceneryHelpers.RotateObj(obj, gx, gy, j, localPos)` (returns Quaternion) |
| Scale logic (LCG + Pow + clamp) | `SceneryHelpers.ScaleObj(obj, gx, gy, j)` (returns float) |
### What does NOT change
- The 9×9 vertex loop (`for (x = 0; x < 9; x++) for (y = 0; y < 9; y++)`).
- Scene selection hash.
- Frequency roll.
- `obj.WeenieObj != 0` skip (weenie entries are dynamic spawns).
- Bounds check `lx, ly ∈ [0, 192)`.
- Per-spawn building check using our `buildingCells` HashSet.
- `BaseLoc.Z` offset application.
- `ScenerySpawn` record shape returned to the renderer.
- `Generate()` method signature — same parameters, same return type.
### What about `obj_within_block`?
We attempted this during the bug investigation but it's too aggressive
when applied with the model's actual sorting sphere radius (rejects
trees that should be there). WB also doesn't apply it. The retail
behavior we couldn't reproduce stays unreproduced for now — we accept
that as a known minor cosmetic discrepancy and move on. The point of
N.1 is matching WB's behavior, not retail's. If WB and retail
disagree, that's a WB-upstream problem to file separately.
## Components
### Files modified
- `src/AcDream.Core/World/SceneryGenerator.cs` — algorithm-call swap.
- `src/AcDream.Core/AcDream.Core.csproj` — already has WB project ref
from N.0.
### Files added
- `src/AcDream.Core/World/WbSceneryAdapter.cs` — `LandBlock →
TerrainEntry[]` and any other small adapters needed.
- `tests/AcDream.Core.Tests/World/SceneryGeneratorWbConformanceTests.cs`
— side-by-side test asserting our generator's output equals what
comes out when the same algorithms are called via WB directly.
### Files deleted (eventually, after flag is on by default)
- The deleted helpers in `SceneryGenerator.cs` mentioned above.
### Feature flag
Phase 1 of the rollout: `ACDREAM_USE_WB_SCENERY=1` (default off — old
path runs). When the env var is set, the new WB-backed path runs.
Phase 2 (after visual verification at Holtburg / `0xA9B1`): flag
default-on. Old path can still be reached via
`ACDREAM_USE_WB_SCENERY=0`.
Phase 3 (one or two sessions later, after no regressions): delete the
flag and the old code paths entirely.
## Done criteria
1. `dotnet build` green with no new warnings.
2. All existing tests pass (870+).
3. New conformance test passes: `SceneryGeneratorWbConformanceTests`
runs both code paths against fixture LandBlock data and asserts
identical spawn lists (same ObjectId, same LocalPosition within
1e-4, same Rotation within 1e-4, same Scale within 1e-4).
4. Visual verification at landblock `0xA9B1` (Holtburg area):
- The offending tree near the road that retail/WB do not show is
**gone** in our render.
- Issue #49's previously missing scenery (the tree from the 9×9
loop expansion) is **still visible**.
- No new visual regressions in surrounding landblocks during a
brief flight around Holtburg.
5. Issue #49 stays closed; no new issues filed.
## Risks (Phase-N.1-specific)
1. **`TerrainEntry` field semantics.** WB packs Type/Scenery/Road/
Height into the `TerrainEntry` struct in a specific format. Getting
the adapter wrong means OnRoad / scenery selection produces
different results than ours. Mitigation: read
`WorldBuilder.Shared/Modules/Landscape/Models/TerrainEntry.cs`
carefully; cross-check against WB's `TerrainUtils.GetRoad` /
`GetTerrainEntryForCell` to confirm field encoding.
2. **`RegionInfo` dependencies.** WB's `TerrainUtils.GetNormal` takes
a `RegionInfo` parameter. We need to either build a minimal
`RegionInfo` from our `Region` dat or call WB's normal calc
differently. Mitigation: investigate during implementation; expect
this is a small wrapper.
3. **`obj.MaxScale / obj.MinScale` divide-by-zero.** Our code checks
`if (obj.MinScale == obj.MaxScale)` first; WB's `ScaleObj` does the
same per-line review of `references/WorldBuilder/Chorizite.OpenGLSDLBackend/Lib/SceneryHelpers.cs:42-51`. Should be a non-issue.
4. **Rotation quaternion convention.** Our rotation produces
`headingQuat * baseLoc.Orientation`. WB's `RotateObj` calls
`SetHeading` which does its own composition. Need to confirm the
resulting quaternion is the same convention our renderer expects.
Mitigation: the conformance test catches this if it's wrong.
## Testing
### Conformance test (new)
`SceneryGeneratorWbConformanceTests`:
- Construct a synthetic `LandBlock` with known terrain data.
- Run `SceneryGenerator.Generate(...)` with `ACDREAM_USE_WB_SCENERY=0`
and again with `=1`.
- Assert spawn counts equal.
- Assert each spawn's ObjectId, LocalPosition (within 1e-4), Rotation
(within 1e-4 per component), Scale (within 1e-4) are equal.
### Existing tests
`SceneryGeneratorTests` covers: road-vertex predicate, edge-vertex
displacement bounds, interior-vertex displacement bounds. These tests
exercise our internal helpers (`IsRoadVertex`, `DisplaceObject`).
After N.1, the `DisplaceObject` test must be either deleted (if we
delete the helper) or replaced (if we keep `IsRoadVertex` as a small
predicate — it's only one bit-test).
### Visual verification
User runs the client against ACE locally:
- Navigate to landblock `0xA9B1` (Holtburg). Verify offending tree
near road is gone.
- Confirm Issue #49's tree is still visible.
- Fly around Holtburg, scan visible scenery for any obvious
regression.
## Out of scope for N.1
- Replacing our `SceneryRenderManager` (we don't have one — we have
`SceneryGenerator` producing `ScenerySpawn[]` and the renderer
consuming it directly). N.1 only touches the generator.
- Replacing our terrain math helpers (that's N.2).
- Replacing the static-object renderer (that's N.6).
- Anything in N.2-N.10.