Ports retail's missile Position handling into the canonical Runtime
placement owner instead of the deleted ApplyAuthoritativePosition
short-circuit. The Create/residence-window halves of the projectile
pipeline (RuntimeProjectile binding, TryBind's adopted-body branch,
the collision/shadow registration) were already canonical from prior
slices; this closes the remaining gap — how an ACCEPTED Position for
an in-flight missile is classified, placed, and presented.
Byte-decode (Step 1 hard gate, before any code was written):
CPhysicsObj::MoveOrTeleport @0x00516330-0x00516438 disassembled from
the PDB-paired binary (Capstone, x86 32-bit thiscall). `ret 0x10`
establishes four stack args; [esp+0x7c] (arg5, the velocity pointer)
is never referenced in any of the three branches (teleport/near/far).
The retail reviewer independently reproduced this by searching the
whole function body for the `24 7c` mod/rm+disp8 encoding a
`[esp+0x7c]` read would require and found zero occurrences. This
retired a fabricated `?? Vector3.Zero` fallback in the deleted method
— retail's PositionPack::UnPack initializes an absent velocity to
zero and MoveOrTeleport never installs it; the projectile's Vector
channel (RuntimeProjectilePhysicsUpdater.ApplyAuthoritativeVector)
remains the sole velocity authority for a missile. D-P5 in the
contract; the Runtime seam commits no velocity from the Position
packet at all.
The unbound-missile fix: RuntimeEntityObjectLifetime's
ClassifyRemoteAcceptedPosition now derives ProjectileAuthoritative
from a CONJUNCTIVE predicate — the Missile bit AND a bound
RuntimeProjectile whose Body is the canonical PhysicsBody — never the
bit alone. Retail places every non-player CPhysicsObj unconditionally
(there is no missile-specific placement gate in MoveOrTeleport or its
callers), so an unbindable or not-yet-bound missile taking the
ordinary remote tail is retail-faithful, not a fallback: the earlier
bit-only discriminator would have silently frozen it instead.
AP-141 records this as a deliberate, recorded divergence, not
fidelity. Retail mechanically WOULD arm a missile's ConstrainTo leash
on any nonzero MoveOrTeleport return: HandleReceivedPosition
@0x00453FD0's only kind test is player-vs-not, ConstrainTo
@0x00454272 has no kind test of its own, and CPhysicsObj::ConstrainTo
@0x00510520 creates a PositionManager on demand via
MakePositionManager @0x00510523 if one doesn't exist. acdream
deliberately does not construct that EntityPhysicsHost/
PositionManager/InterpolationManager chain for a ballistic body — the
route-5b split the C4 route 5 contract rejected — so a live missile
never shows an armed leash and never catches up via the near/
UnroutedCatchUp policy. This divergence is safe specifically because
ACE never sends UpdatePosition for a missile
(references/ACE/Source/ACE.Server/WorldObjects/WorldObject_Tick.cs:
333-334, SendUpdatePosition() commented out inside the
PhysicsState.Missile branch at :265) — every half of this row is
deterministic-test-gated only, never exercised against a real server.
AP-141 also records the surviving ConstrainTo re-anchor divergence
under clause (b): for the adopted-body case (TryBind's shared-body
branch — an ordinary remote whose Missile bit is set by a later
State packet, so it still carries a live RemoteMotion), acdream now
ports retail's teleport-branch and far-branch StopInterpolating
action (Interp.Clear()), but never re-arms or re-anchors the
inherited ConstrainTo leash the way retail's HandleReceivedPosition
@0x00454254/@0x00454272 does on every nonzero return. The risk
column's earlier wording — that a stale leash "would drag the body
toward a stale anchor" — was wrong and is retracted in this same
commit: ConstraintManager.ConstraintPos is write-only in both retail
and the port (never read by AdjustOffset), and
ConstraintManager::adjust_offset @0x00556180 only tapers or zeroes an
already-composed per-tick offset while InContact — a leash brakes
motion the interp/sticky chain already produced, it cannot pull
anything toward the anchor. The real residual is one tick of un-reset
brake accumulator, contact-gated, and it cannot move an airborne
far-snapped missile at all (the clamp branch does not run while
airborne).
NO CONNECTED GATE EXISTS for this route, by design: ACE never sends a
missile UpdatePosition (see above), so retail's own server never
exercises this code path in play. Every proof obligation here is
test-gated only — Runtime and App-level fixtures constructing the
packet directly — never a live client/server capture.
Three review rounds closed 8 MAJOR findings before this landed:
round 1 (A1 App discarded the seam's status; A2/R1 silent swallow on
an unbound missile; A3/R2 the adopted-body teleport_hook never
wired; A4/A5 zero Runtime/App test coverage); round 2 (a
ParentCellId regression introduced by round 1's own R6 finding,
which the retail reviewer retracted the following round as factually
wrong — the fix here is the REVERT to record.FullCellId, not the
relocation round 1 shipped; B2 the far-branch StopInterpolating skip
never extended to the adopted-body case; residual App/Runtime store-
path coverage; a per-packet closure contradicting the file's own
#315 cached-delegate pattern). Round 3 closed on coverage alone (no
defect): the Advance() retry arm's projectile branch — added at
round 2, semantically reordered at round 2's B5 fix (skip prediction
invalidation on a re-parked Contention, since it writes nothing) —
had never been executed by any test; two new tests drive it directly
and are sabotage-verified against both the reordering and the
retry-arm's own SyncProjectilePresentation call site. The one
recorded defect this campaign produced (the ParentCellId regression)
was caused by complying with a review finding that its own author
later retracted — the standing lesson recorded for future rounds is
that review findings are evidence to re-verify against the code, not
commands to obey unconditionally.
Complete Release suite: 11,063 passed / 4 skipped / 0 failed
(baseline 11,036 at 30d3d114, +27 new tests across this campaign).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
465 lines
26 KiB
Markdown
465 lines
26 KiB
Markdown
# C4 route 5 — architecture / adversarial review (2026-08-04)
|
|
|
|
**Verdict: FAIL.**
|
|
|
|
Scope reviewed: the uncommitted working tree at HEAD `30d3d114`, branch
|
|
`claude/acdream-physics-divergence-5aa784` — `git diff HEAD` over 8 files plus
|
|
the untracked `tests/AcDream.Runtime.Tests/Entities/RuntimeProjectilePositionKindTests.cs`.
|
|
`docs/research/2026-08-04-c4-route-5-contract.md` is the contract, not under
|
|
review. Line numbers below are as-of the working tree and will go stale; every
|
|
citation also names the symbol (process rule 6).
|
|
|
|
Independent verification performed for this review: `dotnet build AcDream.slnx -c Release`
|
|
(succeeded, 0 warnings / 0 errors), focused Runtime suites 58/58 green, focused
|
|
`ProjectileControllerTests` 41/41 green. The complete Release suite was NOT run
|
|
here.
|
|
|
|
The design is sound and the Runtime seam is largely a faithful, well-argued
|
|
reproduction. The FAIL rests on four things: two real defects in the ~35 lines
|
|
of App dispatch glue (A1, A2), one pinned contract obligation left unwired with
|
|
a concrete failure scenario (A3), and the fact that the presentation invariant
|
|
(§5 item 2, "#312's layer — tests must assert it") and the entire App dispatch
|
|
have **zero** test coverage (A4, A5) — which is also the direct answer to the
|
|
scoping-gap question at the end.
|
|
|
|
---
|
|
|
|
## MAJOR findings
|
|
|
|
### A1 — the App projectile ack ignores the seam's status and writes cell identity on the outcomes the design pins as "write nothing"
|
|
|
|
**Severity: MAJOR.**
|
|
`src/AcDream.App/Physics/LiveEntityNetworkUpdateController.cs:2137-2144`
|
|
(`OnPosition`, the D-P6 projectile arm):
|
|
|
|
```csharp
|
|
if (earlyRemoteRoute is { } route)
|
|
{
|
|
_remotePlacementDrive.ApplyAcceptedProjectilePosition(
|
|
acceptedPositionCanonical,
|
|
route);
|
|
_projectileController?.SyncPresentationFromResolvedBody(
|
|
acceptedPositionRecord);
|
|
}
|
|
return;
|
|
```
|
|
|
|
The returned `RuntimeRemotePlacementExecutionStatus?` is discarded. The Runtime
|
|
half is deliberately selective — `RuntimeRemotePlacementDriveController.ApplyAcceptedProjectilePosition:975-983`
|
|
skips `SyncProjectilePresentation` for `Deferred`/`RejectedByPlacement`
|
|
("Invariant 2: presentation advances on every committed/stored outcome only")
|
|
and returns `null` without touching anything for `Interpolate`,
|
|
`NoPositionOperation`, `RejectedAuthority`, `RejectedData`, and every ownership
|
|
mismatch. App re-projects presentation for **all** of them.
|
|
|
|
Concrete failure scenario (the near no-op, D-P2's pinned `Interpolate` row):
|
|
|
|
1. An in-flight arrow's canonical body sits in cell A at world P_A.
|
|
2. A grounded, <96 m accepted Position for cell B arrives.
|
|
`RuntimeEntityObjectLifetime.TryApplyPosition` merges it; the merge's
|
|
`RefreshDerivedState` → `SetFullCell` (`RuntimeEntityRecord.cs:230-251`)
|
|
stamps `canonical.FullCellId = B` **before** any classification runs.
|
|
3. The classifier returns `Interpolate` — a pinned no-op. The Runtime seam
|
|
writes nothing: body stays at cell A / P_A, prediction version unchanged.
|
|
4. App nevertheless runs `ProjectileController.SyncPresentationFromResolvedBody:505-520`,
|
|
which does `entity.SetPosition(runtime.Body.Position)` (= P_A, correct) and
|
|
`entity.ParentCellId = record.FullCellId` (= **B**, wrong — `LiveEntityRecord.FullCellId`
|
|
is `Canonical.FullCellId`, `LiveEntityRuntime.cs:189-191`).
|
|
|
|
The render entity is now parented into a cell it is not geometrically inside,
|
|
with a position from the other cell. If B is an indoor EnvCell the arrow has not
|
|
entered, the arrow renders through the wall or is culled. The same write happens
|
|
on `RejectedByPlacement`, where the Runtime comment says in as many words that
|
|
the body must be left "at its prior (already-synced) pose".
|
|
|
|
This is a genuine regression, not parity: the deleted `ApplyAuthoritativePosition`
|
|
had no disposition concept — it always moved the body to the wire cell *and*
|
|
wrote the same cell into the entity, so position and cell were always
|
|
consistent. Splitting the body write out by disposition without splitting the
|
|
presentation write out with it is what creates the divergence.
|
|
|
|
**Fix direction:** return the status through the App boundary and call
|
|
`SyncPresentationFromResolvedBody` only on the outcomes `SyncProjectilePresentation`
|
|
itself covers (non-null status that is neither `Deferred` nor
|
|
`RejectedByPlacement`) — or move the render-entity projection into the same
|
|
gate inside the Runtime seam and have App acknowledge, not decide.
|
|
|
|
---
|
|
|
|
### A2 — a record carrying the Missile bit with no bound `RuntimeProjectile` now has every accepted Position silently dropped, where it previously fell through to the remote tail
|
|
|
|
**Severity: MAJOR (silent, permanent).**
|
|
Discriminator: `LiveEntityNetworkUpdateController.cs:2122-2146` (`isMissilePacket`);
|
|
refusal: `RuntimeRemotePlacementDriveController.ApplyAcceptedProjectilePosition:930-936`
|
|
(`record.Projectile is not RuntimeProjectile projectile` → `return null`).
|
|
|
|
`isMissilePacket` keys purely on the canonical record's Missile bit. It does not
|
|
require that a projectile component exists. The old short-circuit did: the
|
|
deleted `ProjectileController.ApplyAuthoritativePosition` returned `false` when
|
|
`TryGetCurrent` found no bound `RuntimeProjectile`, and `== true` failing meant
|
|
the packet **fell through** to the generic remote tail.
|
|
|
|
`record.Projectile` has exactly one producer: `ProjectileController.TryBind` →
|
|
`LiveEntityRuntime.BindProjectileRuntime` → `RuntimePhysicsState.BindProjectile`
|
|
(verified by grep — no other writer of `Entities.SetProjectile(record, component)`).
|
|
`TryBind` has several ordinary, non-exceptional failure returns, e.g.
|
|
`ProjectileController.cs:160-166`:
|
|
|
|
```
|
|
"Missile 0x… Setup 0x… does not have the supported retail one-sphere collision shape."
|
|
```
|
|
|
|
Scenario: ACE spawns a missile whose Setup does not reduce to the supported
|
|
one-sphere shape (a diagnosed, expected case — it has its own log line).
|
|
`TryBind` returns false; `record.Projectile` stays null; `FinalPhysicsState &
|
|
Missile` stays set. Every subsequent accepted Position for that guid now returns
|
|
at `:2146` having done nothing at all — no `TryApplyGenericRemoteRenderPose`, no
|
|
`RebucketLiveEntity`, no `GetOrCreateRemoteMotionRuntime`. The object freezes at
|
|
its create pose for the rest of its life and never follows the server. Before
|
|
this diff it tracked, via the remote tail.
|
|
|
|
The contract's D-P6 does pin "dispatch the projectile arm and RETURN", so this is
|
|
a defect in the design as implemented rather than a slip against it — but it is
|
|
still a new, silent, permanent freeze, and the same defect class (`the frozen
|
|
entity`) that the 4b-family reviews have been hunting. Reachability caveat,
|
|
stated honestly: ACE never sends a missile `UpdatePosition`
|
|
(`WorldObject_Tick.cs:333-334`), so like every other half of this route it is
|
|
test-reachable only.
|
|
|
|
**Fix direction:** make the discriminator conjunctive — a packet takes the
|
|
projectile arm only when the arm can actually own it (Missile bit **and** a
|
|
bound projectile whose `Body` is the canonical `PhysicsBody`); otherwise let it
|
|
take the ordinary tail, which is what the record's shape actually is. If the
|
|
swallow is intended, it needs its own register row and a test that pins it, and
|
|
neither exists.
|
|
|
|
---
|
|
|
|
### A3 — D-P4's teleport-hook reduction is unwired for the adopted-body case, which the codebase explicitly supports
|
|
|
|
**Severity: MAJOR (contract obligation unmet; concrete scenario).**
|
|
`RuntimeRemotePlacementDriveController.ApplyAcceptedProjectilePosition:944`
|
|
runs exactly one of retail's six `teleport_hook` @0x00514ED0 actions:
|
|
|
|
```csharp
|
|
case RuntimeAuthoritativePositionDisposition.SetPosition:
|
|
_entityObjects.Physics.CollisionReports.LeaveWorld(record);
|
|
```
|
|
|
|
The contract pins more than that: *"The other five actions execute iff their
|
|
owning component exists — for the adopted-body case (a missile that also carries
|
|
a `RemoteMotion`), the existing hook actions' per-manager guards already express
|
|
retail's shape; the implementer wires this without per-packet closures"* (D-P4).
|
|
Nothing wires them, and no note records the omission.
|
|
|
|
The adopted-body case is not hypothetical. `ProjectileController.TryBind`'s
|
|
shared-body branch exists precisely for it — *"If a non-missile incarnation
|
|
already created its MovementManager or entered the Physics-Static animation
|
|
workset, classification adopts that same body"* — and
|
|
`RuntimePhysicsState.BindProjectile:757-830` does not exclude a record that has
|
|
a `RemoteMotion`. `RuntimePhysicsState.cs:697-708` shows the same key can sit in
|
|
both `_spatialRemotes` and `_spatialProjectiles`.
|
|
|
|
Scenario: an object is a live remote with a populated `RemoteMotion.Interp`
|
|
queue and a `ConstrainTo` leash armed by route 4a's
|
|
`RuntimeRemoteSteadyStatePosition.TryArmConstraintAfterOperation`. ACE then sets
|
|
the Missile bit (a State packet); `TryBind` adopts the shared body. The next
|
|
teleport/cell-less Position takes the projectile arm: collisions are force-ended
|
|
and the body is teleported, but `StopInterpolating`, `UnConstrain`,
|
|
`CancelMoveTo`, and `UnStick` never run. The `RemoteMotion` survives in the
|
|
remote workset with a stale waypoint and a live leash anchored at the
|
|
pre-teleport position, and drags the newly-placed missile body back. Retail
|
|
@0x00514EFD/@0x00514F31 runs all six, each guarded on its own manager, so
|
|
retail's own guards would no-op for a pure arrow and would fire here.
|
|
|
|
Note this is not a regression from HEAD — the old short-circuit did not run the
|
|
hook either — but D-P4 made wiring it an explicit obligation of this slice, and
|
|
the faithful port (`RemoteTeleportHook.Execute` + `RemoteTeleportHookActions`,
|
|
`src/AcDream.App/Physics/RemoteTeleportHook.cs`) is sitting in-tree unused for
|
|
this arm. That is exactly the 4b-3 round-2 R2 defect class the contract cited by
|
|
name.
|
|
|
|
**Fix direction:** drive the projectile teleport arm through the same six-action
|
|
bundle (cached delegates, #315 pattern), letting each action's per-component
|
|
guard decide — for an ordinary arrow all five extra actions are no-ops, and the
|
|
adopted-body case gets retail's shape for free.
|
|
|
|
---
|
|
|
|
### A4 — `SyncProjectilePresentation`, the highest-risk reproduced code in the diff, has no test that would fail if it were deleted
|
|
|
|
**Severity: MAJOR (test gap on the invariant the contract singles out).**
|
|
`RuntimeRemotePlacementDriveController.SyncProjectilePresentation:987-1039`.
|
|
|
|
Contract §5 invariant 2: *"Presentation still advances… the shadow registry is
|
|
synced (spatial+visible) or suspended (hidden/non-spatial) exactly per the
|
|
current tail's semantics. A projectile is never left rendered a packet behind
|
|
its body (#312's layer — **tests must assert it**)."*
|
|
|
|
Across all seven new Runtime tests, the only assertion that even touches this
|
|
method is `Assert.True(body.InWorld)` in
|
|
`ApplyAcceptedProjectilePosition_Refused_StillAdvancesPoseNoParkPredictionInvalidated`.
|
|
That assertion is vacuous with respect to the sync: the fixture's `AttachBody`
|
|
(`RuntimeRemotePlacementDriveControllerTests.cs:2860-2880`) calls
|
|
`body.SnapToCell(...)`, and `PhysicsBody.SnapToCell` sets `InWorld = true`
|
|
(`PhysicsBody.cs:201-205`). Nothing in the Refused path clears it. So the
|
|
assertion passes with `SyncProjectilePresentation` entirely removed.
|
|
|
|
Unasserted, at any layer: the shadow-registry publication
|
|
(`ShadowObjects.UpdatePosition`), the spatial+hidden `Suspend`, the non-spatial
|
|
`InWorld = false` + `Active` clear + `Suspend`, the `!body.InWorld` activation
|
|
edge, and the three currency guards at the method's head. The committed-outcome
|
|
tests (`…TeleportCommit…`, `…FarCommit…`) assert status, position, prediction
|
|
version, velocity, `RemoteMotion is null`, and collision-owner count — none of
|
|
which the sync produces.
|
|
|
|
**Fix direction:** one test per branch of the sync, keyed on the observable the
|
|
old tail produced: shadow row present at the resolved pose (spatial+visible),
|
|
`Suspend` called (spatial+hidden and non-spatial), `Active` cleared
|
|
(non-spatial). These are cheap; the fixture already registers shadows in
|
|
`SeedCollisionOwner`.
|
|
|
|
---
|
|
|
|
### A5 — no App-layer test exercises `OnPosition` with a missile packet at all; §7 items 8 and 9 are unwritten
|
|
|
|
**Severity: MAJOR (this is the scoping-gap answer).**
|
|
|
|
`grep -rn "Missile" tests/AcDream.App.Tests/Physics/LiveEntityNetwork*.cs`
|
|
returns **zero** hits. The only App file mentioning `Missile` at all is
|
|
`ProjectileControllerTests.cs`, which never calls `OnPosition`.
|
|
|
|
So none of the following is covered anywhere:
|
|
|
|
- the D-P1/D-P6 discriminator itself in its production call site;
|
|
- invariant 8's mutual exclusion (Missile-set ⇒ projectile arm and **no
|
|
`RemoteMotion` afterward**; Missile-clear ⇒ remote tail and no projectile
|
|
effect);
|
|
- the null-classification swallow (`earlyRemoteRoute is null` with the Missile
|
|
bit set) and its positive half (timestamps consumed, merge advanced);
|
|
- the "no early wire-pose write / no `RebucketLiveEntity`" pins;
|
|
- the presentation ack the App owns — i.e. #312's layer at the layer it renders.
|
|
|
|
Worse, the retired App test `MalformedFreshUpdates_DoNotPoisonCanonicalBodyOrPose`
|
|
justifies its Position half's deletion by pointing at *"LiveEntityNetworkUpdateController's
|
|
own 'invalid-payload swallow' test"*. **That test does not exist.** The
|
|
underlying claim (the shared `CanAcceptPositionPayload` gate rejects the payload
|
|
upstream) is correct and I verified it independently — but the cited successor
|
|
coverage is not there, so the assertion was retired against a coverage claim
|
|
that is false.
|
|
|
|
Both A1 and A2 live inside the ~35 lines the implementer argued were "thin glue
|
|
between two well-tested layers". That is the empirical refutation of the
|
|
argument.
|
|
|
|
---
|
|
|
|
## MINOR findings
|
|
|
|
### A6 — the `!body.InWorld` gate is read after the placement, where the deleted tail captured `wasInWorld` before it
|
|
|
|
`SyncProjectilePresentation:1012-1018` reads `body.InWorld` *after*
|
|
`TryExecuteAcceptedRemotePosition` has run. The canonical commit calls
|
|
`body.SnapToCell(...)` (`RuntimeSetPositionState.cs:4974`), which sets
|
|
`InWorld = true`. The deleted tail captured `bool wasInWorld = body.InWorld;`
|
|
**before** its `SnapToCell`. Consequence: on every committed outcome the
|
|
re-activation branch (`body.LastUpdateTime = clock` + `TransientState |= Active`)
|
|
is now dead — a projectile that had left the world and comes back through a
|
|
committed accepted Position is marked `InWorld` but never re-flagged `Active`,
|
|
and its legacy `LastUpdateTime` is not rebased. It self-heals on the next tick
|
|
because `RetailObjectActivityGate.Evaluate` re-sets the flag
|
|
(`RetailObjectActivityGate.cs:61-67`), which is the only reason this is MINOR
|
|
rather than a frozen body. Fix: capture `body.InWorld` before dispatching the
|
|
placement and pass it in.
|
|
|
|
### A7 — the spatial+hidden branch silently drops `body.LastUpdateTime = currentTime`
|
|
|
|
Deleted tail: `else if (spatial) { body.InWorld = true; body.LastUpdateTime = currentTime; Suspend(); }`.
|
|
New (`SyncProjectilePresentation:1031-1035`): the `LastUpdateTime` write is gone.
|
|
`ProjectileController.TryBind`'s equivalent branch documents why the write
|
|
exists — *"consume the hidden clock so UnHide cannot replay a time backlog"*.
|
|
Bounded by `TryBegin` refusing `Hidden` and by the `RetailObjectQuantumClock`
|
|
being canonical post-R6, but it was a deliberate write and its removal is
|
|
unremarked.
|
|
|
|
### A8 — the shadow publication is silently skipped when the Runtime world frame is unavailable
|
|
|
|
`SyncProjectilePresentation:1019-1030` wraps the shadow update in
|
|
`if (physics.TryGetWorldFrameOffset(...))` and does nothing on false. The deleted
|
|
`ShadowPositionSynchronizer.Sync` always published, using App's live centre, and
|
|
returned early only for `cellId == 0`. `TryGetWorldFrameOffset` additionally
|
|
returns false when `_worldFrameCenterLandblockId == 0`
|
|
(`RuntimePhysicsState.cs:613-632`). Two methods away,
|
|
`StoreAcceptedDestinationPose:1113-1119` treats exactly this case as
|
|
`ThrowIfWorldFrameUnreachable` — #284's "a frame that can never arrive is
|
|
terminal, never silent" policy. The new code neither publishes nor escalates.
|
|
|
|
### A9 — the null-route fallback is not fenced off the local player
|
|
|
|
`LiveEntityNetworkUpdateController.cs:2122-2126`: when `earlyRemoteRoute` is
|
|
null, `isMissilePacket` falls back to a bare `FinalPhysicsState & Missile` test.
|
|
`earlyRemoteRoute` is *always* null for `update.Guid == _playerServerGuid`. A
|
|
local-player record that ever carried the Missile bit would therefore swallow
|
|
its own accepted Position and freeze the player. Currently impossible only
|
|
because ACE never sets Missile on a player — a one-token `update.Guid != _playerServerGuid &&`
|
|
would make it structurally impossible.
|
|
|
|
### A10 — `OwnsFarSnap`'s doc is now false in the kind dimension
|
|
|
|
`src/AcDream.Runtime/Physics/RuntimeRemoteFarSnapPosition.cs:78-79` still states
|
|
this predicate is *"a strict narrowing of `OwnsPlacement` to its far half"*.
|
|
After the widening it narrows only `OwnsPlacement`'s **remote** far half.
|
|
`RuntimeRemoteTeleportPosition.cs:33-35` hedges correctly ("`OwnsPlacement`'s
|
|
remote scope"); the far-snap doc does not. D-P3 asked for exactly this
|
|
correction where a comment would otherwise mislead (process rule 6).
|
|
|
|
### A11 — the cutover route ledger still lists the deleted methods as live routes
|
|
|
|
`docs/research/2026-08-02-cutover-route-inventory.md:666-694` still describes
|
|
`ProjectileController.ApplyAuthoritativePosition` → `RuntimeProjectilePhysicsUpdater.ApplyAuthoritativePosition`
|
|
as a live route with `SnapToCell` + `CommitProjectileCell`. D-P7 scoped the
|
|
re-point grep to `src/` and `docs/architecture/`, so this is outside the letter
|
|
of the obligation — but that file is the C4 route ledger this campaign reads
|
|
from, and it is now wrong about route 5.
|
|
|
|
---
|
|
|
|
## Verified — no finding
|
|
|
|
These were hunted and came back clean; recorded so the next round does not
|
|
re-litigate them.
|
|
|
|
- **P3 (`RuntimeSetPositionState` kind-agnosticism) is CORRECT, and I verified
|
|
it independently rather than accepting the argument.**
|
|
`TryBeginExclusiveAuthoredPlacement:1444-1472` returns `default` (invalid
|
|
token → `Contention`) whenever `_operations.ContainsKey(key)` or
|
|
`HasRetainedCompletion(key)`. Both constructors that stamp
|
|
`Kind = RemoteAuthoritative` — `ParkCollisionResidents`'s window-departure
|
|
park (`:3833`/`:3866`, which itself `continue`s past any record already in
|
|
`_operations`) and `CreateWithdrawalOperation` (`:5338`/`:5350`, reached only
|
|
from `Cancel` after `CancelCoreDeferred`) — install an operation into
|
|
`_operations` under the record's key. A projectile accepted Position arriving
|
|
while one is live cannot capture or be relabelled by it; it refuses with
|
|
`Contention`, which `StoresAcceptedDestination()` puts on the **storing** side,
|
|
so the pose still advances and no ledger column is mislabelled. The two
|
|
kind-conditional stage sites (`IsExactDormantLocalActivationCurrent:5445-5446`,
|
|
`IsDormantLocalActivationPrephaseCurrent:2554-2600`) additionally require
|
|
`InitialLogin`/`LocalAuthoritative` **and** `record.Projectile is null`, and
|
|
the family has zero production callers. Residual, benign and pre-existing: a
|
|
park operation created for a projectile record carries `Kind = RemoteAuthoritative`,
|
|
but it never reaches the drive controller's ledger.
|
|
- **The three ownership fences hold.** `OwnsPlacement` has exactly one
|
|
production reader — `TryExecuteAcceptedRemotePosition:640` (verified by grep
|
|
across `src/`; every other hit is a doc comment or a test). `OwnsFarSnap` and
|
|
`OwnsTeleportPlacement` keep their `RemoteAuthoritative` gate, and a
|
|
projectile route cannot reach `ApplyAcceptedRemoteFarSnap` /
|
|
`ApplyAcceptedRemoteTeleport` / `TryArmConstraintAfterOperation`: the App
|
|
returns before the remote tail whenever `isMissilePacket`, and
|
|
`isMissilePacket` is derived *from* `route.OperationKind` whenever a route
|
|
exists, so the two cannot disagree. The throwing guards stay unreachable, and
|
|
no `RemoteMotion` is ever created on this arm.
|
|
- **The remote route is unchanged.** Moving the `ClassifyRemoteAcceptedPosition`
|
|
call earlier hops only a pure-read guard block
|
|
(`TryGetRecord` / `ReferenceEquals` / `IsCurrentPositionAuthority`) with no
|
|
reentrancy in between; classification itself is side-effect-free. Zero remote
|
|
classifier tests changed expectation, which is the contract's own tripwire.
|
|
- **`StoreAcceptedDestinationPose` never writes a cell**, so on
|
|
`Refused`/`Contention`/`RejectedPreparation` the projectile body's
|
|
`CellPosition` stays at the source cell while `record.FullCellId` is the wire
|
|
cell. This is identical to the accepted remote behaviour (AP-138's residual)
|
|
and the quantum stepper takes the cell from `record.FullCellId` explicitly
|
|
(`RuntimeProjectilePhysicsUpdater.TryBegin`), so it is parity, not a new
|
|
defect. Recorded, not filed.
|
|
|
|
---
|
|
|
|
## Judgment on the four implementer claims
|
|
|
|
**Claim 1 — both `ApplyAuthoritativePosition` overloads and the Runtime updater's
|
|
copy deleted; nothing else called them; no behaviour lost.**
|
|
**Deletions and callers: VERIFIED.** No code reference survives anywhere in
|
|
`src/` or `tests/` (only comments, the contract, and the two research docs noted
|
|
in A11); Release build green with 0 warnings. **"No behaviour lost": PARTIALLY
|
|
FALSE.** Three deliberate behaviours went with them and only one is accounted
|
|
for: the origin-translated world-position finiteness check (genuinely redundant —
|
|
`worldPos` is a finite wire triple plus an integer landblock offset, so it cannot
|
|
be non-finite when `CanAcceptPositionPayload` passed); the `_lastFiniteGameTime`
|
|
rebase on a Position packet (App tick clock — unremarked, low impact); and the
|
|
`wasInWorld` activate edge plus the hidden-branch `LastUpdateTime` write (A6,
|
|
A7 — unremarked).
|
|
|
|
**Claim 2 — 7 call sites modified, 2 assertions retired as obsolete.**
|
|
**Mostly legitimate, with one false coverage claim.** The velocity assertions in
|
|
`FreshVectorAndPositionCorrectionsMutateSameBody` are genuinely obsolete under
|
|
D-P5 and have a real positive successor (`…TeleportCommit…` /
|
|
`…FarCommit…` assert `body.Velocity` bit-identical after a placement). The
|
|
`AuthoritativeMutation.Position` retirement has a genuine, load-bearing successor
|
|
I read and checked —
|
|
`ApplyAcceptedProjectilePosition_DuringOpenQuantum_CompleteAbortsAfterPredictionInvalidated`
|
|
opens a real quantum, runs the arm, and asserts `Complete` returns false with the
|
|
committed pose intact. The rewritten `SyncPresentation_ReentrantGuidReuseNeverTouchesTheReplacement`
|
|
is an improvement, not a dilution: it correctly notes that `RebucketLiveEntity`
|
|
self-suppresses its own guid's visibility callback and switches to a genuine
|
|
spatial edge. **But** the malformed-payload retirement in
|
|
`MalformedFreshUpdates_DoNotPoisonCanonicalBodyOrPose` cites a successor test in
|
|
`LiveEntityNetworkUpdateController` that does not exist (A5). The upstream gate
|
|
does cover the scenario; the citation does not.
|
|
|
|
**Claim 3 — `OwnsPlacement_FalseWhenOperationKindIsNotRemoteAuthoritative`
|
|
legitimately changed its expected outcome.**
|
|
**TRUE, not a test bent to fit the code.** The widening makes
|
|
`ProjectileAuthoritative` a positively-owned kind by construction, so leaving it
|
|
in a "these kinds are refused" list would assert the opposite of the design. The
|
|
positive case is separately and explicitly asserted
|
|
(`OwnsPlacement_TrueForProjectileAuthoritative_…`, including the Create
|
|
exclusion), and the two remaining negatives (`InitialLogin`,
|
|
`LocalAuthoritative`) still pin the predicate's kind dimension. The rename is
|
|
accurate.
|
|
|
|
**Claim 4 — the new tests use airborne destinations to remove a contact-response
|
|
confound.**
|
|
**Legitimate confound removal, with a named residual.** The claim is accurate:
|
|
the shared placement pipeline's contact response is retail landing behaviour,
|
|
shared verbatim with the remote arms, and outside route 5's scope; D-P5's pin is
|
|
specifically "no velocity *from the packet*", which an airborne destination
|
|
isolates cleanly. It is not a test dodging a defect. The residual worth stating:
|
|
**no test now places a projectile into ground contact at all**, so the
|
|
interaction between the projectile arm and contact response — including whether
|
|
the resulting velocity change is the right one for a ballistic body — is
|
|
entirely unexercised. That is a gap, not a dishonesty.
|
|
|
|
---
|
|
|
|
## Judgment on the known scoping gap (§7 item 8, the dual-kind `OnPosition` matrix)
|
|
|
|
**It is a FAIL-level gap. Definite answer: not acceptable.**
|
|
|
|
The implementer's two arguments are (a) the collapse's
|
|
`LiveEntityNetworkOnPositionCollapseMatrixTests` fixture unconditionally
|
|
constructs a `RemoteMotion` a projectile lacks, and (b) the D-P6 dispatch is
|
|
~35 lines of thin glue between two well-tested layers.
|
|
|
|
(a) is a fixture limitation, and a fixture limitation is a reason to extend the
|
|
fixture, not to ship zero coverage. The projectile half of the matrix needs a
|
|
record with the Missile bit, a bound projectile, and **no** `RemoteMotion` —
|
|
which is precisely the assertion the matrix exists to make.
|
|
|
|
(b) is refuted by this diff itself. Two of the five MAJORs above (A1, A2) are
|
|
defects **inside those 35 lines**, and neither is visible from either
|
|
well-tested layer: A1 is an App/Runtime disagreement about which outcomes may
|
|
write presentation, and A2 is a discriminator whose reachable set is wider than
|
|
the arm that consumes it. This is the same shape as 4b-3's three MAJORs — a
|
|
mapping written against one caller's reachable set, and an invariant satisfied
|
|
on one arm only — and a dual-kind matrix was the structural fix there for the
|
|
same reason it is here.
|
|
|
|
Additionally, the gap is broader than §7 item 8 alone: item 9 (the App-level
|
|
invalid-payload swallow) is also unwritten, *and* was cited as existing coverage
|
|
in a retired assertion's justification.
|
|
|
|
The minimum this needs before landing: a dual-kind theory over `OnPosition`
|
|
covering at least far-commit, teleport-commit, near, airborne, and
|
|
null-classification, asserting on the projectile half that no `RemoteMotion`
|
|
exists afterward, no generic wire-pose write occurred, and the render entity's
|
|
position **and** `ParentCellId` agree with the resolved body — that last
|
|
assertion alone would have caught A1.
|