fix(physics): L.2.3c — preserve contact plane through failed step-up
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) <noreply@anthropic.com>
This commit is contained in:
parent
3789491394
commit
d2f6067960
2 changed files with 125 additions and 3 deletions
|
|
@ -1356,6 +1356,19 @@ public sealed class Transition
|
||||||
var ci = CollisionInfo;
|
var ci = CollisionInfo;
|
||||||
var oi = ObjectInfo;
|
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.ContactPlaneValid = false;
|
||||||
ci.ContactPlaneIsWater = false;
|
ci.ContactPlaneIsWater = false;
|
||||||
|
|
||||||
|
|
@ -1381,8 +1394,21 @@ public sealed class Transition
|
||||||
sp.WalkableValid = false;
|
sp.WalkableValid = false;
|
||||||
|
|
||||||
if (!stepDown)
|
if (!stepDown)
|
||||||
|
{
|
||||||
sp.RestoreCheckPos();
|
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;
|
return stepDown;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -1498,11 +1524,18 @@ public sealed class Transition
|
||||||
ci.SetSlidingNormal(ci.CollisionNormal);
|
ci.SetSlidingNormal(ci.CollisionNormal);
|
||||||
|
|
||||||
// Preserve contact plane for next step.
|
// 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)
|
if (ci.ContactPlaneValid)
|
||||||
{
|
{
|
||||||
ci.LastKnownContactPlane = ci.ContactPlane;
|
ci.LastKnownContactPlaneValid = true;
|
||||||
ci.LastKnownContactPlaneCellId = ci.ContactPlaneCellId;
|
ci.LastKnownContactPlane = ci.ContactPlane;
|
||||||
|
ci.LastKnownContactPlaneCellId = ci.ContactPlaneCellId;
|
||||||
ci.LastKnownContactPlaneIsWater = ci.ContactPlaneIsWater;
|
ci.LastKnownContactPlaneIsWater = ci.ContactPlaneIsWater;
|
||||||
|
|
||||||
oi.State |= ObjectInfoState.Contact;
|
oi.State |= ObjectInfoState.Contact;
|
||||||
|
|
@ -1511,6 +1544,19 @@ public sealed class Transition
|
||||||
else
|
else
|
||||||
oi.State &= ~ObjectInfoState.OnWalkable;
|
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
|
else
|
||||||
{
|
{
|
||||||
oi.State &= ~(ObjectInfoState.Contact | ObjectInfoState.OnWalkable);
|
oi.State &= ~(ObjectInfoState.Contact | ObjectInfoState.OnWalkable);
|
||||||
|
|
|
||||||
|
|
@ -430,6 +430,82 @@ public class BSPStepUpTests
|
||||||
"Expected Collide flag set when airborne sphere hits slope (L.2.2)");
|
"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.
|
||||||
|
// =========================================================================
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// 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,
|
||||||
|
/// <c>DoStepUp</c> cleared <see cref="CollisionInfo.ContactPlaneValid"/>
|
||||||
|
/// 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".
|
||||||
|
/// </summary>
|
||||||
|
[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).");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// L.2.3b regression: Path 5 dispatch must be guarded against re-entry while
|
||||||
|
/// a step-up is already in progress. Test runs <c>FindTransitionalPosition</c>
|
||||||
|
/// 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.
|
||||||
|
/// </summary>
|
||||||
|
[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
|
// Helpers
|
||||||
// =========================================================================
|
// =========================================================================
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue