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>
191 lines
8.2 KiB
Markdown
191 lines
8.2 KiB
Markdown
# 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.
|