7f1c1f5aa6
Flips the SetPositionSimple classification (contact, PlayerDistance >= 96 m) for remotes onto 4b-1's drive controller and deletes both legacy far blocks, both duplicated 96f/4f constant pairs, and both `?? Vector3.Zero` fabrications. The 4 m constant now exists exactly once. Teleport and cell-less stay legacy for 4b-3. Retail: MoveOrTeleport @0x00516330's far branch runs StopInterpolating @0x005163CB before SetPositionSimple @0x005163D9 and returns 1 @0x005163E8 regardless — the SetPositionError is discarded — so HandleReceivedPosition arms ConstrainTo @0x00454272 post-move on commit AND on failure. The x87 parity decode at @0x00516393-@0x0051639E puts exactly 96.0 on the far branch. SetPositionSimple @0x005162B0 builds flags 0x1012 at @0x005162C4. Non-commit outcomes still advance the body, because retail's SetPositionInternal @0x00515BD0 commits the destination via store_position @0x00515CE2 when no cell resolves. The partition is by STAGE, not heuristic, enforced by an exhaustive switch: Refused/Contention/NotApplicable/RejectedPreparation store (the placement never executed); Committed/Deferred/RejectedByPlacement do not (the engine ran and refused, matching retail's non-storing returns @0x00515CB2 and @0x00515CD5). Without this a refused far snap froze the remote with an emptied queue. Also fixes a shipped defect this route made live: ParkDeferred's quiescence parks withdrew the entity (InWorld=false, clock suspended, residency removed) and were never restorable, while Forget(restoreCancelledPark: true) runs for every accepted Position on every entity. The restorable decision now lives inside ParkDeferred AFTER SnapToCell, reading body.CellPosition.ObjCellId — the value RestoreParkWithdrawal actually restores at — against every live quiescence rather than one minimum-OperationId token. The three pre-snap fields are hoisted into locals because SnapToCell ends with InWorld = true. ParkCollisionResidents passes restorableOnCancel: false explicitly; the plain unplaceable park is provably unchanged. RestoreParkWithdrawal re-tests the prefix at restore time so a retained route-2 park cannot re-admit into a prefix that began quiescing during the park. CanAttemptDestination is retained as an OPTIMISATION only, with the two Core predicates it cannot reproduce written down at the pre-flight, plus the two properties that depend on it staying there. Four fix rounds and eight Opus reviews. The slice was fully green at 10,990, 10,997 and 11,004 while containing real defects — a frozen remote pinned as correct by its own test, a fallback that over-wrote on the exact retail paths that decline to store, and a park guard incomplete on two independent axes. Register: AP-137 (leftover classifications take AP-87's catch-up; states the cell-less enqueue-vs-place delta deferred to 4b-3, that RejectedData is applied anyway, and the headless divergence), AP-138 (the refusable far placement), AP-136 narrowed to match the relocation. #309's acceptance steps rewritten — step 5 previously asserted a recovery the code does not perform — and gated on a new ACDREAM_PROBE_PARK=1 signal so the check cannot pass while broken. Suite 11,009 passed / 4 skipped / 0 failed against a measured 10,968 baseline. The 10,973 figure recorded earlier was wrong and is corrected here. Connected gate outstanding: the two-client far-snap walk and #309. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
174 lines
10 KiB
Markdown
174 lines
10 KiB
Markdown
# C4 route 4b-2 — round 4 correction (final), 2026-08-04
|
|
|
|
Both round-3 reviews returned **FAIL**, and both bound the failure to the same
|
|
place. **No shared-core edit is warranted.** Review A verified the relocation
|
|
line by line and states plainly: the code is correct.
|
|
|
|
Round 4 is one test, one deleted sentence, two `docs/ISSUES.md` fixes, and a
|
|
handful of doc corrections.
|
|
|
|
## The MAJOR — both reviewers, independently
|
|
|
|
**Nothing in the tree discriminates round 3's predicate from round 2's**, and a
|
|
test doc cites a sibling test that does not exist.
|
|
|
|
`RuntimeRemotePlacementDriveControllerTests.cs:1343` names
|
|
`ConcurrentQuiescences_ParkIsNotRestoredIntoTheQuiescingDestination`. That
|
|
identifier occurs exactly twice in the tree, both inside that one comment.
|
|
|
|
Review A evaluated round 2's form
|
|
(`blockingToken.LandblockPrefix != (result.CellId & 0xFFFF0000)`) against all six
|
|
quiescence tests. **Every row agrees with round 3.** Revert the relocation to the
|
|
single-token comparison and the suite is still green at 11,007. So A1's two
|
|
stated defects — comparing one token instead of the live set, and reading the
|
|
pre-snap cell instead of the post-snap one — have no automated floor under them
|
|
at all, while the tree asserts otherwise.
|
|
|
|
**Write the test.** Both reviewers identified the same and only discriminating
|
|
construction, in `RuntimeAcceptedPositionDriveControllerTests` (route 2 has no
|
|
pre-flight, which is what makes it the caller that can reach the shape):
|
|
|
|
- two live quiescences, source `OperationId` < destination `OperationId`
|
|
- park at `:2948` under the source
|
|
- assert `!IsSpatialRoot(record)` after the next packet's merge-time `Forget`
|
|
|
|
It fails under the round-2 form (`Source != Dest` → restorable → re-admitted into
|
|
the retiring destination) and passes under round 3. Then delete or correct the
|
|
`:1343` cross-reference.
|
|
|
|
## #309 — the gate currently passes while broken
|
|
|
|
**S1.** Step 5 (`docs/ISSUES.md:115-119`) says the player "recovers on the next
|
|
server Position rather than staying withdrawn indefinitely".
|
|
`QuiescingDestinationPrefix_ForcePositionParkIsNotRestored` (`:906-912`) asserts
|
|
the opposite for that packet — `InWorld == false`, clock inactive,
|
|
`!IsSpatialRoot`. Route 2's drive dispatches **only** for `ForcePosition`; an
|
|
ordinary `Apply` merges behind it (`RuntimeAcceptedPositionDriveController.cs:337`).
|
|
So the packet the step names is precisely the one that does not recover it. Name
|
|
what actually does — a later ForcePosition, or the quiescence releasing plus a
|
|
placement.
|
|
|
|
**S2.** Steps 4 and 5 both need a ForcePosition to land *inside* a transient
|
|
collision-prefix quiescence window, which the user cannot synchronise with a
|
|
teleport or portal arrival, and neither step names a signal confirming the park
|
|
fired. A tester sees a clean teleport and records a pass. **Give each step a
|
|
stated confirmation signal** — a probe line, or a documented
|
|
`CaptureOwnership().DeferredCellCount` check. If a deterministic way to hold a
|
|
prefix quiescing for the duration is cheap, prefer it.
|
|
|
|
Step 5's retirement-still-completes clause is good; steps 1-3 and 6 are
|
|
performable as written.
|
|
|
|
## Doc corrections
|
|
|
|
- **D1** — AP-136 says restore covers the plain unplaceable park
|
|
"**unconditionally**". It does not: `ParkDeferred` now gates *every* park,
|
|
including `:3063`, on `!IsCollisionPrefixQuiescing(body.CellPosition.ObjCellId)`
|
|
(`:4358-4360`). The same row states it correctly two sentences later — the row
|
|
contradicts itself.
|
|
- **D2** — AP-136's trailing parenthetical still summarises #309 as the original
|
|
three steps plus "route 2's corrections are unchanged". This commit rewrote
|
|
#309. The row's body says the scope widened; its own summary does not.
|
|
- **D3** — `RuntimeRemotePlacementDriveController.cs:1064-1066` attributes the
|
|
null-body re-test to `PrepareMover`'s guard clause
|
|
(`RuntimeSetPositionState.cs:1563-1577`), which has **no** body test. The real
|
|
dependency is `RuntimeRemotePlacementDriveController.cs:625`
|
|
(`record.PhysicsBody is null` → `NotApplicable`). Conclusion holds; the
|
|
attribution points a maintainer at the wrong guard.
|
|
- **D4 — B2's chosen value was not delivered.** Choosing "state it" over "enforce
|
|
it" is sound and both reviewers concur with the decision — but **none** of the
|
|
four dependencies is annotated at its own site: `PrepareMover`'s guard
|
|
(`:1563-1577`), `IsPreparationAuthorityCurrent` (`:5304-5320`),
|
|
`PrepareDormantLocalActivationOwnership`'s `record.PhysicsBody is not null`
|
|
(`:1380`), `TryBeginExclusiveAuthoredPlacement`'s existing-operation refusal
|
|
(`:1349-1351`). Deleting any one is invisible from the site, which is the whole
|
|
thing "state it" was supposed to buy. Four one-line back-references close it.
|
|
- **D5 — scope the `CurrentCellId` claim to the first submit.** `ParkDeferred`'s
|
|
doc (`:4300-4307`, `:4320-4325`), AP-136 and AP-138(2) all say the pre-engine
|
|
arm "names the destination" in production. True on the first submit, **false
|
|
for a retained retry**: `Advance()` re-reads `record.FullCellId` at submit with
|
|
no fresh merge (`RuntimeRemotePlacementDriveController.cs:941-954`,
|
|
`RuntimeAcceptedPositionDriveController.cs:663-667`), and
|
|
`RemoteTeleportController.cs:532` is a shipped writer that moves it to the
|
|
pre-teleport landblock. `CanAttemptDestination`'s own doc (`:1234-1241`) already
|
|
says this correctly — the three others contradict it. Otherwise the next reader
|
|
concludes the `CurrentCellId` arm is dead code.
|
|
Same family: `:1252-1253` ("every shape this predicate cannot see is a
|
|
merely-swept NEIGHBOUR") is contradicted by the third-landblock shape named ten
|
|
lines above it.
|
|
|
|
## D6 — make the `Captured` claim exact rather than caveated
|
|
|
|
`Captured` is a **park-time** snapshot. For the far snap the window is
|
|
nanoseconds (`SubmitAndResolve`'s `DeferredCell` arm cancels synchronously,
|
|
`:1038`). For **route 2** the park is *retained*
|
|
(`RuntimeAcceptedPositionDriveController.cs:745-753`, `AwaitingCommitWake: true`)
|
|
and the restore lands on the next packet's merge-time `Forget`, ~150 ms later.
|
|
Streaming opens a quiescence per landblock mutation, so a prefix clean at park
|
|
time can be quiescing at restore time, and `RestoreParkWithdrawal` re-admits into
|
|
it unconditionally.
|
|
|
|
It self-heals and is no worse than the baseline, so it is a residual rather than
|
|
a regression — but **add the re-test**: one `IsCollisionPrefixQuiescing(residentCellId)`
|
|
condition at `RuntimeSetPositionState.cs:3488`, in the method that already reads
|
|
that variable. That makes the docs' claim true at the moment it matters instead
|
|
of requiring a "when the park was taken" caveat everywhere it appears. Prefer the
|
|
exact claim over the caveat.
|
|
|
|
## NIT
|
|
|
|
- **N1** — `IsCollisionPrefixQuiescing(0)` re-opens the C3c-F3 sentinel
|
|
conflation. `AdjustToOutside` zeroes the cell id on map-edge failure
|
|
(`LandDefs.cs:139`), and prefix `0x00000000` is the *legitimate* corner
|
|
landblock (0,0) per `BeginCollisionPrefixQuiescence:840-846`, not "no
|
|
landblock". Review A could not construct a reachable path, but this file
|
|
already carries a shipped-crash comment about exactly this conflation. Add the
|
|
`!= 0u` guard.
|
|
- **N2** — `FarSnap_ConcurrentQuiescences_RefusesBeforeOpeningAPark` is
|
|
behaviourally redundant with the destination-prefix test (both fail identically
|
|
if the pre-flight's `IsCollisionPrefixQuiescing` half is removed), and its
|
|
`sourceQuiescence.OperationId < destinationQuiescence.OperationId` assert never
|
|
influences an observed outcome. Keep it, but its doc should not imply stronger
|
|
A5 evidence than it carries.
|
|
- **N3** — `RuntimeAcceptedPositionExecutionStatus.DeferredCell`'s doc ("the
|
|
destination landblock's collision generation was not ready") is the status two
|
|
new route-2 tests assert for a *quiescence* park. Pre-existing, in a file this
|
|
diff does not touch — the next stale comment in line. Fix it or file it.
|
|
|
|
## Do not churn — verified across both reviews
|
|
|
|
The A1 relocation is mechanically correct: `IsCollisionPrefixQuiescing` consults
|
|
the full map (`:827-829`; one live entry per prefix via Remove-then-Add
|
|
`:857`/`:885`); the post-snap cell is exactly what `RestoreParkWithdrawal:3484`
|
|
restores at, with no writer in between; `SnapToCell` really does set
|
|
`InWorld = true` (`PhysicsBody.cs:201-205`) so the three-local hoist is required,
|
|
not stylistic; `RetryDeferred` re-parks without a second `SnapToCell`, keeping
|
|
the decision bound to the cell it was taken against; the plain park at `:3063` is
|
|
**provably** unchanged (the `:3017` quiescence check already returned false, and
|
|
`AdjustToOutside` is idempotent on an adjusted pair); `ParkCollisionResidents`
|
|
passes `false` explicitly; no prefix pin or streaming stall is possible
|
|
(`HasOldPrefixPlacementDebt:3829` skips `WakeableLostCell`, and the operation is
|
|
retired before the restore). All four `ParkDeferred` call sites accounted for.
|
|
|
|
**The pushback is a legitimate correction**, traced independently through
|
|
`TryApplyPosition:1771-1774` → `RefreshDerivedState:230-237` → `SetFullCell`, and
|
|
the remote leg through `RebucketLiveEntity` → `CommitRebucket:1816`. Route 1 and
|
|
remote creates cannot reach the source shape at all (`CurrentCellId` requires
|
|
`body.InWorld`, false for a first-entry body).
|
|
|
|
Both route-2 tests drive production (`MergeAccepted` → real `TryApplyPosition`,
|
|
real `PhysicsTimestampGate`, then `TryExecuteAcceptedLocalPosition`); nothing is
|
|
hand-built. Keeping `FarSnap_QuiescingSourceLandblock_` is right — the state is
|
|
constructible in the fixture, only production cannot reach it — and its caveat is
|
|
accurate. A5 is satisfied at the pre-flight itself. B1, B3, m1-m5, n2, n3 all
|
|
fixed and accurate. **n1 in the round-3 correction was MINE and wrong** —
|
|
classifier `:467` is correct, `:468` is `ConstrainPhase`, and the member-name
|
|
replacement is exact. No added line carries a source line-number citation —
|
|
verified by scanning every `+` line.
|
|
|
|
## Gate
|
|
|
|
Complete Release suite. Baseline **10,968 / 4 / 0** at `1b631f12`; round 3
|
|
measured **11,007 / 4 / 0**. Known flakes, do not chase or conflate: **#302**
|
|
(`PortalProjectionTests…`, App.Tests), **#308** (`NakEmissionTests.LossSoak_…`,
|
|
Core.Net.Tests).
|