docs(spec): remove per-frame indoor walkable-plane synthesis (Bug A)

Slice 2 of 2 in the indoor ContactPlane retention phase. Deletes Transition.TryFindIndoorWalkablePlane + the per-frame synthesis call + outdoor-terrain fallthrough + 9 tests. Replaces with bare return TransitionState.OK; matching retail's BSPTREE::find_collisions OK path (acclient_2013_pseudo_c.txt:323938). ContactPlane is retained via the per-tick seed at PhysicsEngine.ResolveWithTransition:583 (init_contact_plane equivalent) or refreshed by BSP Path 3 / Path 4. Predecessor: de8ffde (Bug B, BSP world-origin fix). Evidence: launch-cp-probe-postfix-v2.log shows 3150 MISS / 3154 indoor-walkable calls (99.87% miss rate) after Bug B, with user-visible "stuck falling when brushing upper floor edge" symptom unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-20 08:32:17 +02:00 · 2026-05-20 08:32:17 +02:00 · 3bec18f0e4
commit 3bec18f0e4
parent de8ffde4ca
1 changed files with 370 additions and 0 deletions
--- 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
@ -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) <noreply@anthropic.com>
+```