docs(spec): indoor BSP world-origin / world-rotation fix (Bug B)
Single-call-site defect in TransitionTypes.cs:1442 — the indoor cell BSP query invokes BSPQuery.FindCollisions without passing the cell's world rotation or world origin. Path 3 step-down + Path 4 land write ContactPlanes with D ≈ 0 instead of the cell's world floor Z. 320 corrupt CP writes per Holtburg session per the [cp-write] probe capture 2026-05-20. Fix: decompose cellPhysics.WorldTransform once, pass rotation + translation. Mirrors the existing correct pattern at :1808 (object BSP via FindObjCollisions). This is slice 1 of 2 for the indoor ContactPlane retention phase. Slice 2 (Bug A — TryFindIndoorWalkablePlane removal) deferred pending Bug B retest. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik
parent
66de00d09a
commit
865634f450
1 changed files with 379 additions and 0 deletions
|
|
@ -0,0 +1,379 @@
|
|||
# Indoor BSP world-origin / world-rotation fix (Bug B)
|
||||
|
||||
**Date:** 2026-05-20
|
||||
**Status:** Spec — awaiting user review before plan-writing.
|
||||
**Phase:** Indoor walking, ContactPlane retention investigation, slice 1 of 2.
|
||||
**Author:** Claude Opus 4.7 (session sad-aryabhata-2d2479).
|
||||
|
||||
## Summary
|
||||
|
||||
The indoor cell BSP collision call at `TransitionTypes.cs:1442` invokes
|
||||
`BSPQuery.FindCollisions` without passing the cell's world rotation or
|
||||
world origin. The parameters default to `Quaternion.Identity` and
|
||||
`Vector3.Zero`. Inside `BSPQuery`, the per-poly world-space plane built
|
||||
by `TransformVertices` + `BuildWorldPlane` for `StepSphereDown` (Path 3)
|
||||
and Path 4 land-on-surface uses those defaults, so for any rotated or
|
||||
translated indoor cell the produced plane is in cell-local space, not
|
||||
world space. The resulting `ContactPlane` is written with the cell's
|
||||
local floor-Z as its `D` component (often ≈ 0) instead of the world
|
||||
floor-Z (e.g. -94.02 for Holtburg cottages). The bug fires every
|
||||
step-down inside an indoor cell during movement.
|
||||
|
||||
Fix: decompose `cellPhysics.WorldTransform` once, pass the rotation as
|
||||
`localToWorld` and the translation as `worldOrigin` to
|
||||
`BSPQuery.FindCollisions`. Mirrors the existing correct pattern at
|
||||
`TransitionTypes.cs:1808` for object BSP queries.
|
||||
|
||||
## Problem
|
||||
|
||||
### Evidence
|
||||
|
||||
Session capture 2026-05-20, `launch-cp-probe.log` (51k lines, 39k
|
||||
`[cp-write]` entries), Holtburg-area walking scenarios. Caller
|
||||
distribution of `[cp-write]` lines:
|
||||
|
||||
```
|
||||
37470 caller=PhysicsEngine.ResolveWithTransition:583 ← per-tick seed
|
||||
456 caller=BSPQuery.FindCollisions:1615 ← Path 4 land
|
||||
320 caller=BSPQuery.StepSphereDown:1123 ← Path 3 step-down
|
||||
251 caller=Transition.FindTransitionalPosition:663 ← sub-step reset
|
||||
224 caller=Transition.ValidateWalkable:1629 ← outdoor / synth
|
||||
146 caller=Transition.ValidateWalkable:1606
|
||||
76 caller=Transition.TransitionalInsert:853
|
||||
76 caller=Transition.TransitionalInsert:815
|
||||
```
|
||||
|
||||
Smoking-gun excerpt from an indoor walking sequence:
|
||||
|
||||
```
|
||||
... ContactPlane: n=(0,0,1) D=-94.020 ← correct floor (from prior tick)
|
||||
[indoor-bsp] cell=0xA9B40150 wpos=(132.628,16.893,94.500) result=OK
|
||||
[indoor-walkable] cell=0xA9B40150 wpos=(132.628,16.893,94.500) result=MISS
|
||||
[cp-write] ContactPlaneValid: True -> False caller=Transition.FindTransitionalPosition:663
|
||||
[indoor-bsp] cell=0xA9B40150 wpos=(132.628,16.893,94.500) result=OK
|
||||
[indoor-walkable] cell=0xA9B40150 wpos=(132.628,16.893,94.500) result=MISS
|
||||
[cp-write] ContactPlaneValid: False -> True caller=BSPQuery.StepSphereDown:1123
|
||||
[cp-write] ContactPlane: n=(0,0,1) D=-94.020 -> n=(0,0,1) D=-0.000 caller=BSPQuery.StepSphereDown:1123
|
||||
[cp-write] LastKnownContactPlane: n=(0,0,1) D=-94.020 -> n=(0,0,1) D=-0.000 caller=BSPQuery.StepSphereDown:1123
|
||||
[indoor-bsp] cell=0xA9B40150 wpos=(132.628,16.893,93.750) result=Adjusted poly=n/a
|
||||
```
|
||||
|
||||
Before the step-down, `ContactPlane.D = -94.020` (the Holtburg cottage
|
||||
floor at world Z = 94.02). After `StepSphereDown:1123` writes,
|
||||
`ContactPlane.D = -0.000` (the world Z=0 plane). The plane's `Normal` is
|
||||
unchanged at `(0, 0, 1)` because the cell rotation is identity-ish along
|
||||
its Z axis for this particular cell — but the `D` component
|
||||
(`-Dot(Normal, worldVertices[0])`) is wrong by the cell's full world-Z
|
||||
offset.
|
||||
|
||||
The corrupted plane writes back to `body.ContactPlane` via
|
||||
`PhysicsEngine.ResolveWithTransition:625`. The next tick's seed at
|
||||
`PhysicsEngine.ResolveWithTransition:583` propagates the corruption:
|
||||
|
||||
```
|
||||
[cp-write] ContactPlane: 0.000 -> n=(0,0,1) D=-0.000 caller=PhysicsEngine.ResolveWithTransition:583
|
||||
```
|
||||
|
||||
320 corrupt step-down writes per session.
|
||||
|
||||
### Why this hadn't surfaced before
|
||||
|
||||
The Phase 2 indoor-walkable synthesis (`TryFindIndoorWalkablePlane`,
|
||||
shipped 2026-05-19) applies `cellPhysics.WorldTransform` manually after
|
||||
the BSP returns (TransitionTypes.cs:1260-1268). So the 6 HIT cases out
|
||||
of 616 indoor BSP OK results wrote a *correct* plane via the synthesis
|
||||
path; the 610 MISS cases fell through to outdoor terrain (also writes
|
||||
a "correct" but wrong-Z plane). The 320 BSP-internal `StepSphereDown`
|
||||
writes are independent of the synthesis path — they fire from inside
|
||||
`BSPQuery.FindCollisions` during Path 3 step-down, where the
|
||||
caller-supplied `localToWorld` + `worldOrigin` are used directly to
|
||||
build the plane. That code path was never inspected during the
|
||||
Phase-2 work.
|
||||
|
||||
## Root cause
|
||||
|
||||
`BSPQuery.FindCollisions` signature (BSPQuery.cs:1505-1516):
|
||||
|
||||
```csharp
|
||||
public static TransitionState FindCollisions(
|
||||
PhysicsBSPNode? root,
|
||||
Dictionary<ushort, ResolvedPolygon> resolved,
|
||||
Transition transition,
|
||||
DatReaderWriter.Types.Sphere localSphere,
|
||||
DatReaderWriter.Types.Sphere? localSphere1,
|
||||
Vector3 localCurrCenter,
|
||||
Vector3 localSpaceZ,
|
||||
float scale,
|
||||
Quaternion localToWorld = default,
|
||||
PhysicsEngine? engine = null,
|
||||
Vector3 worldOrigin = default)
|
||||
```
|
||||
|
||||
`localToWorld` defaults to `Quaternion.Identity` (treated by the body
|
||||
at line 1520) and `worldOrigin` defaults to `Vector3.Zero`. Both are
|
||||
used inside the BSP traversal to convert local-space hit polygons into
|
||||
world-space planes:
|
||||
|
||||
```csharp
|
||||
// BSPQuery.cs:1120-1123 (StepSphereDown, Path 3)
|
||||
var worldNormal = TransformNormal(polyHit.Plane.Normal, localToWorld);
|
||||
var worldVertices = TransformVertices(polyHit.Vertices, localToWorld, scale, worldOrigin);
|
||||
var worldPlane = BuildWorldPlane(worldNormal, worldVertices);
|
||||
collisions.SetContactPlane(worldPlane, path.CheckCellId, false);
|
||||
```
|
||||
|
||||
```csharp
|
||||
// BSPQuery.cs:439-449 (TransformVertices)
|
||||
private static Vector3[] TransformVertices(
|
||||
ReadOnlySpan<Vector3> vertices,
|
||||
Quaternion localToWorld,
|
||||
float scale,
|
||||
Vector3 worldOrigin)
|
||||
{
|
||||
var result = new Vector3[vertices.Length];
|
||||
for (int i = 0; i < vertices.Length; i++)
|
||||
result[i] = Vector3.Transform(vertices[i] * scale, localToWorld) + worldOrigin;
|
||||
return result;
|
||||
}
|
||||
```
|
||||
|
||||
```csharp
|
||||
// BSPQuery.cs:451-456 (BuildWorldPlane)
|
||||
private static Plane BuildWorldPlane(Vector3 worldNormal, ReadOnlySpan<Vector3> worldVertices)
|
||||
{
|
||||
float d = worldVertices.Length > 0
|
||||
? -Vector3.Dot(worldNormal, worldVertices[0])
|
||||
: 0f;
|
||||
return new Plane(worldNormal, d);
|
||||
}
|
||||
```
|
||||
|
||||
With `localToWorld = Identity` and `worldOrigin = Zero`, `worldVertices[i] == localVertices[i]`. For a cell-local floor poly at local Z ≈ 0, `worldPlane.D ≈ 0` instead of the cell's actual world floor Z.
|
||||
|
||||
## Indoor BSP call sites
|
||||
|
||||
Grep for `BSPQuery.FindCollisions\s*\(` in `src/AcDream.Core/Physics/`
|
||||
returns two callers:
|
||||
|
||||
| Line | Call site | localToWorld | worldOrigin |
|
||||
|---|---|---|---|
|
||||
| `TransitionTypes.cs:1442` | Indoor cell BSP | `Quaternion.Identity` | (not passed → `Vector3.Zero`) |
|
||||
| `TransitionTypes.cs:1808` | Object BSP (`FindObjCollisions`) | `obj.Rotation` | `obj.Position` |
|
||||
|
||||
Only `:1442` is broken. `:1808` correctly passes the object's
|
||||
rotation + position because objects always have non-trivial world
|
||||
transforms.
|
||||
|
||||
## Fix
|
||||
|
||||
Single change at `TransitionTypes.cs:1442`. Decompose
|
||||
`cellPhysics.WorldTransform` to extract the rotation quaternion and
|
||||
translation vector, then pass them to `FindCollisions`. Pattern
|
||||
mirrors `:1808`:
|
||||
|
||||
```csharp
|
||||
// New: decompose the cell's world transform.
|
||||
if (!Matrix4x4.Decompose(
|
||||
cellPhysics.WorldTransform,
|
||||
out _, // scale ignored — see Risks
|
||||
out var cellRotation,
|
||||
out var cellOrigin))
|
||||
{
|
||||
// Decompose returned false: matrix has shear or non-uniform scale.
|
||||
// Log + fall through with safe defaults (current broken behavior).
|
||||
// Should never happen for valid EnvCell data.
|
||||
Console.WriteLine(System.FormattableString.Invariant(
|
||||
$"[indoor-bsp] WARN cellPhysics.WorldTransform did not decompose cleanly for cell 0x{sp.CheckCellId:X8}"));
|
||||
cellRotation = Quaternion.Identity;
|
||||
cellOrigin = cellPhysics.WorldTransform.Translation;
|
||||
}
|
||||
|
||||
// Existing FindCollisions call, updated args.
|
||||
var cellState = BSPQuery.FindCollisions(
|
||||
cellPhysics.BSP.Root,
|
||||
cellPhysics.Resolved,
|
||||
this,
|
||||
localSphere,
|
||||
localSphere1,
|
||||
localCurrCenter,
|
||||
Vector3.UnitZ,
|
||||
scale: 1.0f,
|
||||
localToWorld: cellRotation,
|
||||
engine: engine,
|
||||
worldOrigin: cellOrigin);
|
||||
```
|
||||
|
||||
## Verification
|
||||
|
||||
### Pre-fix probe data (baseline)
|
||||
|
||||
- `BSPQuery.StepSphereDown:1123` cp-writes: 320, all with `D` close to
|
||||
the cell's local floor Z (≈ 0) instead of world Z (≈ -94.02 for
|
||||
Holtburg).
|
||||
- `[indoor-bsp]` lines: 616 OK, 230 Adjusted, 31 Slid, 8 Collide.
|
||||
|
||||
### Post-fix expected probe data
|
||||
|
||||
Rerun the same scenarios (cottage entry, indoor standstill, cellar
|
||||
descent, 2nd-floor walk) with `ACDREAM_PROBE_CONTACT_PLANE=1
|
||||
ACDREAM_PROBE_INDOOR_BSP=1`. Expect:
|
||||
|
||||
- `BSPQuery.StepSphereDown:1123` cp-writes: still ~320 but with `D`
|
||||
matching the world floor Z of the cell where the step-down fired.
|
||||
Sample expected line:
|
||||
`[cp-write] ContactPlane: n=(0,0,1) D=-94.020 -> n=(0,0,1) D=-94.020 caller=BSPQuery.StepSphereDown:1123`
|
||||
— i.e., the write would be a no-op (suppressed by the property-
|
||||
setter's value-equality guard) because the new write matches the
|
||||
prior tick's seed value. In practice the write count drops sharply
|
||||
because the value-equality suppression hides duplicate writes.
|
||||
- `[indoor-bsp]` Adjusted/OK distribution should stay similar; Slid /
|
||||
Collide should change only at the margin (the corrupt CP previously
|
||||
may have triggered spurious wall-collide downstream).
|
||||
- Visual: 2nd-floor walking should stop intermittent falling-stuck on
|
||||
random spots (the BSP-internal step-down now writes correct planes
|
||||
during movement). Stationary case (cellar entry, cottage standstill)
|
||||
may still glitch — that's Bug A, out of scope for this slice.
|
||||
|
||||
### Unit test
|
||||
|
||||
`tests/AcDream.Core.Tests/Physics/BSPQueryTests.cs` — add
|
||||
`StepSphereDown_RotatedTranslatedCell_WritesWorldSpacePlane`:
|
||||
|
||||
1. Fixture: a single-poly BSP at cell-local Z = 0.5, single floor
|
||||
triangle at cell-local Z = 0.0.
|
||||
2. Cell `WorldTransform`: rotation = `Quaternion.CreateFromAxisAngle(Vector3.UnitZ, MathF.PI / 4)` (45° around world Z), translation = `(100, 50, 94)`.
|
||||
3. Sphere at cell-local (0, 0, 0.5), radius 0.48.
|
||||
4. Call `BSPQuery.FindCollisions` with the decomposed rotation +
|
||||
translation.
|
||||
5. Assert: returned `TransitionState.Adjusted`, and
|
||||
`transition.CollisionInfo.ContactPlane.D == -94.0` within 1e-4
|
||||
tolerance (= world Z of the floor poly after applying the cell's
|
||||
translation).
|
||||
|
||||
### Visual verification scenarios
|
||||
|
||||
Re-use from prior phase:
|
||||
|
||||
1. **Cottage entry** (outdoor → indoor) — `[cell-transit]` line shows
|
||||
transition; no immediate falling-stuck.
|
||||
2. **Indoor standstill** in a single-floor Holtburg cottage — stable
|
||||
grounding for 10 seconds. *Stationary case — may still glitch.*
|
||||
3. **2nd-floor walking** — climb stairs, walk around upstairs.
|
||||
*Primary success criterion for Bug B alone.*
|
||||
4. **Cellar descent** — walk down stairs to cellar. *Stationary glitch
|
||||
may persist; movement should be smoother.*
|
||||
5. **Single-floor cottage regression check** — confirm M1 baseline is
|
||||
not regressed.
|
||||
|
||||
## Risks
|
||||
|
||||
### R1: Cell scale
|
||||
|
||||
`Matrix4x4.Decompose` returns the cell's scale as the first out-param.
|
||||
This spec uses the hardcoded `scale: 1.0f` regardless. If any indoor
|
||||
cell ships with a non-unit scale (extremely unlikely — never observed
|
||||
in the AC dat), the produced world plane will be wrong.
|
||||
|
||||
Mitigation: low-confidence assumption, leave for now. If a probed run
|
||||
ever shows a cell with scale ≠ 1, add a Console.WriteLine warning at
|
||||
the decompose site and file a follow-up issue.
|
||||
|
||||
### R2: Decompose returns false
|
||||
|
||||
`Matrix4x4.Decompose` returns false if the matrix has shear or other
|
||||
non-decomposable parts. EnvCell `WorldTransform` is always a rigid
|
||||
rotation + translation per the dat format. If decompose fails, we log
|
||||
a warning and fall back to `Quaternion.Identity` + the matrix's
|
||||
`.Translation` — better than `Vector3.Zero` but still wrong if cell
|
||||
is rotated.
|
||||
|
||||
### R3: BSP traversal still uses `localSphere` (local space)
|
||||
|
||||
The fix changes the OUTPUT transformation (local poly → world plane)
|
||||
but the INPUT to BSP (`localSphere`) is already in cell-local space.
|
||||
That's correct and matches retail: the BSP traversal itself works in
|
||||
local coordinates, but the `set_contact_plane` write must produce a
|
||||
world-space plane (the equivalent retail call is `Plane::localtoglobal`
|
||||
at acclient_2013_pseudo_c.txt:323708 / :323921). The fix aligns our
|
||||
behavior with retail's.
|
||||
|
||||
### R4: Bug A still in play
|
||||
|
||||
After Bug B fix, `TryFindIndoorWalkablePlane` still runs every indoor
|
||||
OK frame, still 99% MISS, still falls through to outdoor terrain
|
||||
backstop and writes the wrong plane for the stationary case. The
|
||||
M2-blocker scenarios may not be fully fixed by Bug B alone. Bug A is a
|
||||
separate phase (next slice).
|
||||
|
||||
## Out of scope (file as follow-ups if observed)
|
||||
|
||||
- **Bug A (per-frame synthesis):** Remove `TryFindIndoorWalkablePlane`
|
||||
+ outdoor-terrain fallthrough from the indoor OK branch. Separate
|
||||
phase after Bug B retest. ISSUES #83 stays OPEN until that ships.
|
||||
- **Sub-step `contact_plane_valid = 0` reset at FindTransitionalPosition:663:**
|
||||
Retail also does this (decomp :273733). Not a bug. The 251 writes
|
||||
per session are expected.
|
||||
- **Per-tick seeding at PhysicsEngine.cs:583:** Working correctly. Not
|
||||
touched in this slice. The 37,470 writes are the
|
||||
`SetContactPlane(plane, cellId, isWater)` helper firing 4-6 sub-field
|
||||
writes per call × ~6-9k resolve calls.
|
||||
- **`AcDream.Core.Physics.Transition.FindObjCollisions` (line 1808):**
|
||||
Already correct. Not touched.
|
||||
- **Bug B in other callers:** Grep confirmed only `:1442` and `:1808`
|
||||
call `BSPQuery.FindCollisions`. The 1850 `FindCollisions` overload
|
||||
is a different signature (takes raw polygons, not resolved).
|
||||
Out-of-scope for this slice; visit if probe data ever surfaces a
|
||||
symptom traceable to it.
|
||||
|
||||
## Retail anchors
|
||||
|
||||
- `BSPTREE::find_collisions` — decomp `acclient_2013_pseudo_c.txt:323924`.
|
||||
Inside the BSP traversal, calls `COLLISIONINFO::set_contact_plane`
|
||||
with a world-space plane (after `Plane::localtoglobal` at :323921).
|
||||
- `BSPTREE::step_sphere_down` — decomp `:323565`. Same pattern:
|
||||
`localtoglobal` (at :323708) before `set_contact_plane`.
|
||||
- `Plane::localtoglobal` — applies the cell's world transform to the
|
||||
plane before storing. Our `TransformNormal` + `TransformVertices` +
|
||||
`BuildWorldPlane` chain is the equivalent.
|
||||
|
||||
## Files touched
|
||||
|
||||
- `src/AcDream.Core/Physics/TransitionTypes.cs` — one call site at
|
||||
`:1442`, ~10 new lines for the Decompose + arg pass.
|
||||
- `tests/AcDream.Core.Tests/Physics/BSPQueryTests.cs` — one new test
|
||||
`StepSphereDown_RotatedTranslatedCell_WritesWorldSpacePlane`,
|
||||
~30 lines including fixture setup.
|
||||
|
||||
No other code touched. The `[cp-write]` probe stays in place (still
|
||||
useful for retest verification + future Bug A work).
|
||||
|
||||
## Commit shape
|
||||
|
||||
Single commit:
|
||||
|
||||
```
|
||||
fix(physics): pass cell world-transform to indoor BSP collision
|
||||
|
||||
Indoor cell BSP queries at TransitionTypes.cs:1442 were calling
|
||||
BSPQuery.FindCollisions with Quaternion.Identity + defaulted
|
||||
Vector3.Zero worldOrigin. Inside the BSP, Path 3 (step_sphere_down)
|
||||
and Path 4 (land-on-surface) use those params to build the
|
||||
world-space ContactPlane. Result: planes written with D ≈ 0 instead
|
||||
of the cell's world floor Z (e.g. -94.02 for Holtburg cottages).
|
||||
320 corrupt CP writes per Holtburg session.
|
||||
|
||||
Fix: decompose cellPhysics.WorldTransform once at the call site,
|
||||
pass the rotation as localToWorld and the translation as
|
||||
worldOrigin. Mirrors the existing correct pattern at :1808
|
||||
(FindObjCollisions, passes obj.Rotation + obj.Position).
|
||||
|
||||
Retail oracle: BSPTREE::find_collisions (acclient_2013_pseudo_c.txt:323924)
|
||||
calls Plane::localtoglobal at :323921 before set_contact_plane.
|
||||
Our TransformNormal + TransformVertices + BuildWorldPlane chain is
|
||||
the equivalent — it just needs the right rotation + origin.
|
||||
|
||||
Spec: docs/superpowers/specs/2026-05-20-indoor-bsp-worldorigin-fix-design.md.
|
||||
Evidence: launch-cp-probe.log capture 2026-05-20, [cp-write] probe.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||||
```
|
||||
Loading…
Add table
Add a link
Reference in a new issue