diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 88ea318d..5857ae5c 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -159,7 +159,7 @@ readiness/requeue adaptation. See --- -## 3. Documented approximation (AP) — 103 active rows (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-145 filed 2026-08-05, C4 route 3 round 3 (B5, issue #318) — the local-player collision-shadow presentation write goes through a direct cache `.Set()` that bypasses the publisher's own `ShadowObjects` write, self-healing only once dedup diverges; 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-1 narrowed 2026-07-31 by placement/streaming Slice 4A — the pure canonical retail `SetPosition` transaction exists, but production routes and lost-cell lifetime remain on the legacy resolver until Slice 4B; 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) — 102 active rows (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-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-1 narrowed 2026-07-31 by placement/streaming Slice 4A — the pure canonical retail `SetPosition` transaction exists, but production routes and lost-cell lifetime remain on the legacy resolver until Slice 4B; 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 @@ -172,7 +172,7 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps. | AP-143 | **Filed 2026-08-04 (C4 route 7 D5, headless parent-realize drive). AMENDED 2026-08-04 at the retail-conformance review round (R7 MINOR) — this row originally described only ONE of the three checks the drive skips. Line citations corrected at the round-3 review (N3).** The graphical `EquippedChildRenderController.ValidateParentProjection` performs three retail-anchored checks before accepting a parent-attach request: (1) self-parenting rejection (`relation.ParentGuid == relation.ChildGuid`, `:915-916`); (2) the parent must have a constructed part array (`parent.HasPartArray`, `:920` — the closest acdream analogue to retail's `part_array != 0` guard, AP-142 clause d); (3) `Setup.HoldingLocations` validates the specific holding location (`CSetup::GetHoldingLocation` @0x0050F896, via `PartArray::add_child`). `AcDream.Headless`/`AcDream.Runtime`'s direct-host parent-realize drive (`RuntimeLiveEntitySessionController.ResolveAndCommitChildAttachment`) performs NONE of the three — it commits on the POSITION_TS gate acceptance and relation resolution alone. (1) is inert by construction: D1's re-cell gate reads `parent.FullCellId == 0` (the child was just zeroed by the cell-less edge before D1 runs), and D2's skip-on-equal terminates the resulting one-node cycle — a self-parent headless commits the relation but never observably re-cells through it. (2) has no headless analogue at all (see AP-142 clause d — `HasPartArray` is populated only by the graphical mesh pipeline, never headless, for ANY entity). (3) has no prepared-content surface (repo-wide grep confirms nothing under `src/AcDream.Content`/`AcDream.Bake` carries `Setup.HoldingLocations`). | `src/AcDream.Runtime/Session/RuntimeLiveEntitySessionController.cs` (`ResolveAndCommitChildAttachment`) | Precedent: the content-less host already accepts reduced fidelity elsewhere (`RuntimeLiveEntitySessionController:108-117`'s documented content-less registration). A server-sent self-parent, part-array-less parent, or invalid holding location is unreachable against a well-behaved ACE (ACE only emits `ParentEvent` for a location its own `Player_Inventory`/wield validation already accepted), so this is a defense-in-depth gap, not a live-play one. | A malicious or buggy server could attach a child headless where retail and the graphical host would both reject it — inert against ACE today for all three. Retiring (3) means extending the prepared-content bake format with `Setup.HoldingLocations`, deliberately NOT done in this slice (route 7 contract §4 D5); (2) has no retiring action available until acdream's canonical layer gains its own construction-time part-array concept (a larger architectural question, out of scope here). | `PartArray::add_child` (`CSetup::GetHoldingLocation` 0x0050F896); `CPhysicsObj::enter_cell` 0x00510ed8 (the `part_array` guard); `EquippedChildRenderController.ValidateParentProjection` (graphical port, all three checks) | | AP-144 | **Filed 2026-08-05 (C4 route 3, round-3 review R7). Register discipline finding, not an implementer's disposition** — CLAUDE.md's register rule binds regardless of whether the gap has a live symptom yet. `RuntimeAcceptedPositionDriveController.ReconcileAndAcknowledgePortal`'s teleport-arrival movement-event send gates on `!RuntimeCharacterState.UsePositionFromServer` — retail's `CommandInterpreter::UsePositionFromServer` @0x006B3B40, which is `autonomy_level != 2`. But the retail function that ACTUALLY gates this send is a different one: `CommandInterpreter::SendMovementEvent` @0x006B4680 (the `PlayerTeleported` tail-jump), which gates on `autonomy_level != 0` — the LOOSER test, excluding only level 0, satisfied by BOTH level 1 and level 2. acdream's gate reuses the STRICTER `UsePositionFromServer` test (excluding two of the three levels, 0 AND 1), built from the wrong retail function, so it sends only at level 2 and wrongly suppresses at level 1. | `src/AcDream.Runtime/Session/RuntimeAcceptedPositionDriveController.cs` (`ReconcileAndAcknowledgePortal`, the `!_usePositionFromServer()` guard around `TrySendMovement`); `src/AcDream.Runtime/Gameplay/RuntimeCharacterState.cs` (`UsePositionFromServer`, `AutonomyLevel`) | The two gates agree at level 0 (both suppress) and level 2 (both send); they diverge only at level 1. `RuntimeCharacterState.TrySetAutonomyLevel` has zero production callers today, so no live code path can ever reach `AutonomyLevel == 1` — the divergence is filed for completeness, not because it is currently reachable. | The instant a future feature calls `TrySetAutonomyLevel(1)` (a partial-autonomy mode, if one is ever built), a portal-arrival movement-event ACE expects to receive at level 1 is silently dropped, until this row's fix threads the raw `AutonomyLevel` through the constructor (touching both host compositions) and gates on `!= 0` directly instead of reusing `UsePositionFromServer`. | `CommandInterpreter::UsePositionFromServer` @0x006B3B40 (`autonomy_level != 2`); `CommandInterpreter::SendMovementEvent` @0x006B4680 (`autonomy_level != 0`, the `PlayerTeleported` tail-jump call site) | | AP-146 | **Filed 2026-08-05 (#319 fix, the local player's canonical cell prerequisite; follow-up filed as issue #320).** Retail writes the local player's cell on EVERY physics tick (`CPhysicsObj::SetPositionInternal` @0x00515330, unconditional for any moving body including the player). acdream's canonical `FullCellId` for the LOCAL player is written only at three edges: login activation (`RuntimeSetPositionState.cs:2741-2745`), an accepted inbound Position/ForcePosition (`RuntimeEntityDirectory.RefreshSnapshot` → `RuntimeEntityRecord.cs:234`), and a teleport/portal placement commit (`RuntimeSetPositionState.cs:5001-5007`; `LocalPlayerTeleportController.cs:255`). Ordinary WASD movement passes a LANDBLOCK id, not an exact cell (`LocalPlayerProjectionController.Project`, low 16 bits forced to `0xFFFF` in both branches), and `LiveEntityRuntime.cs:935-938` explicitly PRESERVES the prior canonical cell for that shape rather than writing the coarser value — so the local player's canonical cell is coarse and mostly-frozen between teleports, never per-crossing-fresh. #319's fix makes a player-parented equipped child inherit exactly this same value (D1/D2 propagate the PARENT's canonical cell to the child verbatim) — the child is stale-but-EQUAL wherever the player's own record already is, not a new staleness class. | `src/AcDream.App/Input/LocalPlayerProjectionController.cs` (`Project`); `src/AcDream.App/World/LiveEntityRuntime.cs:935-938` (the landblock-preserve branch); `src/AcDream.Runtime/Physics/RuntimeSetPositionState.cs` (activation `:2741-2745`, teleport commit `:5001-5007`) | Making the local player's canonical cell track ordinary movement exactly (an exact-cell rebucket rather than the landblock-only one) is a LARGER slice than #319's key fix alone — it touches the landblock-preserve contract, `Rebucketed` delta publication cadence (today the player never publishes one during WASD), the route-2/4b-3 `PreMergeCommittedCellId` classification inputs AP-136/AP-138 spent four review rounds pinning, and the portal-space frozen-source-cell race (`LocalPlayerProjectionController.Project:100-103`). Deliberately NOT bundled into #319; filed as its own follow-up, issue #320. | The player's own render/liveness/radar/picking paths already tolerate this staleness today (proven: the player renders correctly everywhere via `Source.ParentCellId`-driven visibility, not `FullCellId`) — verified safe for the EXISTING consumer set. UNRESOLVED (this row's own open item, carried into #320): whether `RuntimeSetPositionState.IsAffectedCollisionResident`'s `ParkCollisionResidents` sweep could retire a spatial-root local player on a stale cell after a long teleport-free WASD run beyond the streaming radius — not established either way; the connected routes exercised so far all teleport between stops, which refreshes the cell and may be masking it. If the player IS a spatial root and this is reachable, the same staleness this row accepts for render/child-inheritance would ALSO apply to collision retirement, which is a materially different risk class. | `CPhysicsObj::SetPositionInternal` 0x00515330 (unconditional per-tick cell write) | -| AP-145 | **Filed 2026-08-05 (C4 route 3, round-3 review B5; carried as issue #318).** `RuntimePlacementPresentationSink.TryPublishPlace` updates the local player's collision-shadow pose by calling `LocalPlayerShadowState.Set` DIRECTLY — a plain cache write with no side effect beyond recording `Current`. This bypasses `LocalPlayerShadowSynchronizer.SyncPose`, the ONLY call site that actually publishes to `PhysicsEngine.ShadowObjects` (`ShadowPositionSynchronizer.Sync(_physics.ShadowObjects, ...)`) — `SyncPose` calls `_state.Set(...)` itself, AFTER publishing, as its own last step. Because `SyncPose`'s own early-return dedup check compares the NEW pose against `_state.Current` (skip if the same cell and within 1e-4 m² / 0.99999 dot-product of orientation), a portal placement's direct `Set` call pre-seeds that cache with the DESTINATION pose — so the very next `SyncPose` call (the player's first post-placement movement tick) can find "nothing changed" and skip its OWN `ShadowObjects` publish too, unless the position has already drifted (settle, gravity) past the dedup threshold by then. | `src/AcDream.App/World/RuntimePlacementPresentationSink.cs` (`TryPublishPlace`, the `_localPlayerShadow.Set(...)` call); `src/AcDream.App/Physics/LocalPlayerShadowSynchronizer.cs` (`SyncPose`, the dedup check and its OWN `_state.Set` call); `src/AcDream.App/Physics/LocalPlayerShadowState.cs` (`Set` — a plain cache write, no publish) | A portal placement's presentation suffix runs once per commit and its direct `Set` call is a narrow, low-frequency path; in practice the destination placement (ring search/floor snap) rarely lands EXACTLY on the pre-placement dedup-cache pose, so the next ordinary per-tick `SyncPose` call from ordinary local-player movement almost always sees the position it had already recorded and diverges from the ACTUAL committed pose by more than the threshold, triggering a real publish. This is why the risk has not been observed live. | If the destination placement happens to land within the dedup tolerance of whatever `_state.Current` held before (e.g., two placements to nearly the same spot, or the shadow was never set to begin with, or nothing calls `SyncPose` again before the player's next teleport), `PhysicsEngine.ShadowObjects` never reflects the destination — anything reading the collision shadow directly (NPC pathing around the player's own body, hit-testing) sees the PRE-teleport pose until an unrelated movement tick forces a real publish. Issue #318's composition test asserts `PhysicsEngine.ShadowObjects` directly (not the cache) to close this. | No retail analogue — retail has no separate shadow-cache/publish split; this is an acdream-only two-object seam (`LocalPlayerShadowState` cache + `LocalPlayerShadowSynchronizer` publisher) that a direct `.Set()` call can desynchronize from | +| ~~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 | **NARROWED 2026-07-31 (placement/streaming Slice 4B2 checkpoint 2).** Core exposes the pure retail `SetPosition` transaction; Runtime owns its exact accepted operation, complete canonical commit, deferred residence, lifetime, generation wake, revisioned host receipts, and exact-key retail collision table/environment-latch/report-result state; and one public generation-gated channel exposes observe/retry/exact-head acknowledgement without another placement queue. Collision starts, expiry/force ends, static and `ReportAsEnvironment` routing, reciprocal eligibility, missile-state clearing, callback ordering, and failed-placement `Collided` versus `NoValidPosition` classification now share one presentation-free owner. Shared local-controller body adoption remains deferred to the atomic all-route ownership cutover. Production zero-delta routes deliberately remain on the legacy resolver until 4B2 supplies exact authored mover preparation, presentation-only rebucketing, placement-prefix quiescence, and the atomic graphical/headless route cutover. | `src/AcDream.Core/Physics/PhysicsSetPosition.cs`; `src/AcDream.Runtime/Physics/RuntimeSetPositionState.cs`; `src/AcDream.Runtime/Physics/RuntimeCollisionReportingState.cs`; `src/AcDream.Runtime/Physics/RuntimePlacementProjectionChannel.cs`; `tests/AcDream.Core.Tests/Physics/PhysicsSetPositionTests.cs`; `tests/AcDream.Runtime.Tests/Physics/RuntimeSetPositionStateTests.cs`; `tests/AcDream.Runtime.Tests/Physics/RuntimeCollisionReportingStateTests.cs`; `docs/research/2026-07-31-canonical-set-position.md`; `docs/research/2026-07-31-runtime-set-position-collision-reporting.md` | The mechanism, ownership, report-result oracle, and host seam land independently without partially changing production placement behavior. | Until 4B2, fresh spawn, same-generation refresh, authoritative Position, portal arrival, external teleport, parent detach, pickup release, and world-drop hydration can still run the old approximation despite the canonical owners now existing. | `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 | | ~~AP-3~~ | **RETIRED 2026-07-31 (Campaign P Slice 1B).** `TransitionalInsert` now returns `OK_TS` immediately for every valid contact plane. Its ordinary StepDown tail is reachable only from invalid contact and retains the retail Contact / `!sphere_path.step_down` / check-cell / ObjectInfo.StepDown gates plus the exact one-versus-two-sphere probe split. | `src/AcDream.Core/Physics/TransitionTypes.cs` (`TransitionalInsert`, `GetStepDownProbePlan`); `tests/AcDream.Core.Tests/Physics/RetailEdgeResponseOrderingTests.cs` | — | — | `CTransition::transitional_insert` 0x0050B6F0, named-retail pseudo-C pc:273191–273307 | diff --git a/docs/research/2026-08-05-c5a-architecture-review-round2.md b/docs/research/2026-08-05-c5a-architecture-review-round2.md new file mode 100644 index 00000000..0524b379 --- /dev/null +++ b/docs/research/2026-08-05-c5a-architecture-review-round2.md @@ -0,0 +1,348 @@ +# C5a — architecture / adversarial review, round 2 (delta) + +**Reviewer:** architecture / adversarial (Opus) +**Scope:** delta over round 1 +(`docs/research/2026-08-05-c5a-architecture-review.md`). Working tree at +branch `claude/acdream-physics-divergence-5aa784`, HEAD `392c1e22`, +uncommitted. Part 2 (the deletion sweep) and the composition-graph +verification are carried forward from round 1 and not re-litigated — but I +did re-confirm the two survivors and the production blast radius are +unchanged by the fix round (see "Regression check" below). +**Date:** 2026-08-05 + +--- + +## VERDICT: **PASS** + +All three MAJORs are genuinely closed. A1 and A2 are now backed by tests I +independently traced as discriminating; A2's fix is the *correct* mirror, not a +symmetric-looking wrong one, and the constructor-parameter removal is safe on +every path. A3 built the real drive rather than arguing around it, and the +position half of route 2's B2 is now pinned end-to-end through real production +machinery with a value the wire cannot supply. + +Two **MINOR** items to fold in before commit (neither blocks the slice): + +- **M1** — the **cell** half of B2 is still not pinned: `entity.ParentCellId` + already equals the asserted value before the drive runs. +- **M2** — the `SyncPose`-inherits-the-guard behavioural nuance is **not** + documented anywhere, contrary to the handoff's claim. + +Plus one **INFO** (five test-file comments still cite the deleted +`PhysicsEngine.Resolve` as live). + +--- + +## Gates I re-measured + +| Gate | Result | +|---|---| +| `dotnet build AcDream.slnx -c Release -m:1` | **Build succeeded. 0 Warning(s), 0 Error(s)** | +| Complete Release suite (`--no-build -m:1`) | **11,106 passed / 4 skipped / 0 failed** | +| Reconciliation | 11,112 − 11 (deleted) + 5 (3 shadow-composition + 1 Withdraw fact + 1 force-position) = **11,106 ✓ exact** | +| Skips | 3 (App) + 1 (Core) = **4, unchanged from baseline ✓** | +| Per-assembly vs handoff | Core 4,259/1, Runtime 1,176/0, Headless 86/0, App 4,132/3 — **matches the handoff's numbers exactly ✓** | +| Sink ctor call sites updated | **6/6** (1 production `LivePresentationComposition.cs:514`, 5 test fixtures) | +| Register blast radius | still **3 rows + 2 section headers**; AP-131, AD-60, AD-61/62, AP-135, AP-141–144, AP-146 untouched ✓ | + +**Regression check on Part 2 (carried, re-verified):** `IsSpawnCellReady` + +`AdjustPosition` still `diff`-clean against HEAD over the full 45-line span. +Production `--numstat` shows executable changes confined to +`LivePresentationComposition` / `SessionPlayerComposition` / +`RuntimePlacementPresentationSink` (+ the deletions and the one seed rename); +`CellTransit`, `ConstraintManager`, `PhysicsBody`, `ResolveResult`, +`HeadlessSessionWorldProjection`, `RuntimeSetPositionState`, and +`RuntimeAcceptedPositionDriveController` are comment/xmldoc only. The fix round +introduced no new executable surface beyond the two sink lines. + +--- + +## A1 — the P4 test now genuinely discriminates. **CLOSED.** + +`tests/AcDream.App.Tests/World/RuntimePlacementShadowCompositionTests.cs:296–367` + +I traced the sabotage myself rather than trusting the claim. With the +`record.ServerGuid == _localPlayerGuid()` gate at +`RuntimePlacementPresentationSink.cs:249` removed: + +1. `SyncPose(childEntity, DestinationPosition, …, DestinationCell, force: true)` + runs. `_liveEntities.IsHidden(0x7000A101)` is false; `cellId != 0`; + `IsCurrentVisibleProjection(childEntity)` resolves the child's **own** record + (`TryGetRecord(entity.ServerGuid)`), `ReferenceEquals` holds, and it is the + current spatial root — so the guard does **not** short-circuit. +2. `ShadowPositionSynchronizer.Sync` → `UpdatePosition(childId, …)`. The + `_entityReg.TryGetValue` at `ShadowObjectRegistry.cs:696` now **succeeds** + (the new baseline `Register` at test `:309–320` put the record there), so + the early return that made v1 vacuous no longer fires. +3. `Register(childId, …, seedCellId: DestinationCell)` → flood from the + destination (the same geometry fact 1 proves floods successfully) → + `DeregisterCore` → row **moves** to `DestinationCell`. + +Result: `Assert.Contains(GetObjectsInCell(SourceCell), child)` at `:352` +**fails**, and `Assert.Null(fixture.LocalShadow.Current)` at `:366` **also +fails** (`_state.Set` runs as `SyncPose`'s last step). Two independent +discriminators, both keyed to the gate. + +The baseline precondition `Assert.Contains(GetObjectsInCell(SourceCell))` at +`:321` is what makes step 2 reachable — it is the thing v1 lacked, and it is +now asserted, not assumed. The `Assert.Equal(1, TotalRegistered)` at `:358` is +supporting only (a move keeps the count at 1); correctly not relied on. The +xmldoc at `:282–293` records the v1 failure honestly rather than quietly +replacing it. + +**Verified discriminating. No second vacuous version.** + +--- + +## A2 — the Withdraw fix is the *correct* mirror, and the parameter removal is safe. **CLOSED.** + +### Is `Suspend` the right counterpart to `SyncPose`'s publish? + +Yes, and I checked the two ways it could have been subtly wrong. + +- **It is not `Deregister`.** `ShadowObjectRegistry.Suspend` + (`src/AcDream.Core/Physics/ShadowObjectRegistry.cs:1480–1498`) removes the + entity from every cell bucket and stashes the cell list in + `_suspendedEntityCells`, but **retains `_entityReg`** — it early-returns + `false` if there is no registration and never removes one. Its own xmldoc + calls it "the registry counterpart of retail + `CPhysicsObj::remove_shadows_from_cells` during temporary + leave-world/pending-cell residence; deliberately not logical teardown." +- **The restore path still works.** This is the trap I looked for: if `Suspend` + had dropped `_entityReg`, then `TryApplyWithdrawalRestoration` → + `TryPublishPlace` → `SyncPose(force: true)` → `UpdatePosition` would hit the + `:696` not-registered early return and **silently no-op while still writing + the cache** — reintroducing the exact AP-145 class on the restore edge. It + does not: `_entityReg` survives `Suspend`, `UpdatePosition` proceeds, and + `Register` → `DeregisterCore` (`:1789`) clears `_suspendedEntities` so the + entity is no longer treated as suspended by `RefloodOwnerForLandblock` + (`:1568`) or the reflood capture (`:1530`). The restore is clean. +- **It matches the established App-layer pairing.** `Suspend` is exactly what + `LiveEntityProjectionWithdrawalController.LeaveWorld` already does + (`:148 _shadows.Suspend(entity.Id)` + `:156 _localPlayerShadow.Clear()`), and + `LocalPlayerShadowSynchronizer.Suspend` (`:109–114`) is precisely that pair in + one call. This is not a novel choice invented for the fix; the sink was the + odd one out. +- **No new early-return.** `Suspend` is unconditional — unlike `SyncPose` it has + no hidden/celless/current-projection guard — so the Withdraw edge cannot + silently skip the way the Place edge theoretically can. + +### Is the constructor-parameter removal safe on every path? + +Yes. `_localPlayerShadow` had exactly two uses in the sink (Place `.Set`, +Withdraw `.Clear`); both are now synchronizer calls, so the field is genuinely +dead. All **6** `new RuntimePlacementPresentationSink(` sites are updated +(1 production + 5 test fixtures) and the Release build is 0-warning. The +production site still constructs the synchronizer from `d.LocalPlayerShadow`, +so the same single `LocalPlayerShadowState` instance is still the one cache — +the removal narrows the sink's surface without changing which object holds +state. This is a genuine simplification, not just a shuffle. + +### Does the new 4th fact discriminate? + +`Withdraw_SuspendsRealPhysicsShadow_NotOnlyTheDedupCache` (`:383–…`) establishes +a **real** source-cell registration (`:391–406`) plus a non-null cache, then +asserts after the Withdraw that `LocalShadow.Current` is null **and** +`GetObjectsInCell(SourceCell)` no longer contains the entity. Under the pre-fix +`_localPlayerShadow.Clear()` the first passes and the **second fails** — and the +test comment at `:419–422` says exactly that, correctly labelling the cache +assertion as the non-discriminating half. Right shape. + +--- + +## A3 — the real drive was built; the **position** half of B2 is closed. **SUBSTANTIALLY CLOSED**, see M1. + +`tests/AcDream.App.Tests/World/RuntimeForcePositionRenderCommitTests.cs` + +This is a real correction, not a re-labelling. The chain is now: + +`LiveEntityHydrationController.OnCreate` → real `RuntimeFirstEntryDriveController` +pump → real `RuntimeEntityObjectLifetime.TryApplyPosition` (asserted to yield +`PositionTimestampDisposition.ForcePosition`, `:111`) → real +`RuntimeAcceptedPositionDriveController.TryExecuteAcceptedLocalPosition` +(`:113–119`, asserted `Committed`) → real +`RuntimePlacementProjectionSubscription` on the same placement channel +(`:309–312`) → real `RuntimePlacementPresentationSink` → render `WorldEntity`. +Nothing between the wire update and the assertion is hand-authored. + +**Why the position assertion is a true discriminator, verified:** + +- The wire carries `Z = 0` (`ForceUpdate` → `ServerPosition(Cell, 15, 15, 0, …)`). + The assertion demands `Z = 0.48f` — the grounded foot-sphere clearance the + **resolver** produces. A value that cannot be an echo of the input is exactly + the right shape for a "came from the committed receipt" claim. +- In this fixture the **only** post-materialization writer of + `entity.Position` is `LiveEntityRuntime.TryApplyRuntimePlacementProjection`, + invoked by the sink. `HostMaterializer` writes it once at create; there is no + `LiveEntityNetworkUpdateController` in the composition, so B1's tolerated + generic write cannot mask anything. Sever the receipt→render write and the + entity stays at the first-entry pose `(10, 10, 0.48)` — `:127` + (`Assert.Equal(ForcedPosition, entity.Position)`) is the assertion that fails, + and `:126` (`NotEqual(positionBeforeForce, …)`) fails with it. +- `positionBeforeForce` is captured live (`:93`) rather than assumed, so the + "it moved" claim cannot be satisfied by a coincidence of constants. + +**Fixture seams — both acceptable, neither touches production:** +`WorldSession.GameActionCapture` is a pre-existing Phase-I.3 test seam +(`src/AcDream.Core.Net/WorldSession.cs:2026`, unmodified by this diff), and +`usePositionFromServer: true` is a legitimate autonomy-level-2 configuration, +not a suppression flag added for the test. Resolving the three obstacles in the +assertions rather than in production code was the correct call. + +**B2's status:** the seam B2 actually named — "canonical body moves, render +entity stays put" — is now genuinely covered end to end. I would record B2 as +**closed for position**, with the cell half called out (M1) rather than assumed. + +--- + +## M1 — MINOR. The **cell** half of B2 is still not pinned + +**File:** `tests/AcDream.App.Tests/World/RuntimeForcePositionRenderCommitTests.cs:128` +(`Assert.Equal(Cell, entity.ParentCellId)`), with the staging at `:73–81`. + +`HostMaterializer` sets `ParentCellId = position.LandblockId` = `Cell` at +materialization (`:412`), and the test deliberately picks landblock-local +`(15,15)` so it lands in the **same** outdoor grid cell as the spawn `(10,10)` +(`TerrainSurface.CellSize = 24` → `cx=0, cy=0` → low word `0x0001` for both). +So `entity.ParentCellId` already equals `Cell` **before** the drive runs — this +is caught vacuous-class #1 (*asserting a field written unconditionally +earlier*). The comment at `:73–76` states the choice as a simplification +("without coupling this test to the outdoor grid-cell formula"); the +consequence is that the cell assertion cannot fail. + +B2's recorded wording is "asserting the render entity's **position/cell** came +from the committed placement receipt." The position half is now airtight; the +cell half is asserted but unfalsifiable. + +**Why it is MINOR, not MAJOR:** the discriminator carrying the test's claim is +the position (including the resolver-only `Z`), it is sabotage-verified, and +the round-1 shadow-composition fact 1 already pins a real cross-cell +`ParentCellId` change (`SourceCell` → `DestinationCell`) through the same sink +code path. Nothing is unprotected; the cell half is simply not proven *by this +test*. + +**Fix direction (small):** force to landblock-local `(30,30)` instead of +`(15,15)` → `cx=1, cy=1` → outdoor low word `0x000A`, i.e. committed cell +`0x0101000A ≠ Cell`. Then assert `entity.ParentCellId` equals the **committed** +cell and differs from the spawn cell, and capture `cellBeforeForce` the way +`positionBeforeForce` is captured. If that is judged out of scope, record in the +commit message that B2 is closed for position and open for cell — do not book +it as full closure. + +--- + +## M2 — MINOR. The `SyncPose`-inherits-the-guard nuance is not documented + +The handoff states the nuance "is now stated in the sink's comment and the test +class doc rather than left implicit." It is not. I grepped both files for +`hidden` / `suspend` / `celless` / `not-current` / `visible projection` / +`IsCurrentVisibleProjection` / `guard`: the only hit is +`RuntimePlacementShadowCompositionTests.cs:347`, inside the P4 test's *sabotage* +reasoning (explaining why the child's own projection is current) — not a +statement of the Place-edge behaviour change. Neither +`RuntimePlacementPresentationSink.cs:245–272` nor the test class doc +(`:20–89`) mentions it. + +The nuance is real and worth one sentence: routing Place through `SyncPose` +means the Place edge now inherits `SyncPose`'s guard +(`LocalPlayerShadowSynchronizer.cs:53–59`) — if `IsHidden(playerGuid)`, +`cellId == 0`, or `!IsCurrentVisibleProjection(entity)`, the Place now +**suspends** the shadow where the old direct write merely cached. Both +`TryApplyInitialCreateCompletion` and `TryApplyWithdrawalRestoration` reach +`TryPublishPlace`, so this is reachable on more than the portal edge. The new +behaviour is *correct* (it is what the next per-tick `Sync` would do anyway, and +it is honest about a shadow that should not be published) — which is exactly +why it belongs in a comment and the commit message rather than being discovered +later as a surprise. + +Same class as round 1's A5: a statement made in the handoff that the code does +not carry. + +--- + +## INFO — five test-file comments still cite the deleted `PhysicsEngine.Resolve` as live + +Production is now clean: every remaining mention in `src/` is an explicit +"deleted, cite by symbol" correction (`CellTransit.cs:880,:1064`, +`HeadlessSessionWorldProjection.cs:797`, `PlayerMovementController.cs:147`). The +A4 fixes are accurate and the `PlayerMovementController` class summary no longer +claims a per-frame call to a deleted method. + +Still stale, in test comments only (no behavioural weight, no compiler signal): + +- `tests/AcDream.App.Tests/Physics/LiveEntityNetworkRemoteTeleportPresentationTests.cs:89` +- `tests/AcDream.Core.Tests/Conformance/Issue107SpawnDiagnosticTests.cs:23,:82` +- `tests/AcDream.Core.Tests/Physics/CellMarchLandblockPreservationTests.cs:22` +- `tests/AcDream.Core.Tests/Physics/CellTransitFindCellSetTests.cs:301` + +Optional sweep; not a gate. + +--- + +## A5 and A6 — **CLOSED, and better than asked** + +- **A5.** `PlayerMovementPlacementTransactionTests.cs:23–41` now states the + asymmetry plainly: render-root publish **did** move + (`RuntimeSetPositionState.cs:2774`), sticky release **did not move anywhere** + (with the `grep` evidence and the `publishSharedState: false` reason), the + behaviour was dead code so nothing regresses today, and "no layer pins the + invariant … any more. That is disposition 3.6's one real coverage loss." That + is the honest version. I re-verified both halves independently. +- **A6.** `TransitionScratchDifferentialTests.cs:218–219` and `:236–239` now + assert `IsCommitted` on both engines with distinguishing messages. The + differential can no longer pass on symmetric failure. + +--- + +## Register evidence — re-verified + +- **AP-1 — retire: still justified.** Zero `PhysicsEngine.Resolve` / + `.ResolvePlacement` receivers in `src/`; the resolver-shaped entry points no + longer exist, so the row's condition is structurally unreopenable. +- **AD-1 — retire: still justified.** The recoverable outdoor demote and the + outdoor-restore `max(terrainZ, z)` lift were `Resolve`'s body; the body is + gone. +- **AP-145 — retire: now correctly scoped, and it does not overclaim.** The row + (`retail-divergence-register.md:175`) covers **both** halves, names + `TryPublishWithdrawal` and the `#184` shape explicitly, states that the sink + no longer holds a `LocalPlayerShadowState` reference at all, and — notably — + **records that the first version of the P4 fact was vacuous and was corrected + at review**. Every claim in it now maps to something I verified: the + publish-before-cache ordering, `Register`'s `DeregisterCore`, `Suspend`'s + retained registration, the single-instance composition, and four + discriminating facts. Nothing in the row claims more than the fix delivers. + The one thing it does **not** mention is the M2 guard nuance — worth a clause. + +--- + +## Flake attribution — confirmed **#302**, not diff-caused + +The reproduced failure is +`PortalProjectionTests.ClipToRegion_FrameOwnedStore_ReusesExactResultArray` +(`tests/AcDream.App.Tests/Rendering/PortalProjectionTests.cs:503`) — exactly the +test `docs/ISSUES.md:1197` files as **#302**: a +`GC.GetAllocatedBytesForCurrentThread()` assertion in `AcDream.App.Tests`, +JIT-tiering sensitive, measured 1-in-6 in isolation and once under full-suite +load. That is the #302 signature, not the load-sensitive `NakEmissionTests` +#308 look-alike that `ISSUES.md:1211–1224` warns has been conflated twice. + +It cannot be diff-caused: the file is untouched (last commit `749e8cee`, zero +working-tree diff), no rendering or portal-projection code is in this change +set, and the assertion measures thread-local GC bytes in a component this diff +does not reach. It passed clean in my own full-suite run +(App 4,132 passed / 0 failed). Correctly named and not chased. + +--- + +## Before commit + +1. **M1** — extend the B2 test to a different outdoor grid cell (local `(30,30)` + → `0x0101000A`) so the cell half is falsifiable, **or** record B2 as + position-closed / cell-open in the commit message. Do not book full closure + silently. +2. **M2** — add the one-sentence guard nuance to the sink's Place comment and + the AP-145 row. +3. Carry forward round 1's commit-message requirements: the §3.1 audit outcome + (11 deleted / 0 re-pointed), the §3.3 covering-test judgment, the §3.6 + coverage-loss declaration (now correctly worded in the test's xmldoc), and + the count reconciliation **11,112 − 11 + 5 = 11,106 / 4 skips**. diff --git a/docs/research/2026-08-05-c5a-architecture-review.md b/docs/research/2026-08-05-c5a-architecture-review.md new file mode 100644 index 00000000..86e5d503 --- /dev/null +++ b/docs/research/2026-08-05-c5a-architecture-review.md @@ -0,0 +1,411 @@ +# C5a — independent architecture / adversarial review + +**Reviewer:** architecture / adversarial (Opus) +**Scope:** the uncommitted working-tree diff at branch +`claude/acdream-physics-divergence-5aa784`, HEAD `392c1e22` +(`git diff HEAD` + the two untracked test files). Contract: +`docs/research/2026-08-05-c5a-contract.md` (input, not under review). +**Date:** 2026-08-05 + +--- + +## VERDICT: **FAIL** + +The **deletion sweep (Part 2) is clean and I would pass it on its own.** Every +structural claim I could falsify held: the two survivors are byte-identical, +the build is 0-warning/0-error, the suite reconciles to the line, the register +edits are exactly three rows, and six of the seven test dispositions are +executed as pinned (one better than pinned). + +The failure is concentrated in **Part 1 — the parity tests and the AP-145 +retirement's evidence chain**: + +- **A1** — the route-7-P4 test cited *by name in the AP-145 retirement row* as + proof does not discriminate. Removing the behaviour it claims to pin leaves + all three of its assertions green. +- **A2** — the AP-145 fix closes the `Place` half of the cache-vs-publish + asymmetry and leaves the **exact mirror image on the `Withdraw` half of the + same method pair**, unfixed and unfiled, with a real collision consequence + during a park. +- **A3** — §5.2's carried route-2 B2 acceptance gap is **not closed**. The new + test never drives an accepted ForcePosition; it hand-authors the receipt, and + the surface it exercises is already covered by an existing test. + +Under this campaign's own standard — "the review IS the coverage gate", and +four vacuous-test classes already caught this session — A1 alone is +disqualifying: a divergence-register retirement must not rest on a test that +passes under its own sabotage. + +--- + +## Gate evidence I measured myself + +| Gate | Result | +|---|---| +| `dotnet build AcDream.slnx -c Release -m:1` | **Build succeeded. 0 Warning(s), 0 Error(s)** | +| Complete Release suite (`--no-build -m:1`) | **11,105 passed / 4 skipped / 0 failed** | +| Reconciliation vs baseline 11,112 | 11,112 − 11 (`PhysicsEngineTests` methods deleted) + 4 (3 shadow-composition + 1 force-position) = **11,105 ✓ exact** | +| Skips | 3 (App) + 1 (Core) = **4 — same as baseline ✓** | +| Survivors byte-identical | `IsSpawnCellReady` + `AdjustPosition`: `PhysicsEngine.cs:1797–1839` (new) vs `1807–1849` (HEAD) — **`diff` clean over the whole 45-line span ✓** | +| Survivor production callers intact | `RuntimeSetPositionState.cs:2188,:4397`; `SessionPlayerComposition.cs:374`; `PhysicsCameraCollisionProbe.cs:38,:100` — **all present ✓** | +| Register blast radius | `git diff -U0` = **6 changed lines**: AD/AP section headers + AD-1, AP-1, AP-145 rows. AP-131, AD-60, AD-61/62, AP-135, AP-141–144, AP-146 **untouched ✓** | +| #316-preserving pair | `LiveEntityNetworkOnPositionCollapseMatrixTests.cs` **not in the modified-file set — zero diff ✓** | +| `SetPosition` → `SeedPlacementForTest` re-point | **83 removals / 83 additions**, receivers all controller-typed; **zero `entity.`/`child.`/`Entity.SetPosition` lines touched ✓** | + +--- + +## The composition-graph change — my judgment: **CORRECT, and correctly argued** + +I attacked this first as instructed. It holds. + +- **Exactly one instance, on every host path.** `new LocalPlayerShadowSynchronizer(` + now has **one** production site in the tree + (`LivePresentationComposition.cs:508`); + `SessionPlayerComposition.cs:804` consumes `live.LocalPlayerShadowSynchronizer`. + `RuntimePlacementPresentationSink` has exactly one production construction + site, also in `LivePresentationComposition.cs:514`. **No other host + constructs either** — `grep` over `src/` for `LivePresentationCompositionPhase` + / `SessionPlayerCompositionPhase` / `new RuntimePlacementPresentationSink` + returns only `GameWindow.cs:1342/1395` and that one file. Headless and the + no-window Runtime host never touch this sink at all. +- **Same arguments before and after.** `GameWindow.cs:1359` feeds + `_localPlayerShadow` into `LivePresentationDependencies.LocalPlayerShadow` + and `GameWindow.cs:1429` feeds *the same field* into + `SessionPlayerDependencies.PlayerShadow`; `_physicsEngine`, `_liveWorldOrigin` + and `_localPlayerIdentity` are likewise the same instances in both records. + `liveEntities` is the same `LiveEntityRuntime` the old + `live.LiveEntities` read. The relocated construction therefore receives an + argument-identical closure. +- **Ordering is safe.** Construction at `:508` precedes the sink at `:514`; + `LivePresentationResult` has a single construction site (`:1209`) reached only + after `:508`; the field is non-nullable and the sink's ctor + `throw`s on null (`RuntimePlacementPresentationSink.cs:60`). There is no path + to a null or a second instance. +- **Lifetime unchanged.** Both phases publish into the same `GameWindow` shell + through `PublishSessionPlayer`, whose "already owns session/player state" + guard (`GameWindow.cs:1063–1085`) proves the two phases are composed as one + transaction. Moving construction one phase earlier does not straddle a reset + boundary. + +One behavioural nuance worth recording (not a defect): routing through +`SyncPose` means the Place edge now inherits `SyncPose`'s guard — if +`IsHidden`, `cellId == 0`, or `!IsCurrentVisibleProjection`, the Place +**suspends** the shadow where the old direct write merely cached. That is the +correct, symmetric behaviour (it is what the very next per-tick `Sync` would do +anyway) and it is inside the §5.1 pre-authorised production change, so it needs +no separate row. It should be stated in the commit message, since it is the one +place the fix does more than "also publish". + +--- + +## Findings + +### A1 — MAJOR. The route-7-P4 test does not discriminate; the AP-145 retirement row cites it as proof + +**File:** `tests/AcDream.App.Tests/World/RuntimePlacementShadowCompositionTests.cs:264–293` +(`Place_ForNonLocalPlayerEntity_NeverTouchesShadowObjects`) + +The test's stated job is to prove that the player-only gate at +`src/AcDream.App/World/RuntimePlacementPresentationSink.cs:249` +(`record.ServerGuid == _localPlayerGuid()`) is what keeps a committed CHILD +from gaining a broadphase row — "This drives that directly rather than arguing +it from inspection." + +It does not. The fixture never registers the child in `ShadowObjects`, and +`ShadowObjectRegistry.UpdatePosition` returns immediately when the entity has +no registration record: + +``` +src/AcDream.Core/Physics/ShadowObjectRegistry.cs:696 + if (!_entityReg.TryGetValue(entityId, out var reg)) + return; // not registered — no-op (callers don't have to gate) +``` + +**Concrete failure scenario (the sabotage that should fail and doesn't):** +delete the `record.ServerGuid == _localPlayerGuid()` gate so every Place calls +`SyncPose`. Trace it: `IsHidden(0x7000A101)` is false (that guid was never +materialised); `IsCurrentVisibleProjection(childEntity)` resolves the child's +own record and returns true; `ShadowPositionSynchronizer.Sync` → +`UpdatePosition(childId, …)` → the early return above → nothing registered. +`TotalRegistered` is still `0`, `GetObjectsInCell(DestinationCell)` is still +empty, `entity.Position` still equals `DestinationPosition`. **All three +assertions pass with the gate removed.** (If instead the `Suspend` branch were +taken, `ShadowObjects.Suspend` on an unregistered id is likewise a no-op — the +test passes either way. It is vacuous on both branches.) + +This is caught vacuous-class #4: *a precondition that made the sabotage +irrelevant*. It is also caught class #2 in part — the two load-bearing +assertions are pure negatives against a registry the fixture guaranteed empty. + +**Why it matters beyond the test file:** the retired AP-145 row +(`docs/architecture/retail-divergence-register.md:175`) lists, among the four +things "#318's composition test … proves", "*a Place for a non-local-player +entity never touches `ShadowObjects` at all (route 7 P4 …)*". A register +retirement is now standing on a claim the cited test does not establish. + +**Fix direction:** give the child a real registration first — mirror fact 1's +baseline `ShadowObjects.Register(entity.Id, …, seedCellId: SourceCell)` and +`Synchronizer.Sync(…, force: true)` — then assert after the Place that the +child's row is **still at `SourceCell` and absent from `DestinationCell`**. +Add `Assert.Null(fixture.LocalShadow.Current)` so the dedup cache is proven +un-polluted too (removing the gate writes the child's pose into the *player's* +cache — a second thing the current test cannot see). + +--- + +### A2 — MAJOR. The fix closes `Place` and leaves the identical asymmetry on `Withdraw`, unfixed and unfiled + +**Files:** `src/AcDream.App/World/RuntimePlacementPresentationSink.cs:307–308` +vs `src/AcDream.App/World/LiveEntityProjectionWithdrawalController.cs:148,:156` + +AP-145 was, verbatim, "a plain cache write with no side effect beyond +recording `Current`" on the local-player shadow. The fix routes `TryPublishPlace` +through the real publisher. Six lines further down in the same class, +`TryPublishWithdrawal` still does: + +``` +src/AcDream.App/World/RuntimePlacementPresentationSink.cs:307 + if (record.ServerGuid == _localPlayerGuid()) + _localPlayerShadow.Clear(); +``` + +— a bare cache clear with **no** `ShadowObjects.Suspend`. The correct pairing +exists elsewhere in the same subsystem and shows what the sink is missing: + +``` +src/AcDream.App/World/LiveEntityProjectionWithdrawalController.cs:148,156 + if (!retainedProjectileShadow) + _shadows.Suspend(entity.Id); // registry + ... + _localPlayerShadow.Clear(); // cache +``` + +**Concrete failure scenario:** a local-player park (`Withdraw`) — the path +`TryApplyWithdrawalRestoration`'s own xmldoc (`:202`) names as touching "the +local-player shadow". The cache says "no shadow"; the registry still carries a +live row for the player at the park's **source** cell. For the whole park +window every other entity's collision sweep in that cell collides with a +phantom player, and nothing self-heals, because a withdrawn player receives no +per-tick `Sync`. Restoration papers over it (`TryPublishPlace` → `SyncPose` +force-republishes), so the symptom is a transient phantom obstruction during a +park — exactly the "why not observed live" shape AP-145 itself carried. + +This is **pre-existing**, not introduced by C5a. But (a) register rule 1 makes +an unrecorded deviation "a bug twice over", (b) this diff is the commit that +retires AP-145 and its retirement text asserts the seam is now symmetric with +ordinary per-tick movement, and (c) it is six lines from the line being fixed — +this is precisely the review's job to catch. + +**Fix direction:** either route the withdrawal through +`_localPlayerShadowSync.Suspend(entity)` (a production behaviour change → its +own commit with its own gate, per the no-workarounds rule), **or** file a new +AP row / issue in this same commit recording the Withdraw-half asymmetry and +its "risk if the assumption breaks" column, and narrow AP-145's retirement text +to the `Place` edge it actually covers. + +--- + +### A3 — MAJOR. §5.2's route-2 B2 acceptance gap is not closed; the test largely duplicates existing coverage + +**File:** `tests/AcDream.App.Tests/World/RuntimeForcePositionRenderCommitTests.cs:57–105` + +The contract's §5.2 deliverable: *"an App-layer test driving an **accepted +ForcePosition end to end** through `RuntimePlacementPresentationSink` / +`TryApplyRuntimePlacementPlace` and asserting the render entity's position/cell +came from the committed placement receipt."* B2's original finding is about a +**ForcePosition** producing a receipt that the render entity then follows. + +What landed does not drive a ForcePosition at all. It hand-authors a +`RuntimePlacementProjectionSnapshot` (`:107–133`) and calls `Sink.TryApply`. +The test's own xmldoc concedes it: *"rather than driving the full +`RuntimeAcceptedPositionDriveController` pipeline."* The receipt's contents are +therefore the **test's assumption**, not the ForcePosition path's output — the +half of B2 that could actually be wrong ("canonical body moves, render entity +stays put") is asserted by narrative. + +Worse, the surface it does exercise is already pinned at HEAD: + +``` +tests/AcDream.App.Tests/World/RuntimePlacementPresentationSinkTests.cs:29 + Place_ReframesAndRebucketsExactSidecarWithoutMutatingRuntimePhysics + :57 Assert.Equal(place.WorldPosition, entity.Position); + :58 Assert.Equal(place.Orientation, entity.Rotation); + :59 Assert.Equal(DestinationCell, entity.ParentCellId); + :60 Assert.True(record.IsSpatiallyProjected); + :61 Assert.True(record.IsSpatiallyVisible); +``` + +Those are the same five facts the new test asserts (`:90–104`). The only deltas +are a stale-wire-pose pre-state and `Portal: default`. That is a real but small +increment; it is not the recorded gap. + +**Fix direction:** drive `RuntimeAcceptedPositionDriveController +.TryExecuteAcceptedLocalPosition` (or the accepted-ForcePosition entry the +route-2 landing added) so the receipt is **produced** by the path under test, +then assert the render entity against the emitted receipt. If that fixture cost +is judged disproportionate, then B2 must be recorded as **still unmet** in the +plan and the commit message, not marked closed — a partial closure silently +booked as full is how an acceptance gap disappears. + +--- + +### A4 — MINOR (one line is borderline MAJOR). Stale citations of the deleted `PhysicsEngine.Resolve` survive the D7 sweep + +All four are plain ``/comment text, so the 0-warning build cannot catch +them: + +| File:line | Text | Why it's wrong now | +|---|---|---| +| `src/AcDream.Runtime/Gameplay/PlayerMovementController.cs:145` | "PhysicsEngine.Resolve is still used each frame to snap the player to terrain/cell floor Z and detect ground contact." | Class-summary architecture note asserting a **per-frame** call to a method that no longer exists. First thing a reader of the local-movement controller sees. | +| `src/AcDream.Core/Physics/CellTransit.cs:878` | "…mirrors the NO-LANDBLOCK contract in `PhysicsEngine.Resolve`." | Cites a deleted contract as the authority for a live early-return. | +| `src/AcDream.Core/Physics/CellTransit.cs:1059` | "handled at the SNAP by `PhysicsEngine.Resolve`'s `AdjustPosition` validation since #107/#111" | The snap path is gone; `AdjustPosition` survives but is now reached only from `PhysicsCameraCollisionProbe`. | +| `src/AcDream.Headless/Hosting/HeadlessSessionWorldProjection.cs:794` | "retiring the duplicate `Resolve`/`ResolvePlacement`/`SetPosition` authority" | The contract acknowledged this mention; the retirement it forecasts has now happened, so the comment should close, not linger as a to-do. | + +The contract's D7 enumerated only three doc targets and scoped the `cref` sweep +to `PhysicsEngine.cs`, so this is strictly beyond-contract — but the campaign's +"cite by symbol, these move" discipline exists for exactly this, and +`PlayerMovementController.cs:145` is materially misleading. + +**Fix direction:** rewrite `:145` to name `ResolveWithTransition` (the real +per-frame resolver) and correct the two `CellTransit` notes to cite +`PhysicsEngine.SetPosition` / `AdjustPosition` by symbol. + +--- + +### A5 — MINOR. A factual error in the rewritten `CommitPreparedPosition` test's xmldoc + +**File:** `tests/AcDream.App.Tests/Input/PlayerMovementPlacementTransactionTests.cs:24–28` + +> "The render-root-publish-on-commit and **sticky-target-release-on-commit** +> invariants this test originally pinned now live INSIDE that Runtime +> final-commit transaction." + +Half true. Render-root publish **does** live there — +`RuntimeSetPositionState.cs:2774` `_physics.Engine.UpdatePlayerCurrCell(result.CellId)` +inside the dormant-activation final commit. Verified. + +The sticky release does **not**. `grep -rn "UnStick" src/` returns **zero** +call sites in `src/AcDream.Runtime/` on that path; the only local-player +`UnStick` is `PlayerMovementController.cs:1892`, inside `SetPositionCore` and +gated on `if (publishSharedState)` — and `PreparePositionForCommit` passes +`publishSharedState: false`. So the unstick-at-first-entry-commit behaviour +was not relocated; it went away with the (already caller-free, therefore +already dead) `CommitPreparedPosition`. That is harmless — it was never running +in production — but the comment states a relocation that did not happen, and a +future reader chasing "where does first-entry unstick happen now?" will be sent +to a method that does not do it. + +**Fix direction:** say plainly that the sticky release had no production caller +and is not performed at first-entry commit today; cite +`RuntimeSetPositionState`'s `UpdatePlayerCurrCell` by symbol for the half that +did move. + +The rewritten test itself is **good**: `Assert.Null(Constraint)` before / +`Assert.NotNull(Constraint)` + `IsConstrained` after makes it discriminating, +and the two retained negatives are correctly framed as "this layer does not do +this" rather than as the pin. + +--- + +### A6 — MINOR. The re-pointed scratch differential never asserts a placement actually happened + +**File:** `tests/AcDream.Core.Tests/Physics/TransitionScratchDifferentialTests.cs:474–520` +(`AssertSetPositionBitwise`), used by `ReusedScratch_MatchesFreshPlacementSearch` + +The differential compares `expected` against `actual` field-by-field, but there +is no positive assertion that either result committed (`Assert.True(expected.IsCommitted)`, +or `Assert.NotEqual(input, expected.Position)` as `InitialPlacementOverlapTests` +does). A future regression that makes canonical `SetPosition` fail *identically* +on the fresh and reused engines leaves the differential green while the +scratch-reuse surface it guards goes unexercised. + +The pre-deletion `ResolvePlacement` arm had the same weakness (it compared `Ok` +rather than asserting it), so this is not a regression introduced here — but +the re-point was the moment to close it, and the new `AssertSetPositionBitwise` +is otherwise excellent (the `ImmutableArray.Equals` reference-comparison note +at `:501–505` is the kind of thing that would have produced a false-fail). + +--- + +## Dispositions — did any of the seven lose coverage? + +| # | Contract disposition | Executed | Coverage verdict | +|---|---|---|---| +| 3.1 | `PhysicsEngineTests` — audit, then delete | 11 methods deleted, 0 re-pointed, 6 `ResolveWithTransition` methods retained | **No loss.** I re-audited all 11 against HEAD: none touches `AdjustPosition` or `IsSpawnCellReady`; `Resolve_ZeroDeltaSnapTrace_IsExplicitlyOptIn` pinned the `[snap]` diagnostic emitted *inside* the deleted body; the rest pinned legacy floor-snap / step-height / portal-transition semantics that die with the method. `PhysicsEngineAdjustPositionTests` (3 tests: sibling-resolve, no-cell, outdoor-snap) covers the survivor. **The §3.1 audit outcome must still appear in the commit message.** | +| 3.2 | `Issue133…` — re-point (named-bug pin) | Re-pointed to canonical `PhysicsEngine.SetPosition` with the exact #133 geometry (dungeon claim `0x00070143`, dungeon block at world-Y 130 → local Y −60, resident Holtburg neighbour at origin) | **No loss.** Asserts `result.CellId == 0x00070143` and `CellId & 0xFFFF0000 == 0x00070000` on a committed result. If anyone reintroduced an lbPrefix resident-block scan into the canonical path, this fails. Best available pin for a defect whose mechanism no longer exists. | +| 3.3 | `InitialPlacementOverlapTests` — verify-then-delete-or-re-point | Re-pointed | **Judgment correct, verified independently.** `grep -c "ShadowObjects.Register" tests/…/PhysicsSetPositionTests.cs` = **0** — that suite has zero other-entity occupancy; its `placementPasses >= 2` arm (`:1152–1194`) is BSP-hook-injection driven, exactly as the implementer said. The re-point is discriminating (`Assert.NotEqual(savedFeet, result.Position)` + the centre-distance ≥ 2r check + the 4 m bound). The two-sphere capsule reconstruction matches the legacy scalar `InitPath(0.48, 1.835)` shape. | +| 3.4 | `TransitionScratchDifferentialTests` — re-point or drop explicitly | Re-pointed with `AssertSetPositionBitwise` | **Preserved.** Both the bitwise fresh-vs-reused comparison and the second-identity (`0x80000102`, hostile) leak check survive; the new asserter covers every `PhysicsSetPositionResult` member including the three id arrays. See **A6** for the one gap. | +| 3.5 | `SetPosition` → `SeedPlacementForTest` | 83 sites / 19 files | **No loss, no meaning change.** 83 removals ↔ 83 additions; the 2-arg → 3-arg conversions all pass `pos` as `cellLocal`, byte-for-byte what the deleted 2-arg overload did; the seed calls the same production `SetPositionCore`, so the AD-61 grounded/zero-velocity start is unchanged. **Zero `entity.`/`child.`/`Entity.SetPosition` (`WorldEntity`) lines touched** — verified by regex over the whole test diff. Both `Assert.Throws` guard sites re-pointed, preserving `EnsurePublishedForRuntimeOperation` coverage. | +| 3.6 | `CommitPreparedPosition` — re-point at the replacement | 1 rewrite + 2 throw-site re-points | **Partial, honestly declared.** The two throw sites re-point cleanly onto `ArmConstraintLeashAtCommittedPlacement`, which carries the same guard. The rewrite is discriminating. **Coverage genuinely lost:** render-root-publish-on-commit and sticky-release-on-commit are no longer pinned at any layer — the former does exist in Runtime (`RuntimeSetPositionState.cs:2774`) but is not asserted by the re-point; the latter does not exist at all (see **A5**). The contract permitted this only with a commit-message note; the note is in the xmldoc and is **half wrong**. | +| 3.7 | `Begin*` wrappers — keep as documented seam | Kept, xmldoc added to both (`RuntimeSetPositionState.cs:1321,:1343`) | **Correct, as pinned.** | + +**Summary: one disposition (3.6) lost real coverage, declared but mis-described. +The other six are clean.** The silent-coverage-loss risk the slice was designed +around did **not** materialise in the deletion sweep — it materialised in the +*new* tests (A1, A3). + +--- + +## Register retirements — verified + +- **AP-1 — retire: justified.** I re-ran the census: zero `PhysicsEngine.Resolve` + / `.ResolvePlacement` receivers in `src/`; the only + `_physics.Engine.SetPosition` sites are in `RuntimeSetPositionState`. After + D1–D3 the resolver-shaped entry points do not exist, so the row's condition + is structurally unreopenable. Correct. +- **AD-1 — retire: justified.** The recoverable outdoor demote and the + outdoor-restore `max(terrainZ, z)` lift were `Resolve`'s body; the body is + gone. Correct. +- **AP-145 — retire: justified in mechanism, overstated in evidence.** The + mechanism claim ("`SyncPose` publishes before it records the cache, so the + cache can no longer be pre-seeded ahead of the real publish") is true and I + verified the `Register` → `DeregisterCore` ordering + (`ShadowObjectRegistry.cs:403`) that backs the no-stale-source-row claim. The + row's fourth proof bullet (route-7 P4) rests on the vacuous test — see **A1**; + and the row's framing implies a symmetry the `Withdraw` half does not have — + see **A2**. +- **Untouchable set held.** AP-131, AD-60, AD-61, AD-62, AP-135, AP-141–144, + AP-146 — all unmodified. Section counts updated correctly (AD 48→47, AP + 103→101 for two retirements). Strikethrough-plus-RETIRED matches the file's + established convention (31 existing `| ~~…~~` rows). +- **#275 surface untouched.** `InboundPhysicsStateController` and + `RuntimeEntityObjectLifetime` are not in the modified-file set. ✓ + +--- + +## Vacuous-test hunt — all four classes, result + +| Class | Hunted in | Result | +|---|---|---| +| Asserting a field written unconditionally earlier | all new/changed assertions | Clean. The shadow-composition tests assert `ShadowObjects` rows, which only `SyncPose` writes; the B2 test's `entity.Position` is written only by `TryApplyRuntimePlacementProjection`. | +| Asserting only negatives | `Place_ForNonLocalPlayerEntity_…`, the rewritten transaction test | **HIT on `Place_ForNonLocalPlayerEntity_…`** (two of three assertions are negatives against a guaranteed-empty registry). The transaction test is clean — it pairs its negatives with a positive (`Constraint` non-null) and a precondition (`Constraint` null before). | +| Fixture staging makes the wrong expression compute the right answer | the destination-cell flood geometry (`DestinationPosition = (202,10,5)` with `worldOffsetX: 192`), the two-sphere capsule reconstructions | Clean. The flood-lands-under-`DestinationCell` construction is load-bearing and documented at `:80–84`; if it were wrong the discriminating assertion would fail, not falsely pass. | +| A precondition that made the sabotage irrelevant | all three shadow-composition facts | **HIT on `Place_ForNonLocalPlayerEntity_…`** — the child is never registered, so `UpdatePosition`'s not-registered early return makes the gate's presence unobservable. Facts 1 and 2 are clean: both establish a **real** source-cell registration first, which is exactly what makes the "row moved to destination / source row gone" assertions bite. | + +Facts 1 and 2 of `RuntimePlacementShadowCompositionTests` are genuinely good +discriminators, and the negative-control comment at `:159–165` (naming the +cache assertion as the shape that would have passed under the bug) is exactly +the right way to document a sabotage argument. The problem is confined to the +third fact. + +--- + +## What must happen before a re-review + +1. **A1** — make `Place_ForNonLocalPlayerEntity_NeverTouchesShadowObjects` + discriminate (register the child first; assert its row stays at `SourceCell`; + assert the player's cache is not polluted). Re-verify by sabotage: remove the + `_localPlayerGuid()` gate and confirm the **new** assertion is the one that + fails. +2. **A2** — either fix the `Withdraw` half as its own reviewed commit, or file + the deviation as a register row **in this commit** and narrow AP-145's + retirement text to the `Place` edge. +3. **A3** — drive an actual accepted ForcePosition, or record B2 as still open. +4. **A4/A5** — correct the four stale `PhysicsEngine.Resolve` citations and the + sticky-release claim in the transaction test's xmldoc. +5. **A6** — add the "a placement actually committed" positive to the differential. +6. **Commit message** must carry: the §3.1 audit outcome (11 deleted / 0 + re-pointed, with the reason), the §3.3 covering-test judgment, the §3.6 + coverage-loss declaration, the count reconciliation (11,112 − 11 + 4 = + 11,105 / 4 skips), and the `SyncPose` suspend-guard nuance noted in the + composition section above. diff --git a/docs/research/2026-08-05-c5a-contract.md b/docs/research/2026-08-05-c5a-contract.md new file mode 100644 index 00000000..6d85d74b --- /dev/null +++ b/docs/research/2026-08-05-c5a-contract.md @@ -0,0 +1,470 @@ +# C5a contract — legacy deletion sweep + carried parity tests (pinned 2026-08-05) + +Pinned at HEAD **`392c1e22`** (branch `claude/acdream-physics-divergence-5aa784`), +i.e. AFTER #319 landed. Every symbol, caller census, and line number below was +**re-verified against this HEAD by grep/read**, not inherited from the C5 +scoping (`2026-08-05-c5-scoping.md`, written at `52175aa1`) — §9 lists every +place the scoping's picture moved. Baseline: complete Release suite +**11,112 passed / 4 skipped / 0 failed**, measured at `392c1e22` (the #319 +commit message records the measurement; re-measure at implementation start, +never inherit — process rule (c)). + +**Scope, stated negatively first:** + +- **NOT #275.** The steady-state inbound-Position merge + (`InboundPhysicsStateController.TryApplyPosition`, the simple overload) and + `RuntimeEntityObjectLifetime`'s wire-derived `FullCellId` refresh (the + `refreshPosition: acceptedPosition` call, **now at `:1926`** post-#319) are + the C5b behaviour change with its own contract. C5a must not modify either + file's executable code (one test-file doc-comment correction is the only + permitted touch near this surface, §1 D7). +- **NOT the probe strip.** All six `ACDREAM_PROBE_*` temporary flags stay + (C5c); they are env-gated and inert to everything here. +- **NOT AP-131, NOT AD-60's legacy half, NOT AP-145's seam** (except the + pre-authorized red branch in §5.1). Those rows stay in the register + untouched. + +**Scope, positively:** the six deletion groups in §1 (~490 production lines), +the seven test-caller dispositions in §3, retirement of register rows **AP-1** +and **AD-1** in the same commit as the deletions, and the two carried parity +tests in §5 (#318 composition; route-2 B2). + +--- + +## 1. Deletion inventory — re-verified at `392c1e22` by symbol + +Caller censuses below are exhaustive over `src/` (all `*.cs`). Method: for +`Resolve`, every `.Resolve(` receiver in `src/` was enumerated and typed — 38 +distinct receiver/site classes, **none** a `PhysicsEngine` (see the grep-hygiene +note in §3.8: two of them are #319's NEW `ParentAttachmentState.Resolve`, a +name collision that did not exist when the scoping ran its census). For the +others, direct symbol grep over `src/` and `tests/`. + +| # | Symbol | Location at HEAD | Production callers | Test callers | ~Lines | +|---|---|---|---|---|---| +| D1 | `PhysicsEngine.Resolve(Vector3, uint, Vector3, float)` | `src/AcDream.Core/Physics/PhysicsEngine.cs:1863`–`~2200` (body ends before `ResolveWithTransition`'s xmldoc; the live method at `:2223` is a **different member** and stays) | **ZERO** | `PhysicsEngineTests.cs` ×12 (`:41,:48,:66,:88,:111,:150,:186,:211,:391,:434,:446,:460`); `Issue133DungeonTeleportPrefixTests.cs:58` | ~360 | +| D2 | `PhysicsEngine.HasCellSurface` | `PhysicsEngine.cs:1767`–`~1789` | only `Resolve` itself (`:1887`) — deletes with D1 | none | ~23 | +| D3 | `PhysicsEngine.ResolvePlacement` | `PhysicsEngine.cs:2748`–`~2815` | **ZERO** (sole non-test mention is the already-recorded retirement comment at `HeadlessSessionWorldProjection.cs:794`) | `InitialPlacementOverlapTests.cs:42`; `TransitionScratchDifferentialTests.cs:185,:194,:208,:217` | ~70 | +| D4 | `PlayerMovementController.SetPosition` (both overloads) | `src/AcDream.Runtime/Gameplay/PlayerMovementController.cs:1746,:1760` | **ZERO** — every `.SetPosition(` in `src/` outside `PhysicsEngine.cs` is `WorldEntity.SetPosition` (receivers `entity.`/`child.`) or Core `PhysicsEngine.SetPosition` via `_physics.Engine.` from `RuntimeSetPositionState` (`:2028,:3125,:4789`), the canonical path | 19 test files, ~80 sites (44 in `PlayerMovementControllerTests.cs` alone) — **fixture setup**, not subject (§3.5) | ~30 gross; ~15 net after the retained seed (§3.5) | +| D5 | `PlayerMovementController.CommitPreparedPosition` | `PlayerMovementController.cs:1789` | **ZERO** — production replacement is `ArmConstraintLeashAtCommittedPlacement` (`:1815`), called from `RuntimeLocalPlayerPhysicsPublicationState.cs:774`; `PreparePositionForCommit` (`:1776`) remains production via `RuntimeLocalPlayerPhysicsPublicationState.cs:219` | `PlayerMovementPlacementTransactionTests.cs:42`; `PlayerMovementControllerTests.cs:1158`; `RuntimeLocalPlayerPhysicsPublicationStateTests.cs:3007` | ~25 | +| D6 | `RuntimeSetPositionState.BeginAcceptedPlacement` / `BeginAuthoredPlacement` | `src/AcDream.Runtime/Physics/RuntimeSetPositionState.cs:1321,:1333` | **ZERO** — pure pass-throughs to `BeginAcceptedPlacementCore`; production reaches the core via `Apply` (`:1304`) and the authored sequence | **39 sites across 9 Runtime test files** (scoping said ~40/10; re-censused) | **0 — KEEP as documented test seam** (§3.7) | +| D7 | Doc hygiene | stale `BlipPosition`/`PlayerMovementController.SetPosition` doc refs at `src/AcDream.Core/Physics/Motion/ConstraintManager.cs:25` (note: the scoping's path lacked `Motion/`) and `src/AcDream.Core/Physics/PhysicsBody.cs:442`; the stale `TryApplyPickup (:1116)` citation in `RuntimeAcceptedPositionDriveControllerTests.cs` (~`:221` region; the method lives in `RuntimeEntityObjectLifetime`, currently ~`:1258+`) — **cite by symbol, not line**, in the correction | — | — | ~20 comment lines | + +Net production deletion: **~490 lines** (the scoping's ~540 minus D6's kept +~25 and D4's retained seed). All deletions are compile-loud. + +**Confirmed unchanged from the scoping's §1c (NOT deletable, re-spot-checked):** +`ILocalPlayerTeleportPlacement`/`LocalPlayerTeleportPlacement.Place` (live +post-commit presentation suffix, `LocalPlayerTeleportController.cs:243` still +calls `entity.SetPosition(controller.Position)` inside it); +`PlayerMovementController.SetPositionCore` (`:1845`, production via +`PreparePositionForCommit`); the route-4 leftovers; AP-135's bookkeeping; +`RuntimeLiveEntitySessionController.cs:141`'s pre-flip path (its file WAS +touched by #319, but the C4/C5-revisit comment's unblock condition is still +unmet). + +--- + +## 2. The survivor hazard — TWO production members inside the deletion region, not one + +The scoping named one. Re-verification at HEAD finds **two**: + +1. **`IsSpawnCellReady` (`PhysicsEngine.cs:1807`)** — production callers + `RuntimeSetPositionState.cs:2169,:4378` and + `SessionPlayerComposition.cs:374`. Sits between `HasCellSurface` (delete) + and `Resolve` (delete). +2. **`AdjustPosition` (`PhysicsEngine.cs:1813`)** — **the scoping never + dispositioned it.** It is production: `PhysicsCameraCollisionProbe.cs:38,:100` + (the camera collision probe), plus six-plus test files + (`PhysicsEngineAdjustPositionTests`, the camera replay suites, + `Issue177StairDescentCameraFloodTests`, ...). It sits between + `IsSpawnCellReady` and `Resolve` — dead centre of the physical block. + +**Pinned survival rule:** the deletion is **member-wise, never region-wise**. +Delete exactly the bodies of `HasCellSurface`, `Resolve`, and +`ResolvePlacement`; `IsSpawnCellReady` (`:1807`–`:1811`) and `AdjustPosition` +(`:1813`–`:1861`) remain byte-identical in executable code. + +**Xmldoc fallout (same commit):** `IsSpawnCellReady`'s summary contains +`` (in the `:1791`–`:1806` block) and `HasCellSurface`'s +summary names "the Resolve safety net"; the second dies with its method, the +first must be rewritten (the "loud outdoor-demote safety net" sentence +describes machinery this commit deletes — rewrite the paragraph to describe +the canonical `SetPosition` reality, do not leave a cref to a deleted symbol). +Sweep `PhysicsEngine.cs` for any other `cref="Resolve"`/`cref="ResolvePlacement"` +after the deletion; the build must be warning-clean on missing crefs. + +--- + +## 3. The seven test-only-caller dispositions — re-verified, each carried forward + +Framing (from the scoping, still correct): deleting a method whose callers are +tests is compile-loud. The silent hazard is the **disposition of the tests +afterward** — deleting a test that pinned MOVED behaviour loses the pin with a +green build. Each case below is a binding disposition; any deviation must be +argued in the commit message. + +### 3.1 `PhysicsEngine.Resolve` unit tests — behaviour GONE → DELETE, after a one-pass audit + +`PhysicsEngineTests.cs` (565 lines, 12 `engine.Resolve(` sites). The +outdoor-demote / legacy floor-snap / terrain-lift semantics die with the +method; canonical replacements have their own suites (`PhysicsSetPositionTests`, +`RuntimeSetPositionStateTests`). **Audit before deleting:** any individual +assertion that actually pins canonical-owned behaviour — specifically +`AdjustPosition` semantics, which SURVIVE — is re-pointed at `AdjustPosition` +directly (note `PhysicsEngineAdjustPositionTests.cs` already exists; a +re-point may land there). The audit's outcome (N assertions re-pointed, M +deleted) goes in the commit message. + +### 3.2 `Issue133DungeonTeleportPrefixTests` — behaviour MOVED → RE-POINT (named-bug regression pin) + +Verified at HEAD: the defect mechanism this test pins — the `lbPrefix` +resident-landblock scan that re-stamped a validated dungeon claim with a +neighbour's prefix — lives **entirely inside `Resolve`'s body** +(`PhysicsEngine.cs:1928,:1937,:1984-1995,:2193`). The canonical +`PhysicsSetPosition` path has **no lbPrefix scan** — the defect class cannot +recur there by construction. That is precisely why the pin must be +**re-pointed, not deleted**: #133 is a closed named bug, and the invariant +(a validated dungeon claim's landblock prefix is authoritative; a committed +cell never gets re-stamped from a neighbouring resident block) must stay +assertable against whatever path owns placement now. + +**Re-point spec:** drive a teleport-classified canonical placement +(`RuntimeSetPositionState`, or Core `PhysicsEngine.SetPosition` if the fixture +cost is lower) with the test's exact geometry — dungeon claim `0x00070143`, +dungeon block world-offset so its local Y is negative, resident neighbour +block at origin containing the same XY — and assert the committed cell keeps +the `0x0007` prefix. Alternative accepted by this contract: PROVE an existing +canonical test already pins prefix authority for an off-bounds dungeon claim +and record the proof (test name + assertion) in the deleting commit. Silent +deletion is a contract violation. + +### 3.3 `InitialPlacementOverlapTests` — behaviour MOVED → VERIFY-THEN-DELETE + +The ring-search half of enter-world placement is ported inside canonical +`SetPosition` (`TransitionTypes.cs:1596` `FindPlacementPosition`, retail +0x0050C170; `PhysicsSetPositionTests.cs` header cites it and has +placement-probe scenarios, e.g. the `placementPasses >= 2` retry arm at +`:1152-1194`). **The audit criterion:** confirm `PhysicsSetPositionTests` +covers the **other-entity-occupancy** ring search this test pins (a relogging +player overlapping a registered creature sphere searches outward to the +nearest clear ring) — occupancy-driven, not merely BSP-failure-driven. If +covered: delete, citing the covering test by name. If not: re-point this +test's scenario through canonical `SetPosition` with a placement class that +reaches `FindPlacementPosition` (~50–100 lines), then delete the +`ResolvePlacement` call. + +### 3.4 `TransitionScratchDifferentialTests` — differential arm → RE-POINT OR DROP EXPLICITLY + +Verified at HEAD: the spec-based sequence arms (`ResolveSpec.Resolve` at +`:618`) call `ResolveWithTransition` — **untouched by this slice**. Only +`ReusedScratch_MatchesFreshPlacementSearch` (`:180`–`:227`, four +`ResolvePlacement` sites) is affected. It is a Slice-I zero-alloc scratch-reuse +differential over the placement search (including the hostile-identity +leak check). **Disposition:** re-point the arm at the canonical entry that +reaches `FindPlacementPos` (Core `SetPosition` with the appropriate placement +class), preserving both the bitwise fresh-vs-reused comparison and the +second-identity leak check. If re-pointing is disproportionate, the arm may be +dropped ONLY with an explicit commit-message decision naming what coverage the +I-slice differential loses — never silently. + +### 3.5 `PlayerMovementController.SetPosition` fixture usage — NEITHER gone nor moved → RETAINED SEED + mechanical re-point + +Census at HEAD: **19 test files** reference `PlayerMovementController` and +call `.SetPosition(`; ~80 sites total; 44 in `PlayerMovementControllerTests.cs`, +8 in `LocalPlayerTeleportControllerTests.cs`, 5 in `HeadlessSessionHostTests.cs`, +the rest 1–4 each. (Per-site care: a file can reference the controller and +still call `WorldEntity.SetPosition` — type each site during the re-point, +don't regex-replace blind.) + +**Pinned design (the scoping's "cheaper and honest" option, adopted):** keep +**ONE** internal, explicitly-named test seed on the controller — rename the +3-arg overload to `SeedPlacementForTest(Vector3 pos, uint cellId, Vector3 cellLocal)` +(internal; xmldoc states it exists ONLY to seed fixtures and that production +placement flows through `PreparePositionForCommit` → +`ArmConstraintLeashAtCommittedPlacement`), delete the 2-arg overload, and +mechanically re-point all ~80 sites. Semantics are reproduced by construction: +the seed calls the SAME `SetPositionCore` (which stays production), so the +grounded, zero-velocity start (the AD-61 force-seed) that dozens of movement +tests assume is unchanged. This is the one production-file signature change +in the slice; its body is untouched. + +### 3.6 `CommitPreparedPosition` tests — behaviour MOVED → RE-POINT at the arm/commit replacement + +Three sites, each audited individually: + +- `PlayerMovementPlacementTransactionTests.cs:42` (100-line file): the + prepared-position transaction assertions run against the production pair + (`PreparePositionForCommit` + `ArmConstraintLeashAtCommittedPlacement`) — + rewrite the test against that pair, or delete it if + `RuntimeLocalPlayerPhysicsPublicationStateTests` provably covers the same + transaction shape (cite which test). +- `PlayerMovementControllerTests.cs:1158` and + `RuntimeLocalPlayerPhysicsPublicationStateTests.cs:3007` both assert + `Throws` on the uncommitted/displaced state. + Audit whether the replacement arm carries an equivalent guard; if yes, + re-point the throw assertion at it; if the guard died with the method, + delete the assertion WITH a commit-message note (guard semantics gone, not + overlooked). + +### 3.7 `BeginAcceptedPlacement`/`BeginAuthoredPlacement` — NEITHER → KEEP AS DOCUMENTED SEAM + +39 sites across 9 Runtime test files at HEAD. The wrappers are pure +pass-throughs to the production core (`BeginAcceptedPlacementCore`); deleting +them buys zero behaviour and costs broad mechanical churn across the Runtime +suite. **Disposition: keep, with an xmldoc sentence on each wrapper naming it +a test seam** (so a future sweep does not re-litigate this). This is a +recorded deliberate exception to "delete every superseded legacy path": the +wrappers are not a legacy PATH — the core they call IS the canonical path. + +### 3.8 Landmines and grep hygiene + +- **The #316-preserving pair** (`LiveEntityNetworkOnPositionCollapseMatrixTests.cs:131,:180`) + pins a defect **preserved verbatim**. C5a must not touch it; it inverts only + with #316's measured fix (C5-gate session / later). +- **#319 introduced `ParentAttachmentState.Resolve`** (`ParentAttachmentState.cs:432`), + called at `EquippedChildRenderController.cs:920` and + `RuntimeLiveEntitySessionController.cs:390`. A mechanical grep for + `.Resolve(` now hits relation resolution — neither site is `PhysicsEngine`. + Any "prove zero callers" re-run during implementation must type receivers, + not count matches. +- The affected-file overlap between #319 and this slice is **empty**: #319 + touched `EquippedChildRenderController`, `LiveEntityHydrationController`, + `LiveEntityPresentationController`, `ParentAttachmentState`, + `RuntimeEntityObjectLifetime`, `RuntimeLiveEntitySessionController` — none + contains a C5a deletion target. Verified. + +--- + +## 4. Register retirements — AP-1 and AD-1, with the code evidence; four rows explicitly untouchable + +A row retires because the code proves its condition met. Both retirements ride +**in the same commit as the D1–D5 deletions** (register rule 1). + +### AP-1 — RETIRE. Evidence at `392c1e22`: + +Row text: "Production zero-delta routes deliberately remain on the legacy +resolver until 4B2..." — **false at HEAD**: + +1. The "legacy resolver" is `PhysicsEngine.Resolve`/`ResolvePlacement`. The + exhaustive receiver census (§1) shows **zero** `PhysicsEngine.Resolve` or + `.ResolvePlacement` call sites in `src/`. +2. Every production placement writer reaches Core `PhysicsEngine.SetPosition` + **only** through `RuntimeSetPositionState` (`:2028,:3125,:4789` — the only + three `_physics.Engine.SetPosition` sites in `src/`). +3. The row's named prerequisites (authored mover, rebucketing, + prefix-quiescence, body publication, atomic route cutover) landed across + C0–C4; the local controller's body adoption landed at C3c. +4. Deleting D1–D5 makes the retirement **structural**: the resolver-shaped + entry points cease to exist, so no future caller can re-open the row's + condition. + +The narrower survivors (#276 settle-cell discard, AD-61 force-seed, AD-62 +non-commit outcomes) are separately filed rows/issues and do not block AP-1's +own condition — deleting AP-1 does not orphan them. + +### AD-1 — RETIRE. Evidence at `392c1e22`: + +Row text: "Production authoritative placement still routes through the legacy +recoverable outdoor demote and outdoor-restore `max(terrainZ, z)` lift" — +**false at HEAD**: that code is `Resolve`'s body (demote at +`PhysicsEngine.cs:~1890-1910`, the outdoor `max(terrain, z)` lift inside the +snap block ~`:2160-2175`) and `Resolve` has zero production callers. The +lost-cell stand-in the row describes is unreachable from production. Deleting +D1/D2 removes the divergent mechanism outright. + +### Must NOT be touched (each blocked on work outside this slice): + +- **AP-131** — retires only with **#275** (C5b): the legacy + `TryApplyPosition` unconditional `installPlacementFrame: true, clearParent: true` + is still the ONLY steady-state production Position merge at HEAD. +- **AD-60's legacy half** — same gate (#275): the + `RefreshSnapshot(..., refreshPosition: acceptedPosition)` site — **`:1926` + at HEAD** (the scoping's `:1918` and the register's `:1338` are both stale; + cite the symbol) — still derives `FullCellId` from bare wire acceptance. +- **AP-145** — retires with **#318's fix**, never with its test. The §5.1 + composition test makes the asymmetry falsifiable; only the pre-authorized + red branch may touch the seam, and then AP-145 retires in THAT commit. +- **AD-61 / AD-62 / AP-135 / AP-141–146** — all carry their own retirement + conditions; none is met by anything in this slice. (AP-146 and the AP-132 + amendment are #319's, three days old — do not disturb.) + +--- + +## 5. The two carried parity tests + +Both are test-only against HEAD's production code, land BEFORE the deletion +commit (they are independent of it and de-risk the slice's review), and both +follow process rule (e): sabotage-verified, with the WHICH-assertion-fails +check, both directions for dual-layer assertions. + +### 5.1 #318 composition test (~150–300 lines, App.Tests) + +Drive a real portal arrival through the canonical drive controller + the +**REAL** `RuntimePlacementPresentationSink` + the **REAL** `PhysicsEngine` +(fixture patterns exist: `RuntimePlacementPresentationSinkTests.cs`, +`RuntimeFirstEntryHostIntegrationTests.cs`). The discriminating assertion: + +> **`PhysicsEngine.ShadowObjects` (`PhysicsEngine.cs:147`) holds a row at the +> destination cell/position** — NEVER merely `LocalPlayerShadowState`'s dedup +> cache. AP-145's bypass (`RuntimePlacementPresentationSink.cs:243` +> `_localPlayerShadow.Set(...)` skipping `LocalPlayerShadowSynchronizer.SyncPose`'s +> publish) both skips the publish AND pre-seeds `SyncPose`'s dedup — a +> cache-only assertion is satisfied by the bug. + +Plus the T8 write-ordering assertion from route 3 §8. Sabotage: perturb +`TryPublishPlace` to the cache-only shape and confirm the `ShadowObjects` +assertion (not an incidental one) fails; separately confirm a cache-only +assertion would pass under the same sabotage — proving the discriminator +discriminates. + +**Pre-authorized red branch:** this test may legitimately FAIL at HEAD — the +composition drives placement with no subsequent movement tick, which is +exactly the window AP-145 says is unpublished. If red: **C5a's deletion work +does not absorb the fix.** The seam fix (routing the placement's shadow update +through the real publish) is a production behaviour change on a +narrow, low-frequency path; it lands as its **own reviewed commit** together +with the now-green test, retires **AP-145**, and closes **#318** — and the +composition test itself is its designed gate (the C4 handoff explicitly ruled +the connected route out as #318 coverage). If green: land as-is; #318 closes; +AP-145's row is then re-argued (its "why not observed" column may become its +retirement argument) — but only with the green evidence cited. + +### 5.2 Route-2 B2 parity test (~100–200 lines, App.Tests) + +The campaign plan's recorded acceptance gap (plan §C4 route 2, recorded unmet +since 2026-08-03): an App-layer test driving an **accepted ForcePosition end +to end** through `RuntimePlacementPresentationSink` / +`TryApplyRuntimePlacementPlace` and asserting **the render entity's +position/cell came from the committed placement receipt** — closing the +"canonical body moves, render entity stays put" silent seam. Expected green +at HEAD (route 2 landed; the seam is merely uncovered). If red, the same +stop-and-report protocol as 5.1: a red parity test is a found defect, not a +test problem; it gets its own investigation before any deletion lands. + +Sabotage: sever the receipt→render write and confirm the position/cell +assertion is the one that fails. + +--- + +## 6. What must REMAIN true — the slice's invariants + +1. **Zero production behaviour change.** The production diff consists of: + member deletions with zero callers (D1–D5), comment/xmldoc edits (D7, §2), + and exactly one signature change with an untouched body (§3.5's seed + rename). No executable production statement is added or modified — + **except** in the pre-authorized 5.1 red-branch commit, which is its own + reviewed landing with its own register action. +2. **The two survivors survive.** `IsSpawnCellReady` and `AdjustPosition` + keep their exact executable bodies and all production callers + (`RuntimeSetPositionState.cs:2169,:4378`; `SessionPlayerComposition.cs:374`; + `PhysicsCameraCollisionProbe.cs:38,:100`). +3. **Every deleted symbol's absence is proven** by the compiler (all deletions + are compile-loud) AND every test caller has an explicit §3 disposition + executed in the same commit — no test deleted whose pinned behaviour moved + without its re-point landing alongside. +4. **AP-1 and AD-1 retire in the SAME commit as the D1–D5 deletions** — never + before (the code proof is the deletion), never after (register rule 1). +5. **The DO-NOT-TOUCH set holds:** AP-131, AD-60, AP-145 (modulo 5.1 red + branch), AD-61/62, AP-135, AP-141–146; the #275 surface files' executable + code; the six probe flags; the #316-preserving test pair. +6. **No skips.** The suite ends at 0 failed with the same 4 skips as + baseline — a new skip is a contract violation (process rule (d)). +7. **Counts are measured and reconciled.** The final suite total will move + (deleted legacy tests down, re-points and two parity tests up); the commit + message reconciles the net against baseline 11,112 explicitly (N deleted, + M added, expected total), never hand-waves it. + +--- + +## 7. Gates + +- **Complete Release suite** (`dotnet test AcDream.slnx -c Release -m:1` with + `ACDREAM_PAK_PATH` set), baseline **11,112 passed / 4 skipped / 0 failed** + at `392c1e22` — re-measured at slice start AND at each commit. Known flakes, + never conflated (they have been conflated twice): **#302** + (`PortalProjectionTests.ClipToRegion_FrameOwnedStore_…`, GC-allocation, + App.Tests — the `WarmedSteadyContactRefreshDoesNotAllocate` look-alike is + this class) and **#308** (`NakEmissionTests.LossSoak_…`, wall-clock, + Core.Net.Tests, full-suite load only). +- **NO connected gate for C5a — argued, not assumed.** Process rule (g): a + gate must be able to see the defect it gates. C5a's reachable defect + classes are (i) a compile break — seen by the build; (ii) silent coverage + loss — seen only by §3's dispositions and the review, invisible to any live + session; (iii) a behavioural regression — **structurally excluded** by + invariant 1: the production binary's reachable code is byte-equivalent, so + a connected session would exercise identical behaviour and measure nothing. + Precedents: route 5 recorded "no live gate can exist" rather than inventing + one; route 6 was a zero-production-line closure. The 5.1 red-branch commit, + if taken, ALSO needs no connected gate: #318's evidence channel is the + composition test **by design** — the C4 handoff explicitly refused to score + the connected route against it. +- **The review IS the coverage gate.** One dual review over the combined + slice diff (deletions + dispositions + parity tests), reviewers on Opus per + the standing audit rule, with §3's table as the review checklist: for each + of the seven cases, the reviewer confirms the disposition was executed as + pinned or the deviation argued. + +--- + +## 8. Size, commit plan, and the split call + +Calibration: campaign landings ran ~127 (route 7) to ~418 (route 3) to +~500 (4b-2) production lines each under full discipline. + +| Piece | Production lines | Test lines | Risk | +|---|---|---|---| +| D1–D5+D7 deletions + §2 xmldoc | ~490 deleted, ~0 added | ~1,500–2,000 deleted/re-pointed across ~25 files | Low — compile-loud; the §3 dispositions are the judgment work | +| §5.1 #318 composition test | 0 (green) / ~10–40 (red branch, own commit) | ~150–300 | Low; red branch is a decision point, pre-planned | +| §5.2 route-2 B2 test | 0 | ~100–200 | Low | + +**Call: C5a HOLDS as one slice, in two (possibly three) ordered commits under +this single contract:** + +1. **Commit 1 — the two parity tests** (test-only). Lands first: independent + of the deletions, de-risks review, and settles 5.1's green/red question + before the sweep. If 5.1 is red, its fix is **commit 1b** (own review + round, retires AP-145, closes #318) before proceeding. +2. **Commit 2 — the deletion sweep**: D1–D5, D7, §2 xmldoc, all §3 + dispositions, AP-1 + AD-1 row deletions. One diff, reviewable as one unit. + +The ~490-line figure is at the top of the campaign's calibration band, but a +deletion of caller-free code is a different risk class from route 3's ~418 +changed lines — the compiler proves most of it. What justifies keeping it +whole rather than splitting D1/D2/D3 (Core) from D4/D5 (Runtime): AP-1's +retirement evidence spans BOTH groups ("the last resolver-shaped entry points" +includes D4/D5), so splitting would either retire AP-1 on a half-proof or +leave the register straddling two commits — both worse than one larger +reviewable deletion. Do NOT fold in: #276's remainder, #317's audit, any +probe change (including the gate-4 `cause=` label improvement — C5c), or any +#275-adjacent edit. + +--- + +## 9. What moved between the scoping (`09911821`, at `52175aa1`) and this contract (`392c1e22`) + +1. **`AdjustPosition` is a second production survivor inside the deletion + region** (`PhysicsCameraCollisionProbe.cs:38,:100`) — the scoping's hazard + note named only `IsSpawnCellReady`. A region-wise delete would have taken + the camera collision probe's cell resolver with it. §2 pins member-wise + deletion. +2. **#319 created a `.Resolve(` name collision**: `ParentAttachmentState.Resolve` + (`:432`), called from two files. The scoping's census predates it. Callers + must be typed, not counted (§3.8). +3. **AD-60's legacy-half site moved to `RuntimeEntityObjectLifetime.cs:1926`** + (scoping said `:1918`; the register row still says `:1338`). C5a doesn't + touch it, but C5b's contract must cite by symbol. +4. **Begin* wrapper census: 39 sites / 9 files** (scoping: ~40 / 10). + Immaterial to the disposition. +5. **The scoping's D7 path `ConstraintManager.cs` is actually + `Motion/ConstraintManager.cs`** (`src/AcDream.Core/Physics/Motion/`). +6. **Everything else in the scoping's §1a/§1c/§2/§3 holds exactly at + `392c1e22`**: zero production callers re-proven for all six groups; #319 + added no caller to any deletable symbol; the #319-touched file set is + disjoint from every deletion target; the seven dispositions carry forward + unchanged in substance. +7. **New since the scoping, absorbed here:** gate 4 closed 2026-08-05 as a + probe-label artifact (`af828a8a`) — the cell-less falsification is no + longer C5-gate-session work; and route 7's gate criterion was corrected to + the positive child-cell-equals-parent assertion (`2687d893`), whose + still-owed connected run belongs to the C4/#319 ledger, not to C5a. diff --git a/src/AcDream.App/Composition/LivePresentationComposition.cs b/src/AcDream.App/Composition/LivePresentationComposition.cs index 83d6e26e..dc573bc4 100644 --- a/src/AcDream.App/Composition/LivePresentationComposition.cs +++ b/src/AcDream.App/Composition/LivePresentationComposition.cs @@ -97,6 +97,12 @@ internal sealed record LivePresentationResult( RenderSceneShadowRuntime? RenderSceneShadow, LiveEntityRuntime LiveEntities, RuntimePlacementPresentationSink PlacementProjection, + // AP-145 fix (2026-08-05, #318): constructed HERE (before the sink that + // consumes it) rather than later in SessionPlayerComposition, so both + // consumers share the ONE synchronizer instance / ONE + // LocalPlayerShadowState cache. Two instances would themselves + // reintroduce a cache-desync class this fix exists to close. + LocalPlayerShadowSynchronizer LocalPlayerShadowSynchronizer, ProjectileController ProjectileController, LiveEntityProjectionWithdrawalController ProjectionWithdrawal, LiveEntityLightController Lights, @@ -491,13 +497,27 @@ internal sealed class LivePresentationCompositionPhase if (visible) entityEffects?.OnPresentationBound(record); }); + // AP-145 fix (2026-08-05, #318): constructed here, BEFORE the + // sink, so the sink can publish the local player's Place + // through the same seam ordinary per-tick movement uses rather + // than writing LocalPlayerShadowState directly. Threaded through + // to SessionPlayerComposition via LivePresentationResult so + // there remains exactly one synchronizer / one cache for the + // whole session — SessionPlayerComposition no longer constructs + // its own. + var localPlayerShadowSynchronizer = new LocalPlayerShadowSynchronizer( + d.PhysicsEngine, + liveEntities, + d.PlayerIdentity, + d.WorldOrigin, + d.LocalPlayerShadow); var placementProjection = new RuntimePlacementPresentationSink( liveEntities, worldTransit, d.WorldGameState, d.WorldEvents, d.EffectPoses, - d.LocalPlayerShadow, + localPlayerShadowSynchronizer, () => d.PlayerIdentity.ServerGuid, guid => { @@ -668,6 +688,7 @@ internal sealed class LivePresentationCompositionPhase renderSceneShadowLease, liveEntities, placementProjection, + localPlayerShadowSynchronizer, projectileController, projectionWithdrawal, lightsLease, @@ -717,6 +738,7 @@ internal sealed class LivePresentationCompositionPhase RenderSceneShadowRuntime>? renderSceneShadowLease, LiveEntityRuntime liveEntities, RuntimePlacementPresentationSink placementProjection, + LocalPlayerShadowSynchronizer localPlayerShadowSynchronizer, ProjectileController projectileController, LiveEntityProjectionWithdrawalController projectionWithdrawal, CompositionAcquisitionScope.CompositionAcquisitionLease lightsLease, @@ -1196,6 +1218,7 @@ internal sealed class LivePresentationCompositionPhase renderSceneShadow, liveEntities, placementProjection, + localPlayerShadowSynchronizer, projectileController, projectionWithdrawal, lightsLease.Resource, diff --git a/src/AcDream.App/Composition/SessionPlayerComposition.cs b/src/AcDream.App/Composition/SessionPlayerComposition.cs index ec4619b9..a29b8846 100644 --- a/src/AcDream.App/Composition/SessionPlayerComposition.cs +++ b/src/AcDream.App/Composition/SessionPlayerComposition.cs @@ -796,12 +796,12 @@ internal sealed class SessionPlayerCompositionPhase d.AnimatedEntities, live.AnimationPresenter, content.AnimationHookFrames); - var localPlayerShadow = new LocalPlayerShadowSynchronizer( - d.PhysicsEngine, - live.LiveEntities, - d.PlayerIdentity, - d.WorldOrigin, - d.PlayerShadow); + // AP-145 fix (2026-08-05, #318): the ONE synchronizer instance is + // now constructed earlier, in LivePresentationComposition, so + // RuntimePlacementPresentationSink's Place edge and this session's + // ordinary per-tick movement publish through the exact same + // publisher / cache rather than two independent instances. + var localPlayerShadow = live.LocalPlayerShadowSynchronizer; var localPlayerProjection = new LocalPlayerProjectionController( new LiveLocalPlayerProjectionRuntime( live.LiveEntities, diff --git a/src/AcDream.App/World/RuntimePlacementPresentationSink.cs b/src/AcDream.App/World/RuntimePlacementPresentationSink.cs index b0320d73..bb77dd12 100644 --- a/src/AcDream.App/World/RuntimePlacementPresentationSink.cs +++ b/src/AcDream.App/World/RuntimePlacementPresentationSink.cs @@ -26,7 +26,7 @@ internal sealed class RuntimePlacementPresentationSink private readonly WorldGameState _worldState; private readonly WorldEvents _worldEvents; private readonly EntityEffectPoseRegistry _effectPoses; - private readonly LocalPlayerShadowState _localPlayerShadow; + private readonly LocalPlayerShadowSynchronizer _localPlayerShadowSync; private readonly Func _localPlayerGuid; private readonly Action _clearSelectionForUnavailableEntity; private readonly Action[] _visibilitySinks; @@ -37,7 +37,7 @@ internal sealed class RuntimePlacementPresentationSink WorldGameState worldState, WorldEvents worldEvents, EntityEffectPoseRegistry effectPoses, - LocalPlayerShadowState localPlayerShadow, + LocalPlayerShadowSynchronizer localPlayerShadowSync, Func localPlayerGuid, Action clearSelectionForUnavailableEntity, IEnumerable>? visibilitySinks = null) @@ -49,8 +49,13 @@ internal sealed class RuntimePlacementPresentationSink _worldEvents = worldEvents ?? throw new ArgumentNullException(nameof(worldEvents)); _effectPoses = effectPoses ?? throw new ArgumentNullException(nameof(effectPoses)); - _localPlayerShadow = localPlayerShadow - ?? throw new ArgumentNullException(nameof(localPlayerShadow)); + // AP-145 fix (2026-08-05, #318, architecture review A2): BOTH the + // Place and Withdraw halves now route the local player's shadow + // exclusively through this ONE publisher (SyncPose / Suspend), which + // owns the LocalPlayerShadowState cache internally — this sink no + // longer needs a direct reference to the cache at all. + _localPlayerShadowSync = localPlayerShadowSync + ?? throw new ArgumentNullException(nameof(localPlayerShadowSync)); _localPlayerGuid = localPlayerGuid ?? throw new ArgumentNullException(nameof(localPlayerGuid)); _clearSelectionForUnavailableEntity = clearSelectionForUnavailableEntity @@ -240,10 +245,46 @@ internal sealed class RuntimePlacementPresentationSink if (record.ServerGuid == _localPlayerGuid()) { - _localPlayerShadow.Set( + // AP-145 fix (2026-08-05, #318): route through the SAME + // publisher ordinary per-tick movement uses + // (LocalPlayerShadowSynchronizer.SyncPose), not a direct + // LocalPlayerShadowState.Set. The old direct write updated only + // the dedup cache, never PhysicsEngine.ShadowObjects — the + // portal jump's real collision shadow stayed at the SOURCE cell + // until an unrelated movement tick happened to drift far enough + // to defeat SyncPose's own dedup check (which the direct write + // had just pre-seeded with the destination pose, so even that + // recovery could silently miss). SyncPose both publishes the + // real ShadowObjects row (via Register, which first deregisters + // any prior cell rows — no stale source-cell row, no duplicate) + // and records the dedup cache as its own last step, in the + // correct order. force:true because this IS the authoritative + // placement commit, not an ordinary per-tick refresh — it must + // never be skipped by the dedup path. + // + // Behaviour-change nuance (architecture review, 2026-08-05): + // routing through SyncPose means Place now inherits SyncPose's + // own admission guard — IsHidden(...), cellId == 0, or + // !IsCurrentVisibleProjection(entity) (not the current spatial + // root / not a current record) — none of which the old direct + // .Set() call ever consulted. Under any of those conditions + // SyncPose calls Suspend(entity) instead of publishing: the real + // ShadowObjects row is REMOVED and the cache is cleared, where + // the old write would have left a stale ShadowObjects row in + // place and simply overwritten the cache. This is the correct, + // symmetric behaviour — it is exactly what the very next + // ordinary per-tick Sync call would do in the same situation — + // and it is covered by the same player-only gate this method + // already had, but it IS new: a Place that lands while the + // record is momentarily not the current visible spatial root + // (a narrow, low-frequency window) now suspends the real shadow + // where it previously left a possibly-stale one untouched. + _localPlayerShadowSync.SyncPose( + entity, entity.Position, entity.Rotation, - record.FullCellId); + record.FullCellId, + force: true); } for (int i = 0; i < _visibilitySinks.Length; i++) @@ -279,7 +320,20 @@ internal sealed class RuntimePlacementPresentationSink if (!IsCurrent(record, entity)) return false; if (record.ServerGuid == _localPlayerGuid()) - _localPlayerShadow.Clear(); + { + // AP-145 fix, Withdraw half (2026-08-05, architecture review + // A2): the exact mirror of the Place-side fix. The old direct + // _localPlayerShadow.Clear() only cleared the dedup cache, + // leaving a LIVE phantom row in PhysicsEngine.ShadowObjects at + // the park's source cell for the whole park window (the #184 + // shape) — every other entity's collision sweep in that cell + // would collide with a player who is, per every other acdream + // predicate, gone. Suspend() does both: real registry suspend + // (ShadowObjectRegistry.Suspend) AND the cache clear, in the + // one call LocalPlayerShadowSynchronizer already exposes for + // exactly this pairing (see its own Suspend/SyncPose split). + _localPlayerShadowSync.Suspend(entity); + } if (!IsCurrent(record, entity)) return false; _clearSelectionForUnavailableEntity(record.ServerGuid); diff --git a/tests/AcDream.App.Tests/World/RuntimeFirstEntryHostIntegrationTests.cs b/tests/AcDream.App.Tests/World/RuntimeFirstEntryHostIntegrationTests.cs index a5834879..02200752 100644 --- a/tests/AcDream.App.Tests/World/RuntimeFirstEntryHostIntegrationTests.cs +++ b/tests/AcDream.App.Tests/World/RuntimeFirstEntryHostIntegrationTests.cs @@ -502,13 +502,26 @@ public sealed class RuntimeFirstEntryHostIntegrationTests AcDream.Runtime.Gameplay .RuntimeLocalPlayerShadowDisposition .ProvenShapeless)); + var localShadowState = new LocalPlayerShadowState(); + var localShadowIdentity = new LocalPlayerIdentityState + { + ServerGuid = playerGuid, + }; + var localShadowOrigin = new LiveWorldOriginState(); + localShadowOrigin.SetPlaceholder(0, 0); + var localShadowSynchronizer = new LocalPlayerShadowSynchronizer( + EntityObjects.Physics.Engine, + Runtime, + localShadowIdentity, + localShadowOrigin, + localShadowState); var sink = new RuntimePlacementPresentationSink( Runtime, new RuntimeWorldTransitState(), WorldState, new WorldEvents(), new EntityEffectPoseRegistry(), - new LocalPlayerShadowState(), + localShadowSynchronizer, () => playerGuid, _ => { }, [ diff --git a/tests/AcDream.App.Tests/World/RuntimeForcePositionRenderCommitTests.cs b/tests/AcDream.App.Tests/World/RuntimeForcePositionRenderCommitTests.cs new file mode 100644 index 00000000..e19088d5 --- /dev/null +++ b/tests/AcDream.App.Tests/World/RuntimeForcePositionRenderCommitTests.cs @@ -0,0 +1,593 @@ +using System.Net; +using System.Numerics; +using AcDream.App.Input; +using AcDream.App.Physics; +using AcDream.App.Rendering; +using AcDream.App.Rendering.Vfx; +using AcDream.App.Streaming; +using AcDream.App.World; +using AcDream.Content; +using AcDream.Content.Pak; +using AcDream.Core.Items; +using AcDream.Core.Net; +using AcDream.Core.Net.Messages; +using AcDream.Core.Physics; +using AcDream.Core.Plugins; +using AcDream.Core.World; +using AcDream.Runtime; +using AcDream.Runtime.Entities; +using AcDream.Runtime.Gameplay; +using AcDream.Runtime.Physics; +using AcDream.Runtime.Session; +using AcDream.Runtime.World; +using DatReaderWriter.DBObjs; + +namespace AcDream.App.Tests.World; + +/// +/// C5a §5.2 (2026-08-05) — the route-2 B2 closure, corrected at the +/// architecture-review re-pass (A3). Route 2's review +/// (docs/research/2026-08-03-c4-route-2-review-findings.md B2) found: +/// "no test drives a route-2 ForcePosition through +/// / +/// LiveEntityRuntime.TryApplyRuntimePlacementPlace and asserts the +/// moved." +/// +/// +/// A3 correction: the first version of this test hand-authored a +/// RuntimePlacementProjectionSnapshot and called Sink.TryApply +/// directly — it never drove a ForcePosition at all, and the five facts it +/// asserted were already pinned by +/// RuntimePlacementPresentationSinkTests.Place_ReframesAndRebucketsExactSidecarWithoutMutatingRuntimePhysics. +/// This version drives the REAL production path: a bare +/// hosts a live local player +/// ( → the real +/// +/// pump, mirroring RuntimeFirstEntryHostIntegrationTests's established +/// composition), then a REAL +/// +/// call — fed by the REAL +/// merge, exactly like RuntimeAcceptedPositionDriveControllerTests.MergeAccepted +/// — commits the canonical placement. The REAL +/// +/// (subscribed to the SAME 's +/// placement channel) synchronously forwards the resulting receipt to the +/// REAL , which writes the +/// render . Nothing in this chain is hand-authored; +/// the receipt is the ForcePosition path's OWN output. +/// +/// +/// +/// Sabotage-verified (manual): with the receipt→render write severed +/// (RuntimePlacementPresentationSink.TryApply short-circuited to +/// acknowledge-and-ignore a Place without calling +/// LiveEntityRuntime.TryApplyRuntimePlacementProjection), the test +/// fails at exactly the position assertion (the entity stays at its +/// pre-force pose); restored, it is green. +/// +/// +public sealed class RuntimeForcePositionRenderCommitTests +{ + private const uint Cell = 0x01010001u; + private const uint PlayerGuid = 0x7000B201u; + // M1 (architecture review round 2, 2026-08-05): landblock-local (30,30) + // deliberately crosses OUT of the spawn's outdoor grid cell (cx=0,cy=0 -> + // low word 0x0001) into cx=1,cy=1 (TerrainSurface.CellSize=24) -> low + // word 0x000A, so ForcedCell != Cell and the cell assertion below is + // actually falsifiable. The prior (15,15) choice stayed inside the SAME + // grid cell as the spawn, so Assert.Equal(Cell, entity.ParentCellId) + // passed before the drive ever ran — a vacuous cell assertion the + // position assertion's own strength (wire Z=0 vs resolved Z=0.48) had + // been masking. + private static readonly Vector3 ForcedPosition = new(30f, 30f, 0.48f); + private const uint ForcedCell = 0x0101000Au; + + [Fact] + public void AcceptedForcePosition_DrivenEndToEnd_MovesRenderEntityFromTheCommittedReceipt() + { + using var fixture = new HostFixture(); + fixture.Controller.OnCreate(Spawn(PlayerGuid, Cell)); + + PlayerMovementController controller = Assert.IsType( + fixture.Movement.Controller); + Assert.True(fixture.Runtime.TryGetRecord(PlayerGuid, out LiveEntityRecord record)); + WorldEntity entity = Assert.IsType(record.WorldEntity); + Vector3 positionBeforeForce = entity.Position; + + // The accepted ForcePosition wire update — a genuine forced position + // and heading correction, admitted by the REAL PhysicsTimestampGate + // (fresh FORCE_POSITION_TS with an equal TELEPORT_TS, retail + // SmartBox::HandleReceivedPosition @0x00453FD0's FORCE_POSITION + // branch) exactly like RuntimeAcceptedPositionDriveControllerTests + // drives it in Runtime.Tests. + WorldSession.EntityPositionUpdate wire = ForceUpdate(ForcedPosition); + Assert.True(fixture.EntityObjects.TryApplyPosition( + wire, + isLocalPlayer: true, + forcePositionRotation: controller.BodyOrientation, + currentLocalVelocity: controller.BodyVelocity, + acknowledgeProjection: null, + out PositionTimestampDisposition disposition, + out _, + out AcceptedPhysicsTimestamps timestamps)); + Assert.Equal(PositionTimestampDisposition.ForcePosition, disposition); + + RuntimeAcceptedPositionExecutionStatus status = + fixture.Drive.TryExecuteAcceptedLocalPosition( + record.Canonical, + wire, + disposition, + timestamps, + timestamps.PreviousTeleport); + + // THE DISCRIMINATING ASSERTIONS. Nothing here is asserted from the + // wire update or a hand-authored snapshot — every value is read back + // from the render entity AFTER the real drive + real subscription + + // real sink chain ran. + Assert.Equal(RuntimeAcceptedPositionExecutionStatus.Committed, status); + Assert.NotEqual(positionBeforeForce, entity.Position); + Assert.Equal(ForcedPosition, entity.Position); + // M1 (architecture review round 2): ForcedCell genuinely differs + // from the spawn Cell — this assertion is independently + // falsifiable, verified by isolating it ahead of the position + // asserts under the same receipt->render sabotage: it fails on its + // own (entity.ParentCellId stays at the spawn Cell), not merely + // alongside the position assertion. + Assert.NotEqual(Cell, entity.ParentCellId); + Assert.Equal(ForcedCell, entity.ParentCellId); + Assert.True(record.IsSpatiallyProjected); + Assert.True(record.IsSpatiallyVisible); + } + + private static WorldSession.EntityPositionUpdate ForceUpdate( + Vector3 position, + ushort positionSequence = 2, + ushort forcePositionSequence = 1) => + new( + PlayerGuid, + new CreateObject.ServerPosition( + Cell, + position.X, + position.Y, + position.Z, + 1f, + 0f, + 0f, + 0f), + Velocity: null, + PlacementId: null, + IsGrounded: true, + InstanceSequence: 1, + PositionSequence: positionSequence, + TeleportSequence: 0, + ForcePositionSequence: forcePositionSequence); + + private static WorldSession.EntitySpawn Spawn(uint guid, uint cell) + { + var position = new CreateObject.ServerPosition( + cell, 10f, 10f, 5f, 1f, 0f, 0f, 0f); + var timestamps = new PhysicsTimestamps( + Position: 1, + Movement: 1, + State: 1, + Vector: 1, + Teleport: 0, + ServerControlledMove: 1, + ForcePosition: 0, + ObjDesc: 1, + Instance: 1); + var physics = new PhysicsSpawnData( + RawState: (uint)PhysicsStateFlags.ReportCollisions, + Position: position, + Movement: null, + AnimationFrame: null, + SetupTableId: 0x02000001u, + MotionTableId: 0x09000001u, + SoundTableId: null, + PhysicsScriptTableId: null, + Parent: null, + Children: null, + Scale: null, + Friction: null, + Elasticity: null, + Translucency: null, + Velocity: null, + Acceleration: null, + AngularVelocity: null, + DefaultScriptType: null, + DefaultScriptIntensity: null, + Timestamps: timestamps); + return new WorldSession.EntitySpawn( + guid, + position, + 0x02000001u, + [], + [], + [], + null, + null, + "force-position fixture", + (uint)ItemType.Creature, + null, + 0x09000001u, + PhysicsState: (uint)PhysicsStateFlags.ReportCollisions, + InstanceSequence: 1, + MovementSequence: 1, + ServerControlSequence: 1, + PositionSequence: 1, + Physics: physics); + } + + /// + /// Mirrors RuntimeFirstEntryHostIntegrationTests.HostFixture's + /// established composition (bare + /// + REAL + REAL + /// ), extended with a + /// constructed + /// against the SAME entity objects, movement state, and generation — + /// exactly the shape RuntimeAcceptedPositionDriveControllerTests + /// .CreateAcceptedPositionDrive uses in Runtime.Tests, adapted to + /// the App-layer bare-lifetime pattern instead of a full + /// GameRuntime. + /// + private sealed class HostFixture : IDisposable + { + internal readonly RuntimeEntityObjectLifetime EntityObjects = new(); + internal readonly LiveEntityRuntime Runtime; + internal readonly LiveEntityHydrationController Controller; + internal readonly AcDream.Runtime.Session.RuntimeFirstEntryDriveController + FirstEntry; + internal readonly RuntimeAcceptedPositionDriveController Drive; + internal readonly RuntimeLocalPlayerMovementState Movement; + internal readonly WorldGameState WorldState = new(); + private readonly WorldSession _session; + + internal HostFixture() + { + EntityObjects.BindEventContext( + static () => new RuntimeGenerationToken(1UL), + static () => 1UL); + EntityObjects.Physics.SetPosition.BeginCollisionGeneration( + Cell & 0xFFFF0000u, 1UL); + EntityObjects.Physics.Engine.AddLandblock( + Cell & 0xFFFF0000u, + new TerrainSurface(new byte[81], new float[256]), + Array.Empty(), + Array.Empty(), + worldOffsetX: 0f, + worldOffsetY: 0f); + EntityObjects.Physics.SetPosition.CommitCollisionGeneration( + Cell & 0xFFFF0000u, 1UL, ready: true); + EntityObjects.Physics.ObserveLocalWorldFrame( + Cell, teleportAdvanced: false); + + Movement = new RuntimeLocalPlayerMovementState(); + var runtimeIdentity = new RuntimeLocalPlayerIdentityState(); + var publication = new RuntimeLocalPlayerPhysicsPublicationState( + EntityObjects.Entities, + EntityObjects.Physics, + Movement, + runtimeIdentity); + Movement.AttachPhysicsPublication(publication); + EntityObjects.LocalPlayerFirstEntry.BindPublication(publication); + runtimeIdentity.ServerGuid = PlayerGuid; + + var spatial = new GpuWorldState(); + spatial.AddLandblock(new LoadedLandblock( + (Cell & 0xFFFF0000u) | 0xFFFFu, + new LandBlock(), + Array.Empty())); + Runtime = new LiveEntityRuntime( + spatial, + new NoopResources(), + EntityObjects); + + FirstEntry = new AcDream.Runtime.Session.RuntimeFirstEntryDriveController( + EntityObjects, + new GameRuntimeClock(), + new UnusedCollisionSource(), + static () => PlayerMovementConstructionOptions.Fallback, + static _ => new RuntimeLocalPlayerPhysicsActivationPreparation( + 0.48f, + 1.835f, + RuntimeLocalPlayerShadowDisposition.ProvenShapeless)); + + var localShadowState = new LocalPlayerShadowState(); + var localShadowIdentity = new LocalPlayerIdentityState + { + ServerGuid = PlayerGuid, + }; + var localShadowOrigin = new LiveWorldOriginState(); + localShadowOrigin.SetPlaceholder(0, 0); + var localShadowSynchronizer = new LocalPlayerShadowSynchronizer( + EntityObjects.Physics.Engine, + Runtime, + localShadowIdentity, + localShadowOrigin, + localShadowState); + var sink = new RuntimePlacementPresentationSink( + Runtime, + new RuntimeWorldTransitState(), + WorldState, + new WorldEvents(), + new EntityEffectPoseRegistry(), + localShadowSynchronizer, + () => PlayerGuid, + _ => { }, + [(_, _) => { }]); + _ = new AcDream.Runtime.Physics.RuntimePlacementProjectionSubscription( + EntityObjects.Placements, + static () => new RuntimeGenerationToken(1UL), + sink); + + var materializer = new HostMaterializer(Runtime); + var identity = new LocalPlayerIdentityState { ServerGuid = PlayerGuid }; + var dormant = new DormantLiveEntityStore(); + var deletion = new LiveEntityDeletionController( + Runtime, + EntityObjects, + new NoopTeardown(), + identity, + dormant); + Controller = new LiveEntityHydrationController( + Runtime, + EntityObjects, + new object(), + materializer, + new NoopRelationships(), + new AcceptingReady(), + new KnownOrigin(), + new NoopNetworkSink(), + new NoopTimestamps(), + identity, + deletion, + dormant, + firstEntry: FirstEntry); + + _session = new WorldSession( + new IPEndPoint(IPAddress.Loopback, 9000), + new FixtureTransport()) + { + // Phase I.3 test seam: intercepts the outbound ack body + // before the wire-write path, which would otherwise NPE on + // an unseeded ISAAC keystream (this fixture never runs a + // real Connect() handshake). This test's subject is the + // canonical commit -> render entity chain, not the outbound + // ack itself. + GameActionCapture = _ => { }, + }; + Drive = new RuntimeAcceptedPositionDriveController( + EntityObjects, + new GameRuntimeClock(), + new UnusedCollisionSource(), + new LocalPlayerOutboundController((_, _, _, _, _, _) => { }), + static () => new RuntimeGenerationToken(1UL), + static () => PlayerGuid, + () => Movement.Controller, + // usePositionFromServer: true (autonomy level 2) suppresses + // the outbound ack this test doesn't exercise — the fixture + // WorldSession never negotiates ISAAC (no real Connect()), + // so an attempted send would throw. This test's subject is + // the canonical commit -> render entity chain, not the + // outbound ack (that is AP-144's own separately-filed row). + static () => true, + () => _session); + } + + public void Dispose() + { + _session.Dispose(); + try + { + Runtime.Clear(); + } + catch + { + // Failure-path assertions are made before Dispose runs. + } + } + } + + private sealed class HostMaterializer(LiveEntityRuntime runtime) + : ILiveEntityProjectionMaterializer + { + public bool TryMaterialize( + RuntimeEntityRecord expectedCanonical, + WorldSession.EntitySpawn canonicalSpawn, + LiveProjectionPurpose purpose, + ulong expectedCreateIntegrationVersion, + AcDream.App.Rendering.LiveEntityAppearanceUpdateState? appearanceUpdate = null) + { + if (canonicalSpawn.Position is not { } position + || canonicalSpawn.SetupTableId is null) + { + return false; + } + + WorldEntity? entity = runtime.MaterializeLiveEntity( + expectedCanonical, + position.LandblockId, + id => new WorldEntity + { + Id = id, + ServerGuid = canonicalSpawn.Guid, + SourceGfxObjOrSetupId = canonicalSpawn.SetupTableId.Value, + Position = new Vector3( + position.PositionX, + position.PositionY, + position.PositionZ), + Rotation = Quaternion.Identity, + MeshRefs = [], + ParentCellId = position.LandblockId, + }, + LiveEntityProjectionKind.World, + initializeProjection: null, + out LiveEntityRecord? record, + LiveEntityMaterializationResidence.AwaitRuntimePlacement); + if (entity is null || record is null) + return false; + if (runtime.IsCurrentCreateIntegration( + expectedCanonical, + expectedCreateIntegrationVersion) + && expectedCanonical.FullCellId != 0u + && !runtime.HasActiveInitialCreateResidence(expectedCanonical) + && !runtime.RebucketLiveEntity( + canonicalSpawn.Guid, + expectedCanonical.FullCellId)) + { + return false; + } + return runtime.IsCurrentRecord(record); + } + + public void ResetSessionState() + { + } + } + + private sealed class NoopResources : ILiveEntityResourceLifecycle + { + public void Register(WorldEntity entity) + { + } + + public void Unregister(WorldEntity entity) + { + } + } + + private sealed class NoopTeardown : ILiveEntityTeardownCoordinator + { + public void TearDown(LiveEntityRecord record) + { + } + + public void ForgetUnknownOwner(uint serverGuid) + { + } + } + + private sealed class NoopRelationships : ILiveEntityRelationshipProjection + { + public void OnSpawn(WorldSession.EntitySpawn spawn) + { + } + + public void OnParent(ParentEvent.Parsed update) + { + } + + public void OnCreateParentAccepted(CreateParentUpdate update) + { + } + + public ChildUnparentDisposition OnChildBecameUnparented(uint childGuid) => + ChildUnparentDisposition.Completed; + + public bool TryApplyAttachedAppearance( + LiveEntityRecord record, + ulong objDescAuthorityVersion) => false; + } + + private sealed class AcceptingReady : ILiveEntityReadyPublisher + { + public bool Publish(LiveEntityReadyCandidate candidate) => true; + } + + private sealed class KnownOrigin : ILiveEntityWorldOriginCoordinator + { + public bool IsKnown => true; + + public LiveEntityOriginInitialization TryInitialize( + WorldSession.EntitySpawn spawn) => new(true, []); + } + + private sealed class NoopNetworkSink : ILiveEntityNetworkUpdateSink + { + public void ApplySameGeneration(SameGenerationCreateObjectEvents events) + { + } + } + + private sealed class NoopTimestamps : IAcceptedLocalPhysicsTimestampPublisher + { + public void Publish(uint serverGuid, AcceptedPhysicsTimestamps timestamps) + { + } + } + + private sealed class UnusedCollisionSource : IPreparedCollisionSource + { + public PreparedAssetPresence ProbeCollision( + PakAssetType type, + uint sourceFileId) => PreparedAssetPresence.Available; + + public PreparedCollisionReadResult ReadSetupCollision( + uint sourceFileId, + CancellationToken cancellationToken = default) => + PreparedCollisionReadResult.Loaded( + new FlatSetupCollision( + System.Collections.Immutable.ImmutableArray< + FlatCollisionCylinder>.Empty, + [new FlatCollisionSphere(Vector3.Zero, 0.48f)], + height: 0f, + radius: 0f, + stepUpHeight: 0.4f, + stepDownHeight: 0.4f)); + + public PreparedCollisionReadResult + ReadGfxObjCollision( + uint sourceFileId, + CancellationToken cancellationToken = default) => + throw new NotSupportedException(); + + public PreparedCollisionReadResult + ReadCellStructureCollision( + uint sourceFileId, + CancellationToken cancellationToken = default) => + throw new NotSupportedException(); + + public PreparedCollisionReadResult ReadEnvCellTopology( + uint sourceFileId, + CancellationToken cancellationToken = default) => + throw new NotSupportedException(); + + public PreparedCollisionSourceStats CollisionStats => default; + + public void Dispose() + { + } + } + + private sealed class FixtureTransport : IWorldSessionTransport + { + public void Send(ReadOnlySpan datagram) + { + } + + public void Send( + IPEndPoint remote, + ReadOnlySpan datagram) + { + } + + public int Receive( + Span destination, + TimeSpan timeout, + out IPEndPoint? from) + { + from = null; + return -1; + } + + public ValueTask ReceiveAsync( + Memory destination, + CancellationToken cancellationToken) => + ValueTask.FromException( + new OperationCanceledException(cancellationToken)); + + public void Dispose() + { + } + } +} diff --git a/tests/AcDream.App.Tests/World/RuntimePlacementPresentationSinkTests.cs b/tests/AcDream.App.Tests/World/RuntimePlacementPresentationSinkTests.cs index f4334976..45f889eb 100644 --- a/tests/AcDream.App.Tests/World/RuntimePlacementPresentationSinkTests.cs +++ b/tests/AcDream.App.Tests/World/RuntimePlacementPresentationSinkTests.cs @@ -1,4 +1,5 @@ using System.Numerics; +using AcDream.App.Input; using AcDream.App.Streaming; using AcDream.App.World; using AcDream.App.Net; @@ -869,7 +870,8 @@ public sealed class RuntimePlacementPresentationSinkTests WorldGameState worldState, WorldEvents worldEvents, EntityEffectPoseRegistry effectPoses, - LocalPlayerShadowState localShadow) + LocalPlayerShadowState localShadow, + LocalPlayerShadowSynchronizer synchronizer) { Spatial = spatial; Runtime = runtime; @@ -878,13 +880,14 @@ public sealed class RuntimePlacementPresentationSinkTests WorldEvents = worldEvents; EffectPoses = effectPoses; LocalShadow = localShadow; + Synchronizer = synchronizer; Sink = new RuntimePlacementPresentationSink( runtime, transit, worldState, worldEvents, effectPoses, - localShadow, + synchronizer, () => Guid, ClearedSelection.Add, [ @@ -908,6 +911,7 @@ public sealed class RuntimePlacementPresentationSinkTests internal WorldEvents WorldEvents { get; } internal EntityEffectPoseRegistry EffectPoses { get; } internal LocalPlayerShadowState LocalShadow { get; } + internal LocalPlayerShadowSynchronizer Synchronizer { get; } internal List<(LiveEntityRecord Record, bool Visible)> Visibility { get; } = []; internal List ClearedSelection { get; } = []; internal int VisibilityFailuresRemaining { get; set; } @@ -915,6 +919,24 @@ public sealed class RuntimePlacementPresentationSinkTests internal static Fixture Create(bool twoLandblocks = false) { + var physics = new PhysicsEngine { DataCache = new PhysicsDataCache() }; + physics.AddLandblock( + SourceCell & 0xFFFF0000u, + new TerrainSurface(new byte[81], new float[256]), + Array.Empty(), + Array.Empty(), + worldOffsetX: 0f, + worldOffsetY: 0f); + if (twoLandblocks) + { + physics.AddLandblock( + DestinationCell & 0xFFFF0000u, + new TerrainSurface(new byte[81], new float[256]), + Array.Empty(), + Array.Empty(), + worldOffsetX: 192f, + worldOffsetY: 0f); + } var spatial = new GpuWorldState(); spatial.AddLandblock(EmptyLandblock(SourceCell | 0xFFFFu)); if (twoLandblocks) @@ -922,7 +944,18 @@ public sealed class RuntimePlacementPresentationSinkTests var resources = new RecordingResources(); LiveEntityRuntime runtime = LiveEntityRuntimeFixture.Create( spatial, - resources); + resources, + physics); + var identity = new LocalPlayerIdentityState { ServerGuid = Guid }; + var origin = new LiveWorldOriginState(); + origin.SetPlaceholder(0, 0); + var localShadow = new LocalPlayerShadowState(); + var synchronizer = new LocalPlayerShadowSynchronizer( + physics, + runtime, + identity, + origin, + localShadow); return new Fixture( spatial, runtime, @@ -930,7 +963,8 @@ public sealed class RuntimePlacementPresentationSinkTests new WorldGameState(), new WorldEvents(), new EntityEffectPoseRegistry(), - new LocalPlayerShadowState()); + localShadow, + synchronizer); } internal LiveEntityRecord Materialize(WorldSession.EntitySpawn spawn) @@ -1069,6 +1103,15 @@ public sealed class RuntimePlacementPresentationSinkTests var worldEvents = new WorldEvents(); var effectPoses = new EntityEffectPoseRegistry(); var localShadow = new LocalPlayerShadowState(); + var localShadowIdentity = new LocalPlayerIdentityState { ServerGuid = Guid }; + var localShadowOrigin = new LiveWorldOriginState(); + localShadowOrigin.SetPlaceholder(0, 0); + var synchronizer = new LocalPlayerShadowSynchronizer( + engine, + runtime, + localShadowIdentity, + localShadowOrigin, + localShadow); WorldEntity entity = record.WorldEntity!; var snapshot = new AcDream.Plugin.Abstractions.WorldEntitySnapshot( entity.Id, @@ -1087,7 +1130,7 @@ public sealed class RuntimePlacementPresentationSinkTests worldState, worldEvents, effectPoses, - localShadow, + synchronizer, () => Guid, _ => { }, [ diff --git a/tests/AcDream.App.Tests/World/RuntimePlacementShadowCompositionTests.cs b/tests/AcDream.App.Tests/World/RuntimePlacementShadowCompositionTests.cs new file mode 100644 index 00000000..550f3815 --- /dev/null +++ b/tests/AcDream.App.Tests/World/RuntimePlacementShadowCompositionTests.cs @@ -0,0 +1,683 @@ +using System.Numerics; +using AcDream.App.Input; +using AcDream.App.Streaming; +using AcDream.App.World; +using AcDream.App.Net; +using AcDream.App.Physics; +using AcDream.App.Rendering.Vfx; +using AcDream.Core.Net; +using AcDream.Core.Net.Messages; +using AcDream.Core.Physics; +using AcDream.Core.Plugins; +using AcDream.Core.World; +using AcDream.Runtime; +using AcDream.Runtime.Entities; +using AcDream.Runtime.Physics; +using AcDream.Runtime.World; +using DatReaderWriter.DBObjs; + +namespace AcDream.App.Tests.World; + +/// +/// C5a §5.1 (2026-08-05) — the #318 composition test, landed WITH its fix +/// (coordinator override of the contract's original "red branch, fix lands +/// separately" plan — every commit in this campaign stays green). Drives a +/// REAL portal arrival through +/// AND the REAL / +/// pair, closing the gap every prior sink test left open: those tests never +/// wired a real at all, so they could only ever +/// assert on 's dedup cache — which +/// AP-145's bug pre-seeded regardless of whether the real collision shadow +/// moved. +/// +/// +/// AP-145, RETIRED by this commit: +/// RuntimePlacementPresentationSink.TryPublishPlace used to call +/// _localPlayerShadow.Set(...) directly — a pure cache write — instead +/// of routing through , +/// the ONLY code path that calls ShadowPositionSynchronizer.Sync → +/// ShadowObjectRegistry.UpdatePosition, the real collision-shadow +/// publish. Two things went wrong together: (1) the portal jump's real +/// shadow entry never moved to the destination, and (2) the direct cache +/// write PRE-SEEDED SyncPose's own dedup check, so even a later +/// per-tick SyncPose call would see "nothing changed" and skip the +/// publish it would otherwise have performed. A cache-only assertion — the +/// shape every prior sink test used — was satisfied by the bug: the cache +/// said the right thing, only the real registry didn't. +/// +/// +/// +/// The fix: TryPublishPlace now calls +/// _localPlayerShadowSync.SyncPose(entity, entity.Position, entity.Rotation, +/// record.FullCellId, force: true) — the SAME publisher ordinary per-tick +/// movement uses, constructed once and shared (composition root: +/// LivePresentationComposition.cs now builds the ONE +/// instance BEFORE the sink and +/// threads it through LivePresentationResult to +/// SessionPlayerComposition.cs, which no longer constructs its own). +/// force: true because this is the authoritative placement commit, +/// not an ordinary per-tick refresh — it must never be skipped by the dedup +/// path. (which +/// UpdatePosition calls internally) deregisters every prior cell row +/// for the entity before adding the new ones, so the SOURCE cell's row is +/// replaced, not duplicated — verified explicitly below. +/// +/// +/// +/// Architecture review A2: the exact mirror asymmetry lived six lines +/// below the Place fix, on TryPublishWithdrawal — a bare +/// _localPlayerShadow.Clear() with no ShadowObjects.Suspend, +/// leaving a LIVE phantom collision row at the park's source cell for the +/// whole park window (the #184 shape). Fixed in this same commit, in the +/// same one-call shape: TryPublishWithdrawal now calls +/// _localPlayerShadowSync.Suspend(entity). +/// +/// +/// +/// Architecture review A1: the original route-7-P4 test +/// () +/// never registered the non-player entity, so +/// ShadowObjectRegistry.UpdatePosition's own not-registered early +/// return made every assertion pass whether or not the player-only gate +/// existed — it was vacuous on both branches. Corrected to establish a REAL +/// baseline registration first, so removing the gate would actually move the +/// row and actually pollute the player's cache. +/// +/// +/// +/// Sabotage-verified (manual, all four facts, both directions): with +/// each fix/gate reverted in turn, the corresponding fact fails at exactly +/// its discriminating assertion; with the fix applied, all four are green. +/// +/// +public sealed class RuntimePlacementShadowCompositionTests +{ + private const uint SourceCell = 0x01010001u; + private const uint DestinationCell = 0x01020001u; + private const uint Guid = 0x7000A101u; + private static readonly Vector3 SourcePosition = new(10f, 10f, 5f); + // Landblock-local (10,10) relative to the destination landblock's + // worldOffsetX=192 — falls in the same grid cell (0x0001, cx=0,cy=0, + // TerrainSurface.CellSize=24) as SourcePosition does in ITS landblock, + // so ShadowObjectRegistry's flood actually lands under DestinationCell. + private static readonly Vector3 DestinationPosition = new(202f, 10f, 5f); + private static readonly Vector3 WirePoseDoubleWrite = new(-900f, -900f, -900f); + private static readonly Quaternion DestinationOrientation = + Quaternion.CreateFromAxisAngle(Vector3.UnitZ, 0.75f); + + /// + /// The discriminating composition. Establishes a REAL baseline shadow + /// registration at the source pose (proving the synchronizer mechanism + /// works in general), drives a real portal arrival through the sink + /// (T8: with a tolerated "wire pose" double-write already sitting on the + /// entity, exactly like LiveEntityNetworkUpdateController's early + /// generic write — the committed suffix must overwrite it), then checks + /// the dedup cache (informational negative control — would pass even + /// under the AP-145 bug, kept to make the sabotage argument + /// self-contained), the REAL + /// registry at the destination (the load-bearing assertion — this is + /// what the fix makes true), AND that the source cell's row is gone, not + /// duplicated. + /// + [Fact] + public void Place_PublishesRealPhysicsShadowAtDestination_NotOnlyTheDedupCache() + { + Fixture fixture = Fixture.Create(); + LiveEntityRecord record = fixture.Materialize(Spawn(Guid, 1, SourceCell)); + WorldEntity entity = Assert.IsType(record.WorldEntity); + + // Baseline: establish the player's REAL collision shadow at the + // source pose exactly like ordinary world entry does, and prove the + // synchronizer mechanism is not itself broken (sanity, not the gate). + fixture.Physics.ShadowObjects.Register( + entity.Id, + gfxObjId: entity.SourceGfxObjOrSetupId, + worldPos: SourcePosition, + rotation: Quaternion.Identity, + radius: 0.48f, + worldOffsetX: 0f, + worldOffsetY: 0f, + landblockId: SourceCell & 0xFFFF0000u, + collisionType: ShadowCollisionType.Sphere, + cylHeight: 1.835f, + seedCellId: SourceCell); + fixture.Synchronizer.Sync(entity, SourceCell, force: true); + Assert.Contains( + fixture.Physics.ShadowObjects.GetObjectsInCell(SourceCell), + e => e.EntityId == entity.Id); + + // T8 (route 3 §8 item 9): model the tolerated generic render-pose + // write (LiveEntityNetworkUpdateController.cs:2284-2314) landing on + // the entity BEFORE the committed placement suffix runs. The + // intermediate "wire pose" must never leak past the Place edge. + entity.Position = WirePoseDoubleWrite; + entity.Rotation = Quaternion.Identity; + + // A real portal arrival through the canonical placement authority. + RuntimePortalPlacementAuthority portal = fixture.BeginPortal( + DestinationCell, + teleportSequence: 1); + record.FullCellId = DestinationCell; + record.CanonicalLandblockId = (DestinationCell & 0xFFFF0000u) | 0xFFFFu; + record.Canonical.AdvancePlacementCommit(); + RuntimePlacementProjectionSnapshot place = Placement( + fixture, + record, + portal, + DestinationPosition, + DestinationOrientation); + + Assert.True(fixture.Sink.TryApply(in place)); + + // T8 END STATE: the committed suffix's resolved pose wins — the + // intermediate wire-pose write never leaks past the Place edge. + Assert.Equal(DestinationPosition, entity.Position); + Assert.Equal(DestinationOrientation, entity.Rotation); + Assert.Equal(DestinationCell, entity.ParentCellId); + + // NEGATIVE CONTROL — deliberately NOT the gate. This is the exact + // assertion shape every prior sink test uses + // (RuntimePlacementPresentationSinkTests.Place_Reframes...); it + // would pass EVEN under the AP-145 bug, since the cache is what the + // buggy direct write updated. Kept here to make the sabotage + // argument self-contained: if this assertion were the only + // coverage, the bug would have been invisible. + Assert.Equal( + new LocalPlayerShadowState.Snapshot( + DestinationPosition, + DestinationOrientation, + DestinationCell), + fixture.LocalShadow.Current); + + // THE DISCRIMINATING ASSERTION — this is what AP-145's bug broke and + // the fix restores: the real collision shadow used by every OTHER + // entity's collision sweep now actually followed the portal jump. + Assert.Contains( + fixture.Physics.ShadowObjects.GetObjectsInCell(DestinationCell), + e => e.EntityId == entity.Id); + + // No double-publish, no stale row left behind: Register/UpdatePosition + // deregisters every prior cell row before adding the new ones, so the + // SOURCE cell must carry zero rows for this entity now. + Assert.DoesNotContain( + fixture.Physics.ShadowObjects.GetObjectsInCell(SourceCell), + e => e.EntityId == entity.Id); + } + + /// + /// The second half of AP-145's mechanism, and the second half of the + /// fix's proof: the OLD direct cache write did not merely skip the + /// publish once — it pre-seeded + /// 's own dedup + /// check, so even a SUBSEQUENT ordinary per-tick Sync call (the + /// thing that would normally self-heal a one-frame miss) saw "nothing + /// changed" and skipped the publish too. With the fix, TryPublishPlace + /// itself now runs the real publish through the SAME synchronizer, so + /// the subsequent ordinary tick's dedup skip is no longer a bug — it is + /// CORRECTLY a no-op, because the real registry is already right. + /// + [Fact] + public void Place_ThenOrdinaryTick_DoesNotNeedToSelfHeal_RealShadowAlreadyRight() + { + Fixture fixture = Fixture.Create(); + LiveEntityRecord record = fixture.Materialize(Spawn(Guid, 1, SourceCell)); + WorldEntity entity = Assert.IsType(record.WorldEntity); + fixture.Physics.ShadowObjects.Register( + entity.Id, + gfxObjId: entity.SourceGfxObjOrSetupId, + worldPos: SourcePosition, + rotation: Quaternion.Identity, + radius: 0.48f, + worldOffsetX: 0f, + worldOffsetY: 0f, + landblockId: SourceCell & 0xFFFF0000u, + collisionType: ShadowCollisionType.Sphere, + cylHeight: 1.835f, + seedCellId: SourceCell); + fixture.Synchronizer.Sync(entity, SourceCell, force: true); + + RuntimePortalPlacementAuthority portal = fixture.BeginPortal( + DestinationCell, + teleportSequence: 1); + record.FullCellId = DestinationCell; + record.CanonicalLandblockId = (DestinationCell & 0xFFFF0000u) | 0xFFFFu; + record.Canonical.AdvancePlacementCommit(); + RuntimePlacementProjectionSnapshot place = Placement( + fixture, + record, + portal, + DestinationPosition, + DestinationOrientation); + Assert.True(fixture.Sink.TryApply(in place)); + + // Simulate the very next ordinary frame's shadow-sync tick — exactly + // what production runs every frame for the local player. Not forced: + // this is the real per-tick call shape, dedup and all. It is + // EXPECTED to be a no-op now: TryPublishPlace's own SyncPose call + // already did the real work, so the cache already matches. + fixture.Synchronizer.Sync(entity, DestinationCell); + + Assert.Contains( + fixture.Physics.ShadowObjects.GetObjectsInCell(DestinationCell), + e => e.EntityId == entity.Id); + Assert.DoesNotContain( + fixture.Physics.ShadowObjects.GetObjectsInCell(SourceCell), + e => e.EntityId == entity.Id); + // Exactly one row for this entity anywhere — the ordinary tick's + // dedup no-op did not create a second one. + Assert.Single( + fixture.Physics.ShadowObjects.AllEntriesForDebug(), + e => e.EntityId == entity.Id); + } + + /// + /// Route 7 P4 still binds: a committed CHILD (or any non-local-player + /// entity) must not gain a broadphase row, and must not have its OWN + /// pose written into the PLAYER's dedup cache. The fix lives entirely + /// inside TryPublishPlace's pre-existing + /// record.ServerGuid == _localPlayerGuid() gate — unchanged by + /// this fix, only what runs INSIDE it changed. + /// + /// + /// C5a architecture review A1 (2026-08-05): the FIRST version of this + /// test registered nothing for the child, so + /// ShadowObjectRegistry.UpdatePosition's own + /// not-registered early return (ShadowObjectRegistry.cs:696) made + /// every assertion pass whether or not the gate existed — removing the + /// gate was unobservable because the sabotaged code path was ALSO a + /// no-op. This version establishes a REAL baseline registration for the + /// child first (mirroring fact 1's own baseline), so a gate-removed + /// sabotage would actually move the child's row and would actually + /// pollute the player's cache — both of which the assertions below now + /// check directly, not by absence of registration. + /// + /// + [Fact] + public void Place_ForNonLocalPlayerEntity_NeverTouchesShadowObjects() + { + const uint ChildGuid = 0x7000A102u; + Fixture fixture = Fixture.Create(); + LiveEntityRecord record = fixture.Materialize(Spawn(ChildGuid, 1, SourceCell)); + WorldEntity entity = Assert.IsType(record.WorldEntity); + Assert.NotEqual(Guid, ChildGuid); + + // REAL baseline: the child has its own genuine collision shadow at + // the source pose, exactly like fact 1 establishes for the player. + // A sabotaged (gate-removed) TryPublishPlace WOULD move this row to + // the destination and WOULD write the child's pose into the + // player's LocalShadow cache — both are asserted against below. + fixture.Physics.ShadowObjects.Register( + entity.Id, + gfxObjId: entity.SourceGfxObjOrSetupId, + worldPos: SourcePosition, + rotation: Quaternion.Identity, + radius: 0.48f, + worldOffsetX: 0f, + worldOffsetY: 0f, + landblockId: SourceCell & 0xFFFF0000u, + collisionType: ShadowCollisionType.Sphere, + cylHeight: 1.835f, + seedCellId: SourceCell); + Assert.Contains( + fixture.Physics.ShadowObjects.GetObjectsInCell(SourceCell), + e => e.EntityId == entity.Id); + + RuntimePortalPlacementAuthority portal = fixture.BeginPortal( + DestinationCell, + teleportSequence: 1); + record.FullCellId = DestinationCell; + record.CanonicalLandblockId = (DestinationCell & 0xFFFF0000u) | 0xFFFFu; + record.Canonical.AdvancePlacementCommit(); + RuntimePlacementProjectionSnapshot place = Placement( + fixture, + record, + portal, + DestinationPosition, + DestinationOrientation); + + Assert.True(fixture.Sink.TryApply(in place)); + + // The render entity DID move (Place still works for a non-player + // entity) — only the shadow-publish branch is player-gated. + Assert.Equal(DestinationPosition, entity.Position); + + // THE DISCRIMINATING ASSERTIONS. Under a sabotaged (gate-removed) + // TryPublishPlace, SyncPose would run for the child: the child's own + // record IS its own current visible projection, so + // IsCurrentVisibleProjection would be true and the real publish + // would execute — moving the row to DestinationCell and vacating + // SourceCell (exactly what fact 1 asserts is CORRECT for the + // player). Here it must NOT happen — this is what tells the + // sabotage apart from the fix. + Assert.Contains( + fixture.Physics.ShadowObjects.GetObjectsInCell(SourceCell), + e => e.EntityId == entity.Id); + Assert.DoesNotContain( + fixture.Physics.ShadowObjects.GetObjectsInCell(DestinationCell), + e => e.EntityId == entity.Id); + Assert.Equal(1, fixture.Physics.ShadowObjects.TotalRegistered); + + // The player's OWN dedup cache must stay untouched by a non-player + // Place — a sabotaged TryPublishPlace would call + // _localPlayerShadowSync.SyncPose using the CHILD's guid check but + // the PLAYER's cache field (the gate exists to prevent exactly a + // non-player entity's pose from being written where the player's + // pose belongs). + Assert.Null(fixture.LocalShadow.Current); + } + + /// + /// Architecture review A2 (2026-08-05): the exact mirror of AP-145's + /// Place-side bug lived six lines below it, on the Withdraw path + /// — TryPublishWithdrawal cleared only the dedup cache, leaving a + /// LIVE phantom row in at the + /// park's source cell for the whole park window (the #184 shape: every + /// other entity's collision sweep in that cell would collide with a + /// player who is, per every other acdream predicate, gone). Fixed in the + /// same commit as the Place half, via the SAME one-call shape: + /// TryPublishWithdrawal now calls + /// _localPlayerShadowSync.Suspend(entity), which does the real + /// registry suspend AND the cache clear together. + /// + [Fact] + public void Withdraw_SuspendsRealPhysicsShadow_NotOnlyTheDedupCache() + { + Fixture fixture = Fixture.Create(); + LiveEntityRecord record = fixture.Materialize(Spawn(Guid, 1, SourceCell)); + WorldEntity entity = Assert.IsType(record.WorldEntity); + + // Baseline: a real collision shadow at the source pose, exactly like + // ordinary world entry / fact 1 establishes. + fixture.Physics.ShadowObjects.Register( + entity.Id, + gfxObjId: entity.SourceGfxObjOrSetupId, + worldPos: SourcePosition, + rotation: Quaternion.Identity, + radius: 0.48f, + worldOffsetX: 0f, + worldOffsetY: 0f, + landblockId: SourceCell & 0xFFFF0000u, + collisionType: ShadowCollisionType.Sphere, + cylHeight: 1.835f, + seedCellId: SourceCell); + fixture.Synchronizer.Sync(entity, SourceCell, force: true); + Assert.Contains( + fixture.Physics.ShadowObjects.GetObjectsInCell(SourceCell), + e => e.EntityId == entity.Id); + Assert.NotNull(fixture.LocalShadow.Current); + + RuntimePlacementProjectionSnapshot withdraw = Placement( + fixture, + record, + portal: default, + entity.Position, + entity.Rotation, + RuntimePlacementProjectionKind.Withdraw); + + Assert.True(fixture.Sink.TryApply(in withdraw)); + + // THE DISCRIMINATING ASSERTIONS. Under the pre-fix bug, the cache + // clears (this passes either way) but the real registry keeps the + // SOURCE-cell row (a live phantom for the park's duration) — that is + // what the sabotage below must be able to catch. + Assert.Null(fixture.LocalShadow.Current); + Assert.DoesNotContain( + fixture.Physics.ShadowObjects.GetObjectsInCell(SourceCell), + e => e.EntityId == entity.Id); + // Suspend removes the entity from every cell bucket (TotalRegistered + // — what any collision sweep can find) but is deliberately NOT + // logical teardown (ShadowObjectRegistry.Suspend's own xmldoc: "the + // registry counterpart of retail CPhysicsObj::remove_shadows_from_cells + // during temporary leave-world/pending-cell residence"), so the + // RETAINED registration survives for TryPublishPlace's later + // force:true SyncPose (WithdrawalRestored, or a fresh Place) to + // re-publish from. + Assert.Equal(0, fixture.Physics.ShadowObjects.TotalRegistered); + Assert.Equal(1, fixture.Physics.ShadowObjects.RetainedRegistrationCount); + } + + private static RuntimePlacementProjectionSnapshot Placement( + Fixture fixture, + LiveEntityRecord record, + RuntimePortalPlacementAuthority portal, + Vector3 position, + Quaternion orientation, + RuntimePlacementProjectionKind kind = RuntimePlacementProjectionKind.Place) + { + RuntimeEntityRecord canonical = record.Canonical; + var token = new RuntimePlacementProjectionToken( + Sequence: 1, + Revision: 1, + Entity: record.ProjectionKey!.Value, + PositionAuthorityVersion: canonical.PositionAuthorityVersion, + SpatialAuthorityVersion: canonical.SpatialAuthorityVersion, + PlacementCommitVersion: canonical.PlacementCommitVersion, + SessionLifetimeVersion: fixture.Runtime.SessionLifetimeVersion, + ExactCellId: canonical.FullCellId, + CollisionGeneration: 1, + Portal: portal); + return new RuntimePlacementProjectionSnapshot( + token, + kind, + position, + orientation, + CellLocalPosition: position, + InContact: false, + OnWalkable: false); + } + + private static WorldSession.EntitySpawn Spawn( + uint guid, + ushort instance, + uint cell) + { + var position = new CreateObject.ServerPosition( + cell, 10f, 10f, 5f, 1f, 0f, 0f, 0f); + var timestamps = new PhysicsTimestamps( + Position: 1, + Movement: 1, + State: 1, + Vector: 1, + Teleport: 0, + ServerControlledMove: 1, + ForcePosition: 0, + ObjDesc: 1, + Instance: instance); + var physics = new PhysicsSpawnData( + RawState: (uint)PhysicsStateFlags.ReportCollisions, + Position: position, + Movement: null, + AnimationFrame: null, + SetupTableId: 0x02000001u, + MotionTableId: 0x09000001u, + SoundTableId: null, + PhysicsScriptTableId: null, + Parent: null, + Children: null, + Scale: null, + Friction: null, + Elasticity: null, + Translucency: null, + Velocity: null, + Acceleration: null, + AngularVelocity: null, + DefaultScriptType: null, + DefaultScriptIntensity: null, + Timestamps: timestamps); + return new WorldSession.EntitySpawn( + guid, + position, + 0x02000001u, + Array.Empty(), + Array.Empty(), + Array.Empty(), + null, + null, + "fixture", + null, + null, + 0x09000001u, + PhysicsState: (uint)PhysicsStateFlags.ReportCollisions, + InstanceSequence: instance, + MovementSequence: 1, + ServerControlSequence: 1, + PositionSequence: 1, + Physics: physics); + } + + private sealed class Fixture + { + private Fixture( + PhysicsEngine physics, + GpuWorldState spatial, + LiveEntityRuntime runtime, + RuntimeWorldTransitState transit, + WorldGameState worldState, + WorldEvents worldEvents, + EntityEffectPoseRegistry effectPoses, + LocalPlayerShadowState localShadow, + LocalPlayerShadowSynchronizer synchronizer) + { + Physics = physics; + Spatial = spatial; + Runtime = runtime; + Transit = transit; + WorldState = worldState; + WorldEvents = worldEvents; + EffectPoses = effectPoses; + LocalShadow = localShadow; + Synchronizer = synchronizer; + Sink = new RuntimePlacementPresentationSink( + runtime, + transit, + worldState, + worldEvents, + effectPoses, + synchronizer, + () => Guid, + _ => { }, + [(_, _) => { }]); + } + + internal PhysicsEngine Physics { get; } + internal GpuWorldState Spatial { get; } + internal LiveEntityRuntime Runtime { get; } + internal RuntimeWorldTransitState Transit { get; } + internal WorldGameState WorldState { get; } + internal WorldEvents WorldEvents { get; } + internal EntityEffectPoseRegistry EffectPoses { get; } + internal LocalPlayerShadowState LocalShadow { get; } + internal LocalPlayerShadowSynchronizer Synchronizer { get; } + internal RuntimePlacementPresentationSink Sink { get; } + + internal static Fixture Create() + { + var physics = new PhysicsEngine { DataCache = new PhysicsDataCache() }; + physics.AddLandblock( + SourceCell & 0xFFFF0000u, + new TerrainSurface(new byte[81], new float[256]), + Array.Empty(), + Array.Empty(), + worldOffsetX: 0f, + worldOffsetY: 0f); + physics.AddLandblock( + DestinationCell & 0xFFFF0000u, + new TerrainSurface(new byte[81], new float[256]), + Array.Empty(), + Array.Empty(), + worldOffsetX: 192f, + worldOffsetY: 0f); + + var spatial = new GpuWorldState(); + spatial.AddLandblock(EmptyLandblock(SourceCell | 0xFFFFu)); + spatial.AddLandblock(EmptyLandblock(DestinationCell | 0xFFFFu)); + var resources = new RecordingResources(); + LiveEntityRuntime runtime = LiveEntityRuntimeFixture.Create( + spatial, + resources, + physics); + + var identity = new LocalPlayerIdentityState { ServerGuid = Guid }; + var origin = new LiveWorldOriginState(); + origin.SetPlaceholder(0, 0); + var localShadow = new LocalPlayerShadowState(); + var synchronizer = new LocalPlayerShadowSynchronizer( + physics, + runtime, + identity, + origin, + localShadow); + + return new Fixture( + physics, + spatial, + runtime, + new RuntimeWorldTransitState(), + new WorldGameState(), + new WorldEvents(), + new EntityEffectPoseRegistry(), + localShadow, + synchronizer); + } + + internal LiveEntityRecord Materialize(WorldSession.EntitySpawn spawn) + { + LiveEntityRecord record = Runtime.RegisterAndMaterializeProjection(spawn); + Assert.False(Runtime.HasActiveInitialCreateResidence( + record.Canonical)); + Assert.True(record.ResourcesRegistered); + WorldEntity entity = record.WorldEntity!; + var snapshot = new AcDream.Plugin.Abstractions.WorldEntitySnapshot( + entity.Id, + entity.SourceGfxObjOrSetupId, + entity.Position, + entity.Rotation); + WorldState.Add(snapshot); + WorldEvents.UpsertCurrent(snapshot); + EffectPoses.PublishMeshRefs(entity); + return record; + } + + internal RuntimePortalPlacementAuthority BeginPortal( + uint cell, + ushort teleportSequence) + { + Assert.True(Transit.TryQueueTeleportStart(teleportSequence)); + Assert.True(Transit.ActivateQueuedTeleport()); + Assert.True(Transit.OfferTeleportDestination( + new RuntimeTeleportDestination( + Guid, + InstanceSequence: 1, + PositionSequence: 1, + TeleportSequence: teleportSequence, + ForcePositionSequence: 1, + new Position( + cell, + new Vector3(1f, 2f, 3f), + Quaternion.Identity)), + teleportTimestampAdvanced: true)); + Assert.True(Transit.TryBeginPortalReveal( + teleportSequence, + cell, + out long generation)); + Assert.True(Transit.TryRegisterHostProjection( + generation, + cell, + out RuntimeWorldHostProjectionToken host)); + return new RuntimePortalPlacementAuthority( + true, + generation, + teleportSequence, + host); + } + + private static LoadedLandblock EmptyLandblock(uint canonicalId) => + new(canonicalId, new LandBlock(), Array.Empty()); + } + + private sealed class RecordingResources : ILiveEntityResourceLifecycle + { + public void Register(WorldEntity entity) { } + public void Unregister(WorldEntity entity) { } + } +}