b3e43d22c9
Some checks are pending
Headless portability / portable-headless (ubuntu-latest) (push) Waiting to run
Headless portability / portable-headless (windows-latest) (push) Waiting to run
Headless portability / linux-graphical (push) Waiting to run
Headless portability / linux-vulkan (push) Waiting to run
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>
290 lines
17 KiB
Markdown
290 lines
17 KiB
Markdown
# 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.
|