diff --git a/docs/research/2026-08-06-276-remainder-scoping.md b/docs/research/2026-08-06-276-remainder-scoping.md new file mode 100644 index 00000000..b7c9ebed --- /dev/null +++ b/docs/research/2026-08-06-276-remainder-scoping.md @@ -0,0 +1,114 @@ +# #276 remainder — SpawnPlacementSettler settle-cell discard: scoping (2026-08-06) + +Analysis complete; **no production change committed.** A candidate fix was +written, built clean, and passed the three existing settler tests — then +**deliberately reverted**, because the only test that would *discriminate* it +needs an EnvCell fixture that was not safe to assemble in the session that +found this. See §4. + +Written at HEAD `1d2d4bb8`. + +--- + +## 1. The headline finding — the issue's framing is half right, and the half it misses is the whole fix + +`docs/ISSUES.md:1640` says the settler "commits `settle.Position` but never +reads `settle.CellId`", so a settle crossing a cell boundary "leaves the body's +cell at the placement cell". The symptom is real. The **outdoor** half of the +explanation is not. + +`PhysicsBody.Position`'s ordinary setter already carries the world displacement +into the landblock-relative frame **and** calls `LandDefs.AdjustToOutside`, +which recomputes the outdoor cell index from the local origin across 24 m cell +crossings and wraps/bumps the landblock across 192 m boundaries +(`src/AcDream.Core/Physics/PhysicsBody.cs:279-282`). So for an outdoor→outdoor +settle, `body.Position = settle.Position` **already lands the correct cell**. +Discarding `settle.CellId` costs nothing there. + +**The real gap is EnvCells.** An EnvCell id is not derivable from a world +position — there is nothing for `AdjustToOutside` to recompute, and its guard +(`(cell & 0xFFFF) is >= 1 and <= 0x40`, `PhysicsBody.cs:266`) deliberately +excludes EnvCell ids (≥ 0x100) from that path entirely. The resolver's +`settle.CellId` is the *only* carrier of an EnvCell identity, and it is exactly +what the settler drops. So the live defect is the issue's parenthetical — +"outdoor/EnvCell seam, stacked EnvCells" — and not its main clause. + +This matters for the fix's risk profile: the change is a **no-op on the outdoor +path that dominates production**, and corrective only at the indoor seam. + +## 2. The fix (validated, then reverted) + +Replace `body.Position = settle.Position;` +(`src/AcDream.Core/Physics/SpawnPlacementSettler.cs:61`) with the identical +guarded shape the per-tick resolve writeback uses at +`src/AcDream.Runtime/Physics/RuntimeOrdinaryPhysicsUpdater.cs:162-167`: + +```csharp +uint resolvedCellId = settle.CellId != 0 ? settle.CellId : cellId; +body.CommitTransitionPosition(resolvedCellId, settle.Position); +``` + +This is the issue's own prescribed shape ("commit the settle's resolved cell +through the same body/cell channel the per-tick resolve writeback uses"). + +**Why the ordering is safe** — worth recording, because reading +`CommitTransitionPosition` (`PhysicsBody.cs:257-277`) in isolation suggests it +pairs the *new* cell with a *stale* local origin. It does not: line 259's +`Position = worldPosition` runs the ordinary setter first, which updates +`CellPosition.Frame.Origin`; line 265 then reads the **already-updated** origin. +Retail anchor: `CPhysicsObj::SetPositionInternal(CTransition const*)` +@0x00515330 commits both `sphere_path.curr_pos.objcell_id` and its frame, +including EnvCells. + +**Verified:** `dotnet build src/AcDream.Core -c Release` → 0 errors, 0 warnings; +`SpawnPlacementSettlerTests` 3/3 pass. + +## 3. Reach — one caller confirmed, one unverified + +| caller | passes as `cellId` | body carries a cell? | +|---|---|---| +| `RuntimeLocalPlayerPhysicsPublicationState.cs:812` (C3c local first-entry) | `activation.Body.CellPosition.ObjCellId` | **Yes**, by construction | +| `LiveEntityNetworkUpdateController.cs:297` (remote spawn seed, #270) | a `cellId` parameter | **UNVERIFIED** | + +`CommitTransitionPosition` early-returns when `CellPosition.ObjCellId == 0` +(`PhysicsBody.cs:261`), so on any body without a cell the fix is an inert no-op +— safe, but it would mean #276 stays open for remotes even after the change. +**Establish the remote body's `CellPosition` provenance before claiming the fix +closes both halves.** The register row AD-61 covers this shared settler. + +## 4. Why no test was committed, and the exact test to write + +The three existing tests +(`tests/AcDream.Core.Tests/Physics/SpawnPlacementSettlerTests.cs`) construct +bodies with **no `CellPosition`**, so every one of them passes identically with +or without the fix — they cannot discriminate it. Shipping the change against +them would repeat the defect this campaign already paid for once (a +conservation test that passed with its own change reverted, C5b finding D3). + +Per §1 the discriminating case must be **indoor**, because an outdoor fixture +also passes both ways via `AdjustToOutside`. Required shape: + +1. `AddLandblock(0xA9B40000, flat terrain, [envCellSurface], portalPlanes, …)` + with an EnvCell surface id ≥ `0x100` — build the floor with + `CellSurfaceTests.MakeFlatFloor`'s 4-vertex quad pattern + (`tests/AcDream.Core.Tests/Physics/CellSurfaceTests.cs:20-38`). +2. Body seeded with `CellPosition.ObjCellId` = an **outdoor** placement cell + (e.g. `0xA9B40001`), positioned just above the EnvCell floor. +3. Call `TrySettle` with that outdoor cell as the wire `cellId`. +4. **Assert** `body.CellPosition.ObjCellId == 0xA9B40100` (the EnvCell). +5. **Sabotage that must redden it:** restore `body.Position = settle.Position` + — the cell stays outdoor, because `AdjustToOutside` cannot invent an EnvCell. + +The open risk, and the reason this was not attempted at the end of a long +session: step 1 must produce a fixture where the resolver genuinely reports the +EnvCell in `settle.CellId`. That may require portal planes for cell membership +(`CellTransit.FindCellList`). A fixture that silently resolves to the outdoor +cell would make the test pass for the wrong reason in the *other* direction — +green, and proving nothing. Verify `settle.CellId` is the EnvCell id before +trusting the assertion. + +## 5. Recommended next step + +Write the §4 test first, watch it fail against HEAD, then apply §2 and watch it +pass, then sabotage. One commit, ~40 lines of test, ~8 of production. Resolve §3 +in the same pass or state plainly that the remote half remains open.