From b52967def3c3f859810fd4ab0654bc48615f8b6b Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 6 Aug 2026 15:52:04 +0200 Subject: [PATCH] =?UTF-8?q?fix(physics):=20AP-156=20=E2=80=94=20flood=20th?= =?UTF-8?q?e=20BSP=20sphere=20where=20the=20geometry=20is,=20not=20at=20th?= =?UTF-8?q?e=20part=20origin?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The AP-152 retail review (docs/research/2026-08-06-ap152-review-retail.md) FAILED `4abd1b5e` and is right. `ShadowObjectRegistry.BuildFloodSpheres` took each physics-BSP part's ROOT BOUNDING SPHERE RADIUS (FlatCollisionAssetBuilder.cs:393 -> LiveEntityCollisionBuilder.cs:137) and centred it on the PART ORIGIN (ShadowShapeBuilder.cs:194), discarding the root sphere's own Origin. Re-measured independently against the installed client_portal.dat, reproducing the reviewer's numbers exactly: 376 of 973 physics-BSP parts have |origin| > radius/2, worst 20.762 m on a 27.708 m sphere (gfx 0x010036DD, Setup 0x0200129A). Over the 172 Setups AP-152 moved onto that path the emitted flood FAILED TO CONTAIN the object's own BSP sphere for 170 of them (73 CylSphere-bearing, 97 Sphere-bearing), worst shortfall 9.911 m on Setup 0x02000255 — whose one part's sphere sits 9.911 m above the part origin — and for 43 the post-AP-152 flood was strictly SMALLER than the pre-AP-152 one. Indoor flooding is 3-D (CellTransit.cs:601 routes every id & 0xFFFF >= 0x0100 candidate through FindTransitCellsSphere), so a tall prop or door slab was absent from EnvCells it physically occupies and therefore never a broadphase candidate there (TransitionTypes.cs:3763 iterates only entries already in the cell). That is the #98 / #168 class AP-152 exists to remove. Retail, re-disassembled from the PDB-paired binary (check_exe_pdb.py MATCH, CodeView GUID 9e847e2f-777c-4bd9-886c-22256bb87f32), every address resolved back through named-retail/symbols.json: CGfxObj::physics_sphere is [gfxobj+0x74] (physics_bsp is [+0x78], as CPartArray::CacheHasPhysicsBSP @0x00518110 reads at 0x00518127), and acclient pseudo-C 0x00534b5b assigns it BSPTREE::GetSphere(physics_bsp). BSPTREE::GetSphere @0x005397e0 8b01 mov eax,[ecx] ; BSPTREE::root_node 83c004 add eax,4 ; past BSPNODE::vfptr -> CSphere sphere So retail's per-part flood sphere IS the BSP root bounding sphere, ORIGIN INCLUDED (acclient.h: BSPNODE { vfptr; CSphere sphere; ... }, CSphere { Vector3 center; float radius; } -> radius at +0xc). CPhysicsObj::find_bbox_cell_list @0x00510fc0 adds the object's own cell and then walks the PART ARRAY: 0x00511012 call 0x518160 (CPartArray::calc_cross_cells_static), which dispatches [edx+0x7c] with (num_parts, parts, cellarray). Its EnvCell body, CEnvCell::find_transit_cells @0x0052cae0: 0x0052cb31 mov edx,[eax+0x20] ; CPhysicsPart::gfxobj (CGfxObj**) 0x0052cb36 mov esi,[ecx+0x74] ; physics_sphere (else +0x90 drawing) 0x0052cb4c add eax,0x30 ; CPhysicsPart::pos 0x0052cb5a call Position::localtolocal ; transform the sphere CENTRE 0x0052cb65 fadd [esi+0xc] ; only NOW the radius Retail transforms the centre through the part's own Position before it ever touches the radius. Carrying the radius alone is not an approximation of that; it is a different sphere. Changes: * `ShadowShape` gains `BoundsCenter` — the bounding sphere's centre in the shape's own local frame, scaled like LocalPosition and Radius. Zero for Cylinder/Sphere shapes, whose LocalPosition already IS their centre. * `ShadowShapeBuilder.FromSetup` gains a `physicsBspBounds` resolver that supplies radius AND centre from ONE call, replacing the placeholder radius plus a downstream substitution. `LiveEntityCollisionBuilder` now holds a single `Func` and derives its dispatch predicate from it, so the gate and the geometry cannot disagree and the radius cannot be taken while the origin is dropped. That split is what produced this bug; it no longer exists. * `FromLandblockBspParts` carries the centre too. A landblock-baked part array is the same CPartArray walk, so stair runs, fences and rock clusters had the identical defect. Both storage forms (flat BSP and the graph fallback) are covered. * `BuildFloodSpheres` places each sphere at partWorldPos + rotate(BoundsCenter, partWorldRot), composed exactly as the ShadowEntry rows are. * The 10-sphere clamp now applies to the CYLSPHERE branch only. Retail's clamp is inside CObjCell::find_cell_list @0x0052b9f0 (0x0052ba21 cmp eax,0xa / 0x0052ba28 mov ebp,0xa); the BSP walk has none and the sorting-sphere overload @0x0052b990 takes one sphere. 7 installed Setups carry more than 10 physics-BSP parts (max 49, Setup 0x02001A91) and their tail parts were dropped from the flood entirely. Without this the new containment assertion would have covered shapes production never floods from. Register. AP-155 was two divergences with different code paths, populations and gates under one id; it is NARROWED to its static-publication half and its flood half is split out as AP-156 WITH ITS DIRECTION CORRECTED. AP-155(b) recorded the approximation as over-inclusive — "floods MORE cells rather than fewer, the safe direction for membership" — and that false direction was the stated reason the residual was safe to defer. It was under-inclusive for 170 of 172. AP-156 records the correction, this fix, and the one genuine residual: acdream's sphere-vs-portal traversal where retail walks each part's sphere against the cell's own portal planes. AP-155(b)'s "acdream approximates retail's bounding BOX" was wrong too — find_bbox_cell_list forms no box. AP-157 filed for the review's F4: retail's third branch floods from ONE CPartArray::GetSortingSphere @0x00518b00 ([partArray+0x54]+0x70 = CSetup::sorting_sphere; 4,154 of 5,935 installed Setups carry a non-zero one) where acdream floods from every Sphere shape, and acdream's cylinder flood ignores CylHeight. Deliberately NOT bundled here: different branch, disjoint population, different live gate. Active AP rows 107 -> 109, literal count. Tests. Both flood tests the review named substituted a CONCENTRIC Radius = 14f at LocalPosition = Zero — the one configuration in which the defect cannot appear. Every fixture is now off-centre by default, and `FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint` drives the production `physicsBspBounds` seam instead of hand-substituting. Five new facts: the flood centres on BoundsCenter not the part origin; it rotates BoundsCenter by the part rotation; it caps cylspheres at ten but never the BSP parts; the landblock path carries the scaled centre in both storage forms; and an installed-DAT containment sweep asserting every emitted BSP flood sphere contains that part's real bounding sphere at entity scale 1.75, behind four external controls — 973 parts, 376 off-centre, 172 affected, and 170 would-fail-if-the-origin-were-discarded, the last of which fails if the population ever stops exercising the field. Nine sabotages, each reverted and re-verified: A drop BoundsCenter from the flood -> 3 Core B rotate by entity rot, not part rot -> 1 Core (the rotation fact only) C FromSetup discards the origin -> 1 Core + 2 App + 1 Content (the shipped defect, now caught in three projects) D drop entScale on BoundsCenter -> 2 App + 1 Content E landblock flat branch drops the centre -> 1 Core F landblock graph branch drops it -> 1 Core G drop partScale on the landblock centre -> 1 Core H re-apply the 10-cap to every branch -> 1 Core I remove the cylsphere cap -> 1 Core AP-152's own two sabotages re-run against this tree: the step-0 gate disabled still reddens exactly its five facts with Headless 89/89 green, and cylinder-first flooding still reddens exactly one. Clean Release build after deleting all 44 bin/obj: 0 errors, 21 pre-existing warnings. Complete suite 11,208 passed / 4 skipped / 0 failed, +5 on the 11,203 baseline at 4abd1b5e — Core 4264 -> 4268, Content 126 -> 127, App unchanged (one rename, not an addition). No new skips. NOT yet gated live. This moves shadow-cell membership for real objects, in both directions, and the connected session must look for both: props and doors that START blocking from a neighbouring cell (the 73 CylSphere+BSP Setups), AND ones that STOP blocking (the 99 Sphere+BSP Setups can shrink; 43 shrink below their pre-4abd1b5e size, which is the regression this fixes). Tall indoor props and door slabs — the ones whose sphere sits metres above the part origin — are where the change is largest. Co-Authored-By: Claude Opus 4.8 --- .../retail-divergence-register.md | 6 +- .../2026-08-06-ap152-review-architecture.md | 370 ++++++++++++++++ .../2026-08-06-ap152-review-retail.md | 416 ++++++++++++++++++ .../Physics/LiveEntityCollisionBuilder.cs | 54 ++- .../Physics/ShadowObjectRegistry.cs | 56 ++- src/AcDream.Core/Physics/ShadowShape.cs | 42 +- .../Physics/ShadowShapeBuilder.cs | 67 ++- .../LiveEntityCollisionBuilderTests.cs | 49 ++- ...pBitfieldSurvivesAppearanceRebuildTests.cs | 5 +- .../Rendering/LiveAppearanceAnimationTests.cs | 1 - ...InstalledSetupBspPrimitiveDispatchTests.cs | 216 +++++++++ .../ShadowObjectRegistryMultiPartTests.cs | 167 ++++++- .../ShadowRegistrationOverflowTests.cs | 65 ++- 13 files changed, 1424 insertions(+), 90 deletions(-) create mode 100644 docs/research/2026-08-06-ap152-review-architecture.md create mode 100644 docs/research/2026-08-06-ap152-review-retail.md diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 9622a48a..77ba77fe 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) — 107 active rows (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) — 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) 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 @@ -182,7 +182,9 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps. | AP-151 | **Filed 2026-08-06 at the #280 retail-conformance review (finding F3).** The reveal gate is materially STRICTER than retail's prefetch predicate on the mesh-build/GPU-upload axis, over an equally large square. Retail's `LScape::PreFetchCells` @0x00505660 requires, per member, only that the DAT records be resident in memory (`DBObj::PreFetch` -> `IN_MEMORY` or `IN_FILE` -> `DBObj::Get` non-null); no geometry construction, no vertex arrays and no GPU upload are part of the blocking predicate — that work happens lazily at draw. acdream's gate requires, for every member of the derived window (25x25 at the shipped High preset): a worker-thread DAT read, a terrain mesh build, a render-thread `TerrainModernRenderer.AddLandblock` upload, a spatial commit, a physics collision-generation admission, and a spawn-adapter activation, all metered at `MaxCompletionsPerFrame`. The hold is therefore systematically longer than retail's for identical content, and nothing currently bounds it. Note this is the OPPOSITE asymmetry from AP-149, which records where the outer ring is WEAKER than retail; both are live simultaneously, on different axes. | `src/AcDream.App/Streaming/StreamingController.cs` (`IsRenderNeighborhoodResident`); `src/AcDream.App/Streaming/GpuWorldState.cs` (`IsRenderReady`); `src/AcDream.App/Rendering/TerrainModernRenderer.cs`; `src/AcDream.App/Streaming/StreamingWorkBudget.cs` | It is what makes "no visible assembly after reveal" true at all: acdream draws through a bindless/MDI pipeline whose landblock slots must exist before the viewport opens, where retail can begin drawing a landblock the frame its DAT record lands. Weakening the predicate to DAT residency would restore retail's hold duration and reintroduce the visible-assembly artifact #280 exists to remove. AD-2's blanket "async readiness gates replace retail's synchronous destination cell load" pre-dates the window being 625 members wide and does not name this axis. | Portal/recall holds of several seconds where retail (warm cache) is near-instant, on EVERY transit rather than only on cold DAT. No upper bound is enforced and no progress readout is shown (#327). A slow disk or a saturated upload budget lengthens the hold without limit. | `LScape::PreFetchCells` 0x00505660; `DBObj::PreFetch`/`DBObj::Get` call sites @0x0050575C, @0x0050579C; `CellManager::PreFetchCells` 0x00455820 | | 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; its cell-membership half is CLOSED by that commit, its static half is not.** Two shape-source divergences between the live and static publication paths, neither previously registered. **(a) 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`, which is 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. **(b) `ShadowObjectRegistry.BuildFloodSpheres` used retail's SECOND priority first** — it preferred Cylinders over everything whenever any Cylinder existed, where `CPhysicsObj::calc_cross_cells` @0x00515230 tests `HAS_PHYSICS_BSP_PS` at 0x00515285 and routes a BSP-bearing object to `CPhysicsObj::find_bbox_cell_list` @0x00510fc0 (`0x0051528f jne 0x515305`) BEFORE reaching its cylsphere branch (0x005152d1) or its sorting-sphere branch (0x005152fb). **(b) is fixed in this commit**: the method now chooses BSP -> Cylinder -> everything else, matching retail's order. What remains open under (b) is that acdream approximates retail's bounding BOX with the BSP parts' bounding SPHERES, capped at 10. | (a) `src/AcDream.App/Streaming/LandblockPhysicsPublisher.cs:1030-1037`; `src/AcDream.Content/LandblockPhysicsContentBuilder.cs:683-690`. (b) `src/AcDream.Core/Physics/ShadowObjectRegistry.cs` (`BuildFloodSpheres`); `tests/AcDream.Core.Tests/Physics/ShadowObjectRegistryMultiPartTests.cs` (`BuildFloodSpheres_BspBearingOwner_FloodsFromBspNotFromCylinder`) | (a) 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. (b)'s bounding-sphere approximation is over-inclusive (a sphere contains the box's inscribed extent but is larger in the diagonal), which floods MORE cells rather than fewer — the safe direction for membership. | (a) 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. (b) An object floods more shadow cells than retail, costing broadphase work; the UNDER-inclusive direction, which is the #98 / #168 symptom class, is what the fix removed. | `CSphere::intersects_sphere` 0x00537a80 / 0x00537fd0; `CPhysicsObj::calc_cross_cells` 0x00515230 (0x00515285 test / 0x0051528f jne); `CPhysicsObj::find_bbox_cell_list` 0x00510fc0; `CObjCell::find_cell_list` 0x0052b9f0 (cylsphere) / 0x0052b990 (sorting sphere); `CPartArray::GetSortingSphere` 0x00518b00 | +| 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-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-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-ap152-review-architecture.md b/docs/research/2026-08-06-ap152-review-architecture.md new file mode 100644 index 00000000..1daa50d0 --- /dev/null +++ b/docs/research/2026-08-06-ap152-review-architecture.md @@ -0,0 +1,370 @@ +# AP-152 architecture review — `4abd1b5e` + +**Reviewer scope:** blast radius, correctness of reach, test quality. Retail +fidelity is a separate reviewer's. +**Worktree:** `.claude/worktrees/resume-session-e0bd03e1-d5bf45`, branch +`claude/resume-session-e0bd03e1-d5bf45`, HEAD `4abd1b5e`. +**Method:** read-only, inline, no subagents. All `bin`/`obj` deleted (44 +directories) before every verdict-deciding build. Five sabotages reproduced; +tree restored and verified clean (`git status --porcelain` empty) after each. + +--- + +## Verdict + +**PASS**, with one high-severity latent risk that must not be treated as +landed behaviour until the connected flood-set diff runs, and four +documentation/framing residuals. + +No defect found. The two load-bearing claims I was asked to attack — the +headless-neutrality claim and the S4 no-op argument — **both hold**, and I +verified each independently rather than accepting the implementer's evidence. + +--- + +## Gates reproduced + +| Gate | Claimed | Measured | | +|---|---|---|---| +| Clean Release build | 0 errors / 21 pre-existing warnings | 0 errors / 21 warnings | ✅ | +| Full solution suite | 11,203 / 4 / 0 | 11,203 passed / 4 skipped / 0 failed | ✅ | +| Headless | 89 / 89 | 89 / 89 | ✅ | +| Delta vs `ec29a732` | +5 (3 Core, 1 App, 1 Content) | +5 `[Fact]` added, **0 removed**; the 5 new facts are exactly the 5 that redden under sabotage B | ✅ | +| Renames 1:1 | 2 renamed, none deleted | `-0 [Fact]` in the test diff | ✅ | +| Register bookkeeping | AP-152 retired, AP-153/154/155 filed | active AP rows 105 → 107 (−1 +3) | ✅ | + +Per-project at HEAD, clean build, `--no-build`: +Cli 4, Bake 15, Headless 89, Content 126, UI.Abstractions 546, Core.Net 764, +Runtime 1222, App 4173/3 skip, Core 4264/1 skip. **Σ 11,203 / 4 / 0.** + +I did not check out `ec29a732` to re-measure the 11,198 baseline (read-only +worktree, and switching would have disturbed the other agent's assumptions). +The +5 arithmetic is corroborated independently: sabotage B (gate disabled) +reddens exactly five tests solution-wide, and they are exactly the five the +commit says it added. + +--- + +## 1. Reach across hosts — **the neutrality claim is TRUE, and stronger than stated** + +I did not verify the claim as written; I enumerated the whole producer set. + +`ShadowShape` is constructed at exactly **eight** sites in `src/`: + +| Site | Emits | +|---|---| +| `ShadowShapeBuilder.cs:134` (step 1) | Cylinder | +| `ShadowShapeBuilder.cs:155` (step 2) | Sphere | +| `ShadowShapeBuilder.cs:192` (step 3) | BSP | +| `ShadowShapeBuilder.cs:276` (`FromLandblockBspParts`) | BSP only — every path in that loop `continue`s or adds `ShadowCollisionType.BSP` | +| `LandblockPhysicsContentBuilder.cs:658`, `:683` | Cylinder only (`:683` converts a Setup Sphere to a height-capped Cylinder) | +| `LandblockPhysicsPublisher.cs:1003`, `:1030` | Cylinder only (same conversion) | + +Steps 1–3 are the only site that could ever produce a heterogeneous list, and +after this change it cannot. So: + +- **Content's two registrations are homogeneous by construction.** + `LandblockPhysicsContentBuilder.cs:619` passes `bspShapes` from + `FromLandblockBspParts` (all-BSP); `:702` passes `setupShapes`, all + `ShadowCollisionType.Cylinder`, with no `ShadowCollisionType.Sphere` + reachable. Confirmed by reading both loops, not by trusting the comment. +- **Every other `RegisterMultiPart` caller replays a stored list.** + `ShadowObjectRegistry.cs:536` (`ReplaceMultiPartPayload`), `:741` + (`UpdatePosition`), `:1616` (`RefloodOwnerForLandblock`), `:2199` (mirror) + all pass `_entityShapes[entityId]`, which was populated by one of the eight + producers above. Homogeneity propagates. +- **Runtime constructs no shapes.** `grep "new ShadowShape("` in + `src/AcDream.Runtime` returns nothing; every `ShadowObjects.*` call there is + `UpdatePosition` / `CommitSetPosition` / `Suspend` / a read. +- **Project references confirm the reach boundary.** + `AcDream.Headless.csproj` → `AcDream.Runtime` only; + `AcDream.Runtime.csproj` → Core, Core.Net, Content, Plugin.Abstractions. + `AcDream.App` is unreachable, so `LiveEntityCollisionBuilder` — the sole + production caller of `FromSetup` (`LiveEntityCollisionBuilder.cs:126`) — + cannot execute in headless. + +**Empirical confirmation, not just structural:** under sabotage B (the step-0 +gate disabled — emission reverts to additive), `AcDream.Headless.Tests` stays +**89/89 green** while Core, App and Content each redden. That is direct +evidence the change does not reach headless, which is the C5b lesson applied +in the correct direction. + +**I believe the headless-neutrality claim.** + +--- + +## 2. Effective-GfxObj identity — **correct, and structurally airtight** + +The gate (`ShadowShapeBuilder.cs:116-124`) and step 3 (`:171-200`) call the +same helper `EffectivePartGfxObjId` (`:289-302`), over the same index range +(`setup.Parts.Count`), with the same predicate instance (`hasPhysicsBsp`). +Therefore + +> `anyPhysicsBspPart == true` ⟺ step 3 emits at least one shape + +is an identity, not a tested property. The trap the contract names — gate +suppresses primitives, step 3 emits nothing, `Build` returns null at +`LiveEntityCollisionBuilder.cs:146`, collision silently deleted — is +unreachable by construction. + +Downstream identity is the same one: `LiveEntityCollisionBuilder.cs:137` +resolves the real BSP radius from `shape.GfxObjId`, which step 3 set to the +effective id at `ShadowShapeBuilder.cs:193`. The predicate itself +(`LiveEntityCollisionBuilder.cs:56`, +`physicsData.GetFlatGfxObj(id)?.PhysicsBsp.RootIndex >= 0`) is the same +function object passed to the gate. Gate, emission, radius resolution and the +collision-time BSP lookup all key on one id. + +**Trap-1 sabotage reproduced.** Replacing the gate body with +`hasPhysicsBsp((uint)setup.Parts[i])`: + +``` +Failed ShadowShapeBuilderTests.FromSetup_DispatchGateReadsTheEffectivePartIdentities +Failed! - Failed: 1, Passed: 4263, Skipped: 1, Total: 4265 +``` + +Exactly one test catches it, and it is the one claimed to. Its discriminating +power is real: with a `setup.Parts` gate the swapped case yields +Cylinder + BSP (2 shapes) and `Assert.Single(swapped)` fails, while the +unswapped case still passes — so the test distinguishes *this* wrong gate +from *no* gate. + +--- + +## 3. Test quality — five sabotages reproduced + +### S4 (flood reverted to cylinder-first) — **the no-op argument holds** + +I reverted `ShadowObjectRegistry.cs:649-652` to cylinder-first and ran the +**whole solution**: + +``` +Failed ShadowObjectRegistryMultiPartTests.BuildFloodSpheres_BspBearingOwner_FloodsFromBspNotFromCylinder +Failed! - Failed: 1, Passed: 4263 ... (Core) +Passed! - all 8 other test assemblies, including Headless 89/89 +``` + +`FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint` — the end-to-end fact — +stayed **green**, exactly as claimed. One test out of 11,203 reddens. + +Combined with §1's producer enumeration, this is a proof rather than an +anecdote: no production producer can emit a list on which the flood dispatch +observably differs, so **the entire measured membership delta is attributable +to the emission gate alone**, and the flood half is behaviour-identical today. + +**Is the flood change justified?** Yes, and it is not a workaround. It is a +faithful port of `calc_cross_cells`' own dispatch (documented at +`ShadowObjectRegistry.cs:600-628`), kept on the same rationale that keeps +`Transition.BspOnlyDispatch`. It is, however, honestly dead against current +inputs — see latent risk #5. The gate alone would produce identical behaviour; +the flood change buys retail-shape correctness for a producer that does not +yet exist. + +### S5′ (old `ScaleFactor` body under the same production sabotage) — **reproduced** + +Production sabotage: `ShadowShapeBuilder.cs:161`, +`Radius: sph.Radius * entScale` → `Radius: sph.Radius`. + +- Corrected test body → **Failed** + (`FromSetup_ScaleFactor_MultipliesAllRadiiAndOffsets`). +- Old test body restored verbatim under the *same* sabotage → **Passed**. + +The old test genuinely covered nothing: its radius/offset assertions sat +inside `if (s.CollisionType == ShadowCollisionType.Cylinder)` on a fixture +with zero CylSpheres. The correction is a strengthening, not a rewrite to +make a failing test pass. + +### Sabotage B (step-0 gate disabled → additive emission) + +Reddens exactly the five added facts, across three projects: + +``` +Core FromSetup_DoorSetup_EmitsBspPartsOnly +Core FromSetup_DispatchGateReadsTheEffectivePartIdentities +Core FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint +App CylSphereAndPhysicsBspPart_EmitsOnlyTheScaledBspShape +Content InstalledSetups_WithBothAPrimitiveAndAPhysicsBspPart_EmitOnlyBspShapes +Headless 89/89 GREEN +``` + +### DAT-sweep control (mine, not on the claimed list) + +The installed-DAT sweep completes in ~700 ms, which is fast enough to look +like a vacuous early return at +`InstalledSetupBspPrimitiveDispatchTests.cs:59-60`. I falsified that: +`ExpectedSetups = 5935 → 5936` produces +`Assert.Equal() Failure: Expected 5936, Actual 5935`. The sweep really +enumerates the installed `client_portal.dat`, and its four external bucket +controls are load-bearing rather than derived from the predicate under test. + +### Test-quality notes + +- No test deleted (`-0 [Fact]` in the diff); both renamed tests are + strengthened (`Assert.Single` + `Assert.All` + two `DoesNotContain` where + there were loose counters). +- The `return` on absent DATs is the established Content.Tests convention + (`ContentConformanceDats.ResolveDatDir`, used identically by six sibling + tests). Not a new skip. +- No `Skip =`, no `try/catch`, no `Thread.Sleep`/`Task.Delay`, no new + `GetEnvironmentVariable`, no suppression flag introduced anywhere in the + diff. +- Known flakes #302/#308/#321 untouched and not conflated. + +--- + +## 4. Downstream consumers of the (now smaller) shape list — clean + +- No production site indexes a `FromSetup` list positionally. The only + positional loops (`LandblockPhysicsPublisher.cs:1083`, `:1098`) walk their + own homogeneous list. +- No site asserts "at least one primitive". `RegisterMultiPart` handles + `shapes.Count == 0` by deregistering (`ShadowObjectRegistry.cs:454`), and + §2 proves the count cannot newly become zero. +- `TransitionTypes.cs:3759 / 3901 / 4089` branch per shape kind, never on the + presence of a kind. +- `WorldSceneDiagnosticsController.cs:222` is debug wireframe drawing. +- `LiveEntityCollisionBuilder.cs:191-201`'s probe counts `cyl` vs `else`; the + contract already documents that its `else` mislabels Sphere as `bsp`. That + matters for reading the un-run connected gate, not for behaviour. + +--- + +## Findings, ranked + +### Defect +None. + +### Latent risk + +**LR-1 [High] — for 99 of the 172 Setups the membership change is a SHRINK, in +the same failure class the commit exists to fix, and it is un-gated.** +`ShadowShapeBuilder.cs:150-164` stops emitting the Setup Sphere for any +Sphere+BSP Setup, so `BuildFloodSpheres` (`ShadowObjectRegistry.cs:654-667`) +now floods only from the BSP parts' bounding spheres — whose production radius +is the flat BSP root bounding sphere (`LiveEntityCollisionBuilder.cs:137`). +Retail's `find_bbox_cell_list` uses a bounding **box** over the whole part +array; acdream approximates it with per-part bounding **spheres**, which +AP-155 correctly registers. Failure scenario: a Sphere+BSP prop whose Setup +sphere is larger than every part's BSP bounding sphere — the contract itself +names `0x02001761` "Boulder" (2.00 m sphere + 1 BSP) as a candidate — loses +shadow cells, and stops blocking when approached from the landcell it dropped +out of while still blocking from another. That is the #98/#168 signature +verbatim. The instrument for this is the contract's §8.2 item 3 keyed +flood-set diff, and it **has not been run**. The commit's "NOT yet gated live" +line covers it; this finding is to make sure the shrink direction, not only +the flip direction, is what the connected session measures, and that the +acceptance criterion "no affected owner ends with an empty set" is checked. + +**LR-2 [Medium] — the blast radius is understated: this also changes the +collision shape set, not only membership, wherever the wire flag is absent.** +`Transition.BspOnlyDispatch` (`TransitionTypes.cs:1348-1349`) reads the +*server's* `PhysicsState`, copied at `LiveEntityCollisionBuilder.cs:161`. The +commit's "the collision half was already inert" is conditional on ACE setting +`HAS_PHYSICS_BSP_PS` from `CSetup.HasPhysicsBSP` for every affected Setup. Any +live entity where that bit is absent previously had its primitive tested and +now does not. Failure scenario: an ACE build (or a different server) that +omits the bit for one of the 172 — pre-change the mover collided with the +cylinder, post-change it collides only with the slab BSP, and the two are not +the same shape. This is retail-*correct* (retail derives the flag from the +parts, `CPartArray::CacheHasPhysicsBSP`), and the underlying dependency is +registered as AP-154, so nothing is hidden — but the commit message and the +"NOT yet gated live" note both say "membership", and the honest statement is +"membership, and collision wherever the server omits the bit". + +**LR-3 [Medium] — `RegisterMultiPart`'s own XML doc still states the rule this +commit inverted.** `src/AcDream.Core/Physics/ShadowObjectRegistry.cs:432-439`: +"when the object has CylSpheres, they alone drive the flood … otherwise the +BSP parts' bounding spheres stand in for the sorting sphere." That is now +false, and it sits on the **public** method, 165 lines above the corrected +`BuildFloodSpheres` block at `:600-628`. A reader who stops at the public +API doc gets the pre-change rule. This is the same class of defect the commit +was written to correct in AP-152's four false statements, and CLAUDE.md's +"never leave them out of sync" applies. + +**LR-4 [Low] — `LiveEntityCollisionBuilder`'s class doc still describes the +additive policy.** `src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs:32`: +"CylSpheres before Spheres, **and** every physics-BSP part." The "and" is +precisely the union this commit removed. + +**LR-5 [Low] — the exclusivity invariant is whole-program, not locally +enforced, and the flood guard's only live proof is synthetic.** Nothing in +`AcDream.Core` prevents a future Content/Runtime producer from handing +`RegisterMultiPart` a mixed list; the BSP-first flood branch is the guard, and +S4 shows it is exercised by exactly one synthetic test. That is acceptable +(it is a faithful port, and it is documented as forward insurance), but it +should be understood as inert code with a synthetic-only witness rather than +as covered behaviour. + +**LR-6 [Low] — "All eight sabotages run and reported" has no artifact.** +`docs/research/2026-08-06-ap152-contract.md` is the pre-implementation +contract; no closeout records sabotage outcomes. Five hold under independent +reproduction here; the other three rest on the commit message alone. + +**LR-7 [Informational] — headless still has no live-entity collision at all +(#330), so graphical and headless now dispatch shapes by different rules for +the same world.** Pre-existing and correctly bounded by the commit ("do not +fix #330 here"), but it is now load-bearing: the fix improves the graphical +host only. + +### Style +Covered by LR-3 / LR-4 (both are stale docs on live symbols, so I ranked them +as latent risk rather than style). + +--- + +## AP-153 / AP-154 / AP-155 — honest residuals, not deferral + +- **AP-153** (retail caches the dispatch flag once at + `InitPartArrayObject+0x7e`; acdream's gate is live). A modelling difference + the fix itself introduces, unreachable against ACE today because humanoid + part swaps involve no physics-BSP GfxObj on either side. Filing it rather + than building stale-flag state is the right call — modelling it would be + speculative machinery for an unreachable case. +- **AP-154** (wire-derived `HAS_PHYSICS_BSP_PS`). **It does not undercut the + gate's premise.** The gate is now derived from the parts, which is exactly + what `CPartArray::CacheHasPhysicsBSP` does; AP-154 is about the *query-time* + guard `BspOnlyDispatch`, which the commit deliberately leaves alone. What it + does do is name the assumption the "already inert" argument rests on — which + is why LR-2 is a framing problem rather than a hidden one. Filing it is + correct; it should not have been resolved inside this commit. +- **AP-155** (static paths emit Setup Spheres as height-capped Cylinders; the + flood approximates retail's bbox with bounding spheres). Its membership half + is genuinely closed here; its static half is explicitly left open. The + second clause is the mechanism behind LR-1, and the row states it plainly + rather than burying it. + +None of the three defers a part of the defect this commit set out to fix. + +--- + +## Rules compliance + +No workaround, suppression flag, grace period, retry loop, or symptom guard. +No `if (problematicState) return` at a symptom site — the gate is at the +producer, derived from the same data retail derives it from. No new skips +(4 skipped, unchanged). No test weakened to pass; both corrected tests are +strictly stronger and neither was deleted. Register updated in the same +commit, with count arithmetic checked (105 → 107). + +--- + +## What I checked, so the PASS is auditable + +1. Full diff of both production files and all five test files. +2. Every `ShadowShape` producer in `src/` (8 sites) and every + `RegisterMultiPart` caller (6 production sites) read individually. +3. `.csproj` reference graph for Headless / Runtime / Content / Core. +4. `grep` for `new ShadowShape(` and `ShadowObjects.*` in `AcDream.Runtime` + (empty / read-only). +5. Downstream consumers keyed on `ShadowCollisionType.Cylinder|Sphere`, on + `shapes[0]`, and on `shapes.Count`. +6. Clean Release build after deleting all 44 `bin`/`obj` dirs: 0 / 21. +7. Full solution suite twice on clean builds: 11,203 / 4 / 0 both times. +8. Five sabotages reproduced (trap-1 gate identity, S4 flood revert, S5′ + scale-factor pair, B gate-disabled, DAT-sweep count control), each with the + tree restored and `git status --porcelain` verified empty afterwards. +9. Register row count at HEAD vs `ec29a732`. +10. Diff scanned for `Skip =`, `try/catch`, sleeps, env-var reads, TODO/HACK. + +Final tree state: **clean**, no modifications left behind. My single write is +this file. diff --git a/docs/research/2026-08-06-ap152-review-retail.md b/docs/research/2026-08-06-ap152-review-retail.md new file mode 100644 index 00000000..d9c8bb8d --- /dev/null +++ b/docs/research/2026-08-06-ap152-review-retail.md @@ -0,0 +1,416 @@ +# AP-152 retail-conformance review — commit `4abd1b5e` + +**Reviewer role:** adversarial retail-conformance. +**Scope:** `ShadowShapeBuilder.FromSetup` step-0 dispatch gate, +`ShadowObjectRegistry.BuildFloodSpheres` priority, the AP-152 retirement text, +and the AP-153/AP-154/AP-155 filings. +**Method:** every retail claim re-derived from the PDB-paired binary +(`C:\Users\erikn\Downloads\acclient.exe`, `check_exe_pdb.py` → `=== MATCH ===`, +linker 2013-09-06T00:17:56Z, CodeView GUID +`9e847e2f-777c-4bd9-886c-22256bb87f32`) with capstone + `pefile`, resolving every +address through `named-retail/symbols.json`. Neither the commit body's nor the +contract's quoted disassembly was used as input. Population facts re-measured +independently against the installed `client_portal.dat` with a scratchpad +`Chorizite.DatReaderWriter` sweep (not the repo's test). + +--- + +## VERDICT: **FAIL** + +The retail port itself is faithful — **every** disassembly claim in the commit +and in the AP-152 retirement text checks out byte-exact, and the change makes +acdream match `CPhysicsObj::FindObjCollisions` and `CPhysicsObj::calc_cross_cells` +where it previously did not. The failure is not in the dispatch port. It is that +the commit **moves 172 Setups onto a flood-sphere approximation whose direction +the register records backwards**, and the false direction is the stated reason +the residual was safe to defer. Measured over the installed DATs: the new flood +fails to contain the object's own BSP bounding sphere for **170 of the 172** +affected Setups, shortfall up to **9.911 m**, and for **43 of them** the flood is +strictly *smaller* than what the code produced before this commit. That is the +#98 / #168 under-inclusive membership class — the exact class the commit's +thesis says it removes. + +--- + +## Part 1 — Independent retail verification (all PASS) + +### 1.1 `CPhysicsObj::calc_cross_cells` @`0x00515230` + +Symbol resolves exactly (`symbols.json` → `CPhysicsObj::calc_cross_cells`, +offset 0). Disassembled: + +``` +0x00515285 f786a800000000000100 test dword ptr [esi + 0xa8], 0x10000 +0x0051528f 7574 jne 0x515305 ; -> BSP +0x00515291 8b4e10 mov ecx, [esi + 0x10] ; part array +0x00515296 7444 je 0x5152dc ; none -> sorting sphere +0x00515298 e8e32d0000 call 0x518080 ; CPartArray::GetNumCylsphere +0x0051529f 743b je 0x5152dc ; zero -> sorting sphere +... +0x005152d1 e81a670100 call 0x52b9f0 ; CObjCell::find_cell_list (cylsphere) +0x005152da eb35 jmp 0x515311 ; PAST the sorting-sphere branch +0x005152dc ... +0x005152e3 e818380000 call 0x518b00 ; CPartArray::GetSortingSphere +0x005152fb e890660100 call 0x52b990 ; CObjCell::find_cell_list (sorting sphere) +0x00515305 680c3f8400 push 0x843f0c +0x0051530c e8afbcffff call 0x510fc0 ; CPhysicsObj::find_bbox_cell_list +``` + +- The `test`/`jne` pair is at exactly `0x00515285` / `0x0051528f` as claimed. **PASS.** +- `jne` target `0x515305` calls `0x00510fc0` = `CPhysicsObj::find_bbox_cell_list`. **PASS.** +- Cylsphere call at `0x005152d1` → `0x0052b9f0`, sorting-sphere call at + `0x005152fb` → `0x0052b990`; both **below** the jump and unreachable from it. **PASS.** +- **Order (review item 2):** confirmed by fall-through, not merely by "BSP wins". + Both `je 0x5152dc` guards (null part array; `GetNumCylsphere == 0`) skip to the + sorting-sphere branch, and the cylsphere branch's `jmp 0x515311` at `0x005152da` + jumps **past** the sorting-sphere branch. Precedence is therefore + **BSP → CylSphere → sorting sphere**, exclusive at every step. **PASS.** + +`0x0052b9f0` and `0x0052b990` are two overloads of the same symbol +`CObjCell::find_cell_list`; the commit distinguishes them correctly by argument +shape. Retail's 10-sphere cap is confirmed independently inside the cylsphere +overload at `0x0052ba21 cmp eax, 0xa`, matching `RetailSphereCap = 10`. + +### 1.2 `CPhysicsObj::FindObjCollisions` @`0x0050f050` (review item 3) + +Left untouched by the commit; re-confirmed so "already correct" is earned: + +``` +0x0050f165 f786a800000000000100 test dword ptr [esi + 0xa8], 0x10000 +0x0050f16f 7431 je 0x50f1a2 ; clear -> primitive dispatch +0x0050f18d e8ee8f0000 call 0x518180 ; CPartArray::FindObjCollisions +0x0050f19d e90e010000 jmp 0x50f2b0 ; UNCONDITIONAL +0x0050f1a2 ... ; CylSphere loop head +0x0050f1d6 0f833b010000 jae 0x50f317 ; loop exhausted -> RETURN +0x0050f21d ... ; Sphere loop head +0x0050f22f 0f84e6000000 je 0x50f31b ; zero Spheres -> RETURN seeded OK_TS +``` + +All five cited instruction addresses are byte-exact and the `jmp 0x50f2b0` is +past both `0x50f1a2` and `0x50f21d`. The dispatch is exclusive. **PASS** — the +query path genuinely did not need changing. (`ebp` at `0x0050f171`/`0x0050f1b2`/ +`0x0050f235` is the ethereal/ignore early-out, not a second shape branch.) + +### 1.3 `CacheHasPhysicsBSP` and AP-153's "exactly one caller" (review item 4) + +Two distinct symbols, both cited correctly in different places: +`CPhysicsObj::CacheHasPhysicsBSP` @`0x0050f570` and +`CPartArray::CacheHasPhysicsBSP` @`0x00518110`. + +Full `.text` + `.rdata` scan for `E8`/`E9` rel32 and absolute-dword references: + +| Target | Refs found | +|---|---| +| `CPhysicsObj::CacheHasPhysicsBSP` `0x0050f570` | **1** — `call` at `0x0051272e` = `CPhysicsObj::InitPartArrayObject+0x7e` | +| `CPartArray::CacheHasPhysicsBSP` `0x00518110` | **1** — `call` at `0x0050f57d` = `CPhysicsObj::CacheHasPhysicsBSP+0xd` | + +**PASS**, and stronger than filed: `InitPartArrayObject` @`0x005126b0` itself has +exactly three callers, all construction — `CPhysicsObj::InitNullObject+0x1f`, +`CPhysicsObj::makeObject+0x3b`, `CBuildingObj::makeBuilding+0x3b`. And +`CPartArray::SetPart` @`0x00518580` (the `AnimPartChanged` swap site) calls +`CPhysicsPart::SetPart` @`0x0050e700` per part and never re-caches. So retail's +dispatch flag is derived once at construction and is genuinely stale after a part +swap. **AP-153 is honestly scoped** and could legitimately claim more evidence +than it does. + +`CPartArray::CacheHasPhysicsBSP`'s body confirms the derivation the acdream gate +imitates: walk `[ecx+0x5c][i]` parts, deref `[part+0x20]` → gfxobj, test +`[gfxobj+0x78]` (physics BSP), OR `0x10000` into `[ecx]` on the first hit. + +### 1.4 Every cited address resolves to the symbol claimed (review item 5) + +| Address | Resolves to | Verdict | +|---|---|---| +| `0x00515230` | `CPhysicsObj::calc_cross_cells` | ✔ | +| `0x0050f050` | `CPhysicsObj::FindObjCollisions` | ✔ | +| `0x00510fc0` | `CPhysicsObj::find_bbox_cell_list` | ✔ | +| `0x0052b9f0` / `0x0052b990` | `CObjCell::find_cell_list` (two overloads) | ✔ | +| `0x0050f570` | `CPhysicsObj::CacheHasPhysicsBSP` | ✔ | +| `0x00518110` | `CPartArray::CacheHasPhysicsBSP` | ✔ | +| `0x00518180` | `CPartArray::FindObjCollisions` | ✔ | +| `0x0050d8d0` | `CPhysicsPart::find_obj_collisions` | ✔ | +| `0x00537a80` / `0x00537fd0` | `CSphere::intersects_sphere` (two overloads) | ✔ | +| `0x00518b00` | `CPartArray::GetSortingSphere` | ✔ | +| `0x0051272e` | `CPhysicsObj::InitPartArrayObject+0x7e` | ✔ | +| `0x00518060/70/80/90` | `GetNumSphere` / `GetSphere` / `GetNumCylsphere` / `GetCylsphere` | ✔ | + +No mis-citation found. + +### 1.5 The four corrections (all re-measured from the installed DAT) + +| Correction | Claim | Measured | Verdict | +|---|---|---|---| +| 2a | max affected primitive = **6.714 m** CylSphere on `0x02001741` | `0x02001741` cyl[0] r = **6.714**, h = 1.476 — the max over the 172 affected | ✔ | +| 2b | `0x0200086E` Sphere origin `(0.759, 0.165, 5.842)` | sph[0] origin `(0.758796, 0.165414, 5.842)`, r = 5.842 | ✔ | +| 3 | cottage door `0x020019FF` Sphere = **0.100 m**; `0.141` is `Setup.Radius` | sph[0] r = **0.1** @ `(0, 0, 0.018)`; `Setup.Radius` = **0.14142136** | ✔ | +| 4 | **two** pinning tests, both corrected, neither deleted | `FromSetup_DoorSetup_ProducesFourShapes` → `_EmitsBspPartsOnly`, and `FromSetup_DoorSetup_SphereAtExpectedLocalOffset` re-hosted `_ => true` → `_ => false` | ✔ | + +Note the correction is *scoped to the affected 172*, which is legitimate but +unstated: the largest primitive anywhere in the DAT is a **15.0 m** Sphere on +`0x02000D7D`. Worth one clause in the row so a later reader doesn't re-derive +6.714 as a global bound. + +### 1.6 Register bookkeeping + +- **AP-152 retirement is earned, not asserted.** `~~AP-152~~` struck through, + past tense, evidence column populated, corrections enumerated. Sabotage-verified + both halves (below). +- **AP row count = literal 107.** Parsed the section: 130 AP rows, 23 struck, **107 + active**. Baseline at `ec29a732` = 105 active. 105 + 3 (AP-153/154/155) − 1 + (AP-152) = **107**. Reconciles. +- **AP-154's grep claim is exact.** `grep -rn "PhysicsStateFlags.HasPhysicsBsp" src/` + → exactly two hits: `TransitionTypes.cs:1349` (the predicate) and + `PhysicsEngine.cs:1614`, which reads `request.MoverPhysicsState` — an unrelated + *mover*-state read, as the row says. +- **ACE derivation confirmed.** `references/ACE/Source/ACE.Server/WorldObjects/WorldObject_Networking.cs:666-669` + — `if (CSetup.HasPhysicsBSP) physicsState |= PhysicsState.HasPhysicsBSP; else &= ~`. +- **Static-publisher homogeneity claim earned.** `LandblockPhysicsPublisher.cs:983` + gates the Setup-primitive block on `entityBspCount == 0`, so that path already + dispatched exclusively and emits Cylinder-typed shapes only. +- **Blast-radius claim earned.** `ShadowShapeBuilder.FromSetup`'s only production + caller is `src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs:126` + (`internal sealed`, AcDream.App) — unreachable from Headless. + +### 1.7 Build and suite + +`bin`/`obj` deleted repo-wide, then Release build (0 errors) and full suite: + +``` +AcDream.Cli.Tests 4 / 0 skip +AcDream.Content.Tests 126 / 0 +AcDream.UI.Abstractions.Tests 546 / 0 +AcDream.Runtime.Tests 1222 / 0 +AcDream.Bake.Tests 15 / 0 +AcDream.Headless.Tests 89 / 0 +AcDream.App.Tests 4173 / 3 skip +AcDream.Core.Net.Tests 764 / 0 +AcDream.Core.Tests 4264 / 1 skip +-------------------------------------------- + 11,203 passed / 4 skipped / 0 failed +``` + +Exactly the commit's stated numbers, including Headless 89/89. + +### 1.8 Sabotage (both restored; tree confirmed clean afterwards) + +| Sabotage | Reddened | +|---|---| +| A — `if (!anyPhysicsBspPart)` → `\|\| true` (restore the additive union) | `FromSetup_DoorSetup_EmitsBspPartsOnly`, `FromSetup_DispatchGateReadsTheEffectivePartIdentities`, `FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint` — 3 failed / 20 passed | +| B — `BuildFloodSpheres` `only` → cylinder-first (drop the BSP arm) | `BuildFloodSpheres_BspBearingOwner_FloodsFromBspNotFromCylinder` only — 1 failed / 9 passed | + +Both facts are load-bearing and neither is over-broad. Sabotage B leaving +`FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint` green is *correct* and +corroborates the commit's own statement that the `BuildFloodSpheres` half is +behaviour-neutral against today's producers. + +--- + +## Part 2 — Findings, by severity + +### F1 (HIGH) — AP-155(b)'s "over-inclusive, the safe direction" is empirically inverted, and it is the justification for deferring the residual + +**Register text (AP-155, Risk column):** *"(b)'s bounding-sphere approximation is +over-inclusive (a sphere contains the box's inscribed extent but is larger in the +diagonal), which floods MORE cells rather than fewer — the safe direction for +membership."* + +That reasoning holds only if the sphere is **concentric** with the geometry. It +is not. Production builds the BSP flood sphere from two different sources: + +- **centre** — `src/AcDream.Core/Physics/ShadowShapeBuilder.cs:194`: + `LocalPosition = partFrame.Origin * entScale`, the part's *placement-frame* + origin; +- **radius** — `src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs:137` → + `_physicsBspRadius` → `src/AcDream.Core/Physics/FlatCollisionAssetBuilder.cs:393`, + `PhysicsBSP.Root.BoundingSphere.**Radius**`. + +The BSP root sphere's own **`Origin` is discarded**. `BuildFloodSpheres` +(`src/AcDream.Core/Physics/ShadowObjectRegistry.cs:661`) then floods from +`entityWorldPos + rotate(LocalPosition)` — the part origin — with that radius. + +**Measured over the installed `client_portal.dat`** (5,935 Setups; 530 carry a +physics-BSP part; 973 BSP parts total): + +- **376 of 973** BSP parts have `|rootSphere.Origin| > radius/2`. Worst: + **20.762 m** offset on a 27.708 m sphere (`gfx 0x010036DD`, Setup `0x0200129A`). +- Over the **172 Setups this commit moves onto that path**, the emitted flood + sphere set fails to contain the true BSP bounding sphere in **170** cases + (73 CylSphere-bearing, 97 Sphere-bearing). Only 2 are covered. +- Worst shortfall **9.911 m**, Setup `0x02000255`: one part, `gfx 0x01000A90`, + BSP root sphere `origin = (0.000, −0.007, 9.911)`, `radius = 10.522` + (`Setup.Height = 18.692`). Production floods from a sphere centred at + `(0,0,0)` — 9.9 m below the geometry's own collision centre. + +**Observable in-game consequence.** Outdoor land flood (`AddAllOutsideCells`) is +XY-driven and mostly forgives this. Indoor / building flood is not: +`CellTransit.BuildShadowCellSet` (`src/AcDream.Core/Physics/CellTransit.cs:601`) +routes every candidate cell with `id & 0xFFFF >= 0x0100` through +`FindTransitCellsSphere`, a 3-D sphere-vs-portal test. A tall dungeon or building +prop whose BSP sphere sits several metres above the part origin will not be +registered into the EnvCells it physically occupies. It is then never a broadphase +candidate there at all (`TransitionTypes.cs:3763` only iterates entries already in +the cell), so it does not collide: **walk through the upper part of a tall indoor +prop, or through a door slab from the storey above/below it**. That is precisely +the #98 / #168 class. + +The register does not merely omit this — it records the opposite and uses the +recorded direction as the reason the residual is safe to leave open. Under the +C4-handoff process finding *"a contract asserting a mechanism that does not exist +caused three separate defects"*, this is the same failure mode, in a register row +rather than a code contract. + +Two secondary inaccuracies in the same row's retail characterisation: + +- *"acdream approximates retail's bounding BOX"* — `find_bbox_cell_list` + @`0x00510fc0` adds the object's own cell and then calls + `CPartArray::calc_cross_cells_static` @`0x00518160`, which dispatches the + virtual at `[cell_vtbl+0x7c]` with `(numParts, parts, cellarray)` — a + `find_transit_cells` part-array overload (`CObjCell::find_transit_cells` + `0x0052b070`/`0x0052b080`; `CEnvCell`'s pair sits at `.rdata` `0x007c8d14`/ + `0x007c8d18`). Retail walks the **actual per-part geometry** through cell + portals despite the function's name. The approximation is coarser than the row + admits, which widens F1 rather than narrowing it. + +### F2 (HIGH) — 43 of the 172 affected Setups get a strictly *smaller* flood than before this commit + +A direct consequence of F1, but it needs stating separately because it +contradicts the commit's own thesis and is unguarded by any test. + +Modelling the pre-`4abd1b5e` `BuildFloodSpheres` (primitives preferred when any +Cylinder exists; otherwise everything, cap 10) against the post-commit set over +the same 172 Setups: + +| | count | +|---|---| +| new flood ⊇ old flood | 129 | +| **new flood ⊉ old flood** | **43** (22 CylSphere-bearing, 21 Sphere-bearing) | + +Worst: **3.493 m** on `0x0200086E` — `Setup.Height = 11.684`, whose Sphere at +`(0.759, 0.165, 5.842) r 5.842` reached `z ≈ 11.68`, while the two surviving BSP +flood spheres (`gfx 0x01001B2B` r 9.015, `gfx 0x01001BB2` r 9.254, both centred at +their part origins) reach only `z ≈ 9.0`. Next: `0x020015D4` 2.405 m, +`0x02000359` 2.083 m, `0x02001761` 1.889 m. + +The commit says the change removes an under-inclusive membership defect. For +these 43 Setups it introduces one. The commit's "NOT yet gated live" note is the +right instinct; the connected gate must specifically look for props and doors +that stopped blocking, not only for ones that started. + +**Neither new test covers this.** `BuildFloodSpheres_BspBearingOwner_...` and +`FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint` both hand-substitute a +synthetic `Radius = 14f` at `LocalPosition = Vector3.Zero`, i.e. a *concentric* +BSP sphere — exactly the configuration in which F1 cannot fire. + +### F3 (MEDIUM) — `RegisterMultiPart`'s own doc comment still states the superseded rule + +`src/AcDream.Core/Physics/ShadowObjectRegistry.cs:432-439` (the sole caller of +`BuildFloodSpheres`, 168 lines above it) still reads: + +> *"Flood spheres follow retail's rule (Ghidra 0x0052b9f0): when the object has +> CylSpheres, they alone drive the flood (base point + cyl radius, capped at 10); +> otherwise the BSP parts' bounding spheres stand in for the sorting sphere."* + +That is the pre-`4abd1b5e` rule and is now false — it is the rule sabotage B +restored, and sabotage B reddened a test. The commit rewrote +`BuildFloodSpheres`' own doc thoroughly and left its caller's contradicting it. +The next reader who greps `RegisterMultiPart` before `BuildFloodSpheres` gets the +wrong mental model of the very behaviour this commit changed. + +### F4 (MEDIUM) — unregistered divergence: retail's third branch is ONE sorting sphere, acdream's is every Sphere shape + +Retail's `calc_cross_cells` fall-through calls `CPartArray::GetSortingSphere` +@`0x00518b00`, which returns `[partArray+0x54] + 0x70` — a single authored +whole-object sphere on the `CSetup` — and floods from that one sphere +(`0x005152fb`). acdream's `only == null` branch +(`ShadowObjectRegistry.cs:652`) applies no filter and floods from **every** +non-BSP, non-Cylinder shape, i.e. the Setup's per-part Sphere array. + +Different DAT field, different cardinality, different extent. The source comment +at `ShadowObjectRegistry.cs:646-648` admits the substitution +(*"which acdream approximates with the remaining shapes' bounding spheres"*), and +AP-155's anchor column even cites `CPartArray::GetSortingSphere 0x00518b00` — but +no AP row states the divergence. The register's own rule ("any commit that +introduces a deviation adds its register row IN THE SAME COMMIT") is not +retroactive, and this predates `4abd1b5e`; but the commit rewrote this exact +method, filed three rows for its neighbours, and stepped over this one. + +Same class, smaller: `BuildFloodSpheres` collapses a Cylinder to a single sphere +at its base point with the cylinder radius and **ignores `CylHeight` entirely**, +where retail's `CObjCell::find_cell_list` @`0x0052b9f0` is handed the CCylSphere +array `(low_pt, radius, height)`. Also unregistered. + +### F5 (LOW) — AP-155 is two divergences in one row; AP-153 and AP-154 are honestly distinct + +Asked whether one of AP-153/154/155 is the same divergence sliced twice: **no**, +but AP-155 has the inverse problem. + +- **AP-153** (flag cached at construction vs. re-derived live) and **AP-154** + (flag taken off the wire vs. derived client-side) concern the same bit but are + genuinely different questions — *when* vs. *where from* — with different sites + (`ShadowShapeBuilder` step 0 / `LiveEntityCollisionBuilder.ReconcileAppearance` + vs. `TransitionTypes.cs:1348` / `LiveEntityCollisionBuilder.cs:161`), different + risks, and different gates. Both scoped honestly. +- **AP-155** bundles (a) the static publishers' Setup-Sphere→height-capped-Cylinder + conversion — a *different* code path, a *different and larger* population, and + its own stated gate — with (b) the flood-priority/approximation question, and + then files (b) as already half-closed by the same commit. Three different + lifecycles under one id. It should be two rows (or three), and (b)'s open + remainder needs the F1 correction before it can be reasoned about at all. + +### F6 (LOW) — commit-body imprecision + +*"Priority at both consumers: BSP -> CylSphere -> Sphere -> nothing."* At +`FindObjCollisions` the third branch is `CPartArray::GetSphere` @`0x00518070` +(the per-part Sphere array, loop head `0x0050f21d`). At `calc_cross_cells` it is +`CPartArray::GetSortingSphere` @`0x00518b00` (one authored whole-object sphere, +`0x005152e3`). Different arrays; "both consumers" is not true of the third rung. +The register row and the source comments state this correctly — only the commit +message is loose. Recorded so a future grep of the log doesn't inherit it. + +--- + +## What was checked and found clean (so the PASS half is auditable) + +- Binary/PDB pairing (`MATCH`), and every disassembly re-derived from that binary + rather than Binary Ninja. +- `calc_cross_cells` full body: flag test address, jump target, both primitive + call sites, and the **fall-through order** including the cylsphere branch's + skip-past-sorting-sphere `jmp`. +- `FindObjCollisions` full body: all five cited addresses, the unconditional + `jmp` past both loops, and identification of `ebp` as the ethereal early-out + rather than a shape branch. +- `CacheHasPhysicsBSP` (both classes) bodies + exhaustive rel32/absolute xref scan; + `InitPartArrayObject` callers; `CPartArray::SetPart` non-recaching. +- `find_bbox_cell_list` → `calc_cross_cells_static` → `[vtbl+0x7c]` chain. +- `GetSortingSphere` body (`[+0x54]+0x70`). +- Retail's 10-sphere cap (`0x0052ba21 cmp eax, 0xa`). +- 12 distinct cited addresses → symbol, no mis-citation. +- All four register corrections re-measured from the installed DAT by an + independent sweep; population 5,935 / 172 / 73 / 99 / 530 reproduced exactly. +- AP active-row count parsed (107) and reconciled against the `ec29a732` baseline (105). +- AP-154's `src/` grep claim (exactly 2 hits, second is a mover-state read). +- ACE's derivation of the wire bit (`WorldObject_Networking.cs:666-669`). +- Static publishers' `entityBspCount == 0` exclusivity gate. +- `FromSetup`'s sole production caller is App-layer (Headless-unreachable). +- `EffectivePartGfxObjId` genuinely shared by step 0 and step 3 — the + "gate and emission read the same identity" trap is real and its test + discriminates in both directions. +- Clean `bin`/`obj` → Release build 0 errors → full suite 11,203 / 4 skip / 0 fail. +- Two sabotages, both reddening only in the intended direction; tree restored and + `git status --porcelain` empty. + +## Recommended before the connected gate + +1. Correct AP-155's Risk column: the approximation is **under**-inclusive for + 170 of the 172 Setups this commit moved onto it, not over-inclusive. +2. Carry the BSP root sphere's `Origin` through `ShadowShape` (or offset + `LocalPosition` by `partFrame.Orientation * rootSphere.Origin`) so the flood + sphere is concentric with the geometry it stands for. That is a one-field fix + at `ShadowShapeBuilder.cs:192-199` + `LiveEntityCollisionBuilder.cs:135-139`, + and it converts F1/F2 from open risk to closed. +3. Add a non-concentric fixture to `ShadowObjectRegistryMultiPartTests` — the two + new flood tests both use `LocalPosition = Zero`, which is the one configuration + where the defect cannot appear. +4. Fix the stale `RegisterMultiPart` doc (F3) and split AP-155 (F5). +5. Instruct the connected gate to look for props/doors that **stopped** blocking + as well as ones that started; 43 Setups shrank. diff --git a/src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs b/src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs index 7e03ecae..3059c162 100644 --- a/src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs +++ b/src/AcDream.App/Physics/LiveEntityCollisionBuilder.cs @@ -29,8 +29,10 @@ internal sealed record LiveEntityCollisionRegistration( /// /// Ports the live-object collision-shape policy used by -/// CPartArray::FindObjCollisions: CylSpheres before Spheres, and every -/// physics-BSP part. +/// CPhysicsObj::FindObjCollisions (0x0050f050), which DISPATCHES +/// rather than unions: every physics-BSP part, ELSE the Setup's CylSpheres, +/// ELSE its Spheres, else nothing (AP-152). The gate itself lives in +/// . /// /// /// A Setup that yields no shape produces no registration. Retail synthesizes @@ -45,37 +47,39 @@ internal sealed record LiveEntityCollisionRegistration( /// internal sealed class LiveEntityCollisionBuilder { - private readonly Func _hasPhysicsBsp; - private readonly Func _physicsBspRadius; + private readonly Func _physicsBspBounds; private readonly LiveEntityDefaultPoseResolver _defaultPose; public LiveEntityCollisionBuilder( PhysicsDataCache physicsData, LiveEntityDefaultPoseResolver defaultPose) : this( - id => physicsData.GetFlatGfxObj(id)?.PhysicsBsp.RootIndex >= 0, id => { FlatPhysicsBsp? flat = physicsData.GetFlatGfxObj(id)?.PhysicsBsp; return flat is { RootIndex: >= 0 } - ? flat.Nodes[flat.RootIndex].BoundingSphere.Radius - : null; + ? flat.Nodes[flat.RootIndex].BoundingSphere + : (FlatCollisionSphere?)null; }, defaultPose) { ArgumentNullException.ThrowIfNull(physicsData); } + /// The part GfxObj's physics-BSP root + /// bounding sphere, or null when it has none. ONE resolver answers both + /// questions the builder asks — "does this part dispatch as BSP?" and + /// "where and how big is its flood sphere?" — so the dispatch gate and + /// the emitted geometry cannot disagree, and the sphere's radius cannot + /// be carried while its origin is dropped. That split is what produced + /// the AP-156 mis-placed flood. internal LiveEntityCollisionBuilder( - Func hasPhysicsBsp, - Func physicsBspRadius, + Func physicsBspBounds, LiveEntityDefaultPoseResolver defaultPose) { - _hasPhysicsBsp = hasPhysicsBsp - ?? throw new ArgumentNullException(nameof(hasPhysicsBsp)); - _physicsBspRadius = physicsBspRadius - ?? throw new ArgumentNullException(nameof(physicsBspRadius)); + _physicsBspBounds = physicsBspBounds + ?? throw new ArgumentNullException(nameof(physicsBspBounds)); _defaultPose = defaultPose ?? throw new ArgumentNullException(nameof(defaultPose)); } @@ -123,25 +127,17 @@ internal sealed class LiveEntityCollisionBuilder IReadOnlyList? defaultPose = _defaultPose.Resolve( spawn.MotionTableId ?? 0u, setup.Parts.Count); - IReadOnlyList raw = ShadowShapeBuilder.FromSetup( + // One resolver drives both the dispatch gate and the emitted BSP + // geometry, and FromSetup applies the entity scale to radius and + // center alike — there is no downstream substitution that could take + // one and drop the other. + IReadOnlyList shapes = ShadowShapeBuilder.FromSetup( setup, scale, - _hasPhysicsBsp, + id => _physicsBspBounds(id) is not null, partPoseOverride: defaultPose, - effectivePartGfxObjIds: effectivePartGfxObjIds); - var shapes = new List(raw.Count); - foreach (ShadowShape shape in raw) - { - if (shape.CollisionType == ShadowCollisionType.BSP) - { - float radius = (_physicsBspRadius(shape.GfxObjId) ?? 2f) * scale; - shapes.Add(shape with { Radius = radius }); - } - else - { - shapes.Add(shape); - } - } + effectivePartGfxObjIds: effectivePartGfxObjIds, + physicsBspBounds: _physicsBspBounds); if (shapes.Count == 0 && !retainEmptyPayload) return null; diff --git a/src/AcDream.Core/Physics/ShadowObjectRegistry.cs b/src/AcDream.Core/Physics/ShadowObjectRegistry.cs index df50055e..f1e3e0a5 100644 --- a/src/AcDream.Core/Physics/ShadowObjectRegistry.cs +++ b/src/AcDream.Core/Physics/ShadowObjectRegistry.cs @@ -430,13 +430,14 @@ public sealed class ShadowObjectRegistry /// /// /// BR-7: the cell set is ONE flood for the whole entity (retail floods - /// per OBJECT with its full sphere set, not per part). Flood spheres - /// follow retail's rule (Ghidra 0x0052b9f0): when the object has - /// CylSpheres, they alone drive the flood (base point + cyl radius, - /// capped at 10); otherwise the BSP parts' bounding spheres stand in - /// for the sorting sphere. Every shape row is then written into every - /// flooded cell, mirroring add_shadows_to_cells (0x00514ae0) + - /// CPartArray::AddPartsShadow. + /// per OBJECT with its full sphere set, not per part). The flood spheres + /// follow CPhysicsObj::calc_cross_cells' own EXCLUSIVE priority — + /// physics-BSP parts, else CylSpheres, else the remaining shapes — see + /// for the disassembly. A BSP part + /// contributes its ROOT BOUNDING SPHERE placed at its real center + /// (), not at the part origin. + /// Every shape row is then written into every flooded cell, mirroring + /// add_shadows_to_cells (0x00514ae0) + CPartArray::AddPartsShadow. /// /// public void RegisterMultiPart( @@ -604,8 +605,17 @@ public sealed class ShadowObjectRegistry /// BSP-bearing (0x00515285 test dword [esi+0xa8],0x10000 / /// 0x0051528f jne 0x515305) → CPhysicsObj::find_bbox_cell_list /// @0x00510fc0. The cylsphere and sorting-sphere branches are BOTH below - /// that jump and unreachable from it. acdream approximates the bbox with - /// the BSP parts' bounding spheres. + /// that jump and unreachable from it. find_bbox_cell_list adds the + /// object's own cell and then walks the PART ARRAY through + /// CPartArray::calc_cross_cells_static @0x00518160's + /// [vtbl+0x7c] dispatch, whose EnvCell body + /// (CEnvCell::find_transit_cells @0x0052cae0) tests each part's + /// CGfxObj::physics_sphere — the BSP root bounding sphere, center + /// transformed through the part's own Position — against the cell's + /// portal planes. acdream floods from those same per-part spheres + /// ( + ) + /// rather than walking portal planes per part; the sphere set is exact, + /// the traversal is the sphere-vs-portal one (AP-156). /// else cylspheres (0x00515298 GetNumCylsphere non-zero) → /// CObjCell::find_cell_list @0x0052b9f0 over the cylsphere array; /// each contributes one sphere at its world BASE point with the cylinder @@ -645,20 +655,42 @@ public sealed class ShadowObjectRegistry // Retail's branch, chosen once: BSP-bbox, else cylspheres, else the // sorting sphere (which acdream approximates with the remaining - // shapes' bounding spheres). + // shapes' bounding spheres — AP-157). ShadowCollisionType? only = anyBsp ? ShadowCollisionType.BSP : anyCyl ? ShadowCollisionType.Cylinder : null; + // The 10-sphere clamp belongs to the CYLSPHERE branch alone. + // CObjCell::find_cell_list @0x0052b9f0 clamps the cylsphere count at + // 0x0052ba21 cmp eax,0xa / 0x0052ba28 mov ebp,0xa. The BSP branch — + // find_bbox_cell_list @0x00510fc0 -> CPartArray::calc_cross_cells_static + // @0x00518160 -> CEnvCell::find_transit_cells @0x0052cae0 — walks EVERY + // part with no cap, and the sorting-sphere overload @0x0052b990 takes a + // single sphere. Applying the clamp to the BSP branch dropped parts + // 11..N out of the flood entirely: 7 installed Setups carry more than + // 10 physics-BSP parts (max 49, Setup 0x02001A91), and landblock-baked + // part arrays — stair runs, fences, rock clusters — routinely do. + int cap = only == ShadowCollisionType.Cylinder ? RetailSphereCap : int.MaxValue; + foreach (var s in shapes) { if (only is { } required && s.CollisionType != required) continue; - if (spheres.Count >= RetailSphereCap) + if (spheres.Count >= cap) break; - var world = entityWorldPos + Vector3.Transform(s.LocalPosition, entityWorldRot); + // Place the sphere where the GEOMETRY is, not where the part + // origin is. Composed exactly as the ShadowEntry rows below are + // (partWorldPos / partWorldRot), then offset by the shape's own + // BoundsCenter — retail's CEnvCell::find_transit_cells @0x0052cae0 + // transforms CGfxObj::physics_sphere's center through the part's + // Position at [part+0x30] before reading its radius at + // 0x0052cb65. Primitives carry BoundsCenter == Zero because their + // LocalPosition already is their center. + var partWorldPos = entityWorldPos + Vector3.Transform(s.LocalPosition, entityWorldRot); + var partWorldRot = entityWorldRot * s.LocalRotation; + var world = partWorldPos + Vector3.Transform(s.BoundsCenter, partWorldRot); spheres.Add(new DatReaderWriter.Types.Sphere { Origin = world, diff --git a/src/AcDream.Core/Physics/ShadowShape.cs b/src/AcDream.Core/Physics/ShadowShape.cs index ccb81bb4..9737b321 100644 --- a/src/AcDream.Core/Physics/ShadowShape.cs +++ b/src/AcDream.Core/Physics/ShadowShape.cs @@ -4,8 +4,9 @@ namespace AcDream.Core.Physics; /// /// One collision-bearing shape attached to a logical PhysicsObj. -/// A door emits 3-4 (CylSphere(s) + Sphere(s) + Part-BSPs); a creature -/// may emit a few; a simple item zero. Positions and rotations are +/// The set is a DISPATCH, not a union (AP-152): a BSP door emits one shape +/// per physics-BSP part and no primitive; a primitive-only object emits its +/// CylSpheres, else its Spheres; a simple item zero. Positions and rotations are /// LOCAL to the entity's origin so /// can re-transform them when the entity moves. /// @@ -15,6 +16,40 @@ namespace AcDream.Core.Physics; /// own transform and dispatches per-part collision to its GfxObj. /// /// +/// +/// 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, @@ -22,4 +57,5 @@ public readonly record struct ShadowShape( float Scale, ShadowCollisionType CollisionType, float Radius, - float CylHeight); + float CylHeight, + Vector3 BoundsCenter = default); diff --git a/src/AcDream.Core/Physics/ShadowShapeBuilder.cs b/src/AcDream.Core/Physics/ShadowShapeBuilder.cs index 103a3b06..ff384f5f 100644 --- a/src/AcDream.Core/Physics/ShadowShapeBuilder.cs +++ b/src/AcDream.Core/Physics/ShadowShapeBuilder.cs @@ -73,7 +73,11 @@ public static class ShadowShapeBuilder /// The entity's overall scale factor; multiplies /// every radius, height, and local offset. /// Predicate: does the GfxObj with this id - /// have a non-null PhysicsBSP? Production: id => cache.GetGfxObj(id)?.BSP?.Root is not null. + /// have a physics BSP? Production derives it from the SAME resolver that + /// supplies + /// (id => _physicsBspBounds(id) is not null, over + /// PhysicsDataCache.GetFlatGfxObj(id)?.PhysicsBsp), so the dispatch + /// gate and the emitted geometry cannot disagree. /// #175: per-part pose override for the /// BSP part shapes — the entity's motion-table DEFAULT-STATE pose (the /// closed pose for doors). Retail collision tests each part's LIVE @@ -89,12 +93,22 @@ public static class ShadowShapeBuilder /// retail AnimPartChanged processing. Collision keeps each Setup /// index and pose, but reads PhysicsBSP from the installed replacement. /// Null or short lists fall back to the Setup identity. + /// The part GfxObj's physics-BSP ROOT + /// bounding sphere — retail's CGfxObj::physics_sphere, which is + /// literally BSPTREE::GetSphere(physics_bsp) @0x005397e0. Supplies + /// BOTH the emitted and its + /// , from one call, so the sphere's + /// size can never be carried while its position is dropped. Null (or a + /// null result) falls back to the loose-but-safe 2 m placeholder at the + /// part origin — a fixture-only configuration; production always supplies + /// it (LiveEntityCollisionBuilder). public static IReadOnlyList FromSetup( Setup setup, float entScale, Func hasPhysicsBsp, IReadOnlyList? partPoseOverride = null, - IReadOnlyList? effectivePartGfxObjIds = null) + IReadOnlyList? effectivePartGfxObjIds = null, + Func? physicsBspBounds = null) { if (setup is null) throw new ArgumentNullException(nameof(setup)); if (hasPhysicsBsp is null) throw new ArgumentNullException(nameof(hasPhysicsBsp)); @@ -185,9 +199,22 @@ public static class ShadowShapeBuilder else partFrame = new Frame { Origin = Vector3.Zero, Orientation = Quaternion.Identity }; - // BSP radius default; caller substitutes the real BoundingSphere.Radius - // at registration time when available. Loose-but-safe broadphase value. - float bspRadius = 2f * entScale; + // The part's physics-BSP root bounding sphere — retail's + // CGfxObj::physics_sphere, assigned BSPTREE::GetSphere(physics_bsp) + // @0x005397e0. RADIUS AND CENTER TOGETHER: retail's per-part + // cross-cell walk (CEnvCell::find_transit_cells @0x0052cae0, + // reached from find_bbox_cell_list @0x00510fc0 through + // CPartArray::calc_cross_cells_static @0x00518160) transforms the + // sphere's CENTER through the part's own Position + // (0x0052cb4c add eax,0x30 → Position::localtolocal) BEFORE it + // reads the radius at 0x0052cb65 fadd [esi+0xc]. The center is not + // the part origin: 376 of the 973 installed physics-BSP parts sit + // further from it than half their own radius. A single resolver + // supplies both so one cannot be taken without the other. + // Absent bounds keep the loose-but-safe 2 m placeholder. + FlatCollisionSphere? bounds = physicsBspBounds?.Invoke(gfxId); + float bspRadius = (bounds?.Radius ?? 2f) * entScale; + Vector3 boundsCenter = (bounds?.Origin ?? Vector3.Zero) * entScale; result.Add(new ShadowShape( GfxObjId: gfxId, @@ -196,7 +223,8 @@ public static class ShadowShapeBuilder Scale: entScale, CollisionType: ShadowCollisionType.BSP, Radius: bspRadius, - CylHeight: 0f)); + CylHeight: 0f, + BoundsCenter: boundsCenter)); } return result; @@ -269,9 +297,27 @@ public static class ShadowShapeBuilder } float partScale = pScale.X > 0f ? pScale.X : 1f; // AC objects are uniformly scaled - float localRadius = hasFlat - ? flat!.Nodes[flat.RootIndex].BoundingSphere.Radius - : phys.BoundingSphere?.Radius ?? 1f; + // Root bounding sphere, CENTER AND RADIUS TOGETHER — see + // ShadowShape.BoundsCenter. Retail's per-part cross-cell walk + // (CEnvCell::find_transit_cells @0x0052cae0) transforms + // CGfxObj::physics_sphere's center through the part's Position + // before using its radius; a landblock-baked part array is the + // same CPartArray walk (CPartArray::calc_cross_cells_static + // @0x00518160), so dropping the center here mis-places the flood + // exactly as it did for live Setups. + float localRadius; + Vector3 localCenter; + if (hasFlat) + { + FlatCollisionSphere root = flat!.Nodes[flat.RootIndex].BoundingSphere; + localRadius = root.Radius; + localCenter = root.Origin; + } + else + { + localRadius = phys.BoundingSphere?.Radius ?? 1f; + localCenter = phys.BoundingSphere?.Origin ?? Vector3.Zero; + } shapes.Add(new ShadowShape( GfxObjId: meshRef.GfxObjId, @@ -280,7 +326,8 @@ public static class ShadowShapeBuilder Scale: partScale, CollisionType: ShadowCollisionType.BSP, Radius: localRadius * partScale, - CylHeight: 0f)); + CylHeight: 0f, + BoundsCenter: localCenter * partScale)); } return shapes; diff --git a/tests/AcDream.App.Tests/Physics/LiveEntityCollisionBuilderTests.cs b/tests/AcDream.App.Tests/Physics/LiveEntityCollisionBuilderTests.cs index 88da872a..572cd07b 100644 --- a/tests/AcDream.App.Tests/Physics/LiveEntityCollisionBuilderTests.cs +++ b/tests/AcDream.App.Tests/Physics/LiveEntityCollisionBuilderTests.cs @@ -98,8 +98,18 @@ public sealed class LiveEntityCollisionBuilderTests new Vector3(192f, -192f, 0f))); } + /// + /// The BSP root bounding sphere reaches the shape WHOLE — radius and + /// centre, both scaled. Retail's CGfxObj::physics_sphere is + /// BSPTREE::GetSphere(physics_bsp) @0x005397e0, and + /// 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. + /// [Fact] - public void BspOnlyPart_UsesRealScaledPhysicsBoundingRadius() + public void BspOnlyPart_UsesRealScaledPhysicsBoundingSphere() { const uint part = 0x0100ABCDu; var setup = new Setup(); @@ -109,8 +119,7 @@ public sealed class LiveEntityCollisionBuilderTests WorldEntity entity = Entity(); record.WorldEntity = entity; var builder = new LiveEntityCollisionBuilder( - id => id == part, - id => id == part ? 3f : null, + id => id == part ? Bsp(3f, centerZ: 2f) : null, PoseResolver()); LiveEntityCollisionRegistration registration = Assert.IsType( @@ -118,7 +127,8 @@ public sealed class LiveEntityCollisionBuilderTests ShadowShape shape = Assert.Single(registration.Shapes); Assert.Equal(ShadowCollisionType.BSP, shape.CollisionType); - Assert.Equal(4.5f, shape.Radius); + Assert.Equal(4.5f, shape.Radius); // 3 m * 1.5 + Assert.Equal(new Vector3(0f, 0f, 3f), shape.BoundsCenter); // 2 m * 1.5 Assert.Equal(part, shape.GfxObjId); } @@ -151,8 +161,7 @@ public sealed class LiveEntityCollisionBuilderTests WorldEntity entity = Entity(); record.WorldEntity = entity; var builder = new LiveEntityCollisionBuilder( - id => id == part, - id => id == part ? 3f : null, + id => id == part ? Bsp(3f, centerZ: 2f) : null, PoseResolver()); LiveEntityCollisionRegistration registration = @@ -162,6 +171,7 @@ public sealed class LiveEntityCollisionBuilderTests ShadowShape shape = Assert.Single(registration.Shapes); Assert.Equal(ShadowCollisionType.BSP, shape.CollisionType); Assert.Equal(4.5f, shape.Radius); // 3 m physics-BSP radius * 1.5 scale + Assert.Equal(new Vector3(0f, 0f, 3f), shape.BoundsCenter); Assert.Equal(part, shape.GfxObjId); } @@ -177,8 +187,7 @@ public sealed class LiveEntityCollisionBuilderTests WorldEntity entity = Entity(); record.WorldEntity = entity; var builder = new LiveEntityCollisionBuilder( - id => id == basePart, - _ => 1f, + id => id == basePart ? Bsp(1f) : null, PoseResolver()); Assert.Null(builder.Build( @@ -202,8 +211,7 @@ public sealed class LiveEntityCollisionBuilderTests WorldEntity entity = Entity(); record.WorldEntity = entity; var builder = new LiveEntityCollisionBuilder( - id => id == replacement, - id => id == replacement ? 2.25f : null, + id => id == replacement ? Bsp(2.25f) : null, PoseResolver()); LiveEntityCollisionRegistration registration = @@ -232,8 +240,7 @@ public sealed class LiveEntityCollisionBuilderTests WorldEntity entity = Entity(); record.WorldEntity = entity; var builder = new LiveEntityCollisionBuilder( - id => id == basePart, - _ => 1f, + id => id == basePart ? Bsp(1f) : null, PoseResolver()); LiveEntityCollisionRegistration initial = Assert.IsType(builder.Build( @@ -265,8 +272,7 @@ public sealed class LiveEntityCollisionBuilderTests WorldEntity entity = Entity(); record.WorldEntity = entity; var builder = new LiveEntityCollisionBuilder( - id => id == basePart || id == replacement, - _ => 1f, + id => id == basePart || id == replacement ? Bsp(1f) : null, PoseResolver()); LiveEntityCollisionRegistration initial = Assert.IsType(builder.Build( @@ -330,8 +336,7 @@ public sealed class LiveEntityCollisionBuilderTests WorldEntity entity = Entity(); record.WorldEntity = entity; var builder = new LiveEntityCollisionBuilder( - id => id == basePart || id == replacement, - _ => 1f, + id => id == basePart || id == replacement ? Bsp(1f) : null, PoseResolver()); LiveEntityCollisionRegistration initial = Assert.IsType(builder.Build( @@ -394,10 +399,20 @@ public sealed class LiveEntityCollisionBuilderTests } private static LiveEntityCollisionBuilder Builder() => new( - _ => false, _ => null, PoseResolver()); + /// + /// A physics-BSP root bounding sphere. DELIBERATELY OFF-CENTRE by + /// default: a GfxObj's BSP is authored in the GfxObj's own coordinates + /// and its root sphere is usually NOT centred on that origin (376 of the + /// 973 installed physics-BSP parts sit further from it than half their + /// radius). A fixture pinned at Vector3.Zero cannot observe the + /// centre at all — which is how AP-156's discarded origin stayed green. + /// + private static FlatCollisionSphere? Bsp(float radius, float centerZ = 1.25f) + => new FlatCollisionSphere(new Vector3(0f, 0f, centerZ), radius); + private static LiveEntityDefaultPoseResolver PoseResolver() => new( _ => null, new NullAnimationLoader(), diff --git a/tests/AcDream.App.Tests/Physics/PvpBitfieldSurvivesAppearanceRebuildTests.cs b/tests/AcDream.App.Tests/Physics/PvpBitfieldSurvivesAppearanceRebuildTests.cs index d46af473..b2f23b66 100644 --- a/tests/AcDream.App.Tests/Physics/PvpBitfieldSurvivesAppearanceRebuildTests.cs +++ b/tests/AcDream.App.Tests/Physics/PvpBitfieldSurvivesAppearanceRebuildTests.cs @@ -142,8 +142,9 @@ public sealed class PvpBitfieldSurvivesAppearanceRebuildTests var setup = new Setup(); setup.Parts.Add(0x0100AB01u); var builder = new LiveEntityCollisionBuilder( - id => id == 0x0100AB01u, - id => id == 0x0100AB01u ? 1f : null, + id => id == 0x0100AB01u + ? new FlatCollisionSphere(new Vector3(0f, 0f, 0.5f), 1f) + : null, new LiveEntityDefaultPoseResolver( _ => null, new NullAnimationLoader(), diff --git a/tests/AcDream.App.Tests/Rendering/LiveAppearanceAnimationTests.cs b/tests/AcDream.App.Tests/Rendering/LiveAppearanceAnimationTests.cs index 8eef106d..f0d53ba3 100644 --- a/tests/AcDream.App.Tests/Rendering/LiveAppearanceAnimationTests.cs +++ b/tests/AcDream.App.Tests/Rendering/LiveAppearanceAnimationTests.cs @@ -149,7 +149,6 @@ public sealed class LiveAppearanceAnimationTests seedCellId: cell, isStatic: false); var builder = new LiveEntityCollisionBuilder( - _ => false, _ => null, new LiveEntityDefaultPoseResolver( _ => null, diff --git a/tests/AcDream.Content.Tests/InstalledSetupBspPrimitiveDispatchTests.cs b/tests/AcDream.Content.Tests/InstalledSetupBspPrimitiveDispatchTests.cs index c8a9df59..0a71f18a 100644 --- a/tests/AcDream.Content.Tests/InstalledSetupBspPrimitiveDispatchTests.cs +++ b/tests/AcDream.Content.Tests/InstalledSetupBspPrimitiveDispatchTests.cs @@ -1,8 +1,11 @@ +using System.Globalization; +using System.Numerics; using AcDream.Core.Physics; using DatReaderWriter; using DatReaderWriter.DBObjs; using DatReaderWriter.Enums; using DatReaderWriter.Options; +using DatReaderWriter.Types; namespace AcDream.Content.Tests; @@ -156,4 +159,217 @@ public sealed class InstalledSetupBspPrimitiveDispatchTests Assert.Empty(affectedThatStillEmitAPrimitive); } + + // EXTERNAL constants for the containment sweep, measured 2026-08-06 by a + // scratch DatReaderWriter sweep that reproduced the geometry by hand + // rather than calling ShadowShapeBuilder, and independently reproduced by + // the AP-152 retail reviewer's own sweep. NOT derived from the code under + // test. + // + // OffCentreParts is the population control: without it, a build in which + // every BSP root sphere happened to sit at its part origin would satisfy + // the containment claim vacuously. + // WouldFailIfOriginDiscarded is the DEFECT control: it re-runs the + // pre-fix composition (radius carried, root-sphere origin dropped) and + // pins how many of the affected Setups it breaks. If that number ever + // goes to zero the fixture population has stopped exercising the field + // and the containment assertion below has stopped meaning anything. + private const int ExpectedPhysicsBspParts = 973; + private const int ExpectedOffCentreParts = 376; // |origin| > radius/2 + private const int ExpectedWouldFailIfOriginDiscarded = 170; // of 172 + private const int ExpectedDeepestBspPartArray = 49; // Setup 0x02001A91 + + /// + /// AP-156. Every flood sphere acdream emits for a physics-BSP part must + /// CONTAIN that part's real collision geometry. + /// + /// + /// A GfxObj's physics BSP is authored in the GfxObj's own coordinates and + /// its root bounding sphere is usually not centred on that origin — 376 + /// of the 973 installed physics-BSP parts sit further from it than half + /// their own radius, worst 20.762 m on a 27.708 m sphere (gfx 0x010036DD, + /// Setup 0x0200129A). acdream used to take the sphere's radius and drop + /// its origin, flooding from the part origin instead: over the 172 + /// AP-152 Setups that failed to contain the object's own BSP sphere for + /// 170 of them, worst shortfall 9.911 m (Setup 0x02000255, whose single + /// part's root sphere sits 9.911 m above the part origin). Indoor floods + /// are 3-D (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. + /// + /// + /// + /// Retail: CGfxObj::physics_sphere ([gfxobj+0x74]) is + /// assigned BSPTREE::GetSphere(physics_bsp) @0x005397e0 — 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 via CPartArray::calc_cross_cells_static @0x00518160, + /// transforms that sphere's CENTRE through the part's own Position at + /// [part+0x30] before reading its radius at [esi+0xc]. + /// + /// + [Fact] + public void InstalledSetups_BspFloodSpheres_ContainTheirOwnBoundingSpheres() + { + string? datDir = ContentConformanceDats.ResolveDatDir(); + if (datDir is null) + return; + + using var dats = new DatCollection(datDir, DatAccessType.Read); + + var boundsCache = new Dictionary(); + FlatCollisionSphere? Bounds(uint gfxObjId) + { + if (boundsCache.TryGetValue(gfxObjId, out FlatCollisionSphere? cached)) + return cached; + FlatCollisionSphere? result = null; + if (dats.Portal.TryGet(gfxObjId, out GfxObj? gfx) + && gfx is not null + && gfx.Flags.HasFlag(GfxObjFlags.HasPhysics) + && gfx.PhysicsBSP?.Root is not null + && gfx.VertexArray is not null + && gfx.PhysicsBSP.Root.BoundingSphere is { } bs) + { + result = new FlatCollisionSphere(bs.Origin, bs.Radius); + } + boundsCache[gfxObjId] = result; + return result; + } + + const float EntScale = 1.75f; // not 1: a dropped scale must show up + int bspParts = 0; + int offCentreParts = 0; + int affected = 0; + int wouldFailIfOriginDiscarded = 0; + float worstShortfall = 0f; + uint worstShortfallSetup = 0u; + int mostBspShapesOnOneSetup = 0; + var uncontained = new List(); + + foreach (uint id in dats.GetAllIdsOfType()) + { + if (!dats.Portal.TryGet(id, out Setup? setup) || setup is null) + continue; + + // Independent oracle: resolve the placement frame from the raw + // Setup and place each part's TRUE root sphere by hand. + AnimationFrame? placement = null; + if (setup.PlacementFrames.TryGetValue(Placement.Resting, out var resting)) + placement = resting; + else if (setup.PlacementFrames.TryGetValue(Placement.Default, out var def)) + placement = def; + else foreach (var kvp in setup.PlacementFrames) { placement = kvp.Value; break; } + + var truth = new List<(Vector3 Centre, float Radius)>(); + for (int i = 0; i < setup.Parts.Count; i++) + { + FlatCollisionSphere? b = Bounds((uint)setup.Parts[i]); + if (b is null) continue; + bspParts++; + if (b.Value.Origin.Length() > b.Value.Radius / 2f) + offCentreParts++; + + Vector3 partOrigin = Vector3.Zero; + Quaternion partRot = Quaternion.Identity; + if (placement is not null && i < placement.Frames.Count) + { + partOrigin = placement.Frames[i].Origin; + partRot = placement.Frames[i].Orientation; + } + truth.Add(( + (partOrigin + Vector3.Transform(b.Value.Origin, partRot)) * EntScale, + b.Value.Radius * EntScale)); + } + if (truth.Count == 0) continue; + + bool hasCylinder = false; + foreach (var cyl in setup.CylSpheres) if (cyl.Radius > 0f) { hasCylinder = true; break; } + bool hasSphere = false; + foreach (var sph in setup.Spheres) if (sph.Radius > 0f) { hasSphere = true; break; } + if (hasCylinder || (setup.CylSpheres.Count == 0 && hasSphere)) + affected++; + + // Production emission, through the production bounds seam. + IReadOnlyList shapes = ShadowShapeBuilder.FromSetup( + setup, + EntScale, + id => Bounds(id) is not null, + physicsBspBounds: Bounds); + + // ShadowObjectRegistry.BuildFloodSpheres' composition, at an + // entity placed at the world origin with identity rotation. + // EVERY BSP shape contributes: retail's BSP branch has no + // sphere cap (the 10-clamp at 0x0052ba21 is inside the cylsphere + // overload only), and 7 installed Setups carry more than 10 + // physics-BSP parts — capping here would silently exclude their + // tail from the containment claim below. + var flood = new List<(Vector3 Centre, float Radius)>(); + var floodIfOriginDiscarded = new List<(Vector3 Centre, float Radius)>(); + foreach (ShadowShape shape in shapes) + { + if (shape.CollisionType != ShadowCollisionType.BSP) continue; + flood.Add(( + shape.LocalPosition + + Vector3.Transform(shape.BoundsCenter, shape.LocalRotation), + shape.Radius)); + floodIfOriginDiscarded.Add((shape.LocalPosition, shape.Radius)); + } + if (flood.Count > mostBspShapesOnOneSetup) + mostBspShapesOnOneSetup = flood.Count; + + float Shortfall(List<(Vector3 Centre, float Radius)> spheres) + { + float worst = 0f; + foreach ((Vector3 tc, float tr) in truth) + { + float best = float.MaxValue; + foreach ((Vector3 fc, float fr) in spheres) + { + float need = (tc - fc).Length() + tr - fr; + if (need < best) best = need; + } + if (best > worst) worst = best; + } + return worst; + } + + const float Tolerance = 1e-3f; + float shortfall = Shortfall(flood); + if (shortfall > Tolerance) + { + uncontained.Add(id); + if (shortfall > worstShortfall) + { + worstShortfall = shortfall; + worstShortfallSetup = id; + } + } + if ((hasCylinder || (setup.CylSpheres.Count == 0 && hasSphere)) + && Shortfall(floodIfOriginDiscarded) > Tolerance) + { + wouldFailIfOriginDiscarded++; + } + } + + // Population + defect controls first. + Assert.Equal(ExpectedPhysicsBspParts, bspParts); + Assert.Equal(ExpectedOffCentreParts, offCentreParts); + Assert.Equal(ExpectedAffected, affected); + Assert.Equal(ExpectedWouldFailIfOriginDiscarded, wouldFailIfOriginDiscarded); + // Cap control: the deepest installed part array is 49 BSP shapes, so + // the containment claim genuinely covers Setups past the retired + // 10-sphere clamp rather than stopping short of them. + Assert.Equal(ExpectedDeepestBspPartArray, mostBspShapesOnOneSetup); + + // The fact. + Assert.True( + uncontained.Count == 0, + $"{uncontained.Count} Setups flood from a sphere that does not contain " + + $"their own physics-BSP bounding sphere; worst shortfall " + + $"{worstShortfall.ToString("F3", CultureInfo.InvariantCulture)} m on " + + $"Setup 0x{worstShortfallSetup:X8}."); + } } diff --git a/tests/AcDream.Core.Tests/Physics/ShadowObjectRegistryMultiPartTests.cs b/tests/AcDream.Core.Tests/Physics/ShadowObjectRegistryMultiPartTests.cs index 99a708ea..0941697f 100644 --- a/tests/AcDream.Core.Tests/Physics/ShadowObjectRegistryMultiPartTests.cs +++ b/tests/AcDream.Core.Tests/Physics/ShadowObjectRegistryMultiPartTests.cs @@ -248,10 +248,26 @@ public class ShadowObjectRegistryMultiPartTests Scale: 1f, CollisionType: ShadowCollisionType.Cylinder, Radius: radius, CylHeight: radius * 2f); - private static ShadowShape Bsp(float radius) => new( - GfxObjId: 0x010044B5u, LocalPosition: Vector3.Zero, LocalRotation: Quaternion.Identity, + /// + /// A physics-BSP part shape. defaults + /// OFF-CENTRE because that is the DAT-real configuration: a GfxObj's + /// physics BSP is authored in the GfxObj's own coordinates and 376 of the + /// 973 installed physics-BSP parts have a root bounding sphere further + /// from the part origin than half their own radius. A fixture pinned at + /// Vector3.Zero is the one configuration in which discarding the + /// centre is invisible. + /// + private static ShadowShape Bsp( + float radius, + Vector3 boundsCenter = default, + Vector3 localPosition = default, + Quaternion localRotation = default) => new( + GfxObjId: 0x010044B5u, + LocalPosition: localPosition, + LocalRotation: localRotation == default ? Quaternion.Identity : localRotation, Scale: 1f, CollisionType: ShadowCollisionType.BSP, - Radius: radius, CylHeight: 0f); + Radius: radius, CylHeight: 0f, + BoundsCenter: boundsCenter == default ? new Vector3(0f, 6f, 0f) : boundsCenter); private static List FloodCellsFor(params ShadowShape[] shapes) { @@ -272,6 +288,7 @@ public class ShadowObjectRegistryMultiPartTests List bspOnly = FloodCellsFor(Bsp(14f)); List mixed = FloodCellsFor(Cyl(0.5f), Bsp(14f)); + // Controls: the two footprints must actually differ, or the fact below // is satisfiable by any dispatch rule at all. Assert.Equal([LbId | 10u], cylinderOnly); @@ -284,6 +301,116 @@ public class ShadowObjectRegistryMultiPartTests Assert.NotEqual(cylinderOnly, mixed); } + /// + /// AP-156. Retail's per-part cross-cell walk transforms the BSP root + /// bounding sphere's CENTRE through the part's own frame before it uses + /// the radius: CGfxObj::physics_sphere is + /// BSPTREE::GetSphere(physics_bsp) @0x005397e0 (root + /// BSPNODE + 4, past its vftable), and + /// CEnvCell::find_transit_cells @0x0052cae0 — the part-array + /// overload reached from find_bbox_cell_list @0x00510fc0 through + /// CPartArray::calc_cross_cells_static @0x00518160's + /// [vtbl+0x7c] — loads it at 0x0052cb36 mov esi,[ecx+0x74], + /// runs Position::localtolocal on it against + /// [part+0x30] (0x0052cb4c add eax,0x30), and only then + /// reads [esi+0xc] for the radius. + /// + /// + /// 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. + /// + /// + [Fact] + public void BuildFloodSpheres_BspShape_CentresOnTheBoundsCentreNotThePartOrigin() + { + // Same sphere, three placements of the SAME 6 m radius: + // a) part origin at (0,0,0), bounds centre at the origin too; + // b) part origin at (0,0,0), bounds centre 30 m along +Y; + // c) part origin 30 m along +Y, bounds centre at the origin. + // (b) and (c) describe the identical world sphere, so they must flood + // the identical cells — and neither may equal (a). + List concentric = FloodCellsFor( + Bsp(6f, boundsCenter: new Vector3(0.001f, 0f, 0f))); + List viaBoundsCentre = FloodCellsFor( + Bsp(6f, boundsCenter: new Vector3(0f, 30f, 0f))); + List viaPartOrigin = FloodCellsFor( + Bsp(6f, + boundsCenter: new Vector3(0.001f, 0f, 0f), + localPosition: new Vector3(0f, 30f, 0f))); + + // Control: the offset must actually move the footprint, or the + // equality below is satisfiable by ignoring BoundsCenter entirely. + Assert.NotEqual(concentric, viaBoundsCentre); + + Assert.Equal(viaPartOrigin, viaBoundsCentre); + } + + /// + /// Retail's 10-sphere clamp is on the CYLSPHERE branch and nowhere else. + /// CObjCell::find_cell_list @0x0052b9f0 clamps the cylsphere count + /// (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 one sphere. + /// 7 installed Setups carry more than 10 physics-BSP parts (max 49, + /// Setup 0x02001A91) and landblock-baked stair runs and fences routinely + /// do, so clamping the BSP branch dropped their tail parts out of the + /// flood entirely. + /// + [Fact] + public void BuildFloodSpheres_CapsCylSpheresAtTenButNeverTheBspParts() + { + // Ten shapes on the owner's own cell, then an eleventh 72 m north. + var near = new Vector3(0f, 0f, 0f); + var far = new Vector3(0f, 72f, 0f); + uint ownCell = LbId | (uint)(1 * 8 + 1 + 1); // (x=1, y=1) + uint farCell = LbId | (uint)(1 * 8 + 4 + 1); // (x=1, y=4) + + var bsp = new ShadowShape[11]; + for (int i = 0; i < 10; i++) + bsp[i] = Bsp(1f, boundsCenter: new Vector3(0.001f, 0f, 0f)); + bsp[10] = Bsp(1f, boundsCenter: far); + List bspCells = FloodCellsFor(bsp); + + var cyls = new ShadowShape[11]; + for (int i = 0; i < 10; i++) + cyls[i] = Cyl(1f) with { LocalPosition = near }; + cyls[10] = Cyl(1f) with { LocalPosition = far }; + List cylCells = FloodCellsFor(cyls); + + Assert.Contains(ownCell, bspCells); + Assert.Contains(ownCell, cylCells); + + // The eleventh BSP part still floods; the eleventh cylsphere does not. + Assert.Contains(farCell, bspCells); + Assert.DoesNotContain(farCell, cylCells); + } + + /// + /// The BoundsCentre is expressed in the SHAPE's own frame, so the part's + /// LocalRotation must carry it — exactly as retail transforms the sphere + /// centre through [part+0x30], the part's full Position, not just + /// its origin. A +Y offset on a part yawed 180 degrees must land at -Y. + /// + [Fact] + public void BuildFloodSpheres_BspShape_RotatesTheBoundsCentreByThePartRotation() + { + Quaternion yaw180 = Quaternion.CreateFromAxisAngle(Vector3.UnitZ, MathF.PI); + + List yawed = FloodCellsFor( + Bsp(6f, boundsCenter: new Vector3(0f, 20f, 0f), localRotation: yaw180)); + List negatedUnrotated = FloodCellsFor( + Bsp(6f, boundsCenter: new Vector3(0f, -20f, 0f))); + List unrotated = FloodCellsFor( + Bsp(6f, boundsCenter: new Vector3(0f, 20f, 0f))); + + Assert.NotEqual(unrotated, negatedUnrotated); // control + Assert.Equal(negatedUnrotated, yawed); + } + /// /// The AP-152 delta end-to-end: a CylSphere+BSP Setup (73 of the 172 /// affected installed Setups are this shape) registered through the @@ -291,6 +418,14 @@ public class ShadowObjectRegistryMultiPartTests /// BuildFloodSpheres' cylinder preference confined the owner to the /// cylinder's cell while its slab BSP reached further — an object absent /// from shadow cells it physically occupies, the #98 / #168 symptom class. + /// + /// + /// The BSP bounds are supplied through the production seam + /// (physicsBspBounds, the same resolver + /// LiveEntityCollisionBuilder passes) and are OFF-CENTRE, so the + /// flood is asserted where the geometry is rather than where the part + /// origin is. + /// /// [Fact] public void FromSetup_CylSphereAndBspSetup_FloodsTheBspFootprint() @@ -303,17 +438,20 @@ public class ShadowObjectRegistryMultiPartTests { Radius = 0.5f, Height = 1f, Origin = Vector3.Zero } }, }; - IReadOnlyList raw = - ShadowShapeBuilder.FromSetup(setup, entScale: 1f, hasPhysicsBsp: id => id == part); - // Production substitutes the real BSP bounding radius at registration - // time (LiveEntityCollisionBuilder.Build); 14 m stands in for a slab - // wide enough to leave its own landcell. - var shapes = raw.Select(s => s.CollisionType == ShadowCollisionType.BSP - ? s with { Radius = 14f } - : s).ToList(); + // 14 m stands in for a slab wide enough to leave its own landcell; + // the +18 m Y offset stands in for the 376-of-973 installed parts + // whose root sphere is nowhere near the part origin. + var bounds = new FlatCollisionSphere(new Vector3(0f, 18f, 0f), 14f); + IReadOnlyList shapes = ShadowShapeBuilder.FromSetup( + setup, + entScale: 1f, + hasPhysicsBsp: id => id == part, + physicsBspBounds: id => id == part ? bounds : null); ShadowShape only = Assert.Single(shapes); Assert.Equal(ShadowCollisionType.BSP, only.CollisionType); + Assert.Equal(14f, only.Radius); + Assert.Equal(new Vector3(0f, 18f, 0f), only.BoundsCenter); var reg = new ShadowObjectRegistry(); const uint ownerId = 0xBEEF02u; @@ -322,8 +460,13 @@ public class ShadowObjectRegistryMultiPartTests shapes, 0x10008u, EntityCollisionFlags.None, OffX, OffY, LbId); List cells = OutdoorCellsHolding(reg, ownerId); - Assert.Contains(LbId | 10u, cells); Assert.True(cells.Count > 1, $"Expected the slab footprint to span more than its own landcell; got {cells.Count}"); + // Landcells are 24 m. The slab's sphere is centred at y = 54 m with + // r = 14 m, so it spans y in [40, 68] — rows 1 and 2, and it comes no + // closer than 16 m to row 0 (y < 24). Flooding it around the PART + // ORIGIN instead (y = 36 m, span [22, 50]) would drag row 0 in. + Assert.Contains(LbId | (uint)(1 * 8 + 2 + 1), cells); // (x=1, y=2) + Assert.DoesNotContain(LbId | (uint)(1 * 8 + 0 + 1), cells); // (x=1, y=0) } } diff --git a/tests/AcDream.Core.Tests/Physics/ShadowRegistrationOverflowTests.cs b/tests/AcDream.Core.Tests/Physics/ShadowRegistrationOverflowTests.cs index 5ea131ff..3639cb6a 100644 --- a/tests/AcDream.Core.Tests/Physics/ShadowRegistrationOverflowTests.cs +++ b/tests/AcDream.Core.Tests/Physics/ShadowRegistrationOverflowTests.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Collections.Immutable; using System.Linq; using System.Numerics; using AcDream.Core.Physics; @@ -99,19 +100,43 @@ public class ShadowRegistrationOverflowTests // ── The builder: one BSP shape per BSP part; shells + no-BSP excluded ── - private static GfxObjPhysics BspGfx(float radius) + /// + /// Graph-form fixture. The bounding sphere is OFF-CENTRE by default + /// because that is the DAT-real case — a GfxObj's physics BSP is authored + /// in the GfxObj's own coordinates and 376 of the 973 installed + /// physics-BSP parts have a root sphere further from the part origin than + /// half their radius. + /// + private static GfxObjPhysics BspGfx(float radius, float centerZ = 0.75f) { var leaf = new PhysicsBSPNode { Type = BSPNodeType.Leaf }; return new GfxObjPhysics { BSP = new PhysicsBSPTree { Root = leaf }, - BoundingSphere = new Sphere { Origin = Vector3.Zero, Radius = radius }, + BoundingSphere = new Sphere + { Origin = new Vector3(0f, 0f, centerZ), Radius = radius }, Resolved = new Dictionary(), PhysicsPolygons = new Dictionary(), Vertices = new VertexArray(), }; } + /// Flat-form fixture — the production storage since I6/I7. + private static GfxObjPhysics FlatBspGfx(float radius, float centerZ) + { + var node = new FlatPhysicsBspNode( + BSPNodeType.Leaf, default, -1, -1, 0, 0, + new FlatCollisionSphere(new Vector3(0f, 0f, centerZ), radius), + new FlatIndexRange(0, 0)); + GfxObjPhysics phys = BspGfx(radius, centerZ); + phys.FlatPhysicsBsp = new FlatPhysicsBsp( + 0, + ImmutableArray.Create(node), + ImmutableArray.Empty, + FlatPolygonTable.Empty); + return phys; + } + [Fact] public void FromLandblockBspParts_OneShapePerBspPart_LocalTransformPreserved() { @@ -136,6 +161,42 @@ public class ShadowRegistrationOverflowTests Assert.All(shapes, s => Assert.Equal(1.05f, s.Radius, 3)); } + /// + /// AP-156, landblock half. A landblock-baked multi-part entity is the + /// same CPartArray walk as a live Setup + /// (CPhysicsObj::find_bbox_cell_list @0x00510fc0 → + /// CPartArray::calc_cross_cells_static @0x00518160 → + /// CEnvCell::find_transit_cells @0x0052cae0), which transforms each + /// part's CGfxObj::physics_sphere CENTRE through the part's own + /// Position before reading its radius. Carrying the radius alone puts a + /// stair run's or fence's flood sphere at the part origin instead of on + /// its geometry. Both storage forms — the flat BSP that production uses + /// and the graph fallback — must supply the centre, SCALED with the part. + /// + [Fact] + public void FromLandblockBspParts_CarriesTheScaledRootSphereCentre() + { + Matrix4x4 halfScale = Matrix4x4.CreateScale(0.5f) + * Matrix4x4.CreateTranslation(0f, 2f, 0f); + + var flat = ShadowShapeBuilder.FromLandblockBspParts( + [new MeshRef(0x01000AC5u, halfScale)], + isBuildingShell: false, + getGfxObj: _ => FlatBspGfx(4f, centerZ: 3f)); + ShadowShape flatShape = Assert.Single(flat); + Assert.Equal(0.5f, flatShape.Scale, 3); + Assert.Equal(2f, flatShape.Radius, 3); // 4 * 0.5 + Assert.Equal(new Vector3(0f, 0f, 1.5f), flatShape.BoundsCenter); // 3 * 0.5 + + // Graph fallback (fixtures without a flat BSP) takes the same path. + var graph = ShadowShapeBuilder.FromLandblockBspParts( + [new MeshRef(0x01000AC5u, halfScale)], + isBuildingShell: false, + getGfxObj: _ => BspGfx(4f, centerZ: 3f)); + ShadowShape graphShape = Assert.Single(graph); + Assert.Equal(new Vector3(0f, 0f, 1.5f), graphShape.BoundsCenter); + } + [Fact] public void FromLandblockBspParts_BuildingShell_ReturnsEmpty() {