From ce638eb56f0e66c68a153d9c14712b6b55f40388 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 5 May 2026 18:02:44 +0200 Subject: [PATCH] docs(research): expand #42 handoff prompt for fresh-session pickup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the original 96-line note with a detailed self-contained brief targeted at someone picking up #42 cold in a new session. Adds: - Explicit ruled-out list (wire data, Euler error, stale velocity, diagnostic instrumentation) — saves rediscovering dead ends. - The user's "buildings + jumping puzzles" constraint that rules out blanket sweep-disable. - Specific file/line targets in PhysicsEngine.cs (470, 478-491, 493-519, 521-530, 532, 534-558) and TransitionTypes.cs (786-846, 1305-1311) with a brief reading order. - Phase 1 / Phase 2 / Phase 3 investigation plan with concrete diagnostic harness (`ACDREAM_AIRBORNE_DIAG=1` + `[SWEEP]` log) and direction-correlation test. - Per-hypothesis fix paths so the agent doesn't re-derive them from the diagnosis. - Full acceptance criteria including build/test gates and visual test sequence (flat / hillside / running / doorway / puzzle / land). - Hard rules (don't blame ACE, don't disable sweep, don't touch L.3 motion code, don't reduce sphere dims, etc.). - cdb breakpoint recipe for retail-vs-acdream A/B comparison. - Pre-session reading list with line numbers. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/research/2026-05-05-issue-42-handoff.md | 443 +++++++++++++++---- 1 file changed, 369 insertions(+), 74 deletions(-) diff --git a/docs/research/2026-05-05-issue-42-handoff.md b/docs/research/2026-05-05-issue-42-handoff.md index b5c63dd..721890d 100644 --- a/docs/research/2026-05-05-issue-42-handoff.md +++ b/docs/research/2026-05-05-issue-42-handoff.md @@ -1,96 +1,391 @@ -# #42 follow-up — PhysicsEngine.ResolveWithTransition airborne XY drift +# Handoff prompt — fix #42 PhysicsEngine airborne XY drift -**Context:** L.3 motion port landed in this branch (commits `de129bc` -`40d88b9` `2365c8c` `d57ace0` `c26bbbb` `b37b713`) — player-remote -running/walking/strafing/NPCs all visually verified clean. M4 jump -landing was fixed (CellId update). But that fix re-enabled -`ResolveWithTransition` per-tick during airborne arcs and exposed a -pre-existing PhysicsEngine bug: stationary jumps render with ~1 m -horizontal offset from the actor's actual XY, then snap back on the -next UM/UP. +Paste this into a fresh Claude Code session at the acdream repo root. +The branch you'll work on is `main` at or after commit `086e65d` +("Merge L.3 motion port"). Read `CLAUDE.md` first for project conventions. -**Confirmed root cause** (A/B-tested 2026-05-05): drift originates -inside `ResolveWithTransition`, not from wire data, local Euler error, -or stale velocity. With the sweep skipped, jumps render geometrically -correct (but body falls through floor). With it enabled, jumps land -correctly but show the drift. So the bug lives in the sweep. +## What you're fixing -## Most likely mechanism +When a retail-controlled toon jumps **in place** (no horizontal input, +just press jump) while observed through acdream, the visible jump arc +renders with a **~1 m horizontal offset** from the actor's actual XY +position. Body lands at the offset. Position snaps back to the actor's +real XY when the actor next sends any UM (turn / step / etc.). -**Initial-overlap depenetration along non-+Z terrain normal.** At -jump start the collision sphere is touching the floor at body Z. Most -outdoor terrain triangles have non-vertical normals (small horizontal -component). The sweep's first-frame action is to resolve the -penetration by separating the sphere along the contact normal — and -on a tilted triangle that separation has horizontal magnitude. The -body gets shoved sideways the first frame; the rest of the arc -carries the offset. +User's exact wording: *"I stand at position X and jump, it looks like +im jumping slightly to the left of X like 1 m-ish (if I observe +jumping char from behind). It also lands at X + 1 m-ish. Position +resets to X when I issue some other command to the client like turning."* -Direction-correlation test: jump at multiple landblock positions; if -drift direction varies with terrain slope orientation (not actor -facing), this hypothesis is confirmed. +This is **filed as `#42` in `docs/ISSUES.md`** with severity MEDIUM. +Pre-existing PhysicsEngine bug, exposed by L.3 M2 (which removed the +mid-arc UP hard-snap that was masking it) and by L.3 M4 (which +re-enabled `ResolveWithTransition` per-tick airborne). -Other candidates ranked by probability: -2. Step-down probe firing despite `isOnGround: false` parameter. -3. EdgeSlide on near-vertical motion against near-vertical surface. +## What's been ruled out (already, A/B-tested 2026-05-05) + +The drift is **definitively inside `PhysicsEngine.ResolveWithTransition`**. +Verified by toggling a single line in `GameWindow.OnLivePositionUpdated`: + +- `rmState.CellId = p.LandblockId;` REMOVED → sweep skipped → jumps + render geometrically clean (no XY drift), but body falls through + the floor (no terrain catch). +- `rmState.CellId = p.LandblockId;` PRESENT → sweep runs → jumps + land cleanly on the floor, but arc shows ~1 m horizontal drift. + +That A/B narrows the bug to one place. **Do not waste time** on: +- ACE wire `VectorUpdate` velocity components (not the source — drift + exists with sweep enabled regardless of wire data) +- Local Euler integration error (not the source — drift is geometric, + visible immediately on jump start) +- Stale `body.Velocity` from before the jump (M3 grounded path + zeroes it every tick; verified clean) +- Wire diagnostic instrumentation (`[VU.WIRE]`, `[UM_RAW]`, etc.) — + irrelevant. The bug is in the sweep, not the inputs. + +The bug is **inside `ResolveWithTransition`** (or one of the helpers +it delegates to: `Transition`, `SpherePath`, `CollisionInfo`, +`AdjustOffset`). + +## Why the fix can't just disable the sweep + +The user explicitly raised: *"What about jumping in to buildings or +on a jumping puzzle?"* + +Right. We need full sphere-vs-geometry resolution mid-arc for: +- Building doorways (jumping into a doorway you can stand in) +- Jumping puzzles (threading between platforms, around obstacles) +- Ceiling clips (hitting an overhang at jump apex) +- Edge collision (clipping against the corner of a platform) + +So the fix has to **keep the sweep running, but stop it from pushing +horizontally on stationary jumps**. Pure-vertical motion against a +roughly-horizontal floor must produce zero XY delta from the sweep. + +## The three ranked hypotheses + +These are the in-sweep mechanisms most likely producing the drift. +Listed by probability; (1) is the leading candidate. + +### H1: Initial-overlap depenetration along non-+Z terrain normal + +At jump start the collision sphere is touching the floor: `body.Z` is +at ground level, sphere radius extends downward into the terrain +triangle. The sweep's first tick has to resolve that overlap by +**separating the sphere along the contact normal**. + +Outdoor terrain is rarely perfectly flat. Most terrain triangles tilt +0.5–10° from horizontal. Their normals have a small horizontal +component. When the sweep separates the sphere along that tilted +normal, the separation has horizontal magnitude → body shoves +sideways → drift carries through the rest of the arc. + +**Direction-correlation test confirms this hypothesis if true:** +jumping at multiple landblock positions, the drift direction should +correlate with **local terrain slope orientation**, not the actor's +facing. (If drift is always relative to actor's facing, H1 is wrong +and H2 or H3 is the source.) + +### H2: Step-down probe firing despite `isOnGround: false` + +`PhysicsEngine.ResolveWithTransition` is called with `isOnGround: +!rm.Airborne` → `false` for airborne. But the sweep's internals may +still execute step-down logic regardless of that flag. Step-down +searches **horizontally** within `stepDownHeight` (0.4 m) for the +nearest walkable surface — even a small horizontal probe can shift +an airborne body sideways if the search finds a "better" surface. + +Already a known asymmetry exists in this region — see `K-fix7` / +`K-fix9` comments in `GameWindow.cs:6611-6620` and the matching +contact-plane gating in `PhysicsEngine.cs:493-519`. Step-up/down may +have a similar gap. + +### H3: EdgeSlide on near-vertical motion grazing a near-vertical surface + +If the sphere even slightly grazes a wall while ascending or +descending, `EdgeSlide` projects the motion vector tangent to the +wall surface, redirecting some Z velocity into XY. Less likely for +open-ground stationary jumps but cannot be ruled out without testing +in different environments. ## Files to investigate -- `src/AcDream.Core/Physics/PhysicsEngine.cs` — `ResolveWithTransition` - + any internal `CTransition` / `find_valid_position` helpers. The - initial-overlap depenetration path is the primary target. -- Reference at `docs/research/named-retail/acclient_2013_pseudo_c.txt`: - - `CTransition::find_valid_position` (called from `transition()`) - - `SpherePath` initialization - - Verbatim retail depenetration logic for airborne bodies +``` +src/AcDream.Core/Physics/PhysicsEngine.cs (658 LOC) + Line 470: ResolveWithTransition entry point + Lines 478-491: Transition / ObjectInfo setup + Lines 493-519: K-fix7 contact-plane gating (existing airborne fix) + Lines 521-530: SlidingNormal seed + Line 532: SpherePath.InitPath (path setup) + Lines 534-558: WalkablePolygon seed (grounded only) -If our port differs from retail in this region, that diff is the bug. +src/AcDream.Core/Physics/TransitionTypes.cs (2200 LOC) + Top-level: Transition, SpherePath, CollisionInfo, ObjectInfo classes + Search for: find_valid_position, step_sphere, AdjustOffset, + StepUp / StepDown / EdgeSlide branches + Lines 786-846: existing step-down + edge-slide branch logic + (where H2/H3 might be firing inappropriately for + airborne) + Lines 1305-1311: another step-down/contact branch worth checking -## Fix paths (in order of preference) +src/AcDream.Core/Physics/CollisionPrimitives.cs (718 LOC) + Sphere-vs-triangle math; depenetration-related primitives if any +``` -**(a) Skip initial-overlap depenetration when airborne.** Gate the -"separate from initial contact plane" step inside -`ResolveWithTransition` on `isOnGround: true`. Trusts the previous -tick's resolve to have left the body in a non-overlapping position. -Most likely correct fix. +The L.4 collision-resolution work added the existing airborne gates +(K-fix7 ContactPlane, K-fix9 ContactPlane during VectorUpdate) but +didn't touch step-up/down or initial-overlap depenetration paths. +Those are the gaps. -**(b) Zero step-up/down for airborne sweeps.** Pass -`stepUpHeight: 0f, stepDownHeight: 0f` from -`GameWindow.cs:6478+` when `rm.Airborne`. No side effects since -airborne bodies don't step. +## Reference: retail named-retail decomp -**(c) Stripped airborne sweep.** Replace the full sphere sweep with a -simpler vertical sphere-vs-terrain intersection + wall-collision -stop. Loses retail fidelity but eliminates all three mechanisms. -Probably overkill if (a) or (b) suffices. +Mandatory cross-reference. Located at: -## Repro +``` +docs/research/named-retail/acclient_2013_pseudo_c.txt (1.4 M lines, PDB-named) +docs/research/named-retail/acclient.h (verbatim retail headers) +docs/research/named-retail/symbols.json (greppable name → address) +``` -1. Launch acdream + retail client side-by-side on local ACE - (127.0.0.1:9000). -2. Have a retail-controlled toon stand still on outdoor terrain. -3. Jump in place. -4. Observe acdream window: arc shows ~1 m XY offset, lands offset, - snaps back on next inbound UM. +Functions to grep for in the named pseudo-C: -To verify hypothesis (1) specifically: repeat the jump on terrain -patches with different visible slope orientations; if drift direction -changes accordingly, depenetration is confirmed. +- `CTransition::init` @ `0x00509dd0` — explicitly clears + `contact_plane_valid = 0` at start of every resolve. Confirms the + airborne should not be running on a stale contact plane (we already + match this — K-fix7). +- `CTransition::find_valid_position` — entry to the actual sweep. + Read this carefully for the airborne-vs-grounded branching. +- `CSpherePath::InitPath` — path-setup; check whether retail does + any initial-overlap separation here. +- `CSpherePath::step_sphere` — the per-step inner loop. +- `CTransition::ValidateWalkable` — rebuilds contact plane when sphere + bottom is within EPSILON of terrain plane (per K-fix7 comment); for + airborne this should not establish a plane. +- `CCollisionInfo::set_contact_plane` and the depenetration paths + that use it. -## Acceptance +**Compare retail behavior against our port byte-for-byte for the +specific airborne-stationary-jump path.** Our port may have +inadvertently introduced behavior retail doesn't have (e.g., +unconditional initial separation), or omitted a retail gate (e.g., +"skip depenetration if `transient_state.Contact == 0`"). -- Stationary jumps render at the actor's actual XY (no perceptible - drift, no snap-back on next UM). -- Wall-collision airborne still works (jumping into doorways, jumping - puzzles where you have to thread between platforms or through arches). +## Investigation plan + +Two phases. Don't skip phase 1. + +### Phase 1: Confirm hypothesis (≤ 2 hours) + +1. Verify the M4 `CellId` fix is on `main` by grepping `GameWindow.cs` + for `rmState.CellId = p.LandblockId`. It should be present at the + top of the M2 player-remote branch in `OnLivePositionUpdated`. + +2. Add lightweight diagnostic logging inside `ResolveWithTransition`: + ``` + [SWEEP] guid= pre=(x,y,z) target=(x,y,z) post=(x,y,z) airborne= + ``` + Gate behind `ACDREAM_AIRBORNE_DIAG=1` so the diagnostic doesn't + spam logs in normal use. + +3. Have the user jump in place at 3+ different landblock positions + with visibly different terrain orientation: + - flat plaza tile + - hillside facing east + - hillside facing north + + For each, capture the sweep's `pre→target→post` deltas across the + first 5 frames of the arc. + +4. **Check direction correlation:** + - If drift direction varies with terrain orientation → H1 confirmed. + The first frame of the sweep is shifting the body horizontally + by an amount proportional to the local terrain slope. + - If drift direction is always relative to actor facing → H1 wrong; + check H2 (step-down probe) by comparing sweeps with non-zero vs + zero `stepDownHeight`. + - If no drift in open ground but drift near walls → H3. + +5. Stop at the first confirmed hypothesis. Move to phase 2 with + precise knowledge of which mechanism is firing. + +### Phase 2: Fix (1–4 hours depending on hypothesis) + +**If H1 confirmed (most likely):** +- Find the depenetration / "separate sphere from initial contact" + path in `Transition` / `SpherePath` / `CollisionInfo`. +- Gate it on `ObjectInfo.State.HasFlag(ObjectInfoState.Contact)` + (i.e., grounded). Trust the previous tick's resolve to have left + the body in a non-overlapping position. +- Compare against retail's `CTransition::init` and `find_valid_position` + — likely retail has this gate and we don't. + +**If H2 confirmed:** +- Find the step-up/step-down branches in `TransitionTypes.cs`. +- Gate them on `oi.Contact` (already exists at line 787 — verify it + fires correctly when airborne). +- May need to additionally zero `StepUpHeight` / `StepDownHeight` + when airborne at the call site in `GameWindow.cs`. + +**If H3 confirmed:** +- Find the EdgeSlide branch in `TransitionTypes.cs`. +- Add a gate that EdgeSlide only fires when motion has horizontal + velocity component above a threshold (so pure-vertical motion is + exempt). +- Verify against retail's edge-slide branch behavior. + +### Phase 3: Visual verification + +User will run retail + acdream side-by-side. Test cases: + +- ✅ Jump in place on flat ground — no XY drift +- ✅ Jump in place on hillside (different orientations) — no XY drift +- ✅ Jump while running forward — arc carries forward momentum (XY + delta from initial wire velocity, NOT from sweep) +- ✅ Jump into a building doorway — body collides with door frame / + threads through doorway correctly +- ✅ Jump from a platform onto another (jumping puzzle mechanic) — + body lands on the target platform +- ✅ Land on slope — sequencer leaves Falling correctly + +If any of these regress, the fix is wrong or incomplete. Iterate. + +## Diagnostic toolchain + +### cdb attach to retail (high-value when comparing retail vs ours) + +Toolchain documented in CLAUDE.md "Retail debugger toolchain" section. +The relevant breakpoints for #42: + +``` +bp acclient!CTransition::init "r $t0 = @$t0 + 1; gc" +bp acclient!CTransition::find_valid_position "r $t1 = @$t1 + 1; gc" +bp acclient!CSpherePath::step_sphere "r $t2 = @$t2 + 1; gc" +``` + +Have the user (running retail with cdb attached) jump in place. The +hit counts and any printed state from breakpoint actions reveal +retail's actual airborne sweep call pattern. Compare against +acdream's instrumented `[SWEEP]` log. Differences are bugs. + +**Important warnings from CLAUDE.md:** +- `qd` / `q` / `qq` are FORBIDDEN inside breakpoint actions — use + `.detach`. +- High-frequency breakpoints lag retail to ACE timeout — counter-only + actions for hot paths. +- `cdb -pd` does NOT survive `Stop-Process -Force`; detach cleanly. + +### Live launch + +```powershell +$env:ACDREAM_DAT_DIR = "$env:USERPROFILE\Documents\Asheron's Call" +$env:ACDREAM_LIVE = "1" +$env:ACDREAM_TEST_HOST = "127.0.0.1" +$env:ACDREAM_TEST_PORT = "9000" +$env:ACDREAM_TEST_USER = "testaccount" +$env:ACDREAM_TEST_PASS = "testpassword" +$env:ACDREAM_AIRBORNE_DIAG = "1" # enable sweep diagnostic once added +dotnet run --project src\AcDream.App\AcDream.App.csproj --no-build -c Debug *>&1 | + Tee-Object -FilePath "launch-42.log" +``` + +Wait 3–5 s between launches (ACE session cleanup race; CLAUDE.md). + +### Test character + +`+Acdream` at server guid `0x5000000A`. Per CLAUDE.md, this is the +GM-marker test toon. The user will control it (or another retail toon) +from a parallel retail client and you observe in acdream. + +## Acceptance criteria + +The fix is done when ALL of these hold: + +- [ ] Stationary jump on flat ground: arc + landing render at actor's + actual XY (zero perceptible drift, zero snap-back on next UM). +- [ ] Stationary jump on hillside: same — drift independent of + terrain orientation. +- [ ] Forward-running jump: arc carries actor's forward XY velocity + (this is server-driven, expected to track). +- [ ] Jump into a doorway: collision resolves correctly. +- [ ] Jump puzzle: thread between two platforms. +- [ ] Build green (`dotnet build`). +- [ ] Tests green (`dotnet test`) — modulo the 8 pre-existing + `Core.Tests` failures unrelated to this work. +- [ ] `[SWEEP]` diagnostic shows zero XY change on a stationary jump's + first sweep frame (the H1 confirmation residue). + +## Hard rules + +- **Don't blame ACE.** ACE is fixed; retail clients work against ACE + without this drift, so our PhysicsEngine port is the bug. +- **Don't disable the sweep wholesale.** The user explicitly needs + jumping puzzles + wall collision to keep working. +- **Don't change `OnLivePositionUpdated` or `TickAnimations`.** The + L.3 motion-port work is correct; this is a `PhysicsEngine` bug. +- **Don't touch `MotionInterpreter` or `AnimationSequencer`.** Same + reason. +- **Don't reduce sphere dims** to "avoid intersection". 0.48 m radius + / 1.2 m height is retail human-scale and matches local-player + collision; changing it on remotes only would cause asymmetric + behavior between local jumps (working) and remote jumps (different + collision profile). +- **Don't add diagnostic logging that prints every frame** without + an env-var gate — already burned the `[VEL_DIAG]` budget once. +- **Don't commit a partial fix that breaks any test in the + acceptance criteria.** Trade off completeness for correctness. + +## Pre-session reading list + +In order, before writing any code: + +1. `docs/ISSUES.md` § `#42 — Airborne XY drift` (the spec) +2. This handoff prompt (you're here) +3. `src/AcDream.Core/Physics/PhysicsEngine.cs:470-560` + (`ResolveWithTransition`) +4. `src/AcDream.Core/Physics/TransitionTypes.cs:780-870` + (existing step-down + edge-slide branches) +5. Grep `K-fix7` and `K-fix9` in `GameWindow.cs` and + `PhysicsEngine.cs` (these are the prior airborne-related fixes; + matching style is helpful) +6. The L.3 motion-port research at + `docs/research/2026-05-04-l3-port/` — + especially `01-per-tick.md` § 5 (`CPhysicsObj::transition`) and + `01-per-tick.md` § 6 (`SetPositionInternal`) for retail's + per-tick collision-sweep contract. +7. Retail's `CTransition::init` at `acclient_2013_pseudo_c.txt` line + 271954 (cited in K-fix7 comment). ## Operating notes -- This is a `PhysicsEngine` bug, not a motion-port bug. The L.3 work - is done; this is a separate investigation. -- The `[VU.WIRE]` instrumentation idea from #42's earlier draft can - be skipped — we already proved wire data isn't the source via the - A/B test. -- cdb attach to retail (`docs/research/2026-05-04-l3-port/`-adjacent - toolchain documented in CLAUDE.md) is available if comparing - retail's airborne sweep behavior against ours becomes useful. +- **Branch off `main`.** Do not work directly on main; use a feature + branch for the fix. Worktree is fine; CLAUDE.md and prior sessions + use `.claude/worktrees/`. +- **TDD where it fits.** A unit test that asserts + `ResolveWithTransition` returns zero XY delta for a stationary + vertical sweep on a tilted-normal triangle would lock the fix. + Existing `tests/AcDream.Core.Tests/Physics/PhysicsEngineTests.cs` + is the right home. +- **Visual verification is the acceptance test.** The unit test + proves the math but the user's eye proves the bug. Plan time for + the user to run the test sequence above. +- **One-session estimate: half a day to a day.** Phase 1 should + identify the hypothesis in under 2 hours; Phase 2 fix depends on + which one. Plan for two iterations if the first fix attempt + doesn't fully resolve. + +## Out of scope + +- `#41` (sub-decimeter velocity-synthesis blips) — separate issue. +- Refactoring NPC vs player-remote convergence in `TickAnimations` + (filed in audit § 6 of `06-acdream-audit.md`) — separate session. +- Local-player jump behavior — already works; don't touch. +- ACE-side fixes — out of scope. + +## Final advice + +The L.3 motion-port history (commits `de129bc`..`5cc2812`) shows what +this codebase rewards: spec-faithful reads of named-retail before +writing any code. Apply the same here. The fix is probably small — +maybe 5–20 lines — but finding it requires reading retail's +`find_valid_position` and our `Transition` carefully and identifying +the diff. Don't guess; verify.