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

fix(physics): S1B — indoor cell membership admits on the part BOX, as retail does (#335, AP-159 narrowed)
Some checks are pending
Headless portability / portable-headless (ubuntu-latest) (push) Waiting to run
Details
Headless portability / portable-headless (windows-latest) (push) Waiting to run
Details
Headless portability / linux-graphical (push) Waiting to run
Details
Headless portability / linux-vulkan (push) Waiting to run
Details

Browse source 
CellTransit.FindTransitCellsBox ports CEnvCell::find_transit_cells'
part-array overload @0x0052cae0 line-for-line: per-portal x per-part
order, the sphere cheap-reject at F_EPSILON+radius, the box admit whose
"Straddle or crossing-side" rule is exactly retail's `eax != side` under
the PDB Sidedness enum, leads-outside placed AFTER the admit, the
unconditional unloaded-neighbour hint without the sphere overload's
re-test, the destination box_intersects_cell gate with its deliberate
no-break, and add_all_outside_cells after the loop. The box-vs-cell BSP
traversal lands in BOTH representations behind the flat-authoritative
dispatcher with a graph referee whose 20,000 installed comparisons are
pinned by assertion (review F5), zero mismatch.

Dual Opus review: PASS on both lenses. The mandatory D0 pseudocode pass
caught that the contract's own supplementary note misattributed the box
block to the sphere overload — it belongs to a SECOND
check_building_transit overload @0x0052c680, whose portal-side
convention is INVERTED and whose admit differs; the pseudocode doc now
records that trap plus two byte confirmations made at review:
which_side @0x00444720 is strictly > eps for POSITIVE, and
intersect_box's in-plane early exit returns CROSSING(3)
(jp @0x005aa1bc -> mov eax,3), settling review items b1/b2 for the
future bridge porter. The bridge itself stays unported as AP-159's
explicit remainder.

The review also retired #335's severity premise honestly: "over-
inclusive only, never a missed one" is wrong at production shape ratios,
where the box (whole-vertex AABB) legitimately exceeds the sphere
(physics-polygon root sphere). Measured, both populations: rigged
(box << sphere) — 1,520 placements, 978 cells removed, 0 added;
production-ratio (box >= sphere) — 950 placements, 20 removed, 1 ADDED
through the loaded-neighbour gate, which is retail's direction, not a
defect. The no-op guard (review F4) asserts removal is nonzero so an
unwired admit cannot pass silently.

Process note: the implementer authored against this session's worktree
at bec5c69d, 25 commits stale — the recorded worktree-base class. All
six files were byte-identical between bases, the diff transplanted
losslessly, and every verdict-bearing run (referee, direction sweeps,
this clean-room) was re-executed on current main. S2's uncommitted
phase-1 edits were stashed for this landing so the suite verdicts
exactly one changeset.

Also untracks 341-slope-capture.jsonl (an accidental add) and
gitignores it.

Clean-room suite: 11,248 passed / 6 skipped / 0 failed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-07 07:46:57 +02:00
parent 1b2580be4c
commit b3e43d22c9
14 changed files with 1600 additions and 4914 deletions
Download patch file Download diff file Expand all files Collapse all files

290
docs/research/2026-08-07-ap159-pseudocode.md Normal file
View file

@ -0,0 +1,290 @@
# AP-159 / #335 — pseudocode: the INDOOR box-admit half of retail's part-array `find_transit_cells`
**Date:** 2026-08-07. Written per the mandatory grep-named → decompile → pseudocode →
port workflow, BEFORE any C# is touched, per the pinned contract
`docs/research/2026-08-07-ap159-s1b-contract.md`.
## 0. Resolving the contract's flagged overload-signature ambiguity
The contract's Oracle map says:
> **Adjacent overload** @0x0052c820 (lines 309968–310126) also contains a box
> refinement block (@0x0052c76c–0x0052c7d2). Its Binary Ninja signature
> (`CSphere const* arg4`) is inconsistent with its body indexing parts — a
> known BN artifact class.
Direct disassembly reading resolves this. The box-refinement block at
addresses **0x0052c76c–0x0052c7d2** does **not** belong to the function
declared at 0x0052c820 — its addresses are numerically *lower* than that
declaration and it is textually printed *before* it in the dump. It belongs
to the **preceding** function, which starts at line 309867:
```
0052c680 void __thiscall CEnvCell::check_building_transit(
class CEnvCell const* this, int32_t arg2, uint32_t const arg3,
class CPhysicsPart** arg4, class CELLARRAY* arg5)
```
This is a **second, part-array overload of `check_building_transit`**
(distinct from the sphere overload at 0x0052c5d0, lines 309827–309863, which
is what `CellTransit.CheckBuildingTransit` already ports). Its signature
genuinely takes `CPhysicsPart** arg4` — fully consistent with a body that
indexes parts (`arg4[eax_1]`, `arg4[var_4c_1]`). There is no BN artifact here;
the confusion was address-adjacency between two *different* functions that
happen to sit back-to-back in the binary and share a demangled base name with
their sibling overloads.
The function actually declared at **0x0052c820** (lines 309968–310122) is the
**sphere overload of `find_transit_cells`** — signature `CSphere const* arg4`
is correct, and its body is pure sphere math (`Frame::globaltolocal`,
`CCellStruct::sphere_intersects_cell`, the ±eps straddle test for exterior
portals). This is **already ported** as `CellTransit.FindTransitCellsSphere`
and needs no changes. It contains no box-refinement block at all.
The function the contract's D1/D2 actually needs — the **part-array
overload of `find_transit_cells`**, cited correctly by address
(**0x0052cae0**, lines 310127–310257) — is a **third, separate function**
from either of the above, and its disassembly matches the #335 ISSUES.md
entry (the true oracle of record) line-for-line: `Position::localtolocal`
sphere cheap-reject → `CPhysicsPart::GetBoundingBox` + `BBox::LocalToLocal` +
`Plane::intersect_box` box admit → `other_cell_id==0xFFFFFFFF` leads-outside
check → `CCellPortal::GetOtherCell` (threading `do_not_load_cells`) →
`CCellStruct::box_intersects_cell` destination gate → `add_all_outside_cells`
after the portal loop. **No contradiction with the #335 entry exists; only
the contract's supplementary "Adjacent overload" note was misattributed.**
This section performs the disentanglement the contract's mandatory D0 step
asked for.
**Consequence for D2's building-bridge decision:** the TRUE box-refinement
counterpart for the outdoor→indoor building bridge is the part-array
`check_building_transit` @0x0052c680 (the function this section just
identified), not the sphere overload @0x0052c5d0 the contract names. Its
control flow operates on a **single fixed portal per call** (index `ebp`
supplied by the caller) rather than looping `this->num_portals`, and its
cheap-reject branch structure (`if (ebp_1==1) {...} else if (ebp_1!=0)
{label:...} else {...}`) has its own three-way-looking shape that does not
cleanly decompose into the same two-branch (`PortalSide ? : `) form the
part-array `find_transit_cells` uses — porting it would mean building a new,
differently-shaped traversal, not "reusing D1's primitive with a small
diff." Per the contract's explicit D2 allowance, this is left unported and
reported as the explicit remainder (see the implementation report).
## 1. `CEnvCell::find_transit_cells` (part-array overload, @0x0052cae0, pc:310127–310257)
Verbatim shape, addresses inline:
```
find_transit_cells(this, numParts, parts[], cellArray):
exitOutside = false
for each portal in this.Portals: # do-while, pc:310140–310252
portalPlane = ResolvePlane(this, portal) # this->portals[i].portal.plane
for each part in parts: # do-while, pc:310147–310246
if part == null: continue
sphere = part.GfxObj.PhysicsSphere ?? part.GfxObj.DrawingSphere # 0x0052cb34-0052cb45
if sphere == null: continue
# --- cheap reject (sphere) --------------------------------
center = this.Pos.LocalToLocal(part.Pos, sphere.Center) # Position::localtolocal @0x0052cb5a
rad = sphere.Radius + F_EPSILON # 0x0052cb65, F_EPSILON=0.000199999995f
dist = Dot(center, portalPlane.N) + portalPlane.D # 0x0052cba4
portalSideRaw = portal.portal_side # this->portals[i].portal_side (int, retail live field)
if portalSideRaw == 1: # 0x0052cba7 — matches our PortalInfo.PortalSide == true
if dist < -rad: continue # skip this part — 0x0052cbb2/0x0052cbc8 (BN artifact
# collapsed the two-branch shape into a spurious
# 3-way if/elseif/else; the real shape is two branches
# both jumping to the SAME box-test label)
else: # portalSideRaw == 0, PortalSide == false
if dist > rad: continue
# --- box admit ---------------------------------------------
box = part.GfxObj.PhysicsBox # CPhysicsPart::GetBoundingBox @0x0050d600, 0x0052cbdd
localBox = BBox.LocalToLocal(box, part.Pos, this.Pos) # @0x005b1e60, 0x0052cbf9
sidedness = portalPlane.intersect_box(localBox) # Plane::intersect_box @0x005aa170, 0x0052cc05
crossingSide = portal.PortalSide ? Positive : Negative
if sidedness != Straddle and sidedness != crossingSide:
continue # box fully on the "still inside this cell" side — 0x0052cc0c falls through
# --- destination resolution (only when box crosses) --------
if portal.OtherCellId == 0xFFFFFFFF: # 0x0052cc21
exitOutside = true
break # out of the PART loop; proceed to next portal — 0x0052cc7e/0x0052ccaa
otherCell = CCellPortal.GetOtherCell(portal, do_not_load_cells) # 0x0052cc2b, threads cellArray+4
if otherCell == null:
cellArray.add_cell(portal.OtherCellId, null) # unconditional load hint — 0x0052cca5
break # next portal — 0x0052ccaa
destBox = BBox.LocalToLocal(box, part.Pos, otherCell.Pos) # 0x0052cc4a
if CCellStruct.box_intersects_cell(otherCell.structure, destBox): # 0x0052cc61 → BSPTREE @0x0053c880
cellArray.add_cell(otherCell.ID, otherCell)
break # next portal — 0x0052cc8d
# else: this part's box didn't actually reach the other cell's
# geometry — continue the PART loop (retest remaining parts
# against the SAME portal/destination) — 0x0052cc63, no break
if exitOutside:
CLandCell.add_all_outside_cells(numParts, parts, cellArray) # 0x0052ccea — already ported (#334)
```
Cross-checked against `docs/ISSUES.md` #335 entry (oracle of record) — every
step (1–4) and every cited address matches. Also cross-checked against the
**pre-existing** ACE-sourced pseudocode at
`docs/research/acclient_indoor_transitions_pseudocode.md` §"EnvCell.find_transit_cells
(parts/AABB variant)" (written 2026-04/05, predates named-retail), which
independently derives the identical shape from ACE's C# port
(`EnvCell.cs:245-309`) — including the exact `Positive`/`Negative` sidedness
naming this doc uses below. Two independent sources (raw PDB-paired
disassembly and an ACE C#-port cross-reference) agree; this doc treats the
agreement as confirmation, not as a new independent fact.
### Structural difference from the already-ported sphere overload
`CellTransit.FindTransitCellsSphere` special-cases exterior portals
(`OtherCellId == 0xFFFF`) with a dedicated symmetric straddle test **before**
touching the loaded/unloaded-neighbour logic (matches the sphere overload's
own `if (*ecx != 0xffffffff)` branch at the very top of its per-portal body).
The part-array overload does **not** do this: cheap-reject and box-admit run
uniformly for every portal (interior or exterior); only **after** the box
passes admit does it check `other_cell_id==0xFFFFFFFF` to decide
interior-vs-exterior handling. This is why D2 needs a genuinely new method
(`FindTransitCellsBox`) rather than a small patch to the existing
`FindTransitCellsSphere`.
## 2. `Plane::intersect_box` (@0x005aa170, pc:439037–439122)
Classifies a box against a plane using **all 8 corners**, short-circuiting
per retail's fixed enumeration order (`min`, then all 7 combinations of
{min,max}³ in the specific order the compiler emitted — order does not
affect the boolean-classification result since it is an AND of per-corner
agreement, only which corner short-circuits first). Epsilon is
`F_EPSILON = 0.000199999995f` throughout, matching
`Plane::which_side` @0x00444720 (PDB-recovered `Sidedness` enum: `Positive`
when `dist >= +eps`, an early-return "clearly negative" case the disassembly
calls out at the very first corner test, and a third `Straddle`/on-plane
case when `-eps <= dist < +eps`).
```
ClassifyBox(plane, box): # shared math for D1
corners = the box's 8 corners (any fixed enumeration; result is order-independent)
side0 = WhichSide(plane, corners[0], F_EPSILON)
if side0 == Straddle: return Straddle
for corner in corners[1..7]:
if WhichSide(plane, corner, F_EPSILON) != side0:
return Straddle
return side0 # Positive or Negative — box is uniformly on one side
WhichSide(plane, point, eps):
dist = Dot(plane.N, point) + plane.D
if dist >= eps: return Positive
if dist < -eps: return Negative
return Straddle
```
**Result contract** (matches the contract's note, re-derived independently
from the raw disassembly and cross-checked): the caller at
`find_transit_cells`'s 0x0052cc0c tests `classification != portal_side_raw`;
the caller at `check_building_transit`'s 0x0052c7a0 tests `classification ==
3 || classification == side`. Both are consistent with `ClassifyBox`
returning `Straddle` whenever corners disagree (never a bare 0/1 in that
case) and `Positive`/`Negative` only when **all 8 corners agree**. `Straddle`
is by construction never equal to a 0/1 `portal_side_raw`, so "!= portal_side"
the two callers' admit rules are NOT equivalent — dual review finding F9/R6. For the pure-side case they are OPPOSITE: `find_transit_cells` (`eax != side`) admits {Positive, Straddle} when side==1, while `check_building_transit` (`eax == 3 || eax == side`) admits {Straddle, Negative} — physically expected (reach-beyond-the-portal vs inside-this-building-cell are opposite questions), and the trap the future bridge porter must not fall into.
**PortalSide mapping** (verified two ways — direct disassembly branch
structure of the part-array `find_transit_cells`'s cheap-reject, AND the
pre-existing `acclient_indoor_transitions_pseudocode.md` §"PortalSide flag
semantics" cross-reference against ACE): retail's raw `portal_side` field is
`1` exactly when our `PortalInfo.PortalSide == true`. The admit rule,
restated without needing that raw integer at all:
```
crossingSide = portal.PortalSide ? Positive : Negative
ADMIT box-crosses-this-portal iff sidedness == Straddle OR sidedness == crossingSide
```
## 3. `BSPNODE::box_intersects_cell_bsp` (@0x0053c880, pc:325993–326087)
Structurally the box-shaped sibling of the already-ported
`BSPNODE::point_inside_cell_bsp` (:325508, `BSPQuery.PointInsideCellBsp`) and
`BSPNODE::sphere_intersects_cell_bsp` (:325546, `BSPQuery.SphereIntersectsCellBsp`):
an iterative walk down `pos_node` only, rejecting (returning "outside") the
instant the box is found to lie entirely on the **negative** side of a
splitting plane, and treating a null `pos_node` (or a leaf) as "inside."
Epsilon is again `F_EPSILON` (line 326001: `0.000199999995f`), reusing the
SAME 8-corner box-vs-plane classification as `Plane::intersect_box` — the
disassembly literally duplicates the corner-enumeration idiom.
```
BoxIntersectsCellBsp(node, boxMin, boxMax):
while node is not a leaf:
if ClassifyBox(node.SplittingPlane, boxMin, boxMax) == Negative:
return false # box entirely behind this splitting plane
if node.PosNode is null:
return true # solid interior — matches point/sphere siblings
node = node.PosNode
return true # reached a leaf
```
This is the **one new traversal shipped in two representations** per the
Slice I4/I5 house rule: `BSPQuery.BoxIntersectsCellBsp` over `CellBSPNode`
(the graph) and `FlatBspQuery.BoxIntersectsCellBsp` over
`FlatCellContainmentBsp`/`FlatCellBspNode` (the flat production shadow),
sharing the `ClassifyBox`/`WhichSide` math (added to `BSPQuery` as internal
statics, exactly the existing pattern `FlatBspQuery` already uses for
polygon math like `PolygonHitsSpherePrecise`).
`CCellStruct::box_intersects_cell` (@0x00533910, pc:317675) is a bare
tailcall into `BSPTREE::box_intersects_cell_bsp` (@0x005398b0, pc:323249),
itself a bare tailcall into the function above — no additional logic, same
shape as `sphere_intersects_cell` → `sphere_intersects_cell_bsp` already
established in this codebase.
## 4. Box transform: retail `BBox::LocalToLocal` (@0x005b1e60)
Needed at two call sites in §1 (`localBox` into `this` cell's frame,
`destBox` into `otherCell`'s frame). Same shape already ported for the
OUTDOOR extent walk's `BBox::LocalToGlobal` (`ShadowPartBox.RefitTo`,
#334): an eight-corner transform-and-refit, not a min/max-only transform (a
rotated box grows, conservatively — retail's own deliberate direction).
`LocalToLocal` differs from `LocalToGlobal` only in that the **destination**
frame carries its own rotation (a cell's `WorldTransform`/
`InverseWorldTransform`), not just a translation offset. D2 adds a sibling
`ShadowPartBox.RefitToLocal(Matrix4x4 worldToLocal, ...)` that composes the
part's world placement (`WorldPosition`/`WorldRotation`, identical to
`RefitTo`) and then applies the destination's full `Matrix4x4` transform
per corner before taking min/max — i.e. `RefitTo` is the degenerate case of
`RefitToLocal` where the destination frame has no rotation (world axes).
## 5. Deliverable mapping
| Contract deliverable | Implementation |
|---|---|
| D1 box primitives, graph | `BSPQuery.ClassifyBox`/`WhichSide` (shared), `BSPQuery.BoxIntersectsCellBsp` |
| D1 box primitives, flat | `FlatBspQuery.BoxIntersectsCellBsp` (delegates classification math to `BSPQuery`) |
| D1 dispatcher (matches existing `SphereIntersectsCell`/`PointInsideCell` shape) | `CollisionTraversal.BoxIntersectsCell` |
| D1 referee | `tests/AcDream.Core.Tests/Physics/BoxIntersectsCellBspDifferentialTests.cs` |
| D2 box transform | `ShadowPartBox.RefitToLocal` |
| D2 indoor-arm rewire | `CellTransit.FindTransitCellsBox` (new), called from `BuildShadowCellSetFromParts`'s indoor branch in place of `FindTransitCellsSphere` |
| D2 building-bridge remainder | Left unported (see §0); reported as the explicit remainder |
| D3 | `tests/AcDream.Core.Tests/Physics/CellTransitFindTransitCellsBoxTests.cs` (synthetic sabotage + inverse guard) + a direction-assertion sweep test |
---
## Byte confirmations (added at the dual review, 2026-08-07 — PDB-paired binary)
- **`Plane::which_side` @0x00444720, full decode:** `fcom dist, eps; test ah,0x41;
jnz` — POSITIVE (0) iff `dist > eps` STRICTLY; then `fcompp dist, -eps; test
ah,0x05; jnp` — NEGATIVE (1) iff `dist < -eps` strictly; IN_PLANE (2)
otherwise (unordered also lands IN_PLANE). The port's `>= eps` boundary tie
is a one-ULP divergence, noted in AP-159's landing record.
- **`Plane::intersect_box` @0x005aa170, first-corner-in-plane early exit:** the
`jp` at 0x005aa1bc targets 0x005aa2e2 = `mov eax, 3` — the early exit returns
**CROSSING (3)**, not IN_PLANE (2). Both live call sites admit 3, so the S1B
port is unaffected; the future `check_building_transit` port inherits this as
a settled fact instead of review item b2.
- `acclient.h:2527`: `Sidedness { POSITIVE=0, NEGATIVE=1, IN_PLANE=2,
CROSSING=3 }` — the header answer the PlaneSide doc previously called
unobservable. `which_side` returns at most 2; `intersect_box` is the only
producer of 3.