From 95ebc03af466fb3a7c85db9beb8de924dd8cdb5d Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 3 Aug 2026 13:44:04 +0200 Subject: [PATCH] docs: file #282-#284 and plan the recent-regression cleanup Three defects introduced by the 2026-08-02/03 stabilization batch, all found while reconciling #281's 43 test failures. Each is an instance of the weakness the placement campaign exists to remove - two owners of one fact with no single writer keeping them agreed - so they are cleared before C4 stacks six more placement routes on top of them. #282: WorldEntity.EffectCellId documents itself as existing only for outdoor dat stabs, whose null render parent still needs retail's outdoor landcell for CObjCell::IsInView gating; live/interior entities were explicitly meant to use ParentCellId. f24532ad began populating it for live entities, and because EntityEffectPoseRegistry.UpdateRoot resolves EffectCellId ?? ParentCellId it now wins - while 12+ sites still write ParentCellId alone. Retail carries one cell per object (CPhysicsObj::set_cell_id @0x0050f4f0, change_cell @0x00513390, ShouldDrawParticles @0x0050fe60). #283: 670f307c gave Runtime a world frame that rebases on the accepted teleport Position, while App's LiveWorldOriginState rebases only after old-window retirement completes. Between those edges the two disagree by the landblock delta. Not yet proven reachable; the plan proves or disproves it before restructuring anything. #284: a placement that cannot resolve returns RetrySetupUnavailable forever with nothing counting it or naming its reason. The fix is observability plus fail-fast on contradictory states, never a retry cap or timeout. Plan sequences S1 (#284) first so the other two are observable rather than archaeological, then S2 (#282), then S3 (#283) immediately before C4 route 3, which shares its portal code. Also records the gating change that would have caught all of this: the complete Release suite must be green before every commit, not a focused subset. Co-Authored-By: Claude Opus 5 --- docs/ISSUES.md | 54 +++++++ .../2026-08-03-recent-regression-cleanup.md | 141 ++++++++++++++++++ 2 files changed, 195 insertions(+) create mode 100644 docs/plans/2026-08-03-recent-regression-cleanup.md diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 98207867..7e475959 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -24,6 +24,60 @@ What does NOT go here: - Every session: scan OPEN issues at start; promote/close anything we touched during the session before ending. - Promoting to a Phase: mark as `DONE (promoted to Phase X)` + commit SHA where the Phase entry landed. +## Recent-regression cleanup — 2026-08-03 + +Plan: [`2026-08-03-recent-regression-cleanup.md`](plans/2026-08-03-recent-regression-cleanup.md). +All three were introduced by the 2026-08-02/03 stabilization batch, found while +reconciling #281's 43 test failures. + +- **#282 — OPEN — live entities now write `EffectCellId`, contradicting its + documented contract, and 12 `ParentCellId` writers cannot keep it in sync.** + `WorldEntity.EffectCellId` (src/AcDream.Core/World/WorldEntity.cs:95-102) + documents itself as existing ONLY for outdoor dat stabs, which keep a null + render parent while retail still gives their physics object an outdoor + landcell for `CObjCell::IsInView` particle gating — "live/interior entities + normally use `ParentCellId` instead." `f24532ad` began setting it on live + entities at rebucket and placement projection + (`LiveEntityRuntime.cs:855,1101,1238`). Because + `EntityEffectPoseRegistry.UpdateRoot:163` resolves + `EffectCellId ?? ParentCellId`, the field now WINS for live entities, while + 12+ production sites still write `ParentCellId` alone. Any of those that + changes cell without a matching rebucket strands particles and lights on the + entity's previous cell: effects vanish behind a wall the object no longer + occupies, or draw through one. Retail has exactly ONE cell per object + (`CPhysicsObj::set_cell_id` @0x0050f4f0, `change_cell` @0x00513390, read by + `ShouldDrawParticles` @0x0050fe60); the two-field split is our adaptation for + the null-render-parent stab case. Fix shape: one owner writes the entity's + visibility cell and the effects path reads that owner. Caught in miniature by + `LiveEntityLightControllerTests.Refresh_FollowsCurrentTopLevelRootAndCell`. +- **#283 — OPEN — Runtime's world frame and App's render origin rebase at + different moments during a teleport.** `670f307c` gave Runtime its own world + frame (`RuntimePhysicsState.ObserveLocalWorldFrame`), which rebases the + instant an accepted Position carries `TeleportAdvanced`. App's + `LiveWorldOriginState` rebases only when + `StreamingOriginRecenterCoordinator.Advance` observes + `IsOriginRecenterRetirementComplete` — many frames later, after the old + window has fully retired. Between those two edges the two owners disagree by + the source-to-destination landblock delta, so a remote Create converted by + Runtime lands a multiple of 192 m from the geometry App is building. Same + failure family as the zero-offset bug `670f307c` fixed, with a wrong origin + instead of a missing one. NOT yet proven reachable live — the portal reveal + gate may or may not exclude Create during that window, and #280 says other + work does continue arriving through it. Two owners of one fact; the campaign + answer is that Runtime owns the frame and App projects it. Route-3 adjacent. +- **#284 — OPEN — a placement that cannot resolve parks forever with no + diagnostic.** A first-entry placement whose world frame is absent returns + `RetrySetupUnavailable` (`RuntimeSetPositionState.PrepareMover:1535-1543`) + and is re-Advanced every pump indefinitely. Nothing counts it, names its + reason, or distinguishes "waiting for something that will arrive" from + "waiting for something that never can". This is why #281's 43 failures + presented as four unrelated symptoms across App and Runtime instead of one + cause. NOT a timeout or grace period — the fix is observability plus + fail-fast on genuinely unresolvable states, matching the committed-invariant + exception pattern established in `01f4791e`. Doing this FIRST makes #282, + #283, and every C4 route cheaper to diagnose and lets the connected gates + fail on nonzero parked entries. + ## C3c placement cutover — 2026-08-02 - **#276 — OPEN — SpawnPlacementSettler discards the settle's resolved diff --git a/docs/plans/2026-08-03-recent-regression-cleanup.md b/docs/plans/2026-08-03-recent-regression-cleanup.md new file mode 100644 index 00000000..adefa8a5 --- /dev/null +++ b/docs/plans/2026-08-03-recent-regression-cleanup.md @@ -0,0 +1,141 @@ +# Recent-regression cleanup — plan (2026-08-03) + +Three defects introduced by the 2026-08-02/03 stabilization batch, found while +reconciling the #281 test failures. All three are **ours, days old, and inside +the least-verified code in the tree**. They are cleared before C4 resumes so +six more placement routes are not stacked on top of them. + +Issues: #282 (two cell fields), #283 (two world origins), #284 (silent park). + +## Why these first + +Every one is an instance of the exact weakness the placement campaign exists to +remove: **two owners of one fact, with no single writer keeping them agreed.** +#282 duplicates "which cell is this in". #283 duplicates "where is zero". #284 +is why both stayed invisible. Fixing them inside C4 would mean diagnosing them +through C4's much larger diff. + +## Standing discipline for this plan + +- Retail is the oracle. Grep `docs/research/named-retail/acclient_2013_pseudo_c.txt` + by `class::method` before writing. +- Root causes only. No timeouts, grace periods, suppression flags, or + catch-and-ignore. #284 in particular is observability + fail-fast, never a + retry cap. +- **The complete Release solution suite must be green before every commit.** + Focused-run-only gating is exactly what let #281–#284 ship. The full suite + takes about 30 seconds; there is no excuse. +- Each fix is its own bisectable commit with root-cause evidence, and updates + the issue + divergence ledgers in that same commit. + +--- + +## S1 — #284: make a parked placement visible (do this first) + +Smallest, and it turns the other two from archaeology into observation. + +1. Classify the park reason at the single site that produces it + (`RuntimeSetPositionState.PrepareMover`): awaiting collision generation, + awaiting Setup, awaiting world frame. +2. Fold per-reason parked counts into the existing physics ownership snapshot + (`RuntimePhysicsState.CaptureOwnership`) so they appear wherever ledgers are + already asserted, and in the connected gates' `report.json`. +3. Fail fast on unresolvable parks. A park awaiting the world frame *while a + local player is already registered* is not a wait — it is a contradiction. + Surface it as a committed invariant exception, the pattern `01f4791e` + established for receipt-ledger violations. +4. Convergence contract: parked entries must be zero at every stable + checkpoint. Wire that into the lifecycle/nine-stop gate assertions. + +**Tests:** each park reason is reported exactly once and clears on resolution; +the contradictory park throws rather than retrying; ledgers converge to zero. +**Gate:** focused Runtime + complete solution suite. + +--- + +## S2 — #282: one owner for an entity's visibility cell + +1. **Establish the retail model.** `CPhysicsObj::set_cell_id` @0x0050f4f0, + `change_cell` @0x00513390, `set_cell_id_recursive` @0x00510da0, + `ShouldDrawParticles` @0x0050fe60. Retail carries ONE cell per physics + object, and particle gating reads that same cell. Write the pseudocode note + before touching C#. +2. **Audit the writers.** 12+ sites write `ParentCellId` + (`LiveEntityNetworkUpdateController` ×4, `RemotePhysicsUpdater` ×2, + `ProjectileController` ×3, `LiveEntityOrdinaryPhysicsUpdater`, + `LocalPlayerProjectionController`, `RemoteTeleportController`, …); 3 write + `EffectCellId`, all in `LiveEntityRuntime`. For each `ParentCellId` writer + record whether it also rebuckets — a rebucket with an exact cell currently + repairs the pair by accident. Produce the table before choosing the fix. +3. **Decide the shape.** `EffectCellId`'s documented purpose is narrow: outdoor + dat stabs that keep a null render parent while retail still gives them an + outdoor landcell. Live/interior entities were explicitly meant to use + `ParentCellId`. Preferred fix, in retail's direction: live entities stop + populating `EffectCellId`, the stab case keeps it as the documented + exception, and one owner writes the visibility cell that the effects path + reads. If the audit shows live entities genuinely need it, the alternative + is a single writer that maintains both — but never 12 independent writers + against a field that wins. +4. **Divergence register.** The two-field split is an adaptation from retail's + single cell. Add the row if none exists; delete it if step 3 collapses the + split. + +**Tests:** an entity crossing a cell boundary keeps its particles and lights +attached; an equipped/attached child keeps its parent-relative behaviour; the +outdoor dat stab case is unchanged. +**Gate:** focused App + complete suite, then a **user visual check** — a +monster with an active spell effect crossing a cell boundary, and a lit static +object, indoors and outdoors. + +--- + +## S3 — #283: one owner for the world origin + +Sequenced last of the three and immediately before C4 route 3, which touches +the same portal code. + +1. **Prove or disprove reachability first.** With S1 landed, assert at the + placement site that Runtime's frame center and App's `LiveWorldOriginState` + center agree; run the portal/recall routes. If they never diverge in + practice, the fix is a permanent invariant rather than a behaviour change — + record that and stop. Do not restructure on a hypothesis. +2. **Retail evidence.** How retail rebases its landblock offsets across a + teleport, and the ordering around `TAS_TUNNEL_CONTINUE` — the same + sequence #280 already needs read. Read once, use twice. +3. **Fix shape.** Runtime owns the world frame; App projects it. Today + `LiveWorldOriginState` is an independent owner with its own rebase edge. + Make it a projection of Runtime's frame, so there is exactly one origin and + the retirement-driven edge becomes a *publication* of that origin rather + than a second decision. This is the same ownership move the campaign has + already applied to entities, physics, and placement. +4. **Ordering invariant.** No placement may commit against an origin the + render side has not adopted. Whether that is expressed as a gate or made + structurally impossible falls out of step 3. + +**Tests:** a teleport whose old-window retirement lags by many frames cannot +commit a placement against a mismatched origin; frame and origin rebase +together; ordinary movement rebases neither (already pinned by +`RuntimeWorldFrameTests`). +**Gate:** focused + complete suite, lifecycle/reconnect, and a **user visual +check** on repeated portal/recall arrivals with objects present. + +--- + +## After S1–S3 + +Resume the original campaign order, unchanged: + +1. **C4 routes 2–7** — ForcePosition, portal (with S3 landed), remote + Create/Position, projectile correction, drops, pickup/parent/delete. + Fold in #276 and #277 where their route becomes authoritative. +2. **#280** — retail destination prefetch, landed adjacent to route 3. +3. **C5** — delete superseded writers, complete suite, lifecycle/reconnect, + nine-stop soak **on the final binary**, two-client observation, the #269/#278 + slope-glide check. Only then retire AP-1, AD-1, AP-131, and AD-60's legacy + half. +4. **AP-22** — `ShadowShapeBuilder` as sole authority for authored Setup + collision shapes. +5. **AD-10** — remote contact-plane projection through the real transition + sweep. +6. Final movement/collision matrix; ledger, architecture, roadmap, milestones, + memory, `CLAUDE.md`, `AGENTS.md`. Vendor Slice 5 resumes only after that.