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>
This commit is contained in:
Erik
parent
f0f6a92a4c
commit
4b07b0f541
2 changed files with 233 additions and 5 deletions
|
|
@ -48,11 +48,15 @@ Copy this block when adding a new issue:
|
|||
|
||||
## #184 — Remote monsters overlap (arms interpenetrating) in a crowd; retail barely overlaps
|
||||
|
||||
**Status:** SYMPTOM RESOLVED + gated (Slices 1+3 shipped, `37a94e1f`+`f51c1dff`); only the optional
|
||||
internal refactor (Slice 2) remains. The reported symptom — packed monsters interpenetrating — is
|
||||
FIXED end-to-end: monsters visible, de-overlapped, at their true creature radii. **Slice 2** (collapse
|
||||
the player-remote Path A into the same catch-up+sweep model + extract a testable `RemotePhysicsUpdater`
|
||||
per Code Structure Rule 1) is an internal refactor with NO visual change — no gate needed; not urgent.
|
||||
**Status:** SYMPTOM RESOLVED + gated (Slices 1+3 shipped, `37a94e1f`+`f51c1dff`); Slice 2 HANDED OFF
|
||||
to a fresh session. The reported symptom — packed monsters interpenetrating — is FIXED end-to-end:
|
||||
monsters visible, de-overlapped, at their true creature radii. **Slice 2** (2a: extract a testable
|
||||
`RemotePhysicsUpdater` per Code Structure Rule 1; 2b: unify the player/NPC fork so grounded player
|
||||
remotes gain the sweep + shadow-follows-resolved, retiring the #40 "remotes skip the transition"
|
||||
adaptation) is a **gated behavior change to the frozen R4/R5 arc** (not a no-gate refactor — the
|
||||
earlier characterization was wrong). The #40 sweep-blip risk was investigated and is DEAD (Path B
|
||||
already runs that config stably + a Core test proves it). Full plan + exact sites + the #40 verdict:
|
||||
`docs/research/2026-07-07-184-slice2-unify-extract-handoff.md`.
|
||||
**Severity:** MEDIUM
|
||||
**Filed:** 2026-07-07
|
||||
**Component:** physics / remote dead-reckoning
|
||||
|
|
|
|||
224
docs/research/2026-07-07-184-slice2-unify-extract-handoff.md
Normal file
224
docs/research/2026-07-07-184-slice2-unify-extract-handoff.md
Normal file
|
|
@ -0,0 +1,224 @@
|
|||
# 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`:
|
||||
- `37a94e1f` — **Slice 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).
|
||||
- `f51c1dff` — **Slice 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).
|
||||
Loading…
Add table
Add a link
Reference in a new issue