acdream/docs/research/2026-08-06-276-remainder-scoping.md
Erik 408c8e8f34 docs: #276 remainder scoping — the outdoor half is already correct; EnvCell is the real gap
Analysis only. A candidate fix was written, built clean and passed the
three existing settler tests, then deliberately REVERTED — the only test
that discriminates it needs an EnvCell fixture that was not safe to
assemble at the end of this session. The production tree is unchanged.

Headline: the issue's framing is half wrong, and the half it misses is the
whole fix. 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 across
24 m cell crossings and wraps/bumps the landblock across 192 m boundaries.
So for an outdoor->outdoor settle, discarding settle.CellId costs nothing.

The live defect is EnvCells. An EnvCell id is not derivable from a world
position, and AdjustToOutside's guard ((cell & 0xFFFF) is >= 1 and <= 0x40)
deliberately excludes EnvCell ids from that path. settle.CellId is the ONLY
carrier of an EnvCell identity, and it is exactly what the settler drops —
so the defect is the issue's parenthetical ("outdoor/EnvCell seam, stacked
EnvCells"), not its main clause. That also means the fix is a no-op on the
outdoor path that dominates production and corrective only at the indoor
seam.

Recorded so the next reader does not repeat the misreading I made:
CommitTransitionPosition looks like it pairs a new cell with a stale local
origin, but line 259's `Position = worldPosition` runs the ordinary setter
first, so line 265 reads the already-updated origin. Retail anchor
CPhysicsObj::SetPositionInternal(CTransition const*) 0x00515330 commits both
objcell_id and frame, including EnvCells.

Also recorded: the three existing settler tests build bodies with no
CellPosition, so every one passes identically with or without the fix.
Shipping against them would repeat C5b finding D3 — a test that passed with
its own change reverted. The doc carries the exact discriminating test, its
required sabotage, and the fixture risk (the resolver must genuinely report
the EnvCell in settle.CellId; a fixture that silently resolves outdoor would
be green and prove nothing).

Open and unverified: whether the remote spawn-seed caller's body carries a
CellPosition at all. CommitTransitionPosition early-returns on a zero cell,
so the fix would be an inert no-op there and #276 would stay open for
remotes. Must be settled before claiming the fix closes both halves.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-08-06 06:33:53 +02:00

6 KiB

#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:

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.

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.