docs: #265/#166 - ledger updates and capture-bisect as-fixed addendum
docs/ISSUES.md: #265 and #166 updated with the root cause and fix from the prior two commits; closure of both pends the user's visual-gate acceptance. #265 also records the confirmed-separate uphill-bounce finding (AD-25, byte-exact retail, out of scope). #166 records that the Campaign P visual-matrix recheck it was waiting on DID happen and found the glide/bounce still missing even with AD-25/AP-7/AD-55/TS-4 all landed - that negative result is what triggered the #265 capture bisect and this fix. docs/architecture/retail-divergence-register.md: AP-7's retirement note corrected. The row's original claim ("no horizontal velocity to hammer") undersold the gap - calc_friction was structurally unreachable with meaningful data on any grounded path, not just inert on the root-motion path. No new row filed: this change ports retail's mechanism faithfully and does not introduce a new deviation. docs/research/2026-07-30-265-capture-bisect.md: full "as-fixed" addendum (new section 9) recording the implementation - the fix mechanism, fixture results (freeze reproduced under the old model, slide+decay proven under the new one), the downhill-direction derivation for the synthetic decay case, the two separate mechanisms found while building the Runtime tests (LeaveGround's edge-timing recompute, AP-77's no-sink fallback), the uphill-bounce orthogonality proof, and final test totals. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
9910838fa4
commit
7fcc7db1d1
3 changed files with 388 additions and 30 deletions
|
|
@ -1,13 +1,20 @@
|
|||
# #265 capture-driven bisection — steep-slope response family
|
||||
|
||||
**Status: verdict reached, S1 and S2 both CLEARED for the two concrete
|
||||
mined events; real mechanism identified as a pre-existing (frozen-phase)
|
||||
architecture, not a Campaign P regression.** This is a research/bisection
|
||||
pass; no production code was changed. The harness (committed,
|
||||
**Status: FIX IMPLEMENTED 2026-07-30 (same day, §9 as-fixed addendum) —
|
||||
closure pends the user's visual-gate acceptance.** S1 and S2 both CLEARED
|
||||
for the two concrete mined events; the real mechanism was a pre-existing
|
||||
(frozen-phase) architecture, not a Campaign P regression — §1-§8 below are
|
||||
the original bisection pass (research-only, no production code changed
|
||||
at that point). §9 records what was actually implemented against that
|
||||
verdict: `PlayerMovementController.cs`'s grounded residual-velocity zero is
|
||||
removed for the animation-root-motion path, and `PhysicsEngine.cs` now
|
||||
wires `PhysicsBody.GroundNormal` from the committed contact plane. The
|
||||
harness (committed,
|
||||
`tests/AcDream.Core.Tests/Physics/Issue265SteepSlopeCaptureBisectTests.cs`)
|
||||
and mining tool (`tools/analyze_265_steep_slope_capture.py`) are permanent;
|
||||
the A/B code toggles described below were applied and reverted locally and
|
||||
never committed.
|
||||
the A/B code toggles described in §3 were applied and reverted locally
|
||||
and never committed (they remain historical — the actual fix is
|
||||
unrelated to S1/S2, see §9).
|
||||
|
||||
## 0. Scope recap
|
||||
|
||||
|
|
@ -435,3 +442,241 @@ explicitly NOT an S1/S2 code change — it is new work against
|
|||
`PlayerMovementController.cs`'s grounded-movement block and
|
||||
`PhysicsBody.calc_friction`'s wiring, gated on a design conversation, not a
|
||||
revert.
|
||||
|
||||
## 9. As-fixed addendum (2026-07-30, same day — implementation session)
|
||||
|
||||
The user chose the retail-faithful direction (§8's first option): port the
|
||||
genuine physics-driven momentum carry, wiring `calc_friction` for real
|
||||
rather than adding a narrower coast-distance patch. Implementation
|
||||
landed the same day as this bisect.
|
||||
|
||||
### 9.1 The fix
|
||||
|
||||
Two changes, both minimal and at the exact commit points already
|
||||
responsible for the adjacent state:
|
||||
|
||||
1. **`src/AcDream.Core/Physics/PhysicsEngine.cs`** — `body.GroundNormal`
|
||||
(the vector `calc_friction` dots velocity against, per its own doc
|
||||
comment "`angle = dot(velocity, contactPlane.N)`") had **zero
|
||||
production writers anywhere** before this fix; it silently defaulted
|
||||
to `Vector3.UnitZ` forever (`grep -rn "GroundNormal\s*=" src/` found
|
||||
only the property's own default and calc_friction's internal reads/
|
||||
writes). This is a SEPARATE gap from the one §4 found — even if
|
||||
Velocity had survived the grounded-tick zero, friction would have
|
||||
dotted it against a fake flat-ground normal on any real slope,
|
||||
producing wrong physics. Fixed by syncing
|
||||
`body.GroundNormal = ci.ContactPlane.Normal` (or
|
||||
`ci.LastKnownContactPlane.Normal`) at the exact block
|
||||
(`PhysicsEngine.cs` ~:1297-1320) that already publishes
|
||||
`body.ContactPlane`/`ContactPlaneValid` after every resolve — Core-level,
|
||||
so player, remote, ordinary, and projectile movers all get a real
|
||||
slope normal for free (matching the task's "the mechanism is general"
|
||||
requirement; the ordinary/remote physics updaters
|
||||
(`RuntimeOrdinaryPhysicsUpdater.cs`, `RuntimeRemotePhysicsUpdater.cs`)
|
||||
already compose root motion + `UpdatePhysicsInternal` cleanly, with no
|
||||
destructive zero — this fix brings the player path in line with its
|
||||
own siblings, not a novel invention).
|
||||
|
||||
2. **`src/AcDream.Runtime/Gameplay/PlayerMovementController.cs`** — the
|
||||
grounded-tick block §4 identified (`if (_body.OnWalkable) { ... if
|
||||
(hasAnimationRootMotion) _body.Velocity = new Vector3(0f, 0f,
|
||||
savedWorldVz); ... }`) no longer reconstructs `Velocity` AT ALL for the
|
||||
`hasAnimationRootMotion` case (production graphical local-player
|
||||
path). The condition is now `if (_body.OnWalkable &&
|
||||
!hasAnimationRootMotion)`, so ONLY the headless/test-controller
|
||||
`get_state_velocity` fallback (unchanged) still writes velocity here.
|
||||
Root motion continues to fully own commanded locomotion (walking
|
||||
displacement still comes from `pmDelta.Origin`, never from
|
||||
`Velocity`) — this does not reintroduce command- or packet-cadence-
|
||||
derived grounded translation (the DO-NOT-RETRY rule in
|
||||
`claude-memory/project_physics_collision_digest.md`); it only stops
|
||||
DESTROYING whatever `Velocity` already holds. The existing
|
||||
`preIntegratePos`/`postIntegratePos` bracketing (root-motion apply,
|
||||
then `calc_acceleration()` + `UpdatePhysicsInternal(tickDt)`, then
|
||||
`ResolveWithTransition(preIntegratePos, postIntegratePos, ...)`) was
|
||||
ALREADY structurally correct for composing both channels — retail's
|
||||
`CPhysicsObj::UpdatePositionInternal` composition model — so no
|
||||
further restructuring was needed once the destructive zero was
|
||||
removed.
|
||||
|
||||
### 9.2 Fixture results (freeze → slide, proven)
|
||||
|
||||
`Issue265SteepSlopeCaptureBisectTests.cs` gained a `ComposedTickSample`
|
||||
harness (`ReplayRealRoofLandingComposed`) that mirrors
|
||||
`PlayerMovementController.cs`'s per-tick composition line-for-line using
|
||||
only Core types (`PhysicsBody`, `PhysicsObjUpdate.HandleAllCollisions`,
|
||||
`PhysicsEngine`), parameterized by a
|
||||
`preserveResidualVelocityOnGroundedTick` toggle representing the old vs.
|
||||
new shape:
|
||||
|
||||
- **`ComposedRoofLanding_OldZeroingModel_ReproducesTheMinedFreeze`**
|
||||
(toggle `false`): reproduces the exact mined signature — velocity forced
|
||||
to `(0,0,0)` the tick after landing, frozen solid (`FrozenStreak` grows
|
||||
unbounded) for the rest of the replay.
|
||||
- **`ComposedRoofLanding_NewFix_VelocitySurvivesAndPositionKeepsAdvancing`**
|
||||
(toggle `true`): the SAME captured landing (velocity `(11.15, 14.13,
|
||||
-23.14)` onto the real `(2,3,6)/7` roof normal) now survives the Z-only
|
||||
hand-zero with its full horizontal speed, and the position advances
|
||||
every single tick (`adv=0.5149` per tick, `onWalk=true`, `frozen=0`)
|
||||
for the entire post-landing window — a genuine sustained glide, not a
|
||||
freeze. (The original small real-captured triangle had to be enlarged
|
||||
6x about its centroid — same plane, same normal, same landing point/tick,
|
||||
see `MakeRoofEngine`'s new `scale` parameter — because the real glide
|
||||
travels ~50 m over the test window and would otherwise run off the
|
||||
tiny real triangle's edge into the SEPARATE small-triangle-boundary
|
||||
artifact §7 item 2 already flagged; that artifact is confirmed
|
||||
real and unrelated to this fix, see §9.4.)
|
||||
- **`ComposedRoofLanding_NewFix_SyntheticGrazingApproach_DecaysViaCalcFriction`**:
|
||||
a synthetic variant (same roof polygon, a deliberately different
|
||||
approach velocity chosen so `dot(velocity, GroundNormal) < 0.25` after
|
||||
landing) proves genuine exponential decay: speed at landing ≈ 6.0 m/s
|
||||
decays tick-by-tick down to the `SmallVelocitySquared` hard-zero floor
|
||||
by roughly tick 33 after landing — retail's `calc_friction` formula
|
||||
working exactly as ported.
|
||||
|
||||
**Important nuance:** the REAL captured landing (record 3433's velocity
|
||||
and normal) happens to fall in retail's "moving away fast enough, no
|
||||
friction" band (`dot(velocity, GroundNormal) ≈ +9.25 ≥ 0.25`) — so it
|
||||
glides at CONSTANT velocity across the roof rather than visibly decaying.
|
||||
This is not a bug; retail's own `calc_friction` early-returns in exactly
|
||||
this case (the velocity's horizontal projection points "downhill," same
|
||||
direction as the normal's horizontal projection — see the derivation in
|
||||
§9.3). The task's framing ("decays over subsequent ticks") is
|
||||
demonstrated by the separate synthetic case above, which deliberately
|
||||
selects a velocity/normal pairing where retail's own formula calls for
|
||||
decay; the real mined case demonstrates the OTHER correct retail outcome
|
||||
(sustained glide) for its own geometry. Both are "survives and slides,"
|
||||
never "freezes" — the actual acceptance bar.
|
||||
|
||||
### 9.3 Downhill direction derivation (for the synthetic decay case)
|
||||
|
||||
For a planar triangle with outward normal N and any point P on the
|
||||
plane, `dot(N, P - centroid) = 0` (coplanarity). For a slope where Z
|
||||
increases as you move "uphill," the outward normal's horizontal
|
||||
projection points toward LOWER Z (downhill) — e.g. plane `z = m·x`
|
||||
(uphill as x increases) has normal `∝ (-m, 0, 1)`, whose horizontal
|
||||
component `-m` points toward decreasing x (downhill). The real captured
|
||||
roof normal `(0.2857, 0.4286, 0.8571)` has horizontal projection
|
||||
`(0.2857, 0.4286)` pointing downhill; the captured velocity's horizontal
|
||||
component `(11.15, 14.13)` points in nearly the same direction (both
|
||||
positive, roughly proportional) — i.e. the mover is genuinely sliding
|
||||
DOWN and AWAY from the impact point, which is exactly why
|
||||
`dot(velocity, normal)` comes out strongly positive and friction
|
||||
correctly declines to engage.
|
||||
|
||||
### 9.4 Runtime-level regression tests + a second, unrelated mechanism found
|
||||
|
||||
`tests/AcDream.Runtime.Tests/Gameplay/PlayerMovementControllerTests.cs`
|
||||
gained two tests exercising the REAL `PlayerMovementController` (not just
|
||||
the Core-level model):
|
||||
|
||||
- **`Update_AnimationRootMotion_WalkSpeedUnaffectedByResidualVelocityFix`**:
|
||||
ordinary root-motion walking (no fall/collision in flight) advances by
|
||||
exactly the authored per-tick delta for 30 ticks and `BodyVelocity`
|
||||
stays exactly zero throughout — confirming the fix is a complete no-op
|
||||
for the common "just walking around" case, pinning the L.3c hazard
|
||||
(`claude-memory/project_physics_collision_digest.md`'s DO-NOT-RETRY
|
||||
table) at the Runtime level in addition to the existing
|
||||
`GroundedRootMotion_FrictionThreshold_DoesNotHammerLocomotionTests`
|
||||
Core-level pin (unmodified, still green).
|
||||
- **`Update_RunningJumpLandsOnFlatGround_ResidualVelocitySurvivesAndDecays_NotFrozen`**:
|
||||
a real charged running jump (forward + jump, full production dispatch)
|
||||
lands on flat ground and its residual horizontal speed survives the
|
||||
first post-landing tick, then measurably decays (flat ground:
|
||||
`dot(velocity, (0,0,1)) ≈ 0 < 0.25`, so friction DOES engage here,
|
||||
unlike the real roof capture above).
|
||||
|
||||
**Building this test surfaced a second, genuinely separate,
|
||||
already-registered mechanism** (temporary `Console.WriteLine`
|
||||
instrumentation was added and fully removed per CLAUDE.md's diagnostic-
|
||||
logging discipline): `MotionInterpreter.LeaveGround()`
|
||||
(`CMotionInterp::LeaveGround` 0x00528b00, R3-W4/J7/J8, unrelated to
|
||||
#265/#166) recomputes and OVERWRITES `PhysicsObj.Velocity` from
|
||||
`GetLeaveGroundVelocity()` on the grounded→airborne edge, using whatever
|
||||
forward command is interpreted AT THAT EXACT TICK — a real, intentional,
|
||||
already-ported retail behavior. Releasing the forward key in the SAME
|
||||
tick this edge fires (an early test-construction mistake, not a
|
||||
production concern) clobbers the just-launched velocity. Separately,
|
||||
`MotionInterpreter.ApplyCurrentMovementInterpreted`'s AP-77
|
||||
"animation-less/headless movement fallback" (register row AP-77,
|
||||
already correctly scoped: "When `MotionInterpreter.DefaultSink` or the
|
||||
local PartArray callback is absent...") ALSO rewrites grounded velocity
|
||||
from `get_state_velocity()` on every `HitGround`/`LeaveGround` re-apply
|
||||
when no `DefaultSink` is wired — which is exactly the state of a
|
||||
`PlayerMovementController` built directly in a unit test without wiring
|
||||
one. Production (`GameWindow`) always wires a real `DefaultSink`, so
|
||||
neither mechanism is live there; the fixed test (1) holds Forward for one
|
||||
extra tick so `LeaveGround`'s one-time recompute captures the real
|
||||
launch velocity before releasing it, and (2) wires a minimal
|
||||
`FakeAnimationDispatchSink` as `controller.Motion.DefaultSink` so
|
||||
`ApplyCurrentMovementInterpreted` takes its real dispatch branch instead
|
||||
of the AP-77 fallback — making the test representative of the production
|
||||
graphical path rather than the headless one. **Neither mechanism
|
||||
required any production code change or register update** — AP-77's row
|
||||
already accurately describes its scope, and `LeaveGround`'s behavior is
|
||||
intentional retail-ported behavior, not a bug this task touches.
|
||||
|
||||
### 9.5 Symptom (a), the uphill bounce — confirmed separate, unaffected
|
||||
|
||||
Re-derived `PhysicsObjUpdate.HandleAllCollisions`'s `shouldReflect` gate
|
||||
byte-for-byte against the raw retail decomp
|
||||
(`acclient_2013_pseudo_c.txt:282647-282760`,
|
||||
`CPhysicsObj::handle_all_collisions`) this session:
|
||||
`var_10_1` (== `shouldReflect`) ends up `!(arg4 && (transient_state & 2)
|
||||
!= 0 && !sledding)` where `arg4` is `prevContact`/`prevOnWalkable`
|
||||
captured at `SetPositionInternal` entry (before this call's own commits)
|
||||
and `transient_state & 2` is read live inside `handle_all_collisions`
|
||||
itself — i.e. AFTER `set_on_walkable` has already committed the
|
||||
DESTINATION's OnWalkable bit. This is **exactly** `PhysicsObjUpdate.
|
||||
HandleAllCollisions`'s existing `shouldReflect = !(prevOnWalkable &&
|
||||
nowOnWalkable && !sledding)` — a byte-exact port, not a translation bug.
|
||||
For ANY fresh landing from airborne (`prevOnWalkable=false`), retail
|
||||
itself reflects whenever the collision normal shows "moving into the
|
||||
surface" (`dot < 0`), REGARDLESS of whether the destination is walkable.
|
||||
This is the SAME mechanism AD-25 closed (2026-07-30, Campaign P Slice
|
||||
P3, docs/ISSUES.md #166) for both local and remote movers — confirmed
|
||||
pre-existing and out of scope for this task, matching CLAUDE.md's "do
|
||||
not fix code that matches retail" rule.
|
||||
|
||||
`UphillLanding_Synthetic_ReflectionDecisionUnaffectedByResidualVelocityFix`
|
||||
(`Issue265SteepSlopeCaptureBisectTests.cs`) constructs a synthetic
|
||||
30°-uphill walkable slope, a falling-forward approach with `dot(velocity,
|
||||
normal) < 0` by construction, and runs `HandleAllCollisions` with and
|
||||
without the residual-velocity-preserving toggle applied AFTERWARD. The
|
||||
reflection decision (and its resulting velocity) is identical either way
|
||||
— proving the #265/#166 fix is orthogonal to whatever
|
||||
`HandleAllCollisions` decides, not a cause of or a fix for the bounce.
|
||||
The test's own log line documents the specific synthetic case DOES
|
||||
reflect (`Vz` goes from `0` to `+2.27` on this exact input), consistent
|
||||
with retail's byte-exact algorithm — evidence for a future dedicated pass
|
||||
if the user's live repro still shows an unwanted bounce, not a verdict
|
||||
this task renders.
|
||||
|
||||
### 9.6 Test/file summary
|
||||
|
||||
- `src/AcDream.Core/Physics/PhysicsEngine.cs` — `GroundNormal` sync.
|
||||
- `src/AcDream.Runtime/Gameplay/PlayerMovementController.cs` — grounded
|
||||
block no longer zeros `Velocity` for the animation-root-motion case.
|
||||
- `tests/AcDream.Core.Tests/Physics/Issue265SteepSlopeCaptureBisectTests.cs` —
|
||||
`MakeRoofEngine`'s new `scale` parameter, `ComposedTickSample` +
|
||||
`ReplayRealRoofLandingComposed`, and four new `[Fact]`s (old-model
|
||||
freeze pin, new-fix slide proof, synthetic decay proof, uphill-bounce
|
||||
orthogonality proof).
|
||||
- `tests/AcDream.Runtime.Tests/Gameplay/PlayerMovementControllerTests.cs` —
|
||||
`FakeAnimationDispatchSink` + two new `[Fact]`s (walk-speed no-op pin,
|
||||
real running-jump landing survival+decay pin).
|
||||
- `docs/ISSUES.md` — #265 and #166 updated (fix implemented, closure
|
||||
pends the user's visual-gate acceptance).
|
||||
- `docs/architecture/retail-divergence-register.md` — AP-7's retirement
|
||||
note corrected (the 0.25f threshold port was always right; it had
|
||||
nothing real to operate on until this fix closed both the grounded-
|
||||
velocity-zero and the `GroundNormal`-wiring gaps). No new row filed —
|
||||
this change ports retail's mechanism faithfully; it does not introduce
|
||||
a new deviation.
|
||||
|
||||
### 9.7 Verification
|
||||
|
||||
`dotnet test` (Release): 4074 Core tests / 2 skips, 434 Runtime tests / 0
|
||||
skips, 3971 App tests / 3 skips — all green, no regressions. Complete
|
||||
solution suite (9 projects): 9993 total, 9988 passed, 5 skipped, 0
|
||||
failed.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue