fix(physics): C4 route 3 — portal placement authority (local player)
Removes a duplicate placement authority for local-player portal arrival. Portalling worked before this change and works after it — this is not a bug fix, EXCEPT that it found and fixed one dead-code production bug. THE PRODUCTION BUG: TryExecuteCanonicalPortalPlacement re-read the accepted destination at Place time, but TryBeginPortalReveal already consumes that slot at Aim time — so the arm was 100% dead code and every real portal Place refused with host-token-unavailable. Found only because we refused to accept 7 skipped tests instead of chasing the count to zero. RETAIL IS THE GENERIC PATH FOR THE THIRD ROUTE RUNNING: SmartBox::TeleportPlayer @0x00453910 = SetPositionSimple(dest, 1) with flags 0x1012, followed by PlayerPositionUpdated. BOTH INVERSIONS, WITH THEIR ANCHORS: unlike route 2, the leash IS armed here (ConstrainTo @0x0045418A) and velocity is zeroed (set_velocity @0x004541B4); unlike route 4b-3, the local teleport_hook runs AFTER placement (@0x004538AE). THE THREE-ROUND DEFECT CHAIN, HONESTLY: - Round 1 released the player at the pre-teleport position while the anim stream marched on — the contract wrongly assumed Place re-fires (process rule 1's third occurrence this campaign). - Round 2's fix inferred commit from a global PendingCount, which three non-committing paths also clear — making the SAME bug complete cleanly and silently. Strictly worse than round 1: round 1 at least tripped portal-complete-before-materialized. - Round 3 latches the commit where it actually happens (ReconcileAndAcknowledgePortal), keyed on reveal generation and teleport sequence, via TryConsumePortalCommit. Two of the three required regression tests landed and are sabotage-verified on both hosts (ParkedPlace_ForgottenByOrdinaryMergeDoesNotLatchAsCommitted / HeadlessPortalPrepareDestinationForgottenByOrdinaryMergeDoesNotLatchAsCommitted). The third (force-arm-takes-the-slot) was judged unnecessary on review: with the inference gone, PendingCount is only a "don't ask yet" guard at both gates, so a force operation occupying or vacating the slot no longer changes an input the commit decision reads — the case collapses into what the landed test already discriminates. THE B2/P3 RESOLUTION: both round-2 reviews were right about different branches of the same synchronous call. RuntimePlacementProjectionSubscription .OnPlacement acknowledges the FIFO head only when TryApply returns true; a Place whose portal authority went stale (transit ended/superseded while parked) used to return false, wedging every later entity's placement receipt behind it forever. Both sinks (RuntimePlacementPresentationSink, HeadlessRuntimePlacementProjectionSink) now acknowledge-and-ignore a stale-authority Place instead of refusing it. The regression test (RuntimePlacementPresentationSinkTests .PortalPlace_StaleTransitHostOrSequenceIsAcknowledgedAndIgnored) had been asserting the old, wrong `false` behaviour; it now asserts and sabotage-verifies the fix. Also lands: AP-144 (register discipline — the portal movement-event send reuses the stricter UsePositionFromServer gate where retail's SendMovementEvent is the looser autonomy_level != 0 test, diverging only at level 1, currently unreachable), AP-145 + issue #318 (the local-player collision-shadow presentation write bypasses its own publisher's ShadowObjects write via a direct cache .Set(), self-healing only once dedup diverges — filed, not fixed, pending a composition test), AD-42 deleted (its last citation retired by the canonical portal arm), AD-2 updated (the wait-cue's trigger predicate now covers a second cause), and two documentation corrections: the enter_world misattribution (both call sites are in SmartBox::HandleCreateObject, only one in the player branch — portal arrival is TeleportPlayer, not enter_world) and the stale "local player never reaches this path" comment on the generic-remote-render-pose write. Suite: 11,090 passed / 4 skipped / 0 failed. No new skips, nothing weakened. STILL OWED: the connected two-client gate, with ACDREAM_PROBE_LOCAL_TELEPORT=1, scored only if [local-tp] lines actually appear in the capture — and explicitly NOT scored as covering issue #318 (no composition test yet asserts PhysicsEngine.ShadowObjects directly). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
cd3129e9d6
commit
e0f96a55bf
24 changed files with 5261 additions and 243 deletions
|
|
@ -24,6 +24,65 @@ What does NOT go here:
|
|||
- Every session: scan OPEN issues at start; promote/close anything we touched during the session before ending.
|
||||
- Promoting to a Phase: mark as `DONE (promoted to Phase X)` + commit SHA where the Phase entry landed.
|
||||
|
||||
## #318 — C4 route 3 §8 items 8/9/10 residual: no end-to-end composition test, no local-player shadow assertion, no T8 ordering
|
||||
|
||||
**Status:** OPEN
|
||||
**Severity:** LOW (does not block round-3 acceptance per both reviewers; carried
|
||||
into C5)
|
||||
**Filed:** 2026-08-05, C4 route 3 round-3 review (retail B5/A5, architecture
|
||||
B5), carried per both reviewers' explicit conditions
|
||||
**Component:** Runtime / portal placement / local-player presentation
|
||||
|
||||
**Description:** The retail review's round-1 §3.4 premise — that
|
||||
`TryApplyRuntimePlacementPlace` does not write pose/rotation/`ParentCellId` or
|
||||
rebucket — was WRONG; round 3 verified it DOES. That closed the original
|
||||
blocking concern, but three narrower gaps remain and both reviewers agreed
|
||||
they must be tracked rather than silently dropped:
|
||||
|
||||
1. No end-to-end composition test exercises the full portal-arrival →
|
||||
canonical commit → presentation-suffix → `PhysicsEngine.ShadowObjects`
|
||||
chain for the LOCAL player specifically (existing tests cover pieces —
|
||||
the canonical commit, the presentation sink's `TryApply`, the drive
|
||||
controller — but not the full composed path with a real
|
||||
`RuntimePlacementPresentationSink` wired to a real `PhysicsEngine`).
|
||||
2. No test asserts the local-player collision SHADOW lands at the
|
||||
destination. The discriminating assertion for that future test:
|
||||
`PhysicsEngine.ShadowObjects` must hold a row at the destination cell/
|
||||
position, not just `LocalPlayerShadowState`'s internal dedup cache — see
|
||||
the register row (AP-131 amendment, filed alongside this issue) for the
|
||||
asymmetry this exposes: `LocalPlayerShadowState.Set` updates the dedup
|
||||
cache without publishing to `ShadowObjects`, self-healing only on the
|
||||
local player's first subsequent movement tick.
|
||||
3. No test proves T8's ordering — that the canonical commit's writes
|
||||
(pose/rotation/`ParentCellId`/rebucket) precede the presentation suffix's
|
||||
OWN redundant writes to the same fields, rather than racing or reversing.
|
||||
|
||||
**Root cause / status:** Not a defect — a coverage gap. The underlying
|
||||
mechanism (`RuntimePlacementPresentationSink.TryApply` →
|
||||
`LiveEntityRuntime.TryApplyRuntimePlacementProjection` →
|
||||
`TryPublishPlace` → `LocalPlayerShadowState.Set`) is correct by code reading
|
||||
and by the individual unit tests that DO exist; what's missing is the
|
||||
COMPOSED, end-to-end proof plus the specific shadow-registry assertion.
|
||||
|
||||
**Files:** `src/AcDream.App/World/RuntimePlacementPresentationSink.cs`
|
||||
(`TryPublishPlace`, `LocalPlayerShadowState.Set` call); `src/AcDream.App/Physics/LocalPlayerShadowState.cs`;
|
||||
`src/AcDream.Core/Physics/PhysicsEngine.cs` (`ShadowObjects`);
|
||||
`src/AcDream.Runtime/Session/RuntimeAcceptedPositionDriveController.cs`
|
||||
(`ReconcileAndAcknowledgePortal`, the T8 probe log).
|
||||
|
||||
**Research:** `docs/research/2026-08-04-c4-route-3-contract.md` §3.4;
|
||||
`docs/research/2026-08-04-c4-route-3-retail-review-round2.md` §D (B5/A5);
|
||||
`docs/research/2026-08-04-c4-route-3-architecture-review-round2.md` B5.
|
||||
|
||||
**Acceptance:** A composition test drives a real portal arrival through the
|
||||
canonical drive controller and the real `RuntimePlacementPresentationSink`
|
||||
against a real `PhysicsEngine`, then asserts `PhysicsEngine.ShadowObjects`
|
||||
holds the local player at the destination position/cell (not merely
|
||||
`LocalPlayerShadowState`'s cache) and that the write ordering matches T8 (a
|
||||
probe or log-order assertion). Do not score the existing connected/manual
|
||||
gate as covering this — it exercises the live path but does not assert the
|
||||
shadow registry specifically.
|
||||
|
||||
## #317 — `TryCommitAuthoritativeVelocity`'s call site has no established retail basis
|
||||
|
||||
**Status:** OPEN
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue