From d2f6067960319c17d4c39364a1049e77c6635923 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 29 Apr 2026 17:24:49 +0200 Subject: [PATCH] =?UTF-8?q?fix(physics):=20L.2.3c=20=E2=80=94=20preserve?= =?UTF-8?q?=20contact=20plane=20through=20failed=20step-up?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "stuck in falling animation against walls" live-test bug (intermittent, hard to recover from). Two compounding issues, fixed at both layers. (1) DoStepUp cleared CollisionInfo.ContactPlaneValid unconditionally at the start of step-up. On step-up FAILURE, RestoreCheckPos restored the position but the contact plane stayed cleared. Added a save/ restore around the clear so a failed step-up returns the mover to its pre-attempt grounded state. (2) ValidateTransition propagated the current frame's invalid contact state into LastKnownContactPlane via: ci.LastKnownContactPlaneValid = ci.ContactPlaneValid This destroyed the prior frame's ground memory whenever the current contact was momentarily lost (StepUpSlide clears ContactPlane). Changed to: only OVERWRITE LastKnown when current is valid. (3) The same ValidateTransition then set oi.State &= ~(Contact | OnWalkable) when ContactPlaneValid was false, even if LastKnown was still valid. Added an "else if (LastKnownContactPlaneValid)" branch that sets Contact + OnWalkable from LastKnown so the animation system sees the mover as grounded. Combined effect: walking into a too-tall wall now consistently slides along the wall without ever flickering to the falling animation. The mover's grounded state survives transient ContactPlane invalidation during the step-up retry cycle. Retail's `transitional_insert` has different upstream invariants that keep ContactPlane valid more often, so retail doesn't need the acdream-specific LastKnown fallback path. ACE has the same pattern as retail; acdream's per-frame Resolve architecture exposes the gap that this fix closes. Tests: - New D1 regression test: grounded mover into too-tall wall — must end frame with grounded state preserved. - New D2 regression test: same scenario — execution time bounded (<100ms) to catch any future recursion issues. Files: - TransitionTypes.cs DoStepUp: save+restore ContactPlane around step-up - TransitionTypes.cs ValidateTransition: preserve LastKnown + grounded state from last-known when current is invalid - BSPStepUpTests.cs: D1, D2 regression tests Test count 825 → 825 (D1+D2 added in L.2.3 patch series). Build clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/AcDream.Core/Physics/TransitionTypes.cs | 52 ++++++++++++- .../Physics/BSPStepUpTests.cs | 76 +++++++++++++++++++ 2 files changed, 125 insertions(+), 3 deletions(-) diff --git a/src/AcDream.Core/Physics/TransitionTypes.cs b/src/AcDream.Core/Physics/TransitionTypes.cs index e4e784c..bd34857 100644 --- a/src/AcDream.Core/Physics/TransitionTypes.cs +++ b/src/AcDream.Core/Physics/TransitionTypes.cs @@ -1356,6 +1356,19 @@ public sealed class Transition var ci = CollisionInfo; var oi = ObjectInfo; + // L.2.3c (2026-04-29): capture the existing contact plane BEFORE + // clearing it. On step-up failure (too-tall wall) we restore it so + // the mover stays grounded — without this, walking into a wall + // dropped OnWalkable and the animation system flickered to falling. + // Retail clears here too (acclient_2013_pseudo_c.txt:273099) but + // its outer transition state seeded the plane back via a different + // path (LastKnownContactPlane retention + check_contact). For + // acdream's per-frame Resolve we restore here directly. + bool savedCpValid = ci.ContactPlaneValid; + Plane savedCp = ci.ContactPlane; + uint savedCpCellId = ci.ContactPlaneCellId; + bool savedCpIsWater = ci.ContactPlaneIsWater; + ci.ContactPlaneValid = false; ci.ContactPlaneIsWater = false; @@ -1381,8 +1394,21 @@ public sealed class Transition sp.WalkableValid = false; if (!stepDown) + { sp.RestoreCheckPos(); + // L.2.3c: restore the pre-step-up contact plane. The mover was + // grounded before the failed climb attempt; failing to climb + // a too-tall wall must not change that. + if (savedCpValid) + { + ci.ContactPlane = savedCp; + ci.ContactPlaneValid = true; + ci.ContactPlaneCellId = savedCpCellId; + ci.ContactPlaneIsWater = savedCpIsWater; + } + } + return stepDown; } @@ -1498,11 +1524,18 @@ public sealed class Transition ci.SetSlidingNormal(ci.CollisionNormal); // Preserve contact plane for next step. - ci.LastKnownContactPlaneValid = ci.ContactPlaneValid; + // L.2.3c (2026-04-29): only OVERWRITE LastKnown when current is valid. + // Previously: `LastKnownValid = ContactPlaneValid` cleared + // LastKnown whenever current was invalid — destroying the prior frame's + // contact memory. After StepUpSlide cleared ContactPlane mid-step + // (failed step-up against a too-tall wall), this propagated to + // LastKnown and the player went airborne for a frame, flickering the + // falling animation. Now LastKnown survives transient losses. if (ci.ContactPlaneValid) { - ci.LastKnownContactPlane = ci.ContactPlane; - ci.LastKnownContactPlaneCellId = ci.ContactPlaneCellId; + ci.LastKnownContactPlaneValid = true; + ci.LastKnownContactPlane = ci.ContactPlane; + ci.LastKnownContactPlaneCellId = ci.ContactPlaneCellId; ci.LastKnownContactPlaneIsWater = ci.ContactPlaneIsWater; oi.State |= ObjectInfoState.Contact; @@ -1511,6 +1544,19 @@ public sealed class Transition else oi.State &= ~ObjectInfoState.OnWalkable; } + else if (ci.LastKnownContactPlaneValid) + { + // L.2.3c: current contact lost transiently (e.g. StepUpSlide + // cleared it during a failed step-up) but the prior frame's + // contact is still valid — keep the mover grounded via the + // last-known plane. Without this, every wall bump dropped the + // player into the falling animation for one frame. + oi.State |= ObjectInfoState.Contact; + if (ci.LastKnownContactPlane.Normal.Z >= PhysicsGlobals.LandingZ) + oi.State |= ObjectInfoState.OnWalkable; + else + oi.State &= ~ObjectInfoState.OnWalkable; + } else { oi.State &= ~(ObjectInfoState.Contact | ObjectInfoState.OnWalkable); diff --git a/tests/AcDream.Core.Tests/Physics/BSPStepUpTests.cs b/tests/AcDream.Core.Tests/Physics/BSPStepUpTests.cs index e6f93a2..258fd58 100644 --- a/tests/AcDream.Core.Tests/Physics/BSPStepUpTests.cs +++ b/tests/AcDream.Core.Tests/Physics/BSPStepUpTests.cs @@ -430,6 +430,82 @@ public class BSPStepUpTests "Expected Collide flag set when airborne sphere hits slope (L.2.2)"); } + // ========================================================================= + // Group D — Phase L.2.3 regression tests + // + // Bugs caught by live testing 2026-04-29: + // D1 — walking into a too-tall wall must NOT clear ContactPlane (animation + // flickers to "falling" when contact is lost mid-step against a wall). + // D2 — Path 5 step-up must NOT recurse infinitely against a tall wall + // (retail guards step_sphere_up with `if (sp.step_up == 0)` per + // acclient_2013_pseudo_c.txt:272954). Without the guard, DoStepUp + // invokes DoStepDown which TransitionalInsert(5)'s into FindObjCollisions + // which hits the same wall AGAIN → recursive DoStepUp. + // ========================================================================= + + /// + /// L.2.3c regression: a grounded mover walking into a too-tall wall must + /// retain its ground contact across the failed step-up. Before the fix, + /// DoStepUp cleared + /// unconditionally; on failure, RestoreCheckPos restored the position but + /// the contact plane stayed cleared, causing OnWalkable to drop and the + /// animation system to interpret the stuck-against-wall state as "airborne". + /// + [Fact] + public void D1_GroundedMover_TooTallWall_PreservesContactPlane() + { + var (root, resolved) = BSPStepUpFixtures.TallWall(); + + // Foot at z=0, walking into the wall. + var from = new Vector3(0.1f, 0f, 0f); + var to = new Vector3(0.6f, 0f, 0f); + + // StepUpHeight 0.04m — too small to climb the 5m wall. + var t = BSPStepUpFixtures.MakeGroundedTransition(from, to, stepUpHeight: 0.04f); + var engine = MakeTestEngine(root, resolved, terrainZ: 0f); + + t.FindTransitionalPosition(engine); + + // After failed step-up + slide, the mover should still be considered + // grounded — either via the live contact plane, the last-known one, + // or the OnWalkable flag preserved by terrain re-detection. + bool stillGrounded = t.CollisionInfo.ContactPlaneValid + || t.CollisionInfo.LastKnownContactPlaneValid + || t.ObjectInfo.State.HasFlag(ObjectInfoState.OnWalkable); + Assert.True(stillGrounded, + "Expected mover to still be grounded after walking into a too-tall " + + "wall (failed step-up should preserve LastKnownContactPlane)."); + } + + /// + /// L.2.3b regression: Path 5 dispatch must be guarded against re-entry while + /// a step-up is already in progress. Test runs FindTransitionalPosition + /// with a tight time budget and verifies it terminates cleanly. Without the + /// guard the recursive DoStepUp churns the contact plane until numAttempts + /// runs out — finishing in an inconsistent state. + /// + [Fact] + public void D2_GroundedMover_TallWall_DoesNotRecurseInfinitely() + { + var (root, resolved) = BSPStepUpFixtures.TallWall(); + + var from = new Vector3(0.1f, 0f, 0f); + var to = new Vector3(0.6f, 0f, 0f); + + var t = BSPStepUpFixtures.MakeGroundedTransition(from, to, stepUpHeight: 0.04f); + var engine = MakeTestEngine(root, resolved, terrainZ: 0f); + + var sw = System.Diagnostics.Stopwatch.StartNew(); + t.FindTransitionalPosition(engine); + sw.Stop(); + + // Bounded execution: even with recursion, this is a 4-step movement. + // 100ms is generous; without the guard, recursion adds noticeable cost. + Assert.True(sw.ElapsedMilliseconds < 100, + $"Step-up against tall wall took {sw.ElapsedMilliseconds}ms — " + + "indicates Path 5 recursing through DoStepUp without guard."); + } + // ========================================================================= // Helpers // =========================================================================