docs(spec): Phase A4 — multi-cell BSP iteration design

Port retail's CTransition::check_other_cells (acclient_2013_pseudo_c.txt :272717-272798) into Transition.FindEnvCollisions so the foot-sphere sees walls in EVERY cell it overlaps, not just the one cell the player's center is in. Closes the Holtburg inn vestibule wall walk-through (cell 0xA9B40164 has only 4 polys; adjacent 0xA9B40157 has 23 walls that are never queried today). Architecture: new CellTransit.FindCellSet overload (preserves the candidate HashSet that FindCellList currently discards), new private Transition.CheckOtherCells method (direct port of the retail loop), one wire-up in FindEnvCollisions between the existing primary-cell BSP return and the synthesis fall-through. ~380 LOC total. Out of scope: FindObjCollisions (already landblock-radius broadphased), synthesis multi-cell search (A3's job), var_4c re-target (defer). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-20 15:50:03 +02:00 · 2026-05-20 15:50:03 +02:00 · b100d54829
commit b100d54829
parent fd9daddb37
1 changed files with 370 additions and 0 deletions
--- a/docs/superpowers/specs/2026-05-20-phase-a4-multi-cell-bsp-design.md
+++ b/docs/superpowers/specs/2026-05-20-phase-a4-multi-cell-bsp-design.md
@ -0,0 +1,370 @@
+# Phase A4 — multi-cell BSP iteration (FindEnvCollisions)
+
+**Date:** 2026-05-20
+**Status:** Spec — awaiting user approval before plan-writing.
+**Phase:** ISSUES #83 fix sequence — Phase A4 of A1→A1.5→A1.6→A1.7→A2→A3→A4.
+**Author:** Claude Opus 4.7.
+
+## Summary
+
+`Transition.FindEnvCollisions` currently queries the **one cell** the
+player's center is in for wall/floor BSP collision. Retail's
+`CTransition::check_other_cells` queries **every cell the sphere
+geometrically overlaps** — primary cell first, then each neighbour cell
+in the `cell_array` set built by `CObjCell::find_cell_list`.
+
+Concrete user-visible symptom: the Holtburg inn vestibule (cell
+`0xA9B40164`) has only 4 BSP polys. When the player stands in the
+vestibule but their foot-sphere extends into the adjacent room (cell
+`0xA9B40157`, 23 walls, 38 % hit rate when the player is centered
+there), the adjacent room's walls are never queried — the player
+walks through them.
+
+Fix: port retail's `check_other_cells` loop. Reuse the existing
+`CellTransit.FindCellList` candidate-set machinery (already builds the
+right set; currently throws it away). Wire it into
+`FindEnvCollisions` after the primary cell's BSP returns OK.
+
+## Retail oracle
+
+`CTransition::check_other_cells` at
+`docs/research/named-retail/acclient_2013_pseudo_c.txt:272717-272798`.
+
+```
+0050ae80      CObjCell::find_cell_list(&this->cell_array, &var_4c, &this->sphere_path);
+0050ae90      if (this->cell_array.num_cells > 0)
+0050aec9          do {
+0050ae98              int32_t* cell = this->cell_array.cells.data[i].cell;
+0050aea4              if ((cell != 0 && cell != arg2)) {  // skip primary
+0050aeaf                  result = cell->find_collisions(this);    // vtable +0x88
+0050aeb7                  switch (result) {
+0050afcf                      case 2: case 3: return result;       // Collided/Adjusted
+0050aef3                      case 4: this->collision_info.contact_plane_valid = 0;
+0050aef9                              this->collision_info.contact_plane_is_water = 0;
+0050af07                              return result;               // Slid
+0050aeb7                  }
+0050aea4              }
+0050aec6              i += 1;
+0050aec9          } while (i < this->cell_array.num_cells);
+```
+
+After the loop: `var_4c` is the cell that NOW contains the sphere
+center if it moved outside the primary cell. Retail re-targets
+`sphere_path.check_cell` and `check_pos.objcell_id` accordingly.
+
+## Architecture
+
+Three pieces, each independently testable.
+
+### 1. `CellTransit.FindCellSet` — new overload
+
+Current `FindCellList` returns a single `uint` (the containing cell)
+and discards the candidate `HashSet<uint>` it builds. The new overload
+preserves it:
+
+```csharp
+public static uint FindCellSet(
+    PhysicsDataCache cache,
+    Vector3 worldSphereCenter,
+    float sphereRadius,
+    uint currentCellId,
+    out IReadOnlyCollection<uint> cellSet);
+```
+
+Returns the containing cell id (same as `FindCellList`). The `cellSet`
+out parameter is the full candidate set (every cell the sphere
+overlaps via portal/landcell-grid expansion). Existing
+`FindCellList` callers are untouched.
+
+Implementation: extract the body of `FindCellList` into a private
+helper that returns both, and make `FindCellList` a thin wrapper that
+discards the set. Zero behavior change to existing call sites.
+
+### 2. `Transition.CheckOtherCells` — new private method
+
+Direct port of retail's loop. Lives in `TransitionTypes.cs` next to
+`FindEnvCollisions`.
+
+```csharp
+private TransitionState CheckOtherCells(
+    PhysicsEngine engine,
+    Vector3 footCenter,
+    float sphereRadius,
+    IReadOnlyCollection<uint> cellSet)
+{
+    var sp = SpherePath;
+    var ci = CollisionInfo;
+    if (engine.DataCache is null) return TransitionState.OK;
+
+    // Deterministic ordering for greppable probe logs.
+    foreach (uint cellId in cellSet.Where(c => c != sp.CheckCellId).OrderBy(c => c))
+    {
+        var cell = engine.DataCache.GetCellStruct(cellId);
+        if (cell?.BSP?.Root is null) continue;   // R2 guard: stale CellPhysics
+
+        // Transform sphere into THIS cell's local space.
+        // (Mirrors the primary-cell pattern at TransitionTypes.cs:1413-1485.)
+        var localCenter      = Vector3.Transform(footCenter, cell.InverseWorldTransform);
+        var localCurrCenter  = Vector3.Transform(sp.GlobalCurrCenter[0].Origin, cell.InverseWorldTransform);
+        var localSphere      = new Sphere { Origin = localCenter, Radius = sphereRadius };
+        Sphere? localSphere1 = sp.NumSphere > 1
+            ? new Sphere {
+                Origin = Vector3.Transform(sp.GlobalSphere[1].Origin, cell.InverseWorldTransform),
+                Radius = sp.GlobalSphere[1].Radius }
+            : null;
+
+        if (!Matrix4x4.Decompose(cell.WorldTransform, out _,
+                out Quaternion cellRotation, out Vector3 cellOrigin))
+        {
+            Console.WriteLine(/* warn — fall back to identity + .Translation */);
+            cellRotation = Quaternion.Identity;
+            cellOrigin   = cell.WorldTransform.Translation;
+        }
+
+        var result = BSPQuery.FindCollisions(
+            cell.BSP.Root, cell.Resolved, this,
+            localSphere, localSphere1, localCurrCenter,
+            Vector3.UnitZ, 1.0f, cellRotation, engine,
+            worldOrigin: cellOrigin);
+
+        if (PhysicsDiagnostics.ProbeIndoorBspEnabled)
+            Console.WriteLine($"[other-cells] primary=0x{sp.CheckCellId:X8} iter=0x{cellId:X8} result={result}");
+
+        switch (result)
+        {
+            case TransitionState.Collided:
+            case TransitionState.Adjusted:
+                if (!ObjectInfo.State.HasFlag(ObjectInfoState.Contact))
+                    ci.CollidedWithEnvironment = true;
+                return result;
+            case TransitionState.Slid:
+                ci.ContactPlaneValid   = false;
+                ci.ContactPlaneIsWater = false;
+                return result;
+            // OK: continue
+        }
+    }
+
+    return TransitionState.OK;
+}
+```
+
+### 3. `Transition.FindEnvCollisions` — wire-up
+
+Existing structure (at `src/AcDream.Core/Physics/TransitionTypes.cs:1407-1605`):
+
+```
+indoor branch:
+    1. transform sphere → cell-local
+    2. primary cell BSP.FindCollisions
+    3. if non-OK: return (existing)
+    4. TryFindIndoorWalkablePlane synthesis      ← unchanged
+    5. if HIT: ValidateWalkable indoor plane     ← unchanged
+    6. if MISS: fall through to outdoor terrain  ← unchanged
+outdoor branch:
+    7. SampleTerrainWalkable + ValidateWalkable
+```
+
+A4 changes one thing: insert a CheckOtherCells call between steps 3
+and 4 (indoor branch only — outdoor multi-cell is covered by terrain
+sampling already).
+
+```diff
+  if (cellState != TransitionState.OK)
+  {
+      if (!ObjectInfo.State.HasFlag(ObjectInfoState.Contact))
+          ci.CollidedWithEnvironment = true;
+      return cellState;
+  }
+
+ // A4: query every other cell the sphere overlaps.
+ // Retail oracle: CTransition::check_other_cells at
+ // acclient_2013_pseudo_c.txt:272717-272798.
+ // Discard the containing-cell return — sp.CheckCellId is already
+ // authoritative for the primary cell we just queried.
+ _ = CellTransit.FindCellSet(engine.DataCache, footCenter, sphereRadius,
+                             sp.CheckCellId, out var cellSet);
+ var otherCellsState = CheckOtherCells(engine, footCenter, sphereRadius, cellSet);
+ if (otherCellsState != TransitionState.OK) return otherCellsState;
+
+  // ── Synthesize indoor walkable contact plane ──────────────
+  bool walkableHit = TryFindIndoorWalkablePlane(...);
+```
+
+The synthesis call (`TryFindIndoorWalkablePlane`) stays primary-only.
+A3 deletes it entirely; making it multi-cell is wasted work.
+
+## Mutation contract
+
+All mutations to `Transition` / `SpherePath` / `CollisionInfo` from
+any cell's BSP query are **kept**. No save/restore. Matches retail
+exactly. Specifically:
+
+- Intermediate cells return OK → BSPQuery wrote nothing concerning to
+  Transition state (or wrote a ContactPlane via Path 5 step-up, which
+  is valid world-space data and retainable across cells).
+- The halting cell writes whatever it writes (wall normal, position
+  adjustment) and that becomes the final state.
+- Slid additionally clears `ContactPlaneValid` + `ContactPlaneIsWater`
+  per retail's explicit case-4 handling.
+
+## Out of scope (intentional)
+
+- **`FindObjCollisions`.** Already broadphases by landblock-radius via
+  `ShadowObjects.GetNearbyObjects(currPos, queryRadius, ..., landblockId, ...)`,
+  not cell-keyed. No change needed for shadow-object collision.
+- **`var_4c` re-target of `check_pos.objcell_id`.** Retail's
+  lines 272760-272797 re-target the sphere's check cell mid-frame if
+  it moved outside the primary cell. Our `PhysicsEngine.ResolveCellId`
+  runs on the next tick to re-resolve cell membership; A1.7 closed
+  the "no cell contains player" gap. Defer the var_4c port — would
+  only matter mid-frame, and we lack evidence it's needed.
+- **Synthesis (`TryFindIndoorWalkablePlane`).** Primary-cell only.
+  A3 deletes it entirely; making it multi-cell would just be wasted
+  work.
+- **A2 (PHSP inversion fix).** Orthogonal — A2 fixes per-poly
+  precision at the tangent boundary; A4 fixes which cells get
+  queried. Land them separately.
+
+## Edge cases
+
+| # | Case | Handling |
+|---|---|---|
+| E1 | Cell set has only the primary cell | Loop iterates zero times, returns OK. No-op. |
+| E2 | Candidate cell's `BSP.Root` is null | `continue` (skip). Matches handoff R2 guard. |
+| E3 | Candidate cell has 0 polys but valid BSP root | BSPQuery returns OK (no leaves intersect). Continue. |
+| E4 | Primary cell returns Slid before A4 runs | Existing `if (cellState != OK) return` halts. `CheckOtherCells` never invoked. ✓ |
+| E5 | Outdoor primary with indoor building stab nearby | `FindCellList`'s outdoor branch handles via `AddAllOutsideCells` + `CheckBuildingTransit`. Cell set already includes both. No new code. |
+| E6 | Same shadow object referenced by two cells | N/A for A4. Shadow dedup is `FindObjCollisions`'s concern, already handled. |
+| E7 | `CellPhysics.WorldTransform` doesn't decompose | Same fallback as primary cell: warn + identity rotation + `.Translation`. |
+| E8 | Indoor primary, sphere extends through exit portal to outdoor | `FindCellList`'s `exitOutside` branch adds adjacent outdoor landcells. They have no BSP → E2 skip. Outdoor terrain collision still flows through the existing fall-through after `CheckOtherCells` returns OK. |
+| E9 | Performance — indoor doorway with 7 cells in set | Each iteration ≈ 50 µs BSP + 3 µs transform = ~370 µs per ResolveWithTransition cycle. At 30 Hz = ~11 ms/sec. Acceptable. |
+
+## Risks
+
+- **R1 — iteration order ambiguity.** Deterministic sort by cellId is
+  arbitrary; on close calls a different cell could be "first to
+  Collide" depending on geometry. But Collided is Collided; the
+  player stops either way. Acceptable.
+- **R2 — outdoor candidate cells in the set with no BSP.** Skipped by
+  E2 guard. They don't contribute BSP collisions. Outdoor terrain
+  continues to flow through the existing post-`CheckOtherCells` path.
+- **R3 — BSPQuery state mutation across cells.** Matches retail's
+  no-save/no-restore behavior. Intermediate OK results don't corrupt
+  state because BSPQuery's mutations on OK are either no-op or
+  valid (world-space CP via Path 5).
+
+## Testing strategy
+
+### Unit tests (~280 LOC)
+
+**`tests/AcDream.Core.Tests/Physics/CellTransitFindCellSetTests.cs`** (~80 LOC, new file):
+- `Sphere_FullyInsidePrimaryCell_ReturnsOnlyPrimary` — sphere doesn't touch portal plane → set = {primary}.
+- `Sphere_StraddlingPortal_ReturnsBothCells` — synthetic 2-cell fixture; sphere overlaps portal plane → set = {primary, neighbour}.
+- `Sphere_AtExitPortal_AddsOutdoorNeighbours` — indoor cell with exit portal; sphere straddles → set includes adjacent outdoor landcells.
+
+**`tests/AcDream.Core.Tests/Physics/TransitionCheckOtherCellsTests.cs`** (~120 LOC, new file):
+- `EmptyCellSet_ReturnsOk` — single-cell case, no-op.
+- `AllCellsReturnOk_ReturnsOk` — three cells, all OK → final OK, no halt mutation.
+- `FirstCollidedHalts_KeepsMutations` — second iterated cell returns Collided; verify `CollidedWithEnvironment = true` and remaining cells NOT queried.
+- `SlidClearsContactPlane` — cell returns Slid; verify `ContactPlaneValid = false` + `ContactPlaneIsWater = false`, remaining cells not queried.
+- `NullBspRootIsSkipped` — cell in set has `BSP.Root == null` → skipped, iteration continues.
+
+**`tests/AcDream.Core.Tests/Physics/FindEnvCollisionsMultiCellTests.cs`** (~80 LOC, new file):
+- End-to-end integration: synthetic primary cell (empty BSP) + adjacent cell (one wall poly); sphere center in primary, sphere extends into adjacent → returns Collided with adjacent cell's wall normal.
+
+### Baseline regression
+
+- `dotnet build -c Debug` green.
+- All existing physics tests pass (1129-test baseline holds, per the
+  2026-05-21 handoff). Specifically the indoor-BSP world-origin fix
+  (Bug B, commit `de8ffde`) and the A1-A1.7 series must remain green.
+
+### Visual acceptance (the real test)
+
+Launch with light-probe set (`ACDREAM_PROBE_INDOOR_BSP=1`,
+`ACDREAM_PROBE_CELL=1`, `ACDREAM_PROBE_CELL_CACHE=1`).
+
+1. **Vestibule wall block (primary acceptance).** Walk to Holtburg
+   inn cell `0xA9B40164` (the 4-poly vestibule). Approach a wall in
+   adjacent cell `0xA9B40157`. Wall must BLOCK when the foot-sphere
+   extends across the cell boundary even though the player's center
+   is still in the vestibule. Probe must show
+   `[other-cells] primary=0xA9B40164 iter=0xA9B40157 result=Collided`.
+2. **Stairs walk-through (secondary, likely closes).** Walk up the
+   inn stairs. Riser walls in the adjacent cell from where the
+   player's foot-sphere center is must block, not pass through. If
+   this fails: file a separate sub-issue for per-cell stair
+   physics-poly coverage. Do NOT extend A4's scope to chase it.
+3. **No regression in existing fixes:**
+   - "Thin air" inside cottages still absent (A1 still works).
+   - Walls outside buildings still solid (A1.5 still works).
+   - Doorway exit still transitions correctly (A1.7 still works).
+   - Free-fall through floor at threshold does NOT regress
+     (synthesis untouched).
+
+**Stop rule.** Per CLAUDE.md: three failed visual verifications = write a handoff, not a fourth attempt.
+
+## Probe
+
+Reuse the existing `ACDREAM_PROBE_INDOOR_BSP` env var (already wired to
+`PhysicsDiagnostics.ProbeIndoorBspEnabled`). Add one log line per
+iterated cell:
+
+```
+[other-cells] primary=0x<id> iter=0x<id> result=<state>
+```
+
+Low volume — only fires when `CheckOtherCells` actually iterates a
+non-primary cell. Suppressed when the flag is off.
+
+## File map
+
+| Change | File | Approx LOC |
+|---|---|---:|
+| New `FindCellSet` overload | `src/AcDream.Core/Physics/CellTransit.cs` | +30 |
+| New `CheckOtherCells` private method | `src/AcDream.Core/Physics/TransitionTypes.cs` | +60 |
+| Wire-up in `FindEnvCollisions` | `src/AcDream.Core/Physics/TransitionTypes.cs` | +6 |
+| Cell-set enumeration tests | `tests/AcDream.Core.Tests/Physics/CellTransitFindCellSetTests.cs` | +80 |
+| CheckOtherCells halt-semantics tests | `tests/AcDream.Core.Tests/Physics/TransitionCheckOtherCellsTests.cs` | +120 |
+| End-to-end multi-cell integration | `tests/AcDream.Core.Tests/Physics/FindEnvCollisionsMultiCellTests.cs` | +80 |
+| **Total** | | **~380 LOC** |
+
+## Acceptance criteria
+
+- [ ] `dotnet build -c Debug` green.
+- [ ] `dotnet test` green; 1129-test physics baseline holds, no
+      regressions in A1/A1.5/A1.6/A1.7/Bug B suites.
+- [ ] Three new test files pass with all listed cases.
+- [ ] Visual acceptance #1 passes — vestibule wall blocks at Holtburg
+      inn.
+- [ ] Visual acceptance #3 passes — no regressions in prior fixes.
+- [ ] Roadmap updated with A4 shipped status.
+- [ ] `docs/research/2026-05-21-open-items-pickup-prompt.md` updated to
+      reflect A4 closed; remaining items (stairs verify, A2, A3,
+      lighting) re-ordered if A4's outcome changes priorities.
+
+## Retail decomp anchors (verbatim)
+
+- `acclient_2013_pseudo_c.txt:272717-272798` — `CTransition::check_other_cells` (A4 oracle).
+- `:323725-323939` — `BSPTREE::find_collisions` (already ported; the call we make per cell).
+- `:309085-309092` — `CObjCell::find_cell_list` sphere-path variant (already ported as `CellTransit.FindCellList`; A4 adds the set-returning overload).
+- `:308742-308783` — `CObjCell::find_cell_list` `(Position, n_spheres, CSphere*)` variant (parent of the sphere-path variant).
+
+## Why A4 doesn't fix everything
+
+A4 is a wall-collision fix. It does NOT:
+
+- Synthesize a floor plane (that's `TryFindIndoorWalkablePlane`'s job —
+  unchanged by A4, removed by A3).
+- Handle the doorway "no floor poly at this XY" free-fall (that's
+  Bug A's actual root cause from the 2026-05-20 session — A3's job
+  to investigate further, after A4 + A2 land).
+- Fix the tangent-boundary `polygon_hits_sphere` rejection (that's
+  A2).
+
+A4 ships the iteration foundation. A2 and A3 build on it. Stairs
+walk-through is a known unknown — A4 *should* fix it via the same
+multi-cell BSP path, but if a stair's geometry is laid out as a
+single cell (no boundary to straddle), the BSP itself may have a
+coverage gap that needs separate investigation.