fix(physics): C4 route 7 — child cell propagation moves from a render tick into Runtime
Retail re-cells children when their parent crosses a cell, recursively, to unbounded depth. acdream did it from a RENDER tick, so headless parented children were cell-less forever and the canonical cell had two writers. This slice makes Runtime the sole authority and demotes App's tick to presentation-only. Contract: docs/research/2026-08-04-c4-route-7-contract.md; the research that unblocked it is docs/research/2026-08-04-retail-parent-cell-propagation.md (ca96ea5e). Retail: SetPositionInternal @0x00515330 branches on `this->cell == curr_cell` @0x0051536d; the changed branch reaches change_cell @0x00513390, whose delegates leave_cell @0x00510f50 and enter_cell @0x00510ed0 self-recurse over children and write the FULL identity (add_object @0x00510ee2, objcell_id @0x00510f1e, part-array cell id @0x00510f2b, cell pointer @0x00510f35). change_cell itself has no child loop. THE TRAP, recorded because it nearly shipped: the depth-1 loop @0x0051539c-0x005153d8 is the SAME-CELL fast path (objcell_id and part-array id only, deliberately not the cell pointer), NOT the propagation. An implementer who finds it first concludes "depth-1, id-only" and strands every equipped item at a landblock boundary — the #184 class. The clincher against that reading: update_object @0x00515d10 early-returns on `parent != 0` @0x00515d40, so a child never runs its own physics tick and parent propagation is the ONLY mechanism maintaining its cell. Route 7 performs NO placement (DoPickupEvent @0x00452240 = unset_parent + leave_world; DoParentEvent @0x00452290 = set_parent + SetPlacementFrame), so it arms ConstrainTo nowhere — the leash rule INVERTS relative to routes 2/4/5, and both reviewers confirmed nothing arms. Propagation is an ITERATIVE WORKLIST, not recursion. The first implementation recursed with a depth-64 cap; both reviews independently found the cap left a truncated tail at a stale NON-ZERO cell — permanently unrecoverable, logged only under a probe flag, and on the withdraw path exactly the #184 shape AP-142 clause (a) exists to reject. Shipping a fresh #184 instance inside the slice that fixes stranded children was not acceptable, so the cap was removed rather than tuned. The worklist retires the cap, the constant, its register clause, and the failure mode together. Termination: every record on the stack is already at the target pair, so nothing can be pushed twice and a hostile A->B->A cycle collapses without a visited set. The child write deliberately bypasses the public RuntimeEntityDirectory .SetFullCell and calls the record method directly. This is LOAD-BEARING: the public method re-enters PropagateFullCellToChildren, which opens with _propagationWorklist.Clear() — routing children through it mid-drain would wipe the shared stack and silently drop every unprocessed sibling. Any future side effect added to the public SetFullCell must be mirrored by hand at that call site. Deliberate divergence, recorded not disguised: retail's removal path leaves children with a null cell pointer but a STALE nonzero objcell_id @0x005133c1. acdream does not reproduce it, because FullCellId != 0 is the liveness predicate at 45+ sites — faithful porting would mark dead children live. AP-142 records this; clause (d) records that acdream cannot gate propagation on HasPartArray the way enter_cell gates on part_array @0x00510ed8, because the flag's only writers are graphical and headless never sets it — the reason is Slice J LAYERING, not a semantic difference (retail's part_array is itself a mesh-construction product, single assignment site makeAnimObject @0x0050e930 -> CPartArray::CreateSetup @0x0050e93e). D7 adopts retail's unset_parent-before-leave_world order @0x0045227f -> @0x00452286, applied to BOTH pickup paths including the dormant executor replay. Its inertness was verified by reverting it and finding all 12 propagation tests still green — reported honestly rather than papered over with a manufactured test, and independently confirmed by both reviewers. ClassifyLeaveWorld and its request/cause types are DELETED: retail has no classification here, and method-per-cause IS the retail dispatch shape. Wiring it would have forced a vacuous teleport-sequence predicate with the #307 shape. Two review rounds plus a coordinator-required third pass; 5 MAJORs. One was a handoff failure worth recording: enter_cell's part_array guard was correctly identified as load-bearing by the research, dropped by the contract when it enumerated the writes, and inherited as an omission by the code — a right finding that evaporated across two handoffs with nobody re-reading the source. Another was a test that survived deleting the entire behaviour it claimed to pin, because its assertion read a field written unconditionally one line earlier. NoProjection is structurally unreachable from TickChild (TryResolveExactAttachment performs a strictly stronger form of the same guard one call earlier). Kept as a fail-safe, unit-tested directly, and documented in two places rather than wrapped in a fabricated end-to-end test. Headless regression test — the direct gate for this defect, which FAILED before this work because no code path existed: RuntimeLiveEntitySessionControllerTests .DirectSink_D5_StandaloneParentEventCommitsChildToParentsExactCell. Probe: ACDREAM_PROBE_CHILD_CELL=1 emits [child-cell] lines at all four write sites (attach / headless-attach / propagate / withdraw / delete). TEMPORARY. Complete Release suite MEASURED at 11,079 passed / 4 skipped / 0 failed (baseline 11,063 atcff52c44, +16). An allocation flake appeared once under load and was proven NOT this slice by reachability — RuntimeCollisionReportingState contains zero SetFullCell and zero ParentAttachments references. STILL OWED: the two-client connected gate (equip/unequip, carry across landblock boundaries, pickup, loot, reconnect) with ACDREAM_PROBE_CHILD_CELL=1, and a session counts only if [child-cell] cause=propagate lines appear. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
19ebf043e3
commit
cd3129e9d6
24 changed files with 4500 additions and 105 deletions
File diff suppressed because one or more lines are too long
|
|
@ -106,13 +106,25 @@ same commit) → docs/handoff commit. No workarounds; no fused slices.
|
|||
`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
|
||||
What actually remained for route 7: the child's canonical cell had two
|
||||
writers (Runtime committed it cell-less unconditionally in
|
||||
`CommitAcceptedParentCellless`, while `EquippedChildRenderController
|
||||
.TickChild` re-cells it from a per-frame render tick), and headless has no
|
||||
.TickChild` re-celled it from a per-frame render tick), and headless had no
|
||||
`EquippedChildRenderController` at all, so every headless parented child
|
||||
stays cell-less forever — the same defect seen from two sides, not two
|
||||
separate gaps.
|
||||
stayed cell-less forever — the same defect seen from two sides, not two
|
||||
separate gaps. **Closed 2026-08-04
|
||||
(`docs/research/2026-08-04-c4-route-7-contract.md`).** Runtime is now the
|
||||
sole canonical writer: `CommitAcceptedParentCellless` completes retail
|
||||
`set_parent`'s attach-time re-cell (D1), and every canonical cell write
|
||||
funnels through one directory chokepoint that recursively propagates to
|
||||
committed children on every parent cell crossing (D2 —
|
||||
`docs/research/2026-08-04-retail-parent-cell-propagation.md`), not only at
|
||||
attach. `TickChild` is demoted to a presentation-only draw-bucket move
|
||||
(D4); the headless host gained its own parent-realize drive running the
|
||||
same commit pair the graphical host does (D5,
|
||||
`RuntimeLiveEntitySessionController.OnParentUpdated`). The direct headless
|
||||
regression test (a bot with an equipped item shows the child's canonical
|
||||
`FullCellId` equal to the parent's) now passes.
|
||||
|
||||
## Slices
|
||||
|
||||
|
|
|
|||
|
|
@ -705,6 +705,30 @@ blanket "gate passed", because it is exactly 4b-2's #309 shape: that session's
|
|||
11 park probes were all one cause, the other cause went unexercised, and
|
||||
without the probe the session would have been recorded as full coverage.
|
||||
|
||||
To close it later: provoke a Position on an entity whose canonical cell is 0 —
|
||||
the unwield-to-3D path is the cheapest reachable trigger — and confirm one
|
||||
`cause=cellless` line. Do not treat it as closed by a teleport-ts session.
|
||||
**Superseded 2026-08-04 by C4 route 7 (see
|
||||
`docs/research/2026-08-04-c4-route-7-contract.md` §11).** The recipe below —
|
||||
"the unwield-to-3D path is the cheapest reachable trigger" — no longer fires.
|
||||
Route 7 ported retail's parent-cell propagation (`docs/research/2026-08-04-retail-parent-cell-propagation.md`):
|
||||
retail's `unset_parent` does no cell work, so a wielded child's unwield
|
||||
Position reaches `MoveOrTeleport` with `this->cell` = the parent's cell —
|
||||
NON-zero — and retail's cell-less branch never fires for unwield either. A
|
||||
committed child's canonical `FullCellId` is now deterministically the
|
||||
parent's (nonzero whenever the parent is celled), so an unwield Position
|
||||
classifies by TELEPORT_TS/distance like any other packet, exactly matching
|
||||
retail's predicate population. This is a correctness fix, not a regression:
|
||||
the OLD recipe only worked because a parented child's canonical cell was
|
||||
whatever the render-tick writer last produced — zero headless and zero in
|
||||
any pre-first-tick window — which was itself the #184-class defect route 7
|
||||
closed.
|
||||
|
||||
**To close the honest gap now:** provoke a Position on a body that is
|
||||
genuinely withdrawn/never-celled at merge time — a body between
|
||||
`CommitWithdrawal`/`CommitAcceptedParentCellless`'s cell-less edge and its
|
||||
next accepted Position, or a body whose initial Create never resolved a
|
||||
cell. Whether ACE ever emits an UpdatePosition in that exact window is
|
||||
UNESTABLISHED; this needs its own investigation before a live recipe can be
|
||||
written down. Test 4 in this contract's plan (`4b-3`, "unwield-to-3D shape
|
||||
classifies `SetPosition`") remains a valid Runtime-level fixture — it
|
||||
directly constructs a record with `PreMergeCommittedCellId == 0` — but it is
|
||||
a SYNTHETIC pre-merge-cell-0 fixture, not a live unwield behavior, and must
|
||||
not be re-labeled as one.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,418 @@
|
|||
# C4 route 7 — ARCHITECTURE / ADVERSARIAL review, round 2 (DELTA) — 2026-08-04
|
||||
|
||||
**Verdict: PASS.**
|
||||
|
||||
All three round-1 blocking findings (A1, A2, A3) are properly remediated, and
|
||||
each remediation is sabotage-sensitive in the direction that matters. Seven new
|
||||
findings (B1–B7) are recorded below; none is a correctness defect in shipped
|
||||
behaviour with a demonstrated failure, and each has a one-to-three-line fix.
|
||||
**B1 and B3's logging clause are recommended before the connected gate**, but
|
||||
neither blocks.
|
||||
|
||||
Scope: delta only. Round 1's accepted conclusions — the `SetFullCell` blast-radius
|
||||
enumeration, the D2 chokepoint-completeness proof, and the D7 inertness
|
||||
verification — are not re-litigated. Round-1 report:
|
||||
[`2026-08-04-c4-route-7-architecture-review.md`](2026-08-04-c4-route-7-architecture-review.md).
|
||||
|
||||
Gates re-run:
|
||||
|
||||
- `dotnet build` Runtime.Tests + App.Tests — 0 warnings, 0 errors.
|
||||
- `dotnet test AcDream.Runtime.Tests` — **1,157 passed / 0 failed** (was 1,156;
|
||||
+1 = the depth-cap test).
|
||||
- `dotnet test AcDream.App.Tests --filter EquippedChild` — **37 passed / 0
|
||||
failed** (was 36; the round-1 D4 test was replaced by two).
|
||||
|
||||
---
|
||||
|
||||
## 1. Round-1 blockers — re-verified
|
||||
|
||||
### A1 (MAJOR, "the D4 test cannot fail") — **FIXED, and the fix is sound.**
|
||||
|
||||
`tests/AcDream.App.Tests/Rendering/EquippedChildProjectionWithdrawalTests.cs:321-393`
|
||||
(`TickChild_D4_PresentationBucketMovesToTheDestinationLandblock`).
|
||||
|
||||
**Is it measuring the draw bucket or a proxy?** The draw bucket.
|
||||
`fixture.Spatial` is a real `GpuWorldState` (`:1443`, `= new()`), and
|
||||
`CopyLiveEntitiesNearLandblock` (`src/AcDream.App/Streaming/GpuWorldState.cs:493-523`)
|
||||
reads `_loadedLiveByLandblock` — the per-landblock `HashSet<WorldEntity>` the
|
||||
renderer's live-entity publication actually maintains. It is not a mirror field
|
||||
and not a derived view. It also `destination.Clear()`s at `:500`, so the
|
||||
four sequential queries in the test do not contaminate each other (without that
|
||||
clear, the `DoesNotContain` assertions would be false-negatives; with it, they
|
||||
are real).
|
||||
|
||||
**Can it pass with the demotion removed?** No, and I checked both sabotage
|
||||
directions:
|
||||
|
||||
- *Delete the `RebucketEquippedChildPresentation` call at
|
||||
`EquippedChildRenderController.cs:411-425`* → the child never leaves
|
||||
`oldLandblock`, so the post-tick `Assert.Contains(newLandblock…)` fails. This
|
||||
is the failure the round-1 test could not produce.
|
||||
- *Revert to the legacy `RebucketLiveEntity`* → the bucket moves (so the
|
||||
`Contains` passes) but `CommitRebucket` → `RuntimeEntityRecord.SetFullCell`
|
||||
bumps `SpatialAuthorityVersion` unconditionally (`RuntimeEntityRecord.cs:246-251`),
|
||||
so the final `Assert.Equal(childSpatialVersionBeforeTick, …)` fails.
|
||||
|
||||
Both halves of the transfer are therefore pinned in opposite directions. The
|
||||
test also cannot pass **vacuously** on an empty query result: the pre-tick
|
||||
`Assert.Contains(oldLandblock…)` and the post-tick `Assert.Contains(newLandblock…)`
|
||||
are positive assertions on two different landblocks, and a
|
||||
`!_availability.IsWorldAvailable` early-return (`GpuWorldState.cs:501`) would
|
||||
fail the first one. The implementer's reported sabotage ("stubbing the demoted
|
||||
call to claim success failed it — empty collection") is consistent with that
|
||||
structure.
|
||||
|
||||
### A2 (MAJOR, "silent write-nothing outcome") — **FIXED. The fork is the right call; `NotAttached`-as-benign is safe.**
|
||||
|
||||
I judged the fork against the alternative I originally suggested, and the
|
||||
implementer's choice is better. Reasoning, done by enumeration rather than by
|
||||
accepting the stated rationale:
|
||||
|
||||
`NotAttached` is returned only when `!HasCommittedParent(serverGuid)` or when an
|
||||
initial-create residence is active. The complete set of things that can remove
|
||||
`_lastAcceptedByChild[child]` is `ParentAttachmentState.RemoveCommittedChild`,
|
||||
reached from exactly six call sites:
|
||||
|
||||
| remover | is it an unwind edge? |
|
||||
|---|---|
|
||||
| `CommitProjection` (`:581`) | re-adds on the next line — transient, unobservable |
|
||||
| `EndChildProjection` (`:753`) | yes — pickup / Position-unparent |
|
||||
| `EndGeneration(child)` (`:709`) | yes — child replaced |
|
||||
| `DeleteGeneration(child)` (`:734`) | yes — child deleted |
|
||||
| `RemoveCommittedParentReferences(parent)` (`:865-873`, from `EndGeneration`/`DeleteGeneration` on the PARENT) | yes — parent deleted/replaced |
|
||||
| `RemoveObject` / `RemoveChild` (`:660`, `:756`) | zero production callers |
|
||||
|
||||
`CommitProjection` is also the **only** writer, and
|
||||
`PrepareAndTryRealize` always calls it before `TryRealize` installs the
|
||||
`AttachedChild` (`EquippedChildRenderController.cs:861-891`), with the recovery
|
||||
branch gated on `Relations.IsCommitted`. So `_attachedByChild` non-empty ⇒
|
||||
committed, **unless one of the five genuine unwind edges has fired**. There is
|
||||
no state in which a child is legitimately attached, rendering, and
|
||||
`HasCommittedParent` is false. `NotAttached` therefore cannot swallow a real
|
||||
pose-loss — it can only fire in a window where a teardown is already in flight
|
||||
and owns the child's fate.
|
||||
|
||||
The parent-deleted case is worth naming because it is the longest window
|
||||
(`_pendingOrphanRemovalByChild` defers the withdrawal across frames,
|
||||
`EquippedChildRenderController.cs:342`/`:584`): during it, D3 has already zeroed
|
||||
the child's canonical cell and the bucket is frozen at the parent's last
|
||||
landblock. That is unchanged from pre-route-7 behaviour (the old
|
||||
`RebucketLiveEntity` would have rebucketed to the same stale
|
||||
`parent.ParentCellId`), so it is not a regression.
|
||||
|
||||
Routing `NoProjection` into `WithdrawForPoseLoss` is also correct for the
|
||||
`_projections.TryGetCurrent` failure it is named for — see **B6** for the second
|
||||
condition that shares the label.
|
||||
|
||||
### A3 (MEDIUM, "P8 enumerated the wrong interface") — **FIXED.**
|
||||
|
||||
`RuntimeEntityDirectory.cs`'s `PropagateFullCellToChildren` doc now carries a
|
||||
dedicated `<para>` naming `GameRuntimeEventHub` as itself an
|
||||
`IRuntimeEntityObjectObserver` that fans out to `IRuntimeEventObserver`, and
|
||||
`RuntimeTraceRecorder.OnEntity` as the real non-stub consumer, and explicitly
|
||||
retracts "no consumer at all". The decision (publish nothing) is unchanged and
|
||||
its stated basis is now true. P4's write-up in the same comment also correctly
|
||||
states the `ProjectionKind is World` reason for the no-spatial-root claim rather
|
||||
than the round-1 relation-based reason.
|
||||
|
||||
### A4, A5, A7, A9, A10 — spot-checked, all correct.
|
||||
|
||||
- **A4** — `RuntimeEntityObjectLifetime.CommitAcceptedParentCellless` carries the
|
||||
payload-contract doc, and `Attach_ParentCelled_…` now subscribes a
|
||||
`RecordingEntityObserver` and asserts the `Withdrawn` delta's
|
||||
`Entity.CellId == parent.FullCellId` and `!= 0`
|
||||
(`RuntimeEntityChildCellPropagationTests.cs:34-57`). Sabotage-sensitive:
|
||||
moving D1's re-cell after the publish makes `withdrawn.Entity.CellId` zero and
|
||||
fails two assertions. This is a real pin, not a source-text pin.
|
||||
- **A5** — `RuntimeInitialCreateContinuationExecutor.cs:2184-2190`: the dormant
|
||||
pickup replay now runs `EndChildProjection` before `LeaveWorld`, matching
|
||||
`TryApplyPickup`. Both pickup paths are one shape.
|
||||
- **A7** — the skip is now keyed on the `(FullCellId, CanonicalLandblockId)`
|
||||
pair (`RuntimeEntityDirectory.cs:389-395`), with the reason stated. Cycle
|
||||
termination is unaffected: A→B→A still writes A, writes B, then finds A's
|
||||
**pair** already equal and stops (`PropagationChokepoint_TerminatesAHostileTwoCycle…`
|
||||
still green).
|
||||
- **A9** — `RebucketEquippedChildPresentation` now carries the same
|
||||
`MaterializationResidence is AwaitRuntimePlacement && HasActiveInitialCreateResidence`
|
||||
gate `RebucketLiveEntity` honours (`LiveEntityRuntime.cs:1140-1145`).
|
||||
- **A10** — the C3c-R1 F6 summary is back on
|
||||
`FirstEntryDriveServesOneRouteAtATimeAndScopesClearToTheOwner`
|
||||
(`RuntimeLiveEntitySessionControllerTests.cs:422-430`); no duplicate
|
||||
`<summary>` remains.
|
||||
|
||||
---
|
||||
|
||||
## 2. New findings
|
||||
|
||||
### B1 — MEDIUM. The `NotAttached` test's "the draw bucket did NOT move" assertion is non-discriminating.
|
||||
|
||||
**Where:** `tests/AcDream.App.Tests/Rendering/EquippedChildProjectionWithdrawalTests.cs:395-450`
|
||||
(`TickChild_D4_NotAttachedDisposition_SkipsTheBucketMoveWithoutFailingTheTick`),
|
||||
final assertion at `:445-448`.
|
||||
|
||||
Unlike its sibling, this test never moves the parent: both entities are spawned
|
||||
at the fixture's default `Cell` and no `CommitRebucket` runs. So
|
||||
`parent.WorldEntity.ParentCellId` still names the **same landblock the child's
|
||||
bucket is already in** (`0x0101FFFF`). If the `HasCommittedParent` guard were
|
||||
deleted outright, `RebucketLiveEntityPresentationOnly` would run and rebucket
|
||||
the child to the landblock it is already in — and
|
||||
`Assert.Contains(buffer, kv => kv.Value == child.WorldEntity)` on `oldLandblock`
|
||||
would still pass.
|
||||
|
||||
**Concrete failure scenario:** delete the `HasCommittedParent` check at
|
||||
`LiveEntityRuntime.cs:1137-1138`. The guard A2 exists to establish is gone; the
|
||||
enum collapses to `Moved`/`NoProjection`; this test stays green.
|
||||
|
||||
The test's **load-bearing half is sound** — `LastFullPoseCompositionVisits` +
|
||||
`AttachedEntityIds` genuinely pin "the tick was not torn down as a pose loss",
|
||||
which is the fork judgement A2 asked for, and *that* half does fail if
|
||||
`NotAttached` were routed to `return false`. Only the bucket clause is
|
||||
decorative.
|
||||
|
||||
**Fix direction:** mirror the sibling — register the second landblock, drive
|
||||
`CommitRebucket(parent, newCell, newLandblock)` before the tick, and assert the
|
||||
child is **still in `oldLandblock`** afterwards. Then the assertion discriminates.
|
||||
|
||||
### B2 — MEDIUM. A6's justification comment contradicts the R6 gap comment two methods away, and the narrowing removed a (weak) recovery path.
|
||||
|
||||
**Where:** `src/AcDream.Runtime/Session/RuntimeLiveEntitySessionController.cs:414-433`
|
||||
(`RetryChildrenWaitingForParent`'s A6 `<para>`) vs `:346-370`
|
||||
(`ResolveAndCommitChildAttachment`'s R6 KNOWN-GAP `<para>`).
|
||||
|
||||
The A6 comment justifies scanning only `_unresolvedByChild` on the grounds that
|
||||
"this drive never populates `_stagedByChild`/`_recoveryByChild` through any path
|
||||
those two sweeps exist to catch". That is false: `ParentAttachmentState.Resolve`
|
||||
sets `_stagedByChild[childGuid]` on accept (`:472-475`), and the R6 gap
|
||||
documented two methods above is *precisely* a relation left staged-but-uncommitted
|
||||
because `TryCommitParent`'s POSITION_TS gate was not satisfied during the child's
|
||||
pending initial-create residence.
|
||||
|
||||
**Concrete failure scenario:** a headless `ParentEvent` arrives while the child's
|
||||
initial residence is pending. `Resolve` stages it; `TryCommitParent` refuses; the
|
||||
relation sits in `_stagedByChild` forever. Under round 1's
|
||||
`ChildrenWaitingForParent` (which unioned all three tables) a later spawn naming
|
||||
the same parent guid would have re-driven it; under `ChildrenUnresolvedForParent`
|
||||
nothing will. Weak in practice — it needs a *new parent generation* to fire — but
|
||||
the narrowing strictly reduced recovery, and the comment claims it did not.
|
||||
|
||||
The R6 gap itself is disclosed honestly and I accept it as an out-of-scope
|
||||
residual (headless committed nothing on this path before D5 existed). The
|
||||
finding is the contradiction, plus the consequence for the **contract §7 headless
|
||||
gate**: its scenario is "the local player equips via the bot command surface and
|
||||
crosses a boundary". If the equip lands during the item's own residence, the gate
|
||||
produces no `cause=headless-attach` line and must be read as a **not-run**, not a
|
||||
pass — the same rule the contract already states for `cause=propagate`.
|
||||
|
||||
**Fix direction:** correct the A6 `<para>` to say what is actually true (this
|
||||
drive promotes unresolved relations; a staged-but-uncommitted relation is the R6
|
||||
gap and is deliberately not retried here), and cross-reference the R6 paragraph.
|
||||
No code change required.
|
||||
|
||||
### B3 — MEDIUM. The depth cap's failure mode contradicts AP-142 clause (a), is unrecoverable, and is invisible without the probe flag.
|
||||
|
||||
**Where:** `src/AcDream.Runtime/Entities/RuntimeEntityDirectory.cs:349-357`
|
||||
(the constant) and `:433-441` (the early return).
|
||||
|
||||
Three separate points, in descending importance:
|
||||
|
||||
1. **The failure mode is the shape the model exists to avoid.** Past depth 64
|
||||
the tail keeps a **stale non-zero** `FullCellId`. Under acdream's single-field
|
||||
model that reads as "still resident at the old cell" at all 45+ predicate
|
||||
sites — exactly the `#184` invisible-but-solid shape that AP-142 **clause (a)**
|
||||
cites as the reason the removal path propagates zero rather than reproducing
|
||||
retail's stale-`objcell_id` residue. The cap therefore reintroduces, as its
|
||||
*chosen* failure mode, the residue the row rejects. Zeroing the truncated
|
||||
subtree instead ("we cannot maintain this, so it is not resident") is both
|
||||
consistent with the model and strictly safer.
|
||||
2. **It is unrecoverable.** Every subsequent crossing truncates at the same
|
||||
depth, so a subtree that falls past the cap once never re-syncs. And because
|
||||
the skip is value-idempotent, a later same-value write prunes even earlier
|
||||
(the asymmetry AP-142 clause (b)'s R10 correction already records). A
|
||||
permanently-stale resident subtree is worse than a transiently-stale one.
|
||||
3. **It is silent in production.** `[child-cell-depth-exceeded]` is emitted only
|
||||
under `PhysicsDiagnostics.ProbeChildCellEnabled`, i.e. only when someone has
|
||||
already set `ACDREAM_PROBE_CHILD_CELL=1`. A defensive guard whose premise is
|
||||
"this must never happen" should be unconditionally observable — a one-shot
|
||||
log or a counter on the ownership ledger — or nobody will ever learn it fired.
|
||||
|
||||
**Is 64 defensible?** Yes, as a number. Real chains are 2–3 deep
|
||||
(player → weapon; quiver → arrow). 64 is ~20× any plausible depth and is well
|
||||
inside a 1 MB stack for a two-frame-per-level recursion. **Is the cap the right
|
||||
mechanism?** An iterative worklist over a pre-sized scratch would remove the
|
||||
hazard entirely, keep 0 B after warmup, and need no cap, no divergence row, and
|
||||
no failure mode — that is the answer I would take if this is revisited. But the
|
||||
cap is a strict improvement over round 1's uncatchable process kill, so it is not
|
||||
a blocker.
|
||||
|
||||
**Fix direction (cheapest first):** (i) make the past-cap log unconditional;
|
||||
(ii) zero the truncated subtree rather than leaving it stale, and amend AP-142
|
||||
clause (e) to say so; (iii) if revisited, replace the recursion with an
|
||||
iterative worklist and retire clause (e).
|
||||
|
||||
### B4 — LOW. The reused scratch buffer is not re-entrancy-safe; round 1's fresh array was.
|
||||
|
||||
**Where:** `src/AcDream.Runtime/Session/RuntimeLiveEntitySessionController.cs:73`
|
||||
(`_unresolvedChildrenScratch`), `:434-440` (filled, then iterated while calling
|
||||
`ResolveAndCommitChildAttachment`).
|
||||
|
||||
`ChildrenUnresolvedForParent` **clears and refills the caller's buffer**
|
||||
(`ParentAttachmentState.cs:657-660`). `RetryChildrenWaitingForParent` then
|
||||
iterates that same instance while calling into
|
||||
`ResolveAndCommitChildAttachment` → `TryCommitParent` /
|
||||
`CommitAcceptedParentCellless` → `AcknowledgeProjectionAndPublish` → synchronous
|
||||
observer fan-out through `GameRuntimeEventHub`.
|
||||
|
||||
**Concrete failure scenario:** an `IRuntimeEventObserver` synchronously feeds a
|
||||
spawn back into the sink → nested `OnSpawned` → nested
|
||||
`RetryChildrenWaitingForParent` → the shared buffer is cleared and refilled →
|
||||
the outer loop's `waiting.Count` and indices now address the inner call's
|
||||
contents, so children are skipped or retried twice. Requires an observer that
|
||||
re-enters the sink; the shipped headless policies are empty stubs, so this is
|
||||
latent, not live. The graphical sibling it mirrors
|
||||
(`ChildrenWaitingForParent` → `.ToArray()`) is re-entrancy-safe by allocation,
|
||||
and round 1's version inherited that safety; the A6 optimization traded it away
|
||||
without a guard.
|
||||
|
||||
**Fix direction:** a `_retryInProgress` flag that falls back to a fresh `List`
|
||||
when re-entered, or state the single-entry precondition in the doc and assert it.
|
||||
|
||||
### B5 — LOW. The recursion no longer routes children through the public `SetFullCell`, creating a silent divergence trap.
|
||||
|
||||
**Where:** `src/AcDream.Runtime/Entities/RuntimeEntityDirectory.cs:409-414` —
|
||||
propagation now calls `child.SetFullCell(...)` (the **record** method) plus a
|
||||
direct recursive call, where round 1 re-entered `RuntimeEntityDirectory.SetFullCell`.
|
||||
|
||||
The two are equivalent **today** — the public method is exactly
|
||||
`EnsureKnown` + `record.SetFullCell` + propagate, and `EnsureKnown` is a
|
||||
validation-only throw (`:812-820`) already subsumed by the preceding
|
||||
`TryGetActive`. The change was needed to thread `depth`.
|
||||
|
||||
**Concrete failure scenario:** anyone later adding a side effect to
|
||||
`RuntimeEntityDirectory.SetFullCell` — an index update, a delta publish, a
|
||||
telemetry counter — gets it for the root and **silently not** for propagated
|
||||
children, reintroducing exactly the "mapping written against one caller's
|
||||
reachable set" class D2 was designed to eliminate. Nothing in the code says the
|
||||
equivalence is load-bearing.
|
||||
|
||||
**Fix direction:** give the public method a private `depth`-carrying overload and
|
||||
have the recursion call *that*, so there is one body; or add one comment line at
|
||||
`:409` naming the equivalence as an invariant to maintain.
|
||||
|
||||
### B6 — LOW. `NoProjection` conflates two different outcomes, and one of them now triggers a teardown that round 1 did not.
|
||||
|
||||
**Where:** `src/AcDream.App/World/LiveEntityRuntime.cs:1131-1152`. The enum doc
|
||||
(`:45-56`) defines `NoProjection` as "the graphical projection itself is gone",
|
||||
but `:1147-1152` also returns it when `RebucketLiveEntityPresentationOnly`
|
||||
returns `false` — which means the **projection operation was displaced by a
|
||||
re-entrant callback** (`IsCurrentProjectionOperation` at `:1029`/`:1054`), i.e. a
|
||||
*newer* rebucket took over. `TickChild` then returns `false` →
|
||||
`Tick()`'s `failed` list → `WithdrawForPoseLoss`.
|
||||
|
||||
**Concrete failure scenario:** a re-entrant observer starts a newer rebucket for
|
||||
the same child mid-`_spatial.RebucketLiveEntity`; the older tick concludes
|
||||
"projection gone" and withdraws the child's projection that the newer operation
|
||||
just legitimately re-established. Low reachability — the
|
||||
`_presentationOnlySpatialMutationDepth` guard (`:3354-3360`) already suppresses
|
||||
the most likely re-entrant rebucket source — but round 1 discarded the bool, so
|
||||
this teardown is new behaviour introduced by the A2 fix.
|
||||
|
||||
**Fix direction:** either split the displaced case into its own disposition
|
||||
(`Superseded`, treated as benign — the newer operation owns the bucket), or
|
||||
document at `:1147` that displacement is deliberately treated as pose loss and
|
||||
why.
|
||||
|
||||
### B7 — INFO. One other test derives a built-collection size from a live production constant.
|
||||
|
||||
Asked directly, so answered directly: I swept `tests/` for the shape that caused
|
||||
the implementer's 64,000-node stack overflow (a test that both *reads* a
|
||||
production constant and *sizes work* by it). Exactly one other match with real
|
||||
blast radius:
|
||||
|
||||
`tests/AcDream.Core.Tests/World/LandblockLoaderTests.cs:210` —
|
||||
`for (uint i = 0; i <= LandblockStaticEntityIdAllocator.MaxCounter + 1u; i++)`
|
||||
builds one `Stab` per iteration. `MaxCounter = 0xFFF`
|
||||
(`src/AcDream.Core/World/LandblockStaticEntityIdAllocator.cs:16`), so 4,097 cheap
|
||||
allocations today. A sabotage raising it by orders of magnitude would OOM or hang
|
||||
rather than stack-overflow, and the test's `Assert.Contains("4096-entry", …)`
|
||||
literal pins the constant — but only *after* the build loop, so the pin does not
|
||||
protect the loop. Milder than the route-7 case and pre-existing; noted, not
|
||||
filed against this slice.
|
||||
|
||||
The other three matches are benign: `LightManagerTests` (`MaxGlobalLights = 128`,
|
||||
+50 iterations of trivial work) and the two `CapacityTrimIdleFrames` frame loops.
|
||||
|
||||
The route-7 depth test's own remaining constant read
|
||||
(`RuntimeEntityChildCellPropagationTests.cs:473`,
|
||||
`for (i = 0; i <= MaxPropagationDepth; i++)`) is **correctly protected**: the
|
||||
literal pin `Assert.Equal(64, RuntimeEntityDirectory.MaxPropagationDepth)` at
|
||||
`:441` fires first, and the chain length at `:442` is a literal. The boundary is
|
||||
pinned in both directions — a cap of 70 would give index 69 the new cell and fail
|
||||
the tail assertion; a cap of 60 would leave indices 61–64 stale and fail the loop.
|
||||
|
||||
---
|
||||
|
||||
## 3. Direct answers to the coordinator's remaining questions
|
||||
|
||||
### The `WarmedSteadyContactRefreshDoesNotAllocate` flake — **#302 class. Not this slice.**
|
||||
|
||||
Stated definitely rather than hedged, because it is provable by reachability:
|
||||
|
||||
- The measured window
|
||||
(`RuntimeCollisionReportingStateTests.cs:2375-2379`) contains **only**
|
||||
`lifetime.Physics.HandleSetPositionCollisions(...)`. Entity registration and
|
||||
shadow setup happen before `GC.GetAllocatedBytesForCurrentThread()` is sampled.
|
||||
- `src/AcDream.Runtime/Physics/RuntimeCollisionReportingState.cs` contains **zero**
|
||||
occurrences of `SetFullCell` and **zero** of `ParentAttachments` (grepped). Its
|
||||
only `_entities` interactions are read-only queries (`TryGetByLocalId`,
|
||||
`IsCurrent`, `SessionLifetimeVersion`, `CurrentLifetimeMutation`) plus
|
||||
`StopMissileAfterCollision`, which mutates `FinalPhysicsState`, not the cell.
|
||||
`StopMissileAfterCollision` is not in the complete `SetFullCell` caller set
|
||||
enumerated in round 1 §0 and re-verified against this diff.
|
||||
|
||||
So `PropagateFullCellToChildren` is not reachable from the measured call at all,
|
||||
and the fixture has no committed parent relations in any case. A
|
||||
`GC.GetAllocatedBytesForCurrentThread() == 0` assertion that passes in isolation
|
||||
and on re-run but fails once under full-suite load is the documented **#302**
|
||||
shape (tiered-JIT / first-touch allocation on the measuring thread), and should
|
||||
be treated as such: re-run and name it, never chase it. The instinct to look
|
||||
rather than auto-dismiss was right — the answer is that the new code is not on
|
||||
that path.
|
||||
|
||||
### Point 6 — deliberately-not-done items: **my "not blocking alone" assessment holds.**
|
||||
|
||||
- **Writer-family coverage for `CommitCanonicalCell` / `RuntimeSetPositionState`.**
|
||||
Still covered *by construction*, and round 2 did not weaken that: both funnel
|
||||
through the identical `RuntimeEntityDirectory.SetFullCell:359-366` line that
|
||||
`CommitRebucket` (tested) and `RefreshSnapshot` (tested) use, and round 1's
|
||||
chokepoint-completeness proof (`RefreshDerivedState` has exactly two callers;
|
||||
`RuntimeEntityRecord.SetFullCell` exactly two) is unchanged by this diff. Two
|
||||
of the four writer families are tested end-to-end; the other two are the same
|
||||
three lines of code.
|
||||
- **Fuller P2 across N crossings** — `NeverArmPartition_D8_…` already drives five
|
||||
crossings; the clock/suspension assertions live in the attach and pickup tests.
|
||||
Thin, not absent.
|
||||
- **Dedicated P5/P6 tests** — still argued by construction, and the construction
|
||||
argument is still valid after round 2: `PropagateFullCellToChildren` remains
|
||||
field-writes + one dictionary probe + `TryGetActive`, with no callback, no
|
||||
LINQ, no closure, and `ChildrenAttachedToParent` still returning the stored
|
||||
`List<uint>` or the cached `Array.Empty<uint>()`. The round-2 restructuring
|
||||
(direct `child.SetFullCell` instead of re-entering the directory method) makes
|
||||
the path *cheaper*, not richer. **B4** is the one place round 2 introduced new
|
||||
re-entrancy surface, and it is in D5's retry, not the propagation.
|
||||
|
||||
---
|
||||
|
||||
## 4. Recommended before the connected gate (non-blocking)
|
||||
|
||||
1. **B1** — make the `NotAttached` test's bucket assertion discriminate (move the
|
||||
parent first). Two lines.
|
||||
2. **B3 clause 3** — make the past-cap log unconditional. One line.
|
||||
3. **B2** — correct the A6 justification comment so it does not contradict the R6
|
||||
gap paragraph, and carry the "no `cause=headless-attach` line ⇒ not-run"
|
||||
reading into the headless gate step. Comment only.
|
||||
|
||||
The **connected two-client gate remains unrun** and is still the acceptance test
|
||||
for the D4 transfer, with the contract's rule intact: a session showing zero
|
||||
`cause=propagate` lines during the landblock-crossing step is a not-run, not a
|
||||
pass. The headless gate now carries the same caveat for `cause=headless-attach`
|
||||
(B2).
|
||||
493
docs/research/2026-08-04-c4-route-7-architecture-review.md
Normal file
493
docs/research/2026-08-04-c4-route-7-architecture-review.md
Normal file
|
|
@ -0,0 +1,493 @@
|
|||
# C4 route 7 — independent ARCHITECTURE / ADVERSARIAL review (2026-08-04)
|
||||
|
||||
**Verdict: FAIL** — remediation is narrow and mechanical (two test gaps plus one
|
||||
incomplete proof enumeration). No defect was found in the shipped propagation
|
||||
mechanism itself; the failure is that the route's single highest-risk,
|
||||
user-visible half (the D4 presentation transfer) has **no test that can fail**,
|
||||
which is the exact recurring defect class route 5's review rounds named and
|
||||
which §3 item 7 / §6 test 10 of the contract explicitly pinned.
|
||||
|
||||
Scope reviewed: uncommitted working-tree diff at HEAD `ca96ea5e`, branch
|
||||
`claude/acdream-physics-divergence-5aa784`. Route-7 contract and route-3
|
||||
scoping docs treated as inputs, not as change under review.
|
||||
|
||||
Gates re-run by the reviewer:
|
||||
|
||||
- `dotnet build tests/AcDream.Runtime.Tests` — 0 warnings, 0 errors.
|
||||
- `dotnet test AcDream.Runtime.Tests` — **1,156 passed / 0 failed**.
|
||||
- `dotnet test AcDream.App.Tests --filter EquippedChild` — **36 passed / 0 failed**.
|
||||
- New `RuntimeEntityChildCellPropagationTests` + the two `DirectSink_D5` tests —
|
||||
**14 passed**.
|
||||
|
||||
---
|
||||
|
||||
## 0. Judgement on the four implementer claims + the `SetFullCell` blast radius
|
||||
|
||||
### Claim 1 — "D7's reorder is unobservable" → **VERIFIED. Genuinely unobservable.**
|
||||
|
||||
`EndChildProjection(guid)` (`ParentAttachmentState.cs:749-754`) touches exactly
|
||||
three tables keyed by the **child** guid: `_stagedByChild`, `_recoveryByChild`,
|
||||
and (via `RemoveCommittedChild`) `_lastAcceptedByChild` plus the removal of that
|
||||
guid from **its parent's** `_committedChildrenByParent` list. It never touches
|
||||
`_committedChildrenByParent[(update.Guid, incarnation)]` — the picked-up
|
||||
entity's own children list.
|
||||
|
||||
The three calls now bracketed by the move are
|
||||
`Physics.CollisionReports.LeaveWorld`, `Physics.SetPosition.Forget`, and
|
||||
`Entities.SuspendObjectClock`. I enumerated every `ParentAttachments` reader
|
||||
reachable from them:
|
||||
|
||||
- `RuntimeSetPositionState.cs:6027` / `:6049` (`ArmLostFamilyDeadlines` /
|
||||
`CancelLostFamilyDeadlines`) read `ChildrenAttachedToParent(operation.Record
|
||||
.ServerGuid, …)` — the record **as a parent**, unaffected.
|
||||
- `RuntimeSetPositionState.cs:3944` (`IsAffectedCollisionResident` →
|
||||
`HasCommittedParent`) reads the record **as a child** and IS order-sensitive —
|
||||
but it is reached only from `ParkCollisionResidents`, a landblock-quiescence
|
||||
entry point, never synchronously from `Forget`.
|
||||
- `CancelExactLostKey` iterates operations by `RuntimeEntityKey`, not by relation.
|
||||
|
||||
No other reader exists. The honest "reverted the order, all 12 tests still
|
||||
passed" report is the correct outcome and should be recorded as such, not
|
||||
papered over with a manufactured test. **One consistency gap follows from it —
|
||||
see A5.**
|
||||
|
||||
### Claim 2 — "`RuntimeEntityChange.Rebucketed` has NO consumer" → **FALSE as stated. Enumeration incomplete. See A3.**
|
||||
|
||||
The implementer enumerated `IRuntimeEntityObjectObserver` implementations and
|
||||
found empty stubs. But `GameRuntimeEventHub` (`GameRuntimeEventHub.cs:45`)
|
||||
*implements* `IRuntimeEntityObjectObserver` and its `OnEntity`
|
||||
(`:239-254`) **fans the delta out to every `IRuntimeEventObserver`**, and
|
||||
`RuntimeTraceRecorder.OnEntity` (`GameRuntimeEvents.cs:246-254`) is a
|
||||
non-stub shipped consumer that records `delta.Change` and
|
||||
`delta.Entity.CellId`. The `IHeadlessBotPolicy` implementations
|
||||
(`HeadlessBotPolicy.cs:48/151/281/516`) are the empty stubs; they are not the
|
||||
whole set. No functional break is demonstrated (the trace recorder is
|
||||
diagnostic and equipped-child `Rebucketed` deltas were graphical-only), so the
|
||||
publication default itself stands — but P8 was answered against the wrong
|
||||
interface and must be restated.
|
||||
|
||||
### Claim 3 — "a committed child never becomes a spatial root" → **VERIFIED, for the right reason (which the claim did not state).**
|
||||
|
||||
The reason is not the parent relation: it is the **projection kind**.
|
||||
`LiveEntityRuntime.HasSpatialRuntimeProjection` (`:3251-3256`) requires
|
||||
`record.ProjectionKind is LiveEntityProjectionKind.World`, and
|
||||
`EquippedChildRenderController.TryRealize` materializes children with
|
||||
`LiveEntityProjectionKind.Attached` (`:569`). So
|
||||
`RefreshSpatialRuntimeIndexes` → `AcknowledgeSpatialProjection(canonical,
|
||||
spatial: false)` → `RemoveSpatialProjection` on every tick, and
|
||||
`_spatialRoots` (`RuntimePhysicsState.cs:697-711`) never gains the child.
|
||||
Headless never calls `AcknowledgeSpatialProjection` on any route-7 path at all
|
||||
(its only callers are `LiveEntityRuntime.cs:3272` and
|
||||
`RuntimeSetPositionState.cs:2756/3664/5138`, none of which route 7 reaches).
|
||||
So no child broadphase gap, and the §0 trap-5a "no per-crossing shadow rebuild"
|
||||
pin holds. **Note for the record:** had the child been kind `World`, the
|
||||
`FullCellId != 0` clause of `HasSpatialRuntimeProjection` would have made D1/D5
|
||||
newly promote children into the physics workset — the claim is right, but it is
|
||||
one line of `ProjectionKind` away from being wrong, and P4's write-up should say
|
||||
so.
|
||||
|
||||
### Claim 4 — "P5/P6 are safe by construction, no dedicated tests" → **ACCEPTABLE for the propagation path; NOT acceptable as a blanket statement for the slice. See A6.**
|
||||
|
||||
- **P6 (zero alloc) on the propagation path: verified.**
|
||||
`ChildrenAttachedToParent` (`ParentAttachmentState.cs:648-658`) returns the
|
||||
stored `List<uint>` or the cached `Array.Empty<uint>()`; both convert to
|
||||
`IReadOnlyList<uint>` by reference conversion (no boxing).
|
||||
`PropagateFullCellToChildren` (`RuntimeEntityDirectory.cs:381-405`) is an
|
||||
index loop with no LINQ, no closures, no temporaries. 0 B confirmed by
|
||||
inspection. `ParentIncarnation` is a `readonly record struct`
|
||||
(`ParentAttachmentState.cs:923`) so the dictionary probe does not box.
|
||||
- **P5 (re-entrancy) on the propagation path: verified.** The only work is
|
||||
`EnsureKnown`, `record.SetFullCell` field writes, `TryGetActive`, and an
|
||||
optional `Console.WriteLine`. Nothing calls back into `RuntimeSetPositionState`
|
||||
/ `RuntimePhysicsState` / any observer, so the recursion cannot re-enter a
|
||||
mid-commit owner. I also confirmed the live `List<uint>` returned by
|
||||
`ChildrenAttachedToParent` cannot be mutated during the loop (nothing on the
|
||||
propagation path touches the relation tables), so the swap-remove in
|
||||
`RemoveCommittedChild` (`:853-859`) is not an aliasing hazard here.
|
||||
- **But the slice's OTHER new path is not zero-alloc and was not measured** —
|
||||
D5's per-spawn `ChildrenWaitingForParent` scan. See A6.
|
||||
|
||||
### The `SetFullCell` blast radius — **enumerated; correct at every site, with one structural caveat (A7).**
|
||||
|
||||
`RuntimeEntityDirectory.SetFullCell` (`:348-355`) now has a side effect for
|
||||
every caller. Full production caller set, each checked:
|
||||
|
||||
| site | value written | children now follow — correct? |
|
||||
|---|---|---|
|
||||
| `RuntimeEntityObjectLifetime.cs:1314` (`TryApplyPickup`) | 0,0 | ✔ retail `leave_world`'s recursive `leave_cell(this,0)` @0x005155E6 |
|
||||
| `:1484` (`WithdrawCommittedChildrenToCellless`) | 0,0 | ✔ explicit D3 edge |
|
||||
| `:1509` then `:1530` (`CommitAcceptedParentCellless`) | 0,0 then parent's | ✔ grandchildren are zeroed then restored inside the same synchronous call |
|
||||
| `:1945` (`CommitRebucket`) | new cell | ✔ D2's headline case |
|
||||
| `:1991` (`CommitWithdrawal`) | 0,0 | ✔ |
|
||||
| `:2737` (`InitializeAcceptedCreateResidence`) | 0,0 | ✔ vacuous — a fresh incarnation has no committed children (`_committedChildrenByParent` is keyed by `(guid, instance)`) |
|
||||
| `RuntimeInitialCreateContinuationExecutor.cs:2188` (dormant pickup replay) | 0,0 | ✔ same shape as `TryApplyPickup` |
|
||||
| `RuntimePhysicsState.cs:2148` (`CommitCanonicalCell`) | new cell | ✔ guarded by an equality early-out at `:2145`, so no probe on a same-cell commit |
|
||||
| `RuntimeSetPositionState.cs:2743` / `:3660` (placement commits) | new cell | ✔ both guarded by `record.FullCellId != result.CellId` |
|
||||
| `:5003` (residency restore) | resident cell | ✔ re-propagates children the park zeroed |
|
||||
| `:5224` (`WithdrawCanonical`) | 0,0 | ✔ park/lost-cell; self-heals through `:5003` |
|
||||
| `LiveEntityRuntime.cs:192` / `:200` (record property setters) | mixed | test-only (no production assignment exists) — but see A7 |
|
||||
|
||||
No caller means "clear only this record": every zeroing site is a
|
||||
retail `leave_world`/`change_cell(null)` analogue, and the two "restore" sites
|
||||
re-propagate. The `RefreshSnapshot` half (`:231-246`) is correctly gated on an
|
||||
actual change and `RefreshDerivedState` has exactly the two callers the contract
|
||||
claimed (record ctor `RuntimeEntityRecord.cs:29` and `RefreshSnapshot`) — I
|
||||
re-verified this repo-wide, so **contract open question 1 is answered: no
|
||||
canonical cell write bypasses the hook.**
|
||||
|
||||
**Termination**: verified by construction and by test. The skip is evaluated in
|
||||
the *parent's* loop before descending, so an A→B→A wire cycle writes A, writes
|
||||
B, then finds A already equal and stops.
|
||||
`PropagationChokepoint_TerminatesAHostileTwoCycle…` pins it. A self-parent
|
||||
(reachable headless, where D5 skips `ValidateParentProjection`'s
|
||||
self-parent rejection at `EquippedChildRenderController.cs:897-898`) also
|
||||
terminates on the first skip.
|
||||
|
||||
**Ordering**: children are written *before* every parent-side dependent step —
|
||||
before `CellCommitted` (`RuntimePhysicsState.cs:2152`), before
|
||||
`AdvancePlacementCommit`/`AcknowledgeSpatialProjection`
|
||||
(`RuntimeSetPositionState.cs:2746-2756`), and before
|
||||
`AcknowledgeProjectionAndPublish` in `CommitRebucket`. There is no synchronous
|
||||
callback anywhere inside the recursion, so no observer can see a
|
||||
partially-propagated tree. This matches retail (`enter_cell`'s child recursion
|
||||
runs inside `change_cell`, before the caller's tail).
|
||||
|
||||
---
|
||||
|
||||
## 1. Findings
|
||||
|
||||
### A1 — MAJOR. The only test for the D4 demotion's presentation half cannot fail if that half is deleted outright.
|
||||
|
||||
**Where:** `tests/AcDream.App.Tests/Rendering/EquippedChildProjectionWithdrawalTests.cs:322-362`
|
||||
(`TickChild_D4_CanonicalCellIsRuntimesWriteNotTheRenderTicks`), against
|
||||
`src/AcDream.App/Rendering/EquippedChildRenderController.cs:403` and `:409-415`.
|
||||
|
||||
The test's two assertions are:
|
||||
|
||||
1. `Assert.Equal(newCell, child.WorldEntity!.ParentCellId!.Value)` — "the render
|
||||
tick moved only the DRAW bucket".
|
||||
2. `Assert.Equal(childSpatialVersionBeforeTick, child.Canonical.SpatialAuthorityVersion)`
|
||||
— "and did not re-write the canonical cell".
|
||||
|
||||
Assertion 1 does **not** test the bucket. `TickChild` writes
|
||||
`child.Entity.ParentCellId = parent.ParentCellId` **unconditionally at
|
||||
`:403`**, before the rebucket call, and `AttachedChild.Entity` is the same
|
||||
`WorldEntity` instance the test reads (the fixture passes
|
||||
`child.WorldEntity!` into the `AttachedChild` ctor —
|
||||
`EquippedChildProjectionWithdrawalTests.cs:1529`). Assertion 2 is a negative
|
||||
that is trivially satisfied by doing nothing.
|
||||
|
||||
**Concrete failure scenario:** delete lines `:409-415` entirely (or let
|
||||
`RebucketEquippedChildPresentation` return `false` on its first guard — see A2).
|
||||
The child's canonical cell is still right, `ParentCellId` still mirrors, both
|
||||
assertions still pass, the whole App suite stays green — and the equipped weapon
|
||||
is **left behind at the previous landblock's draw bucket** when the player
|
||||
crosses a boundary: exactly the "left behind at a landblock boundary (the
|
||||
demotion's specific risk)" regression the contract's §7 gate names, and the
|
||||
`#184` invisible-but-solid family.
|
||||
|
||||
`RebucketEquippedChildPresentation` has **zero** test references repo-wide
|
||||
(grep across `tests/` returns nothing), so this is the slice's only coverage of
|
||||
the presentation transfer.
|
||||
|
||||
Contract §6 test 10 asked for precisely what is missing: *"assert the child's
|
||||
render entity moved buckets (spatial index / visibility state)"*. The
|
||||
implementer's sabotage note covers the canonical half only (reverting to
|
||||
`RebucketLiveEntity` fails assertion 2 — I confirmed that is true, because
|
||||
`RuntimeEntityRecord.SetFullCell` at `:246-251` bumps `SpatialAuthorityVersion`
|
||||
unconditionally); the presentation half's sabotage was not run.
|
||||
|
||||
**Fix direction:** assert against the spatial layer, not the mirror field. The
|
||||
fixture already reaches `LiveEntityRuntime`, so either (a) assert
|
||||
`record.IsSpatiallyVisible` / the spatial index's resident cell for the child
|
||||
key after the tick with the child pre-seeded into a *different* bucket, or
|
||||
(b) assert `RebucketEquippedChildPresentation`'s observable effect through the
|
||||
fake `_spatial` (a recorded `RebucketLiveEntity(key, entity, cell)` call with
|
||||
the new cell). Then run the prescribed sabotage: stub the call out and confirm
|
||||
the test goes red.
|
||||
|
||||
### A2 — MAJOR. The new App entry point can silently write nothing, and its status is discarded. (Route 5's A1 defect class, recurring.)
|
||||
|
||||
**Where:** `src/AcDream.App/World/LiveEntityRuntime.cs:1085-1101`
|
||||
(`RebucketEquippedChildPresentation`) and
|
||||
`src/AcDream.App/Rendering/EquippedChildRenderController.cs:409-415`.
|
||||
|
||||
The method returns `false` — writing nothing — on three conditions
|
||||
(`!HasCommittedParent`, no current projection, no `WorldEntity`). `TickChild`
|
||||
**discards the return value**, then unconditionally returns `true` and raises
|
||||
`ProjectionPoseReady?.Invoke(child.ChildGuid)` at `:416`, i.e. it advances
|
||||
presentation on a write-nothing outcome. That is verbatim the defect the route-7
|
||||
contract lists among the route-5 classes it "addresses by name" (§preamble:
|
||||
*"an App glue site discarding the Runtime seam's status and advancing
|
||||
presentation on write-nothing outcomes"*).
|
||||
|
||||
The `HasCommittedParent` guard is **new**; it did not gate the old
|
||||
`RebucketLiveEntity` call. It is reachable-false while `_attachedByChild` still
|
||||
holds the child, because Runtime clears `_lastAcceptedByChild` (via
|
||||
`EndChildProjection`, `RuntimeEntityObjectLifetime.cs:1307` on pickup and
|
||||
`:1908` on the Position-unparent) **before** the App's
|
||||
`EquippedChildRenderController.OnChildBecameUnparented` (`:264-284`) runs, and
|
||||
that method can defer the teardown across frames
|
||||
(`AdvanceUnparentTransition` + `_pendingOrphanRemovalByChild`, retried at
|
||||
`:342`). During that window every `Tick()` composes the pose, mirrors
|
||||
`ParentCellId`, publishes the pose, raises `ProjectionPoseReady`, and moves
|
||||
**no bucket at all**.
|
||||
|
||||
Today that window is benign (the child is on its way out). The finding is that
|
||||
the benign-ness is accidental and untested: nothing distinguishes "guard
|
||||
correctly declined" from "guard wrongly declined", and A1 means no test would
|
||||
notice either way.
|
||||
|
||||
**Fix direction:** consume the bool. Either treat `false` as a `TickChild`
|
||||
failure (which already has a defined path — `Tick()`'s `failed` list →
|
||||
`WithdrawForPoseLoss` at `:296-297`), or, if declining is legitimate for the
|
||||
unparent window, make that explicit: return a small disposition
|
||||
(`Moved` / `NotAttached`) and assert the `NotAttached` case in a test rather
|
||||
than inferring it. Do not leave a silent bool on the floor.
|
||||
|
||||
### A3 — MEDIUM. P8's consumer enumeration is against the wrong interface; a non-stub `OnEntity` consumer ships.
|
||||
|
||||
**Where:** `src/AcDream.Runtime/GameRuntimeEventHub.cs:45` and `:239-254`;
|
||||
`src/AcDream.Runtime/GameRuntimeEvents.cs:104` and `:246-254`.
|
||||
|
||||
See claim 2 above. `GameRuntimeEventHub` is itself an
|
||||
`IRuntimeEntityObjectObserver` that forwards to every `IRuntimeEventObserver`,
|
||||
and `RuntimeTraceRecorder.OnEntity` records `(delta.Change,
|
||||
delta.Entity.CellId)` for every entity delta including `Rebucketed`.
|
||||
|
||||
**Failure scenario (bounded):** any current or future headless gate / trace
|
||||
assertion that counts entity deltas for an equipped child now sees fewer
|
||||
`Rebucketed` entries than before the demotion, with no note anywhere saying so.
|
||||
I found no such assertion today, so this is a proof defect rather than a live
|
||||
break — but the stated basis for D2's "publish nothing" default ("safe by
|
||||
inspection, every implementation is an empty stub") is not true and must not be
|
||||
carried forward as if it were.
|
||||
|
||||
**Fix direction:** restate P8 in the commit against **both** observer
|
||||
interfaces, name `RuntimeTraceRecorder` explicitly as the one real consumer, and
|
||||
say why a diagnostic trace losing graphical-only child `Rebucketed` entries is
|
||||
acceptable. No code change needed.
|
||||
|
||||
### A4 — MEDIUM. `CommitAcceptedParentCellless` now publishes `Withdrawn` carrying a NON-zero cell.
|
||||
|
||||
**Where:** `src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs:1530-1543`.
|
||||
|
||||
D1's re-cell is placed **before** `AcknowledgeProjectionAndPublish(…,
|
||||
RuntimeEntityChange.Withdrawn, …)`. Prior to this slice, a `Withdrawn` delta
|
||||
from this method always carried `FullCellId == 0`; it now carries the parent's
|
||||
cell whenever the parent is celled — i.e. in the ordinary equip case.
|
||||
|
||||
**Failure scenario:** any observer that treats `Withdrawn` as "no longer
|
||||
resident" and reads `delta.Entity.CellId` to decide where to unregister (the
|
||||
plugin world-state bridge, a future headless bot policy, a future radar/journal
|
||||
consumer) now gets a delta whose kind and payload disagree. The shipped
|
||||
consumers are stubs or diagnostic, so nothing breaks today; the risk is that the
|
||||
invariant "Withdrawn ⇒ cell 0" was previously true and is now silently false,
|
||||
with no test pinning either reading.
|
||||
|
||||
**Fix direction:** cheapest correct answer is to state the new payload contract
|
||||
in the method's doc comment and add one assertion to the D1 attach test
|
||||
(`Attach_ParentCelled_…`) pinning the published change kind **and** the
|
||||
delta's cell, so the pair is deliberate rather than incidental. If any consumer
|
||||
is later found to need the old shape, the re-cell moves after publication —
|
||||
which is safe, because the same-transaction argument is about *callers*, not
|
||||
about the delta.
|
||||
|
||||
### A5 — MEDIUM. D7's reorder was applied to one of the two pickup leave-world sites; the sibling still has retail's inverted order.
|
||||
|
||||
**Where:** fixed at `src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs:1304-1314`;
|
||||
**not** fixed at `src/AcDream.Runtime/Entities/RuntimeInitialCreateContinuationExecutor.cs:2184-2189`
|
||||
(`ApplyPickupAction`, the dormant-residence pickup replay), where
|
||||
`EndChildProjection` still runs **after** `LeaveWorld` / `SetFullCell(0,0)`.
|
||||
|
||||
**Failure scenario:** none observable — the same inertness argument that
|
||||
validates claim 1 applies verbatim to this site. The defect is bookkeeping: D9
|
||||
and the commit will say the recorded T7 inversion is "retired", while a second
|
||||
instance of it remains in-tree at the path a pickup takes when it arrives during
|
||||
a pending initial-create residence. A future reader grepping for the inversion
|
||||
finds it and re-opens a settled question.
|
||||
|
||||
**Fix direction:** either apply the same three-line move at `:2189` (preferred —
|
||||
it is provably inert and makes both pickup paths one shape), or add one
|
||||
sentence to the D7 commit text naming `ApplyPickupAction` as a deliberate,
|
||||
inert survivor.
|
||||
|
||||
### A6 — MEDIUM. D5's per-spawn retry is an O(live-relations) LINQ scan with allocations, on the headless spawn path.
|
||||
|
||||
**Where:** `src/AcDream.Runtime/Session/RuntimeLiveEntitySessionController.cs:143`
|
||||
(`RetryChildrenWaitingForParent` called on **every** accepted spawn) →
|
||||
`src/AcDream.Runtime/Entities/ParentAttachmentState.cs:618-640`
|
||||
(`ChildrenWaitingForParent`), plus `:339-388`
|
||||
(`ResolveAndCommitChildAttachment`).
|
||||
|
||||
`ChildrenWaitingForParent` allocates a `HashSet<uint>`, fully enumerates
|
||||
`_stagedByChild`, `_recoveryByChild` **and** `_unresolvedByChild`, runs
|
||||
`queue.Any(lambda)` per unresolved child, and returns `.ToArray()`.
|
||||
`_recoveryByChild` retains an entry for every child ever committed
|
||||
(`CommitProjection`, `:595`), so it grows with the number of equipped
|
||||
NPCs/players in view. `ResolveAndCommitChildAttachment` additionally allocates
|
||||
three `this`-capturing closures per call (`:341-350`).
|
||||
|
||||
**Failure scenario:** a 30-session headless host (Slice K's stated target)
|
||||
loading a dense landblock does one full scan of three dictionaries per accepted
|
||||
`CreateObject`. With N resident entities of which K are attached children, that
|
||||
is O(N·(N+K)) work and O(N) allocations across the load, on the path Slice K
|
||||
measured to a resource ceiling. The graphical host does the same thing
|
||||
(`EquippedChildRenderController.OnSpawn` → `RetryWaitingDescendants`), so the
|
||||
shape is not novel — but the headless host is the one with a per-process
|
||||
allocation budget and a 30-root gate, and this is new cost there.
|
||||
|
||||
**Fix direction:** the retry only needs children whose *unresolved* queue names
|
||||
this parent. Index that: keep a `Dictionary<uint parentGuid, List<uint> childGuids>`
|
||||
maintained by `Enqueue`/`Resolve`, or at minimum skip the `_recoveryByChild`
|
||||
and `_stagedByChild` sweeps (both are already-resolved states that
|
||||
`ResolveAndCommitChildAttachment` no-ops on). Cache the three callbacks as
|
||||
fields. Then re-measure the K4 30-session resource envelope, or state
|
||||
explicitly that this slice does not.
|
||||
|
||||
### A7 — LOW. The propagation's idempotence key is `FullCellId` alone, but `SetFullCell` takes an independent `canonicalLandblockId`.
|
||||
|
||||
**Where:** `src/AcDream.Runtime/Entities/RuntimeEntityDirectory.cs:389-395`
|
||||
(`child.FullCellId == fullCellId` → `continue`).
|
||||
|
||||
Every production writer derives the landblock from the cell —
|
||||
`(cellId & 0xFFFF0000u) | 0xFFFFu` at `RuntimeEntityRecord.cs:235`,
|
||||
`RuntimePhysicsState.cs:2150`, `RuntimeSetPositionState.cs:2744/3663/5005` — and
|
||||
D1 copies the parent's pair verbatim, so the two fields are coupled today and
|
||||
the skip is safe.
|
||||
|
||||
**Failure scenario:** the coupling is not enforced anywhere.
|
||||
`LiveEntityRecord.CanonicalLandblockId`'s setter
|
||||
(`src/AcDream.App/World/LiveEntityRuntime.cs:200-203`) calls
|
||||
`SetFullCell(Canonical, Canonical.FullCellId, value)` — a same-cell,
|
||||
different-landblock write. It has no production caller today (tests only), but
|
||||
if one ever appears, children keep the **stale** `CanonicalLandblockId` while
|
||||
the parent updates, because the skip fires. `LiveRenderProjectionJournal.cs:273`
|
||||
reads `record.CanonicalLandblockId` to pick the owning landblock, so the
|
||||
symptom would be a child journaled against the wrong landblock.
|
||||
|
||||
**Fix direction:** make the skip test the pair
|
||||
(`child.FullCellId == fullCellId && child.CanonicalLandblockId == canonicalLandblockId`),
|
||||
or assert the derivation invariant in `RuntimeEntityRecord.SetFullCell`. One
|
||||
line either way.
|
||||
|
||||
### A8 — LOW. Wire-driven recursion depth is unbounded; a deep attachment chain is a `StackOverflowException` (process kill).
|
||||
|
||||
**Where:** `src/AcDream.Runtime/Entities/RuntimeEntityDirectory.cs:381-405`
|
||||
(mutual recursion `PropagateFullCellToChildren` ↔ `SetFullCell`).
|
||||
|
||||
Termination against cycles is correct (verified above), but depth is bounded
|
||||
only by the length of the committed parent chain, which is server-supplied.
|
||||
Nothing in `ParentAttachmentState`, `TryApplyParent`, or `TryCommitParent`
|
||||
caps chain depth; only self-parenting is rejected, and only in the graphical
|
||||
`ValidateParentProjection` (which D5 deliberately skips).
|
||||
|
||||
**Failure scenario:** a buggy or hostile server commits a 10k-long
|
||||
A→B→C→… chain; the first cell write on the root overflows the 1 MB stack.
|
||||
`StackOverflowException` is uncatchable in .NET — the process dies, taking all
|
||||
30 headless sessions with it. Not reachable against a well-behaved ACE.
|
||||
|
||||
**Fix direction:** a depth counter with a hard cap (retail's own recursion is
|
||||
equally unbounded, so a cap is an acdream divergence — it belongs in AP-142's
|
||||
clause list) or an explicit iterative worklist over a pre-sized scratch buffer,
|
||||
which also removes the stack cost entirely and stays 0-alloc after warmup.
|
||||
|
||||
### A9 — LOW. The C3c active-residence gate is not carried to the new presentation entry point.
|
||||
|
||||
**Where:** `src/AcDream.App/World/LiveEntityRuntime.cs:1085-1101` vs the gate at
|
||||
`:806-834`.
|
||||
|
||||
The public `RebucketLiveEntity` refuses to install a bucket while an
|
||||
initial-create residence is ACTIVE, with a stated reason ("a re-entrant caller
|
||||
could install a bucket for a suppressed record before its placement ever
|
||||
committed"). `RebucketEquippedChildPresentation` guards on
|
||||
`HasCommittedParent` only.
|
||||
|
||||
**Failure scenario:** bounded — `CommitAcceptedParentCellless` calls
|
||||
`ForgetInitialCreateResidence` (`:1502`) before any `AttachedChild` is
|
||||
installed, so by the time `TickChild` can run, the child's residence is
|
||||
cancelled. The gap is that the guard's *reason* is now implicit in call
|
||||
ordering three files away rather than enforced at the entry point.
|
||||
|
||||
**Fix direction:** add `&& !HasActiveInitialCreateResidence(record.Canonical)`
|
||||
to the new method's guard (cheap, and it makes the "can never become a general
|
||||
bypass" claim in its doc comment actually true), or cite the
|
||||
`ForgetInitialCreateResidence` ordering in that doc comment.
|
||||
|
||||
### A10 — LOW. Duplicate `<summary>` block; one existing test lost its documentation.
|
||||
|
||||
**Where:** `tests/AcDream.Runtime.Tests/Session/RuntimeLiveEntitySessionControllerTests.cs:316-330`.
|
||||
|
||||
The new D5 test's doc comment was inserted **after** the closing `</summary>`
|
||||
of the C3c-R1 F6 block that documented
|
||||
`FirstEntryDriveServesOneRouteAtATimeAndScopesClearToTheOwner`. Both summaries
|
||||
now attach to `DirectSink_D5_StandaloneParentEventCommitsChildToParentsExactCell`;
|
||||
the F6 test is undocumented. Mechanical fix: move the new block below the old
|
||||
one's member, or move the old block down to `:231`.
|
||||
|
||||
---
|
||||
|
||||
## 2. Contract obligations checked and found MET
|
||||
|
||||
Recorded so the next reviewer does not re-derive them.
|
||||
|
||||
- **D2 chokepoint completeness (contract open question 1).** `RefreshDerivedState`
|
||||
has exactly two callers repo-wide (`RuntimeEntityRecord.cs:29` ctor,
|
||||
`RuntimeEntityDirectory.cs:239`); `RuntimeEntityRecord.SetFullCell` has
|
||||
exactly two (`RefreshDerivedState`, `RuntimeEntityDirectory.SetFullCell`).
|
||||
No canonical cell write bypasses the hook.
|
||||
- **D3 delete ordering (P7).** `WithdrawCommittedChildrenToCellless` runs at
|
||||
`:2070` before `DeleteGeneration` at `:2073`, and at `:1005` before
|
||||
`EndGeneration` at `:1009`. In the `EndGeneration` case `RemoveActive` has
|
||||
already run, but the relation table is untouched by it and children are
|
||||
looked up independently, so the read is valid. Recursion to grandchildren
|
||||
rides D2. Pinned by `Delete_ZeroesChildrenBeforeRelationsAreTornDown_P7` and
|
||||
`EndGeneration_ReplacementParentGeneration_…`. No double-withdraw: the second
|
||||
pass would find every child already at 0 and skip.
|
||||
- **No orphaned teardown sites.** `ParentAttachmentState.RemoveObject` /
|
||||
`RemoveChild` have zero production callers, so `DeleteGeneration` /
|
||||
`EndGeneration` are the complete set of relation-teardown edges needing a cell
|
||||
edge, and both got one.
|
||||
- **D8 never-arm partition.** No `ConstrainTo`, park, service-window, or
|
||||
`CanAttemptDestination` code was added; `NeverArmPartition_D8_…` pins the
|
||||
placement-operation count across attach + 5 crossings + pickup + delete.
|
||||
(Contract §6 test 6 also asked for park counts and
|
||||
`RemotePlacementDrivePendingCount`; only `ActiveOperationCount` is asserted —
|
||||
a thinner pin than specified, noted but not a finding on its own.)
|
||||
- **§3 item 10 tripwire.** The classifier diff is a pure deletion of
|
||||
`ClassifyLeaveWorld` + its two types + its one test. `ValidCreateAuthority`
|
||||
survives; no surviving classifier test changed an expectation. 1,156 Runtime
|
||||
tests green.
|
||||
- **D9 bookkeeping.** AP-142 and AP-143 both filed; AP-136's writer list
|
||||
correctly shrinks to the projection materializer alone, and the two mirrored
|
||||
doc comments (`RuntimeSetPositionState.cs:4542`,
|
||||
`RuntimeRemotePlacementDriveController.cs:1619`) were updated in the same
|
||||
diff. Repo grep for stale "TickChild is a canonical writer" claims returns
|
||||
only the corrected sites.
|
||||
- **Headless gate is a real regression test.** Both `DirectSink_D5_…` tests seed
|
||||
parent and child at **distinct** landblocks and assert `NotEqual(0u, …)`
|
||||
alongside the equality, so they cannot pass by coincidence or at zero. Both
|
||||
fail without D1/D2/D5.
|
||||
- **P1.** `P1_SnapshotMutationOfACommittedChild_…` drives an ObjDesc merge and
|
||||
asserts `child.Snapshot.Position` stays null and the cell still tracks the
|
||||
parent — a genuine behavioural pin, not a source-text pin.
|
||||
|
||||
---
|
||||
|
||||
## 3. Remediation required to convert this to PASS
|
||||
|
||||
1. **A1** — rewrite the App test so it asserts the spatial bucket / visibility
|
||||
move, and run the contract's prescribed sabotage (stub
|
||||
`RebucketEquippedChildPresentation` out; the test must go red).
|
||||
2. **A2** — consume `RebucketEquippedChildPresentation`'s status in `TickChild`,
|
||||
and cover the declining branch with a test.
|
||||
3. **A3** — restate P8 against `IRuntimeEventObserver` as well, naming
|
||||
`RuntimeTraceRecorder`.
|
||||
|
||||
A4–A10 are recommended in the same slice (all are one-to-three-line changes or
|
||||
commit-text corrections) but none of them alone blocks the route.
|
||||
|
||||
The **connected two-client gate has not been run** and remains the acceptance
|
||||
test for the D4 transfer regardless of the above — with the contract's own
|
||||
rule that a session showing zero `cause=propagate` lines during the
|
||||
landblock-crossing step is a not-run, not a pass.
|
||||
985
docs/research/2026-08-04-c4-route-7-contract.md
Normal file
985
docs/research/2026-08-04-c4-route-7-contract.md
Normal file
|
|
@ -0,0 +1,985 @@
|
|||
# C4 route 7 — pickup / parent / delete: pinned contract (2026-08-04)
|
||||
|
||||
**Scope:** make Runtime the sole writer of a parented child's canonical cell —
|
||||
port retail `set_parent`'s attach-time `change_cell` half into the Runtime
|
||||
parent commit, add retail's parent-cell-crossing **propagation step** at the
|
||||
one canonical cell-write funnel, demote App's render-tick child rebucket
|
||||
(`EquippedChildRenderController.TickChild`) to presentation-only, give the
|
||||
headless host the parent-realize commit it has never had, adopt retail's
|
||||
pickup ordering, and delete the dead `ClassifyLeaveWorld` classifier entry.
|
||||
Route 7 performs **no placement**: there is no SetPosition, no park, no
|
||||
service window, no leash on this route.
|
||||
|
||||
Pinned at HEAD **`cff52c44`**, clean tree, branch
|
||||
`claude/acdream-physics-divergence-5aa784`. **Line numbers in this contract
|
||||
are as-of `cff52c44` and WILL go stale; every citation also names the symbol —
|
||||
trust the symbol** (process rule 6).
|
||||
|
||||
Predecessor documents, binding where they still apply:
|
||||
|
||||
- [`2026-08-04-retail-parent-cell-propagation.md`](2026-08-04-retail-parent-cell-propagation.md)
|
||||
— **the settling research. Its §10 contract requirements are BINDING and
|
||||
restated in §0 below.** Do not re-derive the retail mechanism; it is read,
|
||||
cited, and offset-verified there.
|
||||
- [`2026-08-04-retail-child-cell-ownership.md`](2026-08-04-retail-child-cell-ownership.md)
|
||||
— the earlier child-cell research: `set_parent` contains no cell write of
|
||||
its own; `unset_parent` performs zero cell work; `leave_world` is where a
|
||||
detaching child is scrubbed.
|
||||
- [`2026-08-04-c4-routes-6-7-scoping.md`](2026-08-04-c4-routes-6-7-scoping.md)
|
||||
§7 — the route scoping. **Its file:line references predate routes 4b-3/5,
|
||||
the OnPosition collapse, and route 6's closure, and are stale throughout;
|
||||
§10 of this contract lists every claim found false or superseded.** Its
|
||||
trap list (T1–T8) survives and is resolved item-by-item below.
|
||||
- [`2026-08-04-c4-route-5-contract.md`](2026-08-04-c4-route-5-contract.md)
|
||||
plus its three dual review rounds — the contract standard, and the
|
||||
recurring defect classes each addressed by name here: an App glue site
|
||||
discarding the Runtime seam's status and advancing presentation on
|
||||
write-nothing outcomes; unrecorded divergences (register rule 1); a pinned
|
||||
obligation left unwired; zero coverage of the presentation layer;
|
||||
negative-only tests.
|
||||
- [`2026-08-04-c4-route-4b-3-contract.md`](2026-08-04-c4-route-4b-3-contract.md)
|
||||
— its 13 "must REMAIN true" invariants; the ones route 7 can even reach are
|
||||
re-asserted in §3.
|
||||
- [`2026-08-04-session-handoff-c4-remaining.md`](2026-08-04-session-handoff-c4-remaining.md)
|
||||
— the six process rules apply verbatim. Rule 1 (the contract causes the
|
||||
defect), rule 4 (assert the layer that broke), and rule 5 (a clean session
|
||||
is not a passed gate) are the load-bearing ones for this route.
|
||||
- [`docs/plans/2026-08-02-placement-cutover.md`](../plans/2026-08-02-placement-cutover.md)
|
||||
— the campaign plan. Its corrected route-7 gap statement ("the child's
|
||||
canonical cell has two writers … the same defect seen from two sides") is
|
||||
exactly this contract's scope; the plan also pins T8 (the `TryCommitParent`
|
||||
`LeaveWorld` omission is retail-REQUIRED).
|
||||
|
||||
**Sequencing:** routes 4a, 4b-1/2/3, the OnPosition collapse, route 5
|
||||
(`36255af0`), and route 6's zero-production closure (`1b484937`) are all in.
|
||||
Route 7 is next; route 3 (portal) remains after it.
|
||||
|
||||
---
|
||||
|
||||
## 0. Facts settled before this contract — BINDING, do not re-derive
|
||||
|
||||
From `2026-08-04-retail-parent-cell-propagation.md` (all addresses verified
|
||||
against `acclient_2013_pseudo_c.txt`, struct offsets closed by the
|
||||
`acclient.h` walk — no PE byte-decode needed):
|
||||
|
||||
1. **Retail re-cells children when the parent crosses a cell, recursively, to
|
||||
unbounded depth.** `CPhysicsObj::SetPositionInternal` @0x00515330 branches
|
||||
on `this->cell == curr_cell` @0x0051536d; the cell-CHANGED branch
|
||||
@0x00515372 calls `change_cell` @0x00513390, which delegates to
|
||||
`leave_cell` @0x0051339f / `enter_cell` @0x005133af. **`change_cell`
|
||||
itself has NO child loop — the recursion lives in the delegates.**
|
||||
`enter_cell` @0x00510ed0 self-recurses over children @0x00510f03 and
|
||||
writes the FULL identity per level: `CObjCell::add_object` @0x00510ee2,
|
||||
`objcell_id` @0x00510f1e, part-array cell id @0x00510f2b, `cell` pointer
|
||||
@0x00510f35, lights @0x00510f3e. `leave_cell` @0x00510f50 mirrors it
|
||||
@0x00510f84. **CORRECTION (post-implementation retail-conformance
|
||||
review, R1 MAJOR): this enumeration silently dropped the guard AROUND
|
||||
all five writes and the recursion itself —
|
||||
`enter_cell`'s entire body is gated on `this->part_array != 0`
|
||||
@0x00510ed8 (the propagation research's own §3 called this "Guard,
|
||||
load-bearing"). A child with a null part array receives NONE of the
|
||||
five writes and its whole subtree is skipped. This is the contract
|
||||
defect the review traced R1 to — process rule 1, "the contract causes
|
||||
the defect" — and it is why an implementer following this list alone
|
||||
ships an unconditional write. See D2's AP-142 clause (d) for why the
|
||||
guard has no reproducible analogue at acdream's canonical layer (its
|
||||
`HasPartArray` field is populated only by the graphical mesh pipeline,
|
||||
never headless) and is therefore recorded, not ported.**
|
||||
2. **The depth-1 loop @0x0051539c–0x005153d8 is the SAME-CELL fast path, NOT
|
||||
the propagation.** It refreshes only each direct child's `objcell_id`
|
||||
(child `+0x4c` @0x005153bd) and part-array id @0x005153cc, deliberately
|
||||
not the `cell` pointer, and only when the parent did NOT change cell. An
|
||||
implementer who finds this loop first will wrongly conclude "depth-1,
|
||||
id-only" and ship a stranded-child bug. **This contract says so
|
||||
explicitly: the propagation is the `else` @0x00515372, not this loop.**
|
||||
3. **The clincher:** `update_object` @0x00515d10 early-returns on
|
||||
`parent != 0` @0x00515d40 — a child never runs its own physics tick, so
|
||||
parent propagation is the ONLY mechanism that maintains a child's cell.
|
||||
4. **The four binding requirements** (research §10): (i) write at attach AND
|
||||
on every parent cell crossing; (ii) the authoritative write belongs on
|
||||
the physics-commit path — Runtime's write must be a PROPAGATION STEP, not
|
||||
a one-shot at `set_parent`; (iii) propagation is recursive — a depth-1
|
||||
implementation needs an explicit stated assumption plus a register row;
|
||||
(iv) write the full identity — id-only leaves the #184 class half-closed.
|
||||
5. **Two adjacent traps:** (a) child cross-cell/shadow lists are NOT
|
||||
refreshed per parent tick — `SetPositionInternal` calls the non-recursive
|
||||
`calc_cross_cells` @0x0051551b; the recursive `recalc_cross_cells`
|
||||
@0x00515a30 runs only at attach (`set_parent` @0x00515b15). Do not
|
||||
rebuild child shadow registrations per crossing. (b) On the removal path
|
||||
(`change_cell` with a null target) retail leaves children with
|
||||
`cell == nullptr` but a STALE non-zero `objcell_id` @0x005133c1 —
|
||||
`leave_cell` never touches child ids. §4 D3 resolves how acdream's
|
||||
single-field model maps this.
|
||||
|
||||
From this contract's own HEAD verification:
|
||||
|
||||
6. **acdream DOES use `FullCellId != 0` / `== 0` as a residency/liveness
|
||||
predicate, pervasively** — 45+ sites, including
|
||||
`RuntimeInitialCreateResidenceState:583` (residence admission),
|
||||
`LiveEntityRuntime` `isOrdinaryRoot` (`:915-918`) and its two sibling
|
||||
predicates (`:3213`, `:3323`), `LiveEntityPresentationController:220`,
|
||||
`RuntimeSetPositionState:2985` (the lost predicate),
|
||||
`HeadlessLocalPlayerFrameHost:87`, and route 4b-3's cell-less
|
||||
classification input (`PreMergeCommittedCellId == 0` → the `SetPosition`
|
||||
cell-less arm). The retail stale-id asymmetry (item 5b) therefore MUST
|
||||
NOT be reproduced literally — see D3.
|
||||
7. **Route 5 and route 6 landed after the scoping**, so the scoping's "route
|
||||
6 first" ordering and its campaign-plan correction are already satisfied
|
||||
(`1b484937` corrected `docs/plans/2026-08-02-placement-cutover.md:97-116`).
|
||||
8. **The canonical cell has exactly ONE funnel.**
|
||||
`RuntimeEntityRecord.SetFullCell` (`RuntimeEntityRecord.cs:244-251`) has
|
||||
exactly two callers: `RuntimeEntityDirectory.SetFullCell`
|
||||
(`RuntimeEntityDirectory.cs:340-346`) and
|
||||
`RuntimeEntityRecord.RefreshDerivedState` (`:230-242`), and
|
||||
`RefreshDerivedState` is itself reached only from the record constructor
|
||||
(`:29`, no children can exist yet) and
|
||||
`RuntimeEntityDirectory.RefreshSnapshot` (`:231-238`). Every producer —
|
||||
`CommitRebucket` (`RuntimeEntityObjectLifetime.cs:1863-1894`),
|
||||
`RuntimePhysicsState.CommitCanonicalCell` (`:2138-2160`, fed by the
|
||||
ordinary/remote/projectile simulation commits and the remote `writeCell`
|
||||
binding `:958-960`), `RuntimeSetPositionState`'s four direct writes
|
||||
(`:2743`, `:3660`, `:5001`, `:5222`), the withdrawal family
|
||||
(`SetFullCell(canonical, 0u, 0u)` at `:1301`, `:1464`, `:1921`, `:2660`),
|
||||
and the wire merge (`RefreshSnapshot` → `RefreshDerivedState`) — funnels
|
||||
through the directory. This is what makes D2's single-chokepoint design
|
||||
sound rather than a per-caller mapping (the 4b-3 review's "mapping
|
||||
written against one caller's reachable set" defect class).
|
||||
9. **The per-parent committed-children list already exists in Runtime.**
|
||||
`ParentAttachmentState.ChildrenAttachedToParent(parentGuid,
|
||||
parentInstanceSequence)` (`ParentAttachmentState.cs:623-633`) returns the
|
||||
exact live CHILDLIST analog (doc comment already cites retail's live
|
||||
CHILDLIST), maintained by `CommitProjection` (`:546-572`) /
|
||||
`RemoveCommittedChild` (`:809-838`). Its two existing consumers are the
|
||||
lost-family deadline arm/cancel (`RuntimeSetPositionState:6024-6036`,
|
||||
`:6046-6060`). It allocates nothing on the read path (returns the stored
|
||||
`List<uint>` or `Array.Empty`).
|
||||
10. **Parented children's snapshots carry no Position.**
|
||||
`InboundPhysicsStateController.ApplyParent` (`:1347-1373`) sets
|
||||
`Position = null` (top-level AND PhysicsSpawnData); `ApplyAcceptedParent`
|
||||
/ `ApplyAcceptedCreateParent` are timestamp-only. So the wire merge's
|
||||
`RefreshDerivedState` cell stamp (`Snapshot.Position is { } position`,
|
||||
`RuntimeEntityRecord.cs:232`) cannot fire for a committed child and
|
||||
cannot fight the propagation. P1 pins this with a test.
|
||||
|
||||
---
|
||||
|
||||
## 1. Site inventory — re-located at `cff52c44`
|
||||
|
||||
Every site verified by reading at HEAD, not inherited from the scoping.
|
||||
|
||||
### 1.1 The Runtime commit family (`src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs`)
|
||||
|
||||
| symbol | at HEAD (was, in scoping) | route-7 relevance |
|
||||
|---|---|---|
|
||||
| `TryApplyPickup` | `:1249-1312` (was `:1226-1245`) | pickup: gate → `RefreshSnapshot` → `ForgetInitialCreateResidence` `:1293` → `AdvancePositionAuthority` `:1294` → `CollisionReports.LeaveWorld` `:1295` → `SetPosition.Forget` `:1297` → `SuspendObjectClock` `:1300` → `SetFullCell(0,0)` `:1301` → `ParentAttachments.EndChildProjection` `:1302` → publish `Withdrawn`. **T7's inversion lives at `:1295-1302`** (leave-world work before the unparent). Dormant-residence deferral `:1255-1281`. |
|
||||
| `TryApplyParent` | `:1336-1386` | accepts/stages the standalone ParentEvent; dormant-residence deferral `:1342-1379`; live path `Entities.TryApplyParent` → `CommitPositionChannelUpdate`. Untouched by this slice. |
|
||||
| `TryApplyCreateParent` | `:1314-1334` | envelope flavor; untouched. |
|
||||
| `TryCommitParent` | `:1388-1441` (was `:1360-1374`) | the parent-relation commit (retail `add_child`-success analog). Carries the C0-4(a) cancellation chokepoint `:1426-1431` and the **F4 deliberate `LeaveWorld` omission comment `:1418-1425`** (T8 — do not "fix"). `AdvanceParentCommit` `:1432`. **D1 does NOT add the re-cell here** — see D1 for why it lives on the cell-less commit's successor instead. |
|
||||
| `CommitAcceptedParentCellless` | `:1443-1474` (was `:1377-1408`) | retail `set_parent`'s `leave_world` edge: cancellations → `CollisionReports.LeaveWorld` `:1462` → `SuspendObjectClock` `:1463` → `SetFullCell(0,0)` `:1464` → publish `Withdrawn`. **D1's extension point: the missing `parent->cell != 0` → `change_cell` half goes immediately after this edge.** |
|
||||
| `TryApplyPosition`'s unparent edge | `EndChildProjection` at `:1838`, after `RefreshSnapshot` `:1830` | the Position-unparent (retail `HandleReceivedPosition`'s `unset_parent` @0x00454129). **Same inversion shape as T7 but on route 4's surface — recorded in §9 as a non-goal, NOT touched here.** Also carries 4b-3's `PreMergeCommittedCellId` measurement `:1801-1814` — see §11 for the cross-contract interaction. |
|
||||
| `CommitRebucket` | `:1863-1894` | the App rebucket's canonical write; publishes `Rebucketed` on an actual cell change. After D4, no equipped-child caller remains. |
|
||||
| `CommitWithdrawal` | `:1896-1929` (was `:1845-1860`) | withdrawal-to-cellless with the C0-4(b) symmetric cancellation; `SetFullCell(0,0)` `:1921`. D2's propagation covers its children automatically. |
|
||||
| `TryAcceptDelete` | `:1973-2034` (was `:1939-1952`) | **`ParentAttachments.DeleteGeneration` runs at `:1996-1998`, BEFORE the active record retires (`RemoveActive` `:2005`), and the delete path performs NO `SetFullCell`** — so D2's chokepoint never fires for a deleted parent's children and D3's explicit delete edge must run before `:1996`. |
|
||||
| `ForgetInitialCreateResidence` / `PreferCancellation` | `:2620-2637` / `:2639-2642` (was `:2552-2569` / `:2571-2574`) | unchanged by this slice. |
|
||||
| `AcknowledgeProjectionAndPublish` | `:2255` on (was `:2187-2213`) | publication discipline: cancellation receipt first, then currency re-check, host ack, publish. Unchanged. |
|
||||
| `CommitChildNoDraw` | `:1931-1950` | retail `set_parent`'s NoDraw inheritance — already ported; untouched. |
|
||||
|
||||
### 1.2 The classifier (`src/AcDream.Runtime/Physics/RuntimeAuthoritativePositionRouteClassifier.cs`)
|
||||
|
||||
| symbol | at HEAD | relevance |
|
||||
|---|---|---|
|
||||
| `RuntimeLeaveWorldCause` | `:31-36` | deleted by D6. |
|
||||
| `RuntimeLeaveWorldRouteRequest` | `:141-145` | deleted by D6. |
|
||||
| `ClassifyLeaveWorld` | `:480-510` (was `:475-505`) | **ZERO production callers at HEAD** (re-verified: repo-wide grep returns the definition, one test at `RuntimeAuthoritativePositionRouteClassifierTests.cs:335-352`, and a comment at `RuntimeInitialCreateContinuationExecutorTests.cs:1672`). Deleted by D6, with the test. |
|
||||
| `ValidCreateAuthority` | `:512-516` (was `:507-512`) | requires `PreviousTeleportSequence == AcceptedTeleportSequence` — the #307 predicate shape. **T3 verified first-hand:** no pickup/parent gate measures a teleport pair (`InboundPhysicsStateController.TryApplyPickup` `:180-186` gates on `TryAcceptPositionChannelEvent` — retail's POSITION stamp @0x0045224B analog; `TryApplyParent` `:263-271` adds only parent-instance currency; `TryCommitParent` `:308-314` re-checks POSITION_TS currency). Wiring the classifier would force a fabricated, vacuously-equal teleport pair. This is D6's second leg. `ValidCreateAuthority` itself SURVIVES (the create route uses it); only the leave-world consumer dies. |
|
||||
|
||||
### 1.3 App (`src/AcDream.App`)
|
||||
|
||||
| symbol | at HEAD | relevance |
|
||||
|---|---|---|
|
||||
| `EquippedChildRenderController.TickChild` | `Rendering/EquippedChildRenderController.cs:373-413`; the rebucket at `:406-408` (was `:405-408`) | the render-tick canonical writer: after pose composition succeeds, `_liveEntities.RebucketLiveEntity(child.ChildGuid, parentCellId)`. **D4's demotion target — the ONLY `RebucketLiveEntity` call in the file (re-verified).** |
|
||||
| `EquippedChildRenderController.PrepareAndTryRealize` | `:841-885` | the graphical realize protocol: `CommitStagedParent` (→ `TryCommitParent`) `:856` → `Relations.CommitProjection` `:857` → `CommitAcceptedParentCellless` `:869-871` → `WithdrawPriorProjection` `:875-881` → `TryRealize`. D1's attach re-cell rides inside the Runtime commit this already calls — the App protocol does not grow a fourth call. |
|
||||
| `EquippedChildRenderController.ValidateParentProjection` | `:887-912` | retail `add_child` validation (Setup `HoldingLocations` via `_dats.Get<Setup>`) — graphical-only today. D5's headless validation question. |
|
||||
| `EquippedChildRenderController.ResolveRelations` | `:786-795` | drives `Relations.Resolve` with snapshot-lookup callbacks — the resolution shape D5's headless drive reproduces Runtime-side. |
|
||||
| `LiveEntityRuntime.RebucketLiveEntity` | `World/LiveEntityRuntime.cs:801-977` (scoping's range still accurate) | the full legacy branch: spatial bucket + `CommitRebucket` `:904-907` + object-clock edges `:919-946` (whose own comment already states "parented/attached objects take retail update_object's parent early-out and remain suspended") + visibility publication. |
|
||||
| `LiveEntityRuntime.RebucketLiveEntityPresentationOnly` | `:993-1065` (was `:993-1050`) | the C3c presentation-only shape: spatial bucket + visibility, **deliberately no `CommitRebucket` / clock work**, guarded by `BeginPresentationOnlySpatialMutation`. Private; sole caller `TryApplyInitialCreateCompletionPresentation` `:1107`. D4 adds the equipped-child entry point beside it. |
|
||||
| `LiveEntityRuntime` wrappers | `TryApplyPickup` `:2312-2316`; `CommitStagedParent` `:2330-2336`; `CommitAcceptedParentCellless` `:2338-2363` (was `:2280-2312`) | the cell-less wrapper's doc (`:2338-2343`) still says "Commits retail `set_parent`'s cell-less edge" — accurate only for `parent->cell == 0`; D9 corrects it with D1. |
|
||||
| `LiveEntityHydrationController.OnPickup` | `World/LiveEntityHydrationController.cs:460-474` (was `:455-474`) | `TryApplyPickup` then `_relationships.OnChildBecameUnparented` — App-level order unchanged by D7 (D7 reorders INSIDE the Runtime method). |
|
||||
| `LiveEntityDeletionController` | `World/LiveEntityDeletionController.cs` | purely logical (re-verified: no placement/cell API). Untouched. |
|
||||
|
||||
### 1.4 Headless (`src/AcDream.Runtime/Session/RuntimeLiveEntitySessionController.cs`)
|
||||
|
||||
| symbol | at HEAD | relevance |
|
||||
|---|---|---|
|
||||
| `OnParentUpdated` | `:312-316` (was `:313-317`) | calls ONLY `Entities.TryApplyParent` — stages the relation forever. **Neither `TryCommitParent` nor `CommitAcceptedParentCellless` has any headless caller (re-verified: the only production callers are `EquippedChildRenderController.cs:856/:869` and the `LiveEntityRuntime` wrappers).** D5's insertion point. |
|
||||
| `OnPickedUp` / `OnDeleted` | `:176-180` / `:155-174` | thin pass-throughs; correct as-is. |
|
||||
|
||||
### 1.5 Confirmed-clean (scoping §7.3, re-verified at HEAD)
|
||||
|
||||
All six cancellation choke points live and symmetric (`:1141`, `:1293-1299`,
|
||||
`:1426-1431`, `:1456-1461`, `:1913-1919`, `:2007-2017`); ordering correct
|
||||
(`AcknowledgeProjectionAndPublish` publishes the cancellation first);
|
||||
receipts host-visible (`RuntimeSetPositionState.PublishCancellation` →
|
||||
`PublishPlacement`, consumed by both graphical sinks and
|
||||
`HeadlessRuntimePlacementProjectionSink`); pickup/parent during a pending
|
||||
residence defer as dormant continuations; grep for
|
||||
`SnapToCell|CommitRebucket|SuspendObjectClock|SetFullCell` across
|
||||
`src/AcDream.App` + `src/AcDream.Headless` still returns no
|
||||
pickup/parent/delete site outside the inventoried ones.
|
||||
|
||||
---
|
||||
|
||||
## 2. Retail ground truth — verified for this contract; verify again yourself
|
||||
|
||||
| claim | anchor | status |
|
||||
|---|---|---|
|
||||
| Pickup = `unset_parent` + `leave_world`, gated ONLY on the POSITION stamp; no placement, no rejection path that skips them once the stamp accepts | `SmartBox::DoPickupEvent` @0x00452240: gate @0x0045224B-0x00452274, stamp write @0x00452278, `unset_parent` @0x0045227F, `leave_world` @0x00452286 | **CORRECTED (retail-conformance review):** the gate span itself is UNVERIFIABLE from this source — Binary Ninja lowered both `DoPickupEvent`'s and `DoParentEvent`'s gate comparisons to a literal always-false expression (`if (-((eax_4 - eax_4)) != 0)`), losing the x87/flag-based wrapped-sequence compare. Only the gate's SHAPE (a wrap-aware sequence compare against `update_times[0]`) and the ORDER of the writes after it are legible — the write order is what D7 relies on and remains ✓. Was previously marked "✓ (scoping §7.2, re-read)", which overstated what the source supports. |
|
||||
| Parent = `set_parent` + `SetPlacementFrame`; `SetParentedState(1)` for a non-player parent gaining its first child | `SmartBox::DoParentEvent` @0x00452290: gate @0x00452296-0x004522C5, @0x004522F4, `set_parent` @0x00452305, `SetPlacementFrame` @0x00452313 | **CORRECTED, same basis as the row above:** the gate span is unverifiable from this source (same BN lowering artifact); the post-gate write order is legible and unaffected. D6's argument does not depend on this citation either — it rests on acdream already enforcing the POSITION_TS gate in `InboundPhysicsStateController` (verified directly), not on retail's gate expression being readable. |
|
||||
| `set_parent` order: `add_child` success → `unset_parent` @0x00515ABA → **one** `leave_world` @0x00515AC1 → `parent =` @0x00515AC6 → `if (parent->cell != 0)` @0x00515AD1 → `change_cell` @0x00515AD6 → `UpdateChild` @0x00515B0E → `recalc_cross_cells` @0x00515B15 → NoDraw inheritance @0x00515B26-38 | `CPhysicsObj::set_parent` @0x00515A90 (4-arg overload @0x00515B50 same shape) | ✓ |
|
||||
| `unset_parent` performs ZERO cell work: `remove_child` → NoDraw restore → `parent = null` → `update_time` → `clear_transient_states` | @0x00513470 (@0x00513484/@0x005134AC/@0x005134BF/@0x005134CE) | ✓ (child-cell-ownership doc §4) |
|
||||
| `leave_world` scrubs the detaching object: `remove_shadows_from_cells` @0x005155DD, recursive `leave_cell(this, 0)` @0x005155E6, zeroes only ITS OWN `objcell_id` @0x005155F4 | `CPhysicsObj::leave_world` @0x005155A0 | ✓ |
|
||||
| The propagation mechanism and its cadence — §0 items 1–3 | @0x00515330 / @0x00513390 / @0x00510ed0 / @0x00510f50 / @0x00515d40 | ✓ (settling research; binding) |
|
||||
| Delete order: `exit_world` @0x0050846B + `leave_world` @0x00508472 run BEFORE `unparent_children` @0x005084B9 — children's cells are nulled by the recursion while still attached, then unparented with no cell restore | `CObjectMaint::DeleteObject` @0x00508460 (same pattern in `DestroyObjects` @0x00508C30) | ✓ |
|
||||
| Cross-cell/shadow: `recalc_cross_cells` @0x00515A30 recurses children @0x00515A79 but runs only at attach; the per-move tail calls the non-recursive forms only @0x0051551B/@0x0051553E-4C | settling research §8 | ✓ (binding trap 5a) |
|
||||
| Route 7 never reaches `HandleReceivedPosition` @0x00453FD0 — `DoPickupEvent`/`DoParentEvent` are separate wire handlers; the single remote `ConstrainTo` arm @0x00454272 is unreachable from this route | scoping T1, re-affirmed | ✓ — the basis of D8 |
|
||||
|
||||
---
|
||||
|
||||
## 3. What must REMAIN true (process rule 1 — for every path, including every refusal)
|
||||
|
||||
1. **A committed child's canonical `FullCellId` equals its parent's at every
|
||||
stable observation point** — after attach (parent celled), after every
|
||||
parent cell crossing (any writer: simulation commit, rebucket, canonical
|
||||
placement, wire merge), after parent teleport, in BOTH hosts. This is the
|
||||
route's headline invariant and the headless gate's assertion (it fails
|
||||
today).
|
||||
2. **The child never becomes a self-simulating object.** Its `ObjectClock`
|
||||
stays suspended, it is never a spatial root, it joins no physics workset,
|
||||
and the propagation path never changes any of that (retail
|
||||
`update_object`'s `parent != 0` early-out @0x00515D40; the existing
|
||||
comment at `LiveEntityRuntime.cs:919-922` already states this rule for
|
||||
the App side).
|
||||
3. **No placement machinery engages on this route.** No
|
||||
`RuntimeSetPositionState` operation, no park, no `DeferredCell`, no
|
||||
service-window pre-flight, no ledger entry — the child cell write is
|
||||
retail `change_cell`: a direct identity write, not a placement (T6). The
|
||||
`ParkCollisionResidents` overlap throw stays unreachable and
|
||||
`RemotePlacementDrivePendingCount` is unaffected by any number of
|
||||
attach/crossing/withdraw events (4b-3 invariants 9/10 extended).
|
||||
4. **`ConstrainTo` is NEVER armed by route 7** — not at attach, not at
|
||||
pickup, not at delete, not on any child, regardless of what routes
|
||||
2/4a/4b/5 established for their arms (T1; D8's partition).
|
||||
5. **The six cancellation choke points and their ordering are unchanged**:
|
||||
exactly-once `ForgetInitialCreateResidence` → `SetPosition.Forget` →
|
||||
`PreferCancellation`, receipt published before the entity delta.
|
||||
6. **`TryCommitParent` keeps exactly zero `CollisionReports.LeaveWorld`
|
||||
calls** (T8; the F4 comment at `:1418-1425` and the campaign plan both
|
||||
pin it — retail `set_parent` has ONE `leave_world` @0x00515AC1, and it is
|
||||
the cell-less commit's edge in acdream's staged protocol).
|
||||
7. **Presentation still advances, and is asserted** (process rule 4 / #312's
|
||||
layer): the equipped child renders in the hand, follows the parent across
|
||||
cell boundaries with no frame where it is bucket-stranded, disappears
|
||||
cleanly on unwield/pickup, and its collision leaves the world with it. A
|
||||
child must never be invisible-but-solid (#184) or solid-but-invisible.
|
||||
8. **No per-crossing child shadow/cross-cell rebuild** (§0 trap 5a). The
|
||||
child's broadphase state is established at attach/unparent edges only.
|
||||
9. **The dormant-residence deferrals are untouched**: pickup/parent arriving
|
||||
during a pending initial residence still enqueue
|
||||
`RuntimeInitialCreateContinuationKind.Pickup`/`Parent` continuations and
|
||||
replay through the executor (`RuntimeInitialCreateContinuationExecutor`'s
|
||||
parent replay `:2110-2126`, which already routes through
|
||||
`RuntimeEntityObjectLifetime.TryCommitParent`).
|
||||
10. **Route 1/2/4/5 classification inputs and dispositions are byte-identical.**
|
||||
Route 7 deletes `ClassifyLeaveWorld` (zero production callers) and
|
||||
changes NOTHING else in the classifier — `ClassifyCreate`'s
|
||||
`Parented`/`PickedUp` residence handling, `ValidCreateAuthority`'s
|
||||
create-route use, and every accepted-position branch stay untouched.
|
||||
Zero expectation changes in surviving classifier tests is the tripwire.
|
||||
11. **AP-135's writes, AP-131, #276, and #316 are untouched** (§9).
|
||||
12. **The lost-family deadline enumeration keeps working**:
|
||||
`ArmLostFamilyDeadlines`/`CancelLostFamilyDeadlines` read
|
||||
`ChildrenAttachedToParent` — D2/D3 change nothing about relation
|
||||
lifetime, only cell values.
|
||||
13. **Ledger convergence**: teardown, session reset, and generation change
|
||||
with committed children present (attached, mid-crossing, mid-unparent)
|
||||
converge the combined ownership ledger to zero — the J-series suites'
|
||||
shape, driven through the new edges.
|
||||
|
||||
---
|
||||
|
||||
## 4. Design decisions — pinned, not open for redesign
|
||||
|
||||
### D1 — the attach half: Runtime completes retail `set_parent`, on the cell-less commit
|
||||
|
||||
Retail's attach sequence (§2 row 3) ends with `if (parent->cell != 0)
|
||||
change_cell(this, parent->cell)`. acdream's realize protocol today ends at
|
||||
the `leave_world` edge (`CommitAcceptedParentCellless`) and lets a render
|
||||
tick supply the re-cell. Pinned:
|
||||
|
||||
- **`CommitAcceptedParentCellless` (or a successor commit it becomes part
|
||||
of) gains retail's second half**: after the cell-less edge's existing
|
||||
writes, if the PARENT's canonical record is active and
|
||||
`parent.FullCellId != 0`, write the child's full canonical cell identity
|
||||
to the parent's exact values (`FullCellId`, `CanonicalLandblockId`) —
|
||||
through the same D2 write path, so attach and crossing are one mechanism,
|
||||
not two. If the parent is cell-less (retail `parent->cell == 0`
|
||||
@0x00515AD1), the child stays cell-less — exactly today's behavior, now
|
||||
by the retail-cited gate instead of by omission.
|
||||
- **Both halves are ONE synchronous Runtime transaction.** No caller may
|
||||
observe the child cell-less between the edge and the re-cell within the
|
||||
same call; no deferred continuation may interleave. The method needs the
|
||||
parent's identity to do this — the natural source is the committed
|
||||
relation (`ParentAttachmentState.TryGetProjection`/`_lastAcceptedByChild`
|
||||
via a lookup, or a parent parameter threaded from the caller, both of
|
||||
which the realize protocol and the executor's parent replay already
|
||||
hold); implementer's choice, pinned constraint: the parent must be
|
||||
resolved by (guid, incarnation) currency, never by guid alone.
|
||||
- **Why not inside `TryCommitParent`?** Retail's cell write follows the
|
||||
`leave_world` (@0x00515AC1 precedes @0x00515AD6). acdream's protocol
|
||||
splits `set_parent` across `TryCommitParent` (relation commit) then
|
||||
`CommitAcceptedParentCellless` (leave-world edge); the re-cell belongs
|
||||
after the second, preserving retail's order. Putting it in
|
||||
`TryCommitParent` would re-cell BEFORE the leave-world edge zeroes it —
|
||||
a self-defeating order. The executor's deferred parent replay and the
|
||||
graphical realize both already call the pair in this order; D5's headless
|
||||
drive calls the same pair.
|
||||
- `UpdateChild` (frame composition) and NoDraw inheritance remain where
|
||||
they are (App pose composition; `CommitChildNoDraw`) — unchanged.
|
||||
- `recalc_cross_cells` @0x00515B15: acdream's analog at attach is the
|
||||
EXISTING behavior (the child's collision reports were force-ended by the
|
||||
cell-less edge; no child broadphase registration exists to rebuild).
|
||||
Pinned: **no new cross-cell/shadow machinery is built at attach**, and P4
|
||||
requires the implementer to state the child's actual broadphase state at
|
||||
each edge with the retail anchor.
|
||||
|
||||
### D2 — the sustaining half: propagation at the one canonical-cell funnel
|
||||
|
||||
**The load-bearing decision.** Retail's trigger is "every mechanism that
|
||||
changes the parent's cell" — in retail that is one function (`change_cell`);
|
||||
in acdream the analog is the one funnel every canonical cell write already
|
||||
passes through (§0 item 8). Pinned:
|
||||
|
||||
- **The propagation hook lives at the directory funnel** — inside
|
||||
`RuntimeEntityDirectory.SetFullCell` and the `RefreshSnapshot` →
|
||||
`RefreshDerivedState` derived write (either by routing the latter through
|
||||
the former or by hooking both; implementer's structural choice, pinned
|
||||
outcome: **no canonical cell write can bypass the hook**). Per-committer
|
||||
hooks (≥8 sites) are REJECTED — that is the "mapping written against one
|
||||
caller's reachable set" defect class, and one missed site is a stranded
|
||||
child.
|
||||
- **The step:** when a record's `FullCellId` changes and
|
||||
`ParentAttachments.ChildrenAttachedToParent(record.ServerGuid,
|
||||
record.Incarnation)` is non-empty, write each active committed child's
|
||||
canonical cell to the parent's new exact values, **recursively** (a
|
||||
child's own committed children follow — retail `enter_cell`/`leave_cell`
|
||||
self-recursion, §0 item 1). Depth-1-only is NOT acceptable without an
|
||||
explicit stated assumption plus a register row (§0 item 4.iii) — and
|
||||
since recursion here is a dictionary probe per level, ship the recursion.
|
||||
- **Termination and idempotence:** skip a child whose `FullCellId` already
|
||||
equals the target value. This terminates any wire-induced relation cycle
|
||||
(self-parenting is already rejected at
|
||||
`EquippedChildRenderController.ValidateParentProjection:890-891`, but
|
||||
A→B→A via wire must still terminate), avoids spurious
|
||||
`SpatialAuthorityVersion` churn, and subsumes retail's same-cell depth-1
|
||||
id refresh (§0 item 2): with one field playing both retail roles, a
|
||||
same-value restamp is unobservable, so the same-cell fast path needs no
|
||||
separate mechanism. **This equivalence is a stated assumption of the
|
||||
single-field model and rides in D9's register row.**
|
||||
- **What the step writes:** the child's canonical `FullCellId` +
|
||||
`CanonicalLandblockId` — acdream's full canonical identity (§0 item 6:
|
||||
the field IS the residency predicate). What it must NOT do: no clock
|
||||
changes, no workset/spatial-root changes, no shadow work, no placement
|
||||
operations, no `CollisionReports` calls, no App callbacks. Field writes
|
||||
plus version bumps only — safe to run re-entrantly inside a
|
||||
`RuntimeSetPositionState`/`RuntimePhysicsState` transaction that is
|
||||
mid-commit on the parent (P5).
|
||||
- **Publication:** per-child lifetime deltas are NOT published from the
|
||||
propagation step, matching the physics-commit precedent
|
||||
(`RuntimePhysicsState.CommitCanonicalCell` publishes no lifetime delta;
|
||||
it fires `CellCommitted`, which is parent-scoped and unchanged). The
|
||||
attach re-cell (D1) rides inside a commit that already publishes; the
|
||||
crossing propagation is silent. **Uniformity note:** today's TickChild
|
||||
path DID publish `Rebucketed` deltas for children via `CommitRebucket`
|
||||
(`:1889-1893`); D4 removes those. P8 requires enumerating `Rebucketed`
|
||||
consumers and confirming none needs a per-child delta — if one does, flip
|
||||
this default and publish uniformly from both D1 and D2, and say so in the
|
||||
commit.
|
||||
- **Allocation:** 0 B on the propagation path (the children list is the
|
||||
stored list; recursion uses the call stack or a pre-sized scratch —
|
||||
Slice I discipline).
|
||||
- **Do not** propagate to `_stagedByChild`/`_recoveryByChild`/unresolved
|
||||
relations — retail's CHILDLIST holds committed children only, and
|
||||
`ChildrenAttachedToParent`'s own doc already pins this ("must not capture
|
||||
staged, unresolved, or future-generation relations").
|
||||
|
||||
### D3 — the withdrawal and delete edges (resolves §0 trap 5b under the single-field model)
|
||||
|
||||
Retail's removal behavior: `leave_cell` recursion nulls each child's `cell`
|
||||
pointer but leaves a stale non-zero `objcell_id`; the functional state is
|
||||
"not resident anywhere." acdream has ONE field, and that field is the
|
||||
residency/liveness predicate at 45+ sites (§0 item 6). Pinned:
|
||||
|
||||
- **Withdrawal propagates zero.** A parent's `SetFullCell(0,0)` (pickup
|
||||
`:1301`, `CommitWithdrawal` `:1921`, cell-less parent commit `:1464`,
|
||||
residence re-begin `:2660`) flows through D2's chokepoint like any other
|
||||
value: committed children (and their subtrees) go cell-less. This is the
|
||||
functional mapping of retail's recursive `leave_cell` — the retail
|
||||
stale-id residue is NOT reproduced, because reproducing it would leave a
|
||||
child "resident" per every acdream predicate while retail's own gating
|
||||
field (`cell == nullptr`) says it is not. **The id/pointer collapse and
|
||||
this deliberate non-reproduction are recorded in D9's register row.**
|
||||
- **Delete gets an explicit edge.** `TryAcceptDelete` performs no
|
||||
`SetFullCell`, and `ParentAttachments.DeleteGeneration` (`:1996`) removes
|
||||
the relations before the record retires — so the chokepoint alone leaves
|
||||
a deleted parent's children stranded at a stale non-zero cell, which
|
||||
under acdream's predicates means "still resident" (the #184 shape, until
|
||||
each child's own DeleteObject arrives). Pinned: **before
|
||||
`DeleteGeneration` runs, the delete path applies the children's
|
||||
leave-world edge** — for each active committed child of the exact deleted
|
||||
incarnation (recursively), cell-less via the same D2 write path. Retail
|
||||
order anchor: `DeleteObject`'s `leave_world` @0x00508472 runs before
|
||||
`unparent_children` @0x005084B9, i.e. children are still attached when
|
||||
the recursion nulls their cells. The children's RELATIONS are then torn
|
||||
down by the existing `DeleteGeneration` exactly as today; the children's
|
||||
own records stay alive awaiting their own wire terminal (retail:
|
||||
`unparent_children` does not destroy children either).
|
||||
- `EndGeneration` (`ParentAttachmentState:665-694`, the replacement-
|
||||
generation path) — same stranding shape, same fix, same edge, applied at
|
||||
its Runtime call site (`RuntimeEntityObjectLifetime:1000`).
|
||||
|
||||
### D4 — the App demotion: TickChild becomes presentation-only (resolves T5)
|
||||
|
||||
- `EquippedChildRenderController.TickChild:406-408` stops calling the
|
||||
public `RebucketLiveEntity` and calls a new **internal equipped-child
|
||||
presentation rebucket** on `LiveEntityRuntime` — the
|
||||
`RebucketLiveEntityPresentationOnly` shape (`:993-1065`: spatial bucket
|
||||
move, visibility resolution, presentation refresh, visibility-change
|
||||
publication, `BeginPresentationOnlySpatialMutation` guard), with
|
||||
**deliberately no `CommitRebucket`, no clock edges** — because after
|
||||
D1/D2, Runtime already owns the canonical commit, which is exactly the
|
||||
C3c precondition that method's doc demands for presentation-only use.
|
||||
(The C3c-R1 R2 warning at `:824-832` — "post-residence moves take the
|
||||
full legacy branch" — does not apply: it protects entities whose ONLY
|
||||
cell authority would otherwise be the graphical rebucket; an equipped
|
||||
child's authority is now the D1/D2 Runtime write.)
|
||||
- The entry point is child-scoped (assert the record has a committed parent
|
||||
relation, or is called only from the equipped-child controller) so it can
|
||||
never become a general bypass of the legacy branch.
|
||||
- **T5's regression risk is the acceptance test, not a reason to keep the
|
||||
old writer:** route 4a's R1 showed that dropping the bucket move leaves an
|
||||
entity body-correct but draw-bucket-stale (invisible-but-solid). The
|
||||
demoted call MUST still move the graphical bucket every time the parent's
|
||||
`ParentCellId` changes — TickChild's existing cadence (per recomposition,
|
||||
with `ParentPresentationMatches`/`CaptureParentPresentation` change
|
||||
detection on `LastParentCellId`, `:426-446`) already provides the
|
||||
trigger; only the canonical half is removed. The connected gate's
|
||||
carry-across-landblock step plus the dual-layer tests (§6) enforce it.
|
||||
- All other TickChild effects (pose composition, `ParentCellId` mirror,
|
||||
draw-visibility inheritance, `PublishChildPose`, `ProjectionPoseReady`)
|
||||
are untouched. `WorldEntity.ParentCellId` remains presentation (AP-133's
|
||||
split is not re-litigated).
|
||||
|
||||
### D5 — the headless parent-realize drive
|
||||
|
||||
`RuntimeLiveEntitySessionController.OnParentUpdated` (`:312-316`) grows the
|
||||
realize that headless never had: after `TryApplyParent` accepts/stages,
|
||||
resolve the staged relation (the `ParentAttachmentState.Resolve` +
|
||||
`TryGetStagedProjection` protocol `ResolveRelations` demonstrates —
|
||||
snapshot-known + instance-currency callbacks, all Runtime-readable) and run
|
||||
the SAME commit pair the graphical protocol runs: `TryCommitParent` →
|
||||
`CommitAcceptedParentCellless`-with-D1. Also drive the deferred/recovery
|
||||
retry the graphical controller performs on parent arrival (children waiting
|
||||
for a parent that appears later — `OnSpawned`'s projection path), to the
|
||||
extent the direct host receives those events; state what is deliberately
|
||||
not driven (pose composition, which is presentation and does not exist
|
||||
headless).
|
||||
|
||||
**The validation gap, pinned rather than discovered later:** retail's
|
||||
`add_child` validates the holding location against the parent's Setup
|
||||
(`CSetup::GetHoldingLocation` @0x0050F896); the graphical host ports this
|
||||
via `ValidateParentProjection`'s DAT read. The headless host reads prepared
|
||||
collision content, which does not expose `Setup.HoldingLocations`. Pinned:
|
||||
**the headless drive commits on gate acceptance + relation resolution
|
||||
alone, skipping the holding-location validation, recorded as a register row
|
||||
in the same commit** (a server-sent invalid location would attach headless
|
||||
where retail/graphical reject — unreachable against a well-behaved ACE, but
|
||||
a divergence and it gets its row; precedent: the content-less host's
|
||||
documented reduced-fidelity registration at
|
||||
`RuntimeLiveEntitySessionController:108-117`). If the reviewer finds
|
||||
`HoldingLocations` cheaply exposable through existing prepared content, that
|
||||
retires the row — but do NOT extend the bake format for it in this slice
|
||||
(stop-and-report if that seems required).
|
||||
|
||||
### D6 — `ClassifyLeaveWorld` is DELETED (resolves T2, informed by T3)
|
||||
|
||||
Delete `ClassifyLeaveWorld` (`:480-510`), `RuntimeLeaveWorldRouteRequest`
|
||||
(`:141-145`), `RuntimeLeaveWorldCause` (`:31-36`), and the one pinning test
|
||||
(`RuntimeAuthoritativePositionRouteClassifierTests:335-352`). Rationale,
|
||||
recorded in the commit:
|
||||
|
||||
- **Retail has no classification here.** `DoPickupEvent` @0x00452240 and
|
||||
`DoParentEvent` @0x00452290 are separate wire handlers dispatching
|
||||
directly; they never reach `HandleReceivedPosition`. Method-per-cause in
|
||||
`RuntimeEntityObjectLifetime` IS the retail shape — the scoping's worry
|
||||
("the cause discriminator is implicit in which method the caller picked")
|
||||
describes retail's own dispatch, not a defect.
|
||||
- **The only gate retail has is the POSITION stamp**, and acdream already
|
||||
enforces exactly that, in Runtime, at
|
||||
`InboundPhysicsStateController.TryApplyPickup/TryApplyParent/TryCommitParent`
|
||||
(§1.2). Wiring the classifier would ADD a second gate
|
||||
(`ValidCreateAuthority`'s teleport-pair equality) that no pickup/parent
|
||||
path can honestly populate (T3, verified) — a vacuous-or-wrong predicate
|
||||
with the #307 defect shape, plus T2's rejected-classification
|
||||
silent-pickup-drop hazard, for zero behavioral gain.
|
||||
- This closes the scoping's "wire it or delete it" demand in the direction
|
||||
the evidence points; the scoping's lean ("wire it") predates the T3
|
||||
verification and is overridden with cause (§10).
|
||||
|
||||
### D7 — pickup ordering adopts retail's (resolves T7)
|
||||
|
||||
`TryApplyPickup` reorders to retail's `unset_parent`-then-`leave_world`:
|
||||
`ParentAttachments.EndChildProjection` moves ahead of
|
||||
`CollisionReports.LeaveWorld` → `SetPosition.Forget` → `SuspendObjectClock`
|
||||
→ `SetFullCell(0,0)` (anchors @0x0045227F before @0x00452286). The
|
||||
cancellation sequence, `AdvancePositionAuthority`, and the publication
|
||||
discipline are unchanged. Verified inert against the new machinery: the
|
||||
picked-up entity's own D2 propagation consults ITS children, not its
|
||||
relation to its parent, so the reorder cannot change propagation; no
|
||||
in-between callback exists (`AcknowledgeProjectionAndPublish` runs after
|
||||
both). This retires the recorded inversion instead of carrying the "not
|
||||
proven inert" caveat forward. **The sibling inversion on the
|
||||
Position-unparent edge (`TryApplyPosition:1830/:1838`) is route 4's surface
|
||||
and is NOT touched — recorded in §9.**
|
||||
|
||||
### D8 — the inverse-leash partition, and the guards that do NOT come along
|
||||
|
||||
The route-7 column of the campaign's constraint-arm partition — stated so an
|
||||
implementer arriving from 4b-2/4b-3/5 ("arm on nonzero return, on every
|
||||
placement outcome") cannot carry the rule across:
|
||||
|
||||
| event | retail path | `ConstrainTo`? | placement? | distance/snap guards? |
|
||||
|---|---|---|---|---|
|
||||
| pickup | `DoPickupEvent` — never reaches `HandleReceivedPosition` | **never** | none | none |
|
||||
| parent (attach) | `DoParentEvent` — same | **never** | none — `change_cell` is an identity write | none |
|
||||
| parent cell crossing (propagation) | `SetPositionInternal` child handling — the PARENT's own route arms whatever ITS route arms; the child arms nothing | **never (for the child)** | none | none |
|
||||
| delete | `DeleteObject` | **never** | none | none |
|
||||
|
||||
Explicitly NOT imported (T4/T6): AP-87's 4 m `BodySnapThreshold`, the 96 m
|
||||
`MaxPhysicsDistance`, `MoveOrTeleport`'s near/far split, 4b-1's
|
||||
service-window machinery, and any `CanAttemptDestination` pre-flight —
|
||||
pickup/parent/delete have no distance concept and no deferrable Core
|
||||
condition (retail's `change_cell` runs no sweep and no `AdjustPosition`).
|
||||
If an implementer finds a reason a child cell write CAN defer, that is a
|
||||
new finding: stop and report.
|
||||
|
||||
### D9 — register and comment bookkeeping, in the implementation commit
|
||||
|
||||
- **ONE new AP row — the parented-child cell model** (three clauses, all
|
||||
intentional-architecture): (a) acdream collapses retail's
|
||||
`cell`-pointer/`objcell_id` pair into one canonical `FullCellId` that is
|
||||
also the residency predicate; consequently (b) the removal path
|
||||
propagates ZERO to children where retail leaves a stale non-zero
|
||||
`objcell_id` under a null pointer (@0x005133C1 / `leave_cell`'s absent id
|
||||
write — deliberate non-reproduction, D3), and (c) retail's same-cell
|
||||
depth-1 per-tick id refresh (@0x005153BD) is subsumed by the
|
||||
value-idempotent chokepoint (D2) rather than ported as a tick loop.
|
||||
Anchors: @0x00513390, @0x00510ed0, @0x00510f50, @0x0051539c-@0x005153d8,
|
||||
@0x00515d40.
|
||||
- **ONE new AP row — headless holding-location validation skip** (D5), if
|
||||
the reviewer confirms no cheap prepared-content read exists.
|
||||
- **AP-136's writer list shrinks**: "the equipped-child renderer
|
||||
`EquippedChildRenderController.TickChild`" dies as a canonical rebucket
|
||||
writer (register line ~287); the surviving non-Position rebucket writer
|
||||
is the projection materializer alone. Update the row and the two doc
|
||||
comments that carry the same claim:
|
||||
`RuntimeSetPositionState.cs:4543` and
|
||||
`RuntimeRemotePlacementDriveController.cs:1617`.
|
||||
- **Comment corrections** (process rule 6, each verified against the code
|
||||
beside it): `LiveEntityRuntime.CommitAcceptedParentCellless`'s doc
|
||||
(`:2338-2343`) — "commits retail set_parent's cell-less edge" gains the
|
||||
D1 second half; the same class's `RebucketLiveEntityPresentationOnly` doc
|
||||
("called ONLY from `TryApplyInitialCreateCompletionPresentation`")
|
||||
updates for the D4 entry point; `TickChild`'s surroundings; the
|
||||
`ParentAttachments.EndChildProjection` doc ("after Pickup or a world
|
||||
Position") if D7's reorder makes its phrasing stale; grep
|
||||
`TickChild|CommitAcceptedParentCellless|RebucketLiveEntity` across
|
||||
`src/` + `docs/architecture/` and re-point every survivor.
|
||||
- **No row deletion.** AP-124, AP-131, AP-132 (queued-parent incarnation
|
||||
gating), AP-133, AP-135 all survive untouched.
|
||||
- **ISSUES.md**: none closed by this slice unless the implementer finds the
|
||||
headless child-cell defect has a filed number (none found at HEAD — it is
|
||||
recorded only in the campaign plan's gap list; update that list's wording
|
||||
when this lands).
|
||||
|
||||
---
|
||||
|
||||
## 5. Proof obligations (must prove, not assume; stated in the implementation commit)
|
||||
|
||||
- **P1 — no merge fights the propagation.** For a committed child, every
|
||||
snapshot-mutation path (ObjDesc, motion, state, PVP bitfield, parent
|
||||
re-commit) leaves `Snapshot.Position` null (§0 item 10), so
|
||||
`RefreshDerivedState` never stamps a child cell from its own snapshot.
|
||||
One test drives each mutation family against an attached child and
|
||||
asserts the canonical cell still tracks the parent.
|
||||
- **P2 — the child stays parent-suspended.** After attach, after ten
|
||||
crossings, and after a parent teleport: child `ObjectClock` suspended,
|
||||
not a spatial root, in no workset, no `RemoteMotion`, body (if any)
|
||||
inactive. (Invariant 2; retail @0x00515D40.)
|
||||
- **P3 — no placement-ledger engagement.** Attach/crossing/withdraw/delete
|
||||
sequences leave `RemotePlacementDrivePendingCount`, SetPosition operation
|
||||
counts, and park counts at their prior values.
|
||||
- **P4 — the child broadphase story, stated.** What is a child's
|
||||
shadow/broadphase registration at attach, across crossings, at
|
||||
unparent-by-Position, at pickup, at parent delete? The implementer writes
|
||||
the answer down with the retail anchors (`leave_world`'s
|
||||
`remove_shadows_from_cells` @0x005155DD; `recalc_cross_cells` at attach
|
||||
only; §0 trap 5a) and confirms the propagation path performs zero shadow
|
||||
work. If a gap is found (e.g. a child shadow that should exist and does
|
||||
not), it is FILED, not silently fixed in this slice.
|
||||
- **P5 — re-entrancy safety of the chokepoint.** The propagation runs
|
||||
inside whatever transaction wrote the parent's cell
|
||||
(`RuntimeSetPositionState` placement commit, `RuntimePhysicsState`
|
||||
simulation commit, `CommitRebucket`, the wire merge). Because it is
|
||||
field-writes-only (D2), it cannot re-enter those owners. Prove with a
|
||||
focused test per writer family plus the existing reset/reentrancy suites
|
||||
green.
|
||||
- **P6 — zero allocation** on the propagation path (the Slice I
|
||||
discipline): a warmed crossing with N children allocates 0 B.
|
||||
- **P7 — delete-edge ordering.** The children's leave-world edge reads
|
||||
`ChildrenAttachedToParent` BEFORE `DeleteGeneration` removes the
|
||||
relations; a test deletes a parent with an attached (and a
|
||||
grand-attached) child and asserts both went cell-less.
|
||||
- **P8 — publication consumers.** Enumerate `RuntimeEntityChange.Rebucketed`
|
||||
consumers; confirm none requires the per-child deltas TickChild's
|
||||
`CommitRebucket` used to produce, or flip D2's publication default and
|
||||
say so. (This is route 5's A1 lesson applied prospectively: the App/host
|
||||
layer must be shown to tolerate the Runtime seam's chosen silence.)
|
||||
|
||||
---
|
||||
|
||||
## 6. Test plan
|
||||
|
||||
Rules (route 5 §7's, verbatim where they apply): assert the layer that
|
||||
historically broke — presentation and canonical cell, not only
|
||||
`InWorld`/clock; assert positive facts, not only negatives; every new test
|
||||
must fail against a broken implementation (no source-text pins). The
|
||||
**dual-HOST discipline** is this route's analog of route 5's dual-kind
|
||||
theories: every Runtime-level scenario runs against the Runtime owners
|
||||
directly (headless-shaped) AND through the graphical wrappers, asserting
|
||||
the same canonical outcome — that is what makes the headless gap a failing
|
||||
test rather than a host-specific accident.
|
||||
|
||||
Focused Runtime tests (`tests/AcDream.Runtime.Tests`):
|
||||
|
||||
1. **Attach, parent celled** (D1): commit pair on a child whose parent has
|
||||
`FullCellId = A` → child ends at A (positive), `Withdrawn`-then-re-celled
|
||||
within one call (no observable cell-less escape), collision reports
|
||||
force-ended, clock suspended, POSITION_TS consumed. Companion: parent
|
||||
cell-less → child stays cell-less (the @0x00515AD1 gate), and a LATER
|
||||
parent cell commit re-cells the child through D2 (the deferred-attach
|
||||
catch-up retail gets for free from propagation).
|
||||
2. **Crossing propagation per writer family** (D2): parent cell changed via
|
||||
(a) `CommitRebucket`, (b) `RuntimePhysicsState.CommitCanonicalCell`
|
||||
(simulation commit), (c) a canonical placement commit
|
||||
(`RuntimeSetPositionState`), (d) the wire merge (`RefreshSnapshot` with
|
||||
a Position) → child follows in every case; grandchild follows
|
||||
(recursion); a cycle (A→B committed both ways by hostile wire) terminates.
|
||||
3. **Same-cell idempotence**: a parent commit to its CURRENT cell leaves
|
||||
child `SpatialAuthorityVersion` unchanged (the D2 short-circuit,
|
||||
positive form: the child was already correct).
|
||||
4. **Withdrawal edges** (D3): pickup of the parent, `CommitWithdrawal` of
|
||||
the parent, and residence re-begin each zero the child (and grandchild);
|
||||
delete of the parent zeroes children BEFORE relations vanish (P7);
|
||||
`EndGeneration` same.
|
||||
5. **Pickup of the child itself** (D7): relation removed before the
|
||||
leave-world writes (order pinned via the relation table's state at the
|
||||
cell write — e.g. a propagation-visible probe or the committed-children
|
||||
list emptiness at `SetFullCell(0,0)` time), cell zeroed, clock
|
||||
suspended, `Withdrawn` published with the cancellation receipt first —
|
||||
and the entity's own children (if any) went cell-less too.
|
||||
6. **Never-arm partition** (D8): after attach + five crossings + pickup +
|
||||
delete, no constraint/`PositionManager` state exists for parent or child
|
||||
beyond what the parent's OWN route had already armed; arm counts
|
||||
unchanged by every route-7 event.
|
||||
7. **No-placement invariant** (P3) and **ledger convergence** (invariant
|
||||
13): teardown/reset/generation-change with children attached,
|
||||
mid-crossing, and mid-unparent.
|
||||
8. **Headless parent-realize** (D5): through
|
||||
`RuntimeLiveEntitySessionController.OnParentUpdated` with a live
|
||||
directory: staged → committed → celled at the parent's cell — **the
|
||||
test named by the route-6 scoping as failing today** (child canonical
|
||||
`FullCellId == parent's` at a stable checkpoint), now in-tree and green;
|
||||
plus the deferred flavor (parent arrives after the relation).
|
||||
9. **Classifier deletion** (D6): surviving classifier tests byte-identical
|
||||
(zero expectation changes — the §3 item 10 tripwire).
|
||||
|
||||
App-layer tests (`tests/AcDream.App.Tests`):
|
||||
|
||||
10. **The demotion keeps presentation whole** (T5/#184's layer): drive the
|
||||
realize + a parent cell change through the graphical stack; assert the
|
||||
child's render entity moved buckets (spatial index / visibility state),
|
||||
`ParentCellId` mirrors the parent, AND the canonical cell was written
|
||||
by Runtime (not by the presentation path — assert
|
||||
`CommitRebucket` was not the writer, e.g. via the presentation-only
|
||||
guard). **Sabotage check (manual, TWO runs — corrected at the
|
||||
architecture review, A1): break D2's propagation and confirm THIS test
|
||||
fails on the CANONICAL half while presentation still moves; separately,
|
||||
stub out the presentation rebucket call (`RebucketEquippedChildPresentation`)
|
||||
and confirm THIS SAME test fails on the PRESENTATION half while the
|
||||
canonical cell is still correct.** The first implementation round ran
|
||||
only the first of these two and shipped an assertion
|
||||
(`child.WorldEntity.ParentCellId`) that TickChild writes unconditionally
|
||||
before the demoted call runs — satisfied whether or not the demoted
|
||||
call executes at all — so the presentation half had zero effective
|
||||
coverage despite the contract asking for it. If EITHER sabotage run
|
||||
leaves the test green, the test is asserting the wrong layer; fix the
|
||||
test. Assert against the actual spatial bucket (e.g. a landblock
|
||||
membership query), not a mirror field TickChild writes elsewhere.
|
||||
11. **Unwield/pickup teardown**: after pickup, the child's projection is
|
||||
gone, no bucket residue, no shadow residue (the invisible-but-solid
|
||||
regression assert, stated positively: the cell is 0, the projection
|
||||
withdrew, the relation is gone).
|
||||
12. **P8's publication check** as a test where feasible (a consumer-facing
|
||||
assertion that the graphical host converges without per-child
|
||||
`Rebucketed` deltas).
|
||||
|
||||
---
|
||||
|
||||
## 7. Gates
|
||||
|
||||
- **Focused**: the §6 suites, green.
|
||||
- **Complete Release suite**:
|
||||
`$env:ACDREAM_PAK_PATH = "$env:USERPROFILE\Documents\Asheron's Call\acdream.pak"`,
|
||||
`dotnet test AcDream.slnx -c Release -m:1`. **Baseline 11,063 passed / 4
|
||||
skipped / 0 failed at `cff52c44`.** The count will move (one classifier
|
||||
test deleted, new suites added) — measure and record the new figure; do
|
||||
not inherit the baseline. Two known flakes, never chase and never
|
||||
conflate: **#302** (`PortalProjectionTests.ClipToRegion_FrameOwnedStore_…`,
|
||||
GC-allocation assertion, App.Tests) and **#308**
|
||||
(`NakEmissionTests.LossSoak_…`, wall-clock deadline, Core.Net.Tests,
|
||||
full-suite load only). If either appears, re-run and say which.
|
||||
- **Connected two-client gate (user-run) — this route IS user-visible.**
|
||||
Probe: `ACDREAM_PROBE_CHILD_CELL=1`, `PhysicsDiagnostics`-owned, marked
|
||||
TEMPORARY with the existing probe family; one `[child-cell]` line per
|
||||
Runtime child-cell write with parent guid, child guid, old→new cell, and
|
||||
cause (`attach` / `propagate` / `withdraw` / `delete` /
|
||||
`headless-attach`). **A session counts as a pass ONLY if the probe shows
|
||||
the propagation executed** (process rule 5; 4b-3's gate precedent — a
|
||||
clean-looking session with zero `cause=propagate` lines during step 2 is
|
||||
a not-run). Recipe (scoping §7.7, carried):
|
||||
1. Equip/unequip cycle — weapon then shield, five times, observer
|
||||
watching: in the hand, at the hand, oriented with the hand, clean
|
||||
disappearance on unwield.
|
||||
2. **Carry across ≥2 landblock boundaries and back**, both directions of
|
||||
observation; expect `cause=propagate` lines at each crossing, child
|
||||
cell always equal to the player's. Include one indoor/dungeon
|
||||
traversal (EnvCell-to-EnvCell crossings are the high-frequency case).
|
||||
3. Pickup: drop the weapon, pick it back up — leaves the ground, no
|
||||
ghost, no invisible collider at the drop site.
|
||||
4. Loot an equipped item from a kill (the delete edge under load).
|
||||
5. Reconnect with equipment — re-attaches.
|
||||
6. Portal recall while equipped — equipment present and following after
|
||||
arrival.
|
||||
Regressions to watch: weapon drawn at the world origin or its last
|
||||
ground position; invisible while equipped; **left behind at a landblock
|
||||
boundary** (the demotion's specific risk); invisible-but-solid at a
|
||||
former position (#184); child culled while the parent is visible or vice
|
||||
versa. Graceful close per the standing ACE session rule.
|
||||
- **Headless gate**: the §6 test 8 assertion (child canonical `FullCellId`
|
||||
equals the parent's at a stable checkpoint) — the direct regression test
|
||||
for the defect, which **fails today** — plus one headless session where
|
||||
the local player equips (via the bot command surface) and crosses a
|
||||
boundary, asserting the same, with `cause=headless-attach`/`propagate`
|
||||
probe lines in the log.
|
||||
|
||||
---
|
||||
|
||||
## 8. Budget and stop conditions
|
||||
|
||||
**Size estimate at HEAD** (supersedes the scoping's §7.6 table, whose shape
|
||||
changed twice — the propagation research added D2/D3, and D6 became a
|
||||
deletion):
|
||||
|
||||
| piece | non-comment production lines |
|
||||
|---|---|
|
||||
| D1 attach re-cell in the cell-less commit | 40-80 |
|
||||
| D2 directory-funnel propagation + recursion/idempotence guards | 50-90 |
|
||||
| D3 delete/EndGeneration explicit edges | 15-35 |
|
||||
| D4 demotion + internal presentation-only child entry point | 40-80 |
|
||||
| D5 headless parent-realize drive | 60-110 |
|
||||
| D6 `ClassifyLeaveWorld` family deletion | net −60 to −70 |
|
||||
| D7 reorder | ~3 |
|
||||
| probe | 15-25 |
|
||||
| **net added** | **~165-355** |
|
||||
|
||||
Within the scoping's 300-490 envelope (below it, thanks to D6 being a
|
||||
deletion). Tests are the larger share, ~450-750 lines.
|
||||
|
||||
**Route 7 remains ONE slice and MUST NOT be split** — re-validated at HEAD:
|
||||
the Runtime canonical write (D1/D2) and the App demotion (D4) are two
|
||||
halves of one transfer. Landing D4 without D1/D2 leaves every equipped
|
||||
child cell-less/stranded (the #184 shape); landing D1/D2 without D4 creates
|
||||
a per-frame two-writer race on the canonical cell — the exact defect class
|
||||
this campaign exists to remove. D5 rides along because it is the same
|
||||
Runtime commit with a thin driver, and the headless gate is the route's
|
||||
direct regression test.
|
||||
|
||||
**Stop and report rather than pushing through when:**
|
||||
|
||||
1. Added production lines exceed **550** — the likely cause would be the
|
||||
propagation needing its own publication/receipt machinery (P8 flipping
|
||||
the default into something structural) or the headless resolve needing
|
||||
more of the graphical protocol than the thin drive assumed; either is a
|
||||
decomposition conversation, not an ad-hoc build.
|
||||
2. Any placement, park, service-window, or `ConstrainTo` machinery starts
|
||||
looking necessary on this route (D8's last paragraph).
|
||||
3. P8 finds a `Rebucketed` consumer that genuinely needs per-child deltas
|
||||
AND publishing them breaks an ordering invariant.
|
||||
4. P4 finds a live child broadphase registration that per-crossing
|
||||
propagation would leave stale (that would mean acdream has child shadow
|
||||
state retail does not, and the design changes).
|
||||
5. D5's validation gap turns out to require extending the prepared-content
|
||||
bake format.
|
||||
6. Any surviving classifier test changes expectation (§3 item 10).
|
||||
7. The complete Release suite deviates from baseline beyond the two named
|
||||
flakes.
|
||||
|
||||
---
|
||||
|
||||
## 9. What this slice does NOT do
|
||||
|
||||
- **AP-131** (shared merge call / `clearParent` gating) — C5. The
|
||||
Position-unparent edge's ordering inversion (`TryApplyPosition:1830/:1838`
|
||||
vs retail @0x00454129) is the same family: **recorded here, not touched**
|
||||
— it belongs with AP-131's route-4-side correction.
|
||||
- **AP-135, #276, #316** — untouched.
|
||||
- **R6-a** (retail `DeclareValid`'s `SetSelectedObject` split-recovery
|
||||
selection transfer) — out of C4, per route 6's closure; file separately.
|
||||
- **`UnparentBeforeRouting` / `ApplyPlacementFrameBeforeRouting`** stay
|
||||
recorded-not-consumed (4b-3's non-goal, carried).
|
||||
- **No child `PositionManager`/interpolation/`RemoteMotion` machinery**; no
|
||||
child self-simulation of any kind.
|
||||
- **No changes to routes 2/4/5/6**, the local-player paths, the remote
|
||||
tail, or the continuation executor beyond the parent-replay path already
|
||||
calling the extended commit.
|
||||
- **No re-litigation of AP-133** (`ParentCellId`/`EffectCellId` split) —
|
||||
the child's render-parent field remains presentation.
|
||||
- **Route 3 (portal)** — after this slice.
|
||||
|
||||
---
|
||||
|
||||
## 10. Stale and false scoping claims — reported, not smoothed (§7 of `2026-08-04-c4-routes-6-7-scoping.md`)
|
||||
|
||||
**Substantively false or superseded (5):**
|
||||
|
||||
1. **T5's open question — "whether retail re-cells a child when its parent
|
||||
crosses a cell is NOT established" — is SETTLED (yes, recursively, from
|
||||
the physics commit)** by the propagation research, which the scoping
|
||||
demanded before demotion. Superseded, exactly as the scoping asked.
|
||||
2. **§7.4's fix shape — "move the parent-cell commit into `TryCommitParent`
|
||||
/ `CommitAcceptedParentCellless`" — is INSUFFICIENT as written.** An
|
||||
attach-only commit is correct at attach and stale from the parent's
|
||||
first crossing (research §10 item 2). D1+D2 replace it: attach half PLUS
|
||||
the sustaining propagation. The scoping's own §7.6 budget row inherited
|
||||
the insufficiency.
|
||||
3. **§7.6's "retail's `change_cell` + `recalc_cross_cells` half" — porting
|
||||
`recalc_cross_cells` per the commit is WRONG**: retail runs the
|
||||
recursive form at attach only; per-move it calls only the non-recursive
|
||||
forms (research §8, binding trap). No cross-cell/shadow rebuild ships.
|
||||
4. **§7.5's lean ("wire `ClassifyLeaveWorld`") is overridden with cause**:
|
||||
T3's verification (no pickup/parent gate measures a teleport pair; the
|
||||
only retail gate is POSITION_TS, already enforced in
|
||||
`InboundPhysicsStateController`) plus retail's separate-wire-handler
|
||||
dispatch make deletion the evidence-backed choice (D6).
|
||||
5. **§8's "recommended order: route 6 first, then route 7" and the
|
||||
campaign-plan correction it demanded are MOOT** — both landed
|
||||
(`1b484937`; plan lines `:97-116` corrected).
|
||||
|
||||
**Stale line references (every RuntimeEntityObjectLifetime citation, plus
|
||||
several others):** `TryApplyPickup :1226-1245` → `:1249-1312`;
|
||||
`TryCommitParent :1360-1374` → `:1388-1441`; `CommitAcceptedParentCellless
|
||||
:1377-1408` → `:1443-1474`; `CommitWithdrawal :1845-1860` → `:1896-1929`;
|
||||
`TryAcceptDelete :1939-1952` → `:1973-2034`; `ForgetInitialCreateResidence
|
||||
:2552-2569` → `:2620-2637`; `AcknowledgeProjectionAndPublish :2187-2213` →
|
||||
`:2255` on; `ClassifyLeaveWorld :475-505` → `:480-510`;
|
||||
`ValidCreateAuthority :507-512` → `:512-516`; TickChild rebucket `:405-408`
|
||||
→ `:406-408`; `RebucketLiveEntityPresentationOnly :993-1050` → `:993-1065`;
|
||||
App wrappers `:2280-2312` → `:2312-2363` (doc comment `:2288-2292` →
|
||||
`:2338-2343`); headless `OnParentUpdated :313-317` → `:312-316`;
|
||||
`OnPickup :455-474` → `:460-474`. The scoping's §7.3 verification table
|
||||
(cancellation choke points `:1074-1094` etc.) is wholly re-verified at the
|
||||
new locations in §1.5. Its structural claims all still hold; only the
|
||||
coordinates moved.
|
||||
|
||||
**Confirmed still true at HEAD:** `ClassifyLeaveWorld` has zero production
|
||||
callers; `RebucketLiveEntity` is not presentation-only (canonical
|
||||
`CommitRebucket` at `:904-907`); headless has no realize; the six
|
||||
cancellation choke points and their C0 fixes; T8's pinned `LeaveWorld`
|
||||
omission; `LiveEntityDeletionController` purely logical; the register's
|
||||
AP-124 status.
|
||||
|
||||
---
|
||||
|
||||
## 11. Cross-contract finding — route 7 changes route 4b-3's cell-less trigger population (reported honestly)
|
||||
|
||||
Route 4b-3's connected gate recorded an honest gap: `cause=cellless` was
|
||||
never observed live, and its closure note says "the unwield-to-3D path is
|
||||
the cheapest reachable trigger" (`2026-08-04-c4-route-4b-3-contract.md`,
|
||||
final section). **After route 7 that provocation stops working, and that is
|
||||
the retail-faithful direction:**
|
||||
|
||||
- Retail: `unset_parent` performs no cell work, so a wielded child's
|
||||
unwield Position reaches `MoveOrTeleport` with `this->cell` = the
|
||||
parent's cell — NON-zero. Retail's cell-less branch does NOT fire for
|
||||
unwield; it fires only for genuinely never-celled/withdrawn bodies.
|
||||
- acdream today: a parented child's canonical cell is whatever the
|
||||
render-tick writer last produced — nonzero in the graphical host while
|
||||
TickChild runs, **zero headless and zero in any pre-first-tick window** —
|
||||
so `PreMergeCommittedCellId == 0` (the 4b-3 D1 input, measured at
|
||||
`TryApplyPosition:1801-1814`) could classify an unwield as cell-less.
|
||||
- After D1/D2: a committed child's pre-merge cell is deterministically the
|
||||
parent's (nonzero whenever the parent is celled), so the unwield Position
|
||||
classifies by TELEPORT_TS/distance — matching retail's predicate
|
||||
population exactly.
|
||||
|
||||
Consequences to carry: (a) 4b-3's test 4 ("unwield-to-3D shape classifies
|
||||
`SetPosition`") remains valid ONLY as a synthetic pre-merge-cell-0 fixture —
|
||||
it must not be re-labeled as the live unwield behavior; (b) the recorded
|
||||
live-closure recipe for `cause=cellless` needs a different provocation
|
||||
(a genuinely withdrawn body receiving a Position without an intervening
|
||||
Create — whether ACE ever emits that shape is unestablished); update the
|
||||
4b-3 contract's closure note in this slice's docs commit rather than
|
||||
leaving a recipe that can no longer fire. No code in the 4b-3 arm changes.
|
||||
|
||||
---
|
||||
|
||||
## 12. Open questions routed to the reviewers
|
||||
|
||||
1. **D2's chokepoint placement** (retail-conformance + architecture): the
|
||||
directory funnel is argued from §0 item 8's caller closure — verify
|
||||
independently that no canonical cell write bypasses
|
||||
`RuntimeEntityDirectory.SetFullCell`/`RefreshSnapshot` at HEAD (the
|
||||
load-bearing claim; if a bypass exists, D2 has a hole exactly where the
|
||||
defect class predicts).
|
||||
2. **D3's delete edge**: confirm by reading the App teardown/orphan flow
|
||||
(`EquippedChildRenderController`'s `_pendingOrphanRemovalByChild`,
|
||||
`LiveEntityRuntimeTeardownController`) that zeroing children's cells at
|
||||
parent delete cannot race a child projection teardown already in flight,
|
||||
and that the child's later own-DeleteObject converges.
|
||||
3. **D4's entry point**: confirm the presentation-only child rebucket
|
||||
cannot be reached for a non-child record (the general-bypass hazard) and
|
||||
that `BeginPresentationOnlySpatialMutation`'s guard semantics hold for
|
||||
the per-frame cadence.
|
||||
4. **D5's validation gap**: confirm no existing prepared-content surface
|
||||
exposes `Setup.HoldingLocations` before accepting the register row; and
|
||||
review what the headless drive deliberately does not drive.
|
||||
5. **P8's publication decision**: adversarially hunt a `Rebucketed`
|
||||
consumer that needs the per-child deltas the demotion removes (route
|
||||
5's A1 class — the App tolerating the seam's silence must be shown, not
|
||||
assumed).
|
||||
6. **D7's inertness argument** — verify no observer distinguishes the
|
||||
reordered pickup sequence (the claim is argued, with the T7 history, not
|
||||
merely asserted; but it is an ordering change on a live path).
|
||||
7. **§11's 4b-3 interaction** — confirm the synthetic fixture reading and
|
||||
that no OTHER consumer of `PreMergeCommittedCellId` changes population
|
||||
when children stop being cell-less.
|
||||
366
docs/research/2026-08-04-c4-route-7-retail-review-round2.md
Normal file
366
docs/research/2026-08-04-c4-route-7-retail-review-round2.md
Normal file
|
|
@ -0,0 +1,366 @@
|
|||
# C4 route 7 — retail-conformance review, ROUND 2 (delta)
|
||||
|
||||
**Date:** 2026-08-04
|
||||
**Reviewer role:** independent retail-conformance reviewer, review-only.
|
||||
**Subject:** the uncommitted working tree at HEAD `19ebf043`, after the round-1
|
||||
fix pass. Round 1 is `docs/research/2026-08-04-c4-route-7-retail-review.md`
|
||||
(verdict FAIL, R1–R11); the parallel architecture review is
|
||||
`docs/research/2026-08-04-c4-route-7-architecture-review.md` (A1–A10).
|
||||
**Scope:** delta only. Round 1's §A retail verification (every address in
|
||||
`enter_cell` / `leave_cell` / `change_cell` / `SetPositionInternal` /
|
||||
`update_object` / `set_parent` / `DoPickupEvent` / `DoParentEvent`) stands
|
||||
unchanged and is not re-litigated here.
|
||||
|
||||
---
|
||||
|
||||
## VERDICT: **PASS**
|
||||
|
||||
All three round-1 MAJORs are closed, and closed properly rather than
|
||||
argued away:
|
||||
|
||||
- **R1** — the implementer's pushback is **CORRECT and my premise was wrong in
|
||||
the letter**: `RuntimeEntityRecord.HasPartArray` does exist on the canonical
|
||||
record. I re-verified the writer enumeration independently and it is complete
|
||||
and correct (§1). R1's *conclusion* — that this is a real, unrecorded retail
|
||||
divergence needing a register row — was right, and AP-142 clause (d) is an
|
||||
honest row that does not mischaracterise retail's intent.
|
||||
- **R2 / A1** — the D4 test now queries real `GpuWorldState` landblock
|
||||
membership before and after the tick. I verified by construction that it
|
||||
cannot pass with the demotion removed (§2).
|
||||
- **R3 / A2** — the typed-disposition fork is **sound**, and the implementer's
|
||||
choice of the sanctioned alternative over the primary suggestion is the
|
||||
better call for a reason the review did not state (§3).
|
||||
|
||||
R5, R7, R9, R10, R11 and all three contract corrections landed. Build green
|
||||
(0 errors); focused suites 22/22 Runtime + 28/28 App. Per process rule 5 that
|
||||
is not the basis of this verdict.
|
||||
|
||||
Seven new MINORs (N1–N7) below, none blocking. Two of them (N1, N5) are the
|
||||
comment-precision class this campaign keeps hitting, and one (N6) is a new
|
||||
untested control-flow branch created by the R3 fix itself.
|
||||
|
||||
---
|
||||
|
||||
## 1. R1 — the pushback, verified on all three questions
|
||||
|
||||
### (a) Is the `HasPartArray` writer enumeration complete, and are both writers graphical-only? — **YES, verified.**
|
||||
|
||||
Repo-wide grep over `src/` + `tests/` (excluding bin/obj) for `HasPartArray`:
|
||||
|
||||
| site | role |
|
||||
|---|---|
|
||||
| `src/AcDream.App/Rendering/DatLiveEntityProjectionMaterializer.cs:203` | `SetHasPartArray(expectedCanonical, true)` — **App / graphical** |
|
||||
| `src/AcDream.App/Rendering/EquippedChildRenderController.cs:609` | `childRecord.HasPartArray = true` — **App / graphical** (the row cites `:591`; see N3) |
|
||||
| `src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs:1167` | `SetHasPartArray(canonical, false)` — Runtime, but a *clear*, not a set |
|
||||
| `RuntimeEntityDirectory.cs:495-498`, `LiveEntityRuntime.cs:250-253/:534-537` | plumbing (setter/forwarder), no value origin |
|
||||
| 6 sites under `tests/` | test-only |
|
||||
| `ProjectileController.cs:681/:720`, `RemotePhysicsUpdater.cs:101`, `LiveEntityAnimationScheduler.cs:240`, `EquippedChildRenderController.cs:920` | **readers only** |
|
||||
|
||||
**No Runtime, Headless, or otherwise presentation-independent site ever sets
|
||||
`HasPartArray = true`, for a child or for an ordinary root.** The implementer's
|
||||
claim is exactly right, and the consequence it draws is right too: gating
|
||||
D1/D2 on `child.HasPartArray` would permanently strand every headless committed
|
||||
child — the functional inverse of this slice's purpose. My round-1 R1 asserted
|
||||
"acdream's propagation has no analogue"; the *field* has an analogue, the
|
||||
*canonical layer* does not. The correction is accepted.
|
||||
|
||||
### (b) Is AP-142 clause (d) an honest description? — **YES, with two precision defects (N1, N2).**
|
||||
|
||||
The row states the guard's address (@0x00510ed8), its full effect (the write
|
||||
**and** the recursion **and** the subtree skip), that acdream writes
|
||||
unconditionally, why gating is not available, and puts the consequence in the
|
||||
risk column. It does not claim fidelity it lacks. That is what a divergence row
|
||||
is supposed to look like.
|
||||
|
||||
### (c) Is retail's guard moot under acdream's structure, or a real behavioural difference? — **A REAL behavioural difference, correctly accepted.**
|
||||
|
||||
Retail's guard is not decorative. `enter_cell`'s `CObjCell::add_object`
|
||||
@0x00510ee2 maintains the cell's object list, which retail uses for *both*
|
||||
drawing and collision/visibility. So a part-array-less object in retail is
|
||||
neither drawn nor cell-resident, and acdream — where `FullCellId` is the
|
||||
residency predicate at 45+ sites — will mark such a child resident. The row says
|
||||
this plainly and puts "acdream celling a child retail would leave nowhere" in
|
||||
the risk column. **No mischaracterisation of retail's intent.** This is the
|
||||
question I was watching for and the row passes it.
|
||||
|
||||
### (b')/R10 — clause (b)'s over-claimed equivalence: **CORRECTED, accurately.**
|
||||
|
||||
The row now reads: *"this is a clean equivalence only on the REMOVAL side. The
|
||||
skip ALSO prunes the child's whole subtree on a same-value WRITE, which retail's
|
||||
`enter_cell` does not do — it recurses over children unconditionally
|
||||
(@0x00510f03); only `leave_cell` prunes (@0x00510f5b, on `cell != 0`).
|
||||
Currently unreachable-by-construction … but it is an asymmetry, not a proven
|
||||
equivalence."* I re-read both addresses: `enter_cell`'s recursion @0x00510f03 is
|
||||
inside the `part_array` guard but has no per-child cell test, and
|
||||
`leave_cell`'s @0x00510f5b prune is exactly as described. **The correction is
|
||||
verbatim-accurate.** R10 closed.
|
||||
|
||||
---
|
||||
|
||||
## 2. R2 / A1 — the D4 presentation test: verified it cannot pass without the demotion
|
||||
|
||||
`tests/AcDream.App.Tests/Rendering/EquippedChildProjectionWithdrawalTests.cs:321-402`.
|
||||
|
||||
Verified by construction, not by taking the sabotage note on trust:
|
||||
|
||||
1. `fixture.Spatial` is a **real `GpuWorldState`** (`:1443`,
|
||||
`internal GpuWorldState Spatial { get; } = new()`), and
|
||||
`CopyLiveEntitiesNearLandblock` (`src/AcDream.App/Streaming/GpuWorldState.cs:493-520`)
|
||||
is a genuine per-landblock query over `_loadedLiveByLandblock`, radius 0, that
|
||||
early-returns empty if the world is unavailable.
|
||||
2. The **pre-assertions** (`Assert.Contains(old, child)` +
|
||||
`Assert.DoesNotContain(new, child)`) do double duty: they establish a
|
||||
distinct starting bucket *and* they prove the query is live — a dead or
|
||||
always-empty query would fail the `Contains` immediately. This is what makes
|
||||
the post-assertion load-bearing rather than vacuous, and it is the exact
|
||||
property round 1's version lacked.
|
||||
3. `TickChild`'s **only** spatial mutation for the child is the demoted call at
|
||||
`EquippedChildRenderController.cs:409-431`. `CommitRebucket` (the Runtime
|
||||
producer driven before the tick) writes canonical fields only and touches no
|
||||
spatial index. So stubbing `RebucketEquippedChildPresentation` leaves the
|
||||
child in `oldLandblock` and `Assert.Contains(newBuffer, child)` fails.
|
||||
|
||||
**R2 closed.** The test now asserts the layer that historically broke.
|
||||
|
||||
---
|
||||
|
||||
## 3. R3 / A2 — the typed-disposition fork: sound, and the deviation from the review's primary suggestion is the better call
|
||||
|
||||
`LiveEntityRuntime.cs:45-61` (the enum), `:1088-1148` (the method),
|
||||
`EquippedChildRenderController.cs:406-431` (the caller).
|
||||
|
||||
The implementer took the *sanctioned alternative* (branch explicitly, do not
|
||||
fail the tick on `NotAttached`) over the review's primary suggestion (assert /
|
||||
propagate as a failure). **That is correct, for a reason worth pinning:**
|
||||
|
||||
- `NotAttached` genuinely coincides with an in-flight subtree withdrawal.
|
||||
`OnChildBecameUnparented` already owns that teardown and drives
|
||||
`BeginProjectionSubtreeWithdrawal`. Returning `false` from `TickChild` would
|
||||
route the same child into `WithdrawForPoseLoss` in the same frame — a
|
||||
**second concurrent withdrawal trigger on a subtree that already has one in
|
||||
flight**. The implementer's stated worry is real, not defensive hand-waving.
|
||||
- `NoProjection` is the branch that *should* fail the tick, and it does. Its
|
||||
first guard (`TryGetCurrent` / `WorldEntity is not { }`) is unreachable from
|
||||
`TickChild` — `TryResolveExactAttachment` (`:379`, and again at `:409`)
|
||||
already required `IsCurrentRecord(child.ChildRecord)` and a non-null
|
||||
`WorldEntity` — so in practice `NoProjection` only arises from
|
||||
`RebucketLiveEntityPresentationOnly` returning `false`, i.e. a projection
|
||||
operation displaced mid-flight, where withdrawal is the right response.
|
||||
|
||||
So the fork partitions cleanly: benign-decline vs displaced-projection, with the
|
||||
new failure path landing only on the genuinely-broken case. **R3 closed** (see
|
||||
N6 for the one gap it leaves).
|
||||
|
||||
A9 was also folded in: the new method now carries the active-initial-create-
|
||||
residence gate (`:1136-1141`), making its "can never become a general bypass"
|
||||
doc claim true at the entry point rather than by call-ordering three files away.
|
||||
|
||||
---
|
||||
|
||||
## 4. Round-1 findings — closure status
|
||||
|
||||
| # | Status |
|
||||
|---|---|
|
||||
| R1 | **CLOSED.** Premise corrected by the implementer (verified §1a); AP-142 clause (d) filed and honest (§1b/c). N1/N2/N4 are precision follow-ups on the row's text, not on the decision. |
|
||||
| R2 | **CLOSED.** §2. |
|
||||
| R3 | **CLOSED.** §3. |
|
||||
| R4 | **PARTIAL — confirmed still non-blocking, now more comfortably so.** Writer families (b) `RuntimePhysicsState.CommitCanonicalCell` and (c) `RuntimeSetPositionState` still have no dedicated propagation test, and P2 is still only asserted on one path. Three things reduce the residual risk below the blocking line: the hook is at the *funnel*, not per-caller (so a missed writer is structurally impossible, not merely unobserved); **both** reviewers have now independently traced the complete `SetFullCell` caller set (the architecture review's 13-row table, which I spot-checked against my own round-1 grep and found consistent); and the connected gate's pass criterion is `cause=propagate` probe lines at each landblock crossing, which *is* the physics/placement writer families in live play. P8 and P4 are now stated in-code (`RuntimeEntityDirectory.cs`'s doc `<para>` blocks) with A3's correction folded in. |
|
||||
| R5 | **CLOSED.** `RuntimeInitialCreateContinuationExecutor.cs:2184-2190` — `EndChildProjection` now precedes `LeaveWorld` on the dormant replay, matching @0x0045227F-before-@0x00452286. Both pickup paths are one shape, and the fix is real code, not a narrowed comment. |
|
||||
| R6 | **CLOSED as documentation.** `RuntimeLiveEntitySessionController.cs:345-368` now states the gap in full — the `TryCommitParent` position-timestamp gate, why nothing re-drives it, why `RetryChildrenWaitingForParent` does not cover it (the relation is staged, not unresolved), that it is not a regression, and that invariant 9 is untouched. That is the right disposition for a pre-existing gap surfaced by a new drive. |
|
||||
| R7 | **CLOSED.** AP-143 now names all three skipped checks with the correct retail anchors and an inertness argument for (1) and (2). Verified against the code: self-parent `:915-916`, `HasPartArray` `:920`, `HoldingLocations` `:924-937`. (Line citations are stale — N3.) |
|
||||
| R8 | **CLOSED / withdrawn.** My nested-descendant concern was wrong: a grandchild's own `ParentEvent` reaches `ResolveAndCommitChildAttachment` directly, and if it arrives before its parent spawns, `RetryChildrenWaitingForParent(childGuid)` fires on that spawn. No transitive descent is needed for this host. The new `ChildrenUnresolvedForParent` (`ParentAttachmentState.cs:642-663`) scopes the sweep correctly and its doc states the narrower claim honestly ("This is a narrower claim than '0 B'"). |
|
||||
| R9 | **CLOSED.** The C3c-R1 F6 summary is back on `FirstEntryDriveServesOneRouteAtATimeAndScopesClearToTheOwner` (`:422-430`); the D5 tests carry their own. |
|
||||
| R10 | **CLOSED.** §1(b'). |
|
||||
| R11 | **CLOSED in substance, incomplete in its stated proof.** See N7. |
|
||||
|
||||
**Contract corrections — all three landed and are accurate:**
|
||||
§0 item 1 now carries the `part_array` guard correction with the "the contract
|
||||
causes the defect" attribution (`:75-87`); §2's two gate rows are marked
|
||||
UNVERIFIABLE with the BN always-false-lowering explanation and the note that
|
||||
D7/D6 do not depend on them (`:236-237`); test 10's sabotage instruction now
|
||||
requires **two** runs with round 1's failure recorded as the cautionary example
|
||||
(`:724-736`).
|
||||
|
||||
---
|
||||
|
||||
## 5. New findings (round 2)
|
||||
|
||||
### N1 — MINOR — AP-142 clause (d) overstates the semantic gap between retail's `part_array` and acdream's `HasPartArray`
|
||||
|
||||
**File:** `docs/architecture/retail-divergence-register.md:172`.
|
||||
**Retail address:** `CPhysicsObj::makeAnimObject` @0x0050e930.
|
||||
|
||||
The row says acdream's flag *"means 'the renderer built a mesh,' not retail's
|
||||
'this CPhysicsObj has ANY part array.'"* Retail's flag has exactly one
|
||||
assignment site:
|
||||
|
||||
```
|
||||
0050e930 int32_t CPhysicsObj::makeAnimObject(CPhysicsObj* this, IDClass<_tagDataID,32,0> arg2, int32_t arg3)
|
||||
0050e93e class CPartArray* eax = CPartArray::CreateSetup(this, arg2, arg3);
|
||||
0050e94d this->part_array = eax;
|
||||
```
|
||||
|
||||
`part_array` **is** the product of building the object's parts from its Setup —
|
||||
i.e. retail's flag also means "the client built this object's parts". The two
|
||||
are near-synonyms, not a broad/narrow pair. The real reason acdream cannot gate
|
||||
on it is a **layering** commitment, not a semantic mismatch: Slice J made the
|
||||
canonical Runtime layer presentation-independent by design, so the only place
|
||||
the flag can be set is App, and headless has no part-construction step at all.
|
||||
|
||||
The row's operative claim and its conclusion survive intact. The framing should
|
||||
be corrected so a future reader does not conclude retail's guard was looser than
|
||||
it is — that misreading would make the divergence look smaller than it is.
|
||||
|
||||
### N2 — MINOR — clause (d)'s risk column scopes the divergence as headless-only; the graphical host has the same window
|
||||
|
||||
**File:** `docs/architecture/retail-divergence-register.md:172` (risk column);
|
||||
`src/AcDream.App/Rendering/EquippedChildRenderController.cs:609` vs `:876-880`.
|
||||
|
||||
The risk column argues (d) is *"a headless-only concern, since the graphical
|
||||
`HasPartArray` gate is already implicitly satisfied by the time `TickChild` can
|
||||
run — a child's own `WorldEntity`/mesh must exist for `TickChild` to reach the
|
||||
rebucket call at all."* That is true of the **rebucket** and false of the
|
||||
**canonical write**, which is what clause (d) is about. The graphical realize
|
||||
order is:
|
||||
|
||||
```
|
||||
PrepareAndTryRealize: CommitStagedParent → CommitProjection
|
||||
→ CommitAcceptedParentCellless ← D1 writes the child's cell HERE
|
||||
→ WithdrawPriorProjection → TryRealize
|
||||
└─ :609 childRecord.HasPartArray = true
|
||||
```
|
||||
|
||||
So the graphical host also cells a child before its part-array flag is true.
|
||||
Retail has no equivalent window: a `CPhysicsObj`'s `part_array` is built at
|
||||
object creation (`makeAnimObject`), long before any `set_parent` @0x00515A90 can
|
||||
run. The window is bounded (nothing reads the child's cell between those two
|
||||
calls in the same synchronous realize) and inert, but the scoping sentence is
|
||||
wrong as written and should say "predominantly headless, plus a bounded
|
||||
graphical realize-ordering window".
|
||||
|
||||
### N3 — MINOR — same-commit stale line citations in both new register rows
|
||||
|
||||
- AP-142 cites `EquippedChildRenderController.cs:591` for the `HasPartArray`
|
||||
writer; it is at **`:609`**.
|
||||
- AP-143 cites `:897-898` (self-parent) and `:902` (`HasPartArray`) in
|
||||
`ValidateParentProjection`; they are at **`:915-916`** and **`:920`**.
|
||||
|
||||
Both are off by the +18 lines this same commit's D4 edit inserted above them —
|
||||
i.e. the citations were written against the pre-fix file and not re-checked
|
||||
after. Contract process rule 6 ("trust the symbol") makes them recoverable, and
|
||||
the symbols are named, so this is cosmetic. It is listed because "verify every
|
||||
comment the fix touched" is a standing rule and these were touched by the fix.
|
||||
|
||||
### N4 — MINOR — clause (e)'s depth-cap residue is the shape clause (a) declares unacceptable, and is strictly worse on the withdraw path
|
||||
|
||||
**File:** `docs/architecture/retail-divergence-register.md:172` (clause e);
|
||||
`src/AcDream.Runtime/Entities/RuntimeEntityDirectory.cs:349-357/:389-397`;
|
||||
`tests/AcDream.Runtime.Tests/Entities/RuntimeEntityChildCellPropagationTests.cs:430-486`.
|
||||
|
||||
Clause (a) says acdream deliberately refuses to reproduce retail's
|
||||
stale-nonzero-cell-under-a-dead-object residue, *because* a stale nonzero
|
||||
`FullCellId` reads as "resident" to every acdream predicate. Clause (e)'s cap
|
||||
re-introduces precisely that residue past 64 levels — the shipped test asserts
|
||||
it directly (`Assert.Equal(originalCell, tail.FullCellId)`).
|
||||
|
||||
On a **crossing** that is a stale-but-plausible cell (cosmetic). On a
|
||||
**withdraw** (`SetFullCell(x, 0, 0)`) the tail keeps a nonzero cell forever,
|
||||
which is the #184 invisible-but-solid shape verbatim. Clause (e) discloses
|
||||
"left at its PRIOR cell rather than partially propagated", so nothing is hidden
|
||||
— but the (a)/(e) interaction is not noted, and the withdraw case is the one
|
||||
worth a sentence. The architecture review's suggested iterative worklist would
|
||||
retire both.
|
||||
|
||||
### N5 — MINOR — the `NotAttached` test's third assertion cannot distinguish decline from a same-destination move
|
||||
|
||||
**File:** `tests/AcDream.App.Tests/Rendering/EquippedChildProjectionWithdrawalTests.cs:404-447`.
|
||||
|
||||
The comment says *"the draw bucket did NOT move, because the guard correctly
|
||||
declined (NotAttached) and `RebucketLiveEntityPresentationOnly` was never
|
||||
called"*, and the assertion is
|
||||
`Assert.Contains(CopyLiveEntitiesNearLandblock(oldLandblock), child)`. In that
|
||||
fixture the parent's `ParentCellId` is never changed, so a rebucket that *did*
|
||||
run would target the child's current bucket and the assertion would pass
|
||||
identically. It cannot fail either way.
|
||||
|
||||
The test's load-bearing assertions — the tick still counted a pose-composition
|
||||
visit and the child is still in `AttachedEntityIds` (i.e. it was **not** treated
|
||||
as pose loss) — are sound and do pin the fork's benign branch. Only the third
|
||||
comment overclaims what its assertion demonstrates. Same class as round 1's R2,
|
||||
one severity lower. Moving the parent to a new cell first would make it real.
|
||||
|
||||
### N6 — MINOR — the `NoProjection` branch is a new withdrawal trigger with no test
|
||||
|
||||
**File:** `src/AcDream.App/Rendering/EquippedChildRenderController.cs:418-424`.
|
||||
|
||||
`TickChild` now returns `false` on `NoProjection`, which routes the child into
|
||||
`WithdrawForPoseLoss` via `Tick()`'s `failed` list — a control-flow edge that
|
||||
did not exist before this slice (the old `RebucketLiveEntity` call's result was
|
||||
never consulted). Round 2 added a test for `Moved` and one for `NotAttached`;
|
||||
`NoProjection` has none. It is the branch with real consequences, and it is
|
||||
reachable only via `RebucketLiveEntityPresentationOnly` returning `false`
|
||||
(a displaced projection operation) — exactly the kind of narrow path that
|
||||
regresses silently.
|
||||
|
||||
### N7 — MINOR — R11's "proven, not merely assumed" enumerates two of three `PhysicsBody` constructors
|
||||
|
||||
**File:** `src/AcDream.App/World/LiveEntityRuntime.cs:1113-1124` (the R11
|
||||
`<para>`).
|
||||
|
||||
The doc argues: a committed child's `Snapshot.Position` is always null, and *"the
|
||||
ONLY production `PhysicsBody` constructors reachable from a live projection
|
||||
(`DatLiveEntityProjectionMaterializer`'s static scheduler, `ProjectileController`)
|
||||
both require a non-null `spawn.Position`"*. Repo-wide grep for
|
||||
`GetOrCreatePhysicsBody` returns a **third** production constructor:
|
||||
`src/AcDream.Runtime/Entities/RuntimeRemoteFirstEntryState.cs:425`, which builds
|
||||
from `lease.InitialCreate.Physics` — and a committed child *can* hold an
|
||||
initial-create lease (that is the whole dormant-residence machinery).
|
||||
|
||||
This matters slightly more than a bookkeeping nit because
|
||||
`RuntimeEntityRecord.SuspendObjectClock` (`RuntimeEntityRecord.cs:94-98`) does
|
||||
**not** synchronise a body's `TransientStateFlags.Active` bit — it only
|
||||
deactivates the clock — so if a committed child ever did hold a body, the
|
||||
dropped `SynchronizePhysicsBodyActiveState` would be a real loss rather than a
|
||||
no-op. The conclusion is still very likely correct (first-entry admission
|
||||
should require a placement position, which a parented child never has), but
|
||||
**I did not resolve that third path and am flagging it rather than guessing.**
|
||||
One sentence covering `RuntimeRemoteFirstEntryState` would convert this back to
|
||||
a genuine proof. This is the same defect shape as the architecture review's A3
|
||||
(an enumeration argued against an incomplete set).
|
||||
|
||||
---
|
||||
|
||||
## 6. Verification performed
|
||||
|
||||
- Re-read `CPhysicsObj::makeAnimObject` @0x0050e930 in
|
||||
`docs/research/named-retail/acclient_2013_pseudo_c.txt` (N1's basis) and
|
||||
re-confirmed `enter_cell` @0x00510f03 / `leave_cell` @0x00510f5b for R10's
|
||||
correction.
|
||||
- Independent repo-wide grep of `HasPartArray` (§1a) and
|
||||
`GetOrCreatePhysicsBody` (N7).
|
||||
- Read `GpuWorldState.CopyLiveEntitiesNearLandblock` and the
|
||||
`ControllerFixture` to confirm the D4 test queries a real spatial index (§2).
|
||||
- Traced `TickChild` → `RebucketEquippedChildPresentation` →
|
||||
`RebucketLiveEntityPresentationOnly` for the disposition fork's reachability
|
||||
(§3).
|
||||
- `dotnet build AcDream.slnx -c Debug` → 0 errors.
|
||||
- `AcDream.Runtime.Tests` filtered to `ChildCellPropagation` +
|
||||
`RuntimeLiveEntitySessionControllerTests` → **22 passed / 0 failed**.
|
||||
- `AcDream.App.Tests` filtered to `EquippedChildProjectionWithdrawalTests` →
|
||||
**28 passed / 0 failed**.
|
||||
- Contract §0 item 1, §2 rows, and §6 test 10 re-read against the code they
|
||||
describe.
|
||||
|
||||
## 7. Recommended before commit (none blocking)
|
||||
|
||||
1. N1 + N2: two sentences in AP-142 clause (d) — correct the `part_array`
|
||||
semantics with the @0x0050e930 anchor, and re-scope the risk column to
|
||||
include the bounded graphical realize-ordering window.
|
||||
2. N3: fix the four line citations (`:591`→`:609`, `:897-898`→`:915-916`,
|
||||
`:902`→`:920`).
|
||||
3. N7: one sentence covering `RuntimeRemoteFirstEntryState.cs:425`, or drop the
|
||||
word "proven".
|
||||
4. N4: one clause noting that (e)'s residue on the *withdraw* path is (a)'s
|
||||
rejected shape.
|
||||
5. N5: move the parent's cell in the `NotAttached` test, or soften its comment.
|
||||
6. N6: a `NoProjection` test, whenever the withdrawal path is next touched.
|
||||
504
docs/research/2026-08-04-c4-route-7-retail-review.md
Normal file
504
docs/research/2026-08-04-c4-route-7-retail-review.md
Normal file
|
|
@ -0,0 +1,504 @@
|
|||
# C4 route 7 — retail-conformance review (independent)
|
||||
|
||||
**Date:** 2026-08-04
|
||||
**Reviewer role:** independent retail-conformance reviewer, review-only (no edits,
|
||||
no commits, no fixes).
|
||||
**Subject:** the uncommitted working-tree diff at branch
|
||||
`claude/acdream-physics-divergence-5aa784`, HEAD `ca96ea5e`
|
||||
(`git diff HEAD` + the two untracked files
|
||||
`docs/research/2026-08-04-c4-route-7-contract.md` and
|
||||
`tests/AcDream.Runtime.Tests/Entities/RuntimeEntityChildCellPropagationTests.cs`).
|
||||
The route-7 contract and the route-3 scoping docs are inputs, not subject.
|
||||
|
||||
---
|
||||
|
||||
## VERDICT: **FAIL**
|
||||
|
||||
Narrow fail. The retail mechanism this slice ports is **correct** — I re-read
|
||||
every cited address in `docs/research/named-retail/acclient_2013_pseudo_c.txt`
|
||||
and the propagation research is accurate on all of them (see §A). The directory
|
||||
chokepoint is genuinely the sole funnel (§B). `ConstrainTo` is never armed (§C).
|
||||
`TryCommitParent` still holds exactly zero `LeaveWorld` calls (§D). D7 adopts
|
||||
retail's real order and is genuinely inert (§E).
|
||||
|
||||
The fail is on three counts, all small to fix:
|
||||
|
||||
1. **R1 (MAJOR)** — retail's `enter_cell` gates the ENTIRE cell write *and the
|
||||
recursion* on `this->part_array != 0` @0x00510ed8. The shipped propagation
|
||||
has no analogue and **no register row**. The propagation research itself
|
||||
flagged this guard as "Guard, load-bearing" (§3); the contract's §0 item 1
|
||||
enumerated `enter_cell`'s five writes and silently dropped the guard, and the
|
||||
code inherited the omission. Register rule 1: a deviation found without a row
|
||||
is a bug twice over.
|
||||
2. **R2 (MAJOR)** — D4's presentation half is **untested**. The one App-layer
|
||||
assertion is satisfied by a line that runs before, and independently of, the
|
||||
demoted call. `RebucketEquippedChildPresentation` has zero references in
|
||||
`tests/` repo-wide. This is exactly the layer contract §6 test 10 named
|
||||
("the child's render entity moved buckets (spatial index / visibility
|
||||
state)") and exactly the #184 / route-4a-R1 regression D4's own text calls its
|
||||
specific risk.
|
||||
3. **R3 (MAJOR)** — `TickChild` discards the demoted call's `bool`, and the new
|
||||
`HasCommittedParent` guard introduces a silent-false path the old
|
||||
`RebucketLiveEntity` did not have. Contract D4 said "**assert** the record has
|
||||
a committed parent relation"; the code returns `false` and the caller ignores
|
||||
it. Route 5's A1 defect class, verbatim.
|
||||
|
||||
Everything else is MINOR. Build is green (`dotnet build AcDream.slnx -c Debug`:
|
||||
0 errors) and the focused Runtime suites pass (21/21, including all 12 new
|
||||
propagation tests and the 2 new headless D5 tests). Per process rule 5 that is
|
||||
not evidence, and it is not what this verdict rests on.
|
||||
|
||||
---
|
||||
|
||||
## A. Retail ground truth — verified first-hand, not inherited
|
||||
|
||||
Every claim below was re-read from
|
||||
`docs/research/named-retail/acclient_2013_pseudo_c.txt` at the stated address.
|
||||
|
||||
| Claim under review | Verdict |
|
||||
|---|---|
|
||||
| `change_cell` @0x00513390 delegates: `leave_cell` @0x0051339f, `enter_cell` @0x005133af, early return @0x005133b5; removal tail @0x005133c1 zeroes only **this**'s `objcell_id`, `cell = nullptr` @0x005133d8. **No child loop of its own.** | ✓ CONFIRMED verbatim |
|
||||
| `enter_cell` @0x00510ed0 self-recurses over `children->objects.data[i]` @0x00510f03; per level writes `CObjCell::add_object` @0x00510ee2, `objcell_id` @0x00510f1e, part-array cell id @0x00510f2b, `cell` pointer @0x00510f35, lights @0x00510f3e. Unbounded depth. | ✓ CONFIRMED verbatim |
|
||||
| `leave_cell` @0x00510f50 mirrors it: self-recursion @0x00510f84, `remove_object` @0x00510f5e, `cell = nullptr` @0x00510fa7, and **never** writes `objcell_id` (source of the stale-id asymmetry). Per-level guard `this->cell != 0` @0x00510f5b prunes an already-cell-less subtree. | ✓ CONFIRMED verbatim |
|
||||
| **The depth-1 loop @0x0051539c-@0x005153d8 is the SAME-CELL fast path.** It sits inside `if (this->cell == curr_cell)` @0x0051536d; the `else` @0x00515372 is `change_cell`. The loop writes child `+0x4c` (`m_position.objcell_id`) @0x005153bd and child `+0x10`'s part-array cell id @0x005153cc — **id only, never `+0x90` (`cell`)**, and **not** recursive. | ✓ CONFIRMED — this is the design's load-bearing claim and it holds |
|
||||
| `update_object` @0x00515d10 early-returns on `parent != 0` @0x00515d40 (`this_3->parent != 0 \|\| this_3->cell == 0 \|\| state & 0x1000000`), clearing `transient_state & ~0x80` and returning. A child never runs its own tick. | ✓ CONFIRMED verbatim |
|
||||
| `DoPickupEvent` @0x00452240 = timestamp gate, stamp write @0x00452278, `unset_parent` @0x0045227f, `leave_world` @0x00452286. **No placement, no `ConstrainTo`, no `HandleReceivedPosition`.** | ✓ CONFIRMED — the `unset_parent`-before-`leave_world` ORDER is unambiguous |
|
||||
| `DoParentEvent` @0x00452290 = gate, `SetParentedState(1)` for a non-player parent gaining its first child @0x004522f4, `set_parent` @0x00452305, `SetPlacementFrame` @0x00452313. No placement, no `ConstrainTo`. | ✓ CONFIRMED verbatim |
|
||||
| `set_parent` @0x00515a90 has **exactly ONE** `leave_world` @0x00515ac1, after `unset_parent` @0x00515aba; then `parent =` @0x00515ac6, `if (edi->cell != 0)` @0x00515ad1, `change_cell` @0x00515ad6, `UpdateChild` @0x00515b0e, `recalc_cross_cells` @0x00515b15, NoDraw inheritance @0x00515b26. | ✓ CONFIRMED — one, and only one, `leave_world` |
|
||||
| Per-move path calls the **non-recursive** `calc_cross_cells` @0x0051551b / shadow rebuild @0x0051553e-4c; the recursive `recalc_cross_cells` @0x00515a30 runs at attach only. | ✓ CONFIRMED |
|
||||
|
||||
**Research-document defects found (2, both minor, neither changes the verdict):**
|
||||
|
||||
- **The "read verbatim" blocks are normalized, not verbatim.** e.g. research §2
|
||||
prints `if ((state & 0x1000) == 0)` where the file reads
|
||||
`if ((*(uint8_t*)((char*)((int16_t)this->state))[1] & 0x10) == 0)`. Same bit,
|
||||
but the doc's own §7 ledger calls these "read directly from the pseudo-C
|
||||
(verbatim, cited)". Paraphrase presented as transcript. Harmless here; a
|
||||
hazard if a future reader diffs against the file.
|
||||
- **The `DoPickupEvent` / `DoParentEvent` gates are NOT readable from this
|
||||
source.** Binary Ninja lowered both to `if (-((eax_4 - eax_4)) != 0)` — a
|
||||
literal always-false expression — because the x87/flag-based wrapped-sequence
|
||||
comparison was lost. The contract's §2 row cites "gate
|
||||
@0x0045224B-0x00452274" as if it were legible; only its *shape* (a
|
||||
wrap-aware sequence compare against `update_times[0]`) is. **Flagged as
|
||||
unverifiable rather than guessed.** It does not affect D7 (the ORDER is
|
||||
legible) or D6 (whose argument is that acdream already enforces the
|
||||
POSITION_TS gate in `InboundPhysicsStateController`, which I verified at
|
||||
`InboundPhysicsStateController.cs:180-186/:263-271/:308-314`).
|
||||
|
||||
---
|
||||
|
||||
## B. D2's chokepoint claim — independently verified
|
||||
|
||||
Contract §12 open question 1 asks whether any canonical cell write bypasses the
|
||||
funnel. Repo-wide grep for `SetFullCell` / `RefreshDerivedState` /
|
||||
`CanonicalLandblockId =` over `src/**` (excluding bin/obj) returns:
|
||||
|
||||
- `RuntimeEntityRecord.SetFullCell` (`RuntimeEntityRecord.cs:244-251`) — two
|
||||
callers only: `RuntimeEntityDirectory.SetFullCell` (`:348-356`, now hooked)
|
||||
and `RuntimeEntityRecord.RefreshDerivedState` (`:230-242`).
|
||||
- `RefreshDerivedState` — reached from the record constructor (`:29`, no
|
||||
children possible) and `RuntimeEntityDirectory.RefreshSnapshot` (`:234-247`,
|
||||
now hooked with an explicit `previousCell` compare).
|
||||
- `LiveEntityRecord.FullCellId` / `.CanonicalLandblockId` setters
|
||||
(`LiveEntityRuntime.cs:189-204`) route to `_directory.SetFullCell`;
|
||||
`LiveEntityRecord.RefreshDerivedState` (`:353-354`) routes to
|
||||
`_directory.RefreshSnapshot`. **No App-side bypass.**
|
||||
- All eight producers (`CommitRebucket`, `RuntimePhysicsState.CommitCanonicalCell`,
|
||||
`RuntimeSetPositionState`'s four writes, the withdrawal family, the executor's
|
||||
`:2188`) call `Entities.SetFullCell`.
|
||||
|
||||
**Conclusion: the chokepoint holds. No hole.** I also confirmed
|
||||
`canonicalLandblockId` is uniformly `(cell & 0xFFFF0000) | 0xFFFF` at every
|
||||
producer, so D2's skip-on-equal-`FullCellId` cannot leave a stale landblock id.
|
||||
|
||||
Cycle termination and re-entrancy were checked by construction: the propagation
|
||||
writes fields and bumps a version only, never mutates the relation tables it is
|
||||
iterating, and `ChildrenAttachedToParent` returns the stored `List<uint>` or
|
||||
`Array.Empty` (0 B).
|
||||
|
||||
---
|
||||
|
||||
## C. Route 7 arms nothing — verified
|
||||
|
||||
`ConstrainTo` / `ConstrainPhase` / `PositionManager` / park / service-window /
|
||||
`CanAttemptDestination` appear **nowhere** in the diff's production hunks. The
|
||||
D8 inversion is respected. `RuntimeEntityChildCellPropagationTests
|
||||
.NeverArmPartition_D8_...` asserts the SetPosition operation ledger is unchanged
|
||||
across attach + five crossings + pickup + delete, and the committed-relation
|
||||
count converges to zero. ✓
|
||||
|
||||
## D. T8 — `TryCommitParent` `LeaveWorld` omission preserved
|
||||
|
||||
`RuntimeEntityObjectLifetime.TryCommitParent` (`:1400-1453`) still has zero
|
||||
`Physics.CollisionReports.LeaveWorld` calls; the F4 comment (`:1430-1437`) is
|
||||
intact and its retail citation (@0x00515A90's single `leave_world`) is correct
|
||||
per §A. No second `LeaveWorld` was added anywhere in the parent commit family. ✓
|
||||
|
||||
## E. D7's inertness — verified, and the implementer's honest report is CORRECT
|
||||
|
||||
The implementer reported that reverting D7's ordering leaves all 12 propagation
|
||||
tests green, and reported it rather than manufacturing a test. **That is true,
|
||||
and "unobservable" is genuinely true against today's code**, for a reason worth
|
||||
writing down:
|
||||
|
||||
- `EndChildProjection(guid)` → `RemoveCommittedChild(guid)` removes the entity
|
||||
as a **child** (`_lastAcceptedByChild` + its parent's committed list). It does
|
||||
**not** touch `_committedChildrenByParent[(guid, incarnation)]` — the entity's
|
||||
own children. `RemoveCommittedParentReferences` would, and is not called.
|
||||
- The only work between the two positions is
|
||||
`CollisionReports.LeaveWorld` → `SetPosition.Forget` → `SuspendObjectClock` →
|
||||
`SetFullCell(0,0)`. I checked every `ChildrenAttachedToParent` consumer
|
||||
(`RuntimeSetPositionState.cs:6015-6063`, `ArmLostFamilyDeadlines` /
|
||||
`CancelLostFamilyDeadlines`): both enumerate `operation.Record`'s **own**
|
||||
children, i.e. the picked-up entity's subtree, which `EndChildProjection` does
|
||||
not alter. Nothing else reads the child-relation table in that window.
|
||||
- Even in a hostile A→B/B→A cycle both orders converge to the same values.
|
||||
|
||||
So: **no observable difference exists that anyone failed to construct.** The
|
||||
correctness is contingent on "no consumer reads the child-relation table between
|
||||
those two points" — a property of today's code, not a semantic equivalence — so
|
||||
adopting retail's real order is right on principle and the change should stand.
|
||||
See R5 for the half that was missed.
|
||||
|
||||
---
|
||||
|
||||
## Findings
|
||||
|
||||
### R1 — MAJOR — retail's `part_array != 0` guard is dropped, with no register row
|
||||
|
||||
**File:** `src/AcDream.Runtime/Entities/RuntimeEntityDirectory.cs:381-403`
|
||||
(`PropagateFullCellToChildren`) and
|
||||
`src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs:1517-1535` (D1).
|
||||
**Retail address contradicted:** `CPhysicsObj::enter_cell` @0x00510ed8.
|
||||
|
||||
Retail:
|
||||
|
||||
```
|
||||
00510ed0 void __thiscall CPhysicsObj::enter_cell(CPhysicsObj* this, CObjCell* arg2)
|
||||
00510ed8 if (this->part_array != 0) <-- the ENTIRE body, including the
|
||||
00510ee2 CObjCell::add_object(...) child recursion, is inside this
|
||||
00510f03 enter_cell(child, arg2) guard
|
||||
00510f1e objcell_id = ...
|
||||
00510f35 this->cell = arg2
|
||||
```
|
||||
|
||||
A child with a null `part_array` receives **nothing** — no `add_object`, no
|
||||
`objcell_id`, no `cell` pointer — and **its whole subtree is skipped**, because
|
||||
the recursion is inside the guard. The propagation research called this out
|
||||
explicitly and named it load-bearing (§3, "Guard, load-bearing"). The route-7
|
||||
contract's §0 item 1 lists the five writes and omits the guard entirely, and the
|
||||
shipped propagation writes the child's cell unconditionally.
|
||||
|
||||
**Why it matters in acdream specifically:** `FullCellId` is the residency/liveness
|
||||
predicate at 45+ sites (the contract's own §0 item 6). Writing a nonzero cell for
|
||||
an object retail would leave nowhere makes it "resident" to every one of those
|
||||
predicates — the #184 shape, arrived at from the other direction. The graphical
|
||||
host's `ValidateParentProjection` checks `parent.HasPartArray`
|
||||
(`EquippedChildRenderController.cs:902`) — the **parent**'s, not the child's —
|
||||
so it is not the analogue.
|
||||
|
||||
**Correct behaviour:** either port the per-level guard (acdream has a
|
||||
`HasPartArray` notion on the graphical record, but not on the canonical one, so
|
||||
this may be a deliberate impossibility headless), **or** file a register row
|
||||
stating that acdream's canonical record has no part-array concept, that the
|
||||
guard is therefore not reproducible at the canonical layer, and what the
|
||||
consequence is. Register rule 1 requires the row in the same commit. Right now
|
||||
neither exists, and AP-142 — which is otherwise a careful, honest row — reads as
|
||||
if the port were complete.
|
||||
|
||||
### R2 — MAJOR — D4's presentation half has zero effective coverage
|
||||
|
||||
**File:** `tests/AcDream.App.Tests/Rendering/EquippedChildProjectionWithdrawalTests.cs:334-374`
|
||||
(assertions at `:368-373`), against
|
||||
`src/AcDream.App/Rendering/EquippedChildRenderController.cs:403` vs `:409-415`.
|
||||
|
||||
The test's only presentation assertion is:
|
||||
|
||||
```csharp
|
||||
Assert.Equal(newCell, child.WorldEntity!.ParentCellId!.Value);
|
||||
```
|
||||
|
||||
`TickChild` sets that field at **line 403**:
|
||||
|
||||
```csharp
|
||||
child.Entity.ParentCellId = parent.ParentCellId; // :403 — unconditional
|
||||
...
|
||||
if (TryResolveExactAttachment(child, out parent)
|
||||
&& parent.ParentCellId is { } parentCellId) // :409
|
||||
{
|
||||
_liveEntities.RebucketEquippedChildPresentation(...); // :412 — the demoted call
|
||||
}
|
||||
```
|
||||
|
||||
The assertion is therefore satisfied whether or not
|
||||
`RebucketEquippedChildPresentation` runs, returns `false`, or is deleted. Grep
|
||||
confirms `RebucketEquippedChildPresentation` has **zero** references under
|
||||
`tests/`. The spatial bucket (`_spatial.RebucketLiveEntity`), the visibility
|
||||
resolution (`IsLiveEntityProjectionResident`), and
|
||||
`PublishProjectionVisibilityChanged` — the things
|
||||
`RebucketLiveEntityPresentationOnly` actually does — are unasserted.
|
||||
|
||||
Contract §6 test 10 required exactly this and required a sabotage check. The
|
||||
test's own doc comment claims sabotage verification, but the sabotage described
|
||||
("reverting TickChild to call the public `RebucketLiveEntity` again makes the
|
||||
final assertion fail, because that call always bumps `SpatialAuthorityVersion`")
|
||||
only exercises the **canonical** half. The presentation half — "left behind at a
|
||||
landblock boundary", D4's named specific risk, and #184's layer — is unguarded.
|
||||
|
||||
**Correct behaviour:** assert the spatial index / visibility state moved (the
|
||||
route-4a R1 lesson), and sabotage-verify by no-op'ing
|
||||
`RebucketEquippedChildPresentation`.
|
||||
|
||||
### R3 — MAJOR — the demoted call's status is discarded, and its new guard can fail silently
|
||||
|
||||
**File:** `src/AcDream.App/Rendering/EquippedChildRenderController.cs:412-415`;
|
||||
`src/AcDream.App/World/LiveEntityRuntime.cs:1085-1101`.
|
||||
|
||||
```csharp
|
||||
internal bool RebucketEquippedChildPresentation(uint serverGuid, uint parentCellId)
|
||||
{
|
||||
if (!_directory.ParentAttachments.HasCommittedParent(serverGuid))
|
||||
return false; // NEW failure mode
|
||||
if (!_projections.TryGetCurrent(serverGuid, out LiveEntityRecord? record)
|
||||
|| record.WorldEntity is not { } entity)
|
||||
return false; // NEW failure mode
|
||||
...
|
||||
}
|
||||
```
|
||||
|
||||
`TickChild` ignores the return and still returns `true` (`:417`). The old
|
||||
`RebucketLiveEntity` had no committed-relation precondition, so both of these are
|
||||
paths that previously could not exist. `EndChildProjection` /
|
||||
`RemoveCommittedChild` run at several points while a projection is still
|
||||
installed (`EquippedChildRenderController.cs:270`, `:1175`, `:1274`, `:1316`;
|
||||
`RuntimeInitialCreateContinuationExecutor.cs:1998`, `:2189`), so a tick landing in
|
||||
that window now silently stops moving the draw bucket, and the pose loop reports
|
||||
success.
|
||||
|
||||
Contract D4 pinned "the entry point is child-scoped (**assert** the record has a
|
||||
committed parent relation…)". A silent `false` swallowed by the caller is the
|
||||
route-5 A1 class the contract's own predecessor list names: *"an App glue site
|
||||
discarding the Runtime seam's status and advancing presentation on write-nothing
|
||||
outcomes."*
|
||||
|
||||
**Correct behaviour:** either throw/assert on the guard (it is an invariant, not a
|
||||
condition), or propagate the `false` into `TickChild`'s result so the existing
|
||||
`WithdrawForPoseLoss` path handles it. Note this finding is only *observable*
|
||||
today because of R2 — with a real bucket assertion, a fix for R3 would be
|
||||
test-covered.
|
||||
|
||||
### R4 — MINOR — contract §6 test 2 shipped 2 of 4 writer families; P2/P4/P5/P6/P8 are unstated
|
||||
|
||||
**File:** `tests/AcDream.Runtime.Tests/Entities/RuntimeEntityChildCellPropagationTests.cs`.
|
||||
|
||||
Shipped: (a) `CommitRebucket` and (d) the wire merge (`RefreshSnapshot`).
|
||||
**Missing:** (b) `RuntimePhysicsState.CommitCanonicalCell` (the simulation commit
|
||||
— the writer that actually fires per tick in live play) and (c) a
|
||||
`RuntimeSetPositionState` canonical placement commit. I verified by reading that
|
||||
both reach `Entities.SetFullCell` (`RuntimePhysicsState.cs:2148`,
|
||||
`RuntimeSetPositionState.cs:2743/:3660/:5003/:5224`), so the hook does fire — the
|
||||
gap is regression coverage, not present-tense correctness. P5 (per-writer-family
|
||||
re-entrancy) has no test at all; P6 (0 B/crossing) has none; P2 asserts only
|
||||
`ObjectClock.IsActive` on one path, not "no workset / not a spatial root / body
|
||||
inactive after N crossings and a teleport"; P4 (the child broadphase story) and
|
||||
P8 (the `Rebucketed` consumer enumeration) are stated nowhere in the tree.
|
||||
|
||||
For P8 I did the enumeration myself: `RuntimeEntityChange.Rebucketed` has **no**
|
||||
production consumer that branches on it (only `GameRuntimeEvents.cs:43`'s enum
|
||||
member and two test assertions). D2's silent-publication default is safe. That
|
||||
should be written into the commit, per §5, rather than left for a reviewer.
|
||||
|
||||
### R5 — MINOR — D7 is half-applied; the executor's replayed pickup keeps the inverted order
|
||||
|
||||
**File:** `src/AcDream.Runtime/Entities/RuntimeInitialCreateContinuationExecutor.cs:2183-2189`.
|
||||
|
||||
```csharp
|
||||
_entities.AdvancePositionAuthority(canonical);
|
||||
_physics.CollisionReports.LeaveWorld(canonical); // leave_world
|
||||
... SetPosition.Forget / SuspendObjectClock / SetFullCell(0,0)
|
||||
_entities.ParentAttachments.EndChildProjection(canonical.ServerGuid); // unset_parent LAST
|
||||
```
|
||||
|
||||
This is the dormant-residence replay of the **same** wire event
|
||||
(`SmartBox::DoPickupEvent` @0x00452240), whose retail order is `unset_parent`
|
||||
@0x0045227f **before** `leave_world` @0x00452286. D7 corrected
|
||||
`RuntimeEntityObjectLifetime.TryApplyPickup` and left this one. Inert for the
|
||||
same reason (§E), but the new comment at
|
||||
`RuntimeEntityObjectLifetime.cs:1304-1306` — "This was inverted;
|
||||
EndChildProjection now runs first" — now reads as a codebase-wide statement that
|
||||
is only half true. (The `TryApplyPosition` unparent edge at `:1908` **is**
|
||||
correctly scoped out as route 4's surface in contract §9; this one is not.)
|
||||
|
||||
### R6 — MINOR — D5's deferred-residence flavor is neither driven nor declared a non-goal
|
||||
|
||||
**File:** `src/AcDream.Runtime/Session/RuntimeLiveEntitySessionController.cs:339-379`.
|
||||
|
||||
`ResolveAndCommitChildAttachment` runs `Resolve` → `TryCommitParent` →
|
||||
`CommitProjection` → `CommitAcceptedParentCellless` synchronously. When the
|
||||
child has a pending initial residence, `Resolve`'s accept callback lands in
|
||||
`RuntimeEntityObjectLifetime.TryApplyParent`'s dormant branch
|
||||
(`:1354-1391`), which advances the gate via `TryAcceptDeferredParent` and
|
||||
`EnqueueDormant`s a `Parent` continuation, returning `true` — so the relation is
|
||||
**staged**, and the immediately following `TryCommitParent` is gated on
|
||||
`gate.PositionTimestamp == positionSequence && child.PositionSequence ==
|
||||
positionSequence` (`InboundPhysicsStateController.cs:308-314`), which the
|
||||
deferred merge has not yet satisfied. The drive returns `false`.
|
||||
|
||||
Nothing re-drives it: the executor's `CommitParentAttachment` deliberately does
|
||||
**not** commit the attach ("it is the App-layer `EquippedChildRenderController`'s
|
||||
job" — `RuntimeInitialCreateContinuationExecutor.cs:2110-2123`), and headless has
|
||||
no such controller and no post-drain retry hook. So headless, a ParentEvent
|
||||
arriving during a pending initial residence leaves the child staged and
|
||||
cell-less indefinitely.
|
||||
|
||||
Not a regression (headless committed nothing before), and the invariant-9
|
||||
deferrals are correctly untouched. But the plan doc's new closure text and D5's
|
||||
doc comment both read as if the headless gap is closed. Contract D5 required
|
||||
"state what is deliberately not driven"; this case is not stated. **Flagging the
|
||||
`TryCommitParent` rejection as inferred-from-reading, not executed** — no test
|
||||
covers it.
|
||||
|
||||
### R7 — MINOR — AP-143 under-describes what the headless drive skips
|
||||
|
||||
**File:** `docs/architecture/retail-divergence-register.md:173`;
|
||||
`src/AcDream.Runtime/Session/RuntimeLiveEntitySessionController.cs:339-379`.
|
||||
|
||||
AP-143 records only the `Setup.HoldingLocations` check. The graphical
|
||||
`ValidateParentProjection` (`EquippedChildRenderController.cs:894-919`) also
|
||||
rejects **self-parenting** (`:897-898`) and requires the parent to have a
|
||||
**part array** (`:902` — retail's @0x00510ed8 guard, and the closest thing
|
||||
acdream has to it). The headless drive performs neither. Self-parenting turns
|
||||
out to be inert by construction (the D1 gate sees `parent.FullCellId == 0`
|
||||
because the child was just zeroed, and D2's skip-on-equal terminates the
|
||||
one-node cycle), but the row should say what it skips, not one of three things.
|
||||
Contract §6 gave zero coverage of the headless rejection paths.
|
||||
|
||||
### R8 — MINOR — the headless drive is not the "same protocol" for nested/recovery cases
|
||||
|
||||
**File:** `src/AcDream.Runtime/Session/RuntimeLiveEntitySessionController.cs:381-393`.
|
||||
|
||||
- The graphical retry is `_relationRecoveryOrder.RealizeDescendants(parentGuid,
|
||||
Relations.ChildrenWaitingForParent, ResolveAndTryRealize)`
|
||||
(`EquippedChildRenderController.cs:925-931`) — a **transitive, parent-first**
|
||||
descent. `RetryChildrenWaitingForParent` iterates the direct waiters on one
|
||||
guid only, so after committing a child, grandchildren waiting on **that** child
|
||||
are not retried in the same pass. The doc comment claims it "mirrors
|
||||
`RetryWaitingDescendants`'s role"; it mirrors a subset.
|
||||
- The drive never calls `MarkProjected`, so `_recoveryByChild` retains every
|
||||
committed relation headless, and it never handles the recovery branch the
|
||||
graphical `ResolveAndTryRealize` handles (`:835-844`). Benign today (recovery
|
||||
is a re-projection concept that does not exist headless, and the entries are
|
||||
cleared by `EndChildProjection`/`EndGeneration`/`Clear`), but undeclared.
|
||||
|
||||
Given AP-142 explicitly ships unbounded-depth recursion as a design commitment,
|
||||
a depth-limited headless realize is worth a sentence somewhere.
|
||||
|
||||
### R9 — MINOR — a doc comment now describes the wrong test method
|
||||
|
||||
**File:** `tests/AcDream.Runtime.Tests/Session/RuntimeLiveEntitySessionControllerTests.cs:316-331`
|
||||
and `:426-431`.
|
||||
|
||||
The pre-existing `<summary>` for
|
||||
`FirstEntryDriveServesOneRouteAtATimeAndScopesClearToTheOwner` (the C3c-R1 F6
|
||||
latch) was left in place and the two new tests were inserted **after** it, so:
|
||||
|
||||
- `DirectSink_D5_StandaloneParentEventCommitsChildToParentsExactCell` now carries
|
||||
**two** `<summary>` elements, the first of which describes drive-controller
|
||||
route latching.
|
||||
- `FirstEntryDriveServesOneRouteAtATimeAndScopesClearToTheOwner` now carries the
|
||||
D5 *deferred-parent* summary.
|
||||
|
||||
Exactly the endemic stale-comment class this campaign keeps hitting, in the
|
||||
commit that was supposed to be checking for it.
|
||||
|
||||
### R10 — MINOR — AP-142 clause (b) overstates the equivalence
|
||||
|
||||
**File:** `docs/architecture/retail-divergence-register.md:172`;
|
||||
`src/AcDream.Runtime/Entities/RuntimeEntityDirectory.cs:391-395`.
|
||||
|
||||
The skip guard (`child.FullCellId == fullCellId → continue`) does two things:
|
||||
it subsumes retail's same-cell id refresh (accurate, as the row says) **and** it
|
||||
prunes the child's entire subtree. Retail's `enter_cell` has no such pruning —
|
||||
it recurses unconditionally (@0x00510f03) and only `leave_cell` prunes
|
||||
(@0x00510f5b, on `cell != 0`). So acdream matches retail exactly on the removal
|
||||
side and diverges on the entry side. Currently unreachable-by-construction (after
|
||||
D4 nothing writes a grandchild's cell independently of its parent), but the row
|
||||
claims a clean equivalence it does not have, and this is precisely the class of
|
||||
"row asserting behaviour the code lacks" in the other direction.
|
||||
|
||||
### R11 — MINOR — the demotion drops `SynchronizePhysicsBodyActiveState` on a child's first projection
|
||||
|
||||
**File:** `src/AcDream.App/World/LiveEntityRuntime.cs:923-927` (legacy branch,
|
||||
no longer reached for children) vs `:995-1067` (presentation-only branch).
|
||||
|
||||
The legacy `RebucketLiveEntity` ran
|
||||
`record.SuspendObjectClock(); SynchronizePhysicsBodyActiveState(record);` when
|
||||
`!wasProjected && !isOrdinaryRoot` — true on a child's first tick after realize.
|
||||
`RebucketLiveEntityPresentationOnly` does neither. The canonical clock is
|
||||
suspended by Runtime (`CommitAcceptedParentCellless`'s
|
||||
`Entities.SuspendObjectClock`), and `SynchronizePhysicsBodyActiveState` no-ops
|
||||
when `record.PhysicsBody is null` (the equipped-item case), so this is very
|
||||
likely inert — but P2's "body (if any) inactive" is the obligation that would
|
||||
have proven it, and it is unproven. **Flagged as unverified rather than
|
||||
asserted.**
|
||||
|
||||
---
|
||||
|
||||
## Register discipline — assessed row by row
|
||||
|
||||
| Row | Assessment |
|
||||
|---|---|
|
||||
| **AP-142** (parented-child single-field cell model) | Clauses (a) and (c) describe shipped behaviour accurately (verified: `PropagateFullCellToChildren` + `WithdrawCommittedChildrenToCellless` + the D1 half). Clause (b) overstates — see R10. **Missing the `part_array` guard clause — see R1.** Anchors all correct. |
|
||||
| **AP-143** (headless holding-location skip) | Accurately describes the skip and the reason. Under-describes the scope — see R7. The claim "repo-wide grep confirms nothing under `src/AcDream.Content`/`AcDream.Bake` carries `HoldingLocations`" is consistent with what I saw; the row correctly refuses to extend the bake format. |
|
||||
| **AP-136** writer-list shrink | ✓ Correct: line 289 now reads "…demoted the third, `EquippedChildRenderController.TickChild`, to a presentation-only bucket move that no longer touches `record.FullCellId`". Matches the code. |
|
||||
| **AP-138** writer-list shrink | ✓ Correct, same shape. |
|
||||
| `RuntimeSetPositionState.cs:4536-4548` doc comment | ✓ Corrected; singular "writer" now, projection materializer only. Matches the code. |
|
||||
| `RuntimeRemotePlacementDriveController.cs:1613-1626` doc comment | ✓ Corrected. Matches the code. |
|
||||
| 4b-3 contract closure note | ✓ Honest and correct. It states plainly that the old `cause=cellless` recipe no longer fires, explains why that is the retail-faithful direction, marks test 4 as a synthetic fixture that must not be relabelled, and says the replacement provocation is UNESTABLISHED rather than inventing one. This is the best-written doc change in the diff. |
|
||||
| Placement-cutover plan gap statement | ✓ Correctly moved to past tense and closed, with the one caveat that "the direct headless regression test … now passes" is true for the non-deferred flavor only (R6). |
|
||||
| No row deletions | ✓ AP-124/131/132/133/135 untouched. |
|
||||
|
||||
---
|
||||
|
||||
## Contract defects found
|
||||
|
||||
1. **§0 item 1 dropped `enter_cell`'s `part_array != 0` guard** (@0x00510ed8),
|
||||
which the research it cites called load-bearing. The contract enumerated the
|
||||
five writes and not the gate around them; the implementation inherited the
|
||||
omission. This is the root of R1 — process rule 1 ("the contract causes the
|
||||
defect") in action.
|
||||
2. **§2's `DoPickupEvent`/`DoParentEvent` gate citations are not readable from
|
||||
the named-retail source** (BN lowered both to a literal always-false
|
||||
expression). The contract presents them as verified. Nothing downstream
|
||||
depends on it, but "✓" is the wrong mark.
|
||||
3. **§4 D4 said "assert"; §6 test 10 said "assert the child's render entity
|
||||
moved buckets".** The implementation did neither, and the contract's own
|
||||
sabotage instruction ("break D2's propagation and confirm THIS test fails on
|
||||
the canonical half **while presentation still moves** — if it stays green,
|
||||
the test is asserting the wrong layer") was applied to the canonical half
|
||||
only. The contract was right; it was not followed. R2/R3.
|
||||
4. **§5's proof obligations are not stated anywhere in the working tree.** P4 and
|
||||
P8 in particular were explicitly "must prove, not assume". P8's answer is
|
||||
benign (I checked), P4's is unwritten.
|
||||
|
||||
## Propagation research defects found
|
||||
|
||||
Two, both cosmetic, listed in §A: the "verbatim" blocks are normalized
|
||||
paraphrase, and §7's read/inferred ledger is otherwise scrupulous and correct.
|
||||
**The verdict, the addresses, the recursion claim, the same-cell-fast-path
|
||||
disambiguation, the `update_object` clincher, and the cross-cells trap are all
|
||||
correct.** The research is the strongest artifact in this slice; the contract
|
||||
weakened one of its findings on the way through (R1).
|
||||
|
||||
---
|
||||
|
||||
## What would flip this to PASS
|
||||
|
||||
- R1: a `part_array` register clause (or the guard, if a canonical analogue
|
||||
exists).
|
||||
- R2: an App test that asserts the spatial bucket / visibility state moved, with
|
||||
the demoted call sabotage-verified.
|
||||
- R3: assert-or-propagate on `RebucketEquippedChildPresentation`'s two guards.
|
||||
|
||||
R4–R11 are worth folding in but none of them alone blocks the slice.
|
||||
Loading…
Add table
Add a link
Reference in a new issue