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>
17 KiB
Issue #319 — retail-conformance DELTA review, round 2 (2026-08-05)
Verdict: PASS, with one MAJOR documentation/gate-coverage finding (D1) that must be closed before the connected gate runs — it does not require a code change.
Scope: delta only, against my round-1 report
(2026-08-05-issue-319-retail-review.md).
Same working tree, same base HEAD af828a8a, uncommitted. Reviewed
git diff HEAD (14 files, +934/-20) plus the three untracked docs.
Independent gates re-run for this round:
dotnet build AcDream.slnx -c Release— exit 0.AcDream.Runtime.TestsRelease — 1176/1176, 0 skipped.AcDream.App.TestsRelease — 4127 passed, 3 skipped, 0 failed.
Matches the coordinator's reported numbers. A green suite is not evidence; every finding below comes from source or pseudo-C.
1. The coordinator's primary question: does deleting our deferral diverge
from retail's queue-by-GUID replay?
No. Answered structurally, and it needs no register row.
Retail's mechanism is real: QueueBlobForObject @0x005092D0 buckets a
missing-parent blob under the parent's GUID on CObjectMaint and replays it
when that GUID is (re)created (AP-132's retail half). The question is whether
acdream still has that mechanism after the App-layer deferral was deleted.
It does, and the deleted code was never it. acdream's port of retail's
per-GUID blob bucket lives in ParentAttachmentState and is untouched by
this diff:
_deferredCreatesByParent(ParentAttachmentState.cs:22-23) — raw CreateObjects waiting on a parent GUID, filled byEnqueueDeferredCreate(:87-119) fromRuntimeEntityObjectLifetime.RegisterEntityCore:798-812._deferredAcceptedRelationsByParent(:34-35) — accepted relations waiting on a parent GUID. Its own doc comment already names the anchor: "Shares the SAME per-guid 'blobs waiting on guid X' shape … retail'sQueueBlobForObject/CObjectMaintbucket does not distinguish a raw Create blob from any other blob type queued against the same guid."- Replay on parent arrival is live:
DetachDeferredCreates/DetachDeferredAcceptedRelationsare driven fromRuntimeInitialCreateContinuationExecutor.ReplayDeferredChildren:1250-1275, whose comment cites retail's atomic per-parent detach (pseudo-C ~93617) and whoseRestoreDeferredCreatespath explicitly preserves retail's "blobs live onCObjectMaint, not on the object instance, so they survive the object and replay against a recreated GUID."
So the deleted DeferCreateObjectRelation was a third, redundant queue at
the App producer layer, sitting downstream of the layer that already
implements retail's mechanism — and downstream of the very gate that makes it
unreachable. Removing it removes duplication, not a retail behaviour.
Verified again for this round that the gate covers both wire shapes:
RegisterEntityCore:798 reads
incoming.ParentGuid ?? incoming.Physics?.Parent?.Guid ?? 0u, and
SameGenerationCreateObjectEvents (the sole source of CreateParentUpdate)
are produced inside AcceptCreate, reached only after that gate passes. The
implementer's structural-unreachability proof is correct for both producers,
which is what round 1's R1 found independently.
No register row is owed. The divergence that exists here — acdream gating
queued relations on parent incarnation where retail's replay is pointer-only —
is AP-132, already filed, and this diff does not widen it. _unresolvedByChild
(the ParentEvent queue) is untouched.
Retail-side consequence of the simplification: with deferral gone, the fix
is exactly "bind to the parent's live incarnation at accept time," and the
parent is guaranteed addressable at that moment by the layer above. That is a
closer mapping of GetObjectA(this, parent_id) @0x00558a2d than the deferred
version was — retail resolves the GUID once, at the attach, against whatever
CObjectMaint currently holds; it does not carry a pending relation forward at
the attach site either. GetNullObject @0x005093e6 (the SetChildren
placeholder) is retail's only "not yet constructed" accommodation, and it lives
on the parent-names-children direction, which acdream does not implement at
this producer. Nothing retail does was lost.
2. The refusal paths leave retail-correct state — verified
Two refusal sites, both new:
(a) AcceptLateBoundCreateObjectRelation's else branch
(EquippedChildRenderController.cs:866-873). Logs to stderr, mutates nothing.
No relation is staged, queued, or committed; the child keeps its own snapshot
and cell. That is a retail-representable state (an unattached object), and the
round-2 test OnCreateParentAccepted_ParentNotYetKnown_RefusesWithoutStateOrCrash
additionally pins that a later parent arrival does not retroactively attach
it — honest about the consequence rather than implying a recovery that does not
exist.
(b) CanCommitIncarnation (ParentAttachmentState.cs:588-604, called at
EquippedChildRenderController.cs:983 and
RuntimeLiveEntitySessionController.cs:414). Verified the full refusal
sequence:
- It is genuinely pure — reads
resolveParentInstance, writes nothing. - Both production call sites now evaluate it before their canonical
mutation (
CommitStagedParent/TryCommitParent), so the A1 torn transaction is structurally impossible, not merely unlikely. - On refusal both call
RejectProjection(:649-658), which removes the staged relation only if it matches exactly — so nothing is stranded to blockResolveforever, and a concurrently-replaced staged relation is not clobbered. - The App site returns
CanAdvanceWireQueue: true, and since the staged relation is gone the enclosingwhileloop's nextTryGetStagedProjectionfails and breaks. No infinite loop, and the recovery branch still runs. CommitProjectionre-checks the same precondition as its own first mutation- free step (:625-626), so the method is non-tearing for any caller, not only the two that pre-check.
The switch from throw to logged false also moots round 1's R6 in the
way that matters: I flagged that I could not exhaustively prove
_inbound._snapshots[parent].InstanceSequence never leads
_activeByGuid[parent].Incarnation inside a re-Create transaction. That gap
still exists as a fact, but its consequence changed from "destructive throw on
an unproven window" to "a logged refusal and an unattached child" — an outcome
retail can represent. Round 1's R6 is withdrawn as a risk and downgraded to
the observation in §5 below.
3. Round-1 findings — disposition
| round 1 | status |
|---|---|
| R1 (both deferred branches unreachable) | Resolved by deletion. The unreachable code is gone; the remaining else branch logs. The AcceptLateBoundCreateObjectRelation <remarks> block (:57-78) states the structural argument correctly, including that it is "not an empirical absence, a structural one." |
R2 (sentinel-0 collides with EndGeneration/DeleteGeneration filters) |
Moot. LateBindParentInstance and DeferCreateObjectRelation are gone; no relation with a placeholder incarnation ever enters _unresolvedByChild. Verified FilterParentCandidates (:1002-1018) now only ever sees wire-named incarnations. |
| R3 (two wrong test rationales) | Corrected, but incompletely — see D2. Both now cite the zero-FullCellId inertness and acknowledge HasCommittedParent is child-keyed and committed pre-fix. |
| R4 (D1's comment made vacuous) | Fixed correctly. RuntimeEntityObjectLifetime.cs:1531-1539 now distinguishes the ParentEvent producer (protection ACTIVE, AP-132) from the CreateObject producer (vacuous-by-design, enforced by CanCommitIncarnation). Accurate. |
| R5 (cross-file "above") | Fixed. LiveEntityPresentationController.cs:211 now names LiveEntityHydrationController.OnLandblockLoaded. |
| R6 (tripwire throw provability) | Withdrawn as a risk — see §2. |
| R7 (hydration gate broader than @0x00515D40) | Fixed. LiveEntityHydrationController.cs:179-187 now records that the exclusion covers the whole candidate loop and rests on route 7's P4 record, not on @0x00515D40 alone. |
4. Contract §1 row 5 correction — verified correct, cite it freely
docs/research/2026-08-05-issue-319-contract.md:82. Every address in the
correction was re-read at the pseudo-C for this round and is exact:
this->m_position.objcell_id = objcell_id@0x00515385 (same-cell branch) ✓- child loop
*(uint32_t*)((char*)eax_2 + 0x4c) = objcell_id_1@0x005153BD ✓ CPartArray::SetCellID@0x005153CC ✓- loop bounds @0x005153AE–@0x005153D8 (
do { … } while (ebx_1 < this->children->num_objects)) ✓ - cross-cell branch
CPhysicsObj::change_cell(this, curr_cell)@0x00515372 ✓ - function header
CPhysicsObj::SetPositionInternal(CPhysicsObj*, CTransition const*)@0x00515330 ✓ (the 4-arg overload at @0x00515BD0 is a different function; the row cites the right one)
The characterisation — "retail's D2 equivalent lives INSIDE THE SAME FUNCTION as the player's own per-tick cell write" — is accurate and is the strongest single anchor for both the equality invariant and AP-146. Safe for future sessions to cite.
5. New findings this round
D1 — MAJOR (documentation + gate coverage; no code change required). The A4 creature-parent correction was applied to RestoreShadow and NOT to the hydration gate, which carries the identical live behaviour change — and no gate watch item covers it.
Sites: src/AcDream.App/World/LiveEntityHydrationController.cs:569-570
(the gate and its comment block :551-569);
docs/research/2026-08-05-issue-319-contract.md:247-276 (§3.1, uncorrected);
docs/research/2026-08-05-issue-319-contract.md:544-559 (§7 Half B watch item,
covers only RestoreShadow); docs/ISSUES.md:13370-13374 (records A4 for §3.2
only).
The architecture review's A4 established that §3.2's "no-ops today on
FullCellId == 0" premise holds only for player-parented children, because
route 7's D1 already re-cells CREATURE-parented children to a nonzero cell — so
the RestoreShadow gate is a live behaviour change for that class. That
correction is applied at LiveEntityPresentationController.cs:212-220 and
mirrored into the contract's Half B recipe.
The identical argument applies to the hydration gate, and nothing records it. Verified chain at HEAD for a creature/static parent (incarnation 0):
OnSpawnstages the relation with the hardcoded0, which matches the parent's real incarnation.- D1's gate
parent.Incarnation == parentInstanceSequence(RuntimeEntityObjectLifetime.cs:1541-1543) passes →SetFullCell(child, parent.FullCellId, …). The child's canonical cell is nonzero at HEAD. ProjectionCellId => WorldEntity is not null ? FullCellId : …(LiveEntityRuntime.cs:387-389) → nonzero for a realized child.OnLandblockLoaded's candidate filter (:571-577) admits onprojectionCellId != 0+ landblock match +SetupTableId is not null. An equipped weapon satisfies all three when the parent's landblock loads.- It therefore enters the second loop and takes one of
ProjectExact(CreateSupersessionRecovery)(:599),RebucketLiveEntity(:611), orProjectExact(SpatialRecovery)(:618).RebucketLiveEntitywritesentity.ParentCellIdunconditionally (LiveEntityRuntime.cs:885-898, no attached-child guard) and calls_spatial.RebucketLiveEntity— a second spatial writer competing with route 7 D4'sRebucketEquippedChildPresentation.
So the hydration gate removes a live code path for creature-parented children,
exactly as the shadow gate does. The direction is right (route 7's P4/D4, and
retail's update_object parent != 0 early-out @0x00515D40 for the canonical
half), but three things are wrong as it stands:
- The gate's own comment (
:551-569) still frames the change as affecting only "a #319-fixed (nonzero) child", i.e. the player class. - The contract's §3.1 still asserts "A committed child today falls through all
three sources to
0" and "The gate is correct TODAY (it changes nothing for a zero-cell child)" — false for the creature class, and it is the sentence a future session will read as the justification. - Half B of the connected gate watches the shadow row but not the hydration path. A creature-parented weapon that stops being re-placed on landblock load is precisely route 7's "left behind at a boundary" regression shape, and nothing in §7 asks the runner to look for it.
Correct behaviour: mirror A4 into the hydration gate's comment and contract §3.1, and add a Half B step that carries an armed NPC across a landblock boundary (the parent's landblock unloading and reloading) and confirms the weapon still follows. Blast radius is one paragraph of docs plus one gate step, not code.
One link in the chain I did not close empirically and am flagging rather
than guessing: whether a realized attached child reliably has
InitialHydrationCompleted == true (TryMarkInitialHydrationCompleted,
LiveEntityRuntime.cs:2913-2928, requires only WorldEntity != null and
ResourcesRegistered, with no ProjectionKind filter). This only decides
which of the three second-loop branches the child took at HEAD — candidacy,
and therefore the behaviour change, holds either way.
D2 — MINOR. The R3 comment corrections are precise for the player row and imprecise for the creature row, in tests whose second row IS the creature class.
tests/AcDream.App.Tests/World/LiveEntityHydrationControllerTests.cs:150-158
and tests/AcDream.App.Tests/World/LiveEntityPresentationControllerTests.cs:102-109.
Both corrected comments end with "what was unreachable pre-fix is the NONZERO
FullCellId this candidate loop actually gates on … the child's cell stays 0
forever for a player parent." True for the player row
(0x50000123u/0x50000456u); false for the creature row
(0x70000099u/0x70000199u), where D1 already produced a nonzero cell at
HEAD — the same D1 fact that drives A4. Correct behaviour: qualify the sentence
per parent class, the way LiveEntityPresentationController.cs:212-220 now
does for the production comment.
D3 — LOW / observation. Two new unconditional Console.Error.WriteLine sites on paths reachable at wire cadence.
EquippedChildRenderController.cs:868-873 and
ParentAttachmentState.cs:601-606. Both are "should be structurally
unreachable" refusals, so volume is expected to be zero — but neither is
rate-limited or routed through a diagnostic owner, and CLAUDE.md's rule 5
prefers a subsystem diagnostic owner over ad-hoc writes. If either ever fires
on a per-frame retry path it becomes a log flood on a host that must survive
long endurance sessions (Slice K4's own constraint). Not blocking; worth a
follow-up rather than a change in this slice.
D4 — OBSERVATION (favourable). The ledger-convergence gap I judged non-blocking in round 1 was closed anyway.
Contract §6 test 9 now exists as three dual-parent-class tests
(ParentAttachmentStateTests: child removal, parent removal, full teardown
with mixed pending state), each asserting all four tables converge to zero.
Notably the teardown test deliberately leaves a live _unresolvedByChild
ParentEvent entry — proving convergence for the queue this fix did not
touch, which is the right target now that the deferred queue is gone. The
round-2 PrepareAndTryRealize_MismatchedIncarnation_RefusesBeforeCanonicalCommit
test is also well-aimed: it asserts at the canonical layer
(snapshot.ParentGuid null, snapshot.Position non-null) rather than at the
relation table, which is the layer the A1 tear actually corrupted.
Contract §6 tests 5 and 6 remain unwritten; my round-1 judgement stands
(test 5's premise is code-verified at
RuntimeEntityObjectLifetime.cs:2750-2752; test 6 pins a flavour the
short-circuit OR makes unobservable). Neither blocks.
6. Errors in the CONTRACT itself, round 2
One, and it is D1's second bullet: §3.1 (:247-276) was not given the A4
correction its twin §3.2 received. Its "the gate is correct TODAY (it changes
nothing for a zero-cell child)" is false for creature-parented children and is
the sentence most likely to be cited later. §3.2's body text (:279-289, "no-ops
today on record.FullCellId == 0") has the same residue, though the §7 Half B
note now overrides it — §3.1 has no such override anywhere.
Everything else re-checked this round — §1's table including the new row-5 correction, §4 F1/F2's pinned constraints, §5's invariants, §7's corrected criterion and its new Half B addendum, AP-142 clause (f), AP-132's amendment, AP-146, and #320 — remains accurate against source and pseudo-C.