From b100d54829e49c5e9ad1718cd67b789045027fcc Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 20 May 2026 15:50:03 +0200 Subject: [PATCH] =?UTF-8?q?docs(spec):=20Phase=20A4=20=E2=80=94=20multi-ce?= =?UTF-8?q?ll=20BSP=20iteration=20design?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- ...26-05-20-phase-a4-multi-cell-bsp-design.md | 370 ++++++++++++++++++ 1 file changed, 370 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-20-phase-a4-multi-cell-bsp-design.md diff --git 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 new file mode 100644 index 0000000..2ccdb62 --- /dev/null +++ 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` it builds. The new overload +preserves it: + +```csharp +public static uint FindCellSet( + PhysicsDataCache cache, + Vector3 worldSphereCenter, + float sphereRadius, + uint currentCellId, + out IReadOnlyCollection 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 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 iter=0x result= +``` + +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.