diff --git a/docs/superpowers/specs/2026-05-20-indoor-walkable-synthesis-removal-design.md b/docs/superpowers/specs/2026-05-20-indoor-walkable-synthesis-removal-design.md new file mode 100644 index 0000000..9de078a --- /dev/null +++ b/docs/superpowers/specs/2026-05-20-indoor-walkable-synthesis-removal-design.md @@ -0,0 +1,370 @@ +# Remove per-frame indoor walkable-plane synthesis (Bug A) + +**Date:** 2026-05-20 +**Status:** Spec — awaiting user review before plan-writing. +**Phase:** Indoor walking, ContactPlane retention investigation, **slice 2 of 2**. +**Predecessor:** Bug B (indoor BSP world-origin fix), shipped `de8ffde` 2026-05-20. +**Author:** Claude Opus 4.7 (session sad-aryabhata-2d2479). + +## Summary + +Indoor cell BSP collision is followed by a per-frame call to +`Transition.TryFindIndoorWalkablePlane` that synthesizes a ContactPlane. +The synthesis MISSES ~99.87% of the time (3150 MISS / 3154 calls in the +2026-05-20 Holtburg session) because the foot sphere is tangent to the +floor — `PolygonHitsSpherePrecise`'s `|dist| > radius − ε` test +correctly rejects tangent contact, matching retail's +`walkable_hits_sphere` semantics. The synthesis was added 2026-05-19 as +a Phase 2 stop-gap to seed a fresh CP every indoor frame. It is not a +retail behavior — retail's `BSPTREE::find_collisions` does NOT +re-synthesize the contact plane on the OK path. Instead, CP is +RETAINED across the OK path from the prior frame's seed +(`PhysicsEngine.ResolveWithTransition:583` — +`init_contact_plane` equivalent), and is REFRESHED when BSP Path 3 +(step_sphere_down) or Path 4 (land-on-surface) actually finds a +walkable polygon. With Bug B (slice 1) now shipped, those BSP-internal +CP writes are correctly world-space, so removing the synthesis leaves a +coherent ContactPlane lifecycle. + +Fix: delete the per-frame `TryFindIndoorWalkablePlane` call + outdoor +terrain fallthrough from the indoor branch of `FindEnvCollisions`. +Replace with `return TransitionState.OK;`. Then delete the unused +helper method, its constant, its probe line, and the 9 tests covering +it. + +## Problem + +### Evidence + +Post-Bug-B session capture 2026-05-20 (`launch-cp-probe-postfix-v2.log`, +56 MB / ~64k probe lines). + +- **Indoor-walkable HIT/MISS:** **4 HIT / 3150 MISS** (99.87% miss rate). +- **User-reported visual symptom:** "Walking up the stairs, if I sort + of just touch the floor on top of me I get stuck in falling + animation." The foot sphere brushes the upper-floor edge from below + → tangent contact → epsilon-rejected by `WalkableHitsSphere` → MISS + → outdoor terrain fallthrough → wrong CP plane (terrain Z, below + indoor floor by ~0.02m due to render Z-bump) → ValidateWalkable + marks player as airborne → falling animation never recovers. +- Bug B fix did NOT close this symptom because Bug B addressed + BSP-internal CP corruption, not the per-frame synthesis path. + +### Why TryFindIndoorWalkablePlane misses ~99.87% + +Phase 2 (commit `eb0f772` 2026-05-19) added the synthesis to seed a +fresh ContactPlane after every indoor BSP returned OK. Phase 3 (commit +`91b29d1` 2026-05-19) routed the synthesis through the retail-faithful +`BSPQuery.FindWalkableSphere` walker. The walker calls +`WalkableHitsSphere` → `PolygonHitsSpherePrecise`, which does: + +```csharp +float dist = Vector3.Dot(polyPlane.Normal, sphereCenter) + polyPlane.D; +float rad = sphereRadius - PhysicsGlobals.EPSILON; // ~radius - 1e-4 +if (MathF.Abs(dist) > rad) return false; +``` + +For a foot sphere tangent to the floor (`dist = radius`), +`MathF.Abs(radius) > radius − ε` evaluates true → reject. This is +correct retail behavior for `walkable_hits_sphere` (decomp +`acclient_2013_pseudo_c.txt:323010`) — the function is designed to +detect OVERLAP, not tangent contact. Retail only calls +`walkable_hits_sphere` from within a downward sphere sweep +(`step_sphere_down`), where the sphere is moving and naturally +penetrates the plane mid-sweep. A standing-grounded player is tangent +to the floor, not overlapping it; retail does NOT call +`find_walkable` for that case. + +The previous handoff +[`docs/research/2026-05-19-indoor-walkable-plane-bsp-port-shipped-handoff.md`](../../research/2026-05-19-indoor-walkable-plane-bsp-port-shipped-handoff.md) +identifies the same root cause and recommends this fix. + +## Retail behavior (from decomp study) + +Subagent study 2026-05-20 of `acclient_2013_pseudo_c.txt` confirms: + +1. **Stationary indoor player** (decomp :273640): `calc_num_steps` + returns 0 → sub-step loop SKIPPED entirely. `init_contact_plane` + (:276183, called from `get_object_info` :279984) pre-seeds + `collision_info.contact_plane{_valid,_cell_id}` from the prior + tick's `CPhysicsObj::contact_plane`. The plane round-trips back to + `CPhysicsObj::contact_plane` unchanged at tick end (:283460). +2. **Moving indoor player** (decomp :273733): sub-step loop sets + `contact_plane_valid = 0` at top of each sub-step. BSP fires; if + step-down (Path 3) or land-on-surface (Path 4) detects a polygon, + `set_contact_plane` (:271925) writes a fresh world-space plane. If + nothing detected (e.g., player is on a flat floor with no + step-down), `contact_plane_valid` stays 0 — momentarily airborne + for that sub-step — until the next sub-step's BSP query catches up. +3. **Indoor OK path** (decomp :323938): when BSP returns OK without + `find_walkable` finding anything, `contact_plane` is NOT touched. + No synthesis, no terrain fallthrough. + +Our `acdream` flow already matches retail at points 1 and 2. Point 3 +is where Bug A lives — we currently synthesize on the OK path. The fix +removes that synthesis. + +## Fix + +### Code changes — `src/AcDream.Core/Physics/TransitionTypes.cs` + +**Delete (4 sites):** + +1. **Method `Transition.TryFindIndoorWalkablePlane`** (~lines 1192-1272, + ~80 lines including doc-comment). +2. **Constant `INDOOR_WALKABLE_PROBE_DISTANCE`** (~line 1281, ~7 lines + including doc-comment). +3. **Per-frame call block in `FindEnvCollisions`** (~lines 1486-1521, + the `bool walkableHit = TryFindIndoorWalkablePlane(...)` through + the `// fallthrough to outdoor terrain` block plus the + `[indoor-walkable]` probe). +4. **Replace the deleted call block with**: + +```csharp + // Indoor BSP returned OK — no wall collision. ContactPlane + // is RETAINED from the prior tick's seed + // (PhysicsEngine.ResolveWithTransition:583, the + // init_contact_plane equivalent), OR refreshed by Path 3 + // step-down / Path 4 land if those fired this tick. Either + // way, no synthesis is needed here — matches retail's + // BSPTREE::find_collisions OK path + // (acclient_2013_pseudo_c.txt:323938). + // + // Do NOT fall through to outdoor terrain backstop: the + // player is in an indoor cell, and the outdoor terrain + // Z is below the indoor floor by ~0.02m (the render Z-bump), + // which would mark the player as airborne. Bug A + // (2026-05-20 slice 2 of indoor ContactPlane retention). + return TransitionState.OK; + } + } +``` + +The exact byte-range and surrounding text will be locked down in the +plan; the conceptual change is the OK path now returns immediately. + +### Test changes + +**Delete:** + +- `tests/AcDream.Core.Tests/Physics/IndoorWalkablePlaneTests.cs` — + entire file (291 lines, 8 tests, all calling + `Transition.TryFindIndoorWalkablePlane`). +- `tests/AcDream.Core.Tests/Physics/TransitionTypesTests.cs` — entire + file (111 lines, 1 test, calls + `Transition.TryFindIndoorWalkablePlane`). + +**Keep:** + +- `tests/AcDream.Core.Tests/Physics/BSPQueryTests.cs` — 5 existing + `FindWalkableSphere_*` tests + the Bug B regression test. + `BSPQuery.FindWalkableSphere` is the underlying API; we keep it for + any future out-of-band use (the spec for Bug B's "out of scope" + section listed spawn-placement / teleport-verification as possible + consumers — none exist yet, but the API stays). + +## Acceptance criteria + +### Probe-equivalence + +Rerun the post-fix Holtburg scenarios with +`ACDREAM_PROBE_CONTACT_PLANE=1 ACDREAM_PROBE_INDOOR_BSP=1`. Expect: + +- `[indoor-walkable]` lines: **zero** (the line is deleted). +- `Transition.ValidateWalkable` cp-write counts: drop dramatically. + Pre-fix counts were 224+146 = 370 (the 224 from the indoor synthesis + HIT path, the 146 from outdoor fallthrough). Post-fix expects ~0–10 + for the outdoor terrain calls that legitimately fire when the player + IS outdoors. +- `BSPQuery.FindCollisions:1615` (Path 4) and + `BSPQuery.StepSphereDown:1123` (Path 3): unchanged or slightly higher + (the resolver still drives Path 3/4 from the existing step-down + mechanism — no change to that path). +- `PhysicsEngine.ResolveWithTransition:583` (the per-tick seed): + unchanged. + +### Visual verification + +User drives the client through the same 5 scenarios: + +1. **Cottage entry** — should be smooth. +2. **Indoor standstill** — should be **stable** (the + stationary-player retention path is now in effect). +3. **2nd-floor walking** — should NOT get stuck in falling animation + when brushing upper floor edges (the user's reported symptom). +4. **Cellar descent** — should descend cleanly onto cellar floor. +5. **Single-floor cottage walk** — regression check (must not degrade). + +**Primary success criterion:** scenarios 2 + 3 (standstill + 2nd-floor +walking) work without falling-stuck. If 2 or 3 still glitch, the +hypothesis is wrong — investigate further. + +### M1-baseline regression check + +Walk Holtburg outdoor → enter inn → walk to NPC → click NPC → press F +on an item near the NPC. The M1 baseline ("Walkable + clickable +world") must not regress. + +## Risks + +### R1: "Flat floor, no step-down" momentary-airborne edge case + +For a moving player on a perfectly flat indoor floor with no +step-down configured AND no wall collision, Path 5 (Contact) returns +OK without writing CP. After Bug A, `ci.ContactPlane` retains the +seed value from `PhysicsEngine.ResolveWithTransition:583`, BUT the +sub-step `FindTransitionalPosition:663` zeros +`ci.ContactPlaneValid` first. If BSP doesn't re-set it, the resolver +sees CPV=false → marks airborne for that sub-step. + +**Mitigation:** retail has the same behavior (decomp study point 2 +above). The "momentary airborne" only lasts a sub-step; the next +tick's `init_contact_plane` re-seeds CPV=true from the body. Visual +verification will surface this if it's a problem in practice. + +**Fallback if R1 hits:** after `return TransitionState.OK;`, +explicitly preserve `ci.ContactPlaneValid = ci.LastKnownContactPlaneValid` +and write `ci.ContactPlane = ci.LastKnownContactPlane` if the BSP +didn't update it during the sub-step. This adds the "last-known +recovery" branch from retail's `validate_transition` (:272565), which +fires when result != OK_TS but could be extended to OK too. Out of +scope for this slice; file as follow-up if symptoms appear. + +### R2: Outdoor → indoor first-frame stale CP + +When the player walks through a door (outdoor cell → indoor cell), +the first indoor frame's seed comes from the prior outdoor terrain +plane. After Bug A, if no step-down fires that frame, `ci.CP` stays +as outdoor terrain plane (slightly below indoor floor). The player +may visually flicker airborne for one frame. + +**Mitigation:** the resolver's step-down configuration is usually +active for any vertical motion. The player walking through a door +likely has enough vertical change to trigger Path 3 → CP refreshed +to indoor floor. + +**Falsification test:** if visual verification shows a one-frame +flicker on outdoor → indoor transition, that's R2 manifesting. File +as follow-up; impact is one frame, not the indefinite stuck-falling +of Bug A. + +### R3: Spawn / teleport into indoor cell with no movement + +If the player teleports inside (e.g., admin command, recall portal) +and stands still: + +- `body.ContactPlaneValid` is reset by the teleport handler (somewhere + in `PhysicsEngine`'s teleport path — must verify). +- First tick: `PhysicsEngine.ResolveWithTransition:581` skips the seed + because `body.ContactPlaneValid` is false. `ci.CP` is default zero. +- BSP runs. With no movement, sub-step loop is SKIPPED (per retail's + `calc_num_steps == 0` path). `ci.CP` stays default zero. +- ValidateTransition end: CPV=false. body.CP stays invalid. Player + treated as airborne. +- Gravity applies. Sphere drops. Next tick: step-down fires → CP set. + +The one-tick flicker is the same as R2. Acceptable. + +**However:** if the teleport handler does NOT reset +`body.ContactPlaneValid`, the seed fires with stale data (the +pre-teleport plane). That's pre-existing behavior, unrelated to Bug +A. Out of scope. + +### R4: BSPQuery.FindWalkableSphere usage post-deletion + +After deleting `Transition.TryFindIndoorWalkablePlane`, the +`BSPQuery.FindWalkableSphere` wrapper has no callers in production +code but does have 5 unit tests. The function remains alive via +tests. + +**Decision:** keep it. The 5 tests document the contract; the +function is a faithful port of `BSPTREE::find_walkable_sphere`. If +future needs arise (e.g., spawn-placement validation when adding a +"summon to indoor cell" feature), the API is ready. If it stays +unused for a phase or two, file a cleanup follow-up. + +## Out of scope (file as follow-ups if observed) + +- **`[cp-write]` probe (committed `66de00d`).** The spike spec said + "remove when the retention fix lands." That's now (Bug A is the + retention fix). However, the probe is gated on a flag, zero-cost + when off, and remains useful for future ContactPlane debugging. + **Decision:** KEEP. The cost (8 fields → 8 properties on + CollisionInfo) is small and the value is high. File a separate + cleanup task if it ever becomes a burden. +- **Sub-step CPV=0 reset at FindTransitionalPosition:663.** Retail + also does this (decomp :273733). Not a bug. +- **Per-tick seeding at PhysicsEngine.cs:583.** Working correctly. + Not touched. +- **Path 5 not writing CP.** Retail's normal-movement + `BSPTREE::find_collisions` calls `find_walkable` internally + (:323924) and writes CP via `set_contact_plane`. Our Path 5 only + checks for walls via `SphereIntersectsPolyInternal`; it does not + call `find_walkable`. This is a pre-existing divergence that R1 + could amplify. **Defer**: if R1 manifests as a visible problem, + add `find_walkable` to Path 5's OK branch as a follow-up slice. + +## Retail anchors + +- `BSPTREE::find_collisions` — decomp `:323924` (set_contact_plane on + find_walkable hit); `:323938` (return OK without touching CP). +- `walkable_hits_sphere` — decomp `:323010`. Calls + `polygon_hits_sphere_precise` which has the same `|dist| > radius − ε` + tangent-rejection. Confirmed retail-faithful. +- `init_contact_plane` — decomp `:276183`. Our equivalent at + `PhysicsEngine.ResolveWithTransition:581-587`. +- `validate_transition` last-known recovery — decomp `:272565`. + Reference for R1's fallback design if needed. + +## Files touched + +- `src/AcDream.Core/Physics/TransitionTypes.cs` — delete ~80 lines + (method + constant + per-frame call block + probe). +- `tests/AcDream.Core.Tests/Physics/IndoorWalkablePlaneTests.cs` — + delete entire file (291 lines). +- `tests/AcDream.Core.Tests/Physics/TransitionTypesTests.cs` — delete + entire file (111 lines). + +Net delta: roughly -480 lines (no additions besides the 8-line +replacement block). + +## Commit shape + +Single commit: + +``` +fix(physics): remove per-frame indoor walkable-plane synthesis + +The indoor branch of FindEnvCollisions called Transition.TryFindIndoorWalkablePlane +every frame to re-synthesize the ContactPlane after BSP returned OK. +The synthesis routed through BSPQuery.FindWalkableSphere → walkable_hits_sphere, +which correctly rejects tangent contact via |dist| > radius − ε. For a +grounded player standing on or brushing a floor, the foot sphere is +tangent — 99.87% MISS rate per the 2026-05-20 [cp-write] probe. +Each MISS fell through to outdoor terrain backstop, writing a +ContactPlane that's below the indoor floor by ~0.02m, marking the +player airborne and triggering the falling-animation stuck symptom. + +Fix: delete the synthesis + outdoor-fallthrough from the indoor OK +path. ContactPlane is retained from the prior tick's seed +(PhysicsEngine.ResolveWithTransition:583, init_contact_plane equivalent) +or refreshed by BSP Path 3 / Path 4 during the same tick. Matches +retail's BSPTREE::find_collisions OK path +(acclient_2013_pseudo_c.txt:323938). + +Also deletes: +- Transition.TryFindIndoorWalkablePlane (~80 lines) +- INDOOR_WALKABLE_PROBE_DISTANCE +- [indoor-walkable] probe log line +- IndoorWalkablePlaneTests.cs (8 tests, the helper's coverage) +- TransitionTypesTests.cs (1 test, also tested the helper) + +Net: -480 lines. BSPQuery.FindWalkableSphere + its 5 tests retained +as the underlying retail-faithful walkable-finder API. + +Closes Bug A in the indoor ContactPlane retention phase. +Spec: docs/superpowers/specs/2026-05-20-indoor-walkable-synthesis-removal-design.md. +Predecessor: de8ffde (Bug B, BSP world-origin fix). + +Co-Authored-By: Claude Opus 4.7 (1M context) +```