fix(physics): AP-156 — flood the BSP sphere where the geometry is, not at the part origin

The AP-152 retail review (docs/research/2026-08-06-ap152-review-retail.md)
FAILED `4abd1b5e` and is right. `ShadowObjectRegistry.BuildFloodSpheres` took
each physics-BSP part's ROOT BOUNDING SPHERE RADIUS
(FlatCollisionAssetBuilder.cs:393 -> LiveEntityCollisionBuilder.cs:137) and
centred it on the PART ORIGIN (ShadowShapeBuilder.cs:194), discarding the root
sphere's own Origin.

Re-measured independently against the installed client_portal.dat, reproducing
the reviewer's numbers exactly: 376 of 973 physics-BSP parts have
|origin| > radius/2, worst 20.762 m on a 27.708 m sphere (gfx 0x010036DD,
Setup 0x0200129A). Over the 172 Setups AP-152 moved onto that path the emitted
flood FAILED TO CONTAIN the object's own BSP sphere for 170 of them (73
CylSphere-bearing, 97 Sphere-bearing), worst shortfall 9.911 m on Setup
0x02000255 — whose one part's sphere sits 9.911 m above the part origin — and
for 43 the post-AP-152 flood was strictly SMALLER than the pre-AP-152 one.
Indoor flooding is 3-D (CellTransit.cs:601 routes every id & 0xFFFF >= 0x0100
candidate through FindTransitCellsSphere), so a tall prop or door slab was
absent from EnvCells it physically occupies and therefore never a broadphase
candidate there (TransitionTypes.cs:3763 iterates only entries already in the
cell). That is the #98 / #168 class AP-152 exists to remove.

Retail, re-disassembled from the PDB-paired binary (check_exe_pdb.py MATCH,
CodeView GUID 9e847e2f-777c-4bd9-886c-22256bb87f32), every address resolved
back through named-retail/symbols.json:

  CGfxObj::physics_sphere is [gfxobj+0x74] (physics_bsp is [+0x78], as
  CPartArray::CacheHasPhysicsBSP @0x00518110 reads at 0x00518127), and
  acclient pseudo-C 0x00534b5b assigns it BSPTREE::GetSphere(physics_bsp).

  BSPTREE::GetSphere @0x005397e0
    8b01        mov eax,[ecx]     ; BSPTREE::root_node
    83c004      add eax,4         ; past BSPNODE::vfptr -> CSphere sphere
  So retail's per-part flood sphere IS the BSP root bounding sphere,
  ORIGIN INCLUDED (acclient.h: BSPNODE { vfptr; CSphere sphere; ... },
  CSphere { Vector3 center; float radius; } -> radius at +0xc).

  CPhysicsObj::find_bbox_cell_list @0x00510fc0 adds the object's own cell and
  then walks the PART ARRAY: 0x00511012 call 0x518160
  (CPartArray::calc_cross_cells_static), which dispatches [edx+0x7c] with
  (num_parts, parts, cellarray). Its EnvCell body,
  CEnvCell::find_transit_cells @0x0052cae0:
    0x0052cb31  mov edx,[eax+0x20]   ; CPhysicsPart::gfxobj (CGfxObj**)
    0x0052cb36  mov esi,[ecx+0x74]   ; physics_sphere (else +0x90 drawing)
    0x0052cb4c  add eax,0x30         ; CPhysicsPart::pos
    0x0052cb5a  call Position::localtolocal   ; transform the sphere CENTRE
    0x0052cb65  fadd [esi+0xc]       ; only NOW the radius
  Retail transforms the centre through the part's own Position before it ever
  touches the radius. Carrying the radius alone is not an approximation of
  that; it is a different sphere.

Changes:

* `ShadowShape` gains `BoundsCenter` — the bounding sphere's centre in the
  shape's own local frame, scaled like LocalPosition and Radius. Zero for
  Cylinder/Sphere shapes, whose LocalPosition already IS their centre.

* `ShadowShapeBuilder.FromSetup` gains a `physicsBspBounds` resolver that
  supplies radius AND centre from ONE call, replacing the placeholder radius
  plus a downstream substitution. `LiveEntityCollisionBuilder` now holds a
  single `Func<uint, FlatCollisionSphere?>` and derives its dispatch predicate
  from it, so the gate and the geometry cannot disagree and the radius cannot
  be taken while the origin is dropped. That split is what produced this bug;
  it no longer exists.

* `FromLandblockBspParts` carries the centre too. A landblock-baked part array
  is the same CPartArray walk, so stair runs, fences and rock clusters had the
  identical defect. Both storage forms (flat BSP and the graph fallback) are
  covered.

* `BuildFloodSpheres` places each sphere at
  partWorldPos + rotate(BoundsCenter, partWorldRot), composed exactly as the
  ShadowEntry rows are.

* The 10-sphere clamp now applies to the CYLSPHERE branch only. Retail's clamp
  is inside CObjCell::find_cell_list @0x0052b9f0
  (0x0052ba21 cmp eax,0xa / 0x0052ba28 mov ebp,0xa); the BSP walk has none and
  the sorting-sphere overload @0x0052b990 takes one sphere. 7 installed Setups
  carry more than 10 physics-BSP parts (max 49, Setup 0x02001A91) and their
  tail parts were dropped from the flood entirely. Without this the new
  containment assertion would have covered shapes production never floods
  from.

Register. AP-155 was two divergences with different code paths, populations
and gates under one id; it is NARROWED to its static-publication half and its
flood half is split out as AP-156 WITH ITS DIRECTION CORRECTED. AP-155(b)
recorded the approximation as over-inclusive — "floods MORE cells rather than
fewer, the safe direction for membership" — and that false direction was the
stated reason the residual was safe to defer. It was under-inclusive for 170
of 172. AP-156 records the correction, this fix, and the one genuine residual:
acdream's sphere-vs-portal traversal where retail walks each part's sphere
against the cell's own portal planes. AP-155(b)'s "acdream approximates
retail's bounding BOX" was wrong too — find_bbox_cell_list forms no box.
AP-157 filed for the review's F4: retail's third branch floods from ONE
CPartArray::GetSortingSphere @0x00518b00 ([partArray+0x54]+0x70 =
CSetup::sorting_sphere; 4,154 of 5,935 installed Setups carry a non-zero one)
where acdream floods from every Sphere shape, and acdream's cylinder flood
ignores CylHeight. Deliberately NOT bundled here: different branch, disjoint
population, different live gate. Active AP rows 107 -> 109, literal count.

Tests. Both flood tests the review named substituted a CONCENTRIC Radius = 14f
at LocalPosition = Zero — the one configuration in which the defect cannot
appear. Every fixture is now off-centre by default, and
`FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint` drives the production
`physicsBspBounds` seam instead of hand-substituting. Five new facts: the
flood centres on BoundsCenter not the part origin; it rotates BoundsCenter by
the part rotation; it caps cylspheres at ten but never the BSP parts; the
landblock path carries the scaled centre in both storage forms; and an
installed-DAT containment sweep asserting every emitted BSP flood sphere
contains that part's real bounding sphere at entity scale 1.75, behind four
external controls — 973 parts, 376 off-centre, 172 affected, and 170
would-fail-if-the-origin-were-discarded, the last of which fails if the
population ever stops exercising the field.

Nine sabotages, each reverted and re-verified:
  A drop BoundsCenter from the flood       -> 3 Core
  B rotate by entity rot, not part rot     -> 1 Core (the rotation fact only)
  C FromSetup discards the origin          -> 1 Core + 2 App + 1 Content
     (the shipped defect, now caught in three projects)
  D drop entScale on BoundsCenter          -> 2 App + 1 Content
  E landblock flat branch drops the centre -> 1 Core
  F landblock graph branch drops it        -> 1 Core
  G drop partScale on the landblock centre -> 1 Core
  H re-apply the 10-cap to every branch    -> 1 Core
  I remove the cylsphere cap               -> 1 Core
AP-152's own two sabotages re-run against this tree: the step-0 gate disabled
still reddens exactly its five facts with Headless 89/89 green, and
cylinder-first flooding still reddens exactly one.

Clean Release build after deleting all 44 bin/obj: 0 errors, 21 pre-existing
warnings. Complete suite 11,208 passed / 4 skipped / 0 failed, +5 on the
11,203 baseline at 4abd1b5e — Core 4264 -> 4268, Content 126 -> 127, App
unchanged (one rename, not an addition). No new skips.

NOT yet gated live. This moves shadow-cell membership for real objects, in
both directions, and the connected session must look for both: props and doors
that START blocking from a neighbouring cell (the 73 CylSphere+BSP Setups),
AND ones that STOP blocking (the 99 Sphere+BSP Setups can shrink; 43 shrink
below their pre-4abd1b5e size, which is the regression this fixes). Tall
indoor props and door slabs — the ones whose sphere sits metres above the part
origin — are where the change is largest.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-06 15:52:04 +02:00
parent 4abd1b5eb7
commit b52967def3
13 changed files with 1424 additions and 90 deletions

File diff suppressed because one or more lines are too long

View file

@ -0,0 +1,370 @@
# AP-152 architecture review — `4abd1b5e`
**Reviewer scope:** blast radius, correctness of reach, test quality. Retail
fidelity is a separate reviewer's.
**Worktree:** `.claude/worktrees/resume-session-e0bd03e1-d5bf45`, branch
`claude/resume-session-e0bd03e1-d5bf45`, HEAD `4abd1b5e`.
**Method:** read-only, inline, no subagents. All `bin`/`obj` deleted (44
directories) before every verdict-deciding build. Five sabotages reproduced;
tree restored and verified clean (`git status --porcelain` empty) after each.
---
## Verdict
**PASS**, with one high-severity latent risk that must not be treated as
landed behaviour until the connected flood-set diff runs, and four
documentation/framing residuals.
No defect found. The two load-bearing claims I was asked to attack — the
headless-neutrality claim and the S4 no-op argument — **both hold**, and I
verified each independently rather than accepting the implementer's evidence.
---
## Gates reproduced
| Gate | Claimed | Measured | |
|---|---|---|---|
| Clean Release build | 0 errors / 21 pre-existing warnings | 0 errors / 21 warnings | ✅ |
| Full solution suite | 11,203 / 4 / 0 | 11,203 passed / 4 skipped / 0 failed | ✅ |
| Headless | 89 / 89 | 89 / 89 | ✅ |
| Delta vs `ec29a732` | +5 (3 Core, 1 App, 1 Content) | +5 `[Fact]` added, **0 removed**; the 5 new facts are exactly the 5 that redden under sabotage B | ✅ |
| Renames 1:1 | 2 renamed, none deleted | `-0 [Fact]` in the test diff | ✅ |
| Register bookkeeping | AP-152 retired, AP-153/154/155 filed | active AP rows 105 → 107 (1 +3) | ✅ |
Per-project at HEAD, clean build, `--no-build`:
Cli 4, Bake 15, Headless 89, Content 126, UI.Abstractions 546, Core.Net 764,
Runtime 1222, App 4173/3 skip, Core 4264/1 skip. **Σ 11,203 / 4 / 0.**
I did not check out `ec29a732` to re-measure the 11,198 baseline (read-only
worktree, and switching would have disturbed the other agent's assumptions).
The +5 arithmetic is corroborated independently: sabotage B (gate disabled)
reddens exactly five tests solution-wide, and they are exactly the five the
commit says it added.
---
## 1. Reach across hosts — **the neutrality claim is TRUE, and stronger than stated**
I did not verify the claim as written; I enumerated the whole producer set.
`ShadowShape` is constructed at exactly **eight** sites in `src/`:
| Site | Emits |
|---|---|
| `ShadowShapeBuilder.cs:134` (step 1) | Cylinder |
| `ShadowShapeBuilder.cs:155` (step 2) | Sphere |
| `ShadowShapeBuilder.cs:192` (step 3) | BSP |
| `ShadowShapeBuilder.cs:276` (`FromLandblockBspParts`) | BSP only — every path in that loop `continue`s or adds `ShadowCollisionType.BSP` |
| `LandblockPhysicsContentBuilder.cs:658`, `:683` | Cylinder only (`:683` converts a Setup Sphere to a height-capped Cylinder) |
| `LandblockPhysicsPublisher.cs:1003`, `:1030` | Cylinder only (same conversion) |
Steps 13 are the only site that could ever produce a heterogeneous list, and
after this change it cannot. So:
- **Content's two registrations are homogeneous by construction.**
`LandblockPhysicsContentBuilder.cs:619` passes `bspShapes` from
`FromLandblockBspParts` (all-BSP); `:702` passes `setupShapes`, all
`ShadowCollisionType.Cylinder`, with no `ShadowCollisionType.Sphere`
reachable. Confirmed by reading both loops, not by trusting the comment.
- **Every other `RegisterMultiPart` caller replays a stored list.**
`ShadowObjectRegistry.cs:536` (`ReplaceMultiPartPayload`), `:741`
(`UpdatePosition`), `:1616` (`RefloodOwnerForLandblock`), `:2199` (mirror)
all pass `_entityShapes[entityId]`, which was populated by one of the eight
producers above. Homogeneity propagates.
- **Runtime constructs no shapes.** `grep "new ShadowShape("` in
`src/AcDream.Runtime` returns nothing; every `ShadowObjects.*` call there is
`UpdatePosition` / `CommitSetPosition` / `Suspend` / a read.
- **Project references confirm the reach boundary.**
`AcDream.Headless.csproj``AcDream.Runtime` only;
`AcDream.Runtime.csproj` → Core, Core.Net, Content, Plugin.Abstractions.
`AcDream.App` is unreachable, so `LiveEntityCollisionBuilder` — the sole
production caller of `FromSetup` (`LiveEntityCollisionBuilder.cs:126`) —
cannot execute in headless.
**Empirical confirmation, not just structural:** under sabotage B (the step-0
gate disabled — emission reverts to additive), `AcDream.Headless.Tests` stays
**89/89 green** while Core, App and Content each redden. That is direct
evidence the change does not reach headless, which is the C5b lesson applied
in the correct direction.
**I believe the headless-neutrality claim.**
---
## 2. Effective-GfxObj identity — **correct, and structurally airtight**
The gate (`ShadowShapeBuilder.cs:116-124`) and step 3 (`:171-200`) call the
same helper `EffectivePartGfxObjId` (`:289-302`), over the same index range
(`setup.Parts.Count`), with the same predicate instance (`hasPhysicsBsp`).
Therefore
> `anyPhysicsBspPart == true` ⟺ step 3 emits at least one shape
is an identity, not a tested property. The trap the contract names — gate
suppresses primitives, step 3 emits nothing, `Build` returns null at
`LiveEntityCollisionBuilder.cs:146`, collision silently deleted — is
unreachable by construction.
Downstream identity is the same one: `LiveEntityCollisionBuilder.cs:137`
resolves the real BSP radius from `shape.GfxObjId`, which step 3 set to the
effective id at `ShadowShapeBuilder.cs:193`. The predicate itself
(`LiveEntityCollisionBuilder.cs:56`,
`physicsData.GetFlatGfxObj(id)?.PhysicsBsp.RootIndex >= 0`) is the same
function object passed to the gate. Gate, emission, radius resolution and the
collision-time BSP lookup all key on one id.
**Trap-1 sabotage reproduced.** Replacing the gate body with
`hasPhysicsBsp((uint)setup.Parts[i])`:
```
Failed ShadowShapeBuilderTests.FromSetup_DispatchGateReadsTheEffectivePartIdentities
Failed! - Failed: 1, Passed: 4263, Skipped: 1, Total: 4265
```
Exactly one test catches it, and it is the one claimed to. Its discriminating
power is real: with a `setup.Parts` gate the swapped case yields
Cylinder + BSP (2 shapes) and `Assert.Single(swapped)` fails, while the
unswapped case still passes — so the test distinguishes *this* wrong gate
from *no* gate.
---
## 3. Test quality — five sabotages reproduced
### S4 (flood reverted to cylinder-first) — **the no-op argument holds**
I reverted `ShadowObjectRegistry.cs:649-652` to cylinder-first and ran the
**whole solution**:
```
Failed ShadowObjectRegistryMultiPartTests.BuildFloodSpheres_BspBearingOwner_FloodsFromBspNotFromCylinder
Failed! - Failed: 1, Passed: 4263 ... (Core)
Passed! - all 8 other test assemblies, including Headless 89/89
```
`FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint` — the end-to-end fact —
stayed **green**, exactly as claimed. One test out of 11,203 reddens.
Combined with §1's producer enumeration, this is a proof rather than an
anecdote: no production producer can emit a list on which the flood dispatch
observably differs, so **the entire measured membership delta is attributable
to the emission gate alone**, and the flood half is behaviour-identical today.
**Is the flood change justified?** Yes, and it is not a workaround. It is a
faithful port of `calc_cross_cells`' own dispatch (documented at
`ShadowObjectRegistry.cs:600-628`), kept on the same rationale that keeps
`Transition.BspOnlyDispatch`. It is, however, honestly dead against current
inputs — see latent risk #5. The gate alone would produce identical behaviour;
the flood change buys retail-shape correctness for a producer that does not
yet exist.
### S5 (old `ScaleFactor` body under the same production sabotage) — **reproduced**
Production sabotage: `ShadowShapeBuilder.cs:161`,
`Radius: sph.Radius * entScale``Radius: sph.Radius`.
- Corrected test body → **Failed**
(`FromSetup_ScaleFactor_MultipliesAllRadiiAndOffsets`).
- Old test body restored verbatim under the *same* sabotage → **Passed**.
The old test genuinely covered nothing: its radius/offset assertions sat
inside `if (s.CollisionType == ShadowCollisionType.Cylinder)` on a fixture
with zero CylSpheres. The correction is a strengthening, not a rewrite to
make a failing test pass.
### Sabotage B (step-0 gate disabled → additive emission)
Reddens exactly the five added facts, across three projects:
```
Core FromSetup_DoorSetup_EmitsBspPartsOnly
Core FromSetup_DispatchGateReadsTheEffectivePartIdentities
Core FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint
App CylSphereAndPhysicsBspPart_EmitsOnlyTheScaledBspShape
Content InstalledSetups_WithBothAPrimitiveAndAPhysicsBspPart_EmitOnlyBspShapes
Headless 89/89 GREEN
```
### DAT-sweep control (mine, not on the claimed list)
The installed-DAT sweep completes in ~700 ms, which is fast enough to look
like a vacuous early return at
`InstalledSetupBspPrimitiveDispatchTests.cs:59-60`. I falsified that:
`ExpectedSetups = 5935 → 5936` produces
`Assert.Equal() Failure: Expected 5936, Actual 5935`. The sweep really
enumerates the installed `client_portal.dat`, and its four external bucket
controls are load-bearing rather than derived from the predicate under test.
### Test-quality notes
- No test deleted (`-0 [Fact]` in the diff); both renamed tests are
strengthened (`Assert.Single` + `Assert.All` + two `DoesNotContain` where
there were loose counters).
- The `return` on absent DATs is the established Content.Tests convention
(`ContentConformanceDats.ResolveDatDir`, used identically by six sibling
tests). Not a new skip.
- No `Skip =`, no `try/catch`, no `Thread.Sleep`/`Task.Delay`, no new
`GetEnvironmentVariable`, no suppression flag introduced anywhere in the
diff.
- Known flakes #302/#308/#321 untouched and not conflated.
---
## 4. Downstream consumers of the (now smaller) shape list — clean
- No production site indexes a `FromSetup` list positionally. The only
positional loops (`LandblockPhysicsPublisher.cs:1083`, `:1098`) walk their
own homogeneous list.
- No site asserts "at least one primitive". `RegisterMultiPart` handles
`shapes.Count == 0` by deregistering (`ShadowObjectRegistry.cs:454`), and
§2 proves the count cannot newly become zero.
- `TransitionTypes.cs:3759 / 3901 / 4089` branch per shape kind, never on the
presence of a kind.
- `WorldSceneDiagnosticsController.cs:222` is debug wireframe drawing.
- `LiveEntityCollisionBuilder.cs:191-201`'s probe counts `cyl` vs `else`; the
contract already documents that its `else` mislabels Sphere as `bsp`. That
matters for reading the un-run connected gate, not for behaviour.
---
## Findings, ranked
### Defect
None.
### Latent risk
**LR-1 [High] — for 99 of the 172 Setups the membership change is a SHRINK, in
the same failure class the commit exists to fix, and it is un-gated.**
`ShadowShapeBuilder.cs:150-164` stops emitting the Setup Sphere for any
Sphere+BSP Setup, so `BuildFloodSpheres` (`ShadowObjectRegistry.cs:654-667`)
now floods only from the BSP parts' bounding spheres — whose production radius
is the flat BSP root bounding sphere (`LiveEntityCollisionBuilder.cs:137`).
Retail's `find_bbox_cell_list` uses a bounding **box** over the whole part
array; acdream approximates it with per-part bounding **spheres**, which
AP-155 correctly registers. Failure scenario: a Sphere+BSP prop whose Setup
sphere is larger than every part's BSP bounding sphere — the contract itself
names `0x02001761` "Boulder" (2.00 m sphere + 1 BSP) as a candidate — loses
shadow cells, and stops blocking when approached from the landcell it dropped
out of while still blocking from another. That is the #98/#168 signature
verbatim. The instrument for this is the contract's §8.2 item 3 keyed
flood-set diff, and it **has not been run**. The commit's "NOT yet gated live"
line covers it; this finding is to make sure the shrink direction, not only
the flip direction, is what the connected session measures, and that the
acceptance criterion "no affected owner ends with an empty set" is checked.
**LR-2 [Medium] — the blast radius is understated: this also changes the
collision shape set, not only membership, wherever the wire flag is absent.**
`Transition.BspOnlyDispatch` (`TransitionTypes.cs:1348-1349`) reads the
*server's* `PhysicsState`, copied at `LiveEntityCollisionBuilder.cs:161`. The
commit's "the collision half was already inert" is conditional on ACE setting
`HAS_PHYSICS_BSP_PS` from `CSetup.HasPhysicsBSP` for every affected Setup. Any
live entity where that bit is absent previously had its primitive tested and
now does not. Failure scenario: an ACE build (or a different server) that
omits the bit for one of the 172 — pre-change the mover collided with the
cylinder, post-change it collides only with the slab BSP, and the two are not
the same shape. This is retail-*correct* (retail derives the flag from the
parts, `CPartArray::CacheHasPhysicsBSP`), and the underlying dependency is
registered as AP-154, so nothing is hidden — but the commit message and the
"NOT yet gated live" note both say "membership", and the honest statement is
"membership, and collision wherever the server omits the bit".
**LR-3 [Medium] — `RegisterMultiPart`'s own XML doc still states the rule this
commit inverted.** `src/AcDream.Core/Physics/ShadowObjectRegistry.cs:432-439`:
"when the object has CylSpheres, they alone drive the flood … otherwise the
BSP parts' bounding spheres stand in for the sorting sphere." That is now
false, and it sits on the **public** method, 165 lines above the corrected
`BuildFloodSpheres` block at `:600-628`. A reader who stops at the public
API doc gets the pre-change rule. This is the same class of defect the commit
was written to correct in AP-152's four false statements, and CLAUDE.md's
"never leave them out of sync" applies.
**LR-4 [Low] — `LiveEntityCollisionBuilder`'s class doc still describes the
additive policy.** `src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs:32`:
"CylSpheres before Spheres, **and** every physics-BSP part." The "and" is
precisely the union this commit removed.
**LR-5 [Low] — the exclusivity invariant is whole-program, not locally
enforced, and the flood guard's only live proof is synthetic.** Nothing in
`AcDream.Core` prevents a future Content/Runtime producer from handing
`RegisterMultiPart` a mixed list; the BSP-first flood branch is the guard, and
S4 shows it is exercised by exactly one synthetic test. That is acceptable
(it is a faithful port, and it is documented as forward insurance), but it
should be understood as inert code with a synthetic-only witness rather than
as covered behaviour.
**LR-6 [Low] — "All eight sabotages run and reported" has no artifact.**
`docs/research/2026-08-06-ap152-contract.md` is the pre-implementation
contract; no closeout records sabotage outcomes. Five hold under independent
reproduction here; the other three rest on the commit message alone.
**LR-7 [Informational] — headless still has no live-entity collision at all
(#330), so graphical and headless now dispatch shapes by different rules for
the same world.** Pre-existing and correctly bounded by the commit ("do not
fix #330 here"), but it is now load-bearing: the fix improves the graphical
host only.
### Style
Covered by LR-3 / LR-4 (both are stale docs on live symbols, so I ranked them
as latent risk rather than style).
---
## AP-153 / AP-154 / AP-155 — honest residuals, not deferral
- **AP-153** (retail caches the dispatch flag once at
`InitPartArrayObject+0x7e`; acdream's gate is live). A modelling difference
the fix itself introduces, unreachable against ACE today because humanoid
part swaps involve no physics-BSP GfxObj on either side. Filing it rather
than building stale-flag state is the right call — modelling it would be
speculative machinery for an unreachable case.
- **AP-154** (wire-derived `HAS_PHYSICS_BSP_PS`). **It does not undercut the
gate's premise.** The gate is now derived from the parts, which is exactly
what `CPartArray::CacheHasPhysicsBSP` does; AP-154 is about the *query-time*
guard `BspOnlyDispatch`, which the commit deliberately leaves alone. What it
does do is name the assumption the "already inert" argument rests on — which
is why LR-2 is a framing problem rather than a hidden one. Filing it is
correct; it should not have been resolved inside this commit.
- **AP-155** (static paths emit Setup Spheres as height-capped Cylinders; the
flood approximates retail's bbox with bounding spheres). Its membership half
is genuinely closed here; its static half is explicitly left open. The
second clause is the mechanism behind LR-1, and the row states it plainly
rather than burying it.
None of the three defers a part of the defect this commit set out to fix.
---
## Rules compliance
No workaround, suppression flag, grace period, retry loop, or symptom guard.
No `if (problematicState) return` at a symptom site — the gate is at the
producer, derived from the same data retail derives it from. No new skips
(4 skipped, unchanged). No test weakened to pass; both corrected tests are
strictly stronger and neither was deleted. Register updated in the same
commit, with count arithmetic checked (105 → 107).
---
## What I checked, so the PASS is auditable
1. Full diff of both production files and all five test files.
2. Every `ShadowShape` producer in `src/` (8 sites) and every
`RegisterMultiPart` caller (6 production sites) read individually.
3. `.csproj` reference graph for Headless / Runtime / Content / Core.
4. `grep` for `new ShadowShape(` and `ShadowObjects.*` in `AcDream.Runtime`
(empty / read-only).
5. Downstream consumers keyed on `ShadowCollisionType.Cylinder|Sphere`, on
`shapes[0]`, and on `shapes.Count`.
6. Clean Release build after deleting all 44 `bin`/`obj` dirs: 0 / 21.
7. Full solution suite twice on clean builds: 11,203 / 4 / 0 both times.
8. Five sabotages reproduced (trap-1 gate identity, S4 flood revert, S5
scale-factor pair, B gate-disabled, DAT-sweep count control), each with the
tree restored and `git status --porcelain` verified empty afterwards.
9. Register row count at HEAD vs `ec29a732`.
10. Diff scanned for `Skip =`, `try/catch`, sleeps, env-var reads, TODO/HACK.
Final tree state: **clean**, no modifications left behind. My single write is
this file.

View file

@ -0,0 +1,416 @@
# AP-152 retail-conformance review — commit `4abd1b5e`
**Reviewer role:** adversarial retail-conformance.
**Scope:** `ShadowShapeBuilder.FromSetup` step-0 dispatch gate,
`ShadowObjectRegistry.BuildFloodSpheres` priority, the AP-152 retirement text,
and the AP-153/AP-154/AP-155 filings.
**Method:** every retail claim re-derived from the PDB-paired binary
(`C:\Users\erikn\Downloads\acclient.exe`, `check_exe_pdb.py``=== MATCH ===`,
linker 2013-09-06T00:17:56Z, CodeView GUID
`9e847e2f-777c-4bd9-886c-22256bb87f32`) with capstone + `pefile`, resolving every
address through `named-retail/symbols.json`. Neither the commit body's nor the
contract's quoted disassembly was used as input. Population facts re-measured
independently against the installed `client_portal.dat` with a scratchpad
`Chorizite.DatReaderWriter` sweep (not the repo's test).
---
## VERDICT: **FAIL**
The retail port itself is faithful — **every** disassembly claim in the commit
and in the AP-152 retirement text checks out byte-exact, and the change makes
acdream match `CPhysicsObj::FindObjCollisions` and `CPhysicsObj::calc_cross_cells`
where it previously did not. The failure is not in the dispatch port. It is that
the commit **moves 172 Setups onto a flood-sphere approximation whose direction
the register records backwards**, and the false direction is the stated reason
the residual was safe to defer. Measured over the installed DATs: the new flood
fails to contain the object's own BSP bounding sphere for **170 of the 172**
affected Setups, shortfall up to **9.911 m**, and for **43 of them** the flood is
strictly *smaller* than what the code produced before this commit. That is the
#98 / #168 under-inclusive membership class — the exact class the commit's
thesis says it removes.
---
## Part 1 — Independent retail verification (all PASS)
### 1.1 `CPhysicsObj::calc_cross_cells` @`0x00515230`
Symbol resolves exactly (`symbols.json``CPhysicsObj::calc_cross_cells`,
offset 0). Disassembled:
```
0x00515285 f786a800000000000100 test dword ptr [esi + 0xa8], 0x10000
0x0051528f 7574 jne 0x515305 ; -> BSP
0x00515291 8b4e10 mov ecx, [esi + 0x10] ; part array
0x00515296 7444 je 0x5152dc ; none -> sorting sphere
0x00515298 e8e32d0000 call 0x518080 ; CPartArray::GetNumCylsphere
0x0051529f 743b je 0x5152dc ; zero -> sorting sphere
...
0x005152d1 e81a670100 call 0x52b9f0 ; CObjCell::find_cell_list (cylsphere)
0x005152da eb35 jmp 0x515311 ; PAST the sorting-sphere branch
0x005152dc ...
0x005152e3 e818380000 call 0x518b00 ; CPartArray::GetSortingSphere
0x005152fb e890660100 call 0x52b990 ; CObjCell::find_cell_list (sorting sphere)
0x00515305 680c3f8400 push 0x843f0c
0x0051530c e8afbcffff call 0x510fc0 ; CPhysicsObj::find_bbox_cell_list
```
- The `test`/`jne` pair is at exactly `0x00515285` / `0x0051528f` as claimed. **PASS.**
- `jne` target `0x515305` calls `0x00510fc0` = `CPhysicsObj::find_bbox_cell_list`. **PASS.**
- Cylsphere call at `0x005152d1``0x0052b9f0`, sorting-sphere call at
`0x005152fb``0x0052b990`; both **below** the jump and unreachable from it. **PASS.**
- **Order (review item 2):** confirmed by fall-through, not merely by "BSP wins".
Both `je 0x5152dc` guards (null part array; `GetNumCylsphere == 0`) skip to the
sorting-sphere branch, and the cylsphere branch's `jmp 0x515311` at `0x005152da`
jumps **past** the sorting-sphere branch. Precedence is therefore
**BSP → CylSphere → sorting sphere**, exclusive at every step. **PASS.**
`0x0052b9f0` and `0x0052b990` are two overloads of the same symbol
`CObjCell::find_cell_list`; the commit distinguishes them correctly by argument
shape. Retail's 10-sphere cap is confirmed independently inside the cylsphere
overload at `0x0052ba21 cmp eax, 0xa`, matching `RetailSphereCap = 10`.
### 1.2 `CPhysicsObj::FindObjCollisions` @`0x0050f050` (review item 3)
Left untouched by the commit; re-confirmed so "already correct" is earned:
```
0x0050f165 f786a800000000000100 test dword ptr [esi + 0xa8], 0x10000
0x0050f16f 7431 je 0x50f1a2 ; clear -> primitive dispatch
0x0050f18d e8ee8f0000 call 0x518180 ; CPartArray::FindObjCollisions
0x0050f19d e90e010000 jmp 0x50f2b0 ; UNCONDITIONAL
0x0050f1a2 ... ; CylSphere loop head
0x0050f1d6 0f833b010000 jae 0x50f317 ; loop exhausted -> RETURN
0x0050f21d ... ; Sphere loop head
0x0050f22f 0f84e6000000 je 0x50f31b ; zero Spheres -> RETURN seeded OK_TS
```
All five cited instruction addresses are byte-exact and the `jmp 0x50f2b0` is
past both `0x50f1a2` and `0x50f21d`. The dispatch is exclusive. **PASS** — the
query path genuinely did not need changing. (`ebp` at `0x0050f171`/`0x0050f1b2`/
`0x0050f235` is the ethereal/ignore early-out, not a second shape branch.)
### 1.3 `CacheHasPhysicsBSP` and AP-153's "exactly one caller" (review item 4)
Two distinct symbols, both cited correctly in different places:
`CPhysicsObj::CacheHasPhysicsBSP` @`0x0050f570` and
`CPartArray::CacheHasPhysicsBSP` @`0x00518110`.
Full `.text` + `.rdata` scan for `E8`/`E9` rel32 and absolute-dword references:
| Target | Refs found |
|---|---|
| `CPhysicsObj::CacheHasPhysicsBSP` `0x0050f570` | **1**`call` at `0x0051272e` = `CPhysicsObj::InitPartArrayObject+0x7e` |
| `CPartArray::CacheHasPhysicsBSP` `0x00518110` | **1**`call` at `0x0050f57d` = `CPhysicsObj::CacheHasPhysicsBSP+0xd` |
**PASS**, and stronger than filed: `InitPartArrayObject` @`0x005126b0` itself has
exactly three callers, all construction — `CPhysicsObj::InitNullObject+0x1f`,
`CPhysicsObj::makeObject+0x3b`, `CBuildingObj::makeBuilding+0x3b`. And
`CPartArray::SetPart` @`0x00518580` (the `AnimPartChanged` swap site) calls
`CPhysicsPart::SetPart` @`0x0050e700` per part and never re-caches. So retail's
dispatch flag is derived once at construction and is genuinely stale after a part
swap. **AP-153 is honestly scoped** and could legitimately claim more evidence
than it does.
`CPartArray::CacheHasPhysicsBSP`'s body confirms the derivation the acdream gate
imitates: walk `[ecx+0x5c][i]` parts, deref `[part+0x20]` → gfxobj, test
`[gfxobj+0x78]` (physics BSP), OR `0x10000` into `[ecx]` on the first hit.
### 1.4 Every cited address resolves to the symbol claimed (review item 5)
| Address | Resolves to | Verdict |
|---|---|---|
| `0x00515230` | `CPhysicsObj::calc_cross_cells` | ✔ |
| `0x0050f050` | `CPhysicsObj::FindObjCollisions` | ✔ |
| `0x00510fc0` | `CPhysicsObj::find_bbox_cell_list` | ✔ |
| `0x0052b9f0` / `0x0052b990` | `CObjCell::find_cell_list` (two overloads) | ✔ |
| `0x0050f570` | `CPhysicsObj::CacheHasPhysicsBSP` | ✔ |
| `0x00518110` | `CPartArray::CacheHasPhysicsBSP` | ✔ |
| `0x00518180` | `CPartArray::FindObjCollisions` | ✔ |
| `0x0050d8d0` | `CPhysicsPart::find_obj_collisions` | ✔ |
| `0x00537a80` / `0x00537fd0` | `CSphere::intersects_sphere` (two overloads) | ✔ |
| `0x00518b00` | `CPartArray::GetSortingSphere` | ✔ |
| `0x0051272e` | `CPhysicsObj::InitPartArrayObject+0x7e` | ✔ |
| `0x00518060/70/80/90` | `GetNumSphere` / `GetSphere` / `GetNumCylsphere` / `GetCylsphere` | ✔ |
No mis-citation found.
### 1.5 The four corrections (all re-measured from the installed DAT)
| Correction | Claim | Measured | Verdict |
|---|---|---|---|
| 2a | max affected primitive = **6.714 m** CylSphere on `0x02001741` | `0x02001741` cyl[0] r = **6.714**, h = 1.476 — the max over the 172 affected | ✔ |
| 2b | `0x0200086E` Sphere origin `(0.759, 0.165, 5.842)` | sph[0] origin `(0.758796, 0.165414, 5.842)`, r = 5.842 | ✔ |
| 3 | cottage door `0x020019FF` Sphere = **0.100 m**; `0.141` is `Setup.Radius` | sph[0] r = **0.1** @ `(0, 0, 0.018)`; `Setup.Radius` = **0.14142136** | ✔ |
| 4 | **two** pinning tests, both corrected, neither deleted | `FromSetup_DoorSetup_ProducesFourShapes``_EmitsBspPartsOnly`, and `FromSetup_DoorSetup_SphereAtExpectedLocalOffset` re-hosted `_ => true``_ => false` | ✔ |
Note the correction is *scoped to the affected 172*, which is legitimate but
unstated: the largest primitive anywhere in the DAT is a **15.0 m** Sphere on
`0x02000D7D`. Worth one clause in the row so a later reader doesn't re-derive
6.714 as a global bound.
### 1.6 Register bookkeeping
- **AP-152 retirement is earned, not asserted.** `~~AP-152~~` struck through,
past tense, evidence column populated, corrections enumerated. Sabotage-verified
both halves (below).
- **AP row count = literal 107.** Parsed the section: 130 AP rows, 23 struck, **107
active**. Baseline at `ec29a732` = 105 active. 105 + 3 (AP-153/154/155) 1
(AP-152) = **107**. Reconciles.
- **AP-154's grep claim is exact.** `grep -rn "PhysicsStateFlags.HasPhysicsBsp" src/`
→ exactly two hits: `TransitionTypes.cs:1349` (the predicate) and
`PhysicsEngine.cs:1614`, which reads `request.MoverPhysicsState` — an unrelated
*mover*-state read, as the row says.
- **ACE derivation confirmed.** `references/ACE/Source/ACE.Server/WorldObjects/WorldObject_Networking.cs:666-669`
`if (CSetup.HasPhysicsBSP) physicsState |= PhysicsState.HasPhysicsBSP; else &= ~`.
- **Static-publisher homogeneity claim earned.** `LandblockPhysicsPublisher.cs:983`
gates the Setup-primitive block on `entityBspCount == 0`, so that path already
dispatched exclusively and emits Cylinder-typed shapes only.
- **Blast-radius claim earned.** `ShadowShapeBuilder.FromSetup`'s only production
caller is `src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs:126`
(`internal sealed`, AcDream.App) — unreachable from Headless.
### 1.7 Build and suite
`bin`/`obj` deleted repo-wide, then Release build (0 errors) and full suite:
```
AcDream.Cli.Tests 4 / 0 skip
AcDream.Content.Tests 126 / 0
AcDream.UI.Abstractions.Tests 546 / 0
AcDream.Runtime.Tests 1222 / 0
AcDream.Bake.Tests 15 / 0
AcDream.Headless.Tests 89 / 0
AcDream.App.Tests 4173 / 3 skip
AcDream.Core.Net.Tests 764 / 0
AcDream.Core.Tests 4264 / 1 skip
--------------------------------------------
11,203 passed / 4 skipped / 0 failed
```
Exactly the commit's stated numbers, including Headless 89/89.
### 1.8 Sabotage (both restored; tree confirmed clean afterwards)
| Sabotage | Reddened |
|---|---|
| A — `if (!anyPhysicsBspPart)``\|\| true` (restore the additive union) | `FromSetup_DoorSetup_EmitsBspPartsOnly`, `FromSetup_DispatchGateReadsTheEffectivePartIdentities`, `FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint` — 3 failed / 20 passed |
| B — `BuildFloodSpheres` `only` → cylinder-first (drop the BSP arm) | `BuildFloodSpheres_BspBearingOwner_FloodsFromBspNotFromCylinder` only — 1 failed / 9 passed |
Both facts are load-bearing and neither is over-broad. Sabotage B leaving
`FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint` green is *correct* and
corroborates the commit's own statement that the `BuildFloodSpheres` half is
behaviour-neutral against today's producers.
---
## Part 2 — Findings, by severity
### F1 (HIGH) — AP-155(b)'s "over-inclusive, the safe direction" is empirically inverted, and it is the justification for deferring the residual
**Register text (AP-155, Risk column):** *"(b)'s bounding-sphere approximation is
over-inclusive (a sphere contains the box's inscribed extent but is larger in the
diagonal), which floods MORE cells rather than fewer — the safe direction for
membership."*
That reasoning holds only if the sphere is **concentric** with the geometry. It
is not. Production builds the BSP flood sphere from two different sources:
- **centre**`src/AcDream.Core/Physics/ShadowShapeBuilder.cs:194`:
`LocalPosition = partFrame.Origin * entScale`, the part's *placement-frame*
origin;
- **radius**`src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs:137`
`_physicsBspRadius``src/AcDream.Core/Physics/FlatCollisionAssetBuilder.cs:393`,
`PhysicsBSP.Root.BoundingSphere.**Radius**`.
The BSP root sphere's own **`Origin` is discarded**. `BuildFloodSpheres`
(`src/AcDream.Core/Physics/ShadowObjectRegistry.cs:661`) then floods from
`entityWorldPos + rotate(LocalPosition)` — the part origin — with that radius.
**Measured over the installed `client_portal.dat`** (5,935 Setups; 530 carry a
physics-BSP part; 973 BSP parts total):
- **376 of 973** BSP parts have `|rootSphere.Origin| > radius/2`. Worst:
**20.762 m** offset on a 27.708 m sphere (`gfx 0x010036DD`, Setup `0x0200129A`).
- Over the **172 Setups this commit moves onto that path**, the emitted flood
sphere set fails to contain the true BSP bounding sphere in **170** cases
(73 CylSphere-bearing, 97 Sphere-bearing). Only 2 are covered.
- Worst shortfall **9.911 m**, Setup `0x02000255`: one part, `gfx 0x01000A90`,
BSP root sphere `origin = (0.000, 0.007, 9.911)`, `radius = 10.522`
(`Setup.Height = 18.692`). Production floods from a sphere centred at
`(0,0,0)` — 9.9 m below the geometry's own collision centre.
**Observable in-game consequence.** Outdoor land flood (`AddAllOutsideCells`) is
XY-driven and mostly forgives this. Indoor / building flood is not:
`CellTransit.BuildShadowCellSet` (`src/AcDream.Core/Physics/CellTransit.cs:601`)
routes every candidate cell with `id & 0xFFFF >= 0x0100` through
`FindTransitCellsSphere`, a 3-D sphere-vs-portal test. A tall dungeon or building
prop whose BSP sphere sits several metres above the part origin will not be
registered into the EnvCells it physically occupies. It is then never a broadphase
candidate there at all (`TransitionTypes.cs:3763` only iterates entries already in
the cell), so it does not collide: **walk through the upper part of a tall indoor
prop, or through a door slab from the storey above/below it**. That is precisely
the #98 / #168 class.
The register does not merely omit this — it records the opposite and uses the
recorded direction as the reason the residual is safe to leave open. Under the
C4-handoff process finding *"a contract asserting a mechanism that does not exist
caused three separate defects"*, this is the same failure mode, in a register row
rather than a code contract.
Two secondary inaccuracies in the same row's retail characterisation:
- *"acdream approximates retail's bounding BOX"*`find_bbox_cell_list`
@`0x00510fc0` adds the object's own cell and then calls
`CPartArray::calc_cross_cells_static` @`0x00518160`, which dispatches the
virtual at `[cell_vtbl+0x7c]` with `(numParts, parts, cellarray)` — a
`find_transit_cells` part-array overload (`CObjCell::find_transit_cells`
`0x0052b070`/`0x0052b080`; `CEnvCell`'s pair sits at `.rdata` `0x007c8d14`/
`0x007c8d18`). Retail walks the **actual per-part geometry** through cell
portals despite the function's name. The approximation is coarser than the row
admits, which widens F1 rather than narrowing it.
### F2 (HIGH) — 43 of the 172 affected Setups get a strictly *smaller* flood than before this commit
A direct consequence of F1, but it needs stating separately because it
contradicts the commit's own thesis and is unguarded by any test.
Modelling the pre-`4abd1b5e` `BuildFloodSpheres` (primitives preferred when any
Cylinder exists; otherwise everything, cap 10) against the post-commit set over
the same 172 Setups:
| | count |
|---|---|
| new flood ⊇ old flood | 129 |
| **new flood ⊉ old flood** | **43** (22 CylSphere-bearing, 21 Sphere-bearing) |
Worst: **3.493 m** on `0x0200086E``Setup.Height = 11.684`, whose Sphere at
`(0.759, 0.165, 5.842) r 5.842` reached `z ≈ 11.68`, while the two surviving BSP
flood spheres (`gfx 0x01001B2B` r 9.015, `gfx 0x01001BB2` r 9.254, both centred at
their part origins) reach only `z ≈ 9.0`. Next: `0x020015D4` 2.405 m,
`0x02000359` 2.083 m, `0x02001761` 1.889 m.
The commit says the change removes an under-inclusive membership defect. For
these 43 Setups it introduces one. The commit's "NOT yet gated live" note is the
right instinct; the connected gate must specifically look for props and doors
that stopped blocking, not only for ones that started.
**Neither new test covers this.** `BuildFloodSpheres_BspBearingOwner_...` and
`FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint` both hand-substitute a
synthetic `Radius = 14f` at `LocalPosition = Vector3.Zero`, i.e. a *concentric*
BSP sphere — exactly the configuration in which F1 cannot fire.
### F3 (MEDIUM) — `RegisterMultiPart`'s own doc comment still states the superseded rule
`src/AcDream.Core/Physics/ShadowObjectRegistry.cs:432-439` (the sole caller of
`BuildFloodSpheres`, 168 lines above it) still reads:
> *"Flood spheres follow retail's rule (Ghidra 0x0052b9f0): when the object has
> CylSpheres, they alone drive the flood (base point + cyl radius, capped at 10);
> otherwise the BSP parts' bounding spheres stand in for the sorting sphere."*
That is the pre-`4abd1b5e` rule and is now false — it is the rule sabotage B
restored, and sabotage B reddened a test. The commit rewrote
`BuildFloodSpheres`' own doc thoroughly and left its caller's contradicting it.
The next reader who greps `RegisterMultiPart` before `BuildFloodSpheres` gets the
wrong mental model of the very behaviour this commit changed.
### F4 (MEDIUM) — unregistered divergence: retail's third branch is ONE sorting sphere, acdream's is every Sphere shape
Retail's `calc_cross_cells` fall-through calls `CPartArray::GetSortingSphere`
@`0x00518b00`, which returns `[partArray+0x54] + 0x70` — a single authored
whole-object sphere on the `CSetup` — and floods from that one sphere
(`0x005152fb`). acdream's `only == null` branch
(`ShadowObjectRegistry.cs:652`) applies no filter and floods from **every**
non-BSP, non-Cylinder shape, i.e. the Setup's per-part Sphere array.
Different DAT field, different cardinality, different extent. The source comment
at `ShadowObjectRegistry.cs:646-648` admits the substitution
(*"which acdream approximates with the remaining shapes' bounding spheres"*), and
AP-155's anchor column even cites `CPartArray::GetSortingSphere 0x00518b00` — but
no AP row states the divergence. The register's own rule ("any commit that
introduces a deviation adds its register row IN THE SAME COMMIT") is not
retroactive, and this predates `4abd1b5e`; but the commit rewrote this exact
method, filed three rows for its neighbours, and stepped over this one.
Same class, smaller: `BuildFloodSpheres` collapses a Cylinder to a single sphere
at its base point with the cylinder radius and **ignores `CylHeight` entirely**,
where retail's `CObjCell::find_cell_list` @`0x0052b9f0` is handed the CCylSphere
array `(low_pt, radius, height)`. Also unregistered.
### F5 (LOW) — AP-155 is two divergences in one row; AP-153 and AP-154 are honestly distinct
Asked whether one of AP-153/154/155 is the same divergence sliced twice: **no**,
but AP-155 has the inverse problem.
- **AP-153** (flag cached at construction vs. re-derived live) and **AP-154**
(flag taken off the wire vs. derived client-side) concern the same bit but are
genuinely different questions — *when* vs. *where from* — with different sites
(`ShadowShapeBuilder` step 0 / `LiveEntityCollisionBuilder.ReconcileAppearance`
vs. `TransitionTypes.cs:1348` / `LiveEntityCollisionBuilder.cs:161`), different
risks, and different gates. Both scoped honestly.
- **AP-155** bundles (a) the static publishers' Setup-Sphere→height-capped-Cylinder
conversion — a *different* code path, a *different and larger* population, and
its own stated gate — with (b) the flood-priority/approximation question, and
then files (b) as already half-closed by the same commit. Three different
lifecycles under one id. It should be two rows (or three), and (b)'s open
remainder needs the F1 correction before it can be reasoned about at all.
### F6 (LOW) — commit-body imprecision
*"Priority at both consumers: BSP -> CylSphere -> Sphere -> nothing."* At
`FindObjCollisions` the third branch is `CPartArray::GetSphere` @`0x00518070`
(the per-part Sphere array, loop head `0x0050f21d`). At `calc_cross_cells` it is
`CPartArray::GetSortingSphere` @`0x00518b00` (one authored whole-object sphere,
`0x005152e3`). Different arrays; "both consumers" is not true of the third rung.
The register row and the source comments state this correctly — only the commit
message is loose. Recorded so a future grep of the log doesn't inherit it.
---
## What was checked and found clean (so the PASS half is auditable)
- Binary/PDB pairing (`MATCH`), and every disassembly re-derived from that binary
rather than Binary Ninja.
- `calc_cross_cells` full body: flag test address, jump target, both primitive
call sites, and the **fall-through order** including the cylsphere branch's
skip-past-sorting-sphere `jmp`.
- `FindObjCollisions` full body: all five cited addresses, the unconditional
`jmp` past both loops, and identification of `ebp` as the ethereal early-out
rather than a shape branch.
- `CacheHasPhysicsBSP` (both classes) bodies + exhaustive rel32/absolute xref scan;
`InitPartArrayObject` callers; `CPartArray::SetPart` non-recaching.
- `find_bbox_cell_list``calc_cross_cells_static``[vtbl+0x7c]` chain.
- `GetSortingSphere` body (`[+0x54]+0x70`).
- Retail's 10-sphere cap (`0x0052ba21 cmp eax, 0xa`).
- 12 distinct cited addresses → symbol, no mis-citation.
- All four register corrections re-measured from the installed DAT by an
independent sweep; population 5,935 / 172 / 73 / 99 / 530 reproduced exactly.
- AP active-row count parsed (107) and reconciled against the `ec29a732` baseline (105).
- AP-154's `src/` grep claim (exactly 2 hits, second is a mover-state read).
- ACE's derivation of the wire bit (`WorldObject_Networking.cs:666-669`).
- Static publishers' `entityBspCount == 0` exclusivity gate.
- `FromSetup`'s sole production caller is App-layer (Headless-unreachable).
- `EffectivePartGfxObjId` genuinely shared by step 0 and step 3 — the
"gate and emission read the same identity" trap is real and its test
discriminates in both directions.
- Clean `bin`/`obj` → Release build 0 errors → full suite 11,203 / 4 skip / 0 fail.
- Two sabotages, both reddening only in the intended direction; tree restored and
`git status --porcelain` empty.
## Recommended before the connected gate
1. Correct AP-155's Risk column: the approximation is **under**-inclusive for
170 of the 172 Setups this commit moved onto it, not over-inclusive.
2. Carry the BSP root sphere's `Origin` through `ShadowShape` (or offset
`LocalPosition` by `partFrame.Orientation * rootSphere.Origin`) so the flood
sphere is concentric with the geometry it stands for. That is a one-field fix
at `ShadowShapeBuilder.cs:192-199` + `LiveEntityCollisionBuilder.cs:135-139`,
and it converts F1/F2 from open risk to closed.
3. Add a non-concentric fixture to `ShadowObjectRegistryMultiPartTests` — the two
new flood tests both use `LocalPosition = Zero`, which is the one configuration
where the defect cannot appear.
4. Fix the stale `RegisterMultiPart` doc (F3) and split AP-155 (F5).
5. Instruct the connected gate to look for props/doors that **stopped** blocking
as well as ones that started; 43 Setups shrank.

View file

@ -29,8 +29,10 @@ internal sealed record LiveEntityCollisionRegistration(
/// <summary> /// <summary>
/// Ports the live-object collision-shape policy used by /// Ports the live-object collision-shape policy used by
/// <c>CPartArray::FindObjCollisions</c>: CylSpheres before Spheres, and every /// <c>CPhysicsObj::FindObjCollisions</c> (0x0050f050), which DISPATCHES
/// physics-BSP part. /// rather than unions: every physics-BSP part, ELSE the Setup's CylSpheres,
/// ELSE its Spheres, else nothing (AP-152). The gate itself lives in
/// <see cref="ShadowShapeBuilder.FromSetup"/>.
/// </summary> /// </summary>
/// <remarks> /// <remarks>
/// A Setup that yields no shape produces no registration. Retail synthesizes /// A Setup that yields no shape produces no registration. Retail synthesizes
@ -45,37 +47,39 @@ internal sealed record LiveEntityCollisionRegistration(
/// </remarks> /// </remarks>
internal sealed class LiveEntityCollisionBuilder internal sealed class LiveEntityCollisionBuilder
{ {
private readonly Func<uint, bool> _hasPhysicsBsp; private readonly Func<uint, FlatCollisionSphere?> _physicsBspBounds;
private readonly Func<uint, float?> _physicsBspRadius;
private readonly LiveEntityDefaultPoseResolver _defaultPose; private readonly LiveEntityDefaultPoseResolver _defaultPose;
public LiveEntityCollisionBuilder( public LiveEntityCollisionBuilder(
PhysicsDataCache physicsData, PhysicsDataCache physicsData,
LiveEntityDefaultPoseResolver defaultPose) LiveEntityDefaultPoseResolver defaultPose)
: this( : this(
id => physicsData.GetFlatGfxObj(id)?.PhysicsBsp.RootIndex >= 0,
id => id =>
{ {
FlatPhysicsBsp? flat = FlatPhysicsBsp? flat =
physicsData.GetFlatGfxObj(id)?.PhysicsBsp; physicsData.GetFlatGfxObj(id)?.PhysicsBsp;
return flat is { RootIndex: >= 0 } return flat is { RootIndex: >= 0 }
? flat.Nodes[flat.RootIndex].BoundingSphere.Radius ? flat.Nodes[flat.RootIndex].BoundingSphere
: null; : (FlatCollisionSphere?)null;
}, },
defaultPose) defaultPose)
{ {
ArgumentNullException.ThrowIfNull(physicsData); ArgumentNullException.ThrowIfNull(physicsData);
} }
/// <param name="physicsBspBounds">The part GfxObj's physics-BSP root
/// bounding sphere, or null when it has none. ONE resolver answers both
/// questions the builder asks — "does this part dispatch as BSP?" and
/// "where and how big is its flood sphere?" — so the dispatch gate and
/// the emitted geometry cannot disagree, and the sphere's radius cannot
/// be carried while its origin is dropped. That split is what produced
/// the AP-156 mis-placed flood.</param>
internal LiveEntityCollisionBuilder( internal LiveEntityCollisionBuilder(
Func<uint, bool> hasPhysicsBsp, Func<uint, FlatCollisionSphere?> physicsBspBounds,
Func<uint, float?> physicsBspRadius,
LiveEntityDefaultPoseResolver defaultPose) LiveEntityDefaultPoseResolver defaultPose)
{ {
_hasPhysicsBsp = hasPhysicsBsp _physicsBspBounds = physicsBspBounds
?? throw new ArgumentNullException(nameof(hasPhysicsBsp)); ?? throw new ArgumentNullException(nameof(physicsBspBounds));
_physicsBspRadius = physicsBspRadius
?? throw new ArgumentNullException(nameof(physicsBspRadius));
_defaultPose = defaultPose _defaultPose = defaultPose
?? throw new ArgumentNullException(nameof(defaultPose)); ?? throw new ArgumentNullException(nameof(defaultPose));
} }
@ -123,25 +127,17 @@ internal sealed class LiveEntityCollisionBuilder
IReadOnlyList<Frame>? defaultPose = _defaultPose.Resolve( IReadOnlyList<Frame>? defaultPose = _defaultPose.Resolve(
spawn.MotionTableId ?? 0u, spawn.MotionTableId ?? 0u,
setup.Parts.Count); setup.Parts.Count);
IReadOnlyList<ShadowShape> raw = ShadowShapeBuilder.FromSetup( // One resolver drives both the dispatch gate and the emitted BSP
// geometry, and FromSetup applies the entity scale to radius and
// center alike — there is no downstream substitution that could take
// one and drop the other.
IReadOnlyList<ShadowShape> shapes = ShadowShapeBuilder.FromSetup(
setup, setup,
scale, scale,
_hasPhysicsBsp, id => _physicsBspBounds(id) is not null,
partPoseOverride: defaultPose, partPoseOverride: defaultPose,
effectivePartGfxObjIds: effectivePartGfxObjIds); effectivePartGfxObjIds: effectivePartGfxObjIds,
var shapes = new List<ShadowShape>(raw.Count); physicsBspBounds: _physicsBspBounds);
foreach (ShadowShape shape in raw)
{
if (shape.CollisionType == ShadowCollisionType.BSP)
{
float radius = (_physicsBspRadius(shape.GfxObjId) ?? 2f) * scale;
shapes.Add(shape with { Radius = radius });
}
else
{
shapes.Add(shape);
}
}
if (shapes.Count == 0 && !retainEmptyPayload) if (shapes.Count == 0 && !retainEmptyPayload)
return null; return null;

View file

@ -430,13 +430,14 @@ public sealed class ShadowObjectRegistry
/// ///
/// <para> /// <para>
/// BR-7: the cell set is ONE flood for the whole entity (retail floods /// BR-7: the cell set is ONE flood for the whole entity (retail floods
/// per OBJECT with its full sphere set, not per part). Flood spheres /// per OBJECT with its full sphere set, not per part). The flood spheres
/// follow retail's rule (Ghidra 0x0052b9f0): when the object has /// follow <c>CPhysicsObj::calc_cross_cells</c>' own EXCLUSIVE priority —
/// CylSpheres, they alone drive the flood (base point + cyl radius, /// physics-BSP parts, else CylSpheres, else the remaining shapes — see
/// capped at 10); otherwise the BSP parts' bounding spheres stand in /// <see cref="BuildFloodSpheres"/> for the disassembly. A BSP part
/// for the sorting sphere. Every shape row is then written into every /// contributes its ROOT BOUNDING SPHERE placed at its real center
/// flooded cell, mirroring add_shadows_to_cells (0x00514ae0) + /// (<see cref="ShadowShape.BoundsCenter"/>), not at the part origin.
/// CPartArray::AddPartsShadow. /// Every shape row is then written into every flooded cell, mirroring
/// add_shadows_to_cells (0x00514ae0) + CPartArray::AddPartsShadow.
/// </para> /// </para>
/// </summary> /// </summary>
public void RegisterMultiPart( public void RegisterMultiPart(
@ -604,8 +605,17 @@ public sealed class ShadowObjectRegistry
/// <item>BSP-bearing (<c>0x00515285 test dword [esi+0xa8],0x10000</c> / /// <item>BSP-bearing (<c>0x00515285 test dword [esi+0xa8],0x10000</c> /
/// <c>0x0051528f jne 0x515305</c>) → <c>CPhysicsObj::find_bbox_cell_list</c> /// <c>0x0051528f jne 0x515305</c>) → <c>CPhysicsObj::find_bbox_cell_list</c>
/// @0x00510fc0. The cylsphere and sorting-sphere branches are BOTH below /// @0x00510fc0. The cylsphere and sorting-sphere branches are BOTH below
/// that jump and unreachable from it. acdream approximates the bbox with /// that jump and unreachable from it. <c>find_bbox_cell_list</c> adds the
/// the BSP parts' bounding spheres.</item> /// object's own cell and then walks the PART ARRAY through
/// <c>CPartArray::calc_cross_cells_static</c> @0x00518160's
/// <c>[vtbl+0x7c]</c> dispatch, whose EnvCell body
/// (<c>CEnvCell::find_transit_cells</c> @0x0052cae0) tests each part's
/// <c>CGfxObj::physics_sphere</c> — the BSP root bounding sphere, center
/// transformed through the part's own Position — against the cell's
/// portal planes. acdream floods from those same per-part spheres
/// (<see cref="ShadowShape.BoundsCenter"/> + <see cref="ShadowShape.Radius"/>)
/// rather than walking portal planes per part; the sphere set is exact,
/// the traversal is the sphere-vs-portal one (AP-156).</item>
/// <item>else cylspheres (<c>0x00515298 GetNumCylsphere</c> non-zero) → /// <item>else cylspheres (<c>0x00515298 GetNumCylsphere</c> non-zero) →
/// <c>CObjCell::find_cell_list</c> @0x0052b9f0 over the cylsphere array; /// <c>CObjCell::find_cell_list</c> @0x0052b9f0 over the cylsphere array;
/// each contributes one sphere at its world BASE point with the cylinder /// each contributes one sphere at its world BASE point with the cylinder
@ -645,20 +655,42 @@ public sealed class ShadowObjectRegistry
// Retail's branch, chosen once: BSP-bbox, else cylspheres, else the // Retail's branch, chosen once: BSP-bbox, else cylspheres, else the
// sorting sphere (which acdream approximates with the remaining // sorting sphere (which acdream approximates with the remaining
// shapes' bounding spheres). // shapes' bounding spheres — AP-157).
ShadowCollisionType? only = ShadowCollisionType? only =
anyBsp ? ShadowCollisionType.BSP anyBsp ? ShadowCollisionType.BSP
: anyCyl ? ShadowCollisionType.Cylinder : anyCyl ? ShadowCollisionType.Cylinder
: null; : null;
// The 10-sphere clamp belongs to the CYLSPHERE branch alone.
// CObjCell::find_cell_list @0x0052b9f0 clamps the cylsphere count at
// 0x0052ba21 cmp eax,0xa / 0x0052ba28 mov ebp,0xa. The BSP branch —
// find_bbox_cell_list @0x00510fc0 -> CPartArray::calc_cross_cells_static
// @0x00518160 -> CEnvCell::find_transit_cells @0x0052cae0 — walks EVERY
// part with no cap, and the sorting-sphere overload @0x0052b990 takes a
// single sphere. Applying the clamp to the BSP branch dropped parts
// 11..N out of the flood entirely: 7 installed Setups carry more than
// 10 physics-BSP parts (max 49, Setup 0x02001A91), and landblock-baked
// part arrays — stair runs, fences, rock clusters — routinely do.
int cap = only == ShadowCollisionType.Cylinder ? RetailSphereCap : int.MaxValue;
foreach (var s in shapes) foreach (var s in shapes)
{ {
if (only is { } required && s.CollisionType != required) if (only is { } required && s.CollisionType != required)
continue; continue;
if (spheres.Count >= RetailSphereCap) if (spheres.Count >= cap)
break; break;
var world = entityWorldPos + Vector3.Transform(s.LocalPosition, entityWorldRot); // Place the sphere where the GEOMETRY is, not where the part
// origin is. Composed exactly as the ShadowEntry rows below are
// (partWorldPos / partWorldRot), then offset by the shape's own
// BoundsCenter — retail's CEnvCell::find_transit_cells @0x0052cae0
// transforms CGfxObj::physics_sphere's center through the part's
// Position at [part+0x30] before reading its radius at
// 0x0052cb65. Primitives carry BoundsCenter == Zero because their
// LocalPosition already is their center.
var partWorldPos = entityWorldPos + Vector3.Transform(s.LocalPosition, entityWorldRot);
var partWorldRot = entityWorldRot * s.LocalRotation;
var world = partWorldPos + Vector3.Transform(s.BoundsCenter, partWorldRot);
spheres.Add(new DatReaderWriter.Types.Sphere spheres.Add(new DatReaderWriter.Types.Sphere
{ {
Origin = world, Origin = world,

View file

@ -4,8 +4,9 @@ namespace AcDream.Core.Physics;
/// <summary> /// <summary>
/// One collision-bearing shape attached to a logical PhysicsObj. /// One collision-bearing shape attached to a logical PhysicsObj.
/// A door emits 3-4 (CylSphere(s) + Sphere(s) + Part-BSPs); a creature /// The set is a DISPATCH, not a union (AP-152): a BSP door emits one shape
/// may emit a few; a simple item zero. Positions and rotations are /// per physics-BSP part and no primitive; a primitive-only object emits its
/// CylSpheres, else its Spheres; a simple item zero. Positions and rotations are
/// LOCAL to the entity's origin so <see cref="ShadowObjectRegistry.UpdatePosition"/> /// LOCAL to the entity's origin so <see cref="ShadowObjectRegistry.UpdatePosition"/>
/// can re-transform them when the entity moves. /// can re-transform them when the entity moves.
/// ///
@ -15,6 +16,40 @@ namespace AcDream.Core.Physics;
/// own transform and dispatches per-part collision to its GfxObj. /// own transform and dispatches per-part collision to its GfxObj.
/// </para> /// </para>
/// </summary> /// </summary>
/// <param name="BoundsCenter">
/// Center of the shape's bounding sphere IN THE SHAPE'S OWN LOCAL FRAME —
/// the frame <see cref="LocalRotation"/> rotates out of and
/// <see cref="Radius"/> is measured in — already multiplied by the entity
/// scale, like <see cref="LocalPosition"/> and <see cref="Radius"/>.
/// <c>Zero</c> for Cylinder/Sphere shapes, whose
/// <see cref="LocalPosition"/> already IS their center.
///
/// <para>
/// BSP shapes need it because a GfxObj's physics BSP is authored in the
/// GfxObj's own coordinates and its root bounding sphere is frequently NOT
/// centered on that origin (376 of the 973 physics-BSP parts in the
/// installed <c>client_portal.dat</c> sit further from it than half their
/// own radius; worst 20.762 m on a 27.708 m sphere, gfx 0x010036DD).
/// <see cref="ShadowObjectRegistry.RegisterMultiPart"/>'s flood needs the
/// sphere's real position, not the part origin's.
/// </para>
///
/// <para>
/// Retail anchor: <c>CGfxObj::physics_sphere</c> (<c>[gfxobj+0x74]</c>) is
/// assigned <c>BSPTREE::GetSphere(physics_bsp)</c> @0x005397e0
/// (<c>mov eax,[ecx]; add eax,4</c> — the root <c>BSPNODE</c>'s
/// <c>CSphere sphere</c>, past its 4-byte vftable), so retail's per-part
/// flood sphere IS the BSP root bounding sphere, origin included.
/// <c>CEnvCell::find_transit_cells</c> @0x0052cae0 — the part-array overload
/// reached from <c>CPhysicsObj::find_bbox_cell_list</c> @0x00510fc0 via
/// <c>CPartArray::calc_cross_cells_static</c> @0x00518160's
/// <c>[vtbl+0x7c]</c> dispatch — loads that sphere at
/// <c>0x0052cb36 mov esi,[ecx+0x74]</c>, transforms its CENTER through the
/// part's own <c>Position</c> at <c>[part+0x30]</c>
/// (<c>0x0052cb4c add eax,0x30</c> / <c>0x0052cb5a call Position::localtolocal</c>),
/// and only then reads the radius at <c>0x0052cb65 fadd [esi+0xc]</c>.
/// </para>
/// </param>
public readonly record struct ShadowShape( public readonly record struct ShadowShape(
uint GfxObjId, uint GfxObjId,
Vector3 LocalPosition, Vector3 LocalPosition,
@ -22,4 +57,5 @@ public readonly record struct ShadowShape(
float Scale, float Scale,
ShadowCollisionType CollisionType, ShadowCollisionType CollisionType,
float Radius, float Radius,
float CylHeight); float CylHeight,
Vector3 BoundsCenter = default);

View file

@ -73,7 +73,11 @@ public static class ShadowShapeBuilder
/// <param name="entScale">The entity's overall scale factor; multiplies /// <param name="entScale">The entity's overall scale factor; multiplies
/// every radius, height, and local offset.</param> /// every radius, height, and local offset.</param>
/// <param name="hasPhysicsBsp">Predicate: does the GfxObj with this id /// <param name="hasPhysicsBsp">Predicate: does the GfxObj with this id
/// have a non-null PhysicsBSP? Production: <c>id => cache.GetGfxObj(id)?.BSP?.Root is not null</c>.</param> /// have a physics BSP? Production derives it from the SAME resolver that
/// supplies <paramref name="physicsBspBounds"/>
/// (<c>id => _physicsBspBounds(id) is not null</c>, over
/// <c>PhysicsDataCache.GetFlatGfxObj(id)?.PhysicsBsp</c>), so the dispatch
/// gate and the emitted geometry cannot disagree.</param>
/// <param name="partPoseOverride">#175: per-part pose override for the /// <param name="partPoseOverride">#175: per-part pose override for the
/// BSP part shapes — the entity's motion-table DEFAULT-STATE pose (the /// BSP part shapes — the entity's motion-table DEFAULT-STATE pose (the
/// closed pose for doors). Retail collision tests each part's LIVE /// closed pose for doors). Retail collision tests each part's LIVE
@ -89,12 +93,22 @@ public static class ShadowShapeBuilder
/// retail <c>AnimPartChanged</c> processing. Collision keeps each Setup /// retail <c>AnimPartChanged</c> processing. Collision keeps each Setup
/// index and pose, but reads PhysicsBSP from the installed replacement. /// index and pose, but reads PhysicsBSP from the installed replacement.
/// Null or short lists fall back to the Setup identity.</param> /// Null or short lists fall back to the Setup identity.</param>
/// <param name="physicsBspBounds">The part GfxObj's physics-BSP ROOT
/// bounding sphere — retail's <c>CGfxObj::physics_sphere</c>, which is
/// literally <c>BSPTREE::GetSphere(physics_bsp)</c> @0x005397e0. Supplies
/// BOTH the emitted <see cref="ShadowShape.Radius"/> and its
/// <see cref="ShadowShape.BoundsCenter"/>, from one call, so the sphere's
/// size can never be carried while its position is dropped. Null (or a
/// null result) falls back to the loose-but-safe 2 m placeholder at the
/// part origin — a fixture-only configuration; production always supplies
/// it (<c>LiveEntityCollisionBuilder</c>).</param>
public static IReadOnlyList<ShadowShape> FromSetup( public static IReadOnlyList<ShadowShape> FromSetup(
Setup setup, Setup setup,
float entScale, float entScale,
Func<uint, bool> hasPhysicsBsp, Func<uint, bool> hasPhysicsBsp,
IReadOnlyList<Frame>? partPoseOverride = null, IReadOnlyList<Frame>? partPoseOverride = null,
IReadOnlyList<uint>? effectivePartGfxObjIds = null) IReadOnlyList<uint>? effectivePartGfxObjIds = null,
Func<uint, FlatCollisionSphere?>? physicsBspBounds = null)
{ {
if (setup is null) throw new ArgumentNullException(nameof(setup)); if (setup is null) throw new ArgumentNullException(nameof(setup));
if (hasPhysicsBsp is null) throw new ArgumentNullException(nameof(hasPhysicsBsp)); if (hasPhysicsBsp is null) throw new ArgumentNullException(nameof(hasPhysicsBsp));
@ -185,9 +199,22 @@ public static class ShadowShapeBuilder
else else
partFrame = new Frame { Origin = Vector3.Zero, Orientation = Quaternion.Identity }; partFrame = new Frame { Origin = Vector3.Zero, Orientation = Quaternion.Identity };
// BSP radius default; caller substitutes the real BoundingSphere.Radius // The part's physics-BSP root bounding sphere — retail's
// at registration time when available. Loose-but-safe broadphase value. // CGfxObj::physics_sphere, assigned BSPTREE::GetSphere(physics_bsp)
float bspRadius = 2f * entScale; // @0x005397e0. RADIUS AND CENTER TOGETHER: retail's per-part
// cross-cell walk (CEnvCell::find_transit_cells @0x0052cae0,
// reached from find_bbox_cell_list @0x00510fc0 through
// CPartArray::calc_cross_cells_static @0x00518160) transforms the
// sphere's CENTER through the part's own Position
// (0x0052cb4c add eax,0x30 → Position::localtolocal) BEFORE it
// reads the radius at 0x0052cb65 fadd [esi+0xc]. The center is not
// the part origin: 376 of the 973 installed physics-BSP parts sit
// further from it than half their own radius. A single resolver
// supplies both so one cannot be taken without the other.
// Absent bounds keep the loose-but-safe 2 m placeholder.
FlatCollisionSphere? bounds = physicsBspBounds?.Invoke(gfxId);
float bspRadius = (bounds?.Radius ?? 2f) * entScale;
Vector3 boundsCenter = (bounds?.Origin ?? Vector3.Zero) * entScale;
result.Add(new ShadowShape( result.Add(new ShadowShape(
GfxObjId: gfxId, GfxObjId: gfxId,
@ -196,7 +223,8 @@ public static class ShadowShapeBuilder
Scale: entScale, Scale: entScale,
CollisionType: ShadowCollisionType.BSP, CollisionType: ShadowCollisionType.BSP,
Radius: bspRadius, Radius: bspRadius,
CylHeight: 0f)); CylHeight: 0f,
BoundsCenter: boundsCenter));
} }
return result; return result;
@ -269,9 +297,27 @@ public static class ShadowShapeBuilder
} }
float partScale = pScale.X > 0f ? pScale.X : 1f; // AC objects are uniformly scaled float partScale = pScale.X > 0f ? pScale.X : 1f; // AC objects are uniformly scaled
float localRadius = hasFlat // Root bounding sphere, CENTER AND RADIUS TOGETHER — see
? flat!.Nodes[flat.RootIndex].BoundingSphere.Radius // ShadowShape.BoundsCenter. Retail's per-part cross-cell walk
: phys.BoundingSphere?.Radius ?? 1f; // (CEnvCell::find_transit_cells @0x0052cae0) transforms
// CGfxObj::physics_sphere's center through the part's Position
// before using its radius; a landblock-baked part array is the
// same CPartArray walk (CPartArray::calc_cross_cells_static
// @0x00518160), so dropping the center here mis-places the flood
// exactly as it did for live Setups.
float localRadius;
Vector3 localCenter;
if (hasFlat)
{
FlatCollisionSphere root = flat!.Nodes[flat.RootIndex].BoundingSphere;
localRadius = root.Radius;
localCenter = root.Origin;
}
else
{
localRadius = phys.BoundingSphere?.Radius ?? 1f;
localCenter = phys.BoundingSphere?.Origin ?? Vector3.Zero;
}
shapes.Add(new ShadowShape( shapes.Add(new ShadowShape(
GfxObjId: meshRef.GfxObjId, GfxObjId: meshRef.GfxObjId,
@ -280,7 +326,8 @@ public static class ShadowShapeBuilder
Scale: partScale, Scale: partScale,
CollisionType: ShadowCollisionType.BSP, CollisionType: ShadowCollisionType.BSP,
Radius: localRadius * partScale, Radius: localRadius * partScale,
CylHeight: 0f)); CylHeight: 0f,
BoundsCenter: localCenter * partScale));
} }
return shapes; return shapes;

View file

@ -98,8 +98,18 @@ public sealed class LiveEntityCollisionBuilderTests
new Vector3(192f, -192f, 0f))); new Vector3(192f, -192f, 0f)));
} }
/// <summary>
/// The BSP root bounding sphere reaches the shape WHOLE — radius and
/// centre, both scaled. Retail's <c>CGfxObj::physics_sphere</c> is
/// <c>BSPTREE::GetSphere(physics_bsp)</c> @0x005397e0, and
/// <c>CEnvCell::find_transit_cells</c> @0x0052cae0 transforms its centre
/// through the part's Position (<c>0x0052cb4c add eax,0x30</c>) before
/// reading the radius at <c>0x0052cb65</c>. Carrying only the radius is
/// AP-156: 170 of the 172 affected installed Setups then flood from a
/// sphere that does not contain their own collision geometry.
/// </summary>
[Fact] [Fact]
public void BspOnlyPart_UsesRealScaledPhysicsBoundingRadius() public void BspOnlyPart_UsesRealScaledPhysicsBoundingSphere()
{ {
const uint part = 0x0100ABCDu; const uint part = 0x0100ABCDu;
var setup = new Setup(); var setup = new Setup();
@ -109,8 +119,7 @@ public sealed class LiveEntityCollisionBuilderTests
WorldEntity entity = Entity(); WorldEntity entity = Entity();
record.WorldEntity = entity; record.WorldEntity = entity;
var builder = new LiveEntityCollisionBuilder( var builder = new LiveEntityCollisionBuilder(
id => id == part, id => id == part ? Bsp(3f, centerZ: 2f) : null,
id => id == part ? 3f : null,
PoseResolver()); PoseResolver());
LiveEntityCollisionRegistration registration = Assert.IsType<LiveEntityCollisionRegistration>( LiveEntityCollisionRegistration registration = Assert.IsType<LiveEntityCollisionRegistration>(
@ -118,7 +127,8 @@ public sealed class LiveEntityCollisionBuilderTests
ShadowShape shape = Assert.Single(registration.Shapes); ShadowShape shape = Assert.Single(registration.Shapes);
Assert.Equal(ShadowCollisionType.BSP, shape.CollisionType); Assert.Equal(ShadowCollisionType.BSP, shape.CollisionType);
Assert.Equal(4.5f, shape.Radius); Assert.Equal(4.5f, shape.Radius); // 3 m * 1.5
Assert.Equal(new Vector3(0f, 0f, 3f), shape.BoundsCenter); // 2 m * 1.5
Assert.Equal(part, shape.GfxObjId); Assert.Equal(part, shape.GfxObjId);
} }
@ -151,8 +161,7 @@ public sealed class LiveEntityCollisionBuilderTests
WorldEntity entity = Entity(); WorldEntity entity = Entity();
record.WorldEntity = entity; record.WorldEntity = entity;
var builder = new LiveEntityCollisionBuilder( var builder = new LiveEntityCollisionBuilder(
id => id == part, id => id == part ? Bsp(3f, centerZ: 2f) : null,
id => id == part ? 3f : null,
PoseResolver()); PoseResolver());
LiveEntityCollisionRegistration registration = LiveEntityCollisionRegistration registration =
@ -162,6 +171,7 @@ public sealed class LiveEntityCollisionBuilderTests
ShadowShape shape = Assert.Single(registration.Shapes); ShadowShape shape = Assert.Single(registration.Shapes);
Assert.Equal(ShadowCollisionType.BSP, shape.CollisionType); Assert.Equal(ShadowCollisionType.BSP, shape.CollisionType);
Assert.Equal(4.5f, shape.Radius); // 3 m physics-BSP radius * 1.5 scale Assert.Equal(4.5f, shape.Radius); // 3 m physics-BSP radius * 1.5 scale
Assert.Equal(new Vector3(0f, 0f, 3f), shape.BoundsCenter);
Assert.Equal(part, shape.GfxObjId); Assert.Equal(part, shape.GfxObjId);
} }
@ -177,8 +187,7 @@ public sealed class LiveEntityCollisionBuilderTests
WorldEntity entity = Entity(); WorldEntity entity = Entity();
record.WorldEntity = entity; record.WorldEntity = entity;
var builder = new LiveEntityCollisionBuilder( var builder = new LiveEntityCollisionBuilder(
id => id == basePart, id => id == basePart ? Bsp(1f) : null,
_ => 1f,
PoseResolver()); PoseResolver());
Assert.Null(builder.Build( Assert.Null(builder.Build(
@ -202,8 +211,7 @@ public sealed class LiveEntityCollisionBuilderTests
WorldEntity entity = Entity(); WorldEntity entity = Entity();
record.WorldEntity = entity; record.WorldEntity = entity;
var builder = new LiveEntityCollisionBuilder( var builder = new LiveEntityCollisionBuilder(
id => id == replacement, id => id == replacement ? Bsp(2.25f) : null,
id => id == replacement ? 2.25f : null,
PoseResolver()); PoseResolver());
LiveEntityCollisionRegistration registration = LiveEntityCollisionRegistration registration =
@ -232,8 +240,7 @@ public sealed class LiveEntityCollisionBuilderTests
WorldEntity entity = Entity(); WorldEntity entity = Entity();
record.WorldEntity = entity; record.WorldEntity = entity;
var builder = new LiveEntityCollisionBuilder( var builder = new LiveEntityCollisionBuilder(
id => id == basePart, id => id == basePart ? Bsp(1f) : null,
_ => 1f,
PoseResolver()); PoseResolver());
LiveEntityCollisionRegistration initial = LiveEntityCollisionRegistration initial =
Assert.IsType<LiveEntityCollisionRegistration>(builder.Build( Assert.IsType<LiveEntityCollisionRegistration>(builder.Build(
@ -265,8 +272,7 @@ public sealed class LiveEntityCollisionBuilderTests
WorldEntity entity = Entity(); WorldEntity entity = Entity();
record.WorldEntity = entity; record.WorldEntity = entity;
var builder = new LiveEntityCollisionBuilder( var builder = new LiveEntityCollisionBuilder(
id => id == basePart || id == replacement, id => id == basePart || id == replacement ? Bsp(1f) : null,
_ => 1f,
PoseResolver()); PoseResolver());
LiveEntityCollisionRegistration initial = LiveEntityCollisionRegistration initial =
Assert.IsType<LiveEntityCollisionRegistration>(builder.Build( Assert.IsType<LiveEntityCollisionRegistration>(builder.Build(
@ -330,8 +336,7 @@ public sealed class LiveEntityCollisionBuilderTests
WorldEntity entity = Entity(); WorldEntity entity = Entity();
record.WorldEntity = entity; record.WorldEntity = entity;
var builder = new LiveEntityCollisionBuilder( var builder = new LiveEntityCollisionBuilder(
id => id == basePart || id == replacement, id => id == basePart || id == replacement ? Bsp(1f) : null,
_ => 1f,
PoseResolver()); PoseResolver());
LiveEntityCollisionRegistration initial = LiveEntityCollisionRegistration initial =
Assert.IsType<LiveEntityCollisionRegistration>(builder.Build( Assert.IsType<LiveEntityCollisionRegistration>(builder.Build(
@ -394,10 +399,20 @@ public sealed class LiveEntityCollisionBuilderTests
} }
private static LiveEntityCollisionBuilder Builder() => new( private static LiveEntityCollisionBuilder Builder() => new(
_ => false,
_ => null, _ => null,
PoseResolver()); PoseResolver());
/// <summary>
/// A physics-BSP root bounding sphere. DELIBERATELY OFF-CENTRE by
/// default: a GfxObj's BSP is authored in the GfxObj's own coordinates
/// and its root sphere is usually NOT centred on that origin (376 of the
/// 973 installed physics-BSP parts sit further from it than half their
/// radius). A fixture pinned at <c>Vector3.Zero</c> cannot observe the
/// centre at all — which is how AP-156's discarded origin stayed green.
/// </summary>
private static FlatCollisionSphere? Bsp(float radius, float centerZ = 1.25f)
=> new FlatCollisionSphere(new Vector3(0f, 0f, centerZ), radius);
private static LiveEntityDefaultPoseResolver PoseResolver() => new( private static LiveEntityDefaultPoseResolver PoseResolver() => new(
_ => null, _ => null,
new NullAnimationLoader(), new NullAnimationLoader(),

View file

@ -142,8 +142,9 @@ public sealed class PvpBitfieldSurvivesAppearanceRebuildTests
var setup = new Setup(); var setup = new Setup();
setup.Parts.Add(0x0100AB01u); setup.Parts.Add(0x0100AB01u);
var builder = new LiveEntityCollisionBuilder( var builder = new LiveEntityCollisionBuilder(
id => id == 0x0100AB01u, id => id == 0x0100AB01u
id => id == 0x0100AB01u ? 1f : null, ? new FlatCollisionSphere(new Vector3(0f, 0f, 0.5f), 1f)
: null,
new LiveEntityDefaultPoseResolver( new LiveEntityDefaultPoseResolver(
_ => null, _ => null,
new NullAnimationLoader(), new NullAnimationLoader(),

View file

@ -149,7 +149,6 @@ public sealed class LiveAppearanceAnimationTests
seedCellId: cell, seedCellId: cell,
isStatic: false); isStatic: false);
var builder = new LiveEntityCollisionBuilder( var builder = new LiveEntityCollisionBuilder(
_ => false,
_ => null, _ => null,
new LiveEntityDefaultPoseResolver( new LiveEntityDefaultPoseResolver(
_ => null, _ => null,

View file

@ -1,8 +1,11 @@
using System.Globalization;
using System.Numerics;
using AcDream.Core.Physics; using AcDream.Core.Physics;
using DatReaderWriter; using DatReaderWriter;
using DatReaderWriter.DBObjs; using DatReaderWriter.DBObjs;
using DatReaderWriter.Enums; using DatReaderWriter.Enums;
using DatReaderWriter.Options; using DatReaderWriter.Options;
using DatReaderWriter.Types;
namespace AcDream.Content.Tests; namespace AcDream.Content.Tests;
@ -156,4 +159,217 @@ public sealed class InstalledSetupBspPrimitiveDispatchTests
Assert.Empty(affectedThatStillEmitAPrimitive); Assert.Empty(affectedThatStillEmitAPrimitive);
} }
// EXTERNAL constants for the containment sweep, measured 2026-08-06 by a
// scratch DatReaderWriter sweep that reproduced the geometry by hand
// rather than calling ShadowShapeBuilder, and independently reproduced by
// the AP-152 retail reviewer's own sweep. NOT derived from the code under
// test.
//
// OffCentreParts is the population control: without it, a build in which
// every BSP root sphere happened to sit at its part origin would satisfy
// the containment claim vacuously.
// WouldFailIfOriginDiscarded is the DEFECT control: it re-runs the
// pre-fix composition (radius carried, root-sphere origin dropped) and
// pins how many of the affected Setups it breaks. If that number ever
// goes to zero the fixture population has stopped exercising the field
// and the containment assertion below has stopped meaning anything.
private const int ExpectedPhysicsBspParts = 973;
private const int ExpectedOffCentreParts = 376; // |origin| > radius/2
private const int ExpectedWouldFailIfOriginDiscarded = 170; // of 172
private const int ExpectedDeepestBspPartArray = 49; // Setup 0x02001A91
/// <summary>
/// AP-156. Every flood sphere acdream emits for a physics-BSP part must
/// CONTAIN that part's real collision geometry.
///
/// <para>
/// A GfxObj's physics BSP is authored in the GfxObj's own coordinates and
/// its root bounding sphere is usually not centred on that origin — 376
/// of the 973 installed physics-BSP parts sit further from it than half
/// their own radius, worst 20.762 m on a 27.708 m sphere (gfx 0x010036DD,
/// Setup 0x0200129A). acdream used to take the sphere's radius and drop
/// its origin, flooding from the part origin instead: over the 172
/// AP-152 Setups that failed to contain the object's own BSP sphere for
/// 170 of them, worst shortfall 9.911 m (Setup 0x02000255, whose single
/// part's root sphere sits 9.911 m above the part origin). Indoor floods
/// are 3-D (<c>CellTransit.BuildShadowCellSet</c> routes every candidate
/// with <c>id &amp; 0xFFFF &gt;= 0x0100</c> through
/// <c>FindTransitCellsSphere</c>), so a tall prop or door slab simply was
/// not registered in the EnvCells it occupies — never a broadphase
/// candidate there, the #98 / #168 class.
/// </para>
///
/// <para>
/// Retail: <c>CGfxObj::physics_sphere</c> (<c>[gfxobj+0x74]</c>) is
/// assigned <c>BSPTREE::GetSphere(physics_bsp)</c> @0x005397e0 — the root
/// <c>BSPNODE</c>'s <c>CSphere</c>, past its 4-byte vftable — and
/// <c>CEnvCell::find_transit_cells</c> @0x0052cae0, the part-array
/// overload reached from <c>CPhysicsObj::find_bbox_cell_list</c>
/// @0x00510fc0 via <c>CPartArray::calc_cross_cells_static</c> @0x00518160,
/// transforms that sphere's CENTRE through the part's own Position at
/// <c>[part+0x30]</c> before reading its radius at <c>[esi+0xc]</c>.
/// </para>
/// </summary>
[Fact]
public void InstalledSetups_BspFloodSpheres_ContainTheirOwnBoundingSpheres()
{
string? datDir = ContentConformanceDats.ResolveDatDir();
if (datDir is null)
return;
using var dats = new DatCollection(datDir, DatAccessType.Read);
var boundsCache = new Dictionary<uint, FlatCollisionSphere?>();
FlatCollisionSphere? Bounds(uint gfxObjId)
{
if (boundsCache.TryGetValue(gfxObjId, out FlatCollisionSphere? cached))
return cached;
FlatCollisionSphere? result = null;
if (dats.Portal.TryGet<GfxObj>(gfxObjId, out GfxObj? gfx)
&& gfx is not null
&& gfx.Flags.HasFlag(GfxObjFlags.HasPhysics)
&& gfx.PhysicsBSP?.Root is not null
&& gfx.VertexArray is not null
&& gfx.PhysicsBSP.Root.BoundingSphere is { } bs)
{
result = new FlatCollisionSphere(bs.Origin, bs.Radius);
}
boundsCache[gfxObjId] = result;
return result;
}
const float EntScale = 1.75f; // not 1: a dropped scale must show up
int bspParts = 0;
int offCentreParts = 0;
int affected = 0;
int wouldFailIfOriginDiscarded = 0;
float worstShortfall = 0f;
uint worstShortfallSetup = 0u;
int mostBspShapesOnOneSetup = 0;
var uncontained = new List<uint>();
foreach (uint id in dats.GetAllIdsOfType<Setup>())
{
if (!dats.Portal.TryGet<Setup>(id, out Setup? setup) || setup is null)
continue;
// Independent oracle: resolve the placement frame from the raw
// Setup and place each part's TRUE root sphere by hand.
AnimationFrame? placement = null;
if (setup.PlacementFrames.TryGetValue(Placement.Resting, out var resting))
placement = resting;
else if (setup.PlacementFrames.TryGetValue(Placement.Default, out var def))
placement = def;
else foreach (var kvp in setup.PlacementFrames) { placement = kvp.Value; break; }
var truth = new List<(Vector3 Centre, float Radius)>();
for (int i = 0; i < setup.Parts.Count; i++)
{
FlatCollisionSphere? b = Bounds((uint)setup.Parts[i]);
if (b is null) continue;
bspParts++;
if (b.Value.Origin.Length() > b.Value.Radius / 2f)
offCentreParts++;
Vector3 partOrigin = Vector3.Zero;
Quaternion partRot = Quaternion.Identity;
if (placement is not null && i < placement.Frames.Count)
{
partOrigin = placement.Frames[i].Origin;
partRot = placement.Frames[i].Orientation;
}
truth.Add((
(partOrigin + Vector3.Transform(b.Value.Origin, partRot)) * EntScale,
b.Value.Radius * EntScale));
}
if (truth.Count == 0) continue;
bool hasCylinder = false;
foreach (var cyl in setup.CylSpheres) if (cyl.Radius > 0f) { hasCylinder = true; break; }
bool hasSphere = false;
foreach (var sph in setup.Spheres) if (sph.Radius > 0f) { hasSphere = true; break; }
if (hasCylinder || (setup.CylSpheres.Count == 0 && hasSphere))
affected++;
// Production emission, through the production bounds seam.
IReadOnlyList<ShadowShape> shapes = ShadowShapeBuilder.FromSetup(
setup,
EntScale,
id => Bounds(id) is not null,
physicsBspBounds: Bounds);
// ShadowObjectRegistry.BuildFloodSpheres' composition, at an
// entity placed at the world origin with identity rotation.
// EVERY BSP shape contributes: retail's BSP branch has no
// sphere cap (the 10-clamp at 0x0052ba21 is inside the cylsphere
// overload only), and 7 installed Setups carry more than 10
// physics-BSP parts — capping here would silently exclude their
// tail from the containment claim below.
var flood = new List<(Vector3 Centre, float Radius)>();
var floodIfOriginDiscarded = new List<(Vector3 Centre, float Radius)>();
foreach (ShadowShape shape in shapes)
{
if (shape.CollisionType != ShadowCollisionType.BSP) continue;
flood.Add((
shape.LocalPosition
+ Vector3.Transform(shape.BoundsCenter, shape.LocalRotation),
shape.Radius));
floodIfOriginDiscarded.Add((shape.LocalPosition, shape.Radius));
}
if (flood.Count > mostBspShapesOnOneSetup)
mostBspShapesOnOneSetup = flood.Count;
float Shortfall(List<(Vector3 Centre, float Radius)> spheres)
{
float worst = 0f;
foreach ((Vector3 tc, float tr) in truth)
{
float best = float.MaxValue;
foreach ((Vector3 fc, float fr) in spheres)
{
float need = (tc - fc).Length() + tr - fr;
if (need < best) best = need;
}
if (best > worst) worst = best;
}
return worst;
}
const float Tolerance = 1e-3f;
float shortfall = Shortfall(flood);
if (shortfall > Tolerance)
{
uncontained.Add(id);
if (shortfall > worstShortfall)
{
worstShortfall = shortfall;
worstShortfallSetup = id;
}
}
if ((hasCylinder || (setup.CylSpheres.Count == 0 && hasSphere))
&& Shortfall(floodIfOriginDiscarded) > Tolerance)
{
wouldFailIfOriginDiscarded++;
}
}
// Population + defect controls first.
Assert.Equal(ExpectedPhysicsBspParts, bspParts);
Assert.Equal(ExpectedOffCentreParts, offCentreParts);
Assert.Equal(ExpectedAffected, affected);
Assert.Equal(ExpectedWouldFailIfOriginDiscarded, wouldFailIfOriginDiscarded);
// Cap control: the deepest installed part array is 49 BSP shapes, so
// the containment claim genuinely covers Setups past the retired
// 10-sphere clamp rather than stopping short of them.
Assert.Equal(ExpectedDeepestBspPartArray, mostBspShapesOnOneSetup);
// The fact.
Assert.True(
uncontained.Count == 0,
$"{uncontained.Count} Setups flood from a sphere that does not contain "
+ $"their own physics-BSP bounding sphere; worst shortfall "
+ $"{worstShortfall.ToString("F3", CultureInfo.InvariantCulture)} m on "
+ $"Setup 0x{worstShortfallSetup:X8}.");
}
} }

View file

@ -248,10 +248,26 @@ public class ShadowObjectRegistryMultiPartTests
Scale: 1f, CollisionType: ShadowCollisionType.Cylinder, Scale: 1f, CollisionType: ShadowCollisionType.Cylinder,
Radius: radius, CylHeight: radius * 2f); Radius: radius, CylHeight: radius * 2f);
private static ShadowShape Bsp(float radius) => new( /// <summary>
GfxObjId: 0x010044B5u, LocalPosition: Vector3.Zero, LocalRotation: Quaternion.Identity, /// A physics-BSP part shape. <paramref name="boundsCenter"/> defaults
/// OFF-CENTRE because that is the DAT-real configuration: a GfxObj's
/// physics BSP is authored in the GfxObj's own coordinates and 376 of the
/// 973 installed physics-BSP parts have a root bounding sphere further
/// from the part origin than half their own radius. A fixture pinned at
/// <c>Vector3.Zero</c> is the one configuration in which discarding the
/// centre is invisible.
/// </summary>
private static ShadowShape Bsp(
float radius,
Vector3 boundsCenter = default,
Vector3 localPosition = default,
Quaternion localRotation = default) => new(
GfxObjId: 0x010044B5u,
LocalPosition: localPosition,
LocalRotation: localRotation == default ? Quaternion.Identity : localRotation,
Scale: 1f, CollisionType: ShadowCollisionType.BSP, Scale: 1f, CollisionType: ShadowCollisionType.BSP,
Radius: radius, CylHeight: 0f); Radius: radius, CylHeight: 0f,
BoundsCenter: boundsCenter == default ? new Vector3(0f, 6f, 0f) : boundsCenter);
private static List<uint> FloodCellsFor(params ShadowShape[] shapes) private static List<uint> FloodCellsFor(params ShadowShape[] shapes)
{ {
@ -272,6 +288,7 @@ public class ShadowObjectRegistryMultiPartTests
List<uint> bspOnly = FloodCellsFor(Bsp(14f)); List<uint> bspOnly = FloodCellsFor(Bsp(14f));
List<uint> mixed = FloodCellsFor(Cyl(0.5f), Bsp(14f)); List<uint> mixed = FloodCellsFor(Cyl(0.5f), Bsp(14f));
// Controls: the two footprints must actually differ, or the fact below // Controls: the two footprints must actually differ, or the fact below
// is satisfiable by any dispatch rule at all. // is satisfiable by any dispatch rule at all.
Assert.Equal([LbId | 10u], cylinderOnly); Assert.Equal([LbId | 10u], cylinderOnly);
@ -284,6 +301,116 @@ public class ShadowObjectRegistryMultiPartTests
Assert.NotEqual(cylinderOnly, mixed); Assert.NotEqual(cylinderOnly, mixed);
} }
/// <summary>
/// AP-156. Retail's per-part cross-cell walk transforms the BSP root
/// bounding sphere's CENTRE through the part's own frame before it uses
/// the radius: <c>CGfxObj::physics_sphere</c> is
/// <c>BSPTREE::GetSphere(physics_bsp)</c> @0x005397e0 (root
/// <c>BSPNODE</c> + 4, past its vftable), and
/// <c>CEnvCell::find_transit_cells</c> @0x0052cae0 — the part-array
/// overload reached from <c>find_bbox_cell_list</c> @0x00510fc0 through
/// <c>CPartArray::calc_cross_cells_static</c> @0x00518160's
/// <c>[vtbl+0x7c]</c> — loads it at <c>0x0052cb36 mov esi,[ecx+0x74]</c>,
/// runs <c>Position::localtolocal</c> on it against
/// <c>[part+0x30]</c> (<c>0x0052cb4c add eax,0x30</c>), and only then
/// reads <c>[esi+0xc]</c> for the radius.
///
/// <para>
/// acdream used to flood from the part ORIGIN with the sphere's radius.
/// Over the installed DAT that failed to contain the object's own BSP
/// sphere for 170 of the 172 AP-152 Setups, shortfall up to 9.911 m
/// (0x02000255) — under-inclusive membership, the #98 / #168 class.
/// </para>
/// </summary>
[Fact]
public void BuildFloodSpheres_BspShape_CentresOnTheBoundsCentreNotThePartOrigin()
{
// Same sphere, three placements of the SAME 6 m radius:
// a) part origin at (0,0,0), bounds centre at the origin too;
// b) part origin at (0,0,0), bounds centre 30 m along +Y;
// c) part origin 30 m along +Y, bounds centre at the origin.
// (b) and (c) describe the identical world sphere, so they must flood
// the identical cells — and neither may equal (a).
List<uint> concentric = FloodCellsFor(
Bsp(6f, boundsCenter: new Vector3(0.001f, 0f, 0f)));
List<uint> viaBoundsCentre = FloodCellsFor(
Bsp(6f, boundsCenter: new Vector3(0f, 30f, 0f)));
List<uint> viaPartOrigin = FloodCellsFor(
Bsp(6f,
boundsCenter: new Vector3(0.001f, 0f, 0f),
localPosition: new Vector3(0f, 30f, 0f)));
// Control: the offset must actually move the footprint, or the
// equality below is satisfiable by ignoring BoundsCenter entirely.
Assert.NotEqual(concentric, viaBoundsCentre);
Assert.Equal(viaPartOrigin, viaBoundsCentre);
}
/// <summary>
/// Retail's 10-sphere clamp is on the CYLSPHERE branch and nowhere else.
/// <c>CObjCell::find_cell_list</c> @0x0052b9f0 clamps the cylsphere count
/// (<c>0x0052ba21 cmp eax,0xa</c> / <c>0x0052ba28 mov ebp,0xa</c>); the
/// BSP branch — <c>find_bbox_cell_list</c> @0x00510fc0 →
/// <c>CPartArray::calc_cross_cells_static</c> @0x00518160 →
/// <c>CEnvCell::find_transit_cells</c> @0x0052cae0 — walks every part with
/// no cap, and the sorting-sphere overload @0x0052b990 takes one sphere.
/// 7 installed Setups carry more than 10 physics-BSP parts (max 49,
/// Setup 0x02001A91) and landblock-baked stair runs and fences routinely
/// do, so clamping the BSP branch dropped their tail parts out of the
/// flood entirely.
/// </summary>
[Fact]
public void BuildFloodSpheres_CapsCylSpheresAtTenButNeverTheBspParts()
{
// Ten shapes on the owner's own cell, then an eleventh 72 m north.
var near = new Vector3(0f, 0f, 0f);
var far = new Vector3(0f, 72f, 0f);
uint ownCell = LbId | (uint)(1 * 8 + 1 + 1); // (x=1, y=1)
uint farCell = LbId | (uint)(1 * 8 + 4 + 1); // (x=1, y=4)
var bsp = new ShadowShape[11];
for (int i = 0; i < 10; i++)
bsp[i] = Bsp(1f, boundsCenter: new Vector3(0.001f, 0f, 0f));
bsp[10] = Bsp(1f, boundsCenter: far);
List<uint> bspCells = FloodCellsFor(bsp);
var cyls = new ShadowShape[11];
for (int i = 0; i < 10; i++)
cyls[i] = Cyl(1f) with { LocalPosition = near };
cyls[10] = Cyl(1f) with { LocalPosition = far };
List<uint> cylCells = FloodCellsFor(cyls);
Assert.Contains(ownCell, bspCells);
Assert.Contains(ownCell, cylCells);
// The eleventh BSP part still floods; the eleventh cylsphere does not.
Assert.Contains(farCell, bspCells);
Assert.DoesNotContain(farCell, cylCells);
}
/// <summary>
/// The BoundsCentre is expressed in the SHAPE's own frame, so the part's
/// LocalRotation must carry it — exactly as retail transforms the sphere
/// centre through <c>[part+0x30]</c>, the part's full Position, not just
/// its origin. A +Y offset on a part yawed 180 degrees must land at -Y.
/// </summary>
[Fact]
public void BuildFloodSpheres_BspShape_RotatesTheBoundsCentreByThePartRotation()
{
Quaternion yaw180 = Quaternion.CreateFromAxisAngle(Vector3.UnitZ, MathF.PI);
List<uint> yawed = FloodCellsFor(
Bsp(6f, boundsCenter: new Vector3(0f, 20f, 0f), localRotation: yaw180));
List<uint> negatedUnrotated = FloodCellsFor(
Bsp(6f, boundsCenter: new Vector3(0f, -20f, 0f)));
List<uint> unrotated = FloodCellsFor(
Bsp(6f, boundsCenter: new Vector3(0f, 20f, 0f)));
Assert.NotEqual(unrotated, negatedUnrotated); // control
Assert.Equal(negatedUnrotated, yawed);
}
/// <summary> /// <summary>
/// The AP-152 delta end-to-end: a CylSphere+BSP Setup (73 of the 172 /// The AP-152 delta end-to-end: a CylSphere+BSP Setup (73 of the 172
/// affected installed Setups are this shape) registered through the /// affected installed Setups are this shape) registered through the
@ -291,6 +418,14 @@ public class ShadowObjectRegistryMultiPartTests
/// BuildFloodSpheres' cylinder preference confined the owner to the /// BuildFloodSpheres' cylinder preference confined the owner to the
/// cylinder's cell while its slab BSP reached further — an object absent /// cylinder's cell while its slab BSP reached further — an object absent
/// from shadow cells it physically occupies, the #98 / #168 symptom class. /// from shadow cells it physically occupies, the #98 / #168 symptom class.
///
/// <para>
/// The BSP bounds are supplied through the production seam
/// (<c>physicsBspBounds</c>, the same resolver
/// <c>LiveEntityCollisionBuilder</c> passes) and are OFF-CENTRE, so the
/// flood is asserted where the geometry is rather than where the part
/// origin is.
/// </para>
/// </summary> /// </summary>
[Fact] [Fact]
public void FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint() public void FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint()
@ -303,17 +438,20 @@ public class ShadowObjectRegistryMultiPartTests
{ Radius = 0.5f, Height = 1f, Origin = Vector3.Zero } }, { Radius = 0.5f, Height = 1f, Origin = Vector3.Zero } },
}; };
IReadOnlyList<ShadowShape> raw = // 14 m stands in for a slab wide enough to leave its own landcell;
ShadowShapeBuilder.FromSetup(setup, entScale: 1f, hasPhysicsBsp: id => id == part); // the +18 m Y offset stands in for the 376-of-973 installed parts
// Production substitutes the real BSP bounding radius at registration // whose root sphere is nowhere near the part origin.
// time (LiveEntityCollisionBuilder.Build); 14 m stands in for a slab var bounds = new FlatCollisionSphere(new Vector3(0f, 18f, 0f), 14f);
// wide enough to leave its own landcell. IReadOnlyList<ShadowShape> shapes = ShadowShapeBuilder.FromSetup(
var shapes = raw.Select(s => s.CollisionType == ShadowCollisionType.BSP setup,
? s with { Radius = 14f } entScale: 1f,
: s).ToList(); hasPhysicsBsp: id => id == part,
physicsBspBounds: id => id == part ? bounds : null);
ShadowShape only = Assert.Single(shapes); ShadowShape only = Assert.Single(shapes);
Assert.Equal(ShadowCollisionType.BSP, only.CollisionType); Assert.Equal(ShadowCollisionType.BSP, only.CollisionType);
Assert.Equal(14f, only.Radius);
Assert.Equal(new Vector3(0f, 18f, 0f), only.BoundsCenter);
var reg = new ShadowObjectRegistry(); var reg = new ShadowObjectRegistry();
const uint ownerId = 0xBEEF02u; const uint ownerId = 0xBEEF02u;
@ -322,8 +460,13 @@ public class ShadowObjectRegistryMultiPartTests
shapes, 0x10008u, EntityCollisionFlags.None, OffX, OffY, LbId); shapes, 0x10008u, EntityCollisionFlags.None, OffX, OffY, LbId);
List<uint> cells = OutdoorCellsHolding(reg, ownerId); List<uint> cells = OutdoorCellsHolding(reg, ownerId);
Assert.Contains(LbId | 10u, cells);
Assert.True(cells.Count > 1, Assert.True(cells.Count > 1,
$"Expected the slab footprint to span more than its own landcell; got {cells.Count}"); $"Expected the slab footprint to span more than its own landcell; got {cells.Count}");
// Landcells are 24 m. The slab's sphere is centred at y = 54 m with
// r = 14 m, so it spans y in [40, 68] — rows 1 and 2, and it comes no
// closer than 16 m to row 0 (y < 24). Flooding it around the PART
// ORIGIN instead (y = 36 m, span [22, 50]) would drag row 0 in.
Assert.Contains(LbId | (uint)(1 * 8 + 2 + 1), cells); // (x=1, y=2)
Assert.DoesNotContain(LbId | (uint)(1 * 8 + 0 + 1), cells); // (x=1, y=0)
} }
} }

View file

@ -1,4 +1,5 @@
using System.Collections.Generic; using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq; using System.Linq;
using System.Numerics; using System.Numerics;
using AcDream.Core.Physics; using AcDream.Core.Physics;
@ -99,19 +100,43 @@ public class ShadowRegistrationOverflowTests
// ── The builder: one BSP shape per BSP part; shells + no-BSP excluded ── // ── The builder: one BSP shape per BSP part; shells + no-BSP excluded ──
private static GfxObjPhysics BspGfx(float radius) /// <summary>
/// Graph-form fixture. The bounding sphere is OFF-CENTRE by default
/// because that is the DAT-real case — a GfxObj's physics BSP is authored
/// in the GfxObj's own coordinates and 376 of the 973 installed
/// physics-BSP parts have a root sphere further from the part origin than
/// half their radius.
/// </summary>
private static GfxObjPhysics BspGfx(float radius, float centerZ = 0.75f)
{ {
var leaf = new PhysicsBSPNode { Type = BSPNodeType.Leaf }; var leaf = new PhysicsBSPNode { Type = BSPNodeType.Leaf };
return new GfxObjPhysics return new GfxObjPhysics
{ {
BSP = new PhysicsBSPTree { Root = leaf }, BSP = new PhysicsBSPTree { Root = leaf },
BoundingSphere = new Sphere { Origin = Vector3.Zero, Radius = radius }, BoundingSphere = new Sphere
{ Origin = new Vector3(0f, 0f, centerZ), Radius = radius },
Resolved = new Dictionary<ushort, ResolvedPolygon>(), Resolved = new Dictionary<ushort, ResolvedPolygon>(),
PhysicsPolygons = new Dictionary<ushort, Polygon>(), PhysicsPolygons = new Dictionary<ushort, Polygon>(),
Vertices = new VertexArray(), Vertices = new VertexArray(),
}; };
} }
/// <summary>Flat-form fixture — the production storage since I6/I7.</summary>
private static GfxObjPhysics FlatBspGfx(float radius, float centerZ)
{
var node = new FlatPhysicsBspNode(
BSPNodeType.Leaf, default, -1, -1, 0, 0,
new FlatCollisionSphere(new Vector3(0f, 0f, centerZ), radius),
new FlatIndexRange(0, 0));
GfxObjPhysics phys = BspGfx(radius, centerZ);
phys.FlatPhysicsBsp = new FlatPhysicsBsp(
0,
ImmutableArray.Create(node),
ImmutableArray<int>.Empty,
FlatPolygonTable.Empty);
return phys;
}
[Fact] [Fact]
public void FromLandblockBspParts_OneShapePerBspPart_LocalTransformPreserved() public void FromLandblockBspParts_OneShapePerBspPart_LocalTransformPreserved()
{ {
@ -136,6 +161,42 @@ public class ShadowRegistrationOverflowTests
Assert.All(shapes, s => Assert.Equal(1.05f, s.Radius, 3)); Assert.All(shapes, s => Assert.Equal(1.05f, s.Radius, 3));
} }
/// <summary>
/// AP-156, landblock half. A landblock-baked multi-part entity is the
/// same <c>CPartArray</c> walk as a live Setup
/// (<c>CPhysicsObj::find_bbox_cell_list</c> @0x00510fc0 →
/// <c>CPartArray::calc_cross_cells_static</c> @0x00518160 →
/// <c>CEnvCell::find_transit_cells</c> @0x0052cae0), which transforms each
/// part's <c>CGfxObj::physics_sphere</c> CENTRE through the part's own
/// Position before reading its radius. Carrying the radius alone puts a
/// stair run's or fence's flood sphere at the part origin instead of on
/// its geometry. Both storage forms — the flat BSP that production uses
/// and the graph fallback — must supply the centre, SCALED with the part.
/// </summary>
[Fact]
public void FromLandblockBspParts_CarriesTheScaledRootSphereCentre()
{
Matrix4x4 halfScale = Matrix4x4.CreateScale(0.5f)
* Matrix4x4.CreateTranslation(0f, 2f, 0f);
var flat = ShadowShapeBuilder.FromLandblockBspParts(
[new MeshRef(0x01000AC5u, halfScale)],
isBuildingShell: false,
getGfxObj: _ => FlatBspGfx(4f, centerZ: 3f));
ShadowShape flatShape = Assert.Single(flat);
Assert.Equal(0.5f, flatShape.Scale, 3);
Assert.Equal(2f, flatShape.Radius, 3); // 4 * 0.5
Assert.Equal(new Vector3(0f, 0f, 1.5f), flatShape.BoundsCenter); // 3 * 0.5
// Graph fallback (fixtures without a flat BSP) takes the same path.
var graph = ShadowShapeBuilder.FromLandblockBspParts(
[new MeshRef(0x01000AC5u, halfScale)],
isBuildingShell: false,
getGfxObj: _ => BspGfx(4f, centerZ: 3f));
ShadowShape graphShape = Assert.Single(graph);
Assert.Equal(new Vector3(0f, 0f, 1.5f), graphShape.BoundsCenter);
}
[Fact] [Fact]
public void FromLandblockBspParts_BuildingShell_ReturnsEmpty() public void FromLandblockBspParts_BuildingShell_ReturnsEmpty()
{ {