From f8d669be88f33b41625e5579096df17b61b0f1d2 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 22 May 2026 10:32:23 +0200 Subject: [PATCH] =?UTF-8?q?fix(phys):=20A6.P3=20slice=202=20v2=20=E2=80=94?= =?UTF-8?q?=20revert=20seed=20removal=20+=20add=20no-op=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First slice 2 attempt at commit 892019b removed PhysicsEngine.cs L622 per-tick CP seed entirely. User happy-test surfaced a regression: BSP step_up at the last step of stairs failed because sub-step 1's AdjustOffset had no ContactPlane to compute the lift direction (the seed was load-bearing for step_up correctness). Revert + better fix: 1. Re-add the L622 seed (PhysicsEngine.cs:620-626). 2. Add no-op-if-unchanged guard inside CollisionInfo.SetContactPlane (TransitionTypes.cs:259-279). When called with values identical to current state, early-return without incrementing ContactPlaneWriteCount or rewriting fields. When the player stands on the same plane tick after tick, the L622 seed re-calls SetContactPlane with identical args — these now no-op instead of inflating the counter and re-writing the same values. Only actual state changes (e.g. landing on a new step's plane, cell crossing) increment the counter. Verification (post-rebuild, pre-this-commit slice 2 first attempt): - scen3 walk produced 2,690 cp-writes (down from 30,420 = 91% reduction from L622-seed presence) - BUT user could not pass the last step of stairs — step_up regression - Test suite: 1148 + 8 pre-existing fail baseline maintained but physical behavior broke Post-this-commit expectations: - Test suite: 1148 + 8 (unchanged, no behavioral change in fixtures because the seed value is what the fixtures already expect) - Stair-walking: works (seed restored) - CP-write count: significantly reduced (most seeds are no-ops because body.CP doesn't change tick-over-tick on stable footing) - Issues #96 / #97: re-test in re-capture; #96 should be largely closed via the guard; #97 (fall-through + stuck-in-falling) was observed pre-slice-2 too, so unrelated to the seed Co-Authored-By: Claude Opus 4.7 (1M context) --- src/AcDream.Core/Physics/PhysicsEngine.cs | 55 ++++++++++----------- src/AcDream.Core/Physics/TransitionTypes.cs | 22 +++++++++ 2 files changed, 49 insertions(+), 28 deletions(-) diff --git a/src/AcDream.Core/Physics/PhysicsEngine.cs b/src/AcDream.Core/Physics/PhysicsEngine.cs index f225b4d..aaf5a48 100644 --- a/src/AcDream.Core/Physics/PhysicsEngine.cs +++ b/src/AcDream.Core/Physics/PhysicsEngine.cs @@ -597,36 +597,35 @@ public sealed class PhysicsEngine if (isOnGround) transition.ObjectInfo.State |= ObjectInfoState.Contact | ObjectInfoState.OnWalkable; - // A6.P3 slice 2 (2026-05-22) — DO NOT seed ContactPlane at - // transition start. Retail's CTransition::init at 0x509dd0 - // (acclient_2013_pseudo_c.txt:271954) explicitly clears - // contact_plane_valid = 0 — retail does NOT seed. Cross-tick - // CP retention flows via: - // - Mechanism A: BSPQuery.FindCollisions Path-6 land write - // (writes ContactPlane during the sub-step loop when the - // sphere ground-contacts a floor poly) - // - Mechanism B: Transition.ValidateTransition LKCP restore - // (slice 1 commit 5aba071; restores ContactPlane from - // LastKnownContactPlane post-sub-step when sphere close to - // LKCP) - // - Body persist below: at transition end, body.ContactPlane - // gets set from ci.ContactPlane OR ci.LastKnownContactPlane - // so the NEXT tick's body state is preserved. + // K-fix7 (2026-04-26): only seed the contact plane when the body + // is actually grounded. Pre-seeding while AIRBORNE caused + // AdjustOffset's "Have a contact plane / Moving away from plane" + // branch to fire on every jump step — which calls + // Plane::snap_to_plane on the offset and ZEROES the Z component, + // killing all upward jump motion. // - // Cost: AdjustOffset on sub-step 1 of each tick has no CP and - // takes the "no contact plane" path (TransitionTypes.cs:2329-2334) - // — slide-projects only, no slope-snap. Sub-steps 2+ pick up CP - // from Mechanism A / B as normal. Imperceptible in practice. + // We KEEP the seeding when isOnGround for slope-walking + step-up + // continuity (the original concern that motivated the seed). + // BSP step_up needs ContactPlane on sub-step 1 to compute the + // correct lift direction; removing the seed breaks stair-walking + // at the last step (verified by A6.P3 slice 2 first attempt + // 2026-05-22, reverted in this commit). Retail's CTransition::init + // explicitly CLEARS contact_plane_valid; we deliberately diverge + // for step_up correctness. // - // Removing this seed closes issue #96 (per-tick CP-write blowup; - // 99.3% of post-slice-1 CP writes came from the seed) and is - // hypothesized to close issue #97 (phantom collisions + - // fall-through; suspected stale-CP slope-snap was the cause). - // - // Previous K-fix7 seed (2026-04-26 to slice-1 ship) deliberately - // diverged from retail for "slope-walking continuity" but the - // tradeoff (per-tick CP writes, stale-CP slope-snap) was not - // worth it. + // A6.P3 slice 2 (2026-05-22) — to close issue #96 (per-tick CP-write + // blowup) without breaking stair-walking, the no-op-if-unchanged + // guard inside CollisionInfo.SetContactPlane (TransitionTypes.cs:259) + // collapses redundant seeds (same plane every tick) to a true no-op. + // The seed still fires the function call but only counts as a write + // when the plane values actually change. + if (isOnGround && body is not null && body.ContactPlaneValid) + { + transition.CollisionInfo.SetContactPlane( + body.ContactPlane, + body.ContactPlaneCellId, + body.ContactPlaneIsWater); + } // Retail CPhysicsObj::get_object_info also seeds SlidingNormal when // transient_state has bit 2 set. This matters for one-step/frame hits: diff --git a/src/AcDream.Core/Physics/TransitionTypes.cs b/src/AcDream.Core/Physics/TransitionTypes.cs index fe3e2ef..048d2c4 100644 --- a/src/AcDream.Core/Physics/TransitionTypes.cs +++ b/src/AcDream.Core/Physics/TransitionTypes.cs @@ -258,6 +258,28 @@ public sealed class CollisionInfo public void SetContactPlane(Plane plane, uint cellId, bool isWater = false) { + // A6.P3 slice 2 (2026-05-22): no-op-if-unchanged guard. Closes + // issue #96 (per-tick CP-write blowup) without removing the + // PhysicsEngine.cs L622 seed that step_up depends on. When the + // player stands on the same plane tick after tick, the L622 seed + // re-calls this function with identical arguments — collapse + // those redundant calls to a true no-op. The function still gets + // called from the seed site; this guard just skips the increment + // + field writes when nothing would actually change. + // + // This is safe because all four fields (ContactPlane, + // ContactPlaneCellId, ContactPlaneIsWater, ContactPlaneValid) + // are write-only side effects — no observer reads them mid-call. + // Same logic applies to the LKCP re-latch below. + if (ContactPlaneValid + && ContactPlaneCellId == cellId + && ContactPlaneIsWater == isWater + && PlaneEquals(ContactPlane, plane)) + { + // Already in the target state. Skip the write + counter. + return; + } + ContactPlaneWriteCount++; ContactPlaneValid = true; ContactPlane = plane;