392c1e22c1
A player-parented child never received a canonical cell. Its FullCellId stayed 0 for its whole attached lifetime, so it could not follow the player across a boundary. Scope was wider than the local player: every REMOTE player's equipment too. ROOT CAUSE. EquippedChildRenderController hardcoded ParentInstanceSequence: 0 for a parented CreateObject. Correct for creatures and statics, which really are sequence 0; wrong for players, whose ObjectInstance is Character.TotalLogins (ACE Player_Networking.cs:37). The relation filed under (playerGuid, 0) while the record carried TotalLogins, so both route-7 write sites — D1's attach re-cell and D2's propagation lookup — keyed on an incarnation that never matched. TryCommitParent did not validate the sequence, so the attach succeeded and printed normally. Silent. A ROUTE 7 REGRESSION (cd3129e9) that un-masked a latent bug: the TickChild call route 7 deleted was keyed on the child guid alone and was structurally immune to a wrong parent key. THE FIX IS TO STOP TREATING PLAYERS DIFFERENTLY, not to special-case them. Retail's attach path is guid-only end to end — PhysicsDesc::get_parent_id @0x00558a18 -> CObjectMaint::GetObjectA @0x00558a2d -> set_parent @0x00558a3e, with SetChildren @0x00509370 hash-walking by guid — and neither set_parent overload (@0x00515A90, @0x00515B50) nor enter_cell @0x00510ED0 contains any player test or instance-sequence read. Our player/non-player split was purely an artifact of keying relations by (guid, incarnation) against a wire message that carries no parent incarnation. Late-binding to whoever currently holds the guid is retail's own semantics. Fixed at BOTH producers: OnSpawn and OnCreateParentAccepted, the second carrying the byte-identical defect and not named in the contract's scope line. THE INVARIANT IS EQUALITY, NOT FRESHNESS. The contract rejected both framings I offered: every one of the 45 FullCellId liveness predicates excludes a committed child on a NON-cell clause first, so the child inherits only the parent record's existing staleness, which is already present today with no symptom. The key fix alone restores child-equals-parent for every parent class. TWO SITES GATED, inert only because the cell was zero and would have woken wrongly: the hydration candidate loop (a nonzero-cell child would take the legacy RebucketLiveEntity -> CommitRebucket, a second canonical writer — route 7's exact defect class) and RestoreShadow (would install a broadphase row for the weapon, the #184 shape, contradicting route 7's P4). Retail anchor: update_object's parent != 0 early-out @0x00515D40 — children are never independently re-placed. THREE MAJORS WERE FIXED BY DELETION. The first pass added a deferral queue for an unaddressable parent, carrying a missing child-freshness gate (A2), a sentinel-0 collision with the generation filters (A3), and unbounded accumulation (A5). Both reviewers then proved the deferred branch unreachable for BOTH producers — RegisterEntityCore defers the entire CreateObject one layer above, reading the same ?? chain, and CreateParentUpdate is produced only inside AcceptCreateCore, after that gate passes. The machinery was deleted rather than repaired, and the diff SHRANK to 76 added / 13 removed from 91/24 while gaining the A1 fix. Retail confirmed the deletion does not diverge: acdream's real port of retail's per-guid replay (QueueBlobForObject) is a different, untouched layer, and the deleted queue was a third redundant one downstream of it. THE GUARD MUST NOT TEAR WHAT IT PROTECTS. The first pass threw InvalidOperationException AFTER the canonical half had committed, so the one time it fired it left the child parented with no committed relation and a staged one blocking Resolve — a torn transaction, the exact outcome the contract pinned against. Now a pure CanCommitIncarnation precondition checked BEFORE the commit at both sites, with a logged refusal instead of a throw. Route 3's N3 principle (do not make a transient fatal on a host that must survive 30 sessions x 2 hours) reinforces it, but the tearing argument stands alone. TEST QUALITY, the recurring lesson in its most refined form. The A1 test initially passed sabotage FOR THE WRONG REASON: a mismatched ChildPositionSequence meant TryCommitParent's own gate refused in either ordering, so the three assertions carrying A1's meaning passed both ways and only an incidental staging assertion failed. It failed on stranding, not tearing. Corrected, the sabotage now names line 925 — Assert.Null(snapshot.ParentGuid), with the parent's guid in it — proving the canonical mutation happened before the catch. "Fails under sabotage" is necessary, not sufficient; WHICH assertion fails is the real question. The dual parent-class matrix (player 0x5… incarnation > 1 vs creature 0x8… incarnation 0, identical outcomes, sabotage-verified in both directions) is the structural fix for how this survived a full dual review and two connected sessions: every prior test and both captured gate logs used sequence-0 parents. Register: AP-142 clause (f); AP-132 amended to distinguish the two producers; new row AP-146 for the local player's coarse canonical cell (retail writes it per tick at SetPositionInternal @0x00515330 — which, per the retail review, ALSO walks this->children writing each child's objcell_id @0x005153AE-@0x005153D8, so retail's per-tick child propagation lives in the same function). That divergence had no row at all, a standing rule-1 violation now corrected. Follow-up #320 filed for making the player's cell track ordinary movement — deliberately excluded here: it touches the landblock-preserve contract, the Rebucketed cadence, route-2/4b-3 classification inputs AP-136/AP-138 spent four review rounds pinning, and the portal-space frozen-source-cell race. Two dual review rounds; 6 architecture MAJORs and 2 retail MAJORs closed. Diagnostic refusals are latched per child guid and the latch clears on Clear()/RemoveChild, so a recycled guid's next incarnation still logs rather than being silently suppressed. Complete Release suite MEASURED at 11,112 passed / 4 skipped / 0 failed (baseline 11,090 at52175aa1, +22). Neither known flake fired. STILL OWED: the connected gate, with the CORRECTED positive criterion — assert the equipped child's FullCellId EQUALS the parent's after a crossing (a zero is a failure, not a silence), run with BOTH a player and a creature parent, plus the new step carrying an armed creature across a landblock unload/reload. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
380 lines
21 KiB
Markdown
380 lines
21 KiB
Markdown
# Issue #319 — independent retail-conformance review (2026-08-05)
|
||
|
||
**Verdict: PASS.**
|
||
|
||
Reviewed: the uncommitted working tree at base HEAD `af828a8a`, branch
|
||
`claude/acdream-physics-divergence-5aa784` — `git diff HEAD` (13 files,
|
||
+709/-30) plus the untracked contract
|
||
`docs/research/2026-08-05-issue-319-contract.md`.
|
||
|
||
Review only. No file in the tree was modified by this review except this
|
||
report.
|
||
|
||
Independent gates run for this review (a green suite is not evidence, but a
|
||
red one is):
|
||
|
||
- `dotnet build AcDream.slnx -c Release` — exit 0.
|
||
- `AcDream.Runtime.Tests` Release — 1170/1170, 0 skipped.
|
||
- `AcDream.App.Tests` Release — 4125 passed, 3 skipped, 0 failed.
|
||
|
||
---
|
||
|
||
## 1. The central retail claim — independently verified, and the contract
|
||
## UNDERSTATES its own anchor
|
||
|
||
Every retail claim below was read at
|
||
`docs/research/named-retail/acclient_2013_pseudo_c.txt` for this review. None
|
||
was inherited from the contract or the investigation.
|
||
|
||
| contract claim | address | verified |
|
||
|---|---|---|
|
||
| CreateObject's attach is GUID-only | `ACCObjectMaint::CreateObject` @0x00558870; `PhysicsDesc::get_parent_id` @0x00558a18 → `if (eax_20 != 0)` @0x00558a1f → `CObjectMaint::GetObjectA(this, eax_20)` @0x00558a2d → `get_parent_location_id` @0x00558a31 → `CPhysicsObj::set_parent(result, arg2, eax_22)` @0x00558a3e | ✓ **exact**. `GetObjectA` takes `(this, guid)`. There is no instance-sequence read, comparison, or argument anywhere in the block. |
|
||
| The reverse direction is the same shape | `CObjectMaint::SetChildren` @0x00509370: `unparent_children` @0x00509379, hash-bucket walk on `PhysicsDesc::get_child_id` @0x005093b2–@0x005093ca, `GetNullObject(this, get_child_id(...), 1)` placeholder @0x005093e6, `set_parent(hash_next_1, arg2, get_child_location_id(...))` @0x005093f8 | ✓ **exact**, including the `GetNullObject` placeholder for a not-yet-constructed child. No instance field participates. |
|
||
| No player branch in the relation path | `set_parent` @0x00515A90 (3-arg) and @0x00515B50 (4-arg, Frame); `enter_cell` @0x00510ED0; `change_cell` @0x00513390 | ✓. Both `set_parent` overloads operate on `CPhysicsObj*` uniformly: `add_child` → `unset_parent` → `leave_world` → `parent = edi` → `if (cell != 0) change_cell` → `UpdateChild` → `recalc_cross_cells`. The only conditional in either body is the parent's `state & 0x4000` no-draw cascade @0x00515B26. `enter_cell`'s only guard is `part_array != 0` @0x00510ED8. **No player test exists in any of them.** |
|
||
| Retail's player cell is written per tick | `CPhysicsObj::SetPositionInternal` @0x00515330 | ✓ — and **stronger than the contract states**, see below. |
|
||
| `update_object` early-outs on a parented object | @0x00515D40: `if ((this_3->parent != 0 \|\| (this_3->cell == 0 \|\| (this_3->state & 0x1000000) != 0))) { transient_state &= ~0x80; return; }` | ✓ **exact**. A parented child is never independently updated or re-placed. |
|
||
|
||
**The contract's §1 row 5 cites the weaker half of its own best anchor.**
|
||
`SetPositionInternal` @0x00515330 does not merely write the mover's own cell:
|
||
in the same-cell branch it writes `this->m_position.objcell_id = objcell_id`
|
||
@0x00515385, then **walks `this->children` and writes each child's cell id
|
||
directly** — `*(uint32_t*)((char*)eax_2 + 0x4c) = objcell_id_1` @0x005153BD,
|
||
followed by `CPartArray::SetCellID` @0x005153CC, looping @0x005153AE–
|
||
@0x005153D8; the cross-cell branch delegates to `change_cell` @0x00515372.
|
||
That is retail's D2 equivalent living **inside the same per-tick function** as
|
||
the player's own cell write. It is a strengthening, not a contradiction: the
|
||
child-equals-parent equality invariant and AP-146's "retail's player cell is
|
||
never stale" both get a single anchor. (AP-142 clause (b) already cites this
|
||
range; only §1's summary table understates it.)
|
||
|
||
### Conclusion on the central claim
|
||
|
||
**Retail does NOT distinguish a player parent, and there is no
|
||
instance-sequence check anywhere in the relation path — confirmed
|
||
independently.** acdream's player/non-player split is exactly what the
|
||
contract says it is: an artifact of keying committed relations by
|
||
`(guid, incarnation)` against a wire message that carries only a GUID.
|
||
**Late-binding to whoever currently holds the GUID is the faithful mapping**,
|
||
because "the current holder of the GUID" is literally what `GetObjectA` /
|
||
`GetNullObject` return. The fix shape is right.
|
||
|
||
---
|
||
|
||
## 2. Scope expansion (`OnCreateParentAccepted`) — JUSTIFIED, and the fix is
|
||
## correct there
|
||
|
||
The contract's Scope line named only `EquippedChildRenderController.OnSpawn`.
|
||
The implementer also fixed `OnCreateParentAccepted`. Verified:
|
||
|
||
- `CreateParentUpdate` is declared at
|
||
`src/AcDream.Runtime/Entities/InboundPhysicsStateController.cs:1399-1405`
|
||
as `(ChildGuid, ParentGuid, ParentLocation, PlacementId,
|
||
ChildInstanceSequence, ChildPositionSequence)`. Its producer,
|
||
`BuildSameGenerationEvents` (`:1240-1248`), fills
|
||
`ChildInstanceSequence` from `ts.Instance` where `ts` is
|
||
`incoming.Physics.Timestamps` and `incoming` **is the child**. So
|
||
`ChildInstanceSequence` is the CHILD's own timestamp, exactly as the
|
||
implementer reports. **No parent instance sequence exists on this shape.**
|
||
- The codebase already knew this: `InboundPhysicsStateController.
|
||
TryApplyCreateParent`'s pre-existing doc comment (`:216-218`) reads
|
||
"Unlike standalone ParentEvent it carries no parent INSTANCE_TS, so only
|
||
the child's shared POSITION_TS participates in freshness."
|
||
- Pre-fix, `OnCreateParentAccepted` hardcoded `ParentInstanceSequence: 0`
|
||
into the identical `Relations.AcceptCreateObjectRelation` call. **Byte-
|
||
identical defect.** Shipping the key fix on one producer and not the other
|
||
would have left #319 half-open for every same-generation ObjDesc refresh of
|
||
a player-parented child.
|
||
|
||
Routing both through one `AcceptLateBoundCreateObjectRelation`
|
||
(`EquippedChildRenderController.cs:836-857`) is the right shape and the right
|
||
commit.
|
||
|
||
---
|
||
|
||
## 3. Findings
|
||
|
||
### R1 — MINOR (incorrect load-bearing comment). Both deferred branches are production-unreachable, not just `OnSpawn`'s.
|
||
|
||
`tests/AcDream.App.Tests/Rendering/EquippedChildProjectionWithdrawalTests.cs:875-881`
|
||
|
||
The test's XML doc asserts:
|
||
|
||
> `CreateParentUpdate`'s producer … has no equivalent parent-addressability
|
||
> precondition, so this is the reachable shape for #319's deferred branch.
|
||
|
||
**Verified false.** The only production driver of `OnCreateParentAccepted` is
|
||
`LiveEntityHydrationController.OnCreate`'s `result.SameGenerationEvents is
|
||
{ } refresh` branch (`LiveEntityHydrationController.cs:340-341` →
|
||
`LiveEntityNetworkUpdateController.cs:380` →
|
||
`LiveEntitySameGenerationUpdateRouter.Apply` → `OnParent` → `:400-401`).
|
||
`SameGenerationCreateObjectEvents` are produced only inside
|
||
`InboundPhysicsStateController.AcceptCreate` (`:70-90`), reached only from
|
||
`RuntimeEntityObjectLifetime.RegisterEntityCore` — and `RegisterEntityCore`'s
|
||
deferral gate runs **before** `PreviewCreateDisposition`:
|
||
|
||
```
|
||
RuntimeEntityObjectLifetime.cs:798-812
|
||
uint parentGuid = incoming.ParentGuid ?? incoming.Physics?.Parent?.Guid ?? 0u;
|
||
if (beginInitialResidence && parentGuid != 0u
|
||
&& !Entities.TryGetActive(parentGuid, out _))
|
||
{ Entities.ParentAttachments.EnqueueDeferredCreate(...); return DeferredForParent: true; }
|
||
```
|
||
|
||
The `??` chain covers both the flattened `ParentGuid` and the nested
|
||
`Physics.Parent.Guid`, i.e. both shapes `OnSpawn` and `OnCreateParentAccepted`
|
||
read. The App graphical host is the `beginInitialResidence: true` caller
|
||
(`LiveEntityRuntime.cs:583`). So by the time either producer runs, the parent
|
||
is active. The implementer's `OnSpawn` finding is correct; the extension of it
|
||
to `OnCreateParentAccepted` is not.
|
||
|
||
Retail address contradicted: none — this is a reachability claim about
|
||
acdream, not about retail.
|
||
|
||
**Not a code defect.** The deferred machinery is correct, and it is genuinely
|
||
defensive: the deferral gate reads `Entities.TryGetActive` (the
|
||
`_activeByGuid` map) while the late-bind reads `TryGetSnapshot` (the
|
||
`_inbound._snapshots` map) — two distinct stores, so a producer-side fallback
|
||
is warranted rather than an assertion. Correct behaviour: the comment should
|
||
say the deferred branch is defensive against store drift and is not exercised
|
||
by any production path today, and the test should be labelled as covering the
|
||
mechanism rather than a reachable production shape.
|
||
|
||
### R2 — MINOR (latent bug, unreachable today by R1). The late-bind sentinel `0` is filtered as if it were a wire-named incarnation.
|
||
|
||
`src/AcDream.Runtime/Entities/ParentAttachmentState.cs:828-834` (`EndGeneration`)
|
||
and `:853-857` (`DeleteGeneration`), via `FilterParentCandidates` (`:1002-1018`).
|
||
|
||
`DeferCreateObjectRelation` (`:412-426`) queues the relation into
|
||
`_unresolvedByChild` carrying `ParentInstanceSequence: 0` plus
|
||
`LateBindParentInstance = true`. The `0` is a **meaningless placeholder** —
|
||
that is the whole premise of the fix. But `EndGeneration`'s retain predicate
|
||
treats it as a wire-named value:
|
||
|
||
```
|
||
relation => relation.ParentInstanceSequence == replacementGeneration
|
||
|| PhysicsTimestampGate.IsNewer(replacementGeneration,
|
||
relation.ParentInstanceSequence)
|
||
```
|
||
|
||
`IsNewer(oldStamp, newStamp)` (`AcDream.Core/Physics/PhysicsTimestampGate.cs:58-63`)
|
||
returns true when `newStamp` is newer. For a player parent replaced at
|
||
generation 7: `0 == 7` false, `IsNewer(7, 0)` false → **the late-bind relation
|
||
is dropped**, which is the exact opposite of its own semantic ("attach to
|
||
whoever currently holds the GUID" — the replacement generation is precisely
|
||
whom it should attach to) and of retail's GUID-keyed blob replay
|
||
(`QueueBlobForObject`, AP-132's retail half: replay on GUID (re)creation with
|
||
only an addressability check).
|
||
|
||
Not reachable in production today because of R1, and **not a regression** —
|
||
pre-fix a CreateObject relation went straight to `_stagedByChild`, which
|
||
`EndGeneration` clears unconditionally via `RemoveParentReferences`
|
||
(`:822-827`). Correct behaviour when the path becomes reachable: both
|
||
`FilterParentCandidates` retain predicates should keep
|
||
`relation.LateBindParentInstance` unconditionally, or the deferred relation
|
||
should carry a nullable incarnation rather than a `0` sentinel that collides
|
||
with a legitimate creature-parent value.
|
||
|
||
### R3 — MINOR (incorrect comment, two sites; the same wrong rationale twice).
|
||
|
||
`tests/AcDream.App.Tests/World/LiveEntityHydrationControllerTests.cs:149-153`
|
||
and `tests/AcDream.App.Tests/World/LiveEntityPresentationControllerTests.cs:101-105`.
|
||
|
||
Both comments justify the fixture setup with:
|
||
|
||
> …giving it a nonzero canonical `FullCellId` via D1/D2 (exercised
|
||
> elsewhere) — impossible to reach before #319's fix for a player-class
|
||
> parent.
|
||
|
||
Two errors:
|
||
|
||
1. **"the relation never committed for a player-class parent at all" is
|
||
false.** `HasCommittedParent` is keyed by the CHILD guid
|
||
(`ParentAttachmentState.cs:571-572`: `_lastAcceptedByChild.ContainsKey(childGuid)`),
|
||
and pre-fix `CommitProjection` succeeded under `(playerGuid, 0)` — the
|
||
contract's own §0 says so verbatim ("`TryCommitParent` never validates the
|
||
sequence; the attach succeeds silently"). A player-parented child **did**
|
||
have a committed parent at HEAD. What was inert at HEAD is the child's
|
||
zero `FullCellId`, which is the contract's actual §3.1/§3.2 argument.
|
||
2. **The nonzero cell in both fixtures does not come from D1/D2.** The
|
||
presentation fixture materialises at `0x01010001u`
|
||
(`LiveEntityPresentationControllerTests.cs:690`, seeded from the spawn's
|
||
own `ServerPosition` at `:744`); the hydration fixture's record carries
|
||
`Cell` from its own spawn. Neither test calls
|
||
`CommitAcceptedParentCellless`.
|
||
|
||
The tests themselves are **valid and their sabotage claims hold** — I
|
||
verified independently that `ProjectionCellId => WorldEntity is not null ?
|
||
FullCellId : …` (`LiveEntityRuntime.cs:387-389`) makes a committed child's
|
||
candidate cell exactly `FullCellId`, so it is 0 at HEAD and the parent's cell
|
||
post-fix; the gates are therefore behavior-preserving at HEAD, exactly as the
|
||
contract argued. Only the stated rationale is wrong. Correct behaviour: cite
|
||
the zero-`FullCellId` inertness, not a non-existent absence of a committed
|
||
relation.
|
||
|
||
### R4 — MINOR (comment made partially false by this diff; the comment itself is untouched).
|
||
|
||
`src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs:1529-1531` (D1):
|
||
|
||
> The committed relation (not the guid alone) resolves the parent so a stale
|
||
> or superseded incarnation can never re-cell the child.
|
||
|
||
After F1 a CreateObject-carried relation is, by construction, always equal to
|
||
the parent's live incarnation at commit (and the new tripwire at
|
||
`ParentAttachmentState.cs:651-661` enforces it). For that producer the
|
||
sentence now describes a protection that is vacuous rather than active. It
|
||
remains true for the `ParentEvent` producer, whose incarnation the wire names
|
||
(AP-132). The diff changed the semantics of a comment it did not edit — the
|
||
exact class the review brief flags. Correct behaviour: qualify the sentence to
|
||
name the ParentEvent producer.
|
||
|
||
### R5 — LOW (comment misdirection).
|
||
|
||
`src/AcDream.App/World/LiveEntityPresentationController.cs:228` — "the same
|
||
predicate the hydration gate above uses". The hydration gate is in a different
|
||
file (`LiveEntityHydrationController.OnLandblockLoaded:561`), not "above".
|
||
|
||
### R6 — OBSERVATION, partly UNVERIFIABLE (flagged rather than guessed). The tripwire converts a silent skip into a production throw.
|
||
|
||
`ParentAttachmentState.CommitProjection` (`:639-661`) now throws
|
||
`InvalidOperationException` when the parent is addressable and the relation's
|
||
incarnation disagrees. The contract explicitly authorised this ("throw or
|
||
logged refusal … pinned outcome: never a silent success"), so it is in scope.
|
||
I enumerated the false-positive surface as far as source allows:
|
||
|
||
- Only the **Staged** branch of `PrepareAndTryRealize`
|
||
(`EquippedChildRenderController.cs:954-960`) reaches it in the graphical
|
||
host; the Recovery branch does not.
|
||
- That branch is gated by `ValidateParentProjection` (`:986-996`), which
|
||
requires a live `LiveEntityRecord` for the parent.
|
||
- A staged relation is staged either at the parent's live incarnation
|
||
(`AcceptLateBoundCreateObjectRelation:846-853`), or after `Resolve`'s
|
||
equality check (`ParentAttachmentState.cs:484-498`), or by the late-bind
|
||
adoption (`:473-483`).
|
||
- A parent generation change removes every staged/recovery/committed
|
||
reference to that parent (`EndGeneration:822-827`,
|
||
`DeleteGeneration:847-852`).
|
||
- In the headless host (`RuntimeLiveEntitySessionController.cs:410`) relations
|
||
originate only from `Enqueue(ParentEvent.Parsed)` and pass `Resolve`'s
|
||
equality gate, so the tripwire is a no-op there.
|
||
|
||
**What I could NOT establish:** whether `_inbound._snapshots[parent].
|
||
InstanceSequence` (the tripwire's source, reached via `TryGetSnapshot`) can
|
||
ever lead `_activeByGuid[parent].Incarnation` inside a re-CreateObject
|
||
transaction — `AcceptCreate` writes `_snapshots[guid] = incoming`
|
||
(`InboundPhysicsStateController.cs:77`) before `RegisterEntityCore` completes
|
||
the record replacement, and the two stores are documented as "related but
|
||
distinct" (`:1198-1210`). I found no call into `CommitProjection` inside that
|
||
window, but I did not prove the absence exhaustively. Reporting as
|
||
unverified. If the connected gate ever produces this exception, the message
|
||
already names #319 and the two incarnations, which is the right diagnostic.
|
||
|
||
### R7 — OBSERVATION. The hydration gate is broader than its cited retail anchor.
|
||
|
||
`LiveEntityHydrationController.cs:551-562` excludes a committed child from the
|
||
**entire** candidate loop, so `ProjectExact(CreateSupersessionRecovery)`
|
||
(`:595`) and `ProjectExact(SpatialRecovery)` (`:614`) are also refused for it,
|
||
not only the legacy `RebucketLiveEntity → CommitRebucket` branch (`:607`).
|
||
|
||
This is what the contract pinned ("the candidate loop excludes records with a
|
||
committed parent", §3.1) and it is behavior-preserving at HEAD (verified: a
|
||
committed child's `projectionCellId` is `FullCellId` = 0 today). But the
|
||
retail anchor in the comment, `update_object`'s `parent != 0` early-out
|
||
@0x00515D40, speaks to **physics re-placement**, not to render-projection
|
||
recovery. The broader exclusion actually rests on route 7's P4 record (a
|
||
committed child is never a spatial root, joins no workset, has no shadow row)
|
||
plus the fact that a child's projection is driven exclusively through
|
||
`EquippedChildRenderController`'s realize/retry path. Worth one sentence in
|
||
AP-142 clause (f) or the contract rather than a code change — as written the
|
||
comment claims more anchoring than @0x00515D40 provides.
|
||
|
||
---
|
||
|
||
## 4. The three unwritten contract tests — NONE blocks
|
||
|
||
- **§6 test 5 (residence non-event).** Not blocking. I verified §3.3's premise
|
||
directly in source rather than relying on the test:
|
||
`RuntimeEntityObjectLifetime.InitializeAcceptedCreateResidence:2750-2752`
|
||
reads `if (canonical.FullCellId != 0u) Entities.SetFullCell(canonical, 0u,
|
||
0u);` immediately before `InitialCreateResidences.Begin`. `Begin`'s
|
||
`FullCellId != 0` refusal is therefore a defensive invariant, not a
|
||
reachable gate, exactly as §3.3 concluded. The zero-then-re-cell atomicity
|
||
claim remains argued-not-measured, but no production change was made at
|
||
that site.
|
||
- **§6 test 6 (unwield classification population).** Not blocking. The
|
||
contract itself recorded (from `af828a8a`) that ACE advances TELEPORT_TS at
|
||
a drop and the classifier is a short-circuit OR
|
||
(`RuntimeAuthoritativePositionRouteClassifier.cs:391`), so the observable
|
||
label is `teleport-ts` either way. The test would pin a flavour, not a
|
||
behaviour the fix changes.
|
||
- **§6 test 9 (ledger convergence).** Not blocking, and I checked why:
|
||
`RestoreShadow`'s refusal for a committed child is byte-identical pre- and
|
||
post-fix (the `FullCellId == 0` clause already refused every committed child
|
||
at HEAD). The one ledger asymmetry that exists —
|
||
`_suspendedShadowOwners.Add` on `BecameHidden`
|
||
(`LiveEntityPresentationController.cs:172`) with no matching removal on
|
||
`BecameVisible` because `RestoreShadow` returns false — is **pre-existing
|
||
and unchanged**, and converges through `Forget`/`Clear` (`:117`, `:125`).
|
||
|
||
The connected §7 gate (both halves, player class mandatory) remains the real
|
||
acceptance test and has not run. That is correctly recorded in ISSUES.md's
|
||
#319 status line, which honestly says "FIX IMPLEMENTED, awaiting the connected
|
||
acceptance gate … NOT YET COMMITTED".
|
||
|
||
---
|
||
|
||
## 5. Register and bookkeeping — verified row by row
|
||
|
||
- **AP-146 (new row, `retail-divergence-register.md:174`).** Retail anchor
|
||
`CPhysicsObj::SetPositionInternal` @0x00515330 **verified** — the address
|
||
resolves to the `(CPhysicsObj*, CTransition const*)` overload (the 4-arg
|
||
`SetPositionInternal` at @0x00515BD0 is a different function; the row cites
|
||
the right one), and the cell write is unconditional across both branches
|
||
(`m_position.objcell_id = …` @0x00515385 same-cell, `change_cell`
|
||
@0x00515372 cross-cell). The three acdream writers, the
|
||
`LocalPlayerProjectionController` landblock coarsening, and the
|
||
`LiveEntityRuntime.cs:935-938` preserve are all cited. The risk column names
|
||
the unresolved `ParkCollisionResidents` question **openly** rather than
|
||
smoothing it. Register rule 1 is satisfied: this was a standing
|
||
undocumented divergence and the diff files it. Header count 102 → 103,
|
||
correct.
|
||
- **AP-142 clause (f).** Every retail address in the clause
|
||
(@0x00558a18/@0x00558a2d/@0x00558a3e, @0x00509370/@0x005093e6) was
|
||
re-verified above and is exact. The clause states the acdream side honestly
|
||
(the `(guid, incarnation)` key is acdream's, retail has none) and names the
|
||
tripwire. Correct.
|
||
- **AP-132 amendment.** The added sentence distinguishes the two producers
|
||
correctly and does not weaken the row's own gate. Verified against the wire
|
||
types: `ParentEvent.Parsed` carries `ParentInstanceSequence`;
|
||
`CreateParentUpdate` carries `ChildInstanceSequence`. Correct.
|
||
- **Route-7 contract §7 supersession blockquote.** Accurate — the old
|
||
criterion is genuinely unfalsifiable under #319 (a zero-cell child emits no
|
||
probe line, which the old criterion read as clean) and it points at the
|
||
corrected positive-equality criterion. Correct.
|
||
- **#320.** A faithful transcription of contract §2.2's five-item enumeration
|
||
plus §9 item 2 as the mandatory first verification step, with the correct
|
||
"do not implement without a fresh retail-conformance argument" gate. It
|
||
inherits exactly what #319 deliberately excluded — nothing more, nothing
|
||
less.
|
||
|
||
---
|
||
|
||
## 6. Errors in the CONTRACT itself
|
||
|
||
Two, neither material to the verdict:
|
||
|
||
1. **§1 row 5 understates its own anchor** (see §1 of this review).
|
||
`SetPositionInternal` @0x00515330 also walks `this->children` writing each
|
||
child's `objcell_id` @0x005153AE–@0x005153D8 — retail's per-tick D2
|
||
equivalent in the same function. This *strengthens* the equality invariant
|
||
and AP-146; citing only the mover's own write leaves the strongest
|
||
available evidence on the table.
|
||
2. **§4 F4's "ISSUES.md: #319 → Recently closed with the commit SHA" was not
|
||
executed** — correctly, since the connected gate has not run and the tree
|
||
is uncommitted. The contract's instruction was written assuming the fix
|
||
commit and the gate land together. Deviation is honest and recorded in the
|
||
issue's own status line.
|
||
|
||
Everything else in the contract that this review touched — §0's defect chain,
|
||
§1's five rows, §2.1's four-consumer enumeration, §3.1/§3.2's wake mechanics,
|
||
§3.3's residence non-event, §4 F1's pinned constraints, §5's invariants — was
|
||
confirmed against source or pseudo-C.
|