fix(review): close the C5b architecture-review findings (D2/D3/D4, L1-L5, S1)
Follow-up to C5b (735f0a72). The retail-conformance review passed, so no production behaviour line moved: the flag truth table and the refreshPosition:false withhold are untouched. This is blast radius, test discrimination and documentation fidelity - plus two findings I could not confirm and am rebutting rather than complying with. D3 - THE PUBLISH-CONSERVATION TEST DID NOT DISCRIMINATE. The reviewer was right and it was the worst finding here: proof obligation 3's test passed identically with C5b reverted. Its only delta assertion FILTERED (Assert.Single(deltas, Rebucketed && parentGuid)), so the pre-C5b stream [Rebucketed] and the post-C5b stream [Updated, Rebucketed] both satisfied it, and childSpatialBefore+1 held in both worlds because whichever site did not move the cell propagated idempotently. It now asserts the complete ordered parent stream plus each element's CellId and Position.ObjCellId. Sabotage: restoring refreshPosition:acceptedPosition turns it red (it was green before), together with the withhold test and the new L5 test. That cardinality change was itself unfiled and is now AP-147: a cell-changing accepted Position publishes TWO entity deltas where it published one, and the intermediate Updated pairs the OLD CellId with the NEW wire Position - a torn pair that did not exist pre-C5b, since both halves used to move inside one publish. No production consumer reads a delta's paired fields, but a recorder/plugin/bot event log would capture it. The row states why suppressing the Updated is not available at that layer (the merge cannot know whether its caller reaches W2). D4 - THE PROJECTILE DOC COMMENT WAS FALSE AND ITS RETAIL ARGUMENT WAS INVERTED. SyncPresentationFromResolvedBody claimed record.FullCellId is "the WIRE cell ... stamped by the merge's RefreshDerivedState/SetFullCell, before classification ever runs" and argued from retail's store_position @0x00515CE2 that the destination cell is the right one. C5b falsified the premise; the missile arm also returns before W2, so nothing stamps the wire cell for a projectile at all. Rewritten. The honest conclusion, which the old text would have called wrong: on a stored outcome presentation now pairs the DESTINATION world position with the SOURCE cell. That is not a choice this method can make differently - StoreAcceptedDestinationPose writes only Position/Orientation, so record.FullCellId and body.CellPosition.ObjCellId now hold the same source cell and reading either yields the same value. The divergence is AP-138 item (1)'s store-writes-pose-but-not-cell residual, retiring via #309, not a field choice here. Projecting the wire cell instead would invent a residency the placement declined - the AP-1 shape C5b closed. L3/L4/L5 - PINNING GAPS, ALL THREE CONFIRMED AND CLOSED. L3: the matrix's oracle passed HasAnimations as a literal, so the merge's old.MotionTableId ?? old.Physics?.MotionTableId and RuntimeAcceptedPositionRouteRequests.Build's canonical-snapshot twin were textually identical and pinned by nothing. The oracle is now BUILT by the production constructor. L4: every fixture set both MotionTableId halves to the same value, so deleting either operand of the ?? was undetectable while the production comment said the mixed case is the real-world one. Six mixed rows added, including the explicit-zero row (a present-but-zero top half is not null, so ?? never reaches the physics half). L5: the retained Rebucketed ternary had zero coverage through TryApplyPosition - every restoreCancelledPark test called Forget directly. Now driven through the real merge, with the wire cell deliberately the SOURCE while the park's committed body cell is the DESTINATION, so the restored residency can only have come from the rollback. Sabotage (each red, each restored): merge ?? -> top half only, 1 red; -> physics half only, 2 red; Build's ?? -> physics half only, 2 red; ternary -> constant Updated, exactly the L5 test red. L1/L2 - THE MISSING TEST IS ADDED; THE DEFECT IS NOT THERE. The reviewer was right that C5b's "no fixture covers pickup at that layer" was inaccurate - LiveEntityNetworkOnPositionCollapseMatrixTests drives the real OnPosition at ~26 sites - and the end-to-end test is added: withdraw -> accepted Position -> IsSpatiallyProjected && FullCellId == wireCell, both guid classes. But ChildUnparentDisposition.Pending is NOT a live defect, because it is production-unreachable. The sole production _withdrawProjection binding (LivePresentationComposition.cs:599) is LiveEntityProjectionWithdrawalController.WithdrawExact, whose only Pending mint is inside its catch block and therefore always carries a non-null Failure - and AdvanceUnparentTransition rethrows at EquippedChildRenderController.cs:1307 BEFORE the return Pending at :1309. The named drop scenario does not reach it anyway (BeginDetachedRemoval has already emptied the capture list) and would be correct if it did: a previously-equipped child is LegacyImmediate, so the FullCellId != 0u gate at DatLiveEntityProjectionMaterializer.cs:767 is never consulted and re-projection uses the wire cell at LiveEntityRuntime.cs:824. Measured while building that test, and NOT what C5b assumed: W2 and W3 are REDUNDANT on the remote tail. Sabotaging W2 alone - adopting the committed cell instead of the wire cell, OR skipping the rebucket outright - leaves the whole file green, because W3's RemoteMotion.CellId write reads through to canonical FullCellId via CommitCanonicalCell, whose CellCommitted recovery re-installs the bucket. Only removing BOTH goes red, and then the new test is the only red in the file. So it is named for what it pins, and AD-60 is amended with the measurement: neither channel is individually load-bearing, so a future retirement of one is caught by nothing else. D2 - REBUTTED, WITH THE REAL GAP FILED INSTEAD. The reviewer's hypothesis was that TryApplyInitialCreateCompletionPresentation's staleness guard lost its ability to detect an intervening steady-state Position when C5b stopped the merge stamping the wire cell, and asked for a PositionAuthorityVersion term. I do not think that is right and did not add it. The receipt's facts are the canonical BODY's pose and cell at publish (PublishExecutorCompletion builds both from the record). Exactly two owners can move them: a Runtime SetPosition commit/withdrawal, every one of which calls AdvancePlacementCommit - the only caller family is RuntimeSetPositionState - and a rebucket, which moves FullCellId. Both are already covered by the two existing terms. An accepted steady-state Position is neither, and C5b did not make it one: the merge refreshes the snapshot and advances PositionAuthorityVersion but never wrote the body, and the App generic tail writes the RENDER entity. The wire-cell half stays covered because W2/W3 commit it in the same call; the paths that return before them leave the record at the last committed cell, which IS the receipt's own cell - correctly not a supersession. Adding the term would decline receipts whose facts are still true, on the entity's FIRST world-visible moment: the pose write and RebucketLiveEntityPresentationOnly would be skipped while TryPublishPlace still publishes, so a packet returning before the render write would leave the sidecar visible at its materialized pose in a wrong bucket. That is the handoff's own "removed the invariant failure while leaving the bug" shape. There IS one supersession neither term covers, and it predates C5b: RuntimeRemotePlacementDriveController.StoreAcceptedDestinationPose writes body.Position/Orientation on the far-snap Refused/Contention arm with no placement commit and no cell move. Filed as #323 with the FIFO-blocking argument for why a receipt can still be pending when it lands, an explicit "not established as reachable", and an explicit "do not fix it with PositionAuthorityVersion". The guard's comment now carries the whole argument instead of one sentence. S1 - DANGLING POINTER CLOSED. InboundPhysicsStateController.cs:610 still said the two-callers-one-rule debt was "tracked for the eventual cutover unification ... See docs/ISSUES.md", which pointed at nothing after C5b closed #275 without a successor. Filed #322, cited from both the comment and #275's closure, including why widening TryApplyPosition's signature to take a route would be the wrong unification. AP-138 amended: C5b staled its round-3 measurement that "both accepted-Position callers commit the accepted wire cell to record.FullCellId before submitting". Route 2 submits from TryExecuteAcceptedLocalPosition ahead of W2, so on a first submit PlacementTouchesPrefix's CurrentCellId arm now names the SOURCE landblock, not the destination. Confined to which prefix the quiescence pre-flight matches, which that row already established is not the correctness mechanism. GATES. Release build 0 errors. Complete suite 11,134 passed / 4 skipped / 0 failed, from the 11,125 / 4 baseline ated806997: net +9, all new tests, no test deleted or weakened, no new skip. Runtime.Tests 1195 -> 1202 (+6 mixed-motion-table rows, +1 park-rollback fact); App.Tests 4132 -> 4134 (+2 guid rows). None of #302/#308/#321 appeared. Not connected-gated - nothing here changes runtime behaviour. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
ed8069976c
commit
23aa62f292
9 changed files with 637 additions and 62 deletions
113
docs/ISSUES.md
113
docs/ISSUES.md
|
|
@ -1698,6 +1698,12 @@ which uses the production classifier as the oracle. Wiring the continuation
|
|||
executor into the steady-state path is a separate structural decision that no
|
||||
longer has any behavioural motivation behind it.
|
||||
|
||||
**Successor filed 2026-08-05 at the C5b review (finding S1): #322.** This
|
||||
closure originally left "the remaining structural item is tracked below" with
|
||||
no ID, and the production comment on
|
||||
`InboundPhysicsStateController.TryApplyPosition` pointed at `docs/ISSUES.md`
|
||||
for a follow-up that did not exist. Both now cite #322.
|
||||
|
||||
---
|
||||
|
||||
## #274 — Restricted/barred-house entry needs a connected retail comparison
|
||||
|
|
@ -13521,3 +13527,110 @@ under repetition, it is a real race and should be escalated out of LOW.
|
|||
Do not add a retry, a `Skip`, or a delay to make it green — this campaign's
|
||||
standing rule is no workarounds without explicit approval, and a masked race is
|
||||
strictly worse than a red test.
|
||||
|
||||
---
|
||||
|
||||
## #322 — Two callers compute the same two pre-placement flags from the same two inputs
|
||||
|
||||
**Status:** OPEN
|
||||
**Severity:** LOW (internal refactor debt; NOT a retail divergence)
|
||||
**Filed:** 2026-08-05 (C5b review, finding S1 — the successor #275 closed
|
||||
without filing)
|
||||
**Component:** Runtime / inbound Position
|
||||
|
||||
**Description.** Since C5b (`735f0a72`) both accepted-Position callers derive
|
||||
retail's two pre-placement writes — `installPlacementFrame` and `clearParent` —
|
||||
from the identical pair `(disposition, hasAnimations)`, in two separate places:
|
||||
|
||||
- `InboundPhysicsStateController.TryApplyPosition` computes them inline,
|
||||
pre-merge, from the retained snapshot `old`.
|
||||
- `RuntimeInitialCreateContinuationExecutor.ApplyPositionAction` reads them off
|
||||
`RuntimeAuthoritativePositionRouteClassifier`'s
|
||||
`ApplyPlacementFrameBeforeRouting`/`UnparentBeforeRouting`, built through
|
||||
`RuntimeAcceptedPositionRouteRequests.Build`.
|
||||
|
||||
The two are pinned EQUAL by test rather than by a shared code path
|
||||
(`InboundPhysicsStateControllerTests.MergedPrePlacementFieldsMatchTheClassifiedRouteFlags`,
|
||||
whose oracle is the production classifier fed through the production `Build`
|
||||
since the C5b review's L3 fix). That is deliberate: it keeps each computation
|
||||
separately sabotage-verifiable. It is still two copies of one rule.
|
||||
|
||||
**Why this is not #275.** #275 was the BEHAVIOURAL unification and is closed:
|
||||
the merge no longer passes unconditional `true/true`, and it no longer derives
|
||||
`FullCellId` from bare wire acceptance. What remains is structural only. There
|
||||
is no behavioural motivation left behind it, which is precisely why it needs
|
||||
its own ID rather than an open-ended "eventual cutover" pointer.
|
||||
|
||||
**Acceptance (either is fine, pick at the time):** (a) wire the continuation
|
||||
executor into the steady-state path so there is one caller, or (b) extract the
|
||||
two-flag derivation into one function both callers call — but if (b), the pin
|
||||
test must be re-argued, since a shared path makes the current
|
||||
merge-vs-classifier sabotage discrimination vacuous.
|
||||
|
||||
**Do not** widen `TryApplyPosition`'s signature to take a full
|
||||
`RuntimeAuthoritativePositionRoute` as a shortcut: the whole point of C5b's
|
||||
finding is that these two flags need no route, no `playerDistance` and no
|
||||
`CommittedCellId`, because retail decides both ahead of `MoveOrTeleport`
|
||||
(`SmartBox::HandleReceivedPosition` @0x00453FD0 — Gate A @0x0045400C returns
|
||||
@0x0045409D before `unset_parent` @0x00454129 and before the `HasAnims`
|
||||
`SetPlacementFrame` gate @0x00454137).
|
||||
|
||||
---
|
||||
|
||||
## #323 — A far-snap store can silently stale a pending initial-create completion receipt
|
||||
|
||||
**Status:** OPEN
|
||||
**Severity:** LOW (narrow, self-healing within one broadcast interval; no
|
||||
observed live symptom)
|
||||
**Filed:** 2026-08-05 (C5b review, finding D2 — found while establishing that
|
||||
C5b did NOT weaken the guard; this gap predates C5b)
|
||||
**Component:** App / placement projection
|
||||
|
||||
**Description.** `LiveEntityRuntime.TryApplyInitialCreateCompletionPresentation`
|
||||
declines a stale `ExecutorCompleted` receipt on two terms:
|
||||
`record.FullCellId != token.ExactCellId` and
|
||||
`record.Canonical.PlacementCommitVersion != token.PlacementCommitVersion`.
|
||||
Together those cover every owner that can move the receipt's facts — the
|
||||
receipt carries the canonical body's pose and cell at publish
|
||||
(`RuntimeSetPositionState.PublishExecutorCompletion`), and only a Runtime
|
||||
SetPosition commit/withdrawal (every one of which calls
|
||||
`AdvancePlacementCommit`; the only caller family is `RuntimeSetPositionState`)
|
||||
or a rebucket can move them.
|
||||
|
||||
**Except one.** `RuntimeRemotePlacementDriveController.StoreAcceptedDestinationPose`
|
||||
writes `body.Position` and `body.Orientation` directly on the far-snap
|
||||
`Refused`/`Contention`/`RejectedPreparation` arm (AP-138 item (1)'s
|
||||
"store, because the resolve never ran"). It bumps no placement commit and
|
||||
moves no cell. If an `ExecutorCompleted` receipt for that entity is still
|
||||
sitting behind an unacknowledged receipt on the shared placement FIFO when
|
||||
that store lands, the receipt drains with a pose that is now older than the
|
||||
body's, and `entity.SetPosition(projection.WorldPosition)` snaps the render
|
||||
entity back.
|
||||
|
||||
**Why the FIFO can be non-empty at that moment.** `PublishExecutorCompletion`
|
||||
dispatches synchronously, but `RuntimePlacementProjectionSubscription.OnPlacement`
|
||||
applies a receipt only when it is the FIFO head, and
|
||||
`RuntimePlacementPresentationSink.TryApply` deliberately refuses (leaves at the
|
||||
head) any `Place`/`Withdraw` for an entity still holding an initial-create
|
||||
residence, for the drive controller's per-frame pump to consume. So one
|
||||
entity's conductor-owned receipt can hold another entity's `ExecutorCompleted`
|
||||
behind it across network packets.
|
||||
|
||||
**Not established:** whether the combination is actually reachable in play — it
|
||||
needs a ≥96 m far-snap classification for an entity whose initial-create
|
||||
completion is still queued, and the far arm's refusal reasons are themselves
|
||||
narrow. Reported rather than fixed for exactly that reason.
|
||||
|
||||
**Do NOT fix it by adding `PositionAuthorityVersion` to the guard.** That was
|
||||
the C5b reviewer's proposed shape and it is wrong: the merge bumps that version
|
||||
on every accepted Position including ones that move nothing, so the guard would
|
||||
decline receipts whose facts are still true, on the entity's FIRST
|
||||
world-visible moment — skipping the pose write and
|
||||
`RebucketLiveEntityPresentationOnly` while `TryPublishPlace` still publishes.
|
||||
The correct shape, if this is ever confirmed reachable, is to make the store
|
||||
arm advertise itself (a body-pose authority version, or routing the store
|
||||
through a commit-versioned seam) so the receipt can see it.
|
||||
|
||||
**Superseded text, for the record.** The comment at the guard used to say only
|
||||
"A newer move superseded this receipt's facts after the drain." It now carries
|
||||
the full argument and cites this issue.
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
|
|
@ -503,38 +503,62 @@ internal sealed class ProjectileController
|
|||
/// before this is called.
|
||||
///
|
||||
/// <para>
|
||||
/// <b>B1/B2 fix (round-2 review — reverting round 1's own R6 "fix",
|
||||
/// which the retail reviewer retracted as factually wrong).</b>
|
||||
/// <b>Which cell this projects — REWRITTEN 2026-08-05 (C5b review, D4).
|
||||
/// The round-2 B1/B2 text this replaces was falsified by C5b and its
|
||||
/// retail argument now points the other way; do not restore it.</b>
|
||||
/// <c>ParentCellId</c> is read from <c>record</c>'s (the local resolved
|
||||
/// by <c>TryGetCurrent</c> below, equal to
|
||||
/// <paramref name="expectedRecord"/> once the currency check passes)
|
||||
/// <c>FullCellId</c> — the WIRE cell — never the body's own
|
||||
/// <c>CellPosition.ObjCellId</c>. On a committed outcome the two agree
|
||||
/// (<c>CommitCanonical</c> writes both together), so the choice is a
|
||||
/// no-op there. On a STORED outcome
|
||||
/// (<c>Refused</c>/<c>Contention</c>/<c>RejectedPreparation</c>),
|
||||
/// <c>StoreAcceptedDestinationPose</c> composes <c>body.Position</c>
|
||||
/// from <c>accepted.PositionX + worldOffset(accepted.LandblockId)</c> —
|
||||
/// the WIRE cell's world frame — while <c>record.FullCellId</c> is that
|
||||
/// SAME wire cell (stamped by the merge's
|
||||
/// <c>RefreshDerivedState</c>/<c>SetFullCell</c>, before classification
|
||||
/// ever runs). The body's OWN <c>CellPosition.ObjCellId</c>, in
|
||||
/// contrast, is never touched by the store fallback and is therefore
|
||||
/// the SOURCE cell the body left — reading it here would pair the new
|
||||
/// destination position with the cell the body is no longer in. Three
|
||||
/// independent checks confirm <c>record.FullCellId</c> is the correct
|
||||
/// source: (1) the sibling remote arm's
|
||||
/// <c>TryApplyGenericRemoteRenderPose</c> pairs the wire world position
|
||||
/// with the wire cell the same way; (2) Runtime's own
|
||||
/// <c>SyncProjectilePresentation</c>, forty lines away in the SAME
|
||||
/// packet, publishes the shadow row at <c>record.FullCellId</c> — so
|
||||
/// reading the body's cell here would disagree with the shadow for the
|
||||
/// SAME body in the SAME call; (3) retail's <c>store_position</c>
|
||||
/// @0x00515CE2 writes the object's whole <c>Position</c> INCLUDING
|
||||
/// <c>objcell_id</c>, so after a retail store the object's cell IS the
|
||||
/// destination (the wire cell) — never the stale one. The body's own
|
||||
/// stale cell after a store is the acdream-side residual (AP-138,
|
||||
/// shared with the remote arms), not the truth to project.
|
||||
/// <c>FullCellId</c> — since C5b (#275) that is the last COMMITTED cell,
|
||||
/// never the wire cell. The steady-state merge withholds the derived
|
||||
/// <c>FullCellId</c> write entirely
|
||||
/// (<c>RuntimeEntityObjectLifetime.TryApplyPosition</c>'s
|
||||
/// <c>RefreshSnapshot(..., refreshPosition: false)</c>, AD-60), and the
|
||||
/// missile arm returns from <c>OnPosition</c> before the prologue
|
||||
/// rebucket (W2) ever runs, so nothing between admission and this call
|
||||
/// stamps the wire cell for a projectile.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// <b>The choice of source is now a no-op on every outcome.</b> On a
|
||||
/// COMMITTED outcome <c>CommitCanonical</c> writes
|
||||
/// <c>record.FullCellId</c> and <c>body.CellPosition.ObjCellId</c>
|
||||
/// together. On a STORED outcome
|
||||
/// (<c>Refused</c>/<c>Contention</c>/<c>RejectedPreparation</c>)
|
||||
/// <c>StoreAcceptedDestinationPose</c> writes <c>body.Position</c> and
|
||||
/// <c>body.Orientation</c> and NOTHING else — it does not move the
|
||||
/// body's <c>CellPosition.ObjCellId</c>, and post-C5b it does not move
|
||||
/// the record's <c>FullCellId</c> either. Both therefore hold the same
|
||||
/// last-committed (SOURCE) cell, so reading either field yields the
|
||||
/// identical value. That is what
|
||||
/// <c>MissileFarRefused_…ParentCellIdAgreesWithCommittedCell</c> pins:
|
||||
/// an identity between the record and the body, not a wire-cell
|
||||
/// constant. Runtime's own <c>SyncProjectilePresentation</c>, forty
|
||||
/// lines away in the SAME packet, publishes the shadow row at
|
||||
/// <c>record.FullCellId</c>, so this keeps presentation and shadow in
|
||||
/// agreement for the same body in the same call.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// <b>The pairing this leaves is genuinely torn, and is named rather
|
||||
/// than papered over.</b> After a stored outcome the render entity gets
|
||||
/// the DESTINATION world position (the store composed it from
|
||||
/// <c>accepted.PositionX + worldOffset(accepted.LandblockId)</c>, the
|
||||
/// wire cell's world frame) paired with the SOURCE cell. Retail cannot
|
||||
/// produce that state twice over: it has no refusable placement at all
|
||||
/// (AD-62), and its <c>store_position</c> @0x00515CE2 writes the whole
|
||||
/// <c>Position</c> INCLUDING <c>objcell_id</c> before
|
||||
/// <c>GotoLostCell</c> @0x00515CF2, so a retail store ends at the
|
||||
/// destination cell AND the destination pose. The divergence is
|
||||
/// acdream's store-writes-pose-but-not-cell residual — AP-138 item (1)'s
|
||||
/// "at the destination, with no resolved cell", shared with both remote
|
||||
/// arms — and its retirement is issue #309 (make the failure path open
|
||||
/// retail's lost-cell registration), NOT reading a different field here.
|
||||
/// Projecting the wire cell instead would invent a residency the
|
||||
/// placement explicitly declined, which is the exact AP-1-shaped write
|
||||
/// C5b closed. Reachability today is deterministic-test-only: ACE never
|
||||
/// sends <c>UpdatePosition</c> for a missile (AP-141's evidence,
|
||||
/// <c>WorldObject_Tick.cs:333-334</c>/<c>:265</c>).
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
|
|
|
|||
|
|
@ -1221,6 +1221,52 @@ public sealed class LiveEntityRuntime : ILiveEntityRadarSource
|
|||
!= token.PlacementCommitVersion)
|
||||
{
|
||||
// A newer move superseded this receipt's facts after the drain.
|
||||
//
|
||||
// WHY THESE TWO TERMS AND NOT `PositionAuthorityVersion` (C5b
|
||||
// review, finding D2 — examined and deliberately left alone;
|
||||
// the sibling guard in
|
||||
// TryGetRuntimePlacementProjectionRecord(requirePlacementVersions:
|
||||
// true) DOES check it, and the asymmetry is intentional).
|
||||
//
|
||||
// This receipt's facts are the CANONICAL BODY's pose and cell at
|
||||
// publish time (RuntimeSetPositionState.PublishExecutorCompletion
|
||||
// builds the snapshot from `record.PhysicsBody` and
|
||||
// `record.FullCellId`). There are exactly two owners that can move
|
||||
// either: a Runtime SetPosition commit/withdrawal, every one of
|
||||
// which calls `AdvancePlacementCommit` — the ONLY caller family is
|
||||
// RuntimeSetPositionState itself — and a rebucket, which moves
|
||||
// `FullCellId`. Both are covered above.
|
||||
//
|
||||
// An intervening accepted steady-state Position is NOT one of
|
||||
// them, and specifically was not turned into one by C5b (#275).
|
||||
// C5b made the merge withhold the `FullCellId` write, but the
|
||||
// merge never wrote the canonical body either: it refreshes the
|
||||
// snapshot and advances `PositionAuthorityVersion`, and the
|
||||
// App-side generic tail writes the RENDER entity, not the body.
|
||||
// The wire-cell half is still covered here because the OnPosition
|
||||
// prologue rebucket (W2) commits it in the same call, so a
|
||||
// cell-changing Position still trips the first term; the paths
|
||||
// that return before W2 (the local force arm, the missile arm)
|
||||
// leave the record exactly where the last commit put it, which is
|
||||
// this receipt's own cell — correctly NOT a supersession.
|
||||
//
|
||||
// Adding `PositionAuthorityVersion` would therefore decline
|
||||
// receipts whose facts are still true, on the entity's FIRST
|
||||
// world-visible moment: the pose write and
|
||||
// RebucketLiveEntityPresentationOnly below would be skipped while
|
||||
// TryPublishPlace still publishes, so a packet that returned
|
||||
// before the generic tail's render write would leave the sidecar
|
||||
// visible at its materialized pose in a possibly-wrong bucket.
|
||||
// That is a behaviour change in the wrong direction, not a
|
||||
// restoration.
|
||||
//
|
||||
// The ONE supersession neither term covers is
|
||||
// RuntimeRemotePlacementDriveController.StoreAcceptedDestinationPose:
|
||||
// the far-snap Refused/Contention arm writes `body.Position` and
|
||||
// `body.Orientation` with no placement commit and no cell move, so
|
||||
// it can stale this receipt's pose silently. That is pre-existing
|
||||
// (it predates C5b, which changed nothing about that arm) and is
|
||||
// filed as docs/ISSUES.md #323 rather than papered over here.
|
||||
return true;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -607,11 +607,14 @@ public sealed class InboundPhysicsStateController
|
|||
/// contact still comes solely from the retained wire packet's own
|
||||
/// <c>IsGrounded</c> bit on both. That remaining structural difference
|
||||
/// (two callers computing the same two flags from the same two inputs
|
||||
/// rather than sharing one code path) is internal refactor debt tracked
|
||||
/// for the eventual cutover unification - it is NOT a retail divergence
|
||||
/// and does not belong in
|
||||
/// docs/architecture/retail-divergence-register.md. See docs/ISSUES.md
|
||||
/// for the tracked follow-up.
|
||||
/// rather than sharing one code path) is internal refactor debt - it is
|
||||
/// NOT a retail divergence and does not belong in
|
||||
/// docs/architecture/retail-divergence-register.md. It is tracked as
|
||||
/// docs/ISSUES.md issue <b>#322</b>, filed 2026-08-05 at the C5b review
|
||||
/// (finding S1): #275 is CLOSED and this comment's former "tracked for
|
||||
/// the eventual cutover unification / see docs/ISSUES.md" wording pointed
|
||||
/// at nothing once it was. #322 also records why widening this method's
|
||||
/// signature to take a whole route would be the wrong unification.
|
||||
/// </summary>
|
||||
public bool TryApplyPosition(
|
||||
WorldSession.EntityPositionUpdate update,
|
||||
|
|
|
|||
|
|
@ -80,6 +80,97 @@ public sealed class LiveEntityNetworkOnPositionCollapseMatrixTests
|
|||
|
||||
private static bool IsPlayer(uint guid) => guid == PlayerGuid;
|
||||
|
||||
// ── Scenario 0: C5b's untested load-bearing claim ───────────────────
|
||||
|
||||
/// <summary>
|
||||
/// C5b (#275) review, findings L1/L2. C5b made the steady-state merge stop
|
||||
/// stamping the wire cell and justified it with a claim it never tested:
|
||||
/// <i>"production installs the bucket at W2 in the same call (verified: no
|
||||
/// return between the recovery call and W2 is conditioned on
|
||||
/// IsSpatiallyProjected or FullCellId)"</i>. The commit also asserted "no
|
||||
/// fixture covers pickup at that layer" — this file drives the real
|
||||
/// <see cref="LiveEntityNetworkUpdateController.OnPosition"/> at ~26 call
|
||||
/// sites, and every W2 assertion the commit added hand-calls
|
||||
/// <c>RebucketLiveEntity</c>/<c>CommitRebucket</c> itself, so nothing
|
||||
/// drove the sequence end-to-end. This does.
|
||||
///
|
||||
/// <para>The scenario is the leave-world/re-entry edge the production
|
||||
/// comment at <c>LiveEntityNetworkUpdateController.cs:2073-2077</c> names:
|
||||
/// the record keeps <c>WorldEntity</c> as its logical/render-resource
|
||||
/// owner while <c>IsSpatiallyProjected</c> is false, and "a fresh retail
|
||||
/// Position is the re-entry edge; testing only for the retained object
|
||||
/// reference leaves dropped inventory permanently invisible after
|
||||
/// InventoryPutObjectIn3D". The materializer DECLINES (the post-C5b
|
||||
/// self-projection state — <c>DatLiveEntityProjectionMaterializer.cs:767</c>
|
||||
/// now sees a canonical cell the merge did not advance), so the only
|
||||
/// things that can restore the bucket and the cell are the two wire-cell
|
||||
/// channels AD-60 says deliberately survive C5b.</para>
|
||||
///
|
||||
/// <para><b>Measured at the review, and NOT what C5b's commit message
|
||||
/// assumed: W2 and W3 are REDUNDANT here.</b> Sabotaging W2 alone (both
|
||||
/// "adopt the committed cell instead of the wire cell" and "skip the
|
||||
/// rebucket entirely") leaves this test — and every other test in this
|
||||
/// file — GREEN, because the post-routing wire-cell adopt
|
||||
/// (<c>TryAdoptWireCellAfterRouting</c>, W3/AP-135) writes
|
||||
/// <c>RemoteMotion.CellId</c>, which reads through to canonical
|
||||
/// <c>FullCellId</c> via <c>CommitCanonicalCell</c>, whose graphical
|
||||
/// <c>CellCommitted</c> recovery re-installs the bucket. Only removing
|
||||
/// BOTH surviving channels turns this red — and it is then the ONLY red
|
||||
/// in the file. So what this test pins is AD-60's actual surviving claim
|
||||
/// ("two wire-cell writers deliberately remain downstream of the merge"),
|
||||
/// not W2 in isolation. Naming it after W2 would have been a third
|
||||
/// contract asserting a mechanism that is not the one doing the work.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>Both guid classes per this file's own discipline.</para>
|
||||
/// </summary>
|
||||
[Theory]
|
||||
[InlineData(PlayerGuid)]
|
||||
[InlineData(CreatureGuid)]
|
||||
public void WithdrawnProjection_AcceptedPositionRestoresBucketAndWireCell(
|
||||
uint guid)
|
||||
{
|
||||
using var fixture = new Fixture(guid, decliningMaterializer: true);
|
||||
Assert.True(fixture.Runtime.TryGetRecord(guid, out LiveEntityRecord record));
|
||||
Assert.True(record.IsSpatiallyProjected);
|
||||
Assert.Equal(SourceCell, record.FullCellId);
|
||||
|
||||
// The withdrawal: render bucket gone, logical record + WorldEntity
|
||||
// retained. This is what makes RequiresSpatialProjectionRecovery true
|
||||
// on the next accepted Position.
|
||||
Assert.True(fixture.Runtime.WithdrawLiveEntityProjection(guid));
|
||||
Assert.False(record.IsSpatiallyProjected);
|
||||
Assert.NotNull(record.WorldEntity);
|
||||
Assert.Equal(SourceCell, record.FullCellId);
|
||||
|
||||
const uint WireCell = SourceLandblock | 0x0002u;
|
||||
fixture.Controller.OnPosition(fixture.Update(
|
||||
new Vector3(13f, 15f, SpawnHeight),
|
||||
WireCell,
|
||||
teleportSequence: 1,
|
||||
guid: guid));
|
||||
|
||||
// The recovery branch really ran and really declined — without this
|
||||
// the assertions below could pass on an entity that never needed
|
||||
// recovering at all.
|
||||
Assert.True(fixture.Lifetime.Entities.TryGetActive(
|
||||
guid, out RuntimeEntityRecord canonical));
|
||||
// The packet really was accepted and really did reach the recovery
|
||||
// branch, which really did decline — without these the cell assertions
|
||||
// below could pass on a rejected packet, or on an entity that never
|
||||
// needed recovering at all.
|
||||
Assert.Equal(
|
||||
WireCell,
|
||||
canonical.Snapshot.Position!.Value.LandblockId);
|
||||
Assert.True(fixture.MaterializerDeclined);
|
||||
|
||||
// W2 is what closes both halves.
|
||||
Assert.True(record.IsSpatiallyProjected);
|
||||
Assert.Equal(WireCell, record.FullCellId);
|
||||
Assert.Equal(WireCell, canonical.FullCellId);
|
||||
Assert.Equal(WireCell, fixture.Entity.ParentCellId);
|
||||
}
|
||||
|
||||
// ── Scenario 1: teleport commit ─────────────────────────────────────
|
||||
|
||||
[Theory]
|
||||
|
|
@ -966,14 +1057,27 @@ public sealed class LiveEntityNetworkOnPositionCollapseMatrixTests
|
|||
private readonly uint _guid;
|
||||
private readonly bool _nullClassification;
|
||||
|
||||
/// <summary>
|
||||
/// C5b review L1/L2: true once the hydration controller's materializer
|
||||
/// was actually reached and DECLINED. Only meaningful when the fixture
|
||||
/// was built with <c>decliningMaterializer: true</c>.
|
||||
/// </summary>
|
||||
internal bool MaterializerDeclined => _decliningMaterializer?.Declined
|
||||
?? false;
|
||||
|
||||
private readonly DecliningMaterializer? _decliningMaterializer;
|
||||
|
||||
internal Fixture(
|
||||
uint guid,
|
||||
bool nullClassification = false,
|
||||
bool withAnimation = false,
|
||||
bool isMissile = false)
|
||||
bool isMissile = false,
|
||||
bool decliningMaterializer = false)
|
||||
{
|
||||
_guid = guid;
|
||||
_nullClassification = nullClassification;
|
||||
_decliningMaterializer =
|
||||
decliningMaterializer ? new DecliningMaterializer() : null;
|
||||
var engine = new PhysicsEngine { DataCache = new PhysicsDataCache() };
|
||||
engine.AddLandblock(
|
||||
SourceLandblock,
|
||||
|
|
@ -1221,7 +1325,8 @@ public sealed class LiveEntityNetworkOnPositionCollapseMatrixTests
|
|||
Runtime,
|
||||
Lifetime,
|
||||
new object(),
|
||||
new NoopMaterializer(),
|
||||
(ILiveEntityProjectionMaterializer?)_decliningMaterializer
|
||||
?? new NoopMaterializer(),
|
||||
new NoopRelationships(),
|
||||
new NoopReadyPublisher(),
|
||||
new AlwaysKnownOrigin(),
|
||||
|
|
@ -1521,6 +1626,36 @@ public sealed class LiveEntityNetworkOnPositionCollapseMatrixTests
|
|||
public void ForgetUnknownOwner(uint serverGuid) { }
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// C5b review L1/L2: models the post-C5b
|
||||
/// <c>DatLiveEntityProjectionMaterializer</c> self-projection branch
|
||||
/// DECLINING (<c>DatLiveEntityProjectionMaterializer.cs:767-787</c>'s
|
||||
/// <c>expectedCanonical.FullCellId != 0u</c> gate not being satisfied),
|
||||
/// which is the state C5b's contract §14 item (1) says production now
|
||||
/// reaches. Records that it was reached so the test cannot pass
|
||||
/// vacuously, and returns <see langword="false"/> — the value
|
||||
/// <c>OnPosition</c>'s recovery call site deliberately ignores.
|
||||
/// </summary>
|
||||
private sealed class DecliningMaterializer
|
||||
: ILiveEntityProjectionMaterializer
|
||||
{
|
||||
internal bool Declined { get; private set; }
|
||||
|
||||
public bool TryMaterialize(
|
||||
RuntimeEntityRecord expectedCanonical,
|
||||
WorldSession.EntitySpawn canonicalSpawn,
|
||||
LiveProjectionPurpose purpose,
|
||||
ulong expectedCreateIntegrationVersion,
|
||||
AcDream.App.Rendering.LiveEntityAppearanceUpdateState?
|
||||
appearanceUpdate = null)
|
||||
{
|
||||
Declined = true;
|
||||
return false;
|
||||
}
|
||||
|
||||
public void ResetSessionState() => Declined = false;
|
||||
}
|
||||
|
||||
private sealed class NoopMaterializer : ILiveEntityProjectionMaterializer
|
||||
{
|
||||
public bool TryMaterialize(
|
||||
|
|
|
|||
|
|
@ -386,30 +386,35 @@ public sealed class InboundPhysicsStateControllerTests
|
|||
|
||||
// The oracle: the production classifier, fed the same wire packet and
|
||||
// the same static HasAnimations proxy.
|
||||
//
|
||||
// C5b review L3 (2026-08-05): the request is now BUILT by the
|
||||
// production constructor rather than hand-written. The old version
|
||||
// passed `HasAnimations: animated` as a literal, so the merge's
|
||||
// `old.MotionTableId ?? old.Physics?.MotionTableId` and
|
||||
// RuntimeAcceptedPositionRouteRequests.Build's canonical-snapshot
|
||||
// equivalent were textually identical and pinned by nothing — either
|
||||
// could have been edited without a red test. Routing the oracle
|
||||
// through Build makes the equality of the two expressions the thing
|
||||
// this matrix actually asserts.
|
||||
using RuntimeEntityObjectLifetime oracle = OracleLifetime();
|
||||
RuntimeEntityRecord canonical = RegisterOracleSnapshot(oracle, seed);
|
||||
RuntimeAuthoritativePositionRoute route =
|
||||
RuntimeAuthoritativePositionRouteClassifier.ClassifyAcceptedPosition(
|
||||
new RuntimeAcceptedPositionRouteRequest(
|
||||
new RuntimeAuthoritativePositionAuthority(
|
||||
new RuntimeGenerationToken(7),
|
||||
new RuntimeEntityKey(guid, 1),
|
||||
PositionAuthorityVersion: 4UL,
|
||||
AcceptedPositionSequence: update.PositionSequence,
|
||||
timestamps.PreviousTeleport,
|
||||
timestamps.Teleport,
|
||||
disposition),
|
||||
RuntimeAcceptedPositionRouteRequests.Build(
|
||||
new RuntimeGenerationToken(7),
|
||||
canonical,
|
||||
new RuntimeEntityKey(guid, 1),
|
||||
update,
|
||||
isLocalPlayer
|
||||
? RuntimePositionEntityKind.LocalPlayer
|
||||
: RuntimePositionEntityKind.Remote,
|
||||
RuntimeAcceptedPositionSource.PositionEvent,
|
||||
update.Position,
|
||||
PlacementFrame: update.PlacementId,
|
||||
PositionPackVelocity: null,
|
||||
CommittedCellId: 0x0101FFFFu,
|
||||
HasContact: true,
|
||||
PlayerDistance: 0f,
|
||||
UsePositionFromServer: true,
|
||||
HasAnimations: animated,
|
||||
default));
|
||||
disposition,
|
||||
timestamps.PreviousTeleport,
|
||||
timestamps.Teleport,
|
||||
playerDistance: 0f,
|
||||
usePositionFromServer: true,
|
||||
committedCellId: 0x0101FFFFu));
|
||||
|
||||
// Rebuild what the classified route would have installed, through the
|
||||
// SAME production merge, and require field equality.
|
||||
|
|
@ -938,6 +943,142 @@ public sealed class InboundPhysicsStateControllerTests
|
|||
teleport,
|
||||
forcePosition);
|
||||
|
||||
/// <summary>
|
||||
/// C5b review L4 (2026-08-05). The merge's HasAnimations proxy is
|
||||
/// <c>old.MotionTableId ?? old.Physics?.MotionTableId</c> — a coalesce
|
||||
/// whose whole point (per the production comment on
|
||||
/// <c>RuntimeAcceptedPositionRouteRequests.Build</c>) is that
|
||||
/// WeenieDescription and ObjDesc merges populate only ONE of the two
|
||||
/// fields, depending on which stage last touched appearance vs
|
||||
/// description. Every pre-existing fixture set both halves to the same
|
||||
/// value, so deleting either operand of the <c>??</c> was undetectable
|
||||
/// while the production comment claimed the mixed case is the real-world
|
||||
/// one. These rows are the mixed cases, and the oracle is still
|
||||
/// <c>Build</c> + the production classifier, so the two independent
|
||||
/// copies of the coalesce stay pinned equal on exactly the inputs that
|
||||
/// discriminate between them.
|
||||
///
|
||||
/// <para>The explicit-zero row is the third discriminator: a present-but-
|
||||
/// zero top half is NOT null, so <c>??</c> never reaches the nonzero
|
||||
/// physics half and the entity is correctly unanimated. Swapping the
|
||||
/// coalesce operands would flip that row alone.</para>
|
||||
/// </summary>
|
||||
[Theory]
|
||||
// topLevelMotionTableId, physicsMotionTableId, expectedAnimated
|
||||
[InlineData(null, 0x09000001u, true)]
|
||||
[InlineData(0x09000001u, null, true)]
|
||||
[InlineData(0x09000001u, 0x09000002u, true)]
|
||||
[InlineData(null, null, false)]
|
||||
[InlineData(0u, 0x09000001u, false)]
|
||||
[InlineData(null, 0u, false)]
|
||||
public void MixedMotionTableSourcesDriveTheSamePlacementFrameGateAsTheRoute(
|
||||
uint? topLevelMotionTableId,
|
||||
uint? physicsMotionTableId,
|
||||
bool expectedAnimated)
|
||||
{
|
||||
const uint guid = 0x80000041u;
|
||||
WorldSession.EntitySpawn seed = WithTimestamps(
|
||||
Spawn(guid, 3, 10, 1, Position(0x0101FFFFu, 10f), 0x408u),
|
||||
teleport: 10,
|
||||
forcePosition: 0);
|
||||
seed = seed with
|
||||
{
|
||||
PlacementId = 7,
|
||||
MotionTableId = topLevelMotionTableId,
|
||||
Physics = seed.Physics!.Value with
|
||||
{
|
||||
AnimationFrame = 7,
|
||||
MotionTableId = physicsMotionTableId,
|
||||
},
|
||||
};
|
||||
|
||||
WorldSession.EntityPositionUpdate update = new(
|
||||
guid,
|
||||
Position(0x0101FFFFu, 20f),
|
||||
Velocity: null,
|
||||
PlacementId: 5,
|
||||
IsGrounded: true,
|
||||
InstanceSequence: 3,
|
||||
PositionSequence: 11,
|
||||
TeleportSequence: 10,
|
||||
ForcePositionSequence: 0);
|
||||
|
||||
var merging = new InboundPhysicsStateController();
|
||||
merging.AcceptCreate(seed);
|
||||
Assert.True(merging.TryApplyPosition(
|
||||
update,
|
||||
isLocalPlayer: false,
|
||||
forcePositionRotation: null,
|
||||
currentLocalVelocity: null,
|
||||
out PositionTimestampDisposition disposition,
|
||||
out WorldSession.EntitySpawn merged,
|
||||
out AcceptedPhysicsTimestamps timestamps));
|
||||
Assert.Equal(PositionTimestampDisposition.Apply, disposition);
|
||||
|
||||
using RuntimeEntityObjectLifetime oracle = OracleLifetime();
|
||||
RuntimeEntityRecord canonical = RegisterOracleSnapshot(oracle, seed);
|
||||
RuntimeAuthoritativePositionRoute route =
|
||||
RuntimeAuthoritativePositionRouteClassifier.ClassifyAcceptedPosition(
|
||||
RuntimeAcceptedPositionRouteRequests.Build(
|
||||
new RuntimeGenerationToken(7),
|
||||
canonical,
|
||||
new RuntimeEntityKey(guid, 1),
|
||||
update,
|
||||
RuntimePositionEntityKind.Remote,
|
||||
RuntimeAcceptedPositionSource.PositionEvent,
|
||||
disposition,
|
||||
timestamps.PreviousTeleport,
|
||||
timestamps.Teleport,
|
||||
playerDistance: 0f,
|
||||
usePositionFromServer: true,
|
||||
committedCellId: 0x0101FFFFu));
|
||||
|
||||
// The classified route's own gate agrees with the fixture's intent…
|
||||
Assert.Equal(!expectedAnimated, route.ApplyPlacementFrameBeforeRouting);
|
||||
// …and the merge, which computed its own copy, lands the SAME frame.
|
||||
// Animated => retail's HasAnims gate @0x00454137 skips SetPlacementFrame
|
||||
// entirely, so the seed's 7 survives; unanimated => the wire's 5 lands.
|
||||
Assert.Equal(expectedAnimated ? 7u : 5u, merged.PlacementId);
|
||||
Assert.Equal(
|
||||
expectedAnimated ? 7u : 5u,
|
||||
merged.Physics!.Value.AnimationFrame);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A minimal Runtime lifetime whose only purpose is to own a canonical
|
||||
/// <see cref="RuntimeEntityRecord"/> so the production
|
||||
/// <c>RuntimeAcceptedPositionRouteRequests.Build</c> can be the oracle
|
||||
/// (C5b review L3). The fixture landblock matches the 0x0101FFFF cells
|
||||
/// every packet in this file uses.
|
||||
/// </summary>
|
||||
private static RuntimeEntityObjectLifetime OracleLifetime()
|
||||
{
|
||||
var engine = new PhysicsEngine { DataCache = new PhysicsDataCache() };
|
||||
engine.AddLandblock(
|
||||
0x01010000u,
|
||||
new TerrainSurface(new byte[81], new float[256]),
|
||||
Array.Empty<CellSurface>(),
|
||||
Array.Empty<PortalPlane>(),
|
||||
worldOffsetX: 0f,
|
||||
worldOffsetY: 0f);
|
||||
return new RuntimeEntityObjectLifetime(engine);
|
||||
}
|
||||
|
||||
private static RuntimeEntityRecord RegisterOracleSnapshot(
|
||||
RuntimeEntityObjectLifetime lifetime,
|
||||
WorldSession.EntitySpawn seed)
|
||||
{
|
||||
RuntimeEntityRecord canonical =
|
||||
lifetime.RegisterEntity(seed).Canonical!;
|
||||
// The pin is only worth anything if the canonical snapshot really is
|
||||
// the same spawn the merge retained.
|
||||
Assert.Equal(seed.MotionTableId, canonical.Snapshot.MotionTableId);
|
||||
Assert.Equal(
|
||||
seed.Physics!.Value.MotionTableId,
|
||||
canonical.Snapshot.Physics!.Value.MotionTableId);
|
||||
return canonical;
|
||||
}
|
||||
|
||||
private static WorldSession.EntitySpawn WithTimestamps(
|
||||
WorldSession.EntitySpawn spawn,
|
||||
ushort? movement = null,
|
||||
|
|
|
|||
|
|
@ -97,6 +97,19 @@ public sealed class RuntimeSteadyStatePositionMergeTests
|
|||
/// Exactly ONE of each, never zero and never two — pinned at the
|
||||
/// observable, not at the source site.
|
||||
///
|
||||
/// <para><b>C5b review D4-round fix (2026-08-05): this test used to pass
|
||||
/// identically with C5b reverted and therefore proved nothing.</b> Its
|
||||
/// only delta assertion was <c>Assert.Single(deltas, d => Rebucketed
|
||||
/// && parentGuid)</c>, which FILTERS — so the pre-C5b world (merge
|
||||
/// publishes Rebucketed, <c>CommitRebucket</c> then early-returns
|
||||
/// publish-less because the cell already matches) and the post-C5b world
|
||||
/// (merge publishes Updated, <c>CommitRebucket</c> publishes Rebucketed)
|
||||
/// both satisfy it. The child arm was equally blind:
|
||||
/// <c>childSpatialBefore + 1UL</c> holds in both worlds because whichever
|
||||
/// site does NOT move the cell propagates idempotently. The discriminating
|
||||
/// observable is the TOTAL ordered parent delta stream:
|
||||
/// <c>[Rebucketed]</c> before, <c>[Updated, Rebucketed]</c> after.</para>
|
||||
///
|
||||
/// <para>Both parent classes per the #319 discipline: a test population
|
||||
/// that only ever sees creature parents is blind to the player class.
|
||||
/// </para>
|
||||
|
|
@ -138,11 +151,33 @@ public sealed class RuntimeSteadyStatePositionMergeTests
|
|||
Assert.True(lifetime.CommitRebucket(parent, OtherCell, LandblockSentinel));
|
||||
|
||||
Assert.Equal(OtherCell, parent.FullCellId);
|
||||
RuntimeEntityDelta rebucketed = Assert.Single(
|
||||
deltas,
|
||||
d => d.Change is RuntimeEntityChange.Rebucketed
|
||||
&& d.Entity.Identity.ServerGuid == parentGuid);
|
||||
Assert.Equal(OtherCell, rebucketed.Entity.CellId);
|
||||
|
||||
// THE discriminating assertion: the complete ordered parent stream.
|
||||
// The merge publishes Updated (it moved no cell); the prologue
|
||||
// rebucket publishes the Rebucketed. Reverting C5b's
|
||||
// `refreshPosition: false` collapses this to the single-element
|
||||
// [Rebucketed] and this assertion fails on both count and order.
|
||||
List<RuntimeEntityDelta> parentDeltas = deltas
|
||||
.Where(d => d.Entity.Identity.ServerGuid == parentGuid)
|
||||
.ToList();
|
||||
Assert.Equal(
|
||||
new[]
|
||||
{
|
||||
RuntimeEntityChange.Updated,
|
||||
RuntimeEntityChange.Rebucketed,
|
||||
},
|
||||
parentDeltas.Select(d => d.Change));
|
||||
// The Updated half carries the OLD cell alongside the NEW wire
|
||||
// position — a torn pair that did not exist pre-C5b, because pre-C5b
|
||||
// the merge stamped both together. Filed as AP-147.
|
||||
Assert.Equal(Cell, parentDeltas[0].Entity.CellId);
|
||||
Assert.Equal(
|
||||
OtherCell,
|
||||
parentDeltas[0].Entity.Position!.Value.ObjCellId);
|
||||
Assert.Equal(OtherCell, parentDeltas[1].Entity.CellId);
|
||||
Assert.Equal(
|
||||
OtherCell,
|
||||
parentDeltas[1].Entity.Position!.Value.ObjCellId);
|
||||
|
||||
// #319's child-equality channel: exactly one propagation, and the
|
||||
// child lands on the parent's new cell.
|
||||
|
|
|
|||
|
|
@ -1564,6 +1564,83 @@ public sealed class RuntimeSetPositionStateTests
|
|||
Assert.Equal(body.CellPosition.ObjCellId, record.FullCellId);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// C5b review L5 (2026-08-05). C5b kept the merge's
|
||||
/// <c>beforeCell != canonical.FullCellId ? Rebucketed : Updated</c>
|
||||
/// ternary alive on the argument that the
|
||||
/// <c>Forget(restoreCancelledPark: true)</c> above it can produce a real
|
||||
/// 0 -> N cell edge INSIDE <c>TryApplyPosition</c>. Every
|
||||
/// <c>restoreCancelledPark</c> test until now called <c>Forget</c>
|
||||
/// directly, so the retained arm had zero coverage through the production
|
||||
/// entry point and the argument was unpinned. This drives the real merge.
|
||||
///
|
||||
/// <para>The wire cell is deliberately the SOURCE cell while the park's
|
||||
/// committed body cell is the DESTINATION: the restored residency — and
|
||||
/// therefore the <c>Rebucketed</c> delta's value — can only have come
|
||||
/// from the park rollback, never from the wire packet, which C5b withholds
|
||||
/// (<c>RuntimeSetPositionState.cs</c>'s <c>RestoreParkWithdrawal</c> writes
|
||||
/// <c>SetFullCell</c> only while <c>FullCellId == 0u</c>, which is exactly
|
||||
/// the state <c>ParkDeferred</c> left behind).</para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void AcceptedPositionCancellingWakeableParkPublishesRebucketedThroughTheMerge()
|
||||
{
|
||||
PhysicsEngine engine = FlatEngine(SourceLandblock, 0f);
|
||||
using var lifetime = new RuntimeEntityObjectLifetime(engine);
|
||||
var generation = new RuntimeGenerationToken(1UL);
|
||||
lifetime.BindEventContext(() => generation, static () => 1UL);
|
||||
const uint guid = 0x7000411Fu;
|
||||
RuntimeEntityRecord record = CreateRecord(lifetime, guid, 1);
|
||||
PhysicsBody body = AttachBody(lifetime, record, SourceCell);
|
||||
|
||||
RuntimeSetPositionOutcome parked = lifetime.Physics.SetPosition.Apply(
|
||||
record,
|
||||
record.PositionAuthorityVersion,
|
||||
Command(CrossLandblockRequest()));
|
||||
Assert.Equal(RuntimeSetPositionStatus.DeferredCell, parked.Status);
|
||||
Assert.Equal(0u, record.FullCellId);
|
||||
Assert.Equal(DestinationCell, body.CellPosition.ObjCellId);
|
||||
|
||||
var observer = new EntityObserver();
|
||||
using IDisposable subscription = lifetime.Events.Subscribe(observer);
|
||||
|
||||
Assert.True(lifetime.TryApplyPosition(
|
||||
new WorldSession.EntityPositionUpdate(
|
||||
guid,
|
||||
new CreateObject.ServerPosition(
|
||||
SourceCell, 11f, 21f, 7f, 1f, 0f, 0f, 0f),
|
||||
Velocity: null,
|
||||
PlacementId: null,
|
||||
IsGrounded: true,
|
||||
InstanceSequence: 1,
|
||||
PositionSequence: 2,
|
||||
TeleportSequence: 0,
|
||||
ForcePositionSequence: 0),
|
||||
isLocalPlayer: false,
|
||||
forcePositionRotation: null,
|
||||
currentLocalVelocity: null,
|
||||
acknowledgeProjection: null,
|
||||
out PositionTimestampDisposition disposition,
|
||||
out _,
|
||||
out _));
|
||||
Assert.Equal(PositionTimestampDisposition.Apply, disposition);
|
||||
|
||||
// The park rolled back inside the merge: 0 -> the body's committed
|
||||
// destination cell, which is NOT the wire cell.
|
||||
Assert.Equal(DestinationCell, record.FullCellId);
|
||||
Assert.NotEqual(SourceCell, record.FullCellId);
|
||||
Assert.True(body.InWorld);
|
||||
|
||||
// THE assertion the retained ternary owns. Collapsing it to a constant
|
||||
// Updated turns this red; the wire-cell withhold cannot rescue it,
|
||||
// because the edge came from the rollback.
|
||||
RuntimeEntityDelta delta = Assert.Single(
|
||||
observer.Deltas,
|
||||
d => d.Entity.Identity.ServerGuid == guid);
|
||||
Assert.Equal(RuntimeEntityChange.Rebucketed, delta.Change);
|
||||
Assert.Equal(DestinationCell, delta.Entity.CellId);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The sibling cancel entry point. Route 2's controller and route 4b's
|
||||
/// remote controller both cancel a <c>DeferredCell</c> park through
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue