acdream/docs/research/2026-07-07-184-slice2-unify-extract-handoff.md
Erik 4b07b0f541 docs(#184): Slice 2 handoff — extract RemotePhysicsUpdater (2a) + unify the player/NPC fork (2b)
Deliberately deferring the last #184 piece (Slice 2) to a fresh focused session per
the user's call. This is a self-contained handoff: the #40 sweep-blip verdict (DEAD,
high confidence — Path B already runs that config stably + a Core test proves it),
the 2a-first-then-2b plan, the exact code sites (Path A :10194-10429, Path B, the
extraction seam :10152-10873, the shared-helper wrinkle), the 2b merge gotchas
(Path A's player-specific omega/compose/diagnostic bits; the coupled :5699 shadow
sync), the preserve-list, and the test/gate plan.

Corrects the earlier ISSUES characterization: Slice 2 is a GATED behavior change to
the frozen R4/R5 arc (players gain the sweep), not a no-gate refactor. The #184
symptom itself stays RESOLVED + gated (Slices 1+3).

Research: workflow wf_c6a2e2b9-833 (3-agent read-only sweep).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-07 23:31:01 +02:00

15 KiB
Raw Blame History

Handoff: #184 Slice 2 — extract RemotePhysicsUpdater (2a) + unify the player/NPC remote fork (2b)

Date: 2026-07-07 · Status: PLANNED + DE-RISKED, implementation NOT started (deliberately deferred to a fresh focused session). Decision (user): do the FULL 2a + 2b (not the focused-2b subset) in a new session.

Read these first (in order):

  1. This handoff — the plan, the exact sites, the #40 verdict, the seams.
  2. Physics digest banner (SSOT): claude-memory/project_physics_collision_digest.md — the #184 Slice 1 + Slice 3 banners (what shipped, the shadow-follows-resolved mechanism, the placement-snap).
  3. Design spec: docs/superpowers/specs/2026-07-07-remote-creature-deoverlap-design.md §2.5 (the original Slice 2/3 staging) + §5 (register bookkeeping — the #40 retirement).
  4. Research behind this handoff: workflow wf_c6a2e2b9-833 (3 read-only Opus agents: #40 risk / extraction design / unification correctness). Findings distilled here.

0. Where #184 stands (what shipped, what this is)

The reported symptom — packed monsters interpenetrating (retail barely overlaps on the SAME ACE) — is FIXED and visually gated. Two commits on claude/vigorous-joliot-f0c3ad:

  • 37a94e1fSlice 1: NPC UpdatePosition → retail MoveOrTeleport placement-snap + grounded movement driven by the interp catch-up feeding the kept ResolveWithTransition sweep
    • shadow-follows-resolved (SyncRemoteShadowToBody, the load-bearing de-overlap persistence).
  • f51c1dffSlice 3: the sweep's mover sphere is Setup-derived (GetSetupCylinder = setup.Radius/setup.Height × ObjScale) so big/small monsters de-overlap at true radii.

Both gates passed (crowd de-overlap; large-monster spacing). Core 2621 / App 741 green. RemoteDeOverlapMechanismTests (4 tests) proves the mechanism in Core.

Slice 2 is the last piece — it is FAITHFULNESS + STRUCTURE, not a bug fix. It does two things:

  • 2a (behavior-neutral): extract the per-remote dead-reckoning update out of the >10k-line GameWindow.cs into a testable RemotePhysicsUpdater (Code Structure Rule 1).
  • 2b (behavior change, GATED): collapse the two-path fork — "Path A" (grounded PLAYER remotes) currently OMITS the sweep; make it run the SAME catch-up+sweep+shadow-follows-resolved as "Path B" (NPCs), so packed players de-overlap too and the "#40 remotes skip the transition" adaptation is retired. The user-visible payoff is minor (players rarely pack); the real win is retail-faithfulness (retail's UpdateObjectInternal has NO player/remote fork).

1. THE #40 VERDICT — DEAD (high confidence). 2b is SAFE. (do NOT re-investigate)

Path A skips the sweep because of a comment citing issue #40 ("running our sweep on tiny per-frame queue catch-up deltas amplifies micro-bounces into visible position blips — staircase + flat-ground blips", GameWindow.cs:10392-10399). The research settled that #40 is a stale observation and adding the sweep to Path A will NOT reintroduce it:

  • The decisive cross-check: Path B (NPCs) ALREADY runs the EXACT #40-feared configuration in production TODAY — clears Body.Velocity, drives translation purely from RemoteMotionCombiner.ComputeOffset catch-up (the SAME driver Path A uses), and calls ResolveWithTransition every tick on those tiny deltas. Path B's own comment says so: "Matches Path A's grounded model (:10113); clearing velocity mirrors Path A (:10125)" (GameWindow.cs ~:10489). It does NOT blip — #170/#171/#184 gates all passed.
  • Empirical proof in Core: RemoteDeOverlapMechanismTests.ConvergingCreatures_RealInterpLoop_DeOverlapsAndAbsorbsTheStallBlip drives the REAL ComputeOffset → InterpolationManager.AdjustOffset catch-up (incl. the fail_count>3 blip-to-tail) for 600 ticks and asserts every tick's net move stays < 0.30 m — the sweep absorbs the stall-blip.
  • #40 predates the sweep rebuild: #40 closed 2026-05-05 (commit 40d88b9), against the ORIGINAL BSP-layer sweep. Every relevant sweep-internals rebuild landed AFTER: the CSphere collision-family port (#182, 96ae2740, 2026-07-07), the #137 corridor sliding-normal/phantom-wall fixes, the L.2.3 step-height series, the #170/#171 chase/sticky work. #40 is against a sweep that no longer exists.
  • The slope-staircase half is STRENGTHENED, not reintroduced: Path A already samples terrainNormal and passes it to ComputeOffset (the 2026-05-05 slope-staircase fix, :10287); adding the sweep additionally provides retail terrain-Z snap along the slope.

Cheapest way to confirm #40 is dead BEFORE the visual gate (do this in 2b): extend RemoteDeOverlapMechanismTests with a converging PLAYER-config pair (same RealInterpLoop shape, player body/flags) and assert the same maxSpike < 0.30. If it passes, #40 is dead in code.


2. The plan — 2a FIRST (fork-preserving), then 2b (unify, gated)

Order matters: 2a before 2b. Do 2a as a strictly fork-preserving pure refactor (the Path A / Path B fork stays inside the new class, byte-for-byte), verify suites green (behavior-neutral, NO gate needed — the two passing gates remain a clean baseline), commit. THEN do 2b as a separate diff that merges the fork inside the now-clean class, with its own test + review + visual gate. Combining them would blow away the clean before/after baseline and make a gate failure unattributable.

⚠️ 2b is NOT a trivial "delete Path A." Path A is mostly subtractive but has small player-specific bits Path B lacks (see §4). Merging by editing Path B's shared logic risks regressing NPCs — reconcile carefully.


3. Slice 2a — the extraction (behavior-neutral)

3.1 The seam (all line numbers are HEAD f51c1dff, current)

  • Extract: the POSITION block of the foreach (var kv in _animatedEntities) loop in TickAnimations (GameWindow.cs): from uint serverGuid = ae.Entity.ServerGuid (:10152) through the sticky-lease tail rm.Host?.PositionManager.UseTime(); (:10873), gated by if (ae.Sequencer is not null && serverGuid != 0 && serverGuid != _playerServerGuid && _remoteDeadReckon.TryGetValue(serverGuid, out var rm) && rm.LastServerPosTime > 0) (~:10167-10182).
  • DO NOT extract the ANIMATION/RENDER half below ~:10876 (seqFrames/Sequencer.Advance, MotionDoneTarget bind, ConsumePendingHooks, PARTSDIAG, MeshRef rebuild). It drives the sequencer + hook router + rendering and must stay in GameWindow.

3.2 Signature + deps

  • RemotePhysicsUpdater.Tick(RemoteMotion rm, AnimatedEntity ae, float dt, int liveCenterX, int liveCenterY). Per-call state is exactly rm, ae, dt; serverGuid derives from ae.Entity.ServerGuid; _liveCenterX/Y are passed per-call (they change on streaming recenter — DO NOT snapshot them in the constructor).
  • Constructor deps (all pure services, no GL/render coupling): PhysicsEngine _physicsEngine, PhysicsDataCache _physicsDataCache, and the live IReadOnlyDictionary<uint, EntitySpawn> _lastSpawnByGuid (a shared reference GameWindow mutates; the updater only reads it, for GetSetupCylinder).
  • Location: src/AcDream.App/Physics/RemotePhysicsUpdater.cs (App-layer — it needs the App-owned RemoteMotion/AnimatedEntity types and the helpers). Register the file if a new folder.

3.3 Helpers to move (with the block)

Helper Site How it moves
IsPlayerGuid :1032 static (guid & 0xFF000000u)==0x50000000u static on the updater (or a tiny shared static)
ApplyPositionManagerDelta :4607 static, pure (body+delta) static on the updater
TickRemoteMoveTo :4864 body is just rm.Movement.UseTime(); inline into the updater
GetSetupCylinder :4572 deps _physicsDataCache + _lastSpawnByGuid move to the updater (constructor deps)
SyncRemoteShadowToBody :4633 deps _liveCenterX/Y + _physicsEngine.ShadowObjects move to the updater; take liveCenterX/Y as args (already the plan for Tick)
ApplyServerControlledVelocityCycle :5570 Path-B-only anim-cycle; Core calls only (ae.Sequencer, IsPlayerGuid, ServerControlledLocomotion.PlanFromVelocity) move verbatim for behavior-neutral 2a (candidate to relocate to the anim half later)

3.4 ⚠️ The one seam WRINKLE — the two helpers are ALSO called outside the DR loop

  • GetSetupCylinder is also called by StickToObjectFromWire (~:4662) and other moveto/sticky sites (:4441-4442, :4492, :4662, :13820-13821, :13872).
  • SyncRemoteShadowToBody is also called by the NPC UP-branch tail in OnLivePositionUpdated (~:6156, the Slice-1 first-UP/no-Sequencer shadow sync).

Decision to make in 2a: either (a) move both into RemotePhysicsUpdater and have GameWindow call them back through the _remotePhysicsUpdater instance from those other sites (clean ownership, but a general GetSetupCylinder living on the updater is a slightly odd home), OR (b) keep GetSetupCylinder on GameWindow and pass it to the updater as a Func dep, moving only SyncRemoteShadowToBody. Recommendation: (a) — GameWindow holds _remotePhysicsUpdater and calls _remotePhysicsUpdater.GetSetupCylinder(...) / .SyncRemoteShadowToBody(..., _liveCenterX, _liveCenterY) from the ~3 other sites. It's the cleanest single-owner.

3.5 2a acceptance

  • GameWindow.cs shrinks by the position block (~700 lines) → Tick(...) call.
  • Suites green (Core 2621 / App 741), NO behavior change, NO gate.
  • The fork (if (IsPlayerGuid(serverGuid) && !rm.Airborne)) is PRESERVED inside the updater.

4. Slice 2b — unify the fork (behavior change, GATED)

4.1 The delta: make grounded PLAYER remotes run the sweep

Path A (grounded player remotes, GameWindow.cs:10194-10429) is Path B minus:

  1. ResolveWithTransition (the sweep) — "Step 4b INTENTIONALLY OMITTED" (:10392-10399).
  2. SyncRemoteShadowToBody (shadow-follows-resolved).
  3. the #173 velocity bounce + the airborne landing detection (both no-op for a grounded body).

But Path A ALSO has bits Path B does NOT — these must survive the merge (do not regress players OR NPCs):

  • Omega fallback: Path A applies ObservedOmega ∥ seqOmega (:10351 — falls back to the sequencer's synthesized omega when ObservedOmega==0). Path B applies ObservedOmega ONLY. Reconcile so players keep the seqOmega fallback WITHOUT changing NPC omega behavior (do NOT just bolt the fallback onto Path B's shared omega — verify against NPC turning first).
  • Compose fallback: Path A has else { rm.Body.Position += offset; } when rm.Host is null (:10318-10321). Path B's equivalent is else if (!rm.Airborne) { rm.Body.Position += ComputeOffset(...); }.
  • Diagnostics: Path A's [SLOPE] (ACDREAM_SLOPE_DIAG) and [OMEGA_DIAG]/[VEL_DIAG] (ACDREAM_REMOTE_VEL_DIAG) blocks — keep or fold as desired (diagnostic-only).
  • Path B's grounded stale-velocity anim-stop is gated !IsPlayerGuid (ApplyServerControlledVelocityCycle) — so it already no-ops for players; leave that gate.

Cleanest merge (do it inside the extracted class): one grounded path = ComputeOffset catch-up (seed the sticky delta frame) → omega (ObservedOmega ∥ seqOmega) → calc_acceleration + UpdatePhysicsInternal → the sweep → store resolved → SyncRemoteShadowToBody#173 bounce → landing. Players and NPCs differ only in the !IsPlayerGuid-gated anim-cycle stop and the diagnostics.

4.2 ⚠️ THE COUPLED SHADOW-SYNC EDIT (research finding 9 — do NOT miss this)

Slice 1 made the per-UP raw-worldPos shadow sync in OnLivePositionUpdated (~:5699) PLAYERS-ONLY (NPC shadows follow the resolved body via SyncRemoteShadowToBody in the tick). If 2b now gives players the tick sweep + SyncRemoteShadowToBody, then the :5699 players-only raw-pos sync becomes the STALE one — packed players would de-overlap in-tick but re-snap-to-overlap once per UP. Retire / unify :5699 so players also get resolved-body sync (mirror how Slice 1 handled NPCs: consider a player UP-branch tail SyncRemoteShadowToBody for the first-UP / no-tick case, and remove or airborne-gate the :5699 raw sync). This edit is OUTSIDE the DR loop — that is exactly why 2b is its own reviewed diff.

4.3 Register bookkeeping (in the 2b commit)

  • Retire the "#40 remotes skip the transition / server already collision-resolved" adaptation — the register row + the GameWindow.cs:10207-10208 premise (ISSUES.md #40 anchors ~:4536/:4551). Cite that Path B already carries the sweep with passing gates.
  • Any new adaptation the merge introduces (e.g. the omega-fallback reconciliation) gets its row.
  • Move the AP-86 note about :5699 being players-only if that changes.

4.4 2b test + gate

  • Test first: the converging PLAYER-pair RealInterpLoop test (§1) — proves no #40 blip + players de-overlap, in Core, before the gate.
  • Visual gate (the acceptance test): (a) a player remote running up/down a hill — no slope staircase; (b) two player remotes packed side-by-side on flat ground — steady-state XY unchanged (no blip) + they de-overlap; (c) a regression pass on remote walk/run/jump/land/turn — UNCHANGED.
  • Suites green (Core / App).

5. Preserve-list (regression watch — the frozen R4/R5 arc)

  • The sweep + transition internals, the shadow registry, CollisionExemption — untouched.
  • Sticky melee #171 (the snapSuppressedByStick gate + StickyManager overwrite of the seeded frame) — survives; it's in Path B and the shared compose.
  • The airborne remote path (already goes through Path B via IsPlayerGuid && Airborne) — 2b changes only the GROUNDED player case; airborne is already swept.
  • The omega integration (both ObservedOmega and the seqOmega fallback for players).
  • The node_fail_counter blip watchdog.
  • Slices 1 + 3 (shadow-follows-resolved, placement-snap, Setup-derived sphere) — the base; unchanged.
  • The #42 self-collision self-skip (movingEntityId: kv.Key) — confirm it's passed on the unified player path (Path A currently does NOT call the sweep, so this is new for players).

6. Pointers

  • Research: workflow wf_c6a2e2b9-833 (this session) — the #40 verdict + extraction design + unification correctness, with file:line evidence.
  • Slices 1/3 commits: 37a94e1f, f51c1dff. The #184 arc plan/design: docs/superpowers/specs/2026-07-07-remote-creature-deoverlap-design.md; docs/research/2026-07-07-remote-creature-deoverlap-handoff.md.
  • Digest (SSOT + the failed-attempt lesson): claude-memory/project_physics_collision_digest.md.
  • Tests: tests/AcDream.Core.Tests/Physics/RemoteDeOverlapMechanismTests.cs (extend with the player-config RealInterpLoop).
  • The register: docs/architecture/retail-divergence-register.md (retire #40; AP-86/AP-87 are the #184 rows; TS-46 is the sphere-scalar residual).