From e6457cc84987211432e2b5549b6c69e70bb1b516 Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 6 Aug 2026 16:44:48 +0200 Subject: [PATCH] =?UTF-8?q?fix(physics):=20close=20the=20AP-156=20fix=20re?= =?UTF-8?q?view=20=E2=80=94=20real=20containment=20oracle,=20type-level=20?= =?UTF-8?q?invariant,=20AP-158?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both review lenses PASSED; this is the cleanup, not a rescue. Evidence: docs/research/2026-08-06-ap156-review-closure.md (the review itself is committed alongside it as the received artifact). R1 — the load-bearing containment test could not fail. Its truth and flood values were two hand-copies of the same expression over the same part set, so the shortfall was algebraically identically zero for any DAT input. The oracle is now PHYSICS-POLYGON VERTICES — a different DAT field from the bounding sphere the builder emits, so the two sides can genuinely disagree. Sabotage-verified three ways after full cleans: dropping the bounds centre in production reddens it (428 Setups, worst 35.869 m on 0x0200129A, matching an independent out-of-repo sweep exactly); dropping only the scale on the centre reddens it (326); and corrupting the TEST's own bounds oracle reddens it (467) where under the shipped oracle that same corruption was invisible by algebra. Renamed accordingly. A6's stale "cap control" comment corrected: that loop is the test's own uncapped re-implementation and cannot observe a cap regression — the cap is covered in Core. R2 — the population was understated. 172 is AP-152's DISPATCH population; AP-156's is 530 BSP-bearing Setups, of which 525 have a flood sphere move and 428 fail vertex containment before the fix (412 at a 1 cm tolerance — the review's figure; the gap is 16 Setups between 1.4 mm and 10 mm, real geometry). 0 fail after, at any tolerance down to zero. Corrected in the AP-156 row, the section-3 header, the C5c handoff and two test docstrings. Dated review artifacts are left as written — "170 of 172" was correct for what they measured, and rewriting evidence to match a later measurement loses provenance. A1 — BoundsCenter = default reopened at the type what the commit closed at the seam. Dropping the default alone would NOT have closed the review's own scenario (a copied Cylinder call site would write Vector3.Zero explicitly and stay green), so ShadowShape's constructor is now private and BSP shapes are built only through ShadowShape.Bsp(..., FlatCollisionSphere localBounds), which takes radius and centre as ONE value and scales them together. There is no expression a caller can write that carries one and drops the other. 22 construction sites converted; the same sabotage now reddens 5 Core tests where the review's sabotage A reached 4, because both BSP producers share one scaling path. A2 — #333 is real and bigger than filed, and its retail question is answered. I disassembled CObjCell::find_obj_collisions @0x0052b750 from the PDB-paired binary myself (check_exe_pdb.py MATCH) rather than inheriting the claim: its only early-out is sphere_path.insert_type == INITIAL_PLACEMENT_ INSERT, then it calls FindObjCollisions on every unparented non-self shadow object UNCONDITIONALLY. Retail has NO distance pre-filter, so acdream's "+ movement + 2f" reach filter is an invention with no register row — filed as AP-158, carrying the disassembly, the F_EPSILON = 0.0002 m contrast, and the measured blast radius (118 of 477 unique installed physics-BSP GfxObjs exceed its ~2.5 m budget, 46 exceed 5 m). Active AP rows 109 -> 110. Recorded prominently in three places a reader will hit: TALL PROPS MAY SHOW NO VISIBLE CHANGE UNTIL #333 LANDS, and a null result at the connected gate is EXPECTED, not evidence against AP-156. LOW items. R3: the comment claiming the cited evidence justified the whole cap line is corrected, but int.MaxValue on the sorting-sphere branch stays — capping at 1 would take Spheres[0], and retail's one sphere is CSetup::sorting_sphere, a different DAT field; capping keeps the wrong field AND flips the substitution under-inclusive (#98/#168 direction). AP-157 already owns it. R4: acdream scales the flood sphere where retail's find_transit_cells never reads gfxobj_scale — added as a second residual on AP-156. R5: retail's slack constant carried into AP-158 and #333. A3: the per-call delegate allocation is back to a cached field, still derived from the single bounds resolver. A5: noted; b52967de's message cannot be amended. Gates: all 44 bin/obj deleted before every verdict-deciding build, each test run gated on a verified "Build succeeded" in the same invocation. Release build 0 errors / 21 pre-existing warnings. Complete suite 11,208 passed / 4 skipped / 0 failed — reconciles exactly with the e2b2d04c baseline; one test renamed, none added, removed or skipped. Nothing conflated with the known load-sensitive flakes #302 / #308 / #321. Co-Authored-By: Claude Opus 4.8 --- docs/ISSUES.md | 55 +- .../retail-divergence-register.md | 5 +- docs/research/2026-08-06-ap156-fix-review.md | 719 ++++++++++++++++++ .../2026-08-06-ap156-review-closure.md | 218 ++++++ .../2026-08-06-c5c-closeout-handoff.md | 23 + .../Physics/LiveEntityCollisionBuilder.cs | 9 +- .../Streaming/LandblockPhysicsPublisher.cs | 30 +- .../LandblockPhysicsContentBuilder.cs | 6 +- .../Physics/ShadowObjectRegistry.cs | 30 +- src/AcDream.Core/Physics/ShadowShape.cs | 197 +++-- .../Physics/ShadowShapeBuilder.cs | 88 +-- .../LiveEntityCollisionBuilderTests.cs | 6 +- .../Physics/RemotePhysicsUpdaterTests.cs | 9 +- ...tailStaticAnimatingObjectSchedulerTests.cs | 9 +- .../World/LiveEntityRuntimeTests.cs | 18 +- ...InstalledSetupBspPrimitiveDispatchTests.cs | 148 ++-- .../Physics/DoorBugTrajectoryReplayTests.cs | 36 +- .../ShadowObjectRegistryMultiPartTests.cs | 110 ++- .../Physics/ShadowObjectRegistryTests.cs | 15 +- .../ShadowRegistrationOverflowTests.cs | 6 +- .../Physics/ShadowSetPositionCommitTests.cs | 15 +- 21 files changed, 1454 insertions(+), 298 deletions(-) create mode 100644 docs/research/2026-08-06-ap156-fix-review.md create mode 100644 docs/research/2026-08-06-ap156-review-closure.md diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 9e33e213..d991f721 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -31,8 +31,50 @@ What does NOT go here: downstream of the AP-156 membership fix, so that fix alone may not be enough to make the worst objects block. **Filed:** 2026-08-06 at the AP-156 fix (commit `b52967de`), which surfaced it. +**Updated 2026-08-06** at the AP-156 fix review: the retail question below is +now ANSWERED, and the filter has its own divergence row, **AP-158**. **Do NOT bundle with AP-156.** Different code path (collision query, not cell -membership), and it needs its own retail question answered first. +membership). + +### ANSWERED — retail has no distance pre-filter at all + +Disassembled from the PDB-paired binary (`C:\Users\erikn\Downloads\acclient.exe`, +`check_exe_pdb.py` → MATCH, CodeView GUID `9e847e2f-777c-4bd9-886c-22256bb87f32`) +for this update, not inherited from Binary Ninja: + +``` +CObjCell::find_obj_collisions @0x0052b750 + 0x0052b759 cmp dword [ebx+0x174], 2 ; sphere_path.insert_type + 0x0052b765 je 0x52b7a0 ; INITIAL_PLACEMENT_INSERT -> return OK_TS + 0x0052b773 mov ecx,[edi+0xc8] ; shadow_object_list.data + 0x0052b77f mov edx,[ecx+0x40] ; physobj->parent + 0x0052b784 jne 0x52b795 ; parented -> skip + 0x0052b786 cmp ecx,[ebx] ; physobj == mover? + 0x0052b788 je 0x52b795 ; self -> skip + 0x0052b78b call 0x50f050 ; CPhysicsObj::FindObjCollisions — UNCONDITIONAL + 0x0052b79e jb 0x52b773 ; loop, bound = [edi+0xc4] +``` + +Agrees with `acclient_2013_pseudo_c.txt:308916-308940`. **There is no distance +test in the function.** So the `+ 2f` slack and the `movement.Length()` term are +acdream's own invention, which is why AP-158 exists. For scale: retail's own +cross-cell slack constant is `F_EPSILON` = `1.9999999e-4` — 0.2 mm, read at +`0x0052cb5f fld dword [0x7c8c70]` — not 2 m. + +### Measured blast radius + +Over the installed `client_portal.dat`, by an independent scratch sweep outside +the repo: **118 of the 477** unique physics-BSP GfxObjs have a root-sphere +offset above the filter's roughly 2.5 m walking budget, and **46** above 5 m. At +a test scale of 1.75 those offsets become 4.4 m and 8.75 m against an unchanged +budget. + +### Consequence for the AP-156 connected gate — read this before running it + +**Tall props may show NO VISIBLE CHANGE at all until this issue is fixed, and a +null result there is EXPECTED rather than evidence against AP-156.** AP-156 puts +the geometry into the correct cell; this filter then discards it one layer down, +for exactly the largest-offset objects AP-156's commit body points the user at. ### The mechanism @@ -78,11 +120,12 @@ finds a tall prop that still does not block after AP-156, look here first. ### What to establish before fixing -1. Does retail have this pre-filter at all? `CPhysicsObj::FindObjCollisions` - @0x0050f050 walks the cell's object list and dispatches per object; the - `+ 2f` slack and the `movement.Length()` term look like acdream's own - broadphase rather than a port. If it is acdream's, it also needs a - divergence-register row, which it does not currently have. +1. ~~Does retail have this pre-filter at all?~~ **ANSWERED above: no.** The + register row is filed as **AP-158**. What remains open is a judgement call, + not a research question: keep the filter as a deliberate optimisation with a + correct measurement point, or delete it and walk the list as retail does. + Deleting it is the retail-faithful option and should be costed first — + `ShadowEntrySnapshot.Capture` already bounds the per-cell list. 2. If it is kept, it must measure from where the geometry is: `ShadowEntry` needs the `BoundsCenter` that `ShadowShape` now carries, and `deltaToCurr` must be taken against `obj.Position + rotate(obj.BoundsCenter, obj.Rotation)`. diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 77ba77fe..0d49d97f 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -162,7 +162,7 @@ readiness/requeue adaptation. See --- -## 3. Documented approximation (AP) — 109 active rows (AP-155 NARROWED and AP-156/AP-157 filed 2026-08-06 at the AP-152 retail-conformance review. AP-155 bundled two divergences with different code paths, populations and gates under one id; its flood half is now AP-156, **with its direction corrected**. AP-155(b) recorded the BSP flood approximation as OVER-inclusive and used that direction as the reason the residual was safe to defer; measured over the installed DAT it was UNDER-inclusive for 170 of the 172 AP-152 Setups, worst shortfall 9.911 m, because `BuildFloodSpheres` carried each physics-BSP part's root bounding-sphere RADIUS while discarding that sphere's own ORIGIN and centring it on the part origin. That is the #98/#168 class, and for 43 Setups the post-AP-152 flood was strictly smaller than the pre-AP-152 one. AP-156 records the correction and the fix — `ShadowShape.BoundsCenter`, filled from the same resolver that supplies the radius, plus the retirement of the 10-sphere clamp on a branch where retail has none — and keeps open only the sphere-vs-portal TRAVERSAL approximation. AP-157 is the previously unregistered third-branch substitution: retail floods from one `CPartArray::GetSortingSphere` where acdream floods from every Sphere shape, and acdream's cylinder flood ignores `CylHeight`. AP-152 RETIRED 2026-08-06, one day after it was filed: `ShadowShapeBuilder.FromSetup` now dispatches BSP-first instead of unioning, and `ShadowObjectRegistry.BuildFloodSpheres` now applies `calc_cross_cells`' own BSP → cylsphere → sorting-sphere order. Four statements in the row were false and are corrected in its retirement text — most importantly its predicted symptom, "catching on a doorway sill", which could not have been occurring: `Transition.BspOnlyDispatch` had already made the extra primitive inert at collision-query time since 2026-05-25. The live half was CELL MEMBERSHIP, the #98/#168 symptom class, which had no such guard. AP-153/AP-154/AP-155 filed at that retirement — retail's dispatch flag is cached once at part-array construction where acdream's gate is live [AP-153]; acdream's query-time guard takes a CLIENT-DERIVED flag off the WIRE and never derives it, an undeclared dependency on ACE reading the same DAT bit [AP-154]; and the static publication paths emit a Setup Sphere as a height-capped Cylinder while `BuildFloodSpheres` approximates retail's bounding BOX with bounding SPHERES [AP-155, whose flood-priority half is closed by the same commit]. AP-152 filed 2026-08-06 at the AP-22 retirement — the LIVE collision path emits Setup primitives and per-part physics-BSP shapes additively where retail's `CPhysicsObj::FindObjCollisions` dispatches exclusively; 172 of 5,935 installed Setups are affected, including BSP doors, so it needs its own visual gate and was deliberately not folded into the AP-22 commit; the count is unchanged because AP-22 retired in the same commit. AP-22 RETIRED 2026-08-06 — retail synthesizes no shape for a shapeless object (`CPhysicsObj::FindObjCollisions` 0x0050f050 exits at `0x0050f22f je 0x50f31b` returning the seeded OK_TS, and `CPartArray::GetRadius`/`GetHeight` are absent from its whole call set), so the invented `setup.Radius` cylinder was deleted rather than re-derived; the row's site list named one file that never contained the fallback and omitted the two that did, one of them the headless-only copy, and its "rare decorative props" risk described an unreachable branch — 0 of 5,935 installed Setups can satisfy the guard. AP-150/AP-151 filed 2026-08-06 at the #280 dual review — the wait cue's five-second arming is acdream's own and not retail's trigger [AP-150], and the reveal gate is materially stricter than retail's DAT-residency prefetch predicate on the mesh-build/GPU-upload axis [AP-151], the opposite asymmetry from AP-149; AP-149 filed 2026-08-05 at the #280 portal-prefetch fix — the reveal gate's outer ring accepts terrain-only publication where retail requires LandBlockInfo and every building EnvCell; the fix closes the reveal-window/visible-window ratio, not this residual; AP-148 filed 2026-08-05 at the C5b closeout — acdream's local-player Gate A requires the wire TELEPORT_TS to be EQUAL where retail requires only that it not be OLDER, verified by disassembly against the PDB-paired binary after two review rounds read the Binary Ninja tautology and missed it; AP-147 filed 2026-08-05 at the C5b architecture review, finding D3 — the accepted-Position delta stream's cardinality change and its torn intermediate; AP-138 amended at the same review — C5b staled its route-2 first-submit `CurrentCellId` measurement; AP-131 RETIRED 2026-08-05, C5b, closing #275 — the steady-state merge's `installPlacementFrame: true, clearParent: true` literals no longer exist; `InboundPhysicsStateController.TryApplyPosition` now computes both flags PRE-MERGE from `(disposition, hasAnimations(old))`, which is exactly `RuntimeAuthoritativePositionRouteClassifier.ClassifyAcceptedPosition`'s own `ApplyPlacementFrameBeforeRouting`/`UnparentBeforeRouting` rows (false/false on the Gate A force row, `!HasAnimations`/true on every accepted non-force route). Retail decides both writes BEFORE `MoveOrTeleport` is consulted — Gate A @0x0045400C returns @0x0045409D ahead of `unset_parent` @0x00454129 and the `HasAnims` `SetPlacementFrame` gate @0x00454137 — so the flags need no route, no player distance and no signature change. The row's predicted symptoms are gone: an animated entity's ordinary Position no longer installs a placement frame retail skips, and a ForcePosition no longer unparents. Evidence: `InboundPhysicsStateControllerTests` — `ApplyOnAnimatedEntity_NeverInstallsTheWirePlacementFrame`, `ApplyOnNonAnimatedEntity_InstallsTheWirePlacementFrame`, `ForcePositionOnParentedLocalPlayer_RetainsTheParentAttachment`, and the 12-row `MergedPrePlacementFieldsMatchTheClassifiedRouteFlags` matrix which uses the production classifier as its oracle rather than re-encoding the table; all four sabotage-verified in both directions. The row's "the legacy caller is deleted at the production cutover" framing was overtaken: the caller was CORRECTED, not deleted, and remains the only production Position wire caller; AP-145 RETIRED 2026-08-05, C5a commit 1, closing #318 — `TryPublishPlace` now publishes the local player's Place through `LocalPlayerShadowSynchronizer.SyncPose`, the same publisher ordinary per-tick movement uses, instead of a direct `LocalPlayerShadowState.Set` that never touched `PhysicsEngine.ShadowObjects`; AP-1 RETIRED 2026-08-05, C5a deletion sweep — `PhysicsEngine.Resolve`/`ResolvePlacement`/`HasCellSurface` deleted outright, zero production callers, so "production zero-delta routes remain on the legacy resolver" is now structurally false; AP-146 filed 2026-08-05, #319 fix — the local player's canonical cell is written only at login/inbound-Position/teleport, not per ordinary-movement tick as retail's SetPositionInternal does; #319's fix makes a player-parented child inherit exactly this coarseness, stale-but-equal to the parent, not a new staleness class; follow-up filed as issue #320; AP-144 filed 2026-08-05, C4 route 3 round 3 (R7) — the portal-arrival movement-event send reuses `UsePositionFromServer` (`autonomy_level != 2`) where retail's actual gate, `SendMovementEvent`, is `autonomy_level != 0`; the two agree everywhere except level 1, which no production caller can reach today; AP-142/AP-143 filed 2026-08-04, C4 route 7 — the parented-child single-field cell model (id/pointer collapse, zero-not-stale removal propagation, same-cell tick-loop subsumption) and the headless parent-realize drive's skipped holding-location validation; AP-141 filed 2026-08-04, C4 route 5, NARROWED 2026-08-04 at the round-2 delta review — the far-branch StopInterpolating clause was wrong for the adopted-body case (it is now ported there) and the row's language now distinguishes "never armed" from "never re-anchored"; CORRECTED 2026-08-04 at the round-3 delta review — the risk column's "would drag the body toward a stale anchor" claim was itself wrong (the leash anchor is write-only; `ConstraintManager::adjust_offset` only brakes, never pulls) and is retracted; every half remains test-gated only, since ACE never sends a missile UpdatePosition; AP-140 filed AND RETIRED 2026-08-04 — filed at the Bug B Opus review because the two accepted-Position routing gates read the client `Airborne` flag, i.e. walkability, where retail's free-flight predicate is CONTACT, and Bug B had just turned "in contact, not on walkable ground" from unreachable into ordinary; retired the same day by pointing both gates at `PhysicsBody.InContact`, retail's literal `transient_state & 1` test at `InterpolationManager::adjust_offset` @0x00555D52 (bit 0 = `CONTACT_TS`, acclient.h:3690), while leaving `Airborne` and all five of its `!Body.OnWalkable` writers untouched — the narrow shape the row itself pinned. A remote sliding on a steep face now interpolates as retail does instead of snapping at UpdatePosition cadence; AP-139 filed 2026-08-04, Bug B remote steep-contact slide — the interpolation-queue clear on the landing edge, carried over from the deleted hand-rolled remote landing block; AP-81 narrowed the same day by that fix, which retired its whole GRAVITY half; AP-87 annotated the same day — its predicted symptom was observed live and then fixed at the source, with the row's own thresholds and conditions deliberately unchanged; AP-138 filed 2026-08-04, C4 route 4b-2 dual Opus review, parts (1) and (2) rewritten the same day at the DELTA review — the far snap's refusable-placement residual: store_position only on the outcomes that never reached the engine, the two quiescence parks made restorable at the source, with the rollback gated on the cell it actually restores into, rather than refused by a pre-flight that structurally cannot see them, and the leash not armed through a superseded incarnation; AP-137 filed 2026-08-04, C4 route 4b-2 and rewritten the same day at that review, `teleport_hook`'s call list completed at the delta review — the acdream-only null/rejected/cell-less leftover arm, what the deleted duplicated 96 m/4 m constant pairs actually computed, and the vacuous headless satisfaction; AP-136 filed 2026-08-04, C4 route 4b-1 review, NARROWED 2026-08-04 at the C4 route 4b-2 delta review and AMENDED 2026-08-04 by the cancelled-park presentation rollback (the row's "restored visible" claim covered only the CANONICAL half; the presentation half was never rolled back, which left a parked-then-cancelled remote that stops moving invisible in the world AND absent from the radar for the rest of the session — a defect, now fixed by the `WithdrawalRestored` receipt, with the selection residual filed as AD-63) — a cancelled lost-cell park re-shows the entity where retail keeps it hidden until cell load, and the rollback's scope now covers the two placement-side quiescence parks whenever the cell it restores into is not itself quiescing — round 4 (2026-08-04) applies that same test a second time at RESTORE time, because a retained park's rollback lands a packet later; AP-135 filed 2026-08-03, C4 route 4a — the airborne no-op's retained acdream bookkeeping; the stated total was 2 rows stale before that filing and is now a literal count of this section; AP-130/AP-131/AP-132 filed 2026-08-02, continuation-executor slice; AP-5 retired 2026-07-31 at Campaign P Slice 2A — every successful `step_down` now performs retail's final `PLACEMENT_INSERT`; AP-3/AP-4 retired 2026-07-31 at Campaign P Slice 1B — `transitional_insert` and `edge_slide` now preserve retail's valid-contact early return and Branch-1-first order; AP-127 retired 2026-07-31 by #268 — the complete augmentation chain is shared by character UI and Runtime movement; AP-30 retired 2026-07-30 by the movement parity audit — retail Frame::is_equal genuinely uses the 0.0002 epsilon [byte-confirmed], so the row recorded a NON-divergence; acdream already matches; AP-129 narrowed 2026-07-30 at the P4 Opus review fix — `CanMoveInto`/`RestrictionDB::IsAllowedIn` are now ported and fed end-to-end (CreateObject HouseOwner/HouseRestrictions/Monarch tail fields + live `House_UpdateRestrictions 0x0248`, resolved through `PhysicsEngine.Objects`), retiring the original "CanMoveInto entirely unmodeled, unconditional fail-closed" gap the row described — the review was triggered by `RestrictionObjPrevalenceInspectionTests` showing 103,766 of 729,888 installed EnvCells (the whole housing estate) carry a baked `RestrictionObj`, so the unconditional fail-closed default would have locked every house for every player including its own owner; AP-10 retired 2026-07-30 at Campaign P Slice P4 — restored retail's 0.1 m dry-corner water sink-in, full suite green proving the sticky-bit no-regression argument; AP-71 retired same slice — `check_entry_restrictions` ported at the head of the indoor `FindEnvCollisions` branch, `CellPhysics.RestrictionObj` wired from the DAT-baked `EnvCell` field in both the dev and production caching paths; AP-128 filed 2026-07-30 at the P3 Opus review — PK-timer clock basis; AP-25 retired 2026-07-30 at Campaign P Slice P1 — the vitae/enchantment-aware run/jump skill chain; AP-7 retired 2026-07-30 at Campaign P Slice P2 — `calc_friction`'s threshold ported to retail's confirmed 0.25f; its still-open cos(10°)-vs-0.99999536f Sledding constant question moved to AD-55) +## 3. Documented approximation (AP) — 110 active rows (AP-158 filed 2026-08-06 at the AP-156 fix review — the shadow broadphase's `maxReach` distance pre-filter is acdream's own invention with NO retail counterpart, and it measures from the part origin, so it can discard a genuine contact for exactly the off-centre parts AP-156 just placed correctly; issue #333. AP-156 CORRECTED at the same review: its population was understated — 172 is AP-152's DISPATCH population, not AP-156's CONTAINMENT population. AP-155 NARROWED and AP-156/AP-157 filed 2026-08-06 at the AP-152 retail-conformance review. AP-155 bundled two divergences with different code paths, populations and gates under one id; its flood half is now AP-156, **with its direction corrected**. AP-155(b) recorded the BSP flood approximation as OVER-inclusive and used that direction as the reason the residual was safe to defer; measured over the installed DAT it was UNDER-inclusive for 428 of the 530 BSP-bearing Setups (the AP-156 fix review corrected the originally-recorded '170 of 172'), because `BuildFloodSpheres` carried each physics-BSP part's root bounding-sphere RADIUS while discarding that sphere's own ORIGIN and centring it on the part origin. That is the #98/#168 class, and for 43 Setups the post-AP-152 flood was strictly smaller than the pre-AP-152 one. AP-156 records the correction and the fix — `ShadowShape.BoundsCenter`, filled from the same resolver that supplies the radius, plus the retirement of the 10-sphere clamp on a branch where retail has none — and keeps open only the sphere-vs-portal TRAVERSAL approximation. AP-157 is the previously unregistered third-branch substitution: retail floods from one `CPartArray::GetSortingSphere` where acdream floods from every Sphere shape, and acdream's cylinder flood ignores `CylHeight`. AP-152 RETIRED 2026-08-06, one day after it was filed: `ShadowShapeBuilder.FromSetup` now dispatches BSP-first instead of unioning, and `ShadowObjectRegistry.BuildFloodSpheres` now applies `calc_cross_cells`' own BSP → cylsphere → sorting-sphere order. Four statements in the row were false and are corrected in its retirement text — most importantly its predicted symptom, "catching on a doorway sill", which could not have been occurring: `Transition.BspOnlyDispatch` had already made the extra primitive inert at collision-query time since 2026-05-25. The live half was CELL MEMBERSHIP, the #98/#168 symptom class, which had no such guard. AP-153/AP-154/AP-155 filed at that retirement — retail's dispatch flag is cached once at part-array construction where acdream's gate is live [AP-153]; acdream's query-time guard takes a CLIENT-DERIVED flag off the WIRE and never derives it, an undeclared dependency on ACE reading the same DAT bit [AP-154]; and the static publication paths emit a Setup Sphere as a height-capped Cylinder while `BuildFloodSpheres` approximates retail's bounding BOX with bounding SPHERES [AP-155, whose flood-priority half is closed by the same commit]. AP-152 filed 2026-08-06 at the AP-22 retirement — the LIVE collision path emits Setup primitives and per-part physics-BSP shapes additively where retail's `CPhysicsObj::FindObjCollisions` dispatches exclusively; 172 of 5,935 installed Setups are affected, including BSP doors, so it needs its own visual gate and was deliberately not folded into the AP-22 commit; the count is unchanged because AP-22 retired in the same commit. AP-22 RETIRED 2026-08-06 — retail synthesizes no shape for a shapeless object (`CPhysicsObj::FindObjCollisions` 0x0050f050 exits at `0x0050f22f je 0x50f31b` returning the seeded OK_TS, and `CPartArray::GetRadius`/`GetHeight` are absent from its whole call set), so the invented `setup.Radius` cylinder was deleted rather than re-derived; the row's site list named one file that never contained the fallback and omitted the two that did, one of them the headless-only copy, and its "rare decorative props" risk described an unreachable branch — 0 of 5,935 installed Setups can satisfy the guard. AP-150/AP-151 filed 2026-08-06 at the #280 dual review — the wait cue's five-second arming is acdream's own and not retail's trigger [AP-150], and the reveal gate is materially stricter than retail's DAT-residency prefetch predicate on the mesh-build/GPU-upload axis [AP-151], the opposite asymmetry from AP-149; AP-149 filed 2026-08-05 at the #280 portal-prefetch fix — the reveal gate's outer ring accepts terrain-only publication where retail requires LandBlockInfo and every building EnvCell; the fix closes the reveal-window/visible-window ratio, not this residual; AP-148 filed 2026-08-05 at the C5b closeout — acdream's local-player Gate A requires the wire TELEPORT_TS to be EQUAL where retail requires only that it not be OLDER, verified by disassembly against the PDB-paired binary after two review rounds read the Binary Ninja tautology and missed it; AP-147 filed 2026-08-05 at the C5b architecture review, finding D3 — the accepted-Position delta stream's cardinality change and its torn intermediate; AP-138 amended at the same review — C5b staled its route-2 first-submit `CurrentCellId` measurement; AP-131 RETIRED 2026-08-05, C5b, closing #275 — the steady-state merge's `installPlacementFrame: true, clearParent: true` literals no longer exist; `InboundPhysicsStateController.TryApplyPosition` now computes both flags PRE-MERGE from `(disposition, hasAnimations(old))`, which is exactly `RuntimeAuthoritativePositionRouteClassifier.ClassifyAcceptedPosition`'s own `ApplyPlacementFrameBeforeRouting`/`UnparentBeforeRouting` rows (false/false on the Gate A force row, `!HasAnimations`/true on every accepted non-force route). Retail decides both writes BEFORE `MoveOrTeleport` is consulted — Gate A @0x0045400C returns @0x0045409D ahead of `unset_parent` @0x00454129 and the `HasAnims` `SetPlacementFrame` gate @0x00454137 — so the flags need no route, no player distance and no signature change. The row's predicted symptoms are gone: an animated entity's ordinary Position no longer installs a placement frame retail skips, and a ForcePosition no longer unparents. Evidence: `InboundPhysicsStateControllerTests` — `ApplyOnAnimatedEntity_NeverInstallsTheWirePlacementFrame`, `ApplyOnNonAnimatedEntity_InstallsTheWirePlacementFrame`, `ForcePositionOnParentedLocalPlayer_RetainsTheParentAttachment`, and the 12-row `MergedPrePlacementFieldsMatchTheClassifiedRouteFlags` matrix which uses the production classifier as its oracle rather than re-encoding the table; all four sabotage-verified in both directions. The row's "the legacy caller is deleted at the production cutover" framing was overtaken: the caller was CORRECTED, not deleted, and remains the only production Position wire caller; AP-145 RETIRED 2026-08-05, C5a commit 1, closing #318 — `TryPublishPlace` now publishes the local player's Place through `LocalPlayerShadowSynchronizer.SyncPose`, the same publisher ordinary per-tick movement uses, instead of a direct `LocalPlayerShadowState.Set` that never touched `PhysicsEngine.ShadowObjects`; AP-1 RETIRED 2026-08-05, C5a deletion sweep — `PhysicsEngine.Resolve`/`ResolvePlacement`/`HasCellSurface` deleted outright, zero production callers, so "production zero-delta routes remain on the legacy resolver" is now structurally false; AP-146 filed 2026-08-05, #319 fix — the local player's canonical cell is written only at login/inbound-Position/teleport, not per ordinary-movement tick as retail's SetPositionInternal does; #319's fix makes a player-parented child inherit exactly this coarseness, stale-but-equal to the parent, not a new staleness class; follow-up filed as issue #320; AP-144 filed 2026-08-05, C4 route 3 round 3 (R7) — the portal-arrival movement-event send reuses `UsePositionFromServer` (`autonomy_level != 2`) where retail's actual gate, `SendMovementEvent`, is `autonomy_level != 0`; the two agree everywhere except level 1, which no production caller can reach today; AP-142/AP-143 filed 2026-08-04, C4 route 7 — the parented-child single-field cell model (id/pointer collapse, zero-not-stale removal propagation, same-cell tick-loop subsumption) and the headless parent-realize drive's skipped holding-location validation; AP-141 filed 2026-08-04, C4 route 5, NARROWED 2026-08-04 at the round-2 delta review — the far-branch StopInterpolating clause was wrong for the adopted-body case (it is now ported there) and the row's language now distinguishes "never armed" from "never re-anchored"; CORRECTED 2026-08-04 at the round-3 delta review — the risk column's "would drag the body toward a stale anchor" claim was itself wrong (the leash anchor is write-only; `ConstraintManager::adjust_offset` only brakes, never pulls) and is retracted; every half remains test-gated only, since ACE never sends a missile UpdatePosition; AP-140 filed AND RETIRED 2026-08-04 — filed at the Bug B Opus review because the two accepted-Position routing gates read the client `Airborne` flag, i.e. walkability, where retail's free-flight predicate is CONTACT, and Bug B had just turned "in contact, not on walkable ground" from unreachable into ordinary; retired the same day by pointing both gates at `PhysicsBody.InContact`, retail's literal `transient_state & 1` test at `InterpolationManager::adjust_offset` @0x00555D52 (bit 0 = `CONTACT_TS`, acclient.h:3690), while leaving `Airborne` and all five of its `!Body.OnWalkable` writers untouched — the narrow shape the row itself pinned. A remote sliding on a steep face now interpolates as retail does instead of snapping at UpdatePosition cadence; AP-139 filed 2026-08-04, Bug B remote steep-contact slide — the interpolation-queue clear on the landing edge, carried over from the deleted hand-rolled remote landing block; AP-81 narrowed the same day by that fix, which retired its whole GRAVITY half; AP-87 annotated the same day — its predicted symptom was observed live and then fixed at the source, with the row's own thresholds and conditions deliberately unchanged; AP-138 filed 2026-08-04, C4 route 4b-2 dual Opus review, parts (1) and (2) rewritten the same day at the DELTA review — the far snap's refusable-placement residual: store_position only on the outcomes that never reached the engine, the two quiescence parks made restorable at the source, with the rollback gated on the cell it actually restores into, rather than refused by a pre-flight that structurally cannot see them, and the leash not armed through a superseded incarnation; AP-137 filed 2026-08-04, C4 route 4b-2 and rewritten the same day at that review, `teleport_hook`'s call list completed at the delta review — the acdream-only null/rejected/cell-less leftover arm, what the deleted duplicated 96 m/4 m constant pairs actually computed, and the vacuous headless satisfaction; AP-136 filed 2026-08-04, C4 route 4b-1 review, NARROWED 2026-08-04 at the C4 route 4b-2 delta review and AMENDED 2026-08-04 by the cancelled-park presentation rollback (the row's "restored visible" claim covered only the CANONICAL half; the presentation half was never rolled back, which left a parked-then-cancelled remote that stops moving invisible in the world AND absent from the radar for the rest of the session — a defect, now fixed by the `WithdrawalRestored` receipt, with the selection residual filed as AD-63) — a cancelled lost-cell park re-shows the entity where retail keeps it hidden until cell load, and the rollback's scope now covers the two placement-side quiescence parks whenever the cell it restores into is not itself quiescing — round 4 (2026-08-04) applies that same test a second time at RESTORE time, because a retained park's rollback lands a packet later; AP-135 filed 2026-08-03, C4 route 4a — the airborne no-op's retained acdream bookkeeping; the stated total was 2 rows stale before that filing and is now a literal count of this section; AP-130/AP-131/AP-132 filed 2026-08-02, continuation-executor slice; AP-5 retired 2026-07-31 at Campaign P Slice 2A — every successful `step_down` now performs retail's final `PLACEMENT_INSERT`; AP-3/AP-4 retired 2026-07-31 at Campaign P Slice 1B — `transitional_insert` and `edge_slide` now preserve retail's valid-contact early return and Branch-1-first order; AP-127 retired 2026-07-31 by #268 — the complete augmentation chain is shared by character UI and Runtime movement; AP-30 retired 2026-07-30 by the movement parity audit — retail Frame::is_equal genuinely uses the 0.0002 epsilon [byte-confirmed], so the row recorded a NON-divergence; acdream already matches; AP-129 narrowed 2026-07-30 at the P4 Opus review fix — `CanMoveInto`/`RestrictionDB::IsAllowedIn` are now ported and fed end-to-end (CreateObject HouseOwner/HouseRestrictions/Monarch tail fields + live `House_UpdateRestrictions 0x0248`, resolved through `PhysicsEngine.Objects`), retiring the original "CanMoveInto entirely unmodeled, unconditional fail-closed" gap the row described — the review was triggered by `RestrictionObjPrevalenceInspectionTests` showing 103,766 of 729,888 installed EnvCells (the whole housing estate) carry a baked `RestrictionObj`, so the unconditional fail-closed default would have locked every house for every player including its own owner; AP-10 retired 2026-07-30 at Campaign P Slice P4 — restored retail's 0.1 m dry-corner water sink-in, full suite green proving the sticky-bit no-regression argument; AP-71 retired same slice — `check_entry_restrictions` ported at the head of the indoor `FindEnvCollisions` branch, `CellPhysics.RestrictionObj` wired from the DAT-baked `EnvCell` field in both the dev and production caching paths; AP-128 filed 2026-07-30 at the P3 Opus review — PK-timer clock basis; AP-25 retired 2026-07-30 at Campaign P Slice P1 — the vitae/enchantment-aware run/jump skill chain; AP-7 retired 2026-07-30 at Campaign P Slice P2 — `calc_friction`'s threshold ported to retail's confirmed 0.25f; its still-open cos(10°)-vs-0.99999536f Sledding constant question moved to AD-55) Wave-0 UI ledger repair (2026-07-10) retired stale AP-38, resolved the AP-84 collision, restored overwritten paperdoll rows as AP-92/AP-93, and registered @@ -183,8 +183,9 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps. | AP-153 | **Filed 2026-08-06 at the AP-152 retirement — a modelling difference the fix itself introduces.** Retail's shape-dispatch flag is CACHED ONCE. `CPartArray::CacheHasPhysicsBSP` @0x00518110 walks the part array, ORs 0x10000 into `CPartArray::pa_state` on the first part whose `gfxobj->physics_bsp` is non-null, and `CPhysicsObj::CacheHasPhysicsBSP` @0x0050f570 mirrors it onto `CPhysicsObj::state+0xa8`. A full `.text` scan for direct call/jmp to 0x0050f570 finds EXACTLY ONE caller, `CPhysicsObj::InitPartArrayObject+0x7e` @0x0051272e — so after an `AnimPartChanged` part swap retail's DISPATCH flag is stale while its per-part test (`CPhysicsPart::find_obj_collisions` @0x0050d8d0) stays live. acdream's step-0 gate is LIVE in both: it re-derives from the effective part identities on every `FromSetup` call. | `src/AcDream.Core/Physics/ShadowShapeBuilder.cs` (`FromSetup` step 0); `src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs` (`ReconcileAppearance`) | The two disagree only when a swap adds or removes the LAST physics-BSP part. Humanoid part swaps (clothing / armour) involve no physics-BSP GfxObj on either side, so this is unreachable against ACE today. Deliberately NOT modelled with cached state — that would be inventing staleness to reproduce a retail bug. | If a server ever swapped a prop's part array across the physics-BSP boundary, acdream would switch its collision geometry on the swap where retail would keep dispatching on the construction-time flag: a prop that gained a BSP part would lose its primitive immediately in acdream and only on re-init in retail. | `CPartArray::CacheHasPhysicsBSP` 0x00518110; `CPhysicsObj::CacheHasPhysicsBSP` 0x0050f570; sole caller `CPhysicsObj::InitPartArrayObject+0x7e` 0x0051272e | | AP-154 | **Filed 2026-08-06 at the AP-152 retirement (contract §11.6) — an undeclared dependency on a specific server implementation.** Retail COMPUTES `HAS_PHYSICS_BSP_PS` itself from its own part array (AP-153's anchors). acdream's query-time guard `Transition.BspOnlyDispatch` reads it out of the SERVER's wire `PhysicsState`: `LiveEntityCollisionBuilder.cs:161` copies `exactRecord.FinalPhysicsState` into `ShadowEntry.State`, and a repo-wide grep for `PhysicsStateFlags.HasPhysicsBsp` in `src/` returns only that predicate and one unrelated mover-state read. acdream never ORs the bit in client-side. It happens to be correct because ACE derives the same DAT bit (`WorldObject_Networking.cs:665-668` from `SetupFlags.HasPhysicsBSP`), overriding the weenie's authored value — which is why a 2018 weenie dump showing `PhysicsState = 0x8` for the cottage door does not contradict our own live capture of `0x10008`. | `src/AcDream.Core/Physics/TransitionTypes.cs:1348` (`BspOnlyDispatch`), call sites `:3911` / `:3954`; `src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs:161` | Narrowed, not closed, by the AP-152 fix: the shape list no longer contains a primitive for a BSP-bearing object, so the guard has nothing left to skip and the OUTCOME is now independent of the wire. The guard itself still keys on the wire. Not bundled — changing `registration.State` touches every consumer of `FinalPhysicsState` (Hidden, Missile, ethereal layer 2, the `[setstate]` log) and needs its own gate. | Against a server that does not derive the bit from the DAT, a BSP-bearing object built by a producer other than `FromSetup` would have its primitive tested where retail tests only the BSP. | `CPartArray::CacheHasPhysicsBSP` 0x00518110 (derives) vs `LiveEntityCollisionBuilder.cs:161` (copies); `HAS_PHYSICS_BSP_PS` acclient.h:2833 | | AP-155 | **Filed 2026-08-06 at the AP-152 retirement; NARROWED 2026-08-06 to its static-publication half alone.** Its flood half was bundled here with a different code path, a different population and a different gate — the exact fault the C4 handoff warns about — and its direction was recorded BACKWARDS; both are now split out as AP-156. **Static paths emit a Setup Sphere as a height-capped CYLINDER.** `LandblockPhysicsPublisher.cs:1030-1037` and `LandblockPhysicsContentBuilder.cs:683-690` both convert a Setup Sphere to `ShadowCollisionType.Cylinder` with `CylHeight = radius * 2f` and the origin shifted down by one radius; the live path emits a true `ShadowCollisionType.Sphere`, produced at exactly ONE site in `src/` (`ShadowShapeBuilder.cs`). Retail tests a Setup Sphere with `CSphere::intersects_sphere` @0x00537a80 / @0x00537fd0 (two overloads) in both cases — 3-D distance, no height clamp. The static paths also derive "has BSP" from `entity.MeshRefs` (the render mesh list) where the live path derives it from `setup.Parts` plus the effective post-`AnimPartChanged` identities; the two sources can disagree. | `src/AcDream.App/Streaming/LandblockPhysicsPublisher.cs:1030-1037`; `src/AcDream.Content/LandblockPhysicsContentBuilder.cs:683-690` | Affects static props only and changes their collision geometry over a much larger population than AP-152's 172, so it needs its own count and its own gate. Deliberately not folded into the AP-152 or AP-156 commits. | A static prop whose Setup carries a Sphere blocks over a height-clamped cylinder instead of a true sphere, and rests one radius lower than the authored origin. | `CSphere::intersects_sphere` 0x00537a80 / 0x00537fd0 | -| AP-156 | **Filed 2026-08-06, split out of AP-155(b) at the AP-152 retail-conformance review, WITH ITS DIRECTION CORRECTED — and its worst half FIXED in the same commit.** **CORRECTION.** AP-155(b) recorded the flood approximation as *over*-inclusive ("a sphere contains the box's inscribed extent but is larger in the diagonal"), and that recorded direction was the stated reason the residual was safe to defer. It was empirically inverted. `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`. Measured over the installed `client_portal.dat`, independently twice: 376 of 973 physics-BSP parts have `|origin| > radius/2`, worst 20.762 m on a 27.708 m sphere (gfx 0x010036DD, Setup 0x0200129A); the emitted flood failed to CONTAIN the object's own BSP sphere for **170 of the 172** AP-152 Setups, worst shortfall 9.911 m (Setup 0x02000255, whose one part's sphere sits 9.911 m above the part origin); and for 43 of them the post-AP-152 flood was strictly SMALLER than the pre-AP-152 one. Indoor floods are 3-D (`CellTransit.cs:601` routes every `id & 0xFFFF >= 0x0100` candidate through `FindTransitCellsSphere`), so a tall prop or door slab was simply absent from EnvCells it occupies and never a broadphase candidate there — UNDER-inclusive membership, the #98 / #168 class. **FIXED HERE.** `ShadowShape.BoundsCenter` carries the root sphere's own centre in the shape's local frame; `FromSetup` and `FromLandblockBspParts` fill it from the SAME resolver that supplies the radius, and `BuildFloodSpheres` places the sphere at `partWorldPos + rotate(BoundsCenter, partWorldRot)`. Retail does exactly this: `CGfxObj::physics_sphere` (`[gfxobj+0x74]`) is assigned `BSPTREE::GetSphere(physics_bsp)` @0x005397e0 (`mov eax,[ecx]; add eax,4` — the root `BSPNODE`'s `CSphere`, past its 4-byte vftable), and `CEnvCell::find_transit_cells` @0x0052cae0 — the part-array overload reached from `CPhysicsObj::find_bbox_cell_list` @0x00510fc0 through `CPartArray::calc_cross_cells_static` @0x00518160's `[vtbl+0x7c]` dispatch — loads it at `0x0052cb36 mov esi,[ecx+0x74]`, transforms its CENTRE through the part's own `Position` at `[part+0x30]` (`0x0052cb4c add eax,0x30` / `0x0052cb5a call Position::localtolocal`), and only then reads the radius at `0x0052cb65 fadd [esi+0xc]`. The same commit also retired the 10-sphere clamp on this branch: retail's clamp lives inside the CYLSPHERE overload alone (`CObjCell::find_cell_list` @0x0052b9f0, `0x0052ba21 cmp eax,0xa` / `0x0052ba28 mov ebp,0xa`) while the BSP walk has none — 7 installed Setups carry more than 10 physics-BSP parts (max 49, Setup 0x02001A91) and their tail parts were dropped from the flood entirely. **WHAT REMAINS OPEN.** acdream floods from the per-part spheres through its own sphere-vs-portal walk (`CellTransit.FindTransitCellsSphere`), where retail hands the part array to each cell's own `find_transit_cells` and tests every part's sphere against that cell's portal planes in cell-local space. The sphere SET is now exact; the TRAVERSAL is still acdream's. `find_bbox_cell_list`'s name notwithstanding, retail never forms a bounding box — AP-155(b)'s "acdream approximates retail's bounding BOX" was wrong as well. | `src/AcDream.Core/Physics/ShadowShape.cs` (`BoundsCenter`); `src/AcDream.Core/Physics/ShadowShapeBuilder.cs` (`FromSetup` step 3, `FromLandblockBspParts`); `src/AcDream.Core/Physics/ShadowObjectRegistry.cs` (`BuildFloodSpheres`); `src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs` (single bounds resolver); tests `ShadowObjectRegistryMultiPartTests.BuildFloodSpheres_BspShape_CentresOnTheBoundsCentreNotThePartOrigin` / `_RotatesTheBoundsCentreByThePartRotation` / `_CapsCylSpheresAtTenButNeverTheBspParts`, `ShadowRegistrationOverflowTests.FromLandblockBspParts_CarriesTheScaledRootSphereCentre`, `InstalledSetupBspPrimitiveDispatchTests.InstalledSetups_BspFloodSpheres_ContainTheirOwnBoundingSpheres` | The traversal residual is a genuine approximation with its own gate, not a deferral of this fix. Closing it means porting the per-cell `find_transit_cells` part-array overload, which is different work from getting the sphere set right. | A cell whose portal geometry a part's sphere overlaps in the sphere-vs-plane sense, but which the part's actual polygons do not reach, joins the object's shadow set: extra broadphase candidates, never a missed one. The under-inclusive direction is what the fix above removed. | `BSPTREE::GetSphere` 0x005397e0; `CGfxObj::physics_sphere` `[gfxobj+0x74]`; `CEnvCell::find_transit_cells` 0x0052cae0 (0x0052cb36 / 0x0052cb4c / 0x0052cb65); `CPhysicsObj::find_bbox_cell_list` 0x00510fc0; `CPartArray::calc_cross_cells_static` 0x00518160; `CObjCell::find_cell_list` 0x0052b9f0 (0x0052ba21) | +| AP-156 | **Filed 2026-08-06, split out of AP-155(b) at the AP-152 retail-conformance review, WITH ITS DIRECTION CORRECTED — and its worst half FIXED in the same commit.** **CORRECTION.** AP-155(b) recorded the flood approximation as *over*-inclusive ("a sphere contains the box's inscribed extent but is larger in the diagonal"), and that recorded direction was the stated reason the residual was safe to defer. It was empirically inverted. `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`. Measured over the installed `client_portal.dat`, independently twice: 376 of 973 physics-BSP parts have `|origin| > radius/2`, worst 20.762 m on a 27.708 m sphere (gfx 0x010036DD, Setup 0x0200129A); **POPULATION CORRECTED 2026-08-06 at the fix review (finding R2).** The row as filed said the flood failed to contain the object's own BSP sphere for '170 of the 172 AP-152 Setups'. That understates it: 172 is AP-152's DISPATCH population (Setups carrying BOTH a primitive and a physics-BSP part). After AP-152 EVERY BSP-bearing Setup floods from its BSP shapes alone, so the discarded origin mis-placed the flood across all 530 of them. Re-measured against PHYSICS-POLYGON VERTICES — a different DAT field from the sphere, so the measurement is not circular — by an independent scratch program outside the repo: **525 of the 530** BSP-bearing Setups have at least one flood sphere move; **428** fail vertex-level containment at a 1 mm tolerance (412 at 1 cm, the figure the fix review quotes); **0** fail after the fix, at any tolerance down to zero. Worst shortfall 35.869 m at entity scale 1.75 on Setup 0x0200129A. The old figures — 170 of 172, worst 9.911 m on 0x02000255 — remain correct for what they measured (root-sphere containment over the 172), and 43 of them had a post-AP-152 flood strictly SMALLER than the pre-AP-152 one. Indoor floods are 3-D (`CellTransit.cs:601` routes every `id & 0xFFFF >= 0x0100` candidate through `FindTransitCellsSphere`), so a tall prop or door slab was simply absent from EnvCells it occupies and never a broadphase candidate there — UNDER-inclusive membership, the #98 / #168 class. **FIXED HERE.** `ShadowShape.BoundsCenter` carries the root sphere's own centre in the shape's local frame; `FromSetup` and `FromLandblockBspParts` fill it from the SAME resolver that supplies the radius, and `BuildFloodSpheres` places the sphere at `partWorldPos + rotate(BoundsCenter, partWorldRot)`. Retail does exactly this: `CGfxObj::physics_sphere` (`[gfxobj+0x74]`) is assigned `BSPTREE::GetSphere(physics_bsp)` @0x005397e0 (`mov eax,[ecx]; add eax,4` — the root `BSPNODE`'s `CSphere`, past its 4-byte vftable), and `CEnvCell::find_transit_cells` @0x0052cae0 — the part-array overload reached from `CPhysicsObj::find_bbox_cell_list` @0x00510fc0 through `CPartArray::calc_cross_cells_static` @0x00518160's `[vtbl+0x7c]` dispatch — loads it at `0x0052cb36 mov esi,[ecx+0x74]`, transforms its CENTRE through the part's own `Position` at `[part+0x30]` (`0x0052cb4c add eax,0x30` / `0x0052cb5a call Position::localtolocal`), and only then reads the radius at `0x0052cb65 fadd [esi+0xc]`. The same commit also retired the 10-sphere clamp on this branch: retail's clamp lives inside the CYLSPHERE overload alone (`CObjCell::find_cell_list` @0x0052b9f0, `0x0052ba21 cmp eax,0xa` / `0x0052ba28 mov ebp,0xa`) while the BSP walk has none — 7 installed Setups carry more than 10 physics-BSP parts (max 49, Setup 0x02001A91) and their tail parts were dropped from the flood entirely. **WHAT REMAINS OPEN.** acdream floods from the per-part spheres through its own sphere-vs-portal walk (`CellTransit.FindTransitCellsSphere`), where retail hands the part array to each cell's own `find_transit_cells` and tests every part's sphere against that cell's portal planes in cell-local space. The sphere SET is now exact; the TRAVERSAL is still acdream's. `find_bbox_cell_list`'s name notwithstanding, retail never forms a bounding box — AP-155(b)'s "acdream approximates retail's bounding BOX" was wrong as well. **SECOND RESIDUAL, added 2026-08-06 at the fix review (finding R4): acdream SCALES the flood sphere; retail does not.** `ShadowShapeBuilder` multiplies both the radius and (new in this commit) the centre by the entity/part scale. Retail's `CEnvCell::find_transit_cells` @0x0052cae0 reads only `CPhysicsPart::pos` (`[part+0x30]`) and never `CPhysicsPart::gfxobj_scale` (`[part+0x24]`), while `CPhysicsPart::find_obj_collisions` @0x0050d8d0 DOES thread `gfxobj_scale.z` into `SPHEREPATH::cache_localspace_sphere` — so retail's cross-cell walk is itself under-inclusive for scaled parts and acdream's is not. Over-inclusive for scale > 1 (safe), under-inclusive for scale < 1 (the #98/#168 direction). **ENFORCEMENT, added 2026-08-06 at the fix review (finding A1).** The invariant now lives at the TYPE, not only at the producer seam: `ShadowShape`'s constructor is private and BSP shapes are built only through `ShadowShape.Bsp(..., FlatCollisionSphere localBounds)`, which takes radius and centre as ONE value and scales them together. The former public 7-argument constructor with `BoundsCenter = default` let a future BSP producer reintroduce this exact bug silently and green. **CONNECTED-GATE NOTE (finding A2). A null result on tall props is EXPECTED until AP-158 / #333 lands, and is not evidence against this fix.** The geometry now lands in the right cell and is then discarded one layer down by acdream's own `maxReach` broadphase filter, which measures from the same part origin: 118 of the 477 unique installed physics-BSP GfxObjs have a root-sphere offset above that filter's roughly 2.5 m walking budget, and 46 above 5 m. | `src/AcDream.Core/Physics/ShadowShape.cs` (`BoundsCenter`); `src/AcDream.Core/Physics/ShadowShapeBuilder.cs` (`FromSetup` step 3, `FromLandblockBspParts`); `src/AcDream.Core/Physics/ShadowObjectRegistry.cs` (`BuildFloodSpheres`); `src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs` (single bounds resolver); tests `ShadowObjectRegistryMultiPartTests.BuildFloodSpheres_BspShape_CentresOnTheBoundsCentreNotThePartOrigin` / `_RotatesTheBoundsCentreByThePartRotation` / `_CapsCylSpheresAtTenButNeverTheBspParts`, `ShadowRegistrationOverflowTests.FromLandblockBspParts_CarriesTheScaledRootSphereCentre`, `InstalledSetupBspPrimitiveDispatchTests.InstalledSetups_BspFloodSpheres_ContainTheirOwnPhysicsPolygons` (oracle swapped to physics-polygon vertices at the fix review, finding R1: the shipped assertion compared two hand-copies of the same expression and was algebraically identically zero for any DAT input) | The traversal residual is a genuine approximation with its own gate, not a deferral of this fix. Closing it means porting the per-cell `find_transit_cells` part-array overload, which is different work from getting the sphere set right. | A cell whose portal geometry a part's sphere overlaps in the sphere-vs-plane sense, but which the part's actual polygons do not reach, joins the object's shadow set: extra broadphase candidates, never a missed one. The under-inclusive direction is what the fix above removed. | `BSPTREE::GetSphere` 0x005397e0; `CGfxObj::physics_sphere` `[gfxobj+0x74]`; `CEnvCell::find_transit_cells` 0x0052cae0 (0x0052cb36 / 0x0052cb4c / 0x0052cb65); `CPhysicsObj::find_bbox_cell_list` 0x00510fc0; `CPartArray::calc_cross_cells_static` 0x00518160; `CObjCell::find_cell_list` 0x0052b9f0 (0x0052ba21) | | AP-157 | **Filed 2026-08-06 at the AP-152 retail-conformance review (finding F4) — an unregistered substitution that predates AP-152 and was stepped over when its neighbours were filed.** `CPhysicsObj::calc_cross_cells`' THIRD branch (`0x005152dc` -> `CPartArray::GetSortingSphere` @0x00518b00 -> `CObjCell::find_cell_list` @0x0052b990) floods from ONE authored whole-object sphere: `GetSortingSphere` returns `[partArray+0x54] + 0x70`, i.e. `CSetup::sorting_sphere` (acclient.h: `CSetup` carries `CSphere sorting_sphere` immediately after `step_up_height`), and that overload takes a single sphere with no cap. acdream's `only == null` branch floods from EVERY non-BSP, non-Cylinder shape instead — the Setup's per-part `Spheres` array. Different DAT field, different cardinality, different extent. 4,154 of 5,935 installed Setups carry a non-zero `SortingSphere` and `DatReaderWriter.Setup` already exposes it, so this is available rather than blocked. Same site, second item: `BuildFloodSpheres` collapses a Cylinder to one 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 as `(low_pt, radius, height)`. | `src/AcDream.Core/Physics/ShadowObjectRegistry.cs` (`BuildFloodSpheres`, the `anyCyl` and `only == null` branches) | Deliberately NOT folded into the AP-156 fix. It is a different branch of `calc_cross_cells`, reached only by objects with neither a physics BSP nor a CylSphere, so its population is disjoint from the 172 AP-152 Setups and its live gate is a different set of objects. Bundling it would make the AP-156 connected gate un-attributable — which is exactly how AP-155 came to carry two lifecycles under one id. | Sorting-sphere half: an object with several authored Spheres floods from all of them rather than from the one authored whole-object sphere — usually wider (max 5 Spheres on any installed Setup, so retail's 10-cap is never the difference), but a `sorting_sphere` LARGER than every per-part Sphere would make acdream under-inclusive, the #98 / #168 direction. CylHeight half: a tall thin cylinder floods a sphere of its radius at its base and can miss the cells its upper half occupies. | `CPhysicsObj::calc_cross_cells` 0x00515230 (0x005152dc / 0x005152e3 / 0x005152fb); `CPartArray::GetSortingSphere` 0x00518b00 (`[+0x54]+0x70`); `CObjCell::find_cell_list` 0x0052b990 (sorting sphere) / 0x0052b9f0 (cylsphere, `(low_pt, radius, height)`) | +| AP-158 | **Filed 2026-08-06 at the AP-156 fix review (finding A2) — an UNREGISTERED INVENTION, not a port, that predates AP-156 and is issue #333.** The shadow broadphase discards a candidate outright when `distToCurr > sphereRadius + obj.Radius + movement.Length() + 2f`. **Retail has no distance pre-filter at all.** `CObjCell::find_obj_collisions` @0x0052b750, disassembled from the PDB-paired binary for this row rather than inherited: it early-returns `OK_TS` only when `sphere_path.insert_type == INITIAL_PLACEMENT_INSERT` (`0x0052b759 cmp dword [ebx+0x174],2` / `0x0052b765 je 0x52b7a0`), then walks `shadow_object_list` (`[cell+0xc8]`, count `[cell+0xc4]`) and calls `CPhysicsObj::FindObjCollisions` (`0x0052b78b call 0x50f050`) on every entry whose `physobj` is unparented (`[physobj+0x40] == 0`) and is not the mover itself — UNCONDITIONALLY. There is no distance test in the function. Neither the `+ 2f` slack nor the `movement.Length()` term has a retail counterpart; retail's own cross-cell slack constant is `F_EPSILON` = 1.9999999e-4 m (`0x0052cb5f fld dword [0x7c8c70]`), 0.0002 m and not 2 m. **Second half of the defect:** the filter measures `currPos - obj.Position`, i.e. from the PART ORIGIN, while `obj.Radius` is the BSP root bounding-sphere radius measured about a centre that AP-156 established is frequently metres away — `ShadowEntry` does not carry the `BoundsCenter` that `ShadowShape` now does. A mover touching the geometry is up to `d + R + r` from the part origin and is admitted only when `d <= movement + 2`, roughly 2.5 m for a walking player. | `src/AcDream.Core/Physics/TransitionTypes.cs:3757-3765`; `ShadowEntry` (`src/AcDream.Core/Physics/ShadowObjectRegistry.cs:2735`) carries no `BoundsCenter` | Deliberately NOT folded into the AP-156 commit: different code path (collision query, not cell membership) and it needed its own retail question answered, which this row answers. The minimal fix is mechanical — carry `BoundsCenter` on `ShadowEntry` and measure from `obj.Position + rotate(obj.BoundsCenter, obj.Rotation)`; only whether to keep the `+ 2f` slack at all is genuinely open. | **This is the gate immediately downstream of AP-156, and it can mask AP-156's entire visible benefit.** 118 of the 477 unique installed physics-BSP GfxObjs have a root-sphere offset above the ~2.5 m budget and 46 above 5 m; at a test scale of 1.75 those become 4.4 m and 8.75 m against an unchanged budget. Worked case: Setup 0x02000255, one part, root sphere origin (0.000, -0.007, 9.911), radius 10.522 — a player against its upper half is ~20.4 m from the part origin while `maxReach` is ~13.5 m. Discarded before `BSPQuery` ever runs. A tall prop that still does not block after AP-156 is THIS row, not a failure of AP-156. | `CObjCell::find_obj_collisions` 0x0052b750 (0x0052b759 / 0x0052b765 / 0x0052b788 / 0x0052b78b), pseudo-C 308916-308940; `CEnvCell::find_transit_cells` 0x0052cae0 (`F_EPSILON` at 0x0052cb5f -> 0x7c8c70); issue #333 | | ~~AP-152~~ | **RETIRED 2026-08-06 (the commit that filed it is one day old; this retirement corrects four statements in it).** `ShadowShapeBuilder.FromSetup` now DISPATCHES instead of unioning: a step-0 gate derived from the parts suppresses steps 1 and 2 whenever any part's EFFECTIVE GfxObj carries a physics BSP. Retail's priority, re-disassembled from the PDB-paired binary for this commit rather than inherited: `CPhysicsObj::FindObjCollisions` @0x0050f050 tests `HAS_PHYSICS_BSP_PS` FIRST (`0x0050f165 test dword [esi+0xa8],0x10000` / `0x0050f16f je 0x50f1a2`) and leaves the BSP branch through the UNCONDITIONAL `0x0050f19d jmp 0x50f2b0`, which is past the CylSphere loop at 0x50f1a2 AND the Sphere loop at 0x50f21d; a CylSphere-bearing object that survives its loop RETURNS (`0x0050f1d6 jae 0x50f317`); a Setup with zero spheres returns the seeded OK_TS (`0x0050f22f je 0x50f31b`). **BSP wins.** **CORRECTION 1 — the row's risk statement was FALSE as written.** It predicted "catching or stopping on a doorway sill". acdream did not test the extra primitive either: `Transition.BspOnlyDispatch` (`TransitionTypes.cs:1348`, landed 2026-05-25 as A6.P7) already skipped BOTH primitive branches (`:3911`, `:3954`) whenever the target's wire `PhysicsState` carries 0x10000, and ACE sets that bit from `CSetup.HasPhysicsBSP` (`WorldObject_Networking.cs:665-668`). The row's own anchor column cites the flag it failed to notice acdream was already keying on. So this retirement is NOT a collision-response change; the live half was CELL MEMBERSHIP, which had no such guard (see AP-155). **CORRECTION 2 — "the affected primitives are small and centred at the part origin" was FALSE in both halves.** The largest is `0x02001741`'s CylSphere at **r = 6.714 m**; `0x0200086E`'s Sphere is r = 5.842 m with origin (0.759, 0.165, 5.842), nowhere near the part origin. **CORRECTION 3 — the cottage door's "~14 cm base Sphere" was the wrong field.** `0x020019FF`'s Sphere radius is **0.100 m** at origin (0, 0, 0.018); `0.141` is `Setup.Radius`, which AP-22 had just finished proving is never collision geometry. **CORRECTION 4 — the row named ONE pinning test where TWO existed.** `FromSetup_DoorSetup_SphereAtExpectedLocalOffset` also failed under the exclusive rule; both are corrected, neither deleted. Population re-measured independently at 172 of 5,935 (73 CylSphere+BSP, 99 Sphere+BSP; 530 carry a physics-BSP part), agreeing exactly with the filing commit's separate sweep, and now pinned by an installed-DAT test with external bucket controls. | RETIRED — `src/AcDream.Core/Physics/ShadowShapeBuilder.cs` (`FromSetup` step 0 gate + `EffectivePartGfxObjId`, shared with step 3 so the two can never read different identities); `tests/AcDream.Core.Tests/Physics/ShadowShapeBuilderTests.cs` (`FromSetup_DoorSetup_EmitsBspPartsOnly`, `FromSetup_DoorSetup_SphereAtExpectedLocalOffset` re-hosted on `_ => false`, `FromSetup_DispatchGateReadsTheEffectivePartIdentities`); `tests/AcDream.App.Tests/Physics/LiveEntityCollisionBuilderTests.cs` (`CylSphereAndPhysicsBspPart_EmitsOnlyTheScaledBspShape` — no App fixture combined a primitive with a BSP part before); `tests/AcDream.Content.Tests/InstalledSetupBspPrimitiveDispatchTests.cs` (population). `Transition.BspOnlyDispatch` is deliberately KEPT: retail genuinely dispatches at the query site too, and it guards against a future additive producer. | — | — | `CPhysicsObj::FindObjCollisions` 0x0050f050 (0x0050f165 / 0x0050f16f / 0x0050f19d / 0x0050f1d6 / 0x0050f22f); `CPhysicsObj::calc_cross_cells` 0x00515230 (0x00515285 / 0x0051528f) -> `CPhysicsObj::find_bbox_cell_list` 0x00510fc0; `CPhysicsPart::find_obj_collisions` 0x0050d8d0; `CPartArray::CacheHasPhysicsBSP` 0x00518110; evidence `docs/research/2026-08-06-ap152-contract.md` | | ~~AP-145~~ | **RETIRED 2026-08-05 (C5a commit 1, closing #318; corrected at the architecture-review re-pass, A1/A2).** `RuntimePlacementPresentationSink.TryPublishPlace` now publishes the local player's Place through `LocalPlayerShadowSynchronizer.SyncPose(entity, entity.Position, entity.Rotation, record.FullCellId, force: true)` — the SAME publisher ordinary per-tick movement uses — instead of writing `LocalPlayerShadowState.Set` directly. `SyncPose` calls `ShadowPositionSynchronizer.Sync` → `ShadowObjectRegistry.UpdatePosition` (the real `PhysicsEngine.ShadowObjects` publish) BEFORE it records the dedup cache as its own last step, so the cache can no longer be pre-seeded ahead of the real publish. `force: true` because this is the authoritative placement commit, not an ordinary refresh — it must never be skipped by `SyncPose`'s own dedup check. **`TryPublishWithdrawal` carried the exact mirror asymmetry** (a bare `_localPlayerShadow.Clear()` with no `ShadowObjects.Suspend`, leaving a live phantom row at the park's source cell for the whole park window — the #184 shape) and is fixed in the SAME commit, same one-call shape: `_localPlayerShadowSync.Suspend(entity)`. The sink no longer holds a direct `LocalPlayerShadowState` reference at all — both halves route exclusively through the one synchronizer, which owns the cache internally. One synchronizer instance is constructed in `LivePresentationComposition.cs` (before the sink) and threaded through `LivePresentationResult` to `SessionPlayerComposition.cs`, which no longer builds its own. `#318`'s composition test (`RuntimePlacementShadowCompositionTests.cs`, 4 facts) proves: the real `ShadowObjects` registry holds a row at the destination cell (not just the cache) after a bare `Place` with no subsequent tick; the SOURCE cell's row is gone, not duplicated; a subsequent ordinary per-tick `Sync` call is a correct no-op; a `Withdraw` suspends the real registry row (not just the cache) — the source cell carries zero rows and the retained (suspendable) registration survives for a later restore; and a Place for a **registered** non-local-player entity leaves its row at the source cell and does not pollute the player's cache (route 7 P4 — the fix lives entirely inside the pre-existing player-only gate; the first version of this fact registered nothing for the child and was vacuous under the gate's own removal, corrected at the review). Sabotage-verified all four facts, both directions: reverted, each fails at its own discriminating assertion; applied, all green. | `src/AcDream.App/World/RuntimePlacementPresentationSink.cs` (`TryPublishPlace`, `TryPublishWithdrawal`); `src/AcDream.App/Composition/LivePresentationComposition.cs` (`LocalPlayerShadowSynchronizer` construction + `LivePresentationResult` field); `src/AcDream.App/Composition/SessionPlayerComposition.cs` (consumes the shared instance); `tests/AcDream.App.Tests/World/RuntimePlacementShadowCompositionTests.cs` | — | — | No retail analogue — retail has no separate shadow-cache/publish split; this was an acdream-only two-object seam (`LocalPlayerShadowState` cache + `LocalPlayerShadowSynchronizer` publisher) that a direct `.Set()`/`.Clear()` call could desynchronize from | | ~~AP-1~~ | **RETIRED 2026-08-05 (C5a deletion sweep).** "Production zero-delta routes deliberately remain on the legacy resolver until 4B2" is false at HEAD: the exhaustive receiver census over `src/` shows zero `PhysicsEngine.Resolve`/`.ResolvePlacement` call sites, and every production placement writer reaches canonical `PhysicsEngine.SetPosition` only through `RuntimeSetPositionState` (three call sites total). C5a deleted `Resolve`, `ResolvePlacement`, and their `HasCellSurface` helper outright — the resolver-shaped entry points this row described no longer exist, so the condition is retired structurally, not just narrowed. The narrower survivors (#276 settle-cell discard, AD-61 force-seed, AD-62 non-commit outcomes) are separately filed rows and are unaffected. | `src/AcDream.Core/Physics/PhysicsSetPosition.cs`; `src/AcDream.Runtime/Physics/RuntimeSetPositionState.cs`; `src/AcDream.Runtime/Physics/RuntimeCollisionReportingState.cs`; `src/AcDream.Runtime/Physics/RuntimePlacementProjectionChannel.cs`; `src/AcDream.Core/Physics/PhysicsEngine.cs` (deletion); `docs/research/2026-08-05-c5a-contract.md` | — | — | `CPhysicsObj::SetPosition` 0x005160C0; `SetPositionInternal` 0x00515BD0; `CPhysicsObj::handle_all_collisions` 0x00514780; `track_object_collision` 0x00513F10; `report_collision_end` 0x00514620; `AdjustPosition` 0x00511D80; `CheckPositionInternal` 0x00511E90; `CTransition::find_valid_position` 0x0050C310; `find_placement_position` 0x0050C170; `validate_placement_transition` 0x0050ADC0; `validate_placement` 0x0050B210 | diff --git a/docs/research/2026-08-06-ap156-fix-review.md b/docs/research/2026-08-06-ap156-fix-review.md new file mode 100644 index 00000000..94092d2e --- /dev/null +++ b/docs/research/2026-08-06-ap156-fix-review.md @@ -0,0 +1,719 @@ +# AP-156 fix review — `b52967de` + `e2b2d04c` + +Adversarial dual-lens review of the AP-152 flood-sphere fix, prompted by the +FAIL in `docs/research/2026-08-06-ap152-review-retail.md`. + +Worktree `.claude/worktrees/resume-session-e0bd03e1-d5bf45`, HEAD `e2b2d04c`. +All 44 `bin`/`obj` deleted before every verdict-deciding build. Six sabotages +applied and restored; `git status --porcelain` empty at the end. + +## VERDICTS + +| Lens | Verdict | +|---|---| +| **Retail conformance** | **PASS** | +| **Architecture** | **PASS** | + +Three things the brief asked me to settle explicitly: + +- **The containment claim — BELIEVED, and true more strongly than asserted.** + The shipped assertion is algebraically tautological (R1), so I re-derived + containment independently against physics-polygon **vertices** rather than + against the root sphere: **0 of 530** BSP-bearing Setups fail after the fix, + **412** failed before it. +- **The cap change — the implementer is RIGHT and both prior reviews were + WRONG.** Byte-verified: the `cmp eax,0xa` clamp is inside the cylsphere + overload only; the BSP walk is uncapped; `@0x0052b990` pushes a literal `1`. +- **The shrink argument — CORRECT, and both prior reviews used the wrong + criterion.** `calc_cross_cells`' BSP branch is reached through an + unconditional jump past both primitive branches, so retail's flood for these + objects contains no primitive contribution at all. "Smaller than before" is + not a defect criterion; containment is. My own sweep reproduces the + implementer's 143. + +--- + +## Part 1 — Independent retail verification + +Method: my own Capstone disassembly of the PDB-paired binary +(`C:\Users\erikn\Downloads\acclient.exe`, `check_exe_pdb.py` → **MATCH**, +CodeView GUID `9e847e2f-777c-4bd9-886c-22256bb87f32`, linker +2013-09-06T00:17:56Z, image base `0x400000`), every address cross-resolved +through `named-retail/symbols.json` and every struct offset cross-checked +against `named-retail/acclient.h`. Not inherited from the commit, not from the +prior reviews. + +### 1.1 Every cited address resolves to the construct claimed — all 12 + +`symbols.json` lookup by address: + +| Address | Symbol | +|---|---| +| `0x0050f050` | `CPhysicsObj::FindObjCollisions` | +| `0x00510fc0` | `CPhysicsObj::find_bbox_cell_list` | +| `0x00515230` | `CPhysicsObj::calc_cross_cells` | +| `0x00518070` / `0x00518090` | `CPartArray::GetSphere` / `GetCylsphere` | +| `0x00518110` | `CPartArray::CacheHasPhysicsBSP` | +| `0x00518160` | `CPartArray::calc_cross_cells_static` | +| `0x00518b00` | `CPartArray::GetSortingSphere` | +| `0x0052b990` / `0x0052b9f0` | `CObjCell::find_cell_list` (two overloads) | +| `0x0052cae0` | `CEnvCell::find_transit_cells` | +| `0x005397e0` | `BSPTREE::GetSphere` | +| `0x004527f0` | `Position::localtolocal` | + +No mis-cited neighbour anywhere in this commit, the register rows, or the +tests. (I looked specifically, given the mis-cite found earlier this session.) + +### 1.2 `BSPTREE::GetSphere` @`0x005397e0` — claim 4b CONFIRMED byte-exact + +``` +0x005397e0 8b01 mov eax, dword ptr [ecx] ; BSPTREE::root_node +0x005397e2 83c004 add eax, 4 ; past BSPNODE::vfptr +0x005397e5 c3 ret +``` + +`acclient.h`: `struct BSPNODE { BSPNODEVtbl *vfptr; CSphere sphere; Plane +splitting_plane; ... }` and `struct CSphere { AC1Legacy::Vector3 center; float +radius; }` → radius at `+0xc` of the returned pointer. Retail's per-part flood +sphere **is** the BSP root bounding sphere, origin included. + +`acclient_2013_pseudo_c.txt:319128` (`0x00534b5b`): +`this->physics_sphere = BSPTREE::GetSphere(this->physics_bsp);` — verbatim. + +### 1.3 `physics_sphere` = `[gfxobj+0x74]`, `physics_bsp` = `[+0x78]` — claim 4c CONFIRMED + +`acclient.h` `CGfxObj` field order: `... use_built_mesh; CSphere +*physics_sphere; BSPTREE *physics_bsp; Vector3 sort_center; unsigned +num_polygons; CPolygon *polygons; CSphere *drawing_sphere; ...` — which places +`physics_sphere` at `+0x74`, `physics_bsp` at `+0x78`, `drawing_sphere` at +`+0x90`. Confirmed against code: + +``` +CPartArray::CacheHasPhysicsBSP @0x00518110 + 0x00518120 8b32 mov esi,[edx] ; parts[i] + 0x00518122 8b7620 mov esi,[esi+0x20] ; CPhysicsPart::gfxobj + 0x00518125 8b36 mov esi,[esi] + 0x00518127 8b7e78 mov edi,[esi+0x78] ; physics_bsp <-- exact addr +``` + +and `find_transit_cells` reads `[ecx+0x74]` with a `[ecx+0x90]` fallback — the +drawing sphere — which only makes sense with this layout. + +### 1.4 `CEnvCell::find_transit_cells` @`0x0052cae0` — claim 4a CONFIRMED, centre-then-radius + +``` +0x0052cb31 8b5020 mov edx,[eax+0x20] ; CPhysicsPart::gfxobj +0x0052cb34 8b0a mov ecx,[edx] +0x0052cb36 8b7174 mov esi,[ecx+0x74] ; physics_sphere +0x0052cb39 85f6 test esi,esi +0x0052cb3d 8bb190.. mov esi,[ecx+0x90] ; else drawing_sphere +0x0052cb4b 56 push esi ; -> &sphere->center +0x0052cb4c 83c030 add eax,0x30 ; CPhysicsPart::pos +0x0052cb4f 50 push eax +0x0052cb50 8d442434 lea eax,[esp+0x34] ; out +0x0052cb54 8d5d54 lea ebx,[ebp+0x54] ; cell->pos +0x0052cb5a e8915cf2ff call 0x4527f0 ; Position::localtolocal +0x0052cb5f d905708c7c00 fld dword [0x7c8c70] ; F_EPSILON = 1.9999999e-4 +0x0052cb65 d8460c fadd dword [esi+0xc] ; ONLY NOW the radius +``` + +`acclient.h` `CPhysicsPart`: `CYpt(0) viewer_heading(4) degrades(0x10) +deg_level(0x14) deg_mode(0x18) draw_state(0x1c) gfxobj(0x20) +gfxobj_scale(0x24) pos(0x30)` — both `+0x20` and `+0x30` are exactly what the +commit says they are. + +The call is `cellPos.localtolocal(&out, &part->pos, &sphere->center)`: the +sphere's **centre** is transformed through the part's own `Position` into the +cell frame, and the radius is only added afterwards to build the plane test. +**Carrying the radius alone is a different sphere — the commit's thesis is +right.** + +### 1.5 The 10-sphere cap — settled against the implementer's claim + +**Claim: the cap lives only in the cylsphere overload. VERIFIED.** + +``` +CObjCell::find_cell_list @0x0052b9f0 (the N-sphere overload) + 0x0052ba1d 8b442414 mov eax,[esp+0x14] ; count + 0x0052ba21 83f80a cmp eax,0xa + 0x0052ba26 760b jbe 0x52ba33 + 0x0052ba28 bd0a0000 mov ebp,0xa ; clamp + ... copy loop into the STATIC array 0x844838..0x8448d8 (10 x 0x10 stride) +``` + +The clamp is a fixed static-buffer capacity, not a policy: the destination +array is exactly ten 16-byte entries and the guard byte at `0x8448d8` sits +immediately past it. + +``` +CObjCell::find_cell_list @0x0052b990 (the sorting-sphere overload) + 0x0052b9d6 6a01 push 1 ; literal count of ONE +``` + +``` +CPhysicsObj::calc_cross_cells @0x00515230 + 0x00515285 test dword [esi+0xa8],0x10000 + 0x0051528f jne 0x515305 ------------------> 0x0051530c call 0x510fc0 (BSP) + 0x005152d1 call 0x52b9f0 (cylsphere, capped) + 0x005152fb call 0x52b990 (sorting sphere, one) +``` + +``` +CPhysicsObj::find_bbox_cell_list @0x00510fc0 + loop over cells [esi+8], no clamp + 0x00511012 call 0x518160 + +CPartArray::calc_cross_cells_static @0x00518160 + 0x0051816e mov esi,[eax+0x5c] ; parts + 0x00518171 mov eax,[eax+0x58] ; num_parts + 0x00518176 call dword [edx+0x7c] ; args (num_parts, parts, cellarray) + +CEnvCell::find_transit_cells @0x0052cae0 + inner part loop 0x52cb22 .. 0x52cc76, bound = [esp+0x6c] = num_parts + outer portal loop, bound = [ebp+0x108] + NO CLAMP ANYWHERE +``` + +**The implementer is correct on all three counts. Both prior reviews passed a +cap that retail does not have on that branch.** 7 installed Setups exceed 10 +physics-BSP parts (max 49, Setup `0x02001A91` — reproduced independently), and +their tail parts were previously dropped from the flood outright. The change is +a correctness improvement in the same direction as the rest of the commit. + +### 1.6 `CPartArray::GetSortingSphere` @`0x00518b00` (AP-157's anchor) + +``` +0x00518b67 8b4654 mov eax,[esi+0x54] +0x00518b6f 83c070 add eax,0x70 +``` + +Exactly `[partArray+0x54] + 0x70`. `acclient.h` `CSetup` places +`CSphere sorting_sphere` immediately after `step_up_height`. AP-157's citation +holds. + +### 1.7 Two retail facts the commit did not use, both relevant + +**`CObjCell::find_obj_collisions` @`0x0052b750` has no distance pre-filter.** +Pseudo-C `308916-308940`: it walks `shadow_object_list` and calls +`CPhysicsObj::FindObjCollisions` on every non-parented, non-self entry +unconditionally. So the `maxReach` early-out at +`src/AcDream.Core/Physics/TransitionTypes.cs:3763` — `sphereRadius + +obj.Radius + movement.Length() + 2f` — is entirely acdream's own invention with +no retail counterpart. **#333's characterisation is correct.** See A2. + +**Retail's own slack constant is 0.0002 m, not 2 m.** `0x0052cb5f fld dword +[0x7c8c70]` reads `1.9999999494757503e-4` (`F_EPSILON`). Anchor for whoever +fixes #333. + +**Retail's cross-cell walk ignores part scale; its collision does not.** +`find_transit_cells` uses only `CPhysicsPart::pos` (`+0x30`) and never +`gfxobj_scale` (`+0x24`), while `CPhysicsPart::find_obj_collisions` +@`0x0050d8d0` explicitly passes `gfxobj_scale.z` into +`SPHEREPATH::cache_localspace_sphere`. See R4. + +--- + +## Part 2 — Independent DAT re-derivation + +Scratch console program **outside the repo**, referencing only +`Chorizite.DatReaderWriter 2.1.7` — deliberately **not** `AcDream.Core`, so +nothing under test is in the loop. Every number recomputed by hand from the +installed `client_portal.dat`. + +| Quantity | Committed constant | My measurement | +|---|---|---| +| Setups | 5935 | **5935** | +| with CylSphere | 678 | **678** | +| Sphere-only, no CylSphere | 3605 | **3605** | +| without any primitive | 1652 | **1652** | +| with a physics-BSP part | 530 | **530** | +| physics-BSP parts | 973 | **973** | +| off-centre parts (`\|o\| > r/2`) | 376 | **376** | +| affected (primitive + BSP) | 172 (73 cyl / 99 sph) | **172 (73 / 99)** | +| deepest BSP part array | 49, `0x02001A91` | **49, `0x02001A91`** | +| Setups with > 10 BSP parts | 7 | **7** | +| worst part-origin offset | 20.762 m on gfx `0x010036DD` (r 27.708) | **20.762 m, gfx `0x010036DD`, r 27.708** | +| affected Setups failing containment pre-fix | 170 | **170**, worst 17.345 m at scale 1.75 = **9.911 m at scale 1**, Setup `0x02000255` | +| affected Setups failing post-fix | 0 | **0** | +| 143-shrink claim | 143 of 172 | **143** | +| max Spheres on any Setup (AP-157) | 5 | **5** (`0x020016F7`) | +| non-zero `SortingSphere` (AP-157) | 4154 | **4154** | + +Every external constant reproduces exactly, including the 9.911 m figure once +the 1.75 test scale is divided out. + +### The stronger, genuinely independent containment result + +Instead of comparing the flood sphere to the root sphere (which is circular — +see R1), I transformed **every physics-polygon vertex** of every physics-BSP +part into the same world frame and asked whether some emitted flood sphere +covers it: + +``` +VERTEX containment failures: after the fix = 0 before the fix = 412 +``` + +Zero across all 530 BSP-bearing Setups. **I believe the containment claim.** + +--- + +## Part 3 — Sabotage reproduction + +Six of the nine claimed sabotages reproduced, including C, H and I as +requested. Each applied, clean-built, run, reverted. + +| # | Sabotage | Claimed | Observed | +|---|---|---|---| +| A | `BuildFloodSpheres`: `world = partWorldPos` | 3 Core | **4 Core** (`FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint`, `_CapsCylSpheresAtTenButNeverTheBspParts`, `_RotatesTheBoundsCentre…`, `_CentresOnTheBoundsCentre…`) | +| C | `FromSetup` `boundsCenter = Vector3.Zero` (the shipped defect) | 1 Core + 2 App + 1 Content | **1 Core + 2 App + 1 Content** ✓ | +| D | drop `* entScale` on `boundsCenter` | 2 App + 1 Content | **2 App + 1 Content** ✓ | +| E | landblock flat branch `localCenter = Zero` | 1 Core | **1 Core** ✓ | +| H | `int cap = RetailSphereCap` (all branches) | 1 Core | **1 Core** ✓ | +| I | cap removed from the cylsphere branch | 1 Core | **1 Core** ✓ | + +The claimed counts are honest; A is understated by one, i.e. coverage is +slightly better than advertised (A5). + +**Trap encountered and avoided:** the naive form of sabotage I +(`int cap = int.MaxValue`) makes `RetailSphereCap` unused, which is +`error CS0219` under `TreatWarningsAsErrors`. The build fails, the stale +sabotage-H DLL survives, and the run reports H's failure as I's. I caught it by +grepping the build output for `Build succeeded` before every test run. Anyone +re-running these must do the same. + +The fixtures are genuinely off-centre now. `ShadowObjectRegistryMultiPartTests.Bsp()` +defaults `BoundsCenter` to `(0, 6, 0)` and requires an explicit `(0.001, 0, 0)` +to get a near-concentric sphere; +`LiveEntityCollisionBuilderTests.Bsp()` defaults `centerZ = 1.25`; +`ShadowRegistrationOverflowTests.BspGfx()` defaults `centerZ = 0.75`. The +concentric `Radius = 14f` at `LocalPosition = Zero` configuration the prior +review named is gone from every flood fixture. + +--- + +## Part 4 — `FromLandblockBspParts` (neither prior review looked) + +`src/AcDream.Core/Physics/ShadowShapeBuilder.cs:268-334`. + +- **The same discard existed.** Pre-fix the method computed only + `localRadius = flat.Nodes[flat.RootIndex].BoundingSphere.Radius` (or + `phys.BoundingSphere?.Radius`) and emitted the shape at `pPos`. Identical + defect, on stair runs, fences and rock clusters. +- **Both storage branches are fixed.** Flat (`:310-315`) and graph fallback + (`:316-320`) each take `Origin` alongside `Radius`. + `ShadowRegistrationOverflowTests.FromLandblockBspParts_CarriesTheScaledRootSphereCentre` + asserts both explicitly; sabotage E reddens the flat branch (verified) and + the same test carries the graph assertion. +- **`partScale` is applied correctly.** `Matrix4x4.Decompose` yields + `M = S·R·T`, so a point `v` in the part's own frame maps to root space as + `pPos + rotate(v · pScale, pRot)`. The code emits + `LocalPosition = pPos` (unscaled — correct, the translation is already + absolute in root space), `Radius = localRadius * partScale`, + `BoundsCenter = localCenter * partScale`, `Scale = partScale`. That composes + exactly as `BuildFloodSpheres` and as `TransitionTypes`' BSP transform + (`local * obj.Scale`, rotate, `+ obj.Position`) consume it. + +Also verified: the two other `new ShadowShape(...)` sites in `src/` +(`LandblockPhysicsPublisher.cs:1003,1030` and +`LandblockPhysicsContentBuilder.cs:658,683`) emit **Cylinder** shapes only, for +which `BoundsCenter == Zero` is correct by definition. There is no third BSP +producer. + +--- + +## Part 5 — Is `BoundsCenter` the right seam? + +**Yes. The implementer's rebuttal of the `LocalPosition`-offset suggestion is +correct, and I verified the reason rather than taking it.** + +`ShadowEntry.Position` is load-bearing in three distinct ways: + +1. **BSP world origin.** `TransitionTypes.cs:3862-3898` transforms the mover's + spheres by `Vector3.Transform(sphere.Origin - obj.Position, invRot) * + invScale` and passes `worldOrigin: obj.Position` into + `CollisionTraversal.FindCollisions`. The physics BSP is authored about the + **part origin**, so shifting `Position` to the bounding-sphere centre would + translate every collision polygon by the offset — up to 20.762 m. +2. **Broadphase reach.** `TransitionTypes.cs:3757`. +3. **Cylinder XY distance.** `TransitionTypes.cs:3760`. + +Moving `LocalPosition` would break (1) outright. A separate field is the only +correct seam. + +**Consistency audit — every site that composes a shape's world placement:** + +| Site | Uses | Correct? | +|---|---|---| +| `ShadowObjectRegistry.cs:474-490` `RegisterMultiPart` → `ShadowEntry` | `LocalPosition` only | ✓ (BSP frame) | +| `:578-593` `ReplaceMultiPartPayload` → `ShadowEntry` | `LocalPosition` only | ✓ | +| `:1484-1500` suspended-owner restore → `ShadowEntry` | `LocalPosition` only | ✓ | +| `:691-693` `BuildFloodSpheres` | `LocalPosition` **+ rotate(BoundsCenter, partWorldRot)** | ✓ | +| `:773`, `:1648`, `:2231` reflood / transfer | delegate to `RegisterMultiPart` | ✓ | + +`BuildFloodSpheres` is the single flood site (`UpdatePosition` routes through +`RegisterMultiPart`), and no site reads the old semantics. The one place the +new field is *missing* and should eventually be present is `ShadowEntry` itself +— that is #333 (A2). + +`ReplaceMultiPartPayload` deliberately does not re-flood; that matches retail +(`CPartArray::SetPart` changes the part read by later tests and does not +recalculate cross-cells). Not a finding. + +--- + +## Part 6 — Is the shrink argument right? + +**Yes. Settle it in favour of the implementer.** + +Prior review F2 counted 43 Setups whose post-`4abd1b5e` flood was not a +superset of the pre-`4abd1b5e` one and called it a new under-inclusive defect. +My sweep puts the post-`b52967de` number at **143 of 172** — larger, exactly as +the implementer says. + +That is not a defect, because retail's flood for a BSP-bearing object contains +**no primitive contribution at all**: + +``` +0x00515285 test dword [esi+0xa8],0x10000 +0x0051528f jne 0x515305 ; -> find_bbox_cell_list, and it never returns + ; into 0x005152d1 / 0x005152fb +``` + +The cylsphere branch (`0x005152d1`) and the sorting-sphere branch +(`0x005152fb`) are both below that jump and unreachable from it — I read the +control flow, not a summary of it. `find_bbox_cell_list` seeds the object's own +cell (`0x00510fd5` → `0x00510fe2 call 0x6b4ff0`) and then walks the part array; +nothing else contributes. + +So for `0x0200086E` — F2's worst case, whose Sphere reached `z ≈ 11.68` while +its BSP parts reach `z ≈ 9.0` — **retail also reaches only `z ≈ 9.0`.** The +pre-commit flood was larger because it included a primitive `calc_cross_cells` +never reads. Shrinking toward the BSP set is convergence, not regression. + +The criterion that *does* discriminate is containment, and it now holds at +vertex level for all 530 (Part 2). + +**What this means for the connected session** (the practical point): +"props that stopped blocking" is **not** by itself a bug report. The question +to ask of any such observation is whether the object's own BSP geometry still +reaches the cell it stopped blocking from. If it does, that is a bug; if it +does not, that is retail. Conversely, "a tall prop that still does not block" +is the expected symptom of #333, not of this fix (A2). + +--- + +## Findings — retail lens + +### R1 (MEDIUM) — the shipped containment assertion is algebraically tautological + +`tests/AcDream.Content.Tests/InstalledSetupBspPrimitiveDispatchTests.cs:266-341`. + +`truth` is built at `:282-284` as +`(partOrigin + rotate(b.Origin, partRot)) * EntScale` from the **same** +`Bounds()` resolver, over the **same** part set (the gate at `:299` is +`id => Bounds(id) is not null`), with the **same** placement-frame priority +(`:259-264` mirrors `ResolvePlacementFrame`). `flood` is built at `:314-317` as +`shape.LocalPosition + rotate(shape.BoundsCenter, shape.LocalRotation)`, and +`FromSetup` sets `LocalPosition = partFrame.Origin * entScale`, +`BoundsCenter = b.Origin * entScale`, `LocalRotation = partFrame.Orientation`. +The two expressions are identical by algebra, so `Shortfall(flood) ≡ 0` for +**any** DAT content and any future DAT. + +Concrete failure scenario: if a later change made `FromSetup` read the wrong +GfxObj's bounding sphere in *both* the resolver and the emission — say a +`SetupId`/`GfxObjId` mix-up inside `Bounds()` — this assertion stays green +while every flood sphere in the game moves. + +It is still a useful change-detector (sabotages C and D redden it, verified), +and `ExpectedWouldFailIfOriginDiscarded = 170` is a real, non-circular +measurement of the defect. But the commit body's framing — *"an installed-DAT +containment sweep asserting every emitted BSP flood sphere contains that part's +real bounding sphere"* — overstates what the code proves. + +Cheap fix: compare against physics-polygon vertices (`gfx.PhysicsPolygons` → +`gfx.VertexArray.Vertices`) instead of against the root sphere. I ran exactly +that and it passes 0/530, so adopting it costs nothing and makes the assertion +mean what it says. + +### R2 (MEDIUM) — the defect population is understated 2.4× + +The commit, AP-156 and both prior reviews all scope the defect to "170 of the +172" AP-152 Setups. After `4abd1b5e` **every** BSP-bearing Setup floods from +its BSP shapes only, so the discarded origin mis-placed the flood for all of +them, not just the ones that also carry a primitive. Measured: + +``` +vertex-containment failures with the origin discarded = 412 of 530 +``` + +The 172 subset is AP-152's population, not AP-156's. The biggest single mover I +found — `gfx 0x010036DD`, 20.762 m offset on a 27.708 m sphere, in Setup +`0x0200129A` — is cited in the commit for its offset but its Setup is not +necessarily in the 172. + +Consequence for the live gate: the user should be told the change touches +**412 installed Setups**, and that the objects most likely to look different +are not confined to the 73 CylSphere + 99 Sphere lists the commit body names. + +### R3 (LOW) — the uncapped third branch is *further* from retail than the 10-cap was + +`src/AcDream.Core/Physics/ShadowObjectRegistry.cs:674`: + +```csharp +int cap = only == ShadowCollisionType.Cylinder ? RetailSphereCap : int.MaxValue; +``` + +`only == null` is the sorting-sphere branch. The commit's own justification for +lifting the cap there is *"the sorting-sphere overload @0x0052b990 takes one +sphere"* — which argues for a cap of **1**, not `int.MaxValue`. The cited +evidence does not support the code written. + +Inert over installed data (I measured max 5 Spheres on any Setup, `0x020016F7`; +max 7 CylSpheres), and AP-157 registers the substitution honestly. But the +comment block at `:664-673` reads as though it justifies the whole line when it +only justifies the BSP half. + +### R4 (LOW) — acdream scales the flood sphere; retail does not — unregistered + +`ShadowShapeBuilder.cs:217` (`boundsCenter * entScale`) and `:330` +(`localCenter * partScale`) are new in this commit. Retail's +`find_transit_cells` uses only `CPhysicsPart::pos` (`+0x30`) and never touches +`gfxobj_scale` (`+0x24`), while `CPhysicsPart::find_obj_collisions` +@`0x0050d8d0` explicitly threads `gfxobj_scale.z` into +`SPHEREPATH::cache_localspace_sphere`. Retail's cross-cell walk is therefore +under-inclusive for scaled parts and acdream's is not. + +Radius scaling predates this commit; centre scaling does not. Direction is +over-inclusive for `scale > 1` (safe) and under-inclusive for `scale < 1` +(the #98/#168 direction). Deserves a sentence in AP-156's residual paragraph; +the row currently lists only the sphere-vs-portal traversal. + +### R5 (INFO) — retail's slack constant, for whoever fixes #333 + +`0x0052cb5f fld dword [0x7c8c70]` = `1.9999999494757503e-4` — the same +`F_EPSILON` AP-30 already byte-confirmed for `Frame::is_equal`. Retail's +cross-cell sphere test is `radius + 0.0002`, not `radius + 2`. + +--- + +## Findings — architecture lens + +### A1 (MEDIUM) — `BoundsCenter = default` reopens, at the type, the exact hole the commit closed at the seam + +`src/AcDream.Core/Physics/ShadowShape.cs:61`: + +```csharp +Vector3 BoundsCenter = default); +``` + +The commit's central structural claim is that *"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."* That is true at +the `LiveEntityCollisionBuilder`/`FromSetup` seam — genuinely well done, and +the single-resolver collapse is the right call. + +It is **not** true of `ShadowShape` itself. This still compiles today: + +```csharp +new ShadowShape(gfxId, pos, rot, scale, ShadowCollisionType.BSP, radius, 0f) +``` + +and silently reproduces AP-156 with a zero centre. Every current BSP producer +passes it, so nothing is broken now — but a future BSP producer (a new static +publisher, a plugin-facing builder, a bake path) gets the old bug for free, +green. + +Concrete failure scenario: someone adds a BSP branch to +`LandblockPhysicsContentBuilder` (which today emits Cylinders at `:658,683` +with no `BoundsCenter`), copies the existing 7-argument call shape, and +reintroduces a 20 m flood mis-placement with no test failing. + +Fix: drop the default and pass `Vector3.Zero` explicitly at the two +primitive sites in `ShadowShapeBuilder` and the four in the publishers. Six +call sites, and the type then enforces the invariant the commit body claims. + +### A2 (MEDIUM) — #333 is a real defect, correctly identified, but deferring it can mask this fix's entire visible benefit + +`e2b2d04c` is honest and well-reasoned: the broadphase at +`TransitionTypes.cs:3757-3765` measures `currPos - obj.Position` (the **part +origin**) against `sphereRadius + obj.Radius + movement + 2f`, where +`obj.Radius` is the root sphere's radius measured about a centre that may be +metres away. Same discarded origin, one layer down. + +I confirmed the two things the issue could not: + +- **Retail has no such filter at all.** `CObjCell::find_obj_collisions` + @`0x0052b750` (pseudo-C 308916-308940) dispatches every non-parented, + non-self shadow object to `CPhysicsObj::FindObjCollisions` unconditionally. + The `+ 2f` and the movement term are acdream's own. The issue's "looks like + acdream's own broadphase rather than a port" is **correct**, and this is an + unregistered divergence that predates the commit — it should carry an AP row, + not only an issue number. +- **Its blast radius is large.** A genuine surface contact is rejected when + `|offset| > sphereRadius_mover + movement + 2` — roughly 2.5 m for a walking + player. **118 of the 477 unique installed physics-BSP GfxObjs** have a root + sphere offset above 2.5 m, and **46** above 5 m. At the test scale 1.75 those + become 4.4 m and 8.75 m against an unchanged ~2.5 m budget. + +The issue's own one-line summary is slightly imprecise — it says contact is +admitted "only when the sphere's centre is within about `movement + 2` metres +of the part origin", where the correct statement is about the **mover's** +distance from the part origin versus `R + r_m + move + 2`. The conclusion is +unchanged. + +Deferral judgement: the split is defensible on attributability grounds (it is +literally the AP-155 lesson), and I would not have bundled a *retail-question* +fix. But the practical consequence must be stated plainly to the user rather +than left in an issue body: **for the tall props with the largest offsets — +exactly the objects the commit body tells the user to go look at — the fix may +produce no visible change at all**, because the geometry now lands in the right +cell and is then discarded by the filter. If the connected gate is run before +#333, a null result on tall props is expected, not evidence against AP-156. + +The minimal correct fix is mechanical (carry `BoundsCenter` on `ShadowEntry` +and measure from `obj.Position + rotate(BoundsCenter, obj.Rotation)`); only the +`+ 2f` question is genuinely open, and it is open independently of that. + +### A3 (LOW) — a cached field became a per-call delegate allocation + +`src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs:136`: + +```csharp +id => _physicsBspBounds(id) is not null, +``` + +replaces the cached `_hasPhysicsBsp` field. The lambda captures `this`, so +Roslyn allocates a fresh `Func` on every `Build` call. `Build` is +per-entity-spawn / per-appearance-rebuild, not per-frame, so this is small — +but the project has spent a whole slice (I1) driving physics-path resolves to +0 B, and a `private readonly Func _hasBounds` initialised once in +both constructors costs nothing and keeps the single-resolver invariant intact. + +Also: the new resolver indexes `flat.Nodes[flat.RootIndex]` where the old gate +only compared `RootIndex >= 0`. A `FlatPhysicsBsp` with `RootIndex >= 0` and an +empty `Nodes` array would now throw where it previously returned `true`. I +found no way to construct one from `FlatCollisionAssetBuilder`, so this is a +note, not a defect. + +### A4 (LOW) — uncapping the BSP branch raises a per-tick cost for moving remotes + +`RuntimeRemotePhysicsUpdater.cs:482` re-floods a remote through +`RegisterMultiPart` on every tick it actually moves, and `RegisterMultiPart` +writes every shape row into every flooded cell. Lifting the cap changes the +flood input from ≤10 spheres to up to 49, and the row count is +`parts × cells` — both factors grow together. + +Bounded in practice: only 7 installed Setups exceed 10 BSP parts, landblock +part arrays are static and register once, and this is what retail does +(`add_shadows_to_cells` @`0x00514ae0` → `CPartArray::AddPartsShadow` walks the +whole part array into every cross-cell). Worth a glance in the connected +session if a dense indoor scene with a many-part animate prop regresses; not +worth pre-emptive work. + +### A5 (LOW) — sabotage A reddens 4 Core tests, not 3 + +Reproduced: `FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint`, +`BuildFloodSpheres_CapsCylSpheresAtTenButNeverTheBspParts`, +`BuildFloodSpheres_BspShape_RotatesTheBoundsCentreByThePartRotation`, +`BuildFloodSpheres_BspShape_CentresOnTheBoundsCentreNotThePartOrigin`. +Coverage is one better than the commit body claims; the ledger is simply stale +by one line. + +### A6 (LOW) — the Content test's "cap control" does not test the cap + +`InstalledSetupBspPrimitiveDispatchTests.cs:302-308` comments that *"EVERY BSP +shape contributes … capping here would silently exclude their tail from the +containment claim below"*, and `:365` asserts +`mostBspShapesOnOneSetup == 49`. But that loop (`:311-319`) is the test's own +re-implementation of the composition with **no cap**, so it cannot observe a +cap regression in `BuildFloodSpheres`. I confirmed empirically: under sabotage +H the Content suite stayed 127/127 green. + +The cap is genuinely covered — by +`BuildFloodSpheres_CapsCylSpheresAtTenButNeverTheBspParts` (sabotages H and I +both redden it, verified). The comment just claims coverage that lives +elsewhere. + +--- + +## Gates reproduced + +All after deleting all 44 `bin`/`obj`. + +| Gate | Claimed | Observed | +|---|---|---| +| Release build | 0 errors, 21 pre-existing warnings | **0 errors, 21 warnings** ✓ | +| Complete suite | 11,208 / 4 skipped / 0 failed | **11,208 / 4 / 0** ✓ | +| Core | 4268 | **4268** (1 skip) ✓ | +| Content | 127 | **127** (0 skips) ✓ | +| App | unchanged, one rename | **4173** (3 skips) ✓ | +| Delta vs `4abd1b5e` | +5 | consistent: +4 Core (3 in `ShadowObjectRegistryMultiPartTests`, 1 in `ShadowRegistrationOverflowTests`) +1 Content, App rename only. Baseline **not** independently re-run — I did not check out `4abd1b5e` in a worktree another agent may share. | +| Active AP register rows | 109 | **109**, literal count of non-struck `AP-` rows in section 3 ✓ (AD 48, IA 18, UN 4) | + +Suite run twice — once on the pristine tree before any sabotage, once on a +fully clean rebuild after every restore. Identical both times. No flake from +#302 / #308 / #321 was hit; nothing was conflated with them. + +## Rules compliance + +- **No workaround, suppression flag, grace period, retry loop or symptom + guard** anywhere in the diff. The fix is at the source: the resolver that + answers "does this dispatch as BSP?" is now the same one that answers "where + and how big is its sphere?". +- **No new skips.** 4 skipped, all pre-existing. +- **No test weakened.** `BspOnlyPart_UsesRealScaledPhysicsBoundingRadius` → + `…BoundingSphere` is a rename that **adds** an assertion; the + `LiveAppearanceAnimationTests` and `Builder()` edits drop a now-merged + parameter with no behavioural change. Every flood fixture moved from + concentric to off-centre, which is strictly stronger. +- **Register discipline honoured.** AP-155 narrowed with its false direction + corrected, AP-156 and AP-157 filed, AP-152 retired, count reconciles at a + literal 109 — all in the same commit as the code. This is the register rule + working as designed: AP-155(b)'s recorded direction was the *stated reason* + the residual was safe to defer, and it was backwards. + +## What I checked and found clean, so the PASS is auditable + +- All 12 cited retail addresses resolve to the named symbol; no mis-cite. +- `BSPTREE::GetSphere`, `find_transit_cells`, `find_bbox_cell_list`, + `calc_cross_cells_static`, `calc_cross_cells`, both `find_cell_list` + overloads, `CacheHasPhysicsBSP`, `GetSortingSphere` — all disassembled from + the PDB-paired binary, not inherited. +- `CGfxObj`, `CPhysicsPart`, `BSPNODE`, `CSphere`, `CSetup` field offsets + cross-checked against `acclient.h` and against the code that reads them. +- Every `new ShadowShape(...)` site in `src/` (8) classified; only two emit BSP + and both carry the centre. +- Every site that composes a shape's world placement (5) classified; none reads + the old semantics. +- Every `BuildShadowCellSet` caller (2) classified; the single-shape + `Register` overload has no production BSP caller. +- `FlatPhysicsBsp.Nodes[RootIndex].BoundingSphere` traced to + `gfxObj.PhysicsBSP.Root.BoundingSphere` in both `PhysicsDataCache` + population paths (`:216`, `:281-287`) and in + `FlatCollisionAssetBuilder` (`:84`, `:393`) — acdream's seam is retail's + `physics_sphere`. +- `ReplaceMultiPartPayload`'s no-reflood behaviour checked against + `CPartArray::SetPart` semantics — retail-faithful. +- Quaternion composition order (`entityWorldRot * s.LocalRotation`, then + `Vector3.Transform(BoundsCenter, partWorldRot)`) checked against the inverse + transform in `TransitionTypes.cs:3859-3876` — consistent. +- Six sabotages reproduced; tree confirmed clean by `git status --porcelain` + before and after. + +--- + +## Recommended, in priority order + +1. Tell the user the live-gate criterion is **containment, not size**, and that + "a prop stopped blocking" is only a bug if its own BSP geometry still + reaches the cell (Part 6). State the population as **412 Setups** (R2). +2. Warn that tall props with large offsets may show **no change** until #333 + lands, and that a null result there is expected (A2). +3. Fix #333 — it is mechanical apart from the `+ 2f` question — and give the + `maxReach` filter its own AP row, since retail has no such filter at all + (A2). +4. Make `ShadowShape.BoundsCenter` required (A1). +5. Swap the Content containment oracle to physics-polygon vertices (R1) — it + passes today, costs nothing, and makes the load-bearing assertion mean + what it says. +6. Add the scale sentence to AP-156's residual paragraph (R4). diff --git a/docs/research/2026-08-06-ap156-review-closure.md b/docs/research/2026-08-06-ap156-review-closure.md new file mode 100644 index 00000000..36d60924 --- /dev/null +++ b/docs/research/2026-08-06-ap156-review-closure.md @@ -0,0 +1,218 @@ +# AP-156 fix review — closure record + +Closes the six findings in +[`2026-08-06-ap156-fix-review.md`](2026-08-06-ap156-fix-review.md) (both lenses +PASS; this is cleanup before merge, not a rescue). Worktree +`.claude/worktrees/resume-session-e0bd03e1-d5bf45`, base `e2b2d04c`. + +All 44 `bin`/`obj` deleted before every verdict-deciding build, and every test +run gated on a grep for `Build succeeded` in the same invocation — the trap the +review documented (a sabotage that trips `CS0219` under +`TreatWarningsAsErrors` fails the build and lets the *previous* sabotage's DLL +report the wrong failure) was not hit. + +--- + +## R1 — the containment oracle now discriminates + +**Done.** `InstalledSetupBspPrimitiveDispatchTests` no longer compares the +emitted flood sphere against a hand-rebuilt copy of the same sphere. It +transforms every **physics-polygon vertex** +(`GfxObj.PhysicsPolygons` → `GfxObj.VertexArray.Vertices`) of every physics-BSP +part into the entity frame and asserts some emitted flood sphere covers it. +Vertices are a different DAT field from the bounding sphere, so the two sides of +the comparison are no longer the same expression. Renamed to +`InstalledSetups_BspFloodSpheres_ContainTheirOwnPhysicsPolygons`. + +New population + defect controls: 973 physics-BSP parts, 530 BSP-bearing +Setups, 376 off-centre parts, **91,689** physics vertices, 428 pre-fix +failures, deepest part array 49. Every one measured first by a scratch +DatReaderWriter console program **outside the repo** that references no acdream +assembly, then reproduced exactly through the production builder. + +**Sabotage-verified, three ways, each after a full clean and a verified +`Build succeeded`:** + +| # | Sabotage | Result | +|---|---|---| +| S1 | `ShadowShape.Bsp` drops the bounds centre (the AP-156 defect itself) | **RED — 428 Setups, worst 35.869 m on 0x0200129A** (matches the independent sweep exactly) | +| S2 | `ShadowShape.Bsp` applies the scale to the radius but not the centre | **RED — 326 Setups, worst 15.288 m** | +| S3 | the TEST's own `Bounds()` oracle corrupted (`bs.Origin` negated) | **RED — 467 Setups, worst 72.134 m** | +| S1b | same sabotage, run against Core rather than Content | **RED — 5 tests**: the 4 the review's sabotage A found, plus `ShadowRegistrationOverflowTests.FromLandblockBspParts_CarriesTheScaledRootSphereCentre`, which sabotage A could not reach because it only touched `BuildFloodSpheres`. The factory refactor makes both BSP producers share one scaling path, so one sabotage now covers both. | +| — | restored | **GREEN — 127/127 Content, 4,268/1 skip Core** | + +S3 is the one that matters for R1. Under the shipped oracle that same +corruption was invisible by algebra: `truth` was +`(partOrigin + rotate(O, R))·s` and `flood` was +`LocalPosition + rotate(BoundsCenter, R)` = `partOrigin·s + rotate(O·s, R)`, so +negating `O` moved both sides identically and the shortfall stayed ≡ 0. The +vertex oracle catches it. That is the tenth green-test-covering-nothing this +campaign, and this is the direction of sabotage that proves it is closed — +breaking the production code was never the hard half. + +## R2 — population corrected, and refined + +**Done, with a refinement the review will want.** The review says 412, not +"170 of 172". Both figures are right about different things, and neither is the +number to quote to the user: + +| Question | Answer | +|---|---| +| Setups carrying both a primitive and a physics-BSP part (AP-152's DISPATCH population) | **172** of 5,935 | +| BSP-bearing Setups (AP-156's population) | **530** | +| …whose flood sphere actually MOVES because of the fix | **525** | +| …failing vertex-level containment before the fix, 1 mm tolerance | **428** | +| …failing at a 1 cm tolerance | **412** ← the review's figure | +| …failing after the fix, at any tolerance down to zero | **0** | + +The 412/428 gap is 16 Setups whose pre-fix shortfall lands between 1.4 mm and +10 mm — real geometry, three orders of magnitude above float noise at these +radii, not a measurement artifact. The test keeps its existing 1 mm tolerance +and therefore pins **428**; the register records both and names the tolerance, +because a bare "412" is unreproducible without it. + +**The number to tell the user is 525** — Setups whose behaviour changes — not +172, and not 412. Corrected in the AP-156 row, the section-3 header, the C5c +handoff, and the two stale test docstrings. The dated review documents +(`2026-08-06-ap152-review-retail.md`, and the fix review itself) are left +as written: "170 of 172" was correct for what those reviews measured +(root-sphere containment over the 172), and rewriting a dated evidence +artifact to match a later measurement is how provenance gets lost. + +The commit's own record cannot be corrected in place — `b52967de` is HEAD~1 and +amending it would rewrite published history. This document plus the register row +is that correction. + +## A1 — the hole is closed at the type + +**Done, and deliberately stronger than the review's recommendation.** The review +suggested dropping the `= default` and passing `Vector3.Zero` explicitly at six +call sites. That does **not** close the review's own failure scenario: someone +adding a BSP branch to `LandblockPhysicsContentBuilder` by copying the adjacent +Cylinder call would then write `Vector3.Zero` explicitly and get the same +20 m mis-placement, still green. + +Instead `ShadowShape`'s constructor is now **private**, and the three factories +are the only way to build one: + +```csharp +ShadowShape.Bsp(gfxObjId, localPosition, localRotation, scale, FlatCollisionSphere localBounds) +ShadowShape.Cylinder(gfxObjId, localPosition, localRotation, scale, radius, cylHeight) +ShadowShape.Sphere(gfxObjId, localPosition, localRotation, scale, radius) +``` + +`Bsp` takes the radius and the centre as **one `FlatCollisionSphere` value** and +scales them together inside the factory. There is no longer an expression a +caller can write that carries one and drops the other — which is the commit +body's own claim ("the radius cannot be taken while the origin is dropped"), +now true of the type and not only of the `FromSetup` seam. 22 construction +sites converted; S1 and S2 above are exactly the two ways the factory could +still be got wrong, and both are red. + +Residual, recorded rather than fixed: `default(ShadowShape)` remains +constructible because C# structs always have a parameterless constructor. It +yields `CollisionType = BSP, Radius = 0` — inert, and no production path +produces one. + +## A2 — #333 updated, AP-158 filed, and the retail claim independently verified + +**Done.** I disassembled `CObjCell::find_obj_collisions` @`0x0052b750` myself +from the PDB-paired binary (`check_exe_pdb.py` → **MATCH**, CodeView GUID +`9e847e2f-777c-4bd9-886c-22256bb87f32`, linker 2013-09-06T00:17:56Z) rather +than inheriting the claim, because Binary Ninja drops flag tests and that has +bitten this campaign repeatedly: + +``` +0x0052b759 cmp dword [ebx+0x174], 2 ; sphere_path.insert_type +0x0052b765 je 0x52b7a0 ; INITIAL_PLACEMENT_INSERT -> return OK_TS +0x0052b773 mov ecx,[edi+0xc8] ; shadow_object_list.data +0x0052b77f mov edx,[ecx+0x40] ; physobj->parent -> skip if set +0x0052b786 cmp ecx,[ebx] ; self -> skip +0x0052b78b call 0x50f050 ; FindObjCollisions — UNCONDITIONAL +0x0052b79e jb 0x52b773 ; loop, bound [edi+0xc4] +``` + +The single early-out is a transition-state test, not a distance test — the whole +function is 0x54 bytes and there is no float comparison in it. Confirms the +review: **retail has no distance pre-filter at all**, so acdream's +`sphereRadius + obj.Radius + movement.Length() + 2f` is an invention. +`[ebx+0x174]` resolves to `sphere_path.insert_type` against `acclient.h`'s +`CTransition` layout and the pseudo-C at 308916-308940 agrees. + +Blast radius reproduced independently: **118 of the 477** unique installed +physics-BSP GfxObjs exceed the ~2.5 m budget, **46** exceed 5 m. + +Filed as **AP-158** — the filter had no register row at all — carrying the +disassembly, the `F_EPSILON` = 1.9999999e-4 m contrast (R5), the measured blast +radius, and the "this can mask AP-156's entire visible benefit" consequence. +#333 updated: its research question is struck through and answered, and what +remains is a judgement call (keep the filter with a correct measurement point, +or delete it and walk the list as retail does — the retail-faithful option, +which should be costed first). + +## The consequence that had to be recorded prominently + +**Tall props may show no visible change at all until #333 lands, and a null +result at the connected gate is EXPECTED, not evidence against AP-156.** +Recorded in three places a reader will actually hit: the AP-156 register row's +`WHAT REMAINS OPEN`, the AP-158 row's risk column, and item 7 of the C5c +handoff's §3.1 gate list — alongside the other half of the same warning, that +"a prop stopped blocking" is only a bug if the object's own BSP geometry still +reaches the cell it stopped blocking from. + +## LOW items + +**R3 — `int.MaxValue` on the sorting-sphere branch.** *Comment corrected; code +deliberately unchanged, and I think the review's implied fix is wrong.* The +review is right that the cited evidence (`0x0052b9d6 push 1`) argues for a cap +of 1, not `int.MaxValue`, and the comment block claimed to justify the whole +line when it justified only the BSP half — that is fixed, and the comment now +says plainly which half is a port and which is not. But capping at 1 would take +`Spheres[0]`, and retail's one sphere is `CSetup::sorting_sphere`, a **different +DAT field**. Capping would not move toward retail; it would keep the wrong field +and additionally make the substitution under-inclusive, which is the #98/#168 +direction. `int.MaxValue` keeps it over-inclusive until AP-157 ports the real +field, which is the honest state. Inert over installed data either way — max 5 +Spheres on any Setup (0x020016F7). + +**R4 — acdream scales the flood sphere, retail does not.** Recorded as a second +residual on the AP-156 row. `find_transit_cells` @0x0052cae0 reads only +`CPhysicsPart::pos` (`+0x30`) and never `gfxobj_scale` (`+0x24`), while +`CPhysicsPart::find_obj_collisions` @0x0050d8d0 does thread `gfxobj_scale.z` +into `SPHEREPATH::cache_localspace_sphere`. Direction: over-inclusive for +scale > 1, under-inclusive for scale < 1. Radius scaling predates the commit; +centre scaling did not, which is why it needed the row. + +**R5 — retail's slack is 0.0002 m, not 2 m.** Carried into AP-158's anchor +column and into #333, where whoever fixes the filter will hit it. + +**A3 — per-call delegate allocation.** Fixed: `LiveEntityCollisionBuilder` now +holds `private readonly Func _hasPhysicsBsp`, initialised once in the +single real constructor and still derived from `_physicsBspBounds`, so the +single-resolver invariant is intact and `Build` allocates no closure. + +**A6 — the Content test's "cap control" does not test the cap.** Comment +corrected: the loop is the test's own uncapped re-implementation and cannot +observe a cap regression; the cap is covered by +`BuildFloodSpheres_CapsCylSpheresAtTenButNeverTheBspParts`, which reddens under +both cap sabotages. What `mostBspShapesOnOneSetup == 49` genuinely proves — +that the containment claim reaches Setups past the retired clamp rather than +stopping short of them — is now what the comment claims. + +**A5 — sabotage A reddens 4 Core tests, not 3.** Noted; `b52967de`'s message +cannot be amended. Coverage is one better than advertised, which is the benign +direction. + +--- + +## Gates + +| Gate | Baseline `e2b2d04c` | Observed | +|---|---|---| +| Release build, all 44 `bin`/`obj` deleted | 0 errors, 21 warnings | **0 errors, 21 warnings** | +| Complete suite, `-m:1`, `ACDREAM_PAK_PATH` set | 11,208 / 4 skipped / 0 failed | **11,208 / 4 / 0** — reconciles exactly; one test RENAMED, none added or removed | +| Content | 127 / 0 skips | **127 / 0 skips** | +| Active AP register rows | 109 | **110** (AP-158 filed) | + +No new skips. Nothing was conflated with the known load-sensitive flakes +#302 / #308 / #321. diff --git a/docs/research/2026-08-06-c5c-closeout-handoff.md b/docs/research/2026-08-06-c5c-closeout-handoff.md index f7ee887f..173a6f10 100644 --- a/docs/research/2026-08-06-c5c-closeout-handoff.md +++ b/docs/research/2026-08-06-c5c-closeout-handoff.md @@ -138,6 +138,29 @@ complete. 5. **AD-65 / AD-66** need a local-player visual gate before any fix (see §5). 6. **C5c proper**: two-client observation, canonical nine-stop soak, lifecycle/reconnect route on the final binary, and the user's visual matrix. +7. **AP-156 — the BSP flood-sphere placement fix (`b52967de`).** + **The criterion is CONTAINMENT, not size, and a null result on tall props is + EXPECTED.** Two things must be said to the user before this one is run, or + its outcome will be misread in both directions: + - *"A prop stopped blocking"* is **not** by itself a bug report. AP-156 + shrinks the flood for 143 of the 172 AP-152 Setups on purpose, because + retail's BSP branch (`calc_cross_cells` @0x00515230, `0x0051528f jne`) + contributes **no primitive at all** — the old flood was larger only + because it included a sphere retail never reads. The question to ask of + any such observation is whether the object's own BSP geometry still + reaches the cell it stopped blocking from. If it does, that is a bug; if + it does not, that is retail. + - *"A tall prop STILL does not block"* is the expected symptom of + **AP-158 / #333**, not of this fix. AP-156 puts the geometry in the right + cell; acdream's own `maxReach` broadphase filter — which retail does not + have at all — then discards it one layer down, for 118 of the 477 unique + installed physics-BSP GfxObjs. **A null result on tall props is not + evidence against AP-156.** Run this gate after #333, or run it on + short/wide off-centre props where the offset is inside the ~2.5 m budget. + + Population, for the user: the change touches **525 of the 530** BSP-bearing + Setups (not the 172 the commit body names — that is AP-152's dispatch + population). See the AP-156 register row. ### 3.2 Then, and only then diff --git a/src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs b/src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs index 3059c162..bb7907e5 100644 --- a/src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs +++ b/src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs @@ -48,6 +48,12 @@ internal sealed record LiveEntityCollisionRegistration( internal sealed class LiveEntityCollisionBuilder { private readonly Func _physicsBspBounds; + /// + /// The dispatch gate, derived from so the + /// two can never disagree (AP-156), and cached once so + /// allocates no closure per call — Slice I1's 0 B/resolve budget. + /// + private readonly Func _hasPhysicsBsp; private readonly LiveEntityDefaultPoseResolver _defaultPose; public LiveEntityCollisionBuilder( @@ -80,6 +86,7 @@ internal sealed class LiveEntityCollisionBuilder { _physicsBspBounds = physicsBspBounds ?? throw new ArgumentNullException(nameof(physicsBspBounds)); + _hasPhysicsBsp = id => _physicsBspBounds(id) is not null; _defaultPose = defaultPose ?? throw new ArgumentNullException(nameof(defaultPose)); } @@ -134,7 +141,7 @@ internal sealed class LiveEntityCollisionBuilder IReadOnlyList shapes = ShadowShapeBuilder.FromSetup( setup, scale, - id => _physicsBspBounds(id) is not null, + _hasPhysicsBsp, partPoseOverride: defaultPose, effectivePartGfxObjIds: effectivePartGfxObjIds, physicsBspBounds: _physicsBspBounds); diff --git a/src/AcDream.App/Streaming/LandblockPhysicsPublisher.cs b/src/AcDream.App/Streaming/LandblockPhysicsPublisher.cs index 175e70fc..c835091c 100644 --- a/src/AcDream.App/Streaming/LandblockPhysicsPublisher.cs +++ b/src/AcDream.App/Streaming/LandblockPhysicsPublisher.cs @@ -1000,14 +1000,13 @@ public sealed class LandblockPhysicsPublisher continue; Vector3 localOffset = cylinder.Origin * scale; - setupShapes.Add(new ShadowShape( - GfxObjId: entity.SourceGfxObjOrSetupId, - LocalPosition: localOffset, - LocalRotation: Quaternion.Identity, - Scale: scale, - CollisionType: ShadowCollisionType.Cylinder, - Radius: radius, - CylHeight: height)); + setupShapes.Add(ShadowShape.Cylinder( + gfxObjId: entity.SourceGfxObjOrSetupId, + localPosition: localOffset, + localRotation: Quaternion.Identity, + scale: scale, + radius: radius, + cylHeight: height)); } if (setup.Cylinders.Length == 0) @@ -1027,14 +1026,13 @@ public sealed class LandblockPhysicsPublisher + Vector3.Transform( -Vector3.UnitZ * radius, Quaternion.Inverse(entity.Rotation)); - setupShapes.Add(new ShadowShape( - GfxObjId: entity.SourceGfxObjOrSetupId, - LocalPosition: localBaseOffset, - LocalRotation: Quaternion.Identity, - Scale: scale, - CollisionType: ShadowCollisionType.Cylinder, - Radius: radius, - CylHeight: radius * 2f)); + setupShapes.Add(ShadowShape.Cylinder( + gfxObjId: entity.SourceGfxObjOrSetupId, + localPosition: localBaseOffset, + localRotation: Quaternion.Identity, + scale: scale, + radius: radius, + cylHeight: radius * 2f)); } } diff --git a/src/AcDream.Content/LandblockPhysicsContentBuilder.cs b/src/AcDream.Content/LandblockPhysicsContentBuilder.cs index fe1e0fcc..74b50ef0 100644 --- a/src/AcDream.Content/LandblockPhysicsContentBuilder.cs +++ b/src/AcDream.Content/LandblockPhysicsContentBuilder.cs @@ -655,12 +655,11 @@ public static class LandblockPhysicsContentBuilder : cylinder.Radius * 4f) * scale; if (radius <= 0f) continue; - setupShapes.Add(new ShadowShape( + setupShapes.Add(ShadowShape.Cylinder( entity.SourceGfxObjOrSetupId, cylinder.Origin * scale, Quaternion.Identity, scale, - ShadowCollisionType.Cylinder, radius, height)); } @@ -680,12 +679,11 @@ public static class LandblockPhysicsContentBuilder + Vector3.Transform( -Vector3.UnitZ * radius, Quaternion.Inverse(entity.Rotation)); - setupShapes.Add(new ShadowShape( + setupShapes.Add(ShadowShape.Cylinder( entity.SourceGfxObjOrSetupId, localBaseOffset, Quaternion.Identity, scale, - ShadowCollisionType.Cylinder, radius, radius * 2f)); } diff --git a/src/AcDream.Core/Physics/ShadowObjectRegistry.cs b/src/AcDream.Core/Physics/ShadowObjectRegistry.cs index f1e3e0a5..d4f0bb2a 100644 --- a/src/AcDream.Core/Physics/ShadowObjectRegistry.cs +++ b/src/AcDream.Core/Physics/ShadowObjectRegistry.cs @@ -663,14 +663,28 @@ public sealed class ShadowObjectRegistry // 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. + // 0x0052ba21 cmp eax,0xa / 0x0052ba28 mov ebp,0xa, and that clamp is a + // fixed static-buffer capacity (the destination array at + // 0x844838..0x8448d8 is exactly ten 16-byte entries), not a policy. + // + // BSP branch: NO CAP, and this is a retail port. find_bbox_cell_list + // @0x00510fc0 -> CPartArray::calc_cross_cells_static @0x00518160 -> + // CEnvCell::find_transit_cells @0x0052cae0 walks every part, bounded + // only by num_parts. Clamping it 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. + // + // only == null (the sorting-sphere branch): int.MaxValue is NOT a + // retail port and the addresses above do not justify it. Retail's + // overload @0x0052b990 pushes a literal 1 (0x0052b9d6 push 1) and + // floods from ONE authored CSetup::sorting_sphere. acdream floods from + // every Sphere shape instead — a different DAT field with a different + // cardinality, which is AP-157, filed and open. Capping at 1 HERE would + // not move toward retail: it would take Spheres[0], which is not the + // sorting sphere. int.MaxValue keeps the substitution in its safe + // (over-inclusive) direction until AP-157 ports the real field. Inert + // over installed data — max 5 Spheres on any Setup (0x020016F7). int cap = only == ShadowCollisionType.Cylinder ? RetailSphereCap : int.MaxValue; foreach (var s in shapes) diff --git a/src/AcDream.Core/Physics/ShadowShape.cs b/src/AcDream.Core/Physics/ShadowShape.cs index 9737b321..b559d88c 100644 --- a/src/AcDream.Core/Physics/ShadowShape.cs +++ b/src/AcDream.Core/Physics/ShadowShape.cs @@ -15,47 +15,158 @@ namespace AcDream.Core.Physics; /// at acclient_2013_pseudo_c.txt:275045-275055. Each part stores its /// own transform and dispatches per-part collision to its GfxObj. /// +/// +/// +/// CONSTRUCTION IS BY FACTORY ONLY (, , +/// ) and the constructor is private. That is the AP-156 +/// invariant expressed at the type rather than only at the producer: a BSP +/// shape's radius and its bounding-sphere CENTRE arrive as one +/// value and are scaled together inside +/// , so no call site — present or future — can take the +/// radius while dropping the origin. That split is exactly what produced +/// AP-156, and with the old public 7-argument constructor a new BSP producer +/// could have reintroduced it silently and green. +/// /// -/// -/// Center of the shape's bounding sphere IN THE SHAPE'S OWN LOCAL FRAME — -/// the frame rotates out of and -/// is measured in — already multiplied by the entity -/// scale, like and . -/// Zero for Cylinder/Sphere shapes, whose -/// already IS their center. -/// -/// -/// 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 client_portal.dat sit further from it than half their -/// own radius; worst 20.762 m on a 27.708 m sphere, gfx 0x010036DD). -/// 's flood needs the -/// sphere's real position, not the part origin's. -/// -/// -/// -/// Retail anchor: CGfxObj::physics_sphere ([gfxobj+0x74]) is -/// assigned BSPTREE::GetSphere(physics_bsp) @0x005397e0 -/// (mov eax,[ecx]; add eax,4 — the root BSPNODE's -/// CSphere sphere, past its 4-byte vftable), so retail's per-part -/// flood sphere IS the BSP root bounding sphere, origin included. -/// CEnvCell::find_transit_cells @0x0052cae0 — the part-array overload -/// reached from CPhysicsObj::find_bbox_cell_list @0x00510fc0 via -/// CPartArray::calc_cross_cells_static @0x00518160's -/// [vtbl+0x7c] dispatch — loads that sphere at -/// 0x0052cb36 mov esi,[ecx+0x74], transforms its CENTER through the -/// part's own Position at [part+0x30] -/// (0x0052cb4c add eax,0x30 / 0x0052cb5a call Position::localtolocal), -/// and only then reads the radius at 0x0052cb65 fadd [esi+0xc]. -/// -/// -public readonly record struct ShadowShape( - uint GfxObjId, - Vector3 LocalPosition, - Quaternion LocalRotation, - float Scale, - ShadowCollisionType CollisionType, - float Radius, - float CylHeight, - Vector3 BoundsCenter = default); +public readonly record struct ShadowShape +{ + private ShadowShape( + uint gfxObjId, + Vector3 localPosition, + Quaternion localRotation, + float scale, + ShadowCollisionType collisionType, + float radius, + float cylHeight, + Vector3 boundsCenter) + { + GfxObjId = gfxObjId; + LocalPosition = localPosition; + LocalRotation = localRotation; + Scale = scale; + CollisionType = collisionType; + Radius = radius; + CylHeight = cylHeight; + BoundsCenter = boundsCenter; + } + + /// Source GfxObj id, for the BSP walk and for diagnostics. + public uint GfxObjId { get; } + + /// Part placement in the entity's own frame, entity-scaled. + public Vector3 LocalPosition { get; } + + /// Part orientation in the entity's own frame. + public Quaternion LocalRotation { get; } + + /// The entity (or part) scale already applied to the geometry. + public float Scale { get; } + + public ShadowCollisionType CollisionType { get; } + + /// Collision radius, entity-scaled. + public float Radius { get; } + + /// Cylinder height, entity-scaled; 0 for BSP and Sphere. + public float CylHeight { get; } + + /// + /// Center of the shape's bounding sphere IN THE SHAPE'S OWN LOCAL FRAME — + /// the frame rotates out of and + /// is measured in — already multiplied by the entity + /// scale, like and . + /// Zero for Cylinder/Sphere shapes, whose + /// already IS their center. + /// + /// + /// 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 client_portal.dat sit further from it than half their + /// own radius; worst 20.762 m on a 27.708 m sphere, gfx 0x010036DD). + /// 's flood needs the + /// sphere's real position, not the part origin's. + /// + /// + /// + /// Retail anchor: CGfxObj::physics_sphere ([gfxobj+0x74]) is + /// assigned BSPTREE::GetSphere(physics_bsp) @0x005397e0 + /// (mov eax,[ecx]; add eax,4 — the root BSPNODE's + /// CSphere sphere, past its 4-byte vftable), so retail's per-part + /// flood sphere IS the BSP root bounding sphere, origin included. + /// CEnvCell::find_transit_cells @0x0052cae0 — the part-array overload + /// reached from CPhysicsObj::find_bbox_cell_list @0x00510fc0 via + /// CPartArray::calc_cross_cells_static @0x00518160's + /// [vtbl+0x7c] dispatch — loads that sphere at + /// 0x0052cb36 mov esi,[ecx+0x74], transforms its CENTER through the + /// part's own Position at [part+0x30] + /// (0x0052cb4c add eax,0x30 / 0x0052cb5a call Position::localtolocal), + /// and only then reads the radius at 0x0052cb65 fadd [esi+0xc]. + /// + /// + public Vector3 BoundsCenter { get; } + + /// + /// One physics-BSP part. is the part + /// GfxObj's physics-BSP ROOT bounding sphere in the GfxObj's OWN frame, + /// unscaled — retail's CGfxObj::physics_sphere. Radius and centre + /// are scaled together here, which is the whole point of taking them as + /// one value. + /// + public static ShadowShape Bsp( + uint gfxObjId, + Vector3 localPosition, + Quaternion localRotation, + float scale, + FlatCollisionSphere localBounds) + => new( + gfxObjId, + localPosition, + localRotation, + scale, + ShadowCollisionType.BSP, + localBounds.Radius * scale, + 0f, + localBounds.Origin * scale); + + /// + /// One Setup CylSphere. already IS the + /// shape's centre, so it carries no separate bounds centre. + /// + public static ShadowShape Cylinder( + uint gfxObjId, + Vector3 localPosition, + Quaternion localRotation, + float scale, + float radius, + float cylHeight) + => new( + gfxObjId, + localPosition, + localRotation, + scale, + ShadowCollisionType.Cylinder, + radius, + cylHeight, + Vector3.Zero); + + /// + /// One Setup Sphere. already IS the + /// shape's centre, so it carries no separate bounds centre. + /// + public static ShadowShape Sphere( + uint gfxObjId, + Vector3 localPosition, + Quaternion localRotation, + float scale, + float radius) + => new( + gfxObjId, + localPosition, + localRotation, + scale, + ShadowCollisionType.Sphere, + radius, + 0f, + Vector3.Zero); +} diff --git a/src/AcDream.Core/Physics/ShadowShapeBuilder.cs b/src/AcDream.Core/Physics/ShadowShapeBuilder.cs index ff384f5f..f121e481 100644 --- a/src/AcDream.Core/Physics/ShadowShapeBuilder.cs +++ b/src/AcDream.Core/Physics/ShadowShapeBuilder.cs @@ -145,14 +145,13 @@ public static class ShadowShapeBuilder { if (cyl.Radius <= 0f) continue; float baseHeight = cyl.Height > 0f ? cyl.Height : cyl.Radius * 4f; - result.Add(new ShadowShape( - GfxObjId: 0u, - LocalPosition: new Vector3(cyl.Origin.X, cyl.Origin.Y, cyl.Origin.Z) * entScale, - LocalRotation: Quaternion.Identity, - Scale: entScale, - CollisionType: ShadowCollisionType.Cylinder, - Radius: cyl.Radius * entScale, - CylHeight: baseHeight * entScale)); + result.Add(ShadowShape.Cylinder( + gfxObjId: 0u, + localPosition: new Vector3(cyl.Origin.X, cyl.Origin.Y, cyl.Origin.Z) * entScale, + localRotation: Quaternion.Identity, + scale: entScale, + radius: cyl.Radius * entScale, + cylHeight: baseHeight * entScale)); } // 2. Spheres — only when no CylSpheres. Retail's CylSphere loop @@ -166,14 +165,12 @@ public static class ShadowShapeBuilder foreach (var sph in setup.Spheres) { if (sph.Radius <= 0f) continue; - result.Add(new ShadowShape( - GfxObjId: 0u, - LocalPosition: new Vector3(sph.Origin.X, sph.Origin.Y, sph.Origin.Z) * entScale, - LocalRotation: Quaternion.Identity, - Scale: entScale, - CollisionType: ShadowCollisionType.Sphere, - Radius: sph.Radius * entScale, - CylHeight: 0f)); + result.Add(ShadowShape.Sphere( + gfxObjId: 0u, + localPosition: new Vector3(sph.Origin.X, sph.Origin.Y, sph.Origin.Z) * entScale, + localRotation: Quaternion.Identity, + scale: entScale, + radius: sph.Radius * entScale)); } } } @@ -211,20 +208,19 @@ public static class ShadowShapeBuilder // 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; + // Absent bounds keep the loose-but-safe 2 m placeholder, centred + // on the part origin because nothing better is known. + // ShadowShape.Bsp scales radius and centre together. + FlatCollisionSphere bounds = + physicsBspBounds?.Invoke(gfxId) + ?? new FlatCollisionSphere(Vector3.Zero, 2f); - result.Add(new ShadowShape( - GfxObjId: gfxId, - LocalPosition: new Vector3(partFrame.Origin.X, partFrame.Origin.Y, partFrame.Origin.Z) * entScale, - LocalRotation: partFrame.Orientation, - Scale: entScale, - CollisionType: ShadowCollisionType.BSP, - Radius: bspRadius, - CylHeight: 0f, - BoundsCenter: boundsCenter)); + result.Add(ShadowShape.Bsp( + gfxObjId: gfxId, + localPosition: new Vector3(partFrame.Origin.X, partFrame.Origin.Y, partFrame.Origin.Z) * entScale, + localRotation: partFrame.Orientation, + scale: entScale, + localBounds: bounds)); } return result; @@ -305,29 +301,19 @@ public static class ShadowShapeBuilder // 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; - } + FlatCollisionSphere localBounds = + hasFlat + ? flat!.Nodes[flat.RootIndex].BoundingSphere + : new FlatCollisionSphere( + phys.BoundingSphere?.Origin ?? Vector3.Zero, + phys.BoundingSphere?.Radius ?? 1f); - shapes.Add(new ShadowShape( - GfxObjId: meshRef.GfxObjId, - LocalPosition: pPos, - LocalRotation: pRot, - Scale: partScale, - CollisionType: ShadowCollisionType.BSP, - Radius: localRadius * partScale, - CylHeight: 0f, - BoundsCenter: localCenter * partScale)); + shapes.Add(ShadowShape.Bsp( + gfxObjId: meshRef.GfxObjId, + localPosition: pPos, + localRotation: pRot, + scale: partScale, + localBounds: localBounds)); } return shapes; diff --git a/tests/AcDream.App.Tests/Physics/LiveEntityCollisionBuilderTests.cs b/tests/AcDream.App.Tests/Physics/LiveEntityCollisionBuilderTests.cs index 572cd07b..3f9ddd80 100644 --- a/tests/AcDream.App.Tests/Physics/LiveEntityCollisionBuilderTests.cs +++ b/tests/AcDream.App.Tests/Physics/LiveEntityCollisionBuilderTests.cs @@ -105,8 +105,10 @@ public sealed class LiveEntityCollisionBuilderTests /// CEnvCell::find_transit_cells @0x0052cae0 transforms its centre /// through the part's Position (0x0052cb4c add eax,0x30) before /// reading the radius at 0x0052cb65. 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. + /// AP-156: 428 of the 530 installed BSP-bearing Setups then flood from + /// spheres that do not contain their own physics polygons. (The row as + /// filed said "170 of 172"; 172 is AP-152's DISPATCH population, not + /// AP-156's containment population — corrected at the fix review.) /// [Fact] public void BspOnlyPart_UsesRealScaledPhysicsBoundingSphere() diff --git a/tests/AcDream.App.Tests/Physics/RemotePhysicsUpdaterTests.cs b/tests/AcDream.App.Tests/Physics/RemotePhysicsUpdaterTests.cs index aea08387..9b3136b8 100644 --- a/tests/AcDream.App.Tests/Physics/RemotePhysicsUpdaterTests.cs +++ b/tests/AcDream.App.Tests/Physics/RemotePhysicsUpdaterTests.cs @@ -81,14 +81,13 @@ public sealed class RemotePhysicsUpdaterTests entity.Position, entity.Rotation, [ - new ShadowShape( + ShadowShape.Cylinder( 0x01000001u, new Vector3(1f, 0f, 0f), Quaternion.Identity, - Scale: 1f, - CollisionType: ShadowCollisionType.Cylinder, - Radius: 0.25f, - CylHeight: 1f), + scale: 1f, + radius: 0.25f, + cylHeight: 1f), ], state: (uint)PhysicsStateFlags.ReportCollisions, flags: EntityCollisionFlags.None, diff --git a/tests/AcDream.App.Tests/Rendering/RetailStaticAnimatingObjectSchedulerTests.cs b/tests/AcDream.App.Tests/Rendering/RetailStaticAnimatingObjectSchedulerTests.cs index 7cfc97c7..163d1770 100644 --- a/tests/AcDream.App.Tests/Rendering/RetailStaticAnimatingObjectSchedulerTests.cs +++ b/tests/AcDream.App.Tests/Rendering/RetailStaticAnimatingObjectSchedulerTests.cs @@ -357,14 +357,13 @@ public sealed class RetailStaticAnimatingObjectSchedulerTests entity.Rotation, new[] { - new ShadowShape( + ShadowShape.Cylinder( GfxId, Vector3.UnitX, Quaternion.Identity, - Scale: 1f, - CollisionType: ShadowCollisionType.Cylinder, - Radius: 0.5f, - CylHeight: 1f), + scale: 1f, + radius: 0.5f, + cylHeight: 1f), }, state: 0u, flags: EntityCollisionFlags.None, diff --git a/tests/AcDream.App.Tests/World/LiveEntityRuntimeTests.cs b/tests/AcDream.App.Tests/World/LiveEntityRuntimeTests.cs index b3ae14e6..0e2fe770 100644 --- a/tests/AcDream.App.Tests/World/LiveEntityRuntimeTests.cs +++ b/tests/AcDream.App.Tests/World/LiveEntityRuntimeTests.cs @@ -364,14 +364,13 @@ public sealed class LiveEntityRuntimeTests entity.Rotation, new[] { - new ShadowShape( + ShadowShape.Cylinder( 0x01000001u, Vector3.UnitX, Quaternion.Identity, - Scale: 1f, - CollisionType: ShadowCollisionType.Cylinder, - Radius: 0.5f, - CylHeight: 1f), + scale: 1f, + radius: 0.5f, + cylHeight: 1f), }, state: (uint)PhysicsStateFlags.ReportCollisions, flags: EntityCollisionFlags.None, @@ -475,14 +474,13 @@ public sealed class LiveEntityRuntimeTests oldEntity.Rotation, new[] { - new ShadowShape( + ShadowShape.Cylinder( 0x01000001u, Vector3.UnitX, Quaternion.Identity, - Scale: 1f, - CollisionType: ShadowCollisionType.Cylinder, - Radius: 0.5f, - CylHeight: 1f), + scale: 1f, + radius: 0.5f, + cylHeight: 1f), }, state: (uint)PhysicsStateFlags.ReportCollisions, flags: EntityCollisionFlags.None, diff --git a/tests/AcDream.Content.Tests/InstalledSetupBspPrimitiveDispatchTests.cs b/tests/AcDream.Content.Tests/InstalledSetupBspPrimitiveDispatchTests.cs index 0a71f18a..460c8ae7 100644 --- a/tests/AcDream.Content.Tests/InstalledSetupBspPrimitiveDispatchTests.cs +++ b/tests/AcDream.Content.Tests/InstalledSetupBspPrimitiveDispatchTests.cs @@ -161,27 +161,53 @@ public sealed class InstalledSetupBspPrimitiveDispatchTests } // 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. + // scratch DatReaderWriter console program OUTSIDE the repo that resolves + // every quantity from client_portal.dat by hand and references no acdream + // assembly. 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. + // BspBearingSetups / PhysicsBspParts / OffCentreParts / PhysicsVertices are + // population controls: without them a broken enumeration, a wrong dat path, + // a silently-empty polygon decode, or a build in which every BSP root + // sphere happened to sit at its part origin would all satisfy the + // containment claim vacuously. + // + // WouldFailIfOriginDiscarded is the DEFECT control: it re-runs the pre-fix + // composition (radius carried, root-sphere origin dropped) against the same + // oracle and pins how many Setups it breaks. If that number ever goes to + // zero the fixture population has stopped exercising the field and the + // containment assertion has stopped meaning anything. + // + // NOTE ON THE POPULATION (AP-156 review finding R2). The defect population + // is NOT the 172 AP-152 Setups. 172 is the DISPATCH population — Setups + // carrying both a primitive and a physics-BSP part. After AP-152 every + // BSP-bearing Setup floods from its BSP shapes alone, so a discarded root + // origin mis-places the flood for all 530 of them. 525 have at least one + // flood sphere move; 428 fail vertex-level containment at the 1 mm + // tolerance below (412 at a 1 cm tolerance — the figure the review quotes). private const int ExpectedPhysicsBspParts = 973; + private const int ExpectedBspBearingSetups = 530; private const int ExpectedOffCentreParts = 376; // |origin| > radius/2 - private const int ExpectedWouldFailIfOriginDiscarded = 170; // of 172 + private const int ExpectedPhysicsVertices = 91689; + private const int ExpectedWouldFailIfOriginDiscarded = 428; // of 530 private const int ExpectedDeepestBspPartArray = 49; // Setup 0x02001A91 /// /// AP-156. Every flood sphere acdream emits for a physics-BSP part must - /// CONTAIN that part's real collision geometry. + /// CONTAIN that part's real collision geometry — and the oracle for "real + /// collision geometry" is the part's PHYSICS-POLYGON VERTICES, not its + /// bounding sphere. + /// + /// + /// That distinction is the point. The first version of this test compared + /// the emitted flood sphere against a hand-rebuilt copy of the same + /// bounding sphere from the same resolver, which made the shortfall + /// algebraically identically zero for any DAT input — a green test that + /// could not fail (review finding R1). Vertices come from a DIFFERENT DAT + /// field (GfxObj.PhysicsPolygons -> GfxObj.VertexArray) + /// than the bounding sphere the builder emits, so the assertion now has + /// something real to disagree with: any error in which sphere is read, + /// where it is placed, or how it is scaled shows up as an uncovered vertex. + /// /// /// /// A GfxObj's physics BSP is authored in the GfxObj's own coordinates and @@ -189,12 +215,9 @@ public sealed class InstalledSetupBspPrimitiveDispatchTests /// 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 (CellTransit.BuildShadowCellSet routes every candidate - /// with id & 0xFFFF >= 0x0100 through + /// its origin, flooding from the part origin instead. Indoor floods are + /// 3-D (CellTransit.BuildShadowCellSet routes every candidate with + /// id & 0xFFFF >= 0x0100 through /// FindTransitCellsSphere), 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. @@ -212,7 +235,7 @@ public sealed class InstalledSetupBspPrimitiveDispatchTests /// /// [Fact] - public void InstalledSetups_BspFloodSpheres_ContainTheirOwnBoundingSpheres() + public void InstalledSetups_BspFloodSpheres_ContainTheirOwnPhysicsPolygons() { string? datDir = ContentConformanceDats.ResolveDatDir(); if (datDir is null) @@ -220,12 +243,17 @@ public sealed class InstalledSetupBspPrimitiveDispatchTests using var dats = new DatCollection(datDir, DatAccessType.Read); + // Two INDEPENDENT reads of the same GfxObj: the bounding sphere the + // builder is handed, and the physics-polygon vertices that are the + // truth it must cover. Only the sphere is fed to ShadowShapeBuilder. var boundsCache = new Dictionary(); + var vertexCache = new Dictionary(); FlatCollisionSphere? Bounds(uint gfxObjId) { if (boundsCache.TryGetValue(gfxObjId, out FlatCollisionSphere? cached)) return cached; FlatCollisionSphere? result = null; + Vector3[] vertices = []; if (dats.Portal.TryGet(gfxObjId, out GfxObj? gfx) && gfx is not null && gfx.Flags.HasFlag(GfxObjFlags.HasPhysics) @@ -234,15 +262,31 @@ public sealed class InstalledSetupBspPrimitiveDispatchTests && gfx.PhysicsBSP.Root.BoundingSphere is { } bs) { result = new FlatCollisionSphere(bs.Origin, bs.Radius); + var collected = new List(); + foreach (var polygon in gfx.PhysicsPolygons.Values) + { + foreach (var vertexId in polygon.VertexIds) + { + if (gfx.VertexArray.Vertices.TryGetValue( + (ushort)vertexId, out var vertex)) + { + collected.Add(vertex.Origin); + } + } + } + vertices = collected.ToArray(); } boundsCache[gfxObjId] = result; + vertexCache[gfxObjId] = vertices; return result; } const float EntScale = 1.75f; // not 1: a dropped scale must show up + const float Tolerance = 1e-3f; int bspParts = 0; + int bspBearingSetups = 0; int offCentreParts = 0; - int affected = 0; + int physicsVertices = 0; int wouldFailIfOriginDiscarded = 0; float worstShortfall = 0f; uint worstShortfallSetup = 0u; @@ -255,7 +299,7 @@ public sealed class InstalledSetupBspPrimitiveDispatchTests continue; // Independent oracle: resolve the placement frame from the raw - // Setup and place each part's TRUE root sphere by hand. + // Setup and place each part's TRUE physics polygons by hand. AnimationFrame? placement = null; if (setup.PlacementFrames.TryGetValue(Placement.Resting, out var resting)) placement = resting; @@ -263,10 +307,11 @@ public sealed class InstalledSetupBspPrimitiveDispatchTests placement = def; else foreach (var kvp in setup.PlacementFrames) { placement = kvp.Value; break; } - var truth = new List<(Vector3 Centre, float Radius)>(); + var truth = new List(); for (int i = 0; i < setup.Parts.Count; i++) { - FlatCollisionSphere? b = Bounds((uint)setup.Parts[i]); + uint partGfxObjId = (uint)setup.Parts[i]; + FlatCollisionSphere? b = Bounds(partGfxObjId); if (b is null) continue; bspParts++; if (b.Value.Origin.Length() > b.Value.Radius / 2f) @@ -279,18 +324,15 @@ public sealed class InstalledSetupBspPrimitiveDispatchTests partOrigin = placement.Frames[i].Origin; partRot = placement.Frames[i].Orientation; } - truth.Add(( - (partOrigin + Vector3.Transform(b.Value.Origin, partRot)) * EntScale, - b.Value.Radius * EntScale)); + foreach (Vector3 vertex in vertexCache[partGfxObjId]) + { + truth.Add( + (partOrigin + Vector3.Transform(vertex, partRot)) * 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++; + bspBearingSetups++; + physicsVertices += truth.Count; // Production emission, through the production bounds seam. IReadOnlyList shapes = ShadowShapeBuilder.FromSetup( @@ -301,11 +343,17 @@ public sealed class InstalledSetupBspPrimitiveDispatchTests // 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. + // Deliberately UNCAPPED, matching production: retail's BSP branch + // has no sphere cap (the 10-clamp at 0x0052ba21 is inside the + // cylsphere overload only). This loop is the test's own + // re-implementation, so it cannot observe a cap regression in + // BuildFloodSpheres — that is covered by + // ShadowObjectRegistryMultiPartTests + // .BuildFloodSpheres_CapsCylSpheresAtTenButNeverTheBspParts, which + // reddens under both cap sabotages. What the mostBspShapesOnOneSetup + // assertion below DOES prove is that the containment claim reaches + // Setups past the retired 10-sphere clamp rather than stopping short + // of them. var flood = new List<(Vector3 Centre, float Radius)>(); var floodIfOriginDiscarded = new List<(Vector3 Centre, float Radius)>(); foreach (ShadowShape shape in shapes) @@ -323,12 +371,12 @@ public sealed class InstalledSetupBspPrimitiveDispatchTests float Shortfall(List<(Vector3 Centre, float Radius)> spheres) { float worst = 0f; - foreach ((Vector3 tc, float tr) in truth) + foreach (Vector3 point in truth) { float best = float.MaxValue; foreach ((Vector3 fc, float fr) in spheres) { - float need = (tc - fc).Length() + tr - fr; + float need = (point - fc).Length() - fr; if (need < best) best = need; } if (best > worst) worst = best; @@ -336,7 +384,6 @@ public sealed class InstalledSetupBspPrimitiveDispatchTests return worst; } - const float Tolerance = 1e-3f; float shortfall = Shortfall(flood); if (shortfall > Tolerance) { @@ -347,28 +394,23 @@ public sealed class InstalledSetupBspPrimitiveDispatchTests worstShortfallSetup = id; } } - if ((hasCylinder || (setup.CylSpheres.Count == 0 && hasSphere)) - && Shortfall(floodIfOriginDiscarded) > Tolerance) - { + if (Shortfall(floodIfOriginDiscarded) > Tolerance) wouldFailIfOriginDiscarded++; - } } // Population + defect controls first. Assert.Equal(ExpectedPhysicsBspParts, bspParts); + Assert.Equal(ExpectedBspBearingSetups, bspBearingSetups); Assert.Equal(ExpectedOffCentreParts, offCentreParts); - Assert.Equal(ExpectedAffected, affected); + Assert.Equal(ExpectedPhysicsVertices, physicsVertices); 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 " + $"{uncontained.Count} Setups flood from spheres that do not contain " + + $"their own physics-polygon geometry; worst shortfall " + $"{worstShortfall.ToString("F3", CultureInfo.InvariantCulture)} m on " + $"Setup 0x{worstShortfallSetup:X8}."); } diff --git a/tests/AcDream.Core.Tests/Physics/DoorBugTrajectoryReplayTests.cs b/tests/AcDream.Core.Tests/Physics/DoorBugTrajectoryReplayTests.cs index b6522f5f..f9d6ea7f 100644 --- a/tests/AcDream.Core.Tests/Physics/DoorBugTrajectoryReplayTests.cs +++ b/tests/AcDream.Core.Tests/Physics/DoorBugTrajectoryReplayTests.cs @@ -921,7 +921,12 @@ public class DoorBugTrajectoryReplayTests { var phys = cache.GetGfxObj(s.GfxObjId); float bspR = phys?.BoundingSphere?.Radius ?? 2f; - shapes.Add(s with { Radius = bspR }); + shapes.Add(ShadowShape.Bsp( + s.GfxObjId, + s.LocalPosition, + s.LocalRotation, + s.Scale, + new FlatCollisionSphere(Vector3.Zero, bspR / s.Scale))); } else { @@ -1103,23 +1108,20 @@ public class DoorBugTrajectoryReplayTests // to the BSP slab; the cylinder is a small foot collider). // entityWorldPos = BSP world pos so LocalPos=0 puts the BSP at // the captured center. cellScope=0u mirrors production. - var bspShape = new ShadowShape( - GfxObjId: DoorGfxObjId, - LocalPosition: Vector3.Zero, - LocalRotation: Quaternion.Identity, - Scale: 1f, - CollisionType: ShadowCollisionType.BSP, - Radius: BspRadius, - CylHeight: 0f); + var bspShape = ShadowShape.Bsp( + gfxObjId: DoorGfxObjId, + localPosition: Vector3.Zero, + localRotation: Quaternion.Identity, + scale: 1f, + localBounds: new FlatCollisionSphere(Vector3.Zero, BspRadius)); - var cylShape = new ShadowShape( - GfxObjId: 0u, - LocalPosition: CylWorldPos - BspWorldPos, // express cyl relative to entity origin - LocalRotation: Quaternion.Identity, - Scale: 1f, - CollisionType: ShadowCollisionType.Cylinder, - Radius: CylRadius, - CylHeight: CylHeight); + var cylShape = ShadowShape.Cylinder( + gfxObjId: 0u, + localPosition: CylWorldPos - BspWorldPos, // express cyl relative to entity origin + localRotation: Quaternion.Identity, + scale: 1f, + radius: CylRadius, + cylHeight: CylHeight); engine.ShadowObjects.RegisterMultiPart( entityId: DoorEntityId, diff --git a/tests/AcDream.Core.Tests/Physics/ShadowObjectRegistryMultiPartTests.cs b/tests/AcDream.Core.Tests/Physics/ShadowObjectRegistryMultiPartTests.cs index 0941697f..ea9864a5 100644 --- a/tests/AcDream.Core.Tests/Physics/ShadowObjectRegistryMultiPartTests.cs +++ b/tests/AcDream.Core.Tests/Physics/ShadowObjectRegistryMultiPartTests.cs @@ -14,38 +14,31 @@ public class ShadowObjectRegistryMultiPartTests private static IReadOnlyList DoorShapes() => new[] { - new ShadowShape( - GfxObjId: 0u, - LocalPosition: new Vector3(0f, 0f, 0.018f), - LocalRotation: Quaternion.Identity, - Scale: 1.0f, - CollisionType: ShadowCollisionType.Cylinder, - Radius: 0.100f, - CylHeight: 0.200f), - new ShadowShape( - GfxObjId: 0x010044B5u, - LocalPosition: Vector3.Zero, - LocalRotation: Quaternion.Identity, - Scale: 1.0f, - CollisionType: ShadowCollisionType.BSP, - Radius: 2.0f, - CylHeight: 0f), - new ShadowShape( - GfxObjId: 0x010044B6u, - LocalPosition: Vector3.Zero, - LocalRotation: Quaternion.Identity, - Scale: 1.0f, - CollisionType: ShadowCollisionType.BSP, - Radius: 2.0f, - CylHeight: 0f), - new ShadowShape( - GfxObjId: 0x010044B6u, - LocalPosition: Vector3.Zero, - LocalRotation: Quaternion.Identity, - Scale: 1.0f, - CollisionType: ShadowCollisionType.BSP, - Radius: 2.0f, - CylHeight: 0f) + ShadowShape.Cylinder( + gfxObjId: 0u, + localPosition: new Vector3(0f, 0f, 0.018f), + localRotation: Quaternion.Identity, + scale: 1.0f, + radius: 0.100f, + cylHeight: 0.200f), + ShadowShape.Bsp( + gfxObjId: 0x010044B5u, + localPosition: Vector3.Zero, + localRotation: Quaternion.Identity, + scale: 1.0f, + localBounds: new FlatCollisionSphere(Vector3.Zero, 2.0f)), + ShadowShape.Bsp( + gfxObjId: 0x010044B6u, + localPosition: Vector3.Zero, + localRotation: Quaternion.Identity, + scale: 1.0f, + localBounds: new FlatCollisionSphere(Vector3.Zero, 2.0f)), + ShadowShape.Bsp( + gfxObjId: 0x010044B6u, + localPosition: Vector3.Zero, + localRotation: Quaternion.Identity, + scale: 1.0f, + localBounds: new FlatCollisionSphere(Vector3.Zero, 2.0f)) }; [Fact] @@ -134,10 +127,8 @@ public class ShadowObjectRegistryMultiPartTests // Two shapes 30m apart in X — must span two outdoor 24m cells. var shapes = new[] { - new ShadowShape(0u, new Vector3( 0f, 0f, 0f), Quaternion.Identity, 1f, - ShadowCollisionType.Cylinder, 1f, 2f), - new ShadowShape(0u, new Vector3(30f, 0f, 0f), Quaternion.Identity, 1f, - ShadowCollisionType.Cylinder, 1f, 2f), + ShadowShape.Cylinder(0u, new Vector3( 0f, 0f, 0f), Quaternion.Identity, 1f, 1f, 2f), + ShadowShape.Cylinder(0u, new Vector3(30f, 0f, 0f), Quaternion.Identity, 1f, 1f, 2f), }; reg.RegisterMultiPart(0x1u, new Vector3(12f, 12f, 50f), Quaternion.Identity, shapes, 0u, EntityCollisionFlags.None, OffX, OffY, LbId); @@ -170,10 +161,8 @@ public class ShadowObjectRegistryMultiPartTests var shapes = new[] { - new ShadowShape(0u, new Vector3(0f, 0f, 0f), Quaternion.Identity, 1f, - ShadowCollisionType.Cylinder, 0.5f, 1f), - new ShadowShape(0u, new Vector3(1f, 0f, 0f), Quaternion.Identity, 1f, - ShadowCollisionType.Cylinder, 0.5f, 1f), + ShadowShape.Cylinder(0u, new Vector3(0f, 0f, 0f), Quaternion.Identity, 1f, 0.5f, 1f), + ShadowShape.Cylinder(0u, new Vector3(1f, 0f, 0f), Quaternion.Identity, 1f, 0.5f, 1f), }; reg.RegisterMultiPart(movingEntityId, new Vector3(10f, 10f, 50f), Quaternion.Identity, shapes, 0u, @@ -243,10 +232,14 @@ public class ShadowObjectRegistryMultiPartTests return cells; } - private static ShadowShape Cyl(float radius) => new( - GfxObjId: 0u, LocalPosition: Vector3.Zero, LocalRotation: Quaternion.Identity, - Scale: 1f, CollisionType: ShadowCollisionType.Cylinder, - Radius: radius, CylHeight: radius * 2f); + private static ShadowShape Cyl(float radius, Vector3 localPosition = default) + => ShadowShape.Cylinder( + gfxObjId: 0u, + localPosition: localPosition, + localRotation: Quaternion.Identity, + scale: 1f, + radius: radius, + cylHeight: radius * 2f); /// /// A physics-BSP part shape. defaults @@ -261,13 +254,15 @@ public class ShadowObjectRegistryMultiPartTests 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, - Radius: radius, CylHeight: 0f, - BoundsCenter: boundsCenter == default ? new Vector3(0f, 6f, 0f) : boundsCenter); + Quaternion localRotation = default) + => ShadowShape.Bsp( + gfxObjId: 0x010044B5u, + localPosition: localPosition, + localRotation: localRotation == default ? Quaternion.Identity : localRotation, + scale: 1f, + localBounds: new FlatCollisionSphere( + boundsCenter == default ? new Vector3(0f, 6f, 0f) : boundsCenter, + radius)); private static List FloodCellsFor(params ShadowShape[] shapes) { @@ -317,9 +312,12 @@ public class ShadowObjectRegistryMultiPartTests /// /// /// 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. + /// Over the installed DAT that failed to contain the parts' own physics + /// polygons for 428 of the 530 BSP-bearing Setups, shortfall up to + /// 35.869 m at entity scale 1.75 (0x0200129A) — under-inclusive + /// membership, the #98 / #168 class. (172 is AP-152's DISPATCH + /// population; the AP-156 row's original "170 of 172" understated this + /// 2.5x and was corrected at the fix review.) /// /// [Fact] @@ -377,8 +375,8 @@ public class ShadowObjectRegistryMultiPartTests 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 }; + cyls[i] = Cyl(1f, near); + cyls[10] = Cyl(1f, far); List cylCells = FloodCellsFor(cyls); Assert.Contains(ownCell, bspCells); diff --git a/tests/AcDream.Core.Tests/Physics/ShadowObjectRegistryTests.cs b/tests/AcDream.Core.Tests/Physics/ShadowObjectRegistryTests.cs index 5a461ca8..8542e130 100644 --- a/tests/AcDream.Core.Tests/Physics/ShadowObjectRegistryTests.cs +++ b/tests/AcDream.Core.Tests/Physics/ShadowObjectRegistryTests.cs @@ -967,14 +967,13 @@ public class ShadowObjectRegistryTests }; } - private static ShadowShape BspShape(uint gfxObjId, float radius) => new( - gfxObjId, - Vector3.Zero, - Quaternion.Identity, - Scale: 1f, - CollisionType: ShadowCollisionType.BSP, - Radius: radius, - CylHeight: 0f); + private static ShadowShape BspShape(uint gfxObjId, float radius) + => ShadowShape.Bsp( + gfxObjId, + Vector3.Zero, + Quaternion.Identity, + scale: 1f, + localBounds: new FlatCollisionSphere(Vector3.Zero, radius)); private static CellPhysics BuildShadowCellSetTests_MakeLeafCell(Matrix4x4 worldTransform) { diff --git a/tests/AcDream.Core.Tests/Physics/ShadowRegistrationOverflowTests.cs b/tests/AcDream.Core.Tests/Physics/ShadowRegistrationOverflowTests.cs index 3639cb6a..e898623f 100644 --- a/tests/AcDream.Core.Tests/Physics/ShadowRegistrationOverflowTests.cs +++ b/tests/AcDream.Core.Tests/Physics/ShadowRegistrationOverflowTests.cs @@ -48,9 +48,9 @@ public class ShadowRegistrationOverflowTests // ── The bug: old per-part Register loses one registration ───────────── - private static ShadowShape Cyl(Vector3 local) => new( - GfxObjId: 0u, LocalPosition: local, LocalRotation: Quaternion.Identity, - Scale: 1f, CollisionType: ShadowCollisionType.Cylinder, Radius: 1f, CylHeight: 2f); + private static ShadowShape Cyl(Vector3 local) => ShadowShape.Cylinder( + gfxObjId: 0u, localPosition: local, localRotation: Quaternion.Identity, + scale: 1f, radius: 1f, cylHeight: 2f); [Fact] public void OldPerPartRegister_CollidingIds_SecondSilentlyOverwritesFirst() diff --git a/tests/AcDream.Core.Tests/Physics/ShadowSetPositionCommitTests.cs b/tests/AcDream.Core.Tests/Physics/ShadowSetPositionCommitTests.cs index 9c8136ba..52dd5292 100644 --- a/tests/AcDream.Core.Tests/Physics/ShadowSetPositionCommitTests.cs +++ b/tests/AcDream.Core.Tests/Physics/ShadowSetPositionCommitTests.cs @@ -461,12 +461,11 @@ public sealed class ShadowSetPositionCommitTests return registry; } - private static ShadowShape Shape(uint gfxObjId, Vector3 local) => new( - gfxObjId, - local, - Quaternion.Identity, - 1f, - ShadowCollisionType.BSP, - 0.25f, - 0f); + private static ShadowShape Shape(uint gfxObjId, Vector3 local) + => ShadowShape.Bsp( + gfxObjId, + local, + Quaternion.Identity, + scale: 1f, + localBounds: new FlatCollisionSphere(Vector3.Zero, 0.25f)); }