From 634bc5513a3e47b81efc6be84709eb564d820ce3 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 4 Aug 2026 04:07:39 +0200 Subject: [PATCH] fix(physics): restore a cancelled park instead of leaving the entity withdrawn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shipped-code defect affecting committed route-2 code, found while reviewing route 4b-1. RuntimeSetPositionState.ParkDeferred withdraws an entity from the world: body.InWorld = false, TransientStateFlags.Active cleared, WithdrawCanonical, SuspendObjectClock. CancelCoreDeferred then removed the operation and rewrote the pending Withdraw into a Discard while restoring NONE of it. So cancelling a wakeable park was strictly worse than keeping one — the park is wakeable, the cancel destroys the only object that could ever wake it, and the entity is left invisible AND intangible with nothing to bring it back. Route 2's re-issue funnel masked this: re-issuing is correct for a one-shot ForcePosition ACE never repeats, and wrong for a repeated remote stream, so the hole was hidden rather than fixed. Retail's own answer is a working park, verified in the decomp rather than assumed: CPhysicsObj::SetPositionInternal @0x00515BD0, when AdjustPosition yields no cell @0x00515C1D, calls prepare_to_leave_visibility @0x00515CDA, store_position @0x00515CE2 (the DESTINATION pose is committed), GotoLostCell @0x00515CF2 registering at m_position.objcell_id read AFTER store_position (so the destination cell), clears transient 0x80 @0x00515CF7, and returns OK @0x00515D07. InitObjCell @0x00508260 drains the lost list on cell load and calls reenter_visibility @0x00516250, which re-places from the object's OWN m_position with flags 0x11. Two corrections to the direction I gave, both forced by evidence and both right: The pose must NOT be rolled back — only the withdrawal. Three shipped route-2 tests capture positionAtPark AFTER the park and assert it survives the cancel, and retail agrees: store_position commits the destination and nothing un-commits it. Restoring residency at the body's committed cell is therefore retail's own cell choice, not merely self-consistent. The gate defaults to FALSE with four explicit opt-ins, rather than defaulting true with opt-outs at the withdrawal callers. That keeps every one of the ~20 shipped Forget/ForgetExactPlacement sites at exactly its current behaviour instead of depending on having correctly enumerated the withdrawal transactions. Review had already found the broad version corrupting five of them (TryApplyPickup, CommitAcceptedParent, CommitAcceptedParentCellless, CommitWithdrawal, CommitPositionChannelUpdate): they hand-roll a partial re-withdrawal that undoes the clock and FullCellId but not InWorld or the _spatialRoots re-registration, leaving a picked-up item both in inventory and an InWorld cellless spatial root in the physics workset. ParkDeferred's restorableOnCancel is opt-in for exactly one of its four callers — the plain unplaceable-destination park. Every quiescence and retirement park is excluded deliberately: those entities are withdrawn because their world is going away, and restoring residency inside a quiescing prefix blocks its retirement. VerifyPositionChannelCancellation now asserts InWorld and IsSpatialRoot per channel — Position is a cancellation and must restore; Pickup and Parent are withdrawals and must not. It previously asserted only !IsDeferred and counts, which is why five green states hid this. Register row AP-136 measured against GotoLostCell/reenter_visibility rather than labelled "retail-shaped". Files #309 (the restore-on-cancel residual, with park-survives recorded as the retail-faithful target and its two blockers named: the NewerPositionPickupAndParentEachCancelExactLostOperation invariant and teardown convergence) and #310 (an unbounded retirement stall — a retained preparation retry pins its prefix through HasOldPrefixPlacementDebt forever, and TickLostCellDeadlines has no production caller so the 25 s timer never fires). This is a user-observable change to shipped paths: restorableOnCancel: true sits in SubmitPreparedPlacementCore, the shared core behind every production placement. AP-136 and #309 carry the proposed two-client check. Gates: complete Release solution 10,973 passed / 4 skipped / 0 failed (baseline 10,938). Every new test discrimination-verified by reverting the fix. Co-Authored-By: Claude Opus 5 --- docs/ISSUES.md | 149 ++++++++++++ .../retail-divergence-register.md | 3 +- .../Entities/RuntimeEntityDirectory.cs | 17 ++ .../Entities/RuntimeEntityObjectLifetime.cs | 12 +- .../Entities/RuntimeEntityRecord.cs | 23 ++ .../Physics/RuntimeSetPositionState.cs | 218 +++++++++++++++++- .../RuntimeAcceptedPositionDriveController.cs | 12 +- .../RuntimeCollisionPrefixQuiescenceTests.cs | 89 +++++++ .../Physics/RuntimeSetPositionStateTests.cs | 153 ++++++++++++ 9 files changed, 666 insertions(+), 10 deletions(-) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 12b2fe09..54403e6a 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -24,6 +24,155 @@ What does NOT go here: - Every session: scan OPEN issues at start; promote/close anything we touched during the session before ending. - Promoting to a Phase: mark as `DONE (promoted to Phase X)` + commit SHA where the Phase entry landed. +## C4 route 4b-1 review — park lifecycle — 2026-08-04 + +#309 and #310 filed from the route 4b-1 dual-review round; #311 filed from +the delta-review round on the same route's remediation. Evidence: +[`2026-08-04-c4-route-4b-1-review-findings.md`](research/2026-08-04-c4-route-4b-1-review-findings.md). + +## #309 — Cancelled lost-cell park re-shows the entity where retail would keep it hidden + +**Status:** OPEN +**Severity:** MEDIUM +**Filed:** 2026-08-04 +**Component:** physics / placement + +**Description:** When a `DeferredCell` park is cancelled by the accepted-Position +merge, we now roll the withdrawal back (`InWorld`, object clock, canonical +residency) so the entity is no longer stranded invisible-and-intangible. But the +entity becomes **visible immediately at the committed destination pose, without +collision**, whereas retail keeps it hidden and re-shows it only when the cell +loads. + +**Root cause / status:** Retail's lost-cell mechanism has no cancel at all. +`CPhysicsObj::SetPositionInternal` @0x00515BD0 commits the destination pose via +`store_position` @0x00515CE2 and registers the object with +`CObjectMaint::GotoLostCell` @0x00515CF2 (@0x00508210). That registration is +removed by exactly one thing — `CObjectMaint::InitObjCell` @0x00508260, which +drains the lost list on cell load and calls `CPhysicsObj::reenter_visibility` +@0x00508296 (@0x00516250), re-placing at the pose `store_position` committed. +An update that performs no SetPosition leaves the registration untouched. + +So the retail-faithful end state is a park that **survives** cancellation. That +was implemented and reverted this round because it reverses a shipped, tested +invariant — `NewerPositionPickupAndParentEachCancelExactLostOperation` +(`tests/AcDream.Runtime.Tests/Physics/RuntimeSetPositionStateTests.cs`, helper +`VerifyPositionChannelCancellation`) asserts `Assert.False(IsDeferred(record))`, +i.e. a newer Position cancels the park — and because surviving parks broke +`GameRuntime` teardown convergence (stage 10). Re-deciding that invariant plus +teardown convergence is the blocker. + +Residual in practice: a remote at 5-10 Hz is superseded within ~150 ms. The case +that bites is a remote that teleports into a non-resident landblock and then +**stops moving**, because ACE stops broadcasting for a stationary entity. + +Also unrestored: the `ShadowObjectRegistry.Suspend` applied by +`WithdrawCanonical`. Un-suspending requires a real placement dispatch +(`ReplacePositionRows`), so the entity rejoins the collision broadphase on its +next placement rather than at cancel time. + +**Files:** `src/AcDream.Runtime/Physics/RuntimeSetPositionState.cs` — +`ParkDeferred`'s `restorableOnCancel`, `Forget`, `RestoreParkWithdrawal`. + +**Acceptance:** Park survives a superseding no-placement Position and wakes via +`InitObjCell`-equivalent collision-generation arrival, with teardown converging +and the newer-Position invariant deliberately re-decided. + +**Connected check required — this is NOT a no-behaviour-change slice.** The +route 4b-1 contract's "no connected gate" line does not apply to the park fix. +`restorableOnCancel: true` is set in `SubmitPreparedPlacementCore`, the shared +core behind EVERY production placement, and the merge-time +`restoreCancelledPark: true` is on the accepted-Position path every remote and +the local player traverse. So shipped behaviour changes for any entity whose +placement parks. + +Proposed connected check (two clients, local ACE): +1. Walk the observed character to a landblock boundary so a remote sits in a + landblock the observer has not streamed, forcing a `DeferredCell` park. +2. Confirm the remote no longer vanishes permanently — the pre-fix symptom was + invisible AND intangible for the rest of the session. +3. Confirm it appears at the SERVER-authoritative destination pose, not at a + stale pre-park pose, and that it becomes collidable once the landblock + publishes. +4. Confirm the local player's own ForcePosition corrections (route 2) still + land unchanged — that path shares the same cancel. +Steps 1-3 are the user-visible acceptance for AP-136's residual. + +## #310 — Retained preparation retry stalls landblock retirement with no bound + +**Status:** OPEN +**Severity:** HIGH +**Filed:** 2026-08-04 +**Component:** physics / streaming + +**Description:** An entity holding a retained preparation retry keeps its +landblock prefix in placement debt, so +`TryAcquireCollisionPrefixMutationPermission` refuses on **every** poll and the +landblock never retires. There is no bound and no timeout. + +**Root cause / status:** `HasOldPrefixPlacementDebt` refuses permission while any +affected root holds an operation, so `LandblockRetirementStage.Physics` never +completes and the retirement coordinator simply retries forever. +`TickLostCellDeadlines` — the only expiry that could break the cycle — has **no +production caller**, so its deadline never fires. The only thing that clears the +debt is an inbound packet for that same entity, which is exactly what a +`RetrySetupUnavailable` on an asset that never loads does not produce. + +Pre-existing and independent of route 4b-1; 4b-1 does not bound it, it only +avoids widening it by declining to retain operations for destinations it cannot +service. Pinned by +`RuntimeCollisionPrefixQuiescenceTests.RetainedPreparationRetryStallsPrefixRetirementIndefinitely` +(1,000 consecutive refusals), which also shows retiring the operation is what +releases the prefix. + +Note this is also why `ParkCollisionResidents`'s overlap throw is unreachable: +permission is refused before `ParkCollisionResidents` is ever entered. + +**Files:** `src/AcDream.Runtime/Physics/RuntimeSetPositionState.cs` — +`HasOldPrefixPlacementDebt`, `TryAcquireCollisionPrefixMutationPermission`, +`TickLostCellDeadlines` (uncalled). + +**Acceptance:** A retained preparation retry cannot block a landblock retirement +indefinitely — either the deadline is driven in production or the retirement can +proceed past stale placement debt. + +## #311 — RetryPendingProjections allocates a fresh array on every non-empty call + +**Status:** OPEN +**Severity:** LOW (perf, not correctness) +**Filed:** 2026-08-04 +**Component:** physics / headless + +**Description:** `RuntimeSetPositionState.RetryPendingProjections` (reached via +`RuntimePlacementProjectionChannel.RetryPending` → +`RuntimePlacementProjectionSubscription.RetryPending`) snapshots the entire +pending-projection dictionary into a fresh array on every call — +`_pendingProjection.Values.ToArray()`. C4 route 4b-1's N3 fix +(`HeadlessSessionEventRoute.RetryPending`, wired from `HeadlessSessionHost.Tick`) +now reaches this path every headless host tick instead of once per session — +new per-tick pressure K4's 30-session resource envelope was not measured with. + +**Root cause / status:** The empty-FIFO case is closed — +`RuntimePlacementProjectionSubscription.HasPendingReceipts` (added alongside +this issue) lets a host early-out before ever reaching +`RetryPendingProjections` when nothing is outstanding, which is the +overwhelming common case in steady state. The non-empty case still +allocates: every call that DOES have an outstanding receipt pays a fresh +`.ToArray()` copy. Closing it needs a non-allocating rewrite of +`RetryPendingProjections` itself (e.g. a reusable scratch buffer, mirroring +the `_driveScratch` pattern `RuntimeRemotePlacementDriveController.Advance` +and `RuntimeFirstEntryDriveController` already use) — deferred rather than +attempted in the C4 route 4b-1 delta-review session that filed this, whose +task scope held `RuntimeSetPositionState.cs` off-limits for that session +(a concurrent, separately-owned change was landing in the same file). + +**Files:** `src/AcDream.Runtime/Physics/RuntimeSetPositionState.cs` — +`RetryPendingProjections`. Call site: +`src/AcDream.Headless/Hosting/HeadlessSessionEventRoute.cs` — `RetryPending`. + +**Acceptance:** A headless tick with N>0 outstanding placement receipts does +not allocate a new array per tick. + ## Recent-regression cleanup — 2026-08-03 Plan: [`2026-08-03-recent-regression-cleanup.md`](plans/2026-08-03-recent-regression-cleanup.md). diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 54a92dfa..cfef3311 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -159,7 +159,7 @@ readiness/requeue adaptation. See --- -## 3. Documented approximation (AP) — 93 active rows (AP-135 filed 2026-08-03, C4 route 4a — the airborne no-op's retained acdream bookkeeping; the stated total was 2 rows stale before that filing and is now a literal count of this section; AP-130/AP-131/AP-132 filed 2026-08-02, continuation-executor slice; AP-1 narrowed 2026-07-31 by placement/streaming Slice 4A — the pure canonical retail `SetPosition` transaction exists, but production routes and lost-cell lifetime remain on the legacy resolver until Slice 4B; AP-5 retired 2026-07-31 at Campaign P Slice 2A — every successful `step_down` now performs retail's final `PLACEMENT_INSERT`; AP-3/AP-4 retired 2026-07-31 at Campaign P Slice 1B — `transitional_insert` and `edge_slide` now preserve retail's valid-contact early return and Branch-1-first order; AP-127 retired 2026-07-31 by #268 — the complete augmentation chain is shared by character UI and Runtime movement; AP-30 retired 2026-07-30 by the movement parity audit — retail Frame::is_equal genuinely uses the 0.0002 epsilon [byte-confirmed], so the row recorded a NON-divergence; acdream already matches; AP-129 narrowed 2026-07-30 at the P4 Opus review fix — `CanMoveInto`/`RestrictionDB::IsAllowedIn` are now ported and fed end-to-end (CreateObject HouseOwner/HouseRestrictions/Monarch tail fields + live `House_UpdateRestrictions 0x0248`, resolved through `PhysicsEngine.Objects`), retiring the original "CanMoveInto entirely unmodeled, unconditional fail-closed" gap the row described — the review was triggered by `RestrictionObjPrevalenceInspectionTests` showing 103,766 of 729,888 installed EnvCells (the whole housing estate) carry a baked `RestrictionObj`, so the unconditional fail-closed default would have locked every house for every player including its own owner; AP-10 retired 2026-07-30 at Campaign P Slice P4 — restored retail's 0.1 m dry-corner water sink-in, full suite green proving the sticky-bit no-regression argument; AP-71 retired same slice — `check_entry_restrictions` ported at the head of the indoor `FindEnvCollisions` branch, `CellPhysics.RestrictionObj` wired from the DAT-baked `EnvCell` field in both the dev and production caching paths; AP-128 filed 2026-07-30 at the P3 Opus review — PK-timer clock basis; AP-25 retired 2026-07-30 at Campaign P Slice P1 — the vitae/enchantment-aware run/jump skill chain; AP-7 retired 2026-07-30 at Campaign P Slice P2 — `calc_friction`'s threshold ported to retail's confirmed 0.25f; its still-open cos(10°)-vs-0.99999536f Sledding constant question moved to AD-55) +## 3. Documented approximation (AP) — 94 active rows (AP-136 filed 2026-08-04, C4 route 4b-1 review — a cancelled lost-cell park re-shows the entity where retail keeps it hidden until cell load; AP-135 filed 2026-08-03, C4 route 4a — the airborne no-op's retained acdream bookkeeping; the stated total was 2 rows stale before that filing and is now a literal count of this section; AP-130/AP-131/AP-132 filed 2026-08-02, continuation-executor slice; AP-1 narrowed 2026-07-31 by placement/streaming Slice 4A — the pure canonical retail `SetPosition` transaction exists, but production routes and lost-cell lifetime remain on the legacy resolver until Slice 4B; AP-5 retired 2026-07-31 at Campaign P Slice 2A — every successful `step_down` now performs retail's final `PLACEMENT_INSERT`; AP-3/AP-4 retired 2026-07-31 at Campaign P Slice 1B — `transitional_insert` and `edge_slide` now preserve retail's valid-contact early return and Branch-1-first order; AP-127 retired 2026-07-31 by #268 — the complete augmentation chain is shared by character UI and Runtime movement; AP-30 retired 2026-07-30 by the movement parity audit — retail Frame::is_equal genuinely uses the 0.0002 epsilon [byte-confirmed], so the row recorded a NON-divergence; acdream already matches; AP-129 narrowed 2026-07-30 at the P4 Opus review fix — `CanMoveInto`/`RestrictionDB::IsAllowedIn` are now ported and fed end-to-end (CreateObject HouseOwner/HouseRestrictions/Monarch tail fields + live `House_UpdateRestrictions 0x0248`, resolved through `PhysicsEngine.Objects`), retiring the original "CanMoveInto entirely unmodeled, unconditional fail-closed" gap the row described — the review was triggered by `RestrictionObjPrevalenceInspectionTests` showing 103,766 of 729,888 installed EnvCells (the whole housing estate) carry a baked `RestrictionObj`, so the unconditional fail-closed default would have locked every house for every player including its own owner; AP-10 retired 2026-07-30 at Campaign P Slice P4 — restored retail's 0.1 m dry-corner water sink-in, full suite green proving the sticky-bit no-regression argument; AP-71 retired same slice — `check_entry_restrictions` ported at the head of the indoor `FindEnvCollisions` branch, `CellPhysics.RestrictionObj` wired from the DAT-baked `EnvCell` field in both the dev and production caching paths; AP-128 filed 2026-07-30 at the P3 Opus review — PK-timer clock basis; AP-25 retired 2026-07-30 at Campaign P Slice P1 — the vitae/enchantment-aware run/jump skill chain; AP-7 retired 2026-07-30 at Campaign P Slice P2 — `calc_friction`'s threshold ported to retail's confirmed 0.25f; its still-open cos(10°)-vs-0.99999536f Sledding constant question moved to AD-55) Wave-0 UI ledger repair (2026-07-10) retired stale AP-38, resolved the AP-84 collision, restored overwritten paperdoll rows as AP-92/AP-93, and registered @@ -282,6 +282,7 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps. | AP-133 | **Filed 2026-08-03 (#282).** A retail `CPhysicsObj` has exactly ONE `cell`; `ShouldDrawParticles` @0x0050fe60 reads that same field and calls `IsInView` on it, and `set_cell_id` @0x0050f4f0 / `change_cell` @0x00513390 are the only things that move it. acdream splits the concept into `WorldEntity.ParentCellId` (render parent, null for outdoor dat stabs and building shells) and `WorldEntity.EffectCellId` (authored landcell for those parentless stabs). Every consumer now resolves through the single `WorldEntity.VisibilityCellId` accessor (`ParentCellId ?? EffectCellId`); live entities carry `ParentCellId` only. | `src/AcDream.Core/World/WorldEntity.cs` (`VisibilityCellId`); writers `LandblockLoader.cs:80,97`, `LandblockBuildFactory.cs:408` | Outdoor dat stabs deliberately keep a null render parent so portal visibility does not filter them as interior geometry, yet retail still gives their physics object a landcell for particle gating. One accessor keeps the two fields from being read in conflicting orders, which is exactly how #282 arose - `EntityEffectPoseRegistry` preferred `EffectCellId` while `WbDrawDispatcher` and the remote spawn seed preferred `ParentCellId`. | A future writer that sets `EffectCellId` on a live entity re-creates #282: it wins `VisibilityCellId` while the 11 per-tick `ParentCellId` writers leave it frozen, stranding that entity's particles and lights on a stale cell so they fail `IsInView` after it crosses a boundary. | `CPhysicsObj::ShouldDrawParticles` 0x0050fe60; `CPhysicsObj::set_cell_id` 0x0050f4f0; `CPhysicsObj::change_cell` 0x00513390 | | AP-134 | **Filed 2026-08-03 (#297).** Retail keeps ONE `PublicWeenieDesc::_bitfield` per object and mutates it in place — `SetPlayerKillerStatus` @0x005AC7C0 rewrites bits 5/21/25 (PK `0x20` / Free `0x200000` / PKLite `0x2000000`, mutually exclusive), driven from `ACCWeenieObject::OnStatUpdated` @0x0058DF20 `case 0x86`, and `IsPK`/`IsImpenetrable`/`IsPKLite` @0x0058C8xx read that same field. acdream replicates the value into FIVE stores: `ClientObject.PublicWeenieBitfield` (the source, written only by `ClientObjectTable.UpdateIntProperty` on PropertyInt 134), `InboundPhysicsStateController._snapshots[guid].ObjectDescriptionFlags`, `RuntimeEntityRecord.Snapshot.ObjectDescriptionFlags`, the decoded `ShadowObjectRegistry` registration + per-cell `ShadowEntry.Flags`, and the local player's `RuntimeMovementSkillState` own-PWD bitfield. Coherence is maintained by two `ObjectUpdated` subscribers (`RuntimeEntityPvpBitfieldSnapshotSync` for the two snapshot stores, `LiveEntityPvpBitfieldSync` for the decoded shadow flags) plus the appearance-rebuild path re-deriving from the snapshot. The two shadow-flag writers are the SAME invalidation applied at the two edges that can invalidate it, not competing authorities. | `src/AcDream.Runtime/Entities/RuntimeEntityPvpBitfieldSnapshotSync.cs`; `src/AcDream.App/Physics/LiveEntityPvpBitfieldSync.cs`; source writer `src/AcDream.Core/Items/ClientObjectTable.cs` (`UpdateIntProperty`, PropertyInt 134); decode `EntityCollisionFlagsExt.FromPwdBitfield` | ACE never re-sends a `PublicWeenieDesc` after login (`EnqueueBroadcastUpdateObject` has zero live callers), so PropertyInt 134 over 0x02CE/0x02CD is the ONLY signal a PK status changed — a client cannot learn it from the bitfield itself. The replication exists because acdream separates wire snapshots, canonical records, and the collision shadow registry, which retail does not; each layer needs the decoded value at a different lifetime. Before #297 the snapshot stores were immutable wire captures; this commit is what converts them into write-through caches, and therefore what creates the invariant. | Any future write path that sets `ClientObject.PublicWeenieBitfield` outside `UpdateIntProperty`, or any NEW decoded cache of the PK bits, silently re-creates #297: the player walks through PKLite opponents and melee/missile admission refuses them, with no test failing. Note the same class already exists one field over — `Properties.Ints[134]` is written by `UpsertProperties` (PlayerDescription 0x0013) and `UpdateProperties` (IdentifyObjectResponse) WITHOUT mirroring into the bitfield (#300), and retail's `OnStatUpdated` also rewrites `_blipColor` (`case 0x5f`) and `_radar_enum` (`case 0x85`) which acdream ignores entirely (#301). | `PublicWeenieDesc::SetPlayerKillerStatus` 0x005AC7C0; `ACCWeenieObject::OnStatUpdated` 0x0058DF20 (`case 0x86`); `ACCWeenieObject::IsPK`/`IsImpenetrable`/`IsPKLite` 0x0058C8xx; retail `PKStatusEnum` `acclient.h:6412-6427` | | AP-135 | **Filed 2026-08-03 (C4 route 4a).** Retail `CPhysicsObj::MoveOrTeleport` 0x00516330 writes NOTHING on the airborne no-op (`arg4 == 0` -> `return 0` @0x0051636D), and `SmartBox::HandleReceivedPosition` 0x00453FD0 skips `ConstrainTo` with it (@0x00454272 sits inside `if (MoveOrTeleport(...) != 0)` @0x00454254). acdream honours that for every retail-modeled write — body pose, interpolation queue, leash, render entity, collision shadow, and the AP-80 velocity-derived animation cycle — but deliberately KEEPS two acdream-only per-packet bookkeeping writes on that branch: `RemoteMotion.CellId = wire landblock` and the `LastServerPos`/`LastServerPosTime` sample. This was pre-existing player-remote behaviour; route 4a extends it to NPC remotes so both arms are identical | `src/AcDream.App/Physics/LiveEntityNetworkUpdateController.cs` (`OnPosition`, both remote airborne-no-op returns) | The cell id is what acdream's OWN per-tick free-fall `ResolveWithTransition` sweep gates on (`rm.CellId != 0`); without it an airborne remote's sphere sweep is skipped and it falls through the floor (#42's neighbourhood). The server sample is what the first grounded packet after the arc synthesizes its velocity from; dropping it would make that velocity span the whole jump. Neither is a retail `CPhysicsObj` field being written | A remote's cell membership tracks the server's landblock during an arc where retail would keep the cell its own physics last resolved. Visible only if the server's mid-arc landblock disagrees with the client's swept cell — the wire cell is authoritative in every case acdream has observed. Retire together with the free-fall sweep gate, when the remote arc is resolved by the same transition machinery the local player uses | `CPhysicsObj::MoveOrTeleport` 0x00516330 (@0x0051636D `return 0`); `SmartBox::HandleReceivedPosition` 0x00453FD0 (@0x00454254/@0x00454272) | +| AP-136 | **Filed 2026-08-04 (C4 route 4b-1 review).** Retail has NO cancel for a lost-cell park. `CPhysicsObj::SetPositionInternal` @0x00515BD0 commits the destination pose with `store_position` @0x00515CE2 and registers the object via `CObjectMaint::GotoLostCell` @0x00515CF2 (@0x00508210); the registration is removed by exactly one thing, `CObjectMaint::InitObjCell` @0x00508260, which drains the lost list on cell load and calls `CPhysicsObj::reenter_visibility` @0x00508296 (@0x00516250) to re-place at the committed pose. An update that performs no SetPosition leaves the registration untouched, so retail keeps the object HIDDEN until its cell loads. acdream's accepted-Position merge cancels the park instead (a shipped, tested invariant), so on cancel we roll the withdrawal back — `InWorld`, object clock, canonical residency — and the entity becomes VISIBLE IMMEDIATELY at the committed destination pose, uncollidable until its landblock publishes. The pose itself is retail-exact and is deliberately not rolled back. The `ShadowObjectRegistry.Suspend` applied by `WithdrawCanonical` is also not lifted, because un-suspending needs a real placement dispatch (`ReplacePositionRows`); the entity rejoins the broadphase on its next placement | `src/AcDream.Runtime/Physics/RuntimeSetPositionState.cs` (`ParkDeferred`'s `restorableOnCancel`, `Forget`, `RestoreParkWithdrawal`) | The alternative — leaving the cancelled park's withdrawal in place — strands the entity invisible AND intangible for the rest of the session, because `CancelCoreDeferred` restores none of it and the operation that was the only thing able to wake it is gone. Restoring at the pre-park pose was tried and is wrong: retail commits the destination pose, and route 2's tests pin that the pose survives the cancel. Restore is scoped to the plain unplaceable-destination park only; quiescence/retirement parks are deliberately NOT restored, since re-admitting a spatial root into a retiring prefix blocks the retirement | A remote that teleports into a non-resident landblock and then STOPS MOVING stays visible at the destination without collision, where retail would hide it and re-show it on cell load — ACE stops broadcasting for a stationary entity, so no later packet corrects it. At 5-10 Hz the ordinary case is superseded within ~150 ms. Retire by making the park SURVIVE cancellation (issue #309), which is blocked on re-deciding the newer-Position-cancels-the-park invariant pinned by `NewerPositionPickupAndParentEachCancelExactLostOperation` and on teardown convergence. **This row carries a user-observable change to shipped paths** — `restorableOnCancel: true` sits in `SubmitPreparedPlacementCore`, the shared core behind every production placement — so it needs the two-client connected check written up in #309 (force a remote to park across a landblock boundary; confirm it no longer vanishes permanently, appears at the server-authoritative destination pose rather than a stale one, becomes collidable once the landblock publishes, and that route 2's local ForcePosition corrections are unchanged) | `CPhysicsObj::SetPositionInternal` @0x00515BD0 (@0x00515C1D/@0x00515CDA/@0x00515CE2/@0x00515CF2/@0x00515CF7/@0x00515D07); `CObjectMaint::GotoLostCell` @0x00508210; `CObjectMaint::InitObjCell` @0x00508260 (@0x00508296); `CPhysicsObj::reenter_visibility` @0x00516250 | ## 4. Temporary stopgap (TS) — 36 active rows (TS-62/TS-63 filed 2026-08-02, continuation-executor slice; TS-4 and TS-8 retired 2026-07-31; Campaign P's goal-enumerated physics stopgaps are now zero. TS-4's graph/flat Path-6 branches match retail's foot SetCollide/Adjusted and head CollisionNormal/Collided split with no BSP-layer sliding-normal write; TS-8's live 0x02C2 carries its complete StatMod through the canonical enchantment record and updates effective stats immediately. Campaign P P7 2026-07-30: TS-25 retired — outbound stance has shipped via RawState.CurrentStyle since #219; TS-24 re-argued to AD-57; TS-40 re-argued to AD-58; TS-35 retired at P5; earlier same campaign: TS-1/TS-5/TS-23/TS-46 retired by ports; TS-23 retired 2026-07-30 at Campaign P Slice P3 — every mover-flags call site (local player world-entry ×2, remote DR sweep ×2, remote teleport, ordinary movers) now ORs in the mover's real PK/PKLite/Impenetrable `ObjectInfoState` bits via the new `ClientObjectTable`-backed `EntityCollisionFlagsExt.ResolveMoverPvpState` — **narrative corrected 2026-08-03 (#297): "real" only became true at #297. Until then the bits existed but the source `PublicWeenieBitfield` was frozen at CreateObject, so every one of those sites read a stale value for the whole session. The site enumeration is also incomplete: `RuntimeSetPositionMoverPreparation.cs:183-188` is a SEVENTH mover-flags site that decodes `record.Snapshot.ObjectDescriptionFlags` directly rather than calling `ResolveMoverPvpState`, and it also derives `ObjectInfoState.IsPlayer` from the PWD bit, contradicting `EntityCollisionFlags.cs:119-123`'s claim that every site uses a GUID-prefix heuristic. See AP-134.** — and `PlayerWeenie.JumpStaminaCost`'s `pk` parameter reads the real `PlayerKillerStatus`/`LastPkAttackTimestamp` pair against a 20-second window instead of a hardcoded `false`; the non-PK invariant (every ACE default-created character) is bit-identical to the pre-P3 value since `ResolveMoverPvpState` and the PK-timer predicate both resolve to a no-op for `PublicWeenieBitfield` absent/0; TS-46 retired 2026-07-30 at Campaign P Slice P3 — the Setup's verbatim ≤2-sphere list (`CPhysicsObj::transition` 0x00512dc0 → `SPHEREPATH::init_sphere` 0x0050c670) now seeds the sweep for the local player, remote dead-reckoning, and ordinary movers alike, replacing the two-scalar (radius, height) capsule reconstruction; remote/ordinary step-up/step-down are now Setup-derived (`CPartArray::GetStepUpHeight`/`GetStepDownHeight`, 0x005180d0/0x005180f0, ×ObjScale) instead of a hardcoded 0.4 m, closing both residuals the row named; TS-5 retired 2026-07-30 at Campaign P Slice P1 — real burden-gated CanJump + real JumpStaminaCost, both decomp-verbatim; TS-1 retired 2026-07-30 at Campaign P Slice P2 — the row was stale; the EdgeSlide → PrecipiceSlide/CliffSlide chain is already a real, tested port; TS-57..TS-61 filed 2026-07-29 during Campaign N — no outbound RejectRetransmit; TS-27 narrowed same slice to the inbound direction) + TS-37 historical note (TS-20 retired 2026-07-16 — the later named-retail audit disproved the proposed DrawingBSP polygon filter; TS-37 is a retired-row historical note, not an active count; TS-39 retired R5-V3 — sticky seams bound to the ported PositionManager/StickyManager, radii threaded; TS-45 retired 2026-07-07 — hand-rolled `SphereCollision` replaced by the faithful CSphere family port, fixing the player-vs-monster crowd wedge; TS-3 retired 2026-07-07 — `frames_stationary_fall` accounting ported in the #182 verbatim UpdateObjectInternal rebuild, fixing the airborne falling-animation wedge; TS-41 retired 2026-07-07 — SERVERVEL synth-velocity remote body-drive replaced by the retail interp catch-up + unconditional MovementManager::UseTime, the remote-creature de-overlap #184; TS-42 retired 2026-07-19 — semantic animation completion now precedes the ordered Target/Movement/PartArray/Position tail; TS-44 narrowed again 2026-07-19 — complete orientation joined interpolation, only during-stick enqueue suppression remains) diff --git a/src/AcDream.Runtime/Entities/RuntimeEntityDirectory.cs b/src/AcDream.Runtime/Entities/RuntimeEntityDirectory.cs index 2b9ca433..07582f30 100644 --- a/src/AcDream.Runtime/Entities/RuntimeEntityDirectory.cs +++ b/src/AcDream.Runtime/Entities/RuntimeEntityDirectory.cs @@ -314,6 +314,23 @@ public sealed class RuntimeEntityDirectory record.SuspendObjectClock(); } + /// + /// Re-activates a clock suspended by , for + /// rolling back a withdrawal that is being cancelled rather than + /// completed. NOT an exact inverse — the retained sub-quantum time is + /// discarded, which is retail's set_active(1) rebase semantic; see + /// . Distinct from + /// , which is the + /// entering-the-world edge and additionally rebases the static/dynamic + /// quantum shape; a rollback re-activates a clock that never conceptually + /// left. + /// + public void ResumeObjectClock(RuntimeEntityRecord record) + { + EnsureKnown(record); + record.ResumeObjectClock(); + } + public void ResetObjectClockForEnterWorld(RuntimeEntityRecord record, bool isStatic) { EnsureKnown(record); diff --git a/src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs b/src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs index a3660e92..cc9d53cc 100644 --- a/src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs +++ b/src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs @@ -1706,7 +1706,17 @@ public sealed class RuntimeEntityObjectLifetime : IDisposable RuntimePlacementCancellationReceipt cancellation = default; if (acceptedPosition) { - cancellation = Physics.SetPosition.Forget(canonical); + // A CANCELLATION, not a withdrawal: a newer accepted Position + // supersedes the in-flight placement but the entity stays in the + // world. If that placement was a DeferredCell park, cancelling it + // without rolling the withdrawal back left the entity invisible + // AND intangible with nothing able to wake it - see + // RuntimeSetPositionState.Forget. Every OTHER Forget call in this + // class is a withdrawal transaction and deliberately does not opt + // in. + cancellation = Physics.SetPosition.Forget( + canonical, + restoreCancelledPark: true); } Entities.RefreshSnapshot( canonical, diff --git a/src/AcDream.Runtime/Entities/RuntimeEntityRecord.cs b/src/AcDream.Runtime/Entities/RuntimeEntityRecord.cs index 1d8def85..66530f0f 100644 --- a/src/AcDream.Runtime/Entities/RuntimeEntityRecord.cs +++ b/src/AcDream.Runtime/Entities/RuntimeEntityRecord.cs @@ -97,6 +97,29 @@ public sealed class RuntimeEntityRecord ObjectClockEpoch++; } + /// + /// Re-activates a clock suspended by . + /// + /// Deliberately NOT an exact inverse: + /// preserves the + /// retained sub-quantum _pending time while + /// zeroes it, so resuming + /// DISCARDS whatever fraction of a quantum was outstanding at suspend. + /// That is the retail semantic, not an oversight — retail's + /// set_active(1) rebases update_time to the current timer so + /// the reactivation frame does not catch up suppressed time. + /// + /// The epoch is also asymmetric on purpose: suspend bumps + /// unconditionally, resume bumps only on a real inactive-to-active edge + /// (Activate's own return contract), so a redundant resume is not + /// observable as a clock change. + /// + internal void ResumeObjectClock() + { + if (ObjectClock.Activate()) + ObjectClockEpoch++; + } + internal void ResetObjectClockForEnterWorld(bool isStatic) { ObjectClock.ResetForEnterWorld(isStatic); diff --git a/src/AcDream.Runtime/Physics/RuntimeSetPositionState.cs b/src/AcDream.Runtime/Physics/RuntimeSetPositionState.cs index 41d47d86..79eb7b18 100644 --- a/src/AcDream.Runtime/Physics/RuntimeSetPositionState.cs +++ b/src/AcDream.Runtime/Physics/RuntimeSetPositionState.cs @@ -410,6 +410,16 @@ internal sealed class RuntimeSetPositionState : IDisposable internal bool EnteringWorldFromCelllessResidence { get; set; } internal bool DormantLocalActivation { get; set; } + /// + /// The entity state overwrote, captured + /// BEFORE it snapped the body to the (unplaceable) destination and + /// ran . rolls + /// back to it when it cancels a wakeable park - see + /// for why the rollback target is + /// the PRE-PARK pose and never the destination. + /// + internal ParkWithdrawal ParkWithdrawal { get; set; } + /// /// #284: why this operation's mover preparation is currently parked, /// or when it is not. @@ -483,10 +493,22 @@ internal sealed class RuntimeSetPositionState : IDisposable DormantLocalActivation = false; ParkReason = RuntimeSetPositionParkReason.None; PreparedCommandAwaitingWithdrawalAck = null; + ParkWithdrawal = default; InPool = false; } } + /// + /// The exact set of entity state withdraws, + /// captured before it is overwritten so can roll a + /// cancelled park back instead of stranding the entity. + /// + private readonly record struct ParkWithdrawal( + bool Captured, + bool InWorld, + TransientStateFlags TransientState, + bool ClockActive); + private sealed class CollisionPrefixQuiescence { internal required RuntimeCollisionPrefixQuiescenceToken Token @@ -1288,8 +1310,15 @@ internal sealed class RuntimeSetPositionState : IDisposable ForgetPlacementCompletionCore(token); } + /// + /// — see . It + /// defaults to false so every shipped caller keeps its exact prior + /// behaviour; only a caller that is genuinely CANCELLING a placement + /// intent (rather than withdrawing the entity from the world) opts in. + /// internal RuntimePlacementCancellationReceipt ForgetExactPlacement( - in RuntimeEntityPlacementToken token) + in RuntimeEntityPlacementToken token, + bool restoreCancelledPark = false) { EnsureNotDisposed(); ForgetPlacementCompletionCore(token); @@ -1303,7 +1332,10 @@ internal sealed class RuntimeSetPositionState : IDisposable // against `_operations` immediately above with nothing reentrant in // between - passing it straight through is equivalent to (and safer // than) re-deriving it from `operation`. - return CancelCore(token.Entity, token); + return CancelCore( + token.Entity, + token, + restoreCancelledPark: restoreCancelledPark); } internal RuntimeEntityPlacementToken TryBeginExclusiveAuthoredPlacement( @@ -3016,7 +3048,10 @@ internal sealed class RuntimeSetPositionState : IDisposable _preparedMovers[operation.Key] = canonicalRequest; if (result.IsDeferred) - return ParkDeferred(operation, result); + // The one restorable park: the destination is not placeable right + // now. Every other ParkDeferred caller is a quiescence/retirement + // withdrawal - see ParkDeferred's restorableOnCancel doc. + return ParkDeferred(operation, result, restorableOnCancel: true); if (!CommitCanonical(operation, result)) { @@ -3302,22 +3337,155 @@ internal sealed class RuntimeSetPositionState : IDisposable return true; } + /// + /// Cancelling a wakeable lost-cell park ROLLS THE ENTITY BACK. Without + /// that, 's withdrawal (body.InWorld = + /// false, object clock suspended, + /// clearing canonical residency) outlived the operation that was the only + /// thing able to wake it: removes the + /// operation and restores none of that, so the entity was left invisible + /// AND intangible for the rest of the session. The producing sequence is + /// mundane - the accepted-Position merge + /// (RuntimeEntityObjectLifetime.TryApplyPosition) calls this on + /// EVERY accepted Position, so packet N parked and packet N+1 destroyed + /// the park ~150 ms later, before any collision-generation wake could + /// fire. + /// + /// This also closes the same hole on route 2's shipped + /// DeferredCell path, which was masked only by its re-issue funnel + /// - correct for a one-shot ForcePosition, absent for a repeated remote + /// stream. + /// + /// + /// distinguishes a CANCELLATION + /// from a WITHDRAWAL, and defaults to false so every shipped caller keeps + /// its exact prior behaviour. + /// + /// Only a cancellation may roll a park back. Most callers of this + /// method are withdrawal transactions - pickup, parent attach, explicit + /// withdrawal, position-channel teardown - which deliberately take the + /// entity OUT of the world and then hand-roll their own partial + /// re-withdrawal. Rolling a park back underneath one of those would set + /// body.InWorld = true and re-register the entity in + /// _spatialRoots, neither of which their cleanup undoes (it only + /// zeroes the clock and FullCellId) - leaving e.g. a picked-up item + /// simultaneously in inventory AND an InWorld cell-less spatial root + /// handed to the CPhysics::UseTime workset by + /// CopySpatialRootsTo, which filters only on key and + /// IsCurrent. + /// + /// The rollback exists because 's + /// withdrawal otherwise outlives the operation that was the only thing + /// able to wake it: restores neither + /// InWorld, the object clock, nor canonical residency, so the entity + /// was left invisible AND intangible for the rest of the session. The + /// accepted-Position merge produced that every ~150 ms - packet N parked, + /// packet N+1 destroyed the park before any collision-generation wake could + /// fire. It closes the same hole on route 2's shipped DeferredCell + /// path, which was masked only by its re-issue funnel. + /// internal RuntimePlacementCancellationReceipt Forget( RuntimeEntityRecord record, - bool releasePreparedMover = false) + bool releasePreparedMover = false, + bool restoreCancelledPark = false) { EnsureNotDisposed(); ArgumentNullException.ThrowIfNull(record); RuntimePlacementCancellationReceipt receipt = default; if (record.Key is { } key) { + // Captured before CancelCore, which retires the operation to the + // pool and resets every field on it. + ParkWithdrawal withdrawal = + restoreCancelledPark + && _operations.TryGetValue(key, out Operation? parked) + && parked.WakeableLostCell + && ReferenceEquals(parked.Record, record) + ? parked.ParkWithdrawal + : default; receipt = CancelCore(key); if (releasePreparedMover) _preparedMovers.Remove(key); + if (withdrawal.Captured) + RestoreParkWithdrawal(record, withdrawal); } return receipt; } + /// + /// Undoes 's WITHDRAWAL - InWorld, the + /// transient-state bits, the object clock, and canonical residency - so a + /// cancelled park cannot leave the entity invisible and intangible with + /// nothing able to wake it. + /// + /// The committed pose is deliberately left alone. + /// snaps the body to the destination before + /// withdrawing, which is precisely retail's lost-cell behaviour: + /// CPhysicsObj::SetPositionInternal @0x00515BD0 calls + /// store_position @0x00515CE2 on the no-cell branch and returns + /// OK_SPE. Retail commits the destination pose and never rolls it + /// back, and route 2's shipped tests pin the same contract by capturing + /// the pose AFTER the park and asserting it survives the cancel. Residency + /// is therefore restored at the body's committed cell, keeping residency + /// and pose consistent, rather than at the stale pre-park cell. + /// + /// Re-entering at a destination whose collision is not yet published + /// cannot re-arm 's overlap throw: + /// that throw fires only for a spatial root still holding an ACTIVE + /// operation, and the cancel this restore follows has already retired the + /// operation. An ordinary retirement re-parks the entity through the + /// normal path instead. + /// + /// Residual divergence, measured against retail rather than + /// labelled retail-shaped. Retail would keep the object hidden and + /// re-show it on cell load - a lost registration is removed ONLY by + /// CObjectMaint::InitObjCell @0x00508260, which drains the lost + /// list and calls CPhysicsObj::reenter_visibility @0x00508296 + /// (@0x00516250). We instead re-show it immediately at the committed + /// destination pose, uncollidable until its landblock publishes. For a + /// remote at 5-10 Hz the next packet supersedes within ~150 ms; a remote + /// that teleports into a non-resident landblock and then STOPS MOVING + /// holds that state, because ACE stops broadcasting for a stationary + /// entity. The retail-faithful end state is a park that SURVIVES + /// cancellation, blocked on re-deciding the newer-Position-cancels-the-park + /// invariant pinned by + /// NewerPositionPickupAndParentEachCancelExactLostOperation and on + /// teardown convergence. See the divergence register row and its issue. + /// + /// The shadow-object suspension + /// applied is NOT lifted here: ShadowObjectRegistry.Suspend drops + /// the owner's cell rows and only a real placement dispatch + /// (ReplacePositionRows) rebuilds them, so un-suspending would mean + /// re-running a placement re-entrantly from inside a cancel. The entity is + /// restored visible and simulated and rejoins the collision broadphase on + /// its next placement - recorded with the divergence above, not papered + /// over. + /// + private void RestoreParkWithdrawal( + RuntimeEntityRecord record, + in ParkWithdrawal withdrawal) + { + if (!_entities.IsCurrent(record)) + return; + uint residentCellId = 0u; + if (record.PhysicsBody is { } body) + { + body.InWorld = withdrawal.InWorld; + body.TransientState = withdrawal.TransientState; + residentCellId = body.CellPosition.ObjCellId; + } + if (withdrawal.ClockActive) + _entities.ResumeObjectClock(record); + if (residentCellId != 0u && record.FullCellId == 0u) + { + _entities.SetFullCell( + record, + residentCellId, + (residentCellId & 0xFFFF0000u) | 0xFFFFu); + _physics.AcknowledgeSpatialProjection(record, spatial: true); + } + } + internal void LeaveWorld(RuntimeEntityRecord record) { EnsureNotDisposed(); @@ -4085,14 +4253,39 @@ internal sealed class RuntimeSetPositionState : IDisposable /// so derived false and /// the parked operation skipped the QuiescenceHeld stage entirely. /// + /// + /// opts this park into + /// 's rollback. It is true for exactly one caller: + /// the plain "destination is not placeable right now" park, which is the + /// one that strands an entity when a later packet cancels it. + /// + /// It is deliberately FALSE for every quiescence/retirement park. + /// Those withdraw the entity precisely because its own landblock prefix + /// is being retired or held, so restoring canonical residency there would + /// re-admit a spatial root into the prefix that is trying to quiesce and + /// block the retirement outright - a streaming stall traded for a + /// stranded entity. Those entities are withdrawn because their world is + /// going away, not because a destination was momentarily unavailable. + /// private RuntimeSetPositionOutcome ParkDeferred( Operation operation, in PhysicsSetPositionResult result, bool publishImmediately = true, ulong? collisionGenerationOverride = null, - uint? collisionPrefixOverride = null) + uint? collisionPrefixOverride = null, + bool restorableOnCancel = false) { PhysicsBody body = operation.Body!; + // Captured BEFORE the withdrawal below clears it. Only the + // WITHDRAWAL is captured, never the pose: the SnapToCell on the next + // line deliberately commits the destination pose and that commit + // stands, exactly as retail's store_position @0x00515CE2 commits it + // on the lost-cell branch and never un-commits it. + operation.ParkWithdrawal = new ParkWithdrawal( + Captured: restorableOnCancel, + InWorld: body.InWorld, + TransientState: body.TransientState, + ClockActive: operation.Record.ObjectClock.IsActive); body.Orientation = result.Orientation; body.SnapToCell( result.CellId, @@ -5216,18 +5409,31 @@ internal sealed class RuntimeSetPositionState : IDisposable private RuntimePlacementCancellationReceipt CancelCore( RuntimeEntityKey key, in RuntimeEntityPlacementToken expectedToken, - bool preserveLostFamily = false) + bool preserveLostFamily = false, + bool restoreCancelledPark = false) { if (!_operations.TryGetValue(key, out Operation? current) || current.Token != expectedToken) { return default; } + // Opt-in only - see Forget's doc comment for why a withdrawal must + // never roll a park back. This overload is the path route 2's + // controller and route 4b's remote controller cancel through + // (ForgetExactPlacement); they opt in, the initial-create residence + // and continuation executors deliberately do not. + RuntimeEntityRecord parkedRecord = current.Record; + ParkWithdrawal withdrawal = + restoreCancelledPark && current.WakeableLostCell + ? current.ParkWithdrawal + : default; _ = CancelCoreDeferred( key, cancelLostFamily: false, preserveLostFamily, out RuntimePlacementProjectionSnapshot? discard); + if (withdrawal.Captured) + RestoreParkWithdrawal(parkedRecord, withdrawal); return discard is { } projection ? new RuntimePlacementCancellationReceipt(projection) : default; diff --git a/src/AcDream.Runtime/Session/RuntimeAcceptedPositionDriveController.cs b/src/AcDream.Runtime/Session/RuntimeAcceptedPositionDriveController.cs index 5f5709ce..1af11389 100644 --- a/src/AcDream.Runtime/Session/RuntimeAcceptedPositionDriveController.cs +++ b/src/AcDream.Runtime/Session/RuntimeAcceptedPositionDriveController.cs @@ -307,8 +307,13 @@ public sealed class RuntimeAcceptedPositionDriveController _pending = null; RuntimeSetPositionState setPosition = _entityObjects.Physics.SetPosition; setPosition.ForgetPlacementCompletion(pending.Token); + // Cancellation, not withdrawal: the placement intent is abandoned but + // the local player stays in the world, so a DeferredCell park must be + // rolled back rather than left stranding the body. RuntimePlacementCancellationReceipt cancellation = - setPosition.ForgetExactPlacement(pending.Token); + setPosition.ForgetExactPlacement( + pending.Token, + restoreCancelledPark: true); if (cancellation.IsValid) setPosition.PublishCancellation(cancellation); } @@ -796,8 +801,11 @@ public sealed class RuntimeAcceptedPositionDriveController RuntimeSetPositionState setPosition, in RuntimeEntityPlacementToken token) { + // Cancellation, not withdrawal - see AbandonPending. RuntimePlacementCancellationReceipt cancellation = - setPosition.ForgetExactPlacement(token); + setPosition.ForgetExactPlacement( + token, + restoreCancelledPark: true); if (cancellation.IsValid) setPosition.PublishCancellation(cancellation); } diff --git a/tests/AcDream.Runtime.Tests/Physics/RuntimeCollisionPrefixQuiescenceTests.cs b/tests/AcDream.Runtime.Tests/Physics/RuntimeCollisionPrefixQuiescenceTests.cs index a244db9c..57c5271c 100644 --- a/tests/AcDream.Runtime.Tests/Physics/RuntimeCollisionPrefixQuiescenceTests.cs +++ b/tests/AcDream.Runtime.Tests/Physics/RuntimeCollisionPrefixQuiescenceTests.cs @@ -319,6 +319,95 @@ public sealed partial class RuntimeCollisionPrefixQuiescenceTests submitted.Status); } + /// + /// C4 route 4b-1 review finding B2, pinned. The contract asked whether + /// ParkCollisionResidents's overlap throw stays unreachable under a + /// design that lets remotes hold placement operations. It does, and for a + /// STRUCTURAL reason rather than an incidental one: + /// TryAcquireCollisionPrefixMutationPermission consults + /// HasOldPrefixPlacementDebt FIRST and refuses permission outright, + /// so ParkCollisionResidents is never entered while any affected + /// root holds an operation. (Additionally, a parked record has + /// FullCellId == 0, so it is not an affected resident at all.) + /// + /// The real hazard is not a throw, it is an unbounded streaming + /// stall — and this test PINS it as a stall, not as a pass. A retained + /// preparation retry keeps its landblock prefix in placement debt, so the + /// retirement is refused on EVERY poll and the landblock never retires. + /// There is no bound: the retirement coordinator simply retries, and + /// TickLostCellDeadlines — the only expiry that could break the + /// cycle — has NO production caller, so its deadline never fires. The only + /// thing that clears it is an inbound packet for that same entity, which + /// is exactly what a RetrySetupUnavailable on an asset that never + /// loads does not produce. + /// + /// This is a pre-existing hazard independent of route 4b-1, filed as + /// its own issue. 4b-1 does NOT bound it; it only avoids widening it, by + /// declining to retain operations for destinations it cannot service. + /// + [Fact] + public void RetainedPreparationRetryStallsPrefixRetirementIndefinitely() + { + using var fixture = new Fixture(); + RuntimeEntityRecord record = fixture.Add( + 0x7000300Bu, + 1, + CellP, + new Vector3(10f, 28f, 7f)); + + // A retained preparation retry: begun, never prepared — the shape a + // RetrySetupUnavailable on an asset that never resolves leaves behind. + RuntimeEntityPlacementToken retained = fixture.Lifetime.Physics + .SetPosition.BeginAcceptedPlacement( + record, + record.PositionAuthorityVersion, + RuntimeSetPositionOperationKind.RemoteAuthoritative); + Assert.True(retained.IsValid); + + RuntimeCollisionPrefixQuiescenceToken token = fixture.Begin(2UL); + + // The production retirement path, polled hard. Permission is refused + // every single time; nothing in the system advances it. + for (int poll = 0; poll < 1_000; poll++) + { + Assert.False( + fixture.TryAcquire(token, out _), + $"retirement unexpectedly acquired permission on poll {poll}; " + + "if this now succeeds the stall has been bounded and " + + "this test's pinned decision must be revisited"); + } + + // ParkCollisionResidents was never entered, so its overlap throw could + // not fire — the contract's item 6, proven structurally. + Assert.True(fixture.Lifetime.Physics.IsSpatialRoot(record)); + Assert.False(fixture.Lifetime.Physics.SetPosition.TryPeekProjection( + out _)); + + // And the discriminator: retiring the retained operation is what + // releases the prefix. Once the debt is gone the ordinary two-phase + // handshake proceeds — ParkCollisionResidents withdraws the residents + // and permission follows the withdrawal acknowledgements — so drain + // those exactly as the production host does. + _ = fixture.Lifetime.Physics.SetPosition.ForgetExactPlacement(retained); + bool acquired = false; + for (int poll = 0; poll < 32 && !acquired; poll++) + { + acquired = fixture.TryAcquire(token, out _); + if (acquired) + break; + if (fixture.Lifetime.Physics.SetPosition.TryPeekProjection( + out RuntimePlacementProjectionSnapshot projection)) + { + Assert.True(fixture.Lifetime.Physics.SetPosition + .AcknowledgeProjection(projection.Token)); + } + } + Assert.True( + acquired, + "clearing the retained preparation retry must let the prefix " + + "retire; if it does not, the stall has a second cause"); + } + [Fact] public void QueriedNeighborPrefixHoldsResultWithoutRequestDependency() { diff --git a/tests/AcDream.Runtime.Tests/Physics/RuntimeSetPositionStateTests.cs b/tests/AcDream.Runtime.Tests/Physics/RuntimeSetPositionStateTests.cs index 1cd31f01..0fba612b 100644 --- a/tests/AcDream.Runtime.Tests/Physics/RuntimeSetPositionStateTests.cs +++ b/tests/AcDream.Runtime.Tests/Physics/RuntimeSetPositionStateTests.cs @@ -1487,6 +1487,126 @@ public sealed class RuntimeSetPositionStateTests placed.Token)); } + /// + /// C4 route 4b-1 review, the decisive case. ParkDeferred withdraws + /// the entity from the world (InWorld = false, object clock + /// suspended, canonical residency dropped). The merge-time + /// Forget that every accepted Position performs + /// (RuntimeEntityObjectLifetime.TryApplyPosition) then removes the + /// operation and restores NONE of that, so the entity is left invisible + /// AND intangible with nothing left that could ever wake it. + /// + /// Retail cannot reach that state: CPhysicsObj::SetPositionInternal + /// @0x00515BD0 commits the destination pose via store_position + /// @0x00515CE2 and registers the object with + /// CObjectMaint::GotoLostCell @0x00515CF2 / @0x00508210, and the + /// ONLY remover of that registration is + /// CObjectMaint::InitObjCell @0x00508260, which drains the lost + /// list on cell load and calls CPhysicsObj::reenter_visibility + /// @0x00508296 / @0x00516250 for every object in it. A retail lost object + /// is never silently un-registered. + /// + [Fact] + public void CancellingWakeableParkLeavesEntityWithdrawnWithNothingToWakeIt() + { + PhysicsEngine engine = FlatEngine(SourceLandblock, 0f); + using var lifetime = new RuntimeEntityObjectLifetime(engine); + RuntimeEntityRecord record = CreateRecord(lifetime, 0x7000411Bu, 1); + PhysicsBody body = AttachBody(lifetime, record, SourceCell); + + Assert.True(body.InWorld); + Assert.True(record.ObjectClock.IsActive); + Assert.Equal(SourceCell, record.FullCellId); + + RuntimeSetPositionOutcome parked = lifetime.Physics.SetPosition.Apply( + record, + record.PositionAuthorityVersion, + Command(CrossLandblockRequest())); + + Assert.Equal(RuntimeSetPositionStatus.DeferredCell, parked.Status); + Assert.True(lifetime.Physics.SetPosition.IsDeferred(record)); + Assert.False(body.InWorld); + Assert.False(record.ObjectClock.IsActive); + Assert.Equal(0u, record.FullCellId); + Vector3 parkedPosition = body.Position; + + // The merge-time cancel every accepted Position performs. + _ = lifetime.Physics.SetPosition.Forget( + record, + restoreCancelledPark: true); + + // The park is gone (the shipped newer-Position-cancels-the-park + // invariant, pinned by + // NewerPositionPickupAndParentEachCancelExactLostOperation) - so the + // entity MUST have been rolled back, or nothing is left to wake it. + Assert.False(lifetime.Physics.SetPosition.IsDeferred(record)); + Assert.True( + body.InWorld, + "cancelled park left the entity withdrawn: InWorld=false"); + Assert.True( + record.ObjectClock.IsActive, + "cancelled park left the object clock suspended"); + Assert.True( + record.FullCellId != 0u, + "cancelled park left the entity without canonical residency"); + Assert.True( + lifetime.Physics.IsSpatialRoot(record), + "cancelled park left the entity out of the physics workset"); + Assert.Equal( + TransientStateFlags.Active, + body.TransientState & TransientStateFlags.Active); + + // The pose committed at park time STANDS - retail's store_position + // @0x00515CE2 commits the destination on the lost-cell branch and + // never un-commits it. Residency follows the body's committed cell so + // the two cannot disagree. + Assert.Equal(parkedPosition, body.Position); + Assert.Equal(body.CellPosition.ObjCellId, record.FullCellId); + } + + /// + /// The sibling cancel entry point. Route 2's controller and route 4b's + /// remote controller both cancel a DeferredCell park through + /// ForgetExactPlacement, not through Forget, so the rollback + /// has to live at the shared CancelCore layer or the identical + /// stranded-entity hole stays open on exactly the routes that produce it. + /// + [Fact] + public void CancellingWakeableParkByExactTokenAlsoRestoresTheEntity() + { + PhysicsEngine engine = FlatEngine(SourceLandblock, 0f); + using var lifetime = new RuntimeEntityObjectLifetime(engine); + RuntimeEntityRecord record = CreateRecord(lifetime, 0x7000411Cu, 1); + PhysicsBody body = AttachBody(lifetime, record, SourceCell); + + RuntimeEntityPlacementToken token = lifetime.Physics.SetPosition + .BeginAcceptedPlacement( + record, + record.PositionAuthorityVersion, + RuntimeSetPositionOperationKind.RemoteAuthoritative); + Assert.True(token.IsValid); + RuntimeSetPositionOutcome parked = lifetime.Physics.SetPosition + .SubmitPreparedPlacement(token, Command(CrossLandblockRequest())); + Assert.Equal(RuntimeSetPositionStatus.DeferredCell, parked.Status); + Assert.False(body.InWorld); + Assert.False(record.ObjectClock.IsActive); + Assert.Equal(0u, record.FullCellId); + + _ = lifetime.Physics.SetPosition.ForgetExactPlacement( + token, + restoreCancelledPark: true); + + Assert.False(lifetime.Physics.SetPosition.IsDeferred(record)); + Assert.True(body.InWorld, "exact-token cancel left the entity withdrawn"); + Assert.True( + record.ObjectClock.IsActive, + "exact-token cancel left the object clock suspended"); + Assert.True( + record.FullCellId != 0u, + "exact-token cancel left the entity without canonical residency"); + Assert.True(lifetime.Physics.IsSpatialRoot(record)); + } + [Fact] public void ReentrantNewerPositionDuringPickupDiscardSuppressesStalePickupDelta() { @@ -2406,6 +2526,7 @@ public sealed class RuntimeSetPositionStateTests "DormantLocalActivation", "ParkReason", "PreparedCommandAwaitingWithdrawalAck", + "ParkWithdrawal", "InPool", ]; string[] expectedFieldNames = expectedPropertyNames @@ -2985,6 +3106,38 @@ public sealed class RuntimeSetPositionStateTests .DeferredSetPositionCount); Assert.Equal(0, lifetime.Physics.CaptureOwnership() .LostCellDeadlineCount); + + // Pickup and Parent are WITHDRAWAL transactions, not cancellations: + // each hand-rolls its own partial re-withdrawal after the Forget, + // undoing the object clock and FullCellId but NOTHING else. The + // park-rollback the accepted-Position merge opts into must never fire + // underneath them - it would set InWorld and re-add the entity to + // _spatialRoots, neither of which their cleanup undoes, leaving a + // picked-up item both in inventory AND an InWorld cell-less spatial + // root inside the CPhysics::UseTime workset. The counts above cannot + // see either field, so assert them directly. + // + // Position is the one CANCELLATION here: a newer accepted Position + // supersedes the placement while the entity stays in the world, so it + // must roll back rather than strand the body. + if (channel is CancellationChannel.Position) + { + Assert.True( + record.PhysicsBody?.InWorld ?? false, + "the cancellation channel must roll the park back"); + Assert.True( + lifetime.Physics.IsSpatialRoot(record), + "the cancellation channel must restore the spatial root"); + } + else + { + Assert.False( + record.PhysicsBody?.InWorld ?? false, + "a withdrawal channel must not leave the body InWorld"); + Assert.False( + lifetime.Physics.IsSpatialRoot(record), + "a withdrawal channel must not leave the entity a spatial root"); + } } private static void CommitParent(