erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

docs: indoor walkable-plane BSP port partial-ship handoff

Browse source 
Foundation work (6 commits ff548b9..f845b22) landed but visual
verification 2026-05-19 FAILED to fix the user-reported indoor bugs.
Documenting the deeper diagnosis + the next phase target without
reverting the foundation work.

What landed (kept):
- BSPQuery.FindWalkableInternal gained ref ushort hitPolyId (Task 1).
- New public BSPQuery.FindWalkableSphere wrapper over the existing
  retail-faithful walkable finder (Task 2).
- Transition.TryFindIndoorWalkablePlane refactored through it,
  PointInPolygonXY deleted (Task 3).
- [indoor-walkable] runtime-toggleable probe (Task 4).
- 5 new tests + 9 updated existing tests, all green; build clean.

What didn't fix: cellar descent FAIL, 2nd-floor walking FAIL
(intermittent falling-stuck), single-floor cottage REGRESSION (was
stable, now intermittent falling-stuck), phantom collisions PERSIST.

Probe evidence: 1443 MISS / 2 HIT over 1445 calls. Smoking gun:
foot-sphere-tangent-to-floor case fails PolygonHitsSpherePrecise's
|dist| > radius - epsilon check by ~0.0002. The BSP walker is
correct; the caller (TryFindIndoorWalkablePlane) is misusing it.

Root cause (deeper than originally diagnosed): TryFindIndoorWalkablePlane
exists only as a Phase 2 commit eb0f772 stop-gap. Retail doesn't
synthesize a ContactPlane per frame — retail RETAINS the previous
frame's plane when the BSP says no collision. Retail's find_walkable
only runs inside step_sphere_down (a sweep), never as a standing-still
query.

Next phase target: port retail's ContactPlane retention so the
resolver retains state across frames. Likely eliminates the per-frame
TryFindIndoorWalkablePlane call entirely. Foundation work (BSP walker
+ probe + tests) remains useful regardless.

ISSUES #83 remains OPEN with the deeper diagnosis.
Roadmap header updated to reflect partial-ship status.
Handoff at docs/research/2026-05-19-indoor-walkable-plane-bsp-port-shipped-handoff.md.

Spec: docs/superpowers/specs/2026-05-19-indoor-walkable-plane-bsp-port-design.md
Plan: docs/superpowers/plans/2026-05-19-indoor-walkable-plane-bsp-port.md

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik 2026-05-20 07:03:14 +02:00
parent f845b2241a
commit c6b3fd6ebf
3 changed files with 253 additions and 18 deletions
Download patch file Download diff file Expand all files Collapse all files

187
docs/research/2026-05-19-indoor-walkable-plane-bsp-port-shipped-handoff.md Normal file
View file

@ -0,0 +1,187 @@
# Indoor walkable-plane BSP port — partial-ship handoff (2026-05-19)
**Outcome:** Foundation shipped (6 commits). Visual verification FAILED. User-reported bugs (cellar descent, 2nd-floor walking, phantom collisions) remain unresolved. Root cause now diagnosed deeper than originally thought; next phase needs to port retail's `ContactPlane` retention mechanism. Foundation work (BSP walker + probe + tests) is useful regardless of the next approach.
---
## TL;DR
I diagnosed the wrong root cause initially. I assumed `TryFindIndoorWalkablePlane`'s linear first-match XY scan picking the wrong polygon was the bug, and built a retail-faithful BSP-walker replacement (`BSPQuery.FindWalkableSphere` wrapper over the existing `FindWalkableInternal` port of `BSPNODE::find_walkable` + `BSPLEAF::find_walkable`). The BSP walker is correct, but it returns MISS for the standing-grounded case (foot sphere tangent to floor → `PolygonHitsSpherePrecise` correctly rejects tangent contact by ~0.0002 epsilon).
The actual root cause: **`TryFindIndoorWalkablePlane` shouldn't exist at all**. It was added as a Phase 2 commit `eb0f772` stop-gap to synthesize a `ContactPlane` every frame when the indoor BSP returns OK. Retail doesn't do this — retail RETAINS the previous frame's `ContactPlane` when the collision dispatcher reports no collision. There is no retail analog of `find_walkable` as a standing-still query. `find_walkable` only runs inside a downward sphere sweep (`step_sphere_down`), where the sphere is moving and the overlap test is meaningful.
---
## What shipped (foundation)
6 commits, `ff548b9` → `f845b22`. `dotnet build -c Debug` clean; 8 pre-existing test failures unchanged baseline; 5 new tests + 9 updated existing tests all pass.
| # | SHA | Subject |
|---|---|---|
| 1 | `ff548b9` | `refactor(physics): expose hitPolyId from FindWalkableInternal` |
| 2 | `7f55e14` | `feat(physics): add BSPQuery.FindWalkableSphere wrapper` (+ 4 unit tests) |
| 3 | `86ecdf9` | `fix(physics): tighten FindWalkableSphere test assertions + header` (code review fix) |
| 4 | `91b29d1` | `fix(physics): route indoor walkable-plane synthesis through retail BSP walker` |
| 5 | `7c516ed` | `fix(physics): document adjustedCenter discard + restore wall-poly test` (code review fix) |
| 6 | `f845b22` | `feat(physics): add [indoor-walkable] probe line` |
**Files touched:**
- `src/AcDream.Core/Physics/BSPQuery.cs` — `FindWalkableInternal` gained `ref ushort hitPolyId`; new public `FindWalkableSphere` wrapper.
- `src/AcDream.Core/Physics/TransitionTypes.cs` — `TryFindIndoorWalkablePlane` refactored from `static` linear scan to instance method routing through `FindWalkableSphere` with `WalkableAllowance` save/restore. `PointInPolygonXY` deleted. `[indoor-walkable]` probe added at the `FindEnvCollisions` callsite.
- `tests/AcDream.Core.Tests/Physics/BSPQueryTests.cs` — 4 new `FindWalkableSphere` unit tests.
- `tests/AcDream.Core.Tests/Physics/TransitionTypesTests.cs` — new file, integration test for two-overlapping-floors + WalkableAllowance preservation.
- `tests/AcDream.Core.Tests/Physics/IndoorWalkablePlaneTests.cs` — 9 tests updated to new instance-method + sphereRadius signature with BSP fixtures; 2 `PointInPolygonXY` tests deleted; 1 new wall-poly integration test.
---
## Visual verification — FAIL (user-driven, 2026-05-19)
Launch flags: `ACDREAM_DEVTOOLS=1`, `ACDREAM_PROBE_INDOOR_BSP=1`. Log: `launch-walkable-fix-6.log` (latest run).
User report verbatim:
> Cant walk down to the cellar. Looks like ground is blocking.
> I get stuck sometimes in a falling animation at random places.
> When I walk up on second floors. I get stuck sometimes on random places in falling animation.
> Lightning is still broken.
> Get phantom collison in rooms.
> NO change
Result against acceptance scenarios:
| Scenario | Pre-ship | Post-ship | Outcome |
|---|---|---|---|
| Cellar descent | "ground blocking" | "ground blocking" | **FAIL** — no change |
| 2nd-floor walking | "snaps back / invisible obstacles" | "intermittent falling-stuck" | **FAIL** — different symptom, still broken |
| Single-floor cottage walking | stable | "intermittent falling-stuck at random spots" | **REGRESSION** — degraded from stable to unstable |
| Phantom collisions in rooms | present | present | **PERSIST** |
| Indoor lightning (#79/#80/#81/#82) | broken | broken | unchanged (out of scope for this phase) |
---
## Probe evidence (from launch 1)
`[indoor-walkable]` probe captured 1445 calls in a Holtburg-area session. **1443 MISS / 2 HIT.**
Sample HIT line:
```
[indoor-walkable] cell=0xA9B40150 wpos=(132.258,16.524,94.480) probe=0.50 result=HIT poly=0x0000 wn=(0.000,0.000,1.000) wD=-94.020 dz=+0.46
```
Sample MISS line:
```
[indoor-walkable] cell=0xA9B40150 wpos=(132.258,16.524,94.500) probe=0.50 result=MISS
```
The 20mm Z oscillation between `94.480` (HIT) and `94.500` (MISS) is the smoking gun:
- World physics floor (after +0.02f cell-origin Z-bump in `PhysicsDataCache.CacheCellStruct`) is at `Z=94.020`.
- When foot center is at `Z=94.500` (= floor + radius), distance to plane = `0.48` = sphere radius. `PolygonHitsSpherePrecise` checks `|dist| > radius - epsilon` (line 117 of BSPQuery.cs). `0.48 > 0.4798` → **rejected by ~0.0002**.
- When foot center is at `Z=94.480` (= floor + 0.46), distance = `0.46 < 0.4798` → accepted, HIT.
- The resolver oscillates between these two positions as the indoor walkable plane and the outdoor terrain backstop alternate as the contact source.
---
## Why the fix doesn't work — deeper diagnosis
`TryFindIndoorWalkablePlane` exists only as a Phase 2 stop-gap (commit `eb0f772`). It was added because the indoor BSP collision branch in `FindEnvCollisions` returns OK when the player is grounded standing still, but the resolver then needed a `ContactPlane` to feed `ValidateWalkable`. Without a synthesized indoor plane, the code fell through to outdoor terrain backstop, which is BELOW the indoor floor by `+0.02f`, marking the player as floating → falling-stuck. The Phase 2 fix synthesized a plane from `cellPhysics.Resolved` via a linear XY scan.
My Task 3 refactor swapped that linear scan for the retail-faithful BSP walker (`BSPQuery.FindWalkableInternal`). The BSP walker is correct — it implements `BSPNODE::find_walkable` + `BSPLEAF::find_walkable` faithfully. But in retail, this function is called from `BSPTREE::step_sphere_down` inside a movement sweep, where the sphere is moving downward. `walkable_hits_sphere` requires the sphere to overlap the plane (`|dist| < radius - eps`), which is satisfied during the sweep because the moving sphere penetrates the plane mid-sweep. In our standing-grounded use case, the sphere is tangent (foot resting on floor), not penetrating → no overlap → no walkable found → MISS.
**Retail's actual flow for the standing-grounded case:**
1. Player at rest on floor. ContactPlane retained from previous frame.
2. Frame tick. Gravity + movement applied.
3. `CTransition::transitional_insert` runs.
4. `find_collisions` Path 5 (Contact branch): `sphere_intersects_poly` test.
- If the sphere penetrates the floor (gravity moved it slightly down), `step_sphere_up` runs → `step_down` → `step_sphere_down` → `find_walkable` → finds the floor → `adjust_sphere_to_plane` snaps it up to tangent → ContactPlane updated.
- If the sphere does NOT penetrate (still tangent from last frame), Path 5 returns OK. **ContactPlane is NOT recomputed — it's retained from last frame.**
5. Player walks horizontally. Same as above — ContactPlane persists.
Our acdream code:
- Per-frame `FindEnvCollisions` calls indoor BSP `FindCollisions`.
- Indoor BSP returns OK (no collision).
- We call `TryFindIndoorWalkablePlane` to RECOMPUTE the ContactPlane from scratch. This is the WRONG behavior — retail doesn't recompute.
- The recomputation fails (BSP walker can't handle tangent sphere) or succeeds with a slightly-off plane (linear scan returning the wrong polygon's Z).
- Either way: the ContactPlane is unstable frame-to-frame → resolver state oscillates → player gets stuck in falling animation.
---
## Recommended next phase: ContactPlane retention
Port retail's `ContactPlane` retention so the resolver retains the previous frame's plane when the BSP says "no collision," instead of re-synthesizing every frame.
**Investigation targets (retail decomp):**
- `CTransition::transitional_insert` (acclient_2013_pseudo_c.txt:273137) — the main per-frame resolver entry. Note line 273165: `if (edi != OK_TS) this->sphere_path.neg_poly_hit = 0;` — only mutates state on non-OK results.
- `CPhysicsObj::transition` family — where `LastKnownContactPlane` is read/written.
- Search the decomp for `last_known_contact_plane` and `contact_plane_valid` to map the full lifecycle.
- `CTransition::check_walkable` (referenced at line 273202) — possibly involved in walkable persistence.
**Likely shape of the fix:**
- In `Transition.FindEnvCollisions` (TransitionTypes.cs:1262), when indoor BSP returns OK, DO NOT call `TryFindIndoorWalkablePlane`. Instead, retain the existing `CollisionInfo.ContactPlane` (which was set by the previous frame's step-up or step-down).
- Only update the ContactPlane when an actual collision/step event occurs (Path 4 land, Path 5 step-up-success, Path 3 step-down-success).
- Outdoor terrain backstop remains for the outdoor case but is gated on `!IsIndoor(cellId)`.
**Foundation work to keep:**
- `BSPQuery.FindWalkableSphere` wrapper — useful for any future "find a walkable plane indoors" query (e.g., spawn-placement, teleport-target verification).
- `FindWalkableInternal`'s `hitPolyId` ref param — same.
- `[indoor-walkable]` probe — keep, but expect it to fire less often once retention is in place (only when the sphere is actually penetrating).
- All 5 new tests + 9 updated tests — they verify the BSP walker's correctness, which is unchanged in the next phase.
**Foundation work to delete (or refactor):**
- `Transition.TryFindIndoorWalkablePlane` — likely deleted entirely, OR kept as an out-of-band synthesis path for edge cases (initial spawn, cell-id promotion mid-frame) but no longer called per-frame from `FindEnvCollisions`.
- `INDOOR_WALKABLE_PROBE_DISTANCE` constant — deleted with `TryFindIndoorWalkablePlane`, or kept for the out-of-band use case.
---
## What NOT to do
- **Do not** add a sphere-offset hack to make `PolygonHitsSpherePrecise` accept tangent contact. That mis-aligns acdream's overlap semantics with retail's. The right answer is to not call `find_walkable` in the standing-still case at all.
- **Do not** revert the 6 foundation commits. They are correct retail-faithful ports; the BSP walker is needed for legitimate use cases (just not the one we wired it to).
- **Do not** widen the +0.02f Z-bump or try to compensate for it in the resolver. The bump is a render concern; it should remain transparent to physics. The bug is in the per-frame ContactPlane recompute, not the bump itself.
---
## Quick reference for the next-session implementer
**Spec to read first (this phase's, for context — but don't re-execute it):**
- `docs/superpowers/specs/2026-05-19-indoor-walkable-plane-bsp-port-design.md` (committed `165f67a`)
- `docs/superpowers/plans/2026-05-19-indoor-walkable-plane-bsp-port.md` (committed `e62d076`)
**Code anchors:**
- [`src/AcDream.Core/Physics/TransitionTypes.cs:1262`](../../src/AcDream.Core/Physics/TransitionTypes.cs#L1262) — `FindEnvCollisions` indoor branch.
- [`src/AcDream.Core/Physics/TransitionTypes.cs:1192`](../../src/AcDream.Core/Physics/TransitionTypes.cs#L1192) — `TryFindIndoorWalkablePlane` (the thing to likely delete in the next phase).
- [`src/AcDream.Core/Physics/CollisionInfo`](../../src/AcDream.Core/Physics/) — search for `ContactPlane` write sites to map who currently sets it.
- [`src/AcDream.Core/Physics/SpherePath`](../../src/AcDream.Core/Physics/) — `LastKnownContactPlane`-style fields if any exist.
**Retail decomp anchors:**
- `docs/research/named-retail/acclient_2013_pseudo_c.txt:273099` — `CTransition::step_up`.
- `docs/research/named-retail/acclient_2013_pseudo_c.txt:273137` — `CTransition::transitional_insert`.
- `docs/research/named-retail/acclient_2013_pseudo_c.txt:323565` — `BSPTREE::step_sphere_up`.
- `docs/research/named-retail/acclient_2013_pseudo_c.txt:326793` — `BSPLEAF::find_walkable` (already ported, behavior verified).
**Visual verification scenarios (re-use for the next phase):**
1. Cellar descent (the primary failing scenario)
2. 2nd-floor walking
3. Single-floor cottage (regression check — must NOT degrade)
4. Phantom collisions (cascade check — if root cause is fixed, these should improve)
**Launch command:**
```powershell
$env:ACDREAM_DAT_DIR = "$env:USERPROFILE\Documents\Asheron's Call"
$env:ACDREAM_LIVE = "1"
$env:ACDREAM_TEST_HOST = "127.0.0.1"
$env:ACDREAM_TEST_PORT = "9000"
$env:ACDREAM_TEST_USER = "testaccount"
$env:ACDREAM_TEST_PASS = "testpassword"
$env:ACDREAM_DEVTOOLS = "1"
$env:ACDREAM_PROBE_INDOOR_BSP = "1"
dotnet run --project src\AcDream.App\AcDream.App.csproj --no-build -c Debug 2>&1 | Tee-Object -FilePath "launch-next-phase.log"
```
---
## Session lessons (for future Claude)
1. **Brainstorm a hypothesis-test before a full spec.** I diagnosed the wrong root cause and built 6 commits on it. A small spike (add the probe FIRST, capture a log, look at it before designing the fix) would have surfaced the 99.9% MISS rate immediately and pointed at the deeper issue.
2. **Tangent contact is the dominant grounded case.** Any test fixture designed to exercise `walkable_hits_sphere` MUST include the tangent case (`dist == radius`), not just penetrating cases. My unit tests used Z=0.4 with radius=0.48 (overlap = 0.4 < 0.4798, passes easily) — comfortable but unrepresentative.
3. **`find_walkable` is a sweep query, not a query.** It's only meaningful when called from `step_sphere_down`. Any caller using it as "stand here, find my floor" is misusing the algorithm. Retail doesn't have such a caller because retail retains ContactPlane across frames.
4. **The +0.02f cell-origin Z-bump is a render artifact bleeding into physics.** It creates a 20mm offset between visual and physics floors. This is fine when the resolver retains state but breaks when the resolver re-computes every frame. The bump is not the root cause but it amplifies the oscillation symptom.