test(physics): C4 route 6 — drops/split-recovery closure, zero production lines
Route 6 needs no production change and this commit contains none: C3c (529e0e9d) already flipped both hosts' Create paths onto the residence lease, so a dropped item is byte-for-byte route 1's create classification (RuntimePositionEntityKind.Remote, RuntimeCreateResidenceKind.TopLevel, ClassifyCreate -> SetPosition with InitialCreateFlags = Placement | Slide). Route 6 is a SOURCE of route-1 traffic, not a route of its own. Both drop flavours converge on LiveEntityHydrationController.OnCreate -> RegisterEntityWithInitialResidence — the whole-item drop through ItemInteractionController's DropToWorld (no physics, no position; the server decides), and split-to-world through TryRecoverUnknownPosition's call to the identical entry point. Contract: docs/research/2026-08-04-c4-route-6-contract.md. Retires a FALSE PREMISE from the campaign plan (:97-100), which claimed split-recovery creates "need an effect-replay suppression signal". Verified against the decomp instead of assumed: play_default_script @0x005132B0 / @0x00513300 has exactly three call sites in the entire pseudo-C dump — DefaultScriptPartHook::Execute @0x00526c08, DefaultScriptHook::Execute @0x00526c14, and ACCWeenieObject::DoCollision @0x0058c3b4 — and NONE from set_description or CreateObject. Neither client plays a default script at create, so there is nothing to suppress. acdream's only create-time replay is the F754/F755 queue drain keyed by server GUID, which is retail's own HandleCreateObject @0x00454C80 behaviour. The plan's other two clauses were closed at C0 (TryCommitParent/CommitWithdrawal cancellation symmetry; host-visible cancellation receipts); the list now states what actually remains — route 7's child-cell two-writer split and the headless parent-realize gap. Retail split marking recorded for the record: UIAttemptSplitTo3D @0x0058D850 stores only splitStackSize/splitClassID/splitTime and performs no placement; DeclareValid @0x0058E340's recovery action is SetSelectedObject @0x0058E481 — a SELECTION transfer with a 10-second expiry, not effect suppression and not placement. UIAttemptPutIn3D @0x0058D700 records no marker at all. Seven tests over the now-flipped path (whole item, split stack, new-GUID recovery, second drop, unavailable destination, newer Position after the pending identity is consumed, plus the #314 repro), each sabotage-verified: the production path was broken on purpose, the test was confirmed to fail, and the sabotage reverted. R6-c is now settled by assertion rather than argument — BuildSpawn's wholesale clone of Children/Movement/AnimationFrame/ SetupTableId is measured, not reasoned about. FOUND WHILE TESTING — #314, filed not fixed (this route is zero-production by contract). BuildSpawn resets top-level MovementSequence/ServerControlSequence to 0 but its Timestamps `with` block overrides only Position/Teleport/ ForcePosition/Instance, leaving Physics.Timestamps.Movement and .ServerControlledMove at the SOURCE item's values. HasConsistentCreateIdentityAndParent requires the two projections to agree, so a split whose source carries nonzero Movement timestamps — plausible for any item dropped once, picked up, and split again — fails the predicate and throws instead of completing the canonical transaction. Verified in source, not taken on report. Note this is a crash in the exact mechanism the scoping cited as EVIDENCE that drops already converge: code reading said the path converges, driving it said it throws. Fixed in the immediately following commit. Also filed: #313 (DeclareValid's SetSelectedObject port is missing and the container-split flavour records no marker — selection UX, deliberately not implemented inside a placement closure) and #315 (route 4b-3's per-packet runTeleportHook Func<bool> closure at three RunRemoteArmTail call sites; the network packet path, not Slice I's per-frame resolve path — filed now because route 5 adds a fourth site). AP-124 stays open and registered. Test lines are 410 against a 150-250 guidance, accepted: the excess is a real ItemInteractionController harness plus the #314 repro, which is what found the defect. A mock that proved nothing would have been shorter and worthless. Complete Release suite MEASURED at 11,020 passed / 4 skipped / 0 failed (baseline 11,013/4/0 at6dc7ba51; +7 new). Neither known flake fired. Connected gate (user-run) still owed: drop a whole item, split a stack to the ground, drop a second within ~1 m, repeat indoors and after a portal recall, then walk two landblocks away and back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
21cd6e9b2b
commit
1b484937b6
5 changed files with 694 additions and 4 deletions
141
docs/ISSUES.md
141
docs/ISSUES.md
|
|
@ -24,6 +24,147 @@ 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 6 — drops and split-recovery closure — 2026-08-04
|
||||
|
||||
#313 and #314 filed from the route 6 closure session (zero production lines;
|
||||
evidence + coverage tests only). #315 filed from the same session, carried
|
||||
over from the route 4b-3 round-2 reviews. Evidence:
|
||||
[`2026-08-04-c4-route-6-contract.md`](research/2026-08-04-c4-route-6-contract.md),
|
||||
[`2026-08-04-c4-routes-6-7-scoping.md`](research/2026-08-04-c4-routes-6-7-scoping.md).
|
||||
|
||||
## #313 — `DeclareValid`'s `SetSelectedObject` split-recovery is not ported
|
||||
|
||||
**Status:** OPEN
|
||||
**Severity:** LOW (selection UX, not placement)
|
||||
**Filed:** 2026-08-04
|
||||
**Component:** UI / inventory / selection
|
||||
|
||||
**Description:** Retail's `ACCWeenieObject::DeclareValid @0x0058E340` reads
|
||||
the split marker recorded by `UIAttemptSplitTo3D @0x0058D850` /
|
||||
`UIAttemptSplitToContainer @0x0058D7D0` (three fields: `splitStackSize`,
|
||||
`splitClassID`, `splitTime`) and, on a matching WCID + stack-size within the
|
||||
10-second window, runs `ACCWeenieObject::SetSelectedObject(this->id, 0)`
|
||||
@0x0058E481 — a SELECTION transfer to the newly-materialized split result,
|
||||
not effect suppression and nothing placement-related (verified against
|
||||
`acclient_2013_pseudo_c.txt`; see
|
||||
`docs/research/2026-08-04-c4-route-6-contract.md`). acdream's
|
||||
`PendingSplitToWorldProjection`
|
||||
(`src/AcDream.App/World/InventoryWorldDropProjectionController.cs`)
|
||||
implements the 10-second recognition window (`RetailRecognitionSeconds`) but
|
||||
has no selection dependency at all — the split result never becomes the
|
||||
selected object after a ground split, and the container-split flavor
|
||||
(`UIAttemptSplitToContainer`'s equivalent) records no marker at all.
|
||||
|
||||
**Root cause:** `InventoryWorldDropProjectionController`'s constructor takes
|
||||
interaction / objects / runtime / hydration / clock and nothing selection-
|
||||
related; `PendingSplitToWorldProjection.TryResolve` never calls anything
|
||||
resembling `SetSelectedObject`.
|
||||
|
||||
**Files:** `src/AcDream.App/World/InventoryWorldDropProjectionController.cs`.
|
||||
Whatever owns "currently selected object" client-side (search for
|
||||
`selectedObjectId` — `ItemInteractionController` already takes one as a
|
||||
`Func<uint>`, so the write side needs a matching setter/owner).
|
||||
|
||||
**Acceptance:** Split a partial stack to the ground; the newly-created pile
|
||||
becomes the selected object (matching retail's post-split selection
|
||||
behavior), with a 10-second recognition window identical to the existing
|
||||
recovery window. Out of C4 scope — do not implement as part of a placement-
|
||||
focused change; this is selection UX and mixing it into a placement closure
|
||||
makes the landing un-reviewable (per the route 6 contract).
|
||||
|
||||
## #314 — Split recovery throws instead of recovering when the source's retained Movement/ServerControlledMove timestamps are nonzero
|
||||
|
||||
**Status:** OPEN
|
||||
**Severity:** MEDIUM (can turn a normal split-to-ground into a client
|
||||
exception instead of a placed item)
|
||||
**Filed:** 2026-08-04
|
||||
**Component:** physics / inventory / entity lifetime
|
||||
|
||||
**Description:** Discovered while writing C4 route 6's "split stack" / "new
|
||||
GUID recovery" coverage tests
|
||||
(`tests/AcDream.App.Tests/World/LiveEntityHydrationControllerTests.cs`,
|
||||
`SplitSourceWithRetainedMovementTimestamps_ThrowsInsteadOfRecovering`).
|
||||
`PendingSplitToWorldProjection.BuildSpawn`
|
||||
(`src/AcDream.App/World/InventoryWorldDropProjectionController.cs:171-209`)
|
||||
resets the top-level `MovementSequence` / `ServerControlSequence` to `0`
|
||||
(`:201-202`) when constructing the synthetic spawn for the new split-result
|
||||
GUID, but its `Physics.Timestamps` override list only touches `Position` /
|
||||
`Teleport` / `ForcePosition` / `Instance` (`:182-188`) — it does NOT reset
|
||||
`Physics.Timestamps.Movement` / `.ServerControlledMove` to match. Those two
|
||||
fields instead retain the SOURCE item's original values verbatim.
|
||||
|
||||
`RuntimeEntityObjectLifetime.HasConsistentCreateIdentityAndParent`
|
||||
(`src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs:2321-2327`)
|
||||
requires the flattened top-level sequence fields to agree exactly with the
|
||||
embedded `PhysicsSpawnData.Timestamps` — by design, since they are two
|
||||
projections of the same wire packet
|
||||
(`RegisterEntityCore` throws `"CreateObject 0x{guid} has inconsistent
|
||||
instance or parent projections."` at `:748-749` when they disagree). Retail's
|
||||
per-object `update_times` timestamp channels are monotonic counters that do
|
||||
NOT reset when an item re-enters a container, so any split source that ever
|
||||
received a Movement or ServerControlledMove wire update during an earlier
|
||||
stint with world presence (e.g. dropped once before, picked back up, split
|
||||
again) carries nonzero values in exactly the two fields `BuildSpawn` forgets
|
||||
to reset. The split recovery then throws `InvalidOperationException` instead
|
||||
of completing the canonical create-placement transaction, inside
|
||||
`InventoryWorldDropProjectionController.TryRecoverUnknownPosition` — an
|
||||
unhandled exception on the ordinary network/UI event path.
|
||||
|
||||
**Root cause:** Asymmetric field reset in `BuildSpawn`'s two `with`
|
||||
expressions — the top-level projection and the embedded `PhysicsSpawnData`
|
||||
projection of the same synthetic spawn are constructed independently and
|
||||
fell out of sync.
|
||||
|
||||
**Fix shape (not applied — C4 route 6 is a zero-production-line closure by
|
||||
contract):** either also reset `Timestamps.Movement` / `.ServerControlledMove`
|
||||
to `0` in `BuildSpawn`'s `Timestamps with { ... }` block, or don't reset the
|
||||
top-level `MovementSequence` / `ServerControlSequence` at all and let them
|
||||
inherit the source's values instead (whichever direction is retail-correct
|
||||
needs a decompiled cross-check of what `UIAttemptSplitTo3D`'s resulting
|
||||
CreateObject actually carries for these two channels — not established by
|
||||
this filing).
|
||||
|
||||
**Files:** `src/AcDream.App/World/InventoryWorldDropProjectionController.cs:182-188,200-207`;
|
||||
consumed by `src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs:2321-2327`.
|
||||
|
||||
**Acceptance:** Split a stack of an item whose weenie has previously been
|
||||
dropped to the ground and picked back up (so its retained Movement /
|
||||
ServerControlledMove timestamp channels are nonzero) a second time; the
|
||||
split result places normally instead of throwing.
|
||||
|
||||
## #315 — `runTeleportHook` builds a `Func<bool>` closure per network packet
|
||||
|
||||
**Status:** OPEN
|
||||
**Severity:** LOW (real allocation regression, not correctness; not on the
|
||||
per-frame resolve path Slice I's 0 B/resolve discipline governs)
|
||||
**Filed:** 2026-08-04
|
||||
**Component:** physics / networking
|
||||
|
||||
**Description:** Carried over from the C4 route 4b-3 round-2 architecture
|
||||
reviews (both said defer, but flagged that route 5 will add a fourth call
|
||||
site once it lands). Three `RunRemoteArmTail` call sites currently in
|
||||
`src/AcDream.App/Physics/LiveEntityNetworkUpdateController.cs` each build a
|
||||
`Func<bool>` delegate per inbound packet to pass into
|
||||
`ApplyRemoteContactRouting`. This is a real allocation regression versus the
|
||||
`3e002993` baseline, on the 5-10 Hz network packet path — not the per-frame
|
||||
physics resolve path Slice I's zero-allocation discipline covers, so it did
|
||||
not show up in that gate.
|
||||
|
||||
**Root cause:** `ApplyRemoteContactRouting`'s public signature takes a
|
||||
`Func<bool>` parameter, and existing tests inject lambdas into it directly —
|
||||
changing the signature to a non-allocating shape (a struct callback, a
|
||||
cached delegate, or an explicit two-phase call) touches test call sites
|
||||
across the file, which is why both round-2 reviews deferred it rather than
|
||||
fixing it inline.
|
||||
|
||||
**Files:** `src/AcDream.App/Physics/LiveEntityNetworkUpdateController.cs` —
|
||||
`RunRemoteArmTail` call sites feeding `ApplyRemoteContactRouting`.
|
||||
|
||||
**Acceptance:** The three (four, once route 5 lands) `RunRemoteArmTail` call
|
||||
sites do not allocate a fresh delegate per packet; existing
|
||||
`ApplyRemoteContactRouting` tests continue to pass, updated for whatever
|
||||
non-allocating shape replaces the `Func<bool>` parameter.
|
||||
|
||||
## 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
|
||||
|
|
|
|||
|
|
@ -94,10 +94,25 @@ same commit) → docs/handoff commit. No workarounds; no fused slices.
|
|||
- The exact-Setup mover chain (`PrepareMover` /
|
||||
`RuntimeSetPositionMoverPreparer.TryBuild` /
|
||||
`IPreparedCollisionSource.ReadSetupCollision`) exists piecewise, unwired.
|
||||
- Route-6 split-recovery creates need an effect-replay suppression signal;
|
||||
route-7 needs `TryCommitParent`/`CommitWithdrawal` cancellation-symmetry
|
||||
fixes and host-visible cancellation receipts; headless lacks any
|
||||
parent-realize sequence (pre-existing, adjacent).
|
||||
- **Corrected 2026-08-04 (C4 route 6 closure,
|
||||
`docs/research/2026-08-04-c4-route-6-contract.md`): all three clauses
|
||||
above were stale.** Route-6 split-recovery does NOT need an effect-replay
|
||||
suppression signal — that premise was unsubstantiated; acdream's only
|
||||
create-time effect replay is the F754/F755 queue drain keyed by server
|
||||
GUID, and the one plausible mechanism (a cloned `DefaultScriptType`
|
||||
surviving `BuildSpawn`) never fires at create in either client
|
||||
(`CPhysicsObj::play_default_script @0x005132B0`/`@0x00513300` has exactly
|
||||
two callers, both animation hooks, verified against
|
||||
`acclient_2013_pseudo_c.txt`). Route-7's `TryCommitParent`/
|
||||
`CommitWithdrawal` cancellation-symmetry fixes and host-visible
|
||||
cancellation receipts were BOTH closed at C0 (see the C0 slice below).
|
||||
What actually remains for route 7: the child's canonical cell has two
|
||||
writers (Runtime commits it cell-less unconditionally in
|
||||
`CommitAcceptedParentCellless`, while `EquippedChildRenderController
|
||||
.TickChild` re-cells it from a per-frame render tick), and headless has no
|
||||
`EquippedChildRenderController` at all, so every headless parented child
|
||||
stays cell-less forever — the same defect seen from two sides, not two
|
||||
separate gaps.
|
||||
|
||||
## Slices
|
||||
|
||||
|
|
|
|||
124
docs/research/2026-08-04-c4-route-6-contract.md
Normal file
124
docs/research/2026-08-04-c4-route-6-contract.md
Normal file
|
|
@ -0,0 +1,124 @@
|
|||
# C4 route 6 — drops and split-recovery: pinned contract (2026-08-04)
|
||||
|
||||
Scoped in
|
||||
[`2026-08-04-c4-routes-6-7-scoping.md`](2026-08-04-c4-routes-6-7-scoping.md)
|
||||
§6. Pinned after route 4b-3 landed (`6dc7ba51`, gate passed `21cd6e9b`).
|
||||
|
||||
**Route 6 requires ZERO production lines.** It is a closure, not a slice: C3c
|
||||
(`529e0e9d`) already flipped both hosts' Create paths onto the residence lease,
|
||||
and a dropped item is byte-for-byte route 1's create classification. The
|
||||
deliverable is evidence + coverage tests + one planning-record correction.
|
||||
|
||||
**If implementing this contract requires a production change, STOP AND REPORT.**
|
||||
A production diff here means the scoping's central finding is wrong, and that
|
||||
is a finding worth more than the slice.
|
||||
|
||||
## Why there is nothing to build
|
||||
|
||||
A dropped item is an ordinary non-local CreateObject:
|
||||
`RuntimePositionEntityKind.Remote`, `RuntimeCreateResidenceKind.TopLevel`,
|
||||
`ClassifyCreate` → disposition `SetPosition` with
|
||||
`InitialCreateFlags = Placement | Slide`. Route 6 is a *source* of route-1
|
||||
traffic, not a route of its own.
|
||||
|
||||
Both drop flavours already converge on the canonical transaction:
|
||||
|
||||
- **Whole-item drop** — `ItemInteractionController.ExecutePlacementActions`'s
|
||||
`DropToWorld` case sends the drop with no physics and no position; the
|
||||
server's CreateObject returns through
|
||||
`LiveEntityHydrationController.OnCreate` → `RegisterLiveEntity` →
|
||||
`RegisterEntityWithInitialResidence`.
|
||||
- **Split-to-world** — `InventoryWorldDropProjectionController`'s
|
||||
`TryRecoverUnknownPosition` calls **the identical `_hydration.OnCreate`
|
||||
entry point**. Same residence lease, same conductor, same placement.
|
||||
|
||||
So the campaign handoff's route-6 requirement ("`TryRecoverUnknownPosition`
|
||||
may create the logical object, but it must enter the same canonical
|
||||
create-placement transaction") is already met, by C3c, with zero route-6 code.
|
||||
|
||||
## The false premise this closure exists to retire
|
||||
|
||||
`docs/plans/2026-08-02-placement-cutover.md:98-100` states route-6
|
||||
split-recovery creates "need an effect-replay suppression signal". **That is
|
||||
unsubstantiated and must be corrected in this landing.**
|
||||
|
||||
acdream's only create-time effect replay is the F754/F755 queue drain keyed by
|
||||
server GUID (`EntityEffectController.ReplayPendingForLiveEntity`), written only
|
||||
by inbound `PlayPhysicsScript`/`PlayPhysicsScriptType` for that exact GUID. A
|
||||
fresh split GUID has nothing queued unless ACE actually sent an effect for it,
|
||||
and draining it then is retail's own behaviour
|
||||
(`SmartBox::HandlePlayScriptID` @0x00452020 / `HandlePlayScriptType`
|
||||
@0x00452070 queue while absent; `HandleCreateObject` @0x00454C80 drains).
|
||||
|
||||
The one plausible mechanism — a cloned `DefaultScriptType` surviving
|
||||
`BuildSpawn` — **does not fire at create in either client.** acdream's
|
||||
`PlayDefault` has exactly two callers, both animation hooks
|
||||
(`DefaultScriptHook`, `DefaultScriptPartHook`). Retail matches:
|
||||
`CPhysicsObj::play_default_script` @0x005132B0 / @0x00513300 is reached only
|
||||
from `ACCWeenieObject::DoCollision` @0x0058C3A0 (call @0x0058C3B4) and the
|
||||
animation-hook dispatcher (@0x00526C08, @0x00526C14). **Neither client plays a
|
||||
default script from `set_description` or CreateObject.**
|
||||
|
||||
**Verify both retail claims yourself before relying on them.**
|
||||
|
||||
## Retail truth for split-recovery marking
|
||||
|
||||
`ACCWeenieObject::UIAttemptSplitTo3D` @0x0058D850 records exactly three fields:
|
||||
`splitStackSize` @0x0058D8A2, `splitClassID` @0x0058D8A8, `splitTime`
|
||||
@0x0058D8AE. `UIAttemptSplitToContainer` @0x0058D7D0 records the identical
|
||||
three. The consumer is `ACCWeenieObject::DeclareValid` @0x0058E340, whose
|
||||
recovery action is `SetSelectedObject(this->id, 0)` @0x0058E481 — **a SELECTION
|
||||
transfer, with a 10-second expiry @0x0058E49F-@0x0058E4B2. Not effect
|
||||
suppression, and nothing placement-related.**
|
||||
|
||||
`ACCWeenieObject::UIAttemptPutIn3D` @0x0058D700 (whole-item drop) records no
|
||||
marker at all and performs no placement.
|
||||
|
||||
## Deliverables
|
||||
|
||||
1. **Coverage tests, 150-250 lines**, against the now-flipped path — the list
|
||||
the campaign handoff names: whole item, split stack, new GUID, second drop
|
||||
position, unavailable destination, newer Position arriving while waiting.
|
||||
Note "attached child becoming a world root" from that list is **NOT route
|
||||
6's** — it is a cell-less Position on an existing entity, owned by 4b-3.
|
||||
2. **R6-c settled by assertion, not by argument.** `BuildSpawn` clones
|
||||
`Children` / `Movement` / `AnimationFrame` / `SetupTableId` wholesale from
|
||||
the source. Expected inert for a stackable inventory item but never
|
||||
measured. Assert it in the tests rather than reasoning about it.
|
||||
3. **The plan correction** at `docs/plans/2026-08-02-placement-cutover.md:98-100`.
|
||||
All three clauses are stale: the effect-replay premise is unsubstantiated
|
||||
(above), and route 7's `TryCommitParent`/`CommitWithdrawal`
|
||||
cancellation-symmetry and host-visible cancellation receipts were BOTH
|
||||
closed at C0. Replace with what actually remains: route 7's child-cell
|
||||
two-writer split and the headless parent-realize gap.
|
||||
4. **R6-a filed as an issue, NOT implemented here.** `DeclareValid`'s
|
||||
`SetSelectedObject` is not ported and the container-split flavour has no
|
||||
marker. That is selection UX, not placement; mixing it into a placement
|
||||
closure makes the landing un-reviewable. Add to `docs/ISSUES.md`.
|
||||
|
||||
## What must remain true
|
||||
|
||||
- **Zero production lines.** No `src/**` change. See the stop condition above.
|
||||
- **Tests must fail against broken behaviour.** No source-text pins, no
|
||||
tautologies. The 4b-3 lesson (round-2 finding B1): a test asserting only what
|
||||
must NOT happen cannot detect a deleted write — assert the positive half too.
|
||||
- AP-124 (the WCID/count approximation in the recovery match) stays open and
|
||||
registered; retiring it needs ACE to send CreateObject to the initiator, not
|
||||
a client change.
|
||||
|
||||
## Gates
|
||||
|
||||
- Complete Release suite. **Baseline 11,013 passed / 4 skipped / 0 failed** at
|
||||
`6dc7ba51` — measure and record; do not inherit. Known flakes, do not chase
|
||||
and do not conflate: #302 (`PortalProjectionTests`, GC-allocation) and #308
|
||||
(`NakEmissionTests.LossSoak_…`, wall-clock, full-suite load only).
|
||||
- **Connected gate (user-run), cheap and directly visible:** drop a whole item
|
||||
on open ground — it must land at your feet, resting, immediately pickable.
|
||||
Split a partial stack to the ground — correct quantity on the pile, remainder
|
||||
in inventory. Drop a second item within ~1 m — both remain visible and
|
||||
separately pickable. Repeat once indoors and once after a portal recall. Walk
|
||||
two landblocks away and back — both piles still there, still pickable.
|
||||
**Regressions:** item at world origin or your *previous* position (stale
|
||||
pose); invisible but blocking (#184 class); sunk into or floating above the
|
||||
floor; not pickable; the split pile never appears (recovery window failed);
|
||||
the second drop swallowed by the first.
|
||||
Loading…
Add table
Add a link
Reference in a new issue