feat(physics): C4 route 2 — ForcePosition through the canonical placement
A local-player ForcePosition had TWO independent writers for one accepted
packet: LocalForcePositionTransaction snapped the physics body
(PlayerMovementController.BlipPosition, a raw SnapToCell with no collision
resolve), while LiveEntityNetworkUpdateController's generic tail separately
wrote position/cell/rotation to the render WorldEntity from the raw wire and
rebucketed it. Two stores, one packet — the divergence class 670f307c fixed on
the remote path. The outbound AutonomousPosition ack also fired BEFORE any
canonical commit existed: we told ACE "got it, I'm here" before deciding where
"here" was, and the trailing isCurrent() could only suppress the continuation,
never recall the packet.
RuntimeAcceptedPositionDriveController is now the one Runtime-owned seam. Both
hosts call the identical TryExecuteAcceptedLocalPosition; App and headless
project the committed result through the existing placement projection sink
(LiveEntityRuntime.TryApplyRuntimePlacementPlace already performed the same
four writes, from committed state rather than a wire guess).
Retail: SmartBox::HandleReceivedPosition @0x00453FD0's FORCE_POSITION branch is
get_heading -> Frame::set_heading -> SmartBox::BlipPlayer @0x00453940 -> stamp
POSITION_TS -> SendPositionEvent @0x00454091 -> return @0x0045409D. BlipPlayer
is CPhysicsObj::SetPositionSimple @0x005162B0 with flags 0x1012
(Teleport|Slide|SendPositionEvent) — a real collision-resolving SetPosition,
not a snap. The pinned classifier already encoded this exactly.
Named behaviour changes:
* The ack is now an OUTPUT of the committed route, fired strictly after the
canonical commit and exactly once per accepted force packet.
* The ForcePosition route no longer re-arms the constraint leash. The force
branch returns at 0x0045409D, ahead of all three ConstrainTo sites
(0x00454272, 0x0045418A, 0x004541EC); the old re-arm cited retail's "Player,
normal" branch, which BlipPlayer is not on. The teleport, CommitPreparedPosition
and first-entry callers legitimately still constrain and are untouched.
* A force correction that terminates WITHOUT committing still sends its
position event and is not retried — retail's BlipPlayer discards
SetPositionSimple's SetPositionError return and acks unconditionally.
A single _pending funnel owns the in-flight placement, deciding on the token's
PositionAuthorityVersion against the record's: equal -> clear; advanced with the
newest accepted event still a force -> re-issue, re-classified; advanced to an
ordinary Apply -> clear, since newer server truth owns that pose. This closes a
double-apply/double-ack and a silently-dropped correction that two earlier
iterations of this slice each introduced.
AD-62 records the residual: a ForcePosition our async collision publication
cannot carry to a committed placement is not re-applied. Retail has no park —
its world is fully resident and its placement synchronous — so the state is
unreachable there. AP-131 is NOT retired; its legacy Position caller is route 4.
Deleted: LocalForcePositionTransaction, PlayerMovementController.BlipPosition,
HeadlessSessionWorldProjection.BlipLocalPlayer.
Gates: complete Release solution 10,858 passed / 4 skipped / 0 failed (baseline
10,844/4/0). Two independent Opus reviews (retail-conformance and
architecture/adversarial) PASS on the final diff after three FAIL rounds; every
intermediate state was fully green, so the suite caught none of the four real
defects. Connected acceptance is NOT run: nothing a user can do makes ACE emit
a ForcePosition without retail's @pklite, which acdream does not implement — see
docs/research/2026-08-03-c4-route-2-visual-gate.md.
Known gap, recorded not claimed: the plan's acceptance item 2 is unmet. The App
double-write check is a source pin, and "the committed projection moves the
render entity" is uncovered at any layer (#292). Filed alongside: #286-#291,
#293-#296.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
22a5c95400
commit
9966b53174
25 changed files with 4292 additions and 195 deletions
291
docs/research/2026-08-03-c4-route-2-implementation-plan.md
Normal file
291
docs/research/2026-08-03-c4-route-2-implementation-plan.md
Normal file
|
|
@ -0,0 +1,291 @@
|
|||
# C4 route 2 — ForcePosition: implementation plan (2026-08-03)
|
||||
|
||||
Executes `docs/research/2026-08-03-c4-route-2-contract.md`. The contract is the
|
||||
WHAT; this is the verified HOW. Every claim below was checked against source or
|
||||
the named retail decomp in this session — do not re-derive them, and do not
|
||||
contradict them without new evidence.
|
||||
|
||||
## 1. Retail truth (verified this session, not inherited)
|
||||
|
||||
`SmartBox::HandleReceivedPosition` @0x00453FD0
|
||||
(`docs/research/named-retail/acclient_2013_pseudo_c.txt:92896`). The
|
||||
FORCE_POSITION branch is the whole route:
|
||||
|
||||
```
|
||||
if (arg2 == player && newer_event(player, FORCE_POSITION_TS, arg9))
|
||||
{
|
||||
if (<force ts is not older>)
|
||||
{
|
||||
get_heading(player);
|
||||
Frame::set_heading(&dest, heading); // 00454068 preserve OUR heading
|
||||
SmartBox::BlipPlayer(this, &dest); // 00454074
|
||||
player->update_times[0] = arg7; // 00454079 stamp POSITION_TS
|
||||
cmdinterp->SendPositionEvent(); // 00454091 ack
|
||||
return; // 0045409d
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`SmartBox::BlipPlayer` @0x00453940 (line 92528) is:
|
||||
|
||||
```
|
||||
distance = Position::distance(&player->m_position, dest);
|
||||
CPhysicsObj::SetPositionSimple(player, dest, 1); // 00453968
|
||||
SmartBox::PlayerPositionUpdated(this, 0, distance); // 00453976
|
||||
```
|
||||
|
||||
`CPhysicsObj::SetPositionSimple` @0x005162B0 (line 284276) with `arg3 != 0`
|
||||
builds `SetPositionStruct` with flags **`0x1012`** and calls
|
||||
`CPhysicsObj::SetPosition`. `0x1012` decodes against
|
||||
`src/AcDream.Core/Physics/PhysicsSetPosition.cs:63-76` as
|
||||
`Teleport(0x002) | Slide(0x010) | SendPositionEvent(0x1000)` — byte-for-byte
|
||||
`RuntimeAuthoritativePositionRouteClassifier.AuthoritativeTeleportFlags`
|
||||
(`RuntimeAuthoritativePositionRouteClassifier.cs:198-200`). **The pinned
|
||||
classifier route is confirmed correct; do not touch it.**
|
||||
|
||||
Three consequences that decide this slice:
|
||||
|
||||
### 1a. ForcePosition is a real SetPosition, not a snap
|
||||
|
||||
Retail runs the full transition with Teleport|Slide. Today's
|
||||
`PlayerMovementController.BlipPosition`
|
||||
(`src/AcDream.Runtime/Gameplay/PlayerMovementController.cs:1923-1937`) is
|
||||
`_body.SnapToCell(...)` — no transition, no collision, no contact plane, no
|
||||
shadow commit, no `FullCellId`/`PlacementCommitVersion` advance. Closing that
|
||||
gap is the point of route 2.
|
||||
|
||||
### 1b. The force branch runs NO ConstrainTo
|
||||
|
||||
Every `CPhysicsObj::ConstrainTo` call in `HandleReceivedPosition` is at
|
||||
0x00454272 (remote MoveOrTeleport tail), 0x0045418A (player teleport-newer
|
||||
branch), and 0x004541EC (player ordinary branch). The force branch returns at
|
||||
0x0045409D, **before all three**. The classifier already encodes this as
|
||||
`ConstrainPhase: None`.
|
||||
|
||||
`BlipPosition` calls `RearmConstraintLeashAtCurrentPosition()` and its comment
|
||||
cites *"retail 'Player, normal' branch"* — a branch `BlipPlayer` is not on.
|
||||
That leash re-arm is an unbacked deviation on this route.
|
||||
|
||||
**Required:** the new route honours `ConstrainPhase: None` — no leash re-arm on
|
||||
ForcePosition. Call it out explicitly in the commit message as a named
|
||||
behaviour change with these addresses, and add it to the connected gate's watch
|
||||
list (#167 was a leash bug; the user's live observation governs). Do not touch
|
||||
`ArmConstraintLeashAtCommittedPlacement` (C3c/AD-42 first-entry) or the
|
||||
teleport/`CommitPreparedPosition` callers — they are on branches that DO
|
||||
constrain.
|
||||
|
||||
### 1c. Heading preservation already happens upstream — do not re-derive it
|
||||
|
||||
Retail replaces the destination heading with the player's current heading
|
||||
BEFORE the SetPosition. Our accepted-position merge already does this via the
|
||||
`forcePositionRotation` argument
|
||||
(`RuntimeLiveEntitySessionController.cs:179`, App's
|
||||
`_authorityGate.TryAcceptPosition(..., _playerController.BodyOrientation, ...)`
|
||||
at `LiveEntityNetworkUpdateController.cs:1050-1052`). Verify it lands in
|
||||
`record.Snapshot` before you build the route request; assert it in a test. The
|
||||
seam must NOT apply a second heading substitution.
|
||||
|
||||
Also noted, NOT in scope: retail's `PlayerPositionUpdated(this, 0, distance)`
|
||||
gates `set_viewer`/`LScape::update_viewpoint` on
|
||||
`distance >= GetAutonomyBlipDistance` (0x004538C0-0x004538E2). We publish the
|
||||
render root unconditionally. File it as a follow-up observation in the closeout
|
||||
note; do not change it here.
|
||||
|
||||
## 2. Verified mechanism map
|
||||
|
||||
| Thing | Location | Note |
|
||||
|---|---|---|
|
||||
| Route classifier (pinned, correct) | `RuntimeAuthoritativePositionRouteClassifier.cs:308` | one production consumer today: `RuntimeInitialCreateContinuationExecutor.cs:1948` |
|
||||
| Begin | `RuntimeSetPositionState.TryBeginExclusiveAuthoredPlacement:1309` | no first-entry-only precondition |
|
||||
| Prepare+submit+commit | `RuntimeSetPositionState.TryPrepareAndSubmitAuthoredPlacement:1658` → `PrepareMover:1522` → `SubmitPreparedPlacementCore:2777` → `Engine.SetPosition:2962` → `CommitCanonical:4411` | |
|
||||
| Deferred wake | `CommitCollisionGeneration:3973` → `RetryDeferred:4192` → `CommitCanonical:4374` | drives parked operations without our help |
|
||||
| **The template to mirror** | `RuntimeFirstEntryDriveController.TryCompleteContinuationPlacement:280-380` | begin/prepare/submit + outcome switch + projection acknowledgement |
|
||||
| Runtime world frame | `RuntimePhysicsState.ObserveLocalWorldFrame:535`; `resolveWorldOffsetFromRuntimeFrame` param | **use it** — satisfies contract §7, one conversion site |
|
||||
| App projector (already exists) | `LiveEntityRuntime.TryApplyRuntimePlacementPlace` (`LiveEntityRuntime.cs:1231+`) | writes `entity.SetPosition(projection.WorldPosition)`, `entity.Rotation`, `entity.ParentCellId = token.ExactCellId`, `RebucketLiveEntity` — from the COMMITTED result |
|
||||
| App sink | `RuntimePlacementPresentationSink.TryApply:67` / `TryPublishPlace:162` | |
|
||||
| Headless sink | `HeadlessRuntimePlacementProjectionSink` | |
|
||||
|
||||
**This is the crux:** `TryApplyRuntimePlacementPlace` already performs, from
|
||||
canonical committed state, exactly the four writes the generic tail performs
|
||||
from raw wire (`LiveEntityNetworkUpdateController.cs:1264-1281`). Deleting the
|
||||
generic tail for the local player is a straight substitution of the committed
|
||||
result for the wire guess — not a loss of function.
|
||||
|
||||
## 3. The two duplicate authorities to delete
|
||||
|
||||
**Graphical** — `src/AcDream.App/Physics/LocalForcePositionTransaction.cs`
|
||||
(whole file) and its single call site
|
||||
`LiveEntityNetworkUpdateController.cs:1113-1129`; plus the generic tail
|
||||
`:1264-1282` **for the local player only** (remotes still need it — that is
|
||||
route 4).
|
||||
|
||||
**Headless** — `RuntimeLiveEntitySessionController.OnPositionUpdated:217-231`'s
|
||||
`ProjectPosition(..., isLocalPlayer: true, ForcePosition)` +
|
||||
`SendImmediatePosition` pair, and
|
||||
`HeadlessSessionWorldProjection.BlipLocalPlayer:707-727`. Headless has no
|
||||
`WorldEntity` in this path, so it has only the first duplicate — but it is a
|
||||
duplicate all the same, and contract §4 requires both hosts on the identical
|
||||
Runtime path.
|
||||
|
||||
## 4. Design
|
||||
|
||||
New Runtime type, modelled directly on `RuntimeFirstEntryDriveController`:
|
||||
|
||||
**`src/AcDream.Runtime/Session/RuntimeAcceptedPositionDriveController.cs`**
|
||||
|
||||
Constructor takes the same collaborators that controller takes
|
||||
(`_entityObjects`, `IPreparedCollisionSource`, the simulation clock,
|
||||
`LocalPlayerOutboundController`, the session accessor, the local-player
|
||||
identity). Follow that file's ctor and null-validation style exactly.
|
||||
|
||||
### Entry point
|
||||
|
||||
```csharp
|
||||
internal RuntimeAcceptedPositionExecutionStatus TryExecuteAcceptedLocalPosition(
|
||||
RuntimeEntityRecord record,
|
||||
in WorldSession.EntityPositionUpdate update,
|
||||
PositionTimestampDisposition disposition,
|
||||
in AcceptedPhysicsTimestamps timestamps,
|
||||
ushort previousTeleportSequence);
|
||||
```
|
||||
|
||||
**Scope this slice to ForcePosition on the live local player.** Any other
|
||||
disposition, any other entity kind, and the not-applicable cases below return
|
||||
`NotApplicable`, and the caller then does exactly what it does today. Route 2
|
||||
must not perturb routes 1/3/4.
|
||||
|
||||
Return `NotApplicable` when:
|
||||
- `disposition is not PositionTimestampDisposition.ForcePosition`;
|
||||
- the record is not the local player;
|
||||
- `record.PhysicsBody is null` (no canonical body → nothing to place);
|
||||
- **an initial-Create residence is still active for the record.** Route 1 owns
|
||||
it: the executor already retains the Position as a tail action
|
||||
(`RuntimeInitialCreateContinuationExecutor.ApplyPositionAction`) and already
|
||||
carries `SendPositionImmediately` (`:717`, `:2576`). Confirm that ack
|
||||
actually fires on that path and say so in the closeout; if it does not, that
|
||||
is a route-1 defect — file it, do not paper over it here.
|
||||
|
||||
### Body
|
||||
|
||||
1. Build `RuntimeAcceptedPositionRouteRequest` deriving every field the way
|
||||
`RuntimeInitialCreateContinuationExecutor.ApplyPositionAction:1907-1946`
|
||||
derives it. Specifically: `HasContact = update.IsGrounded` (the wire bit,
|
||||
never a live body query); `HasAnimations` from
|
||||
`Snapshot.MotionTableId ?? Snapshot.Physics?.MotionTableId` non-zero;
|
||||
`CommittedCellId = record.FullCellId`; `PlacementFacts` from
|
||||
`record.FinalPhysicsState` and `Snapshot.SetupTableId is not null`;
|
||||
`Source = PositionEvent`; `UsePositionFromServer` and `PlayerDistance` from
|
||||
the same Runtime owners C0 established (`RuntimeCharacterState.AutonomyLevel
|
||||
!= 2`; the live movement controller). Read C0's notes in
|
||||
`docs/plans/2026-08-02-placement-cutover.md` first.
|
||||
2. `ClassifyAcceptedPosition`. Not accepted → stamp-only, mirroring
|
||||
`:1950-1984`; return without ack.
|
||||
3. `TryBeginExclusiveAuthoredPlacement(record, record.PositionAuthorityVersion,
|
||||
route.OperationKind)`. Invalid token → `Contention`; the caller retries on a
|
||||
later packet. Do NOT invent a retry loop.
|
||||
4. `TryPrepareAndSubmitAuthoredPlacement(record, token, route.OperationKind,
|
||||
route.SetPositionFlags, _collisionSource, _clock.SimulationTimeSeconds,
|
||||
out outcome, resolveWorldOffsetFromRuntimeFrame: true)`.
|
||||
5. Outcome switch, mirroring `TryCompleteContinuationPlacement:340-380`:
|
||||
- `CommittedHostAcknowledgementPending` → §4a reconcile, then §4b ack.
|
||||
- `DeferredCell` → §4c.
|
||||
- anything else → forget + `PublishCancellation`; **no ack**.
|
||||
|
||||
### 4a. Post-commit local reconciliation
|
||||
|
||||
`CommitCanonical` writes the body, contact plane, `FullCellId`,
|
||||
`PlacementCommitVersion`, shadow membership and the spatial acknowledgement. It
|
||||
does NOT do the three controller-local things `BlipPosition` also did. Add ONE
|
||||
new method to `PlayerMovementController` next to `BlipPosition`, e.g.
|
||||
`CommitCanonicalForcePositionFrame()`, that:
|
||||
|
||||
- resets `_prevPhysicsPos`/`_currPhysicsPos` to the committed body position
|
||||
(kills the render-lerp residual);
|
||||
- calls `UpdateCellId(_body.CellPosition.ObjCellId, "force-position")` so the
|
||||
render root chokepoint `PhysicsEngine.UpdatePlayerCurrCell` still runs;
|
||||
- does **not** re-arm the constraint leash (§1b);
|
||||
- does **not** write the body — the canonical commit already did.
|
||||
|
||||
Then `BlipPosition` has no remaining caller: **delete it** along with
|
||||
`HeadlessSessionWorldProjection.BlipLocalPlayer`. If a test is its only other
|
||||
caller, the test moves to the new path — do not keep the method alive for
|
||||
tests.
|
||||
|
||||
### 4b. The ack
|
||||
|
||||
`SendPositionImmediately` is an OUTPUT of the committed route. Fire
|
||||
`LocalPlayerOutboundController.SendImmediatePosition(session, controller)`
|
||||
only after §4a, only when `route.SendPositionImmediately`, and only once. This
|
||||
is the named behaviour change the contract calls out: today the packet leaves
|
||||
before any commit and the trailing `isCurrent()` cannot recall it.
|
||||
|
||||
### 4c. Deferred cell
|
||||
|
||||
`DeferredCell` means the destination landblock's collision generation is not
|
||||
ready; the operation parks and the existing `CommitCollisionGeneration` wake
|
||||
resubmits and commits it. The ack must still fire exactly once, after that
|
||||
commit.
|
||||
|
||||
Retain the pending ack keyed by the placement token, and resolve it on a pump
|
||||
`Advance()` called from the SAME two host sites that already call
|
||||
`RuntimeFirstEntryDriveController.DriveAll()` —
|
||||
`LiveEntityHydrationController.cs:405` (graphical) and
|
||||
`HeadlessSessionWorldProjection.cs:571,594,612` (headless). Find the existing
|
||||
read-only way to ask "did this token's operation commit / is it gone" before
|
||||
adding anything; only add a minimal internal query to `RuntimeSetPositionState`
|
||||
if none exists. Fire once on the commit transition; drop the pending ack on
|
||||
cancellation, supersession, entity teardown, generation change and reset, and
|
||||
fold its count into the ownership ledger / `IsConverged` so a leaked pending
|
||||
ack cannot hide.
|
||||
|
||||
Also consume the parked `Withdraw` at the FIFO head exactly the way
|
||||
`TryCompleteContinuationPlacement:351-365` does, if and only if it is ours.
|
||||
|
||||
### 4d. Host cutover
|
||||
|
||||
- **App** `LiveEntityNetworkUpdateController.OnPosition`: replace the
|
||||
`LocalForcePositionTransaction.Apply` block with the Runtime call. When the
|
||||
status is anything other than `NotApplicable`, return before the generic tail
|
||||
— App projects the committed result through the existing placement sink. Keep
|
||||
every currency re-check that is still meaningful. Inject the Runtime seam the
|
||||
way the class already borrows Runtime owners (`_localPlayerOutbound` is the
|
||||
precedent); do not add a service locator or a window back-reference.
|
||||
- **Headless** `RuntimeLiveEntitySessionController.OnPositionUpdated`: replace
|
||||
the `ProjectPosition(isLocalPlayer: true, ForcePosition)` +
|
||||
`SendImmediatePosition` pair with the same call. Leave the `Apply`-
|
||||
disposition `OfferTeleportDestination` and `TryCompletePortal` alone — route 3.
|
||||
- Delete `LocalForcePositionTransaction.cs`.
|
||||
|
||||
## 5. Non-negotiables
|
||||
|
||||
- Root causes only. No timeout, grace period, suppression flag,
|
||||
catch-and-swallow, duplicated placement writer, or test-only bypass.
|
||||
- Never `git add -A` / `git add .` / `git reset --hard` / `git checkout -- <path>`.
|
||||
- Do not weaken an existing assertion to make a test pass. If a fixture models
|
||||
the old duplicate-write behaviour, re-model it on the committed projection.
|
||||
- Cite retail as `named symbol @address` (+ the pseudo-C line) in every comment
|
||||
on ported behaviour.
|
||||
- Update `docs/ISSUES.md` and
|
||||
`docs/architecture/retail-divergence-register.md` in the SAME commit as the
|
||||
behaviour change. AP-131 is **not** retired here — its named legacy Position
|
||||
caller is route 4.
|
||||
|
||||
## 6. Acceptance
|
||||
|
||||
1. Focused Runtime tests for the seam: force route classification; ack strictly
|
||||
after commit; ack exactly once; no ack on a rejected/cancelled operation;
|
||||
the displaced-authority case that `LocalForcePositionTransaction`'s trailing
|
||||
`isCurrent()` covered today; the `DeferredCell` → wake → commit → single ack
|
||||
sequence; heading preserved; leash NOT re-armed; `NotApplicable` while a
|
||||
first-entry residence is active.
|
||||
2. App tests proving the generic tail no longer double-writes the local player,
|
||||
and that the committed projection is what moves the render entity.
|
||||
3. Headless tests proving the identical Runtime path.
|
||||
4. `dotnet build -c Release` clean.
|
||||
5. **Complete Release solution suite green — not a focused subset.** Baseline
|
||||
**10,844 passed / 4 skipped / 0 failed**. Any deviation is a regression
|
||||
introduced by this work.
|
||||
6. Connected (user-gated): a server-forced correction leaves the player at the
|
||||
corrected position, heading preserved, exactly one outbound
|
||||
`AutonomousPosition`, no double-apply, and no leash misbehaviour after the
|
||||
correction.
|
||||
479
docs/research/2026-08-03-c4-route-2-review-findings.md
Normal file
479
docs/research/2026-08-03-c4-route-2-review-findings.md
Normal file
|
|
@ -0,0 +1,479 @@
|
|||
# C4 route 2 — dual review findings and required fixes (2026-08-03)
|
||||
|
||||
Both mandated reviews returned **FAIL** on the first implementation pass.
|
||||
Nothing is committed. This is the consolidated fix list; it supersedes the
|
||||
implementer's own closing report where they disagree.
|
||||
|
||||
Reviews: retail-conformance (Opus) and architecture/adversarial (Opus), run
|
||||
independently against the same uncommitted diff.
|
||||
|
||||
## Verified correct — do not churn these
|
||||
|
||||
Both reviews independently confirmed, with addresses:
|
||||
|
||||
- `AuthoritativeTeleportFlags` = `Teleport|Slide|SendPositionEvent` = `0x1012`,
|
||||
byte-exact against `CPhysicsObj::SetPositionSimple` @0x005162B0.
|
||||
- Leash re-arm removal is retail-correct: the FORCE_POSITION branch returns at
|
||||
0x0045409D, strictly before all three `ConstrainTo` sites (0x00454272,
|
||||
0x0045418A, 0x004541EC). The teleport / `CommitPreparedPosition` /
|
||||
`ArmConstraintLeashAtCommittedPlacement` callers correctly still constrain.
|
||||
- Heading preservation happens exactly once, upstream in
|
||||
`InboundPhysicsStateController.ApplyAcceptedPosition:788-798`. The seam does
|
||||
not re-apply or drop it.
|
||||
- Ack ordering is correct and fires exactly once on both the synchronous and
|
||||
the deferred path; the `CanSendPositionEvent` gate matches retail's
|
||||
`CommandInterpreter::SendPositionEvent` @0x006B4770, and correctly does NOT
|
||||
apply `ShouldSendPositionEvent`'s rate limit (retail's force branch calls
|
||||
SendPositionEvent directly).
|
||||
- Route-request field derivation matches the continuation executor field for
|
||||
field.
|
||||
- Contract items 2, 4 (the Runtime command itself), 6, and 7 pass. AP-131
|
||||
correctly not retired.
|
||||
- The re-modelled `PlayerMovementControllerTests` are relocations, not
|
||||
weakenings.
|
||||
|
||||
## Required fixes, in priority order
|
||||
|
||||
### R1 — HIGH — the DeferredCell park cannot survive in production
|
||||
|
||||
`RuntimeEntityObjectLifetime.cs:1636` calls `Physics.SetPosition.Forget(canonical)`
|
||||
on EVERY accepted Position, which unconditionally cancels the entity's in-flight
|
||||
operation. ACE broadcasts at 5-10 Hz, so any park lasting longer than ~100-200 ms
|
||||
is guaranteed to be cancelled before its collision generation commits — exactly
|
||||
the far-destination case the deferred path exists to serve.
|
||||
|
||||
Chain: park -> cancelled -> `RetryDeferred` never runs -> no `Place` receipt ->
|
||||
`ReconcileAndAcknowledge` never runs -> **the body is never moved and no ack is
|
||||
ever sent.** The old `LocalForcePositionTransaction` / `BlipLocalPlayer` pair
|
||||
applied the correction synchronously and unconditionally. Retail's `BlipPlayer`
|
||||
@0x00453940 has no "give up quietly" state at all.
|
||||
|
||||
**Park-and-hope is not a valid mechanism here. Required direction:**
|
||||
|
||||
1. Do not open a park that can never wake. Before submitting, establish that the
|
||||
destination's collision is publishable (R2), and mirror the existing
|
||||
C3c-R1-F7 guard shape (`IHeadlessCollisionNeighborhood.IsWithinServiceWindow`,
|
||||
`HeadlessSessionWorldProjection.cs:20-27`) rather than inventing a new one.
|
||||
2. Where a park still legitimately occurs, the seam must detect that its
|
||||
operation was cancelled — `RuntimeSetPositionState.IsPlacementCompletionTracked`
|
||||
(`:1245`) is the existing read-only query — and **re-issue the placement from
|
||||
the current canonical snapshot** on the next accepted Position or `Advance()`.
|
||||
The snapshot already carries the latest accepted pose, so re-issuing is
|
||||
correct, not a replay of stale state.
|
||||
3. A force correction must never be silently dropped. That is the retail
|
||||
invariant this route exists to preserve.
|
||||
|
||||
Do NOT resolve this with a timeout, a settle window, a retry counter, or by
|
||||
exempting ForcePosition from `Forget`. If you conclude the correct answer is a
|
||||
deliberate, cited divergence, STOP and report rather than shipping one.
|
||||
|
||||
### R2 — HIGH — headless lost its destination-collision publication
|
||||
|
||||
`HeadlessSessionWorldProjection.BlipLocalPlayer` (deleted) called
|
||||
`_collision.CenterOn(position.LandblockId)`. The headless collision neighborhood
|
||||
is a hard 3x3 window (`BuildPublicationPlan`, `:462-488`) moved ONLY by
|
||||
`CenterOn`. The surviving callers are spawn (`:562`), the controller-null login
|
||||
branch (`:603`), teleport prep (`:640`) and portal arrival (`:683`) — a
|
||||
ForcePosition on a live local player now reaches none of them. `PumpFirstEntry`
|
||||
(`:624`) polls the stale `_requestedLocalPlayerCell`, which nothing updates on
|
||||
this path either.
|
||||
|
||||
Restore a real mechanism (re-center plus the `_requestedLocalPlayerCell`
|
||||
update), or gate on `IsWithinServiceWindow` and handle out-of-window explicitly.
|
||||
The deleted `CenterCount` assertion in `HeadlessSessionHostTests.cs:430` is the
|
||||
invariant; restore it rather than the changed number.
|
||||
|
||||
The in-test justification ("retail's BlipPlayer has no streaming-window
|
||||
concept") is true of retail and irrelevant: the window is OUR adaptation, and
|
||||
retail has no equivalent because retail has every landblock resident.
|
||||
|
||||
### R3 — HIGH — login-window ForcePosition now does nothing at all
|
||||
|
||||
`RuntimeLiveEntitySessionController.cs:229-254`. Previously every accepted local
|
||||
Position ran `_worldProjection.ProjectPosition`, whose controller-null branch
|
||||
(`HeadlessSessionWorldProjection.cs:593-607`) set `_requestedLocalPlayerCell`,
|
||||
called `CenterOn`, and pumped `_firstEntry.DriveAll()`. Now a ForcePosition
|
||||
takes the new branch, the drive returns `NotApplicable` (residence active), and
|
||||
nothing happens.
|
||||
|
||||
Restore the pump for the controller-absent case, and delete the comment at
|
||||
`:239-240` claiming "there is no legacy fallback to run instead" — there was
|
||||
one; it is the `else` branch this change routed around.
|
||||
|
||||
### R4 — MEDIUM-HIGH — the force-ack steals a receipt the sink declined
|
||||
|
||||
`RuntimeAcceptedPositionDriveController.cs:366-375` unconditionally calls
|
||||
`AcknowledgeProjection(outcome.Projection)`. `RuntimePlacementProjectionSubscription`
|
||||
deliberately leaves a declined `Place` at the FIFO head for a later retry
|
||||
(`:118-121`); this consumes and destroys it.
|
||||
|
||||
The sink declines for real production reasons — `!_spatial.IsLoaded(landblock)`
|
||||
(`LiveEntityRuntime.cs:1210-1219`) and stale transit authority
|
||||
(`RuntimePlacementPresentationSink.cs:90-96`). In those cases `entity.SetPosition`
|
||||
/ `Rotation` / `ParentCellId` / `RebucketLiveEntity` / `IsSpatiallyProjected`
|
||||
are never written, and because the generic tail is now skipped there is no
|
||||
second writer to cover it — the render entity silently stays put while the
|
||||
canonical body moved.
|
||||
|
||||
The `RuntimeFirstEntryDriveController` mirror is safe ONLY because a residence
|
||||
makes the sink decline by design and a follow-up `ExecutorCompleted` receipt
|
||||
re-binds presentation. Route 2 has no such follow-up. Drop the force-ack and let
|
||||
the subscription's retry contract stand, or provide a real follow-up binding.
|
||||
|
||||
### R5 — MEDIUM-HIGH — `Contention` and `Rejected` silently drop the correction
|
||||
|
||||
`RuntimeAcceptedPositionDriveController.cs:265-271` returns `Contention` when
|
||||
`TryBeginExclusiveAuthoredPlacement` fails; neither status creates a pending
|
||||
entry, and both hosts then return without blipping or acking
|
||||
(`LiveEntityNetworkUpdateController.cs:1140`). The `Contention` doc comment
|
||||
promises "a later accepted Position, or this controller's own Advance pump,
|
||||
retries" — the pump provably cannot retry something never recorded, and a later
|
||||
Position carries a different pose. Retail always applies.
|
||||
|
||||
The most likely trigger is R1's parked operation, which makes every subsequent
|
||||
ForcePosition `Contention`. Fixing R1 largely fixes this; the status handling
|
||||
must still not silently drop.
|
||||
|
||||
Also fix the `Rejected` enum doc: it claims "No SetPosition ran; no ack was
|
||||
sent", which is false at the two `SubmitAndResolve` sites (`:361`, `:415`) where
|
||||
a SetPosition ran and was cancelled.
|
||||
|
||||
### R6 — MEDIUM — `_pending` leaks and can be silently overwritten
|
||||
|
||||
`RuntimeAcceptedPositionDriveController.cs:289-324`. `Advance()` exits only on
|
||||
record-key release or acknowledged completion. A mid-session cancellation
|
||||
(supersession, lost-cell deadline, `ParkCollisionResidents`, generation cancel —
|
||||
all route through `ForgetPlacementCompletionCore`) leaves `_pending` set
|
||||
forever, so `AcceptedPositionDrivePendingCount` keeps `IsConverged` false for
|
||||
the rest of the session. `GameWindowLifetime.DisposeGameRuntime:490-498` throws
|
||||
on non-convergence.
|
||||
|
||||
Plan §4c required dropping the pending ack on cancellation, supersession,
|
||||
teardown, generation change and reset. Only teardown and reset shipped. Use
|
||||
`IsPlacementCompletionTracked`. Also: the `_pending = null` cleanups are all
|
||||
guarded by `if (!firstAttempt)`, and `SubmitAndResolve(firstAttempt: true)`
|
||||
assigns `_pending` without inspecting an existing one — make it refuse to
|
||||
overwrite a live pending.
|
||||
|
||||
### R7 — MEDIUM — the 0.48 fixture, the changed assertion, and the false doc
|
||||
|
||||
**This is a test-fixture artifact, NOT a production regression.** The headless
|
||||
fixture's `LoadedSetupCollisionSource` returns one sphere
|
||||
`(Vector3.Zero, 0.48f)` — centre AT the origin, bottom 0.48 m below the feet.
|
||||
The real human Setup `0x02000001` is `(0,0,0.475) r=0.48` plus
|
||||
`(0,0,1.350) r=0.48` (`Ts46SphereListConformanceTests.cs:35-39`), so the foot
|
||||
sphere's bottom is origin - 0.005 and a settled origin lands on the floor within
|
||||
5 mm. The control is in this same changeset: the new Runtime fixture uses the
|
||||
dummy sphere (offset == radius) and asserts the origin lands exactly on the
|
||||
floor (`RuntimeAcceptedPositionDriveControllerTests.cs:178`).
|
||||
|
||||
Required:
|
||||
1. Give the headless fixture the retail offset `(0f, 0f, 0.475f) r=0.48` and
|
||||
**restore the `Z == 50f` assertion**. Changing an assertion to match new
|
||||
output is the plan's own forbidden move.
|
||||
2. Delete the false comment at `HeadlessSessionHostTests.cs:419-427` — it
|
||||
describes the sphere CENTRE and then asserts it about `controller.Position`,
|
||||
which is the ORIGIN (`PlayerMovementController.cs:304` -> `PhysicsBody.cs:153`,
|
||||
retail `CPhysicsObj::m_position.frame.origin`).
|
||||
3. Correct the `docs/ISSUES.md` #285 "retail fidelity gain" paragraph. Retail's
|
||||
`BlipPlayer` has never lifted the origin by a sphere radius. Left as-is this
|
||||
becomes the citation a future session trusts.
|
||||
|
||||
### R8 — MEDIUM — acceptance gaps
|
||||
|
||||
- No App-layer test exists proving the generic tail no longer double-writes the
|
||||
local player and that the committed projection is what moves the render
|
||||
entity. The plan's acceptance item 2 is unmet; three App tests were deleted
|
||||
and replaced with a comment. Given R4, this is precisely the seam that is
|
||||
broken.
|
||||
- `RuntimeAcceptedPositionDriveControllerTests.cs:310-311` asserts
|
||||
`acksAfterFirstResolve <= 1`, so the test **passes with zero acks** — the
|
||||
DeferredCell park -> wake -> commit -> single-ack sequence is unverified,
|
||||
while `docs/ISSUES.md` claims it is covered. Fix the fixture so the contact
|
||||
gate is satisfied and assert exactly one, or state plainly that it is
|
||||
unverified. Do not leave the overclaim in the record.
|
||||
|
||||
### R9 — LOW — hygiene
|
||||
|
||||
- `RuntimeAcceptedPositionDriveController` is `public sealed` with an internal
|
||||
ctor and all-internal members; its template `RuntimeFirstEntryDriveController`
|
||||
is `internal sealed`. Make it internal unless the public surface is genuinely
|
||||
required (if it is, say why).
|
||||
- `PlayerMovementController.cs:632` has a stale `<see cref="BlipPosition"/>` to
|
||||
a deleted member. Harmless only while `GenerateDocumentationFile` is off;
|
||||
`TreatWarningsAsErrors` is on, so it breaks the build the day docs are enabled.
|
||||
- `HeadlessSessionHost._currentSession` is never cleared on teardown.
|
||||
- Headless without a content lease leaves the drive controller null, so the
|
||||
ForcePosition and its ack are dropped entirely
|
||||
(`HeadlessSessionHost.cs:568-581`); previously the ack fired unconditionally.
|
||||
- `LiveEntityNetworkUpdateController.cs:1140-1155` fires
|
||||
`MarkLiveOwnerPoseDirty` and `ObserveAcceptedLocalPosition` for ANY non-
|
||||
`NotApplicable` status including `Rejected` and `Contention` — moving the
|
||||
streaming observer to a landblock we explicitly refused to place into.
|
||||
|
||||
## Gate
|
||||
|
||||
Unchanged: complete Release solution suite, not a focused subset. Pre-change
|
||||
baseline is 10,844 / 4 skipped / 0 failed; the first pass reached 10,848 with
|
||||
the defects above, so a green suite is necessary and demonstrably not
|
||||
sufficient. Both reviews must be re-run on the fixed diff before commit.
|
||||
|
||||
---
|
||||
|
||||
# ROUND 2 — residuals after the R1-R9 fix round (2026-08-03)
|
||||
|
||||
Both delta reviews returned FAIL again. Suite is green at 10,853 / 4 / 0, which
|
||||
again proves nothing. R2, R3, R4, R5, R6, R7 and R9 are confirmed genuinely
|
||||
fixed and must not be churned. Three blocking residuals remain, and **two of
|
||||
them are defects in the R1 reissue mechanism itself.**
|
||||
|
||||
**Root of the problem: `_pending` has no single owner and no single lifecycle
|
||||
rule.** Round 1 bolted reissue onto ad-hoc per-branch bookkeeping. B1 wants MORE
|
||||
reissuing, N1 wants LESS, and N2 wants reissue to be a DIFFERENT route — they
|
||||
look contradictory only because there is no unifying rule. There is one.
|
||||
|
||||
## The unified mechanism (implement exactly this — it replaces the ad-hoc rules)
|
||||
|
||||
`RuntimeEntityPlacementToken` already carries `PositionAuthorityVersion`
|
||||
(`RuntimeSetPositionState.cs:50`). Make that the single decision input.
|
||||
|
||||
**One rule:** the drive owns at most one in-flight placement for the local
|
||||
player. After any terminal outcome, and on every `Advance()`, compare the
|
||||
committed/parked token's `PositionAuthorityVersion` against the live record's
|
||||
current `PositionAuthorityVersion`:
|
||||
|
||||
- **Equal** — the canonical accepted authority has not moved since this
|
||||
operation began. Nothing is outstanding. Clear `_pending`. Do not reissue.
|
||||
- **Advanced** — a newer accepted Position arrived while we were in flight, and
|
||||
it may have been the thing that killed our operation. Consult the newest
|
||||
accepted event's disposition (do NOT reuse `stale.Route`):
|
||||
- still **ForcePosition** — reissue, re-classifying from the current record.
|
||||
- now an ordinary **Apply** — clear `_pending` and do NOT reissue. The
|
||||
correction was superseded by newer server truth; the ordinary route owns
|
||||
that pose. This is not a silent drop: retail applies each event as it
|
||||
arrives, and a force correction overtaken by a newer position is moot.
|
||||
|
||||
Route every terminal branch through one `_pending` funnel. No branch may assign
|
||||
or clear it directly.
|
||||
|
||||
### B1 — BLOCKING — a force correction is still silently dropped (conformance)
|
||||
|
||||
When a parked operation wakes and its `Place` is ACCEPTED by the sink, the
|
||||
operation leaves `_operations` but the completion is retained. `CancelCoreDeferred`
|
||||
then returns early at `RuntimeSetPositionState.cs:5141` without reaching
|
||||
`ForgetPlacementCompletionCore`, so the retained completion survives. The next
|
||||
ForcePosition hits `HasRetainedCompletion` (`:1319`) -> invalid token ->
|
||||
`Contention` (`RuntimeAcceptedPositionDriveController.cs:289-295`); both hosts
|
||||
return without placing or acking, and the next `Advance()` consumes the OLD
|
||||
completion and acks the OLD pose. That packet's correction is lost.
|
||||
|
||||
One-frame window on the graphical host only (`_session.Tick()` inbound dispatch
|
||||
precedes `RetryPending()` in `RetailLiveFrameCoordinator`); headless is immune
|
||||
because its pump is adjacent to the readiness check. The DECLINED-`Place`
|
||||
variant is unaffected and needs no change.
|
||||
|
||||
The unified rule fixes this: the new packet advanced `PositionAuthorityVersion`
|
||||
past the committed token, and the newest event is a ForcePosition, so it
|
||||
reissues.
|
||||
|
||||
### N1 — BLOCKING — stale `_pending` causes a second placement AND a second ack
|
||||
|
||||
`SubmitAndResolve(firstAttempt: true)` (`:297`) never inspects `_pending`, and
|
||||
only the `!firstAttempt` branches clear it (`:508-511`, `:482-484`, `:549-551`).
|
||||
So: park -> `Forget` wipes the watch -> the same packet's ForcePosition Begins
|
||||
cleanly and Commits -> ack fires -> `_pending` still holds the dead P1 -> next
|
||||
`Advance()` finds the watch dead -> `ReissueFromCanonical` -> **a second
|
||||
canonical placement and a second outbound `AutonomousPosition` for one server
|
||||
correction.**
|
||||
|
||||
That is the double-apply/double-ack class this entire slice exists to delete
|
||||
(`670f307c`), reintroduced. `AssignPending` does not catch it because it is only
|
||||
reached on the DeferredCell/retryable branches. The unified funnel fixes it:
|
||||
equal versions -> clear, no reissue.
|
||||
|
||||
### N2 — BLOCKING — reissue applies force semantics to an ordinary pose
|
||||
|
||||
`ReissueFromCanonical` (`:418-447`) reuses `stale.Route` verbatim —
|
||||
`SetPositionSimple` + `Teleport|Slide|SendPositionEvent` +
|
||||
`SendPositionImmediately: true`. But the commonest way a park dies is an
|
||||
ordinary `Apply` Position, whose retail route
|
||||
(`RuntimeAuthoritativePositionRouteClassifier.cs:368-388`) is
|
||||
`Interpolate`/`NoPositionOperation`, `PhysicsSetPositionFlags.None`,
|
||||
`ConstrainPhase.BeforePositionOperation`, `SendPositionImmediately: false`.
|
||||
|
||||
So the reissue converts an ordinary server echo into a hard `Teleport|Slide`
|
||||
canonical placement, sends an ack retail would never send on that branch, and
|
||||
skips the `ConstrainTo` the ordinary branch runs. My round-1 direction sanctioned
|
||||
re-issuing THE CORRECTION; it did not sanction re-classifying a different
|
||||
disposition's pose as a force. The unified rule fixes this by consulting the
|
||||
newest accepted disposition.
|
||||
|
||||
If any residual divergence remains after this, it needs a
|
||||
`docs/architecture/retail-divergence-register.md` row in the same commit.
|
||||
|
||||
### B2 — BLOCKING (record accuracy) — the plan claims coverage it does not have
|
||||
|
||||
`docs/plans/2026-08-02-placement-cutover.md:287` reads "R8 added the App-layer
|
||||
double-write source pins the plan's own acceptance item required." That is a
|
||||
claim of coverage. The truth, per the adversarial review:
|
||||
|
||||
- acceptance item 2's first half is **source-pinned, not proven** — the
|
||||
`Assert.Single` regex would still pass if a second write were spelled
|
||||
differently, and no test exercises the branch;
|
||||
- acceptance item 2's second half — **"the committed projection is what moves
|
||||
the render entity"** — is **uncovered at any layer**. No test drives a route-2
|
||||
ForcePosition through `RuntimePlacementPresentationSink` /
|
||||
`TryApplyRuntimePlacementPlace` and asserts the `WorldEntity` moved.
|
||||
|
||||
Correct the text to record the gap explicitly. A documented gap is acceptable;
|
||||
a false claim of coverage is not. Same rule that produced R7.
|
||||
|
||||
## Non-blocking — record, do not fix in this round
|
||||
|
||||
- **N3** — headless never calls `RetryPending` after construction (grep finds no
|
||||
caller outside `src/AcDream.App/`). R4's fix depends on the subscription's
|
||||
retry, so a declined headless `Place` would wedge the ordered stream. Latent,
|
||||
not proven reachable. File it.
|
||||
- **N4** — `Advance()` lacks the `_driving` reentrancy latch its template
|
||||
`RuntimeFirstEntryDriveController.DriveAll:128-148` has. No live re-entrant
|
||||
path today. Hygiene.
|
||||
- **N5** — `CenterOnAcceptedForcePosition` does not restore
|
||||
`controller.LocalEntityId = record.LocalEntityId ?? 0u` (inert today, but an
|
||||
unreplaced deletion); `_movementTruthDiagnostics.OnServerEcho` no longer fires
|
||||
for a local ForcePosition (diagnostic only).
|
||||
- **R9 residue** — `ConstraintManager.cs:25` and `PhysicsBody.cs:442` still cite
|
||||
the deleted `BlipPosition` in `<c>` tags (build-safe, but false docs).
|
||||
- **Route-1 ack** — the plan required confirming whether the ack fires while an
|
||||
initial-Create residence owns the record. It does not; `SendPositionImmediately`
|
||||
is consumed only as a trace fact in the continuation executor (`:717`, `:2576`).
|
||||
Not a regression, but the plan said file it. File it.
|
||||
- **AD register row** — the headless 3x3 collision window is now a named member
|
||||
of the Runtime-facing `IRuntimeDirectWorldProjection` contract with an ordering
|
||||
requirement retail has no analogue for, and no existing row covers it (AD-6 is
|
||||
retired; AD-2 is the graphical reveal barrier). Recommend a row.
|
||||
- **Stale comment** — the `HeadlessSessionHostTests` comment references "the
|
||||
previous 50.48f assertion", which does not exist at HEAD.
|
||||
|
||||
---
|
||||
|
||||
# ROUND 3 — final round (2026-08-03)
|
||||
|
||||
The round-3 **adversarial** review returned **PASS**. The round-3
|
||||
**conformance** review returned **FAIL on one item**. This round closes that
|
||||
item and files the adversarial review's non-blocking findings. The round-2
|
||||
unified `_pending` funnel was confirmed sound by both reviewers and was NOT
|
||||
restructured.
|
||||
|
||||
## The blocker — a terminal-without-commit sent no ack (CLOSED)
|
||||
|
||||
`SettlePending`'s Equal branch was reached both by a successful commit and by a
|
||||
terminal outcome that never committed (non-retryable prepare failure, the
|
||||
`default:` Rejected/Cancelled branch, and both `Advance` death branches). In the
|
||||
non-commit case the body never moved AND no outbound `AutonomousPosition` left.
|
||||
|
||||
The conformance reviewer proposed a `committed` flag plus a
|
||||
one-re-issue-per-`Advance` guard. That is more than retail requires, and the
|
||||
retail evidence was re-verified from
|
||||
`docs/research/named-retail/acclient_2013_pseudo_c.txt` before coding:
|
||||
|
||||
- `SmartBox::BlipPlayer` @0x00453940 (line 92528) calls
|
||||
`CPhysicsObj::SetPositionSimple(this->player, edi_1, 1)` @0x00453968 and
|
||||
**discards its return value**. `BlipPlayer` itself returns `void`.
|
||||
- `CPhysicsObj::SetPositionSimple` @0x005162B0 (line 284276) is declared
|
||||
`enum SetPositionError __thiscall`. Other retail call sites DO test it —
|
||||
`if (CPhysicsObj::SetPositionSimple(...) == OK_SPE)` at @0x0055605D and
|
||||
@0x00556021 — which proves the discard in `BlipPlayer` is deliberate, not a
|
||||
decompiler artifact.
|
||||
- `SmartBox::HandleReceivedPosition` @0x00453FD0's FORCE_POSITION branch calls
|
||||
`SmartBox::BlipPlayer` @0x00454074, stamps `update_times[0]` @0x00454079, then
|
||||
runs `cmdinterp->SendPositionEvent()` @0x00454091 **unconditionally** and
|
||||
returns @0x0045409D.
|
||||
|
||||
Retail's semantics are therefore: **attempt the placement once; if it fails the
|
||||
body simply does not move; acknowledge regardless; never retry.** No commit flag
|
||||
and no recursion guard are needed to express that.
|
||||
|
||||
**Implemented.** The ack is now sent exactly once per BEGUN placement, at its
|
||||
terminal outcome — from `ReconcileAndAcknowledge` on the commit path, or from
|
||||
`SettlePending` on a non-commit terminal. `SettlePending` gained a
|
||||
`positionEventOwed` parameter; `Pending` gained a `PositionEventOwed` field so
|
||||
the re-issue retry marker (which stands for a packet whose placement was never
|
||||
begun) cannot double-ack. The commit paths pass `false` because their ack has
|
||||
already left. The non-commit ack runs no reconciliation — the body did not move,
|
||||
so there is no committed frame to reconcile — and therefore carries the body's
|
||||
unchanged pose, which is exactly what retail's ack carries after a failed
|
||||
`SetPositionSimple` and is informative to the server: its force did not take.
|
||||
The `CanSendPositionEvent` gate was left untouched; it is retail's own
|
||||
(`CommandInterpreter::SendPositionEvent` @0x006B4770 tests the transient-state
|
||||
contact bits), so a legitimately airborne body still suppresses the send on both
|
||||
paths.
|
||||
|
||||
`ReconcileAndAcknowledge` was split so both paths share one outbound site,
|
||||
`SendPositionEvent`.
|
||||
|
||||
## Other items closed this round
|
||||
|
||||
- **AD-62 rewritten.** The `DeferredCell`-park precondition is dropped — it was
|
||||
never required, and the never-parked failure paths reach the same outcome. The
|
||||
row now leads with the general rule and keeps the named shapes as examples,
|
||||
adds the externally-blocked `Contention` shape (nothing recorded, nothing
|
||||
pumps it) and the other `PositionAuthorityVersion` advances (`TryApplyPickup`
|
||||
`RuntimeEntityObjectLifetime.cs:1116`, `CommitPositionChannelUpdate` `:2041`,
|
||||
`AdvanceCreateAuthority` `:2466`), and separates the shapes that now lose only
|
||||
the re-apply from the narrower shapes that still lose the ack too.
|
||||
- **Overstated doc corrected.** `AcceptedForceObservation`'s comment claimed the
|
||||
record's current version equals the recorded force's version *if and only if*
|
||||
the newest accepted event was that force. False — `AdvancePositionAuthority`
|
||||
has four call sites. It is now stated as the one-way test it actually is.
|
||||
- **Vacuous assertion deleted.** The `gameActions.Count <= 2` assertion in
|
||||
`Advanced_ReissuesWhenTheNewestAcceptedEventIsStillAForcePosition` could not
|
||||
fail: that fixture's `CommitLandblockCollision` adds both landblocks at
|
||||
`worldOffsetX/Y: 0f` while the world frame places the deferred landblock at
|
||||
+192/+192, so the body lands over no terrain, `InContact` is false, and every
|
||||
ack is suppressed — the count is 0. It was also too loose to encode "at most
|
||||
one per packet". Deleted rather than shipped; the test's real discriminators
|
||||
(body position, `PendingCount`) stay.
|
||||
|
||||
## Tests
|
||||
|
||||
Two added, both with a verified discrimination check (the implementation was
|
||||
temporarily broken in each direction and the intended test observed to fail,
|
||||
then reverted and re-verified):
|
||||
|
||||
- `TerminalWithoutCommit_SendsExactlyOnePositionEventAndLeavesTheBodyUnmoved` —
|
||||
a park retired without committing sends exactly one `AutonomousPosition`,
|
||||
performs no placement of its own, and never repeats on further pumps.
|
||||
- `Committed_SendsExactlyOnePositionEventAcrossTheCommitAndTheSettle` — the new
|
||||
settle-side ack does not become a second ack on the committed path.
|
||||
|
||||
Two existing tests changed their ack expectation from `Assert.Empty` to
|
||||
`Assert.Single`, because they exercise terminal-without-commit paths whose
|
||||
`Empty` encoded the defect this round removes:
|
||||
`Equal_ClearsPendingWithoutReissuingWhenNoNewerAcceptedAuthorityArrived` and
|
||||
`Advanced_DoesNotReissueWhenTheNewestAcceptedEventIsAnOrdinaryApply`. In the
|
||||
latter the single ack belongs to the FORCE packet, not to the ordinary echo —
|
||||
retail's ordinary branch has no unconditional `SendPositionEvent`.
|
||||
|
||||
Measurement note, recorded because it corrects an assumption in this file's
|
||||
round-2 text: the `DeferredCell` park these fixtures use is the POST-engine
|
||||
quiescence park, so `_physics.Engine.SetPosition` has already moved the
|
||||
canonical body to the destination while the placement itself is withdrawn and
|
||||
parked. The new test therefore captures its unmoved/no-further-placement
|
||||
baselines at the park, and asserts them across the terminal settle, which is the
|
||||
thing under test.
|
||||
|
||||
## Filed, not fixed
|
||||
|
||||
`docs/ISSUES.md` #293 (the `DeferredCell` branch still consumes `Withdraw`
|
||||
receipts the sink may have declined — the same shape R4 removed for `Place`),
|
||||
#294 (`ReconcileAndAcknowledge` runs before the funnel's currency guard on the
|
||||
deferred wake), #295 (the retry marker inflates
|
||||
`AcceptedPositionDrivePendingCount`, which is an in-flight-placement counter),
|
||||
#296 (a retryable prepare is reported to hosts as `Contention`, conflating a
|
||||
retained-and-pumping case with a dropped one).
|
||||
|
||||
## Gate
|
||||
|
||||
Complete Release solution suite, unchanged discipline: green is necessary and
|
||||
demonstrably not sufficient — all four prior states were green and three were
|
||||
defective.
|
||||
135
docs/research/2026-08-03-c4-route-2-visual-gate.md
Normal file
135
docs/research/2026-08-03-c4-route-2-visual-gate.md
Normal file
|
|
@ -0,0 +1,135 @@
|
|||
# C4 route 2 — ForcePosition: connected visual gate (2026-08-03)
|
||||
|
||||
The user-facing acceptance test for route 2. Route 2 is code-complete and
|
||||
suite-green before this runs; this document is the hand-off.
|
||||
|
||||
## Why the obvious recipe does NOT work
|
||||
|
||||
The route-2 contract's acceptance line says *"ACE `@teleport`-style
|
||||
displacement"*. That is wrong about which route it exercises, and following it
|
||||
would have produced a false pass.
|
||||
|
||||
Verified in ACE this session: `PositionPack`'s constructor
|
||||
(`references/ACE/Source/ACE.Server/Network/Structure/PositionPack.cs:36-57`)
|
||||
advances `ObjectTeleport` when `adminMove == true` (lines 49-52) and only ever
|
||||
*reads* `ObjectForcePosition` (line 54). Every admin move command —
|
||||
`@teleto`, `@teletome`, `@teleloc`, `@movetome` — routes through
|
||||
`Player_Location.cs:654 Teleport()` / `SendUpdatePosition(true)` and therefore
|
||||
advances **TELEPORT_TS, not FORCE_POSITION_TS**. Those commands exercise
|
||||
**route 3**, not route 2.
|
||||
|
||||
`SequenceType.ObjectForcePosition` is advanced at exactly **two** places in the
|
||||
whole ACE tree:
|
||||
|
||||
1. `references/ACE/Source/ACE.Server/WorldObjects/Player.cs:1148` — the PK Lite
|
||||
entry collision bump.
|
||||
2. `references/ACE/Source/ACE.Server/WorldObjects/Player_Tick.cs:488` — the
|
||||
anti-cheat z-position rubber-band.
|
||||
|
||||
(2) requires the server to believe you are fly-hacking — same landblock, a
|
||||
claimed Z more than 10 units above your last ground contact, more than a second
|
||||
after your last jump, Jump skill under 1000, and still flagged airborne. It is
|
||||
not reachable by legitimate play and there is no command path into it. Do not
|
||||
build the gate on it.
|
||||
|
||||
So (1) is the gate.
|
||||
|
||||
## The only reliable lever is deferred — what that means
|
||||
|
||||
The reachable trigger is the PK Lite entry-collision bump, which requires
|
||||
retail's `@pklite`. That is a **client** command, not a server one — ACE has no
|
||||
`pklite` text-command handler, so typing `@pklite` into chat today forwards as
|
||||
inert text. Retail's client turns it into a game action instead:
|
||||
|
||||
- `ClientCommunicationSystem::DoPKLite` @`0x0057A490`
|
||||
(`acclient_2013_pseudo_c.txt:390106`) — rejects with error `0x507` when
|
||||
`ACCWeenieObject::IsPlayerKiller` @`0x0058C910` is true (PK bit `0x20` OR
|
||||
PKLite bit `0x2000000`), otherwise calls
|
||||
- `CM_Character::Event_EnterPKLite` @`0x006A13F0` (line 680071) — a 12-byte
|
||||
parameterless game action, opcode `0x28F`, no payload.
|
||||
|
||||
acdream does not implement it, and **the user deferred implementing it
|
||||
(2026-08-03).** Scoping is preserved in this session's research so it can be
|
||||
picked up cheaply: ~40 lines of production code across `ClientCommandId`,
|
||||
`RetailClientCommandCatalog`, `ClientCommandRequests.BuildParameterless`,
|
||||
`WorldSession`, `ClientCommandController`, `LiveSessionCommandRouter`, and
|
||||
`LiveSessionRuntimeFactory` — every pattern already exists, including
|
||||
`WeenieError 0x0507`.
|
||||
|
||||
**Consequence, stated plainly: route 2's own behaviour is NOT visually
|
||||
verifiable in this campaign.** Nothing a user can do makes ACE emit a
|
||||
ForcePosition. Route 2's acceptance therefore rests on its automated Runtime /
|
||||
App / Headless tests and the complete Release suite. The connected pass below
|
||||
is a **regression check on the blast radius**, not acceptance of the new route.
|
||||
Do not record it as the latter.
|
||||
|
||||
## What the connected pass actually covers
|
||||
|
||||
Launch acdream in **Release** with `ACDREAM_RETAIL_UI=1` against the local ACE
|
||||
at `127.0.0.1:9000`.
|
||||
|
||||
Route 2 deletes `PlayerMovementController.BlipPosition` and
|
||||
`HeadlessSessionWorldProjection.BlipLocalPlayer`, removes App's generic-tail
|
||||
double-write for the local player, and re-routes the local player's accepted
|
||||
placement through the canonical Runtime SetPosition transaction. So the things
|
||||
to confirm are that ordinary play is untouched:
|
||||
|
||||
- Ordinary running, turning, walk/run toggle. No tethering, no rubber-band, no
|
||||
drift against the server.
|
||||
- A jump and a landing.
|
||||
- Walking through a doorway into an interior and back out.
|
||||
- A portal recall, and a portal into a dungeon.
|
||||
- Two-client observation: `+Acdream` seen from the retail client moves smoothly
|
||||
and lands where acdream shows it.
|
||||
|
||||
Anything wrong there is route 2's fault even though route 2 did not intend to
|
||||
touch it.
|
||||
|
||||
## Optional cheap shot (may not fire)
|
||||
|
||||
The second ACE call site is the anti-cheat z-position rubber-band
|
||||
(`Player_Tick.cs:459-490`). It needs: same landblock as your last ground
|
||||
contact, a claimed Z more than 10 units above it, more than a second since your
|
||||
last jump, Jump skill under 1000, and the server still flagging you airborne.
|
||||
There is no command path into it and normal play cannot satisfy it (falling
|
||||
makes the Z delta negative, and walking up terrain keeps refreshing the ground
|
||||
position), but a `@teleloc` straight up ~15 units within the same landblock is
|
||||
a two-minute experiment with a definitive tell: ACE's console logs
|
||||
`z-pos hacking detected for +Acdream` at `Player_Tick.cs:486` immediately
|
||||
before it force-bumps you.
|
||||
|
||||
If that line appears, you have a genuine ForcePosition and the checks below
|
||||
apply. If it does not, the route is untested by observation — which is the
|
||||
expected outcome.
|
||||
|
||||
**If a ForcePosition does occur, must be true:**
|
||||
- You end up at the corrected position and stay there. No visible double-apply,
|
||||
no snap-then-yank-back over one or two frames.
|
||||
- **Your facing does not change.** Retail's force branch replaces the
|
||||
destination heading with your current heading before placing
|
||||
(`HandleReceivedPosition` @`0x00453FD0`, `Frame::set_heading` at
|
||||
`0x00454068`).
|
||||
- Exactly **one** outbound `AutonomousPosition` for the correction.
|
||||
|
||||
**The two named behaviour changes:**
|
||||
|
||||
1. **The ack now fires after the canonical commit, not before it.** Previously
|
||||
the client told ACE "got it, I'm here" before deciding where "here" was.
|
||||
Symptom of a regression: ACE re-sending corrections, or a visible fight
|
||||
between client and server position after the bump.
|
||||
|
||||
2. **The ForcePosition route no longer re-arms the constraint leash.** Retail's
|
||||
force branch returns at `0x0045409D`, ahead of all three `ConstrainTo` call
|
||||
sites (`0x00454272`, `0x0045418A`, `0x004541EC`); our old `BlipPosition`
|
||||
re-armed the leash citing a branch it was not on. #167 was a leash bug, so
|
||||
this is the change most worth your eyes if you get a bump. Symptom of a
|
||||
problem: after the bump, movement feels tethered, rubber-bands back toward
|
||||
the pre-bump spot, or the leash trips on ordinary running shortly after.
|
||||
|
||||
## Known, deliberately unchanged
|
||||
|
||||
Retail's `SmartBox::PlayerPositionUpdated` (@`0x00453870`) gates its
|
||||
`set_viewer` / `LScape::update_viewpoint` re-seat on
|
||||
`distance >= GetAutonomyBlipDistance` (`0x004538C0-0x004538E2`). We publish the
|
||||
render root unconditionally. Not changed in this slice; recorded here so the
|
||||
next reader does not rediscover it as a bug.
|
||||
Loading…
Add table
Add a link
Reference in a new issue