fix(physics): close #77 — auto-walk honors ACE CanCharge bit; zero velocity in turn-in-place
Two related close-range bugs reported in #77 share a root in PlayerMovementController.DriveServerAutoWalk + BeginServerAutoWalk: 1. **Walk-vs-run misclassification.** BeginServerAutoWalk decided `_autoWalkInitiallyRunning = (initialDist - distanceToObject) >= 1.0f`, forcing run at any chase past ~1.6 m. ACE's wire-level walk-vs-run answer is the MovementParameters CanCharge bit (0x10), which Creature.SetWalkRunThreshold sets when server-side player→target distance >= WalkRunThreshold/2 (= 7.5 m default). Retail's MovementParameters::get_command (decomp 0x0052aa00) gates the run path on CanCharge first; the inner walk_run_threshold check practically always walks given ACE's 15 m default. The hardcoded 1.0 m threshold pushed run into the 3-5 m walk-range the user reported should walk. 2. **Velocity leak in turn-in-place phase.** When the auto-walked body crossed the destination, desiredYaw flipped ~180°, walkAligned dropped to false, and the `if (!moveForward) return true;` branch returned without zeroing body velocity. The body kept the prior frame's running velocity (RunAnimSpeed × runRate ≈ 11 m/s) and slid 4-5 m past the target before the turn-around rotation completed — the "runs and slides away, runs back, picks up" symptom in #77 bug B. Changes: - `CreateObject.ServerMotionState.CanCharge`: new bool prop reading bit 0x10 of MoveToParameters. Cross-ref ACE MovementParams.CanCharge = 0x10. - `PlayerMovementController.BeginServerAutoWalk`: replaces the unused `walkRunThreshold` parameter with `bool canCharge`; sets `_autoWalkInitiallyRunning = canCharge`. - `PlayerMovementController.DriveServerAutoWalk` turn-in-place branch: calls `_motion.DoMotion(Ready, 1.0)` and zeros body horizontal velocity (preserving Z for gravity). No-op for case (a) initial-turn with stationary body; fixes (b) overshoot recovery and (c) settling cases. - `GameWindow.OnLiveMotionUpdated`: passes `update.MotionState.CanCharge` through; [autowalk-begin] trace shows `canCharge=` instead of `walkRunThresh=`. - `GameWindow.InstallSpeculativeTurnToTarget`: predicts ACE's CanCharge from local distance using ACE's exact 7.5 m rule, so the speculative install agrees with the wire-triggered overwrite that arrives moments later. Visual-verified at Holtburg 2026-05-18: walk-range NPC click walks + fires Use, walk-range F-key pickup walks + no overshoot, far-range (8-10 m) pickup still runs. Test baseline unchanged (8 Core pre-existing failures, 0 net-new failures across Core/Net/UI/App suites). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik
parent
677266d477
commit
3be700020b
3 changed files with 93 additions and 47 deletions
|
|
@ -430,7 +430,7 @@ public sealed class PlayerMovementController
|
|||
float minDistance,
|
||||
float distanceToObject,
|
||||
bool moveTowards,
|
||||
float walkRunThreshold)
|
||||
bool canCharge)
|
||||
{
|
||||
_autoWalkActive = true;
|
||||
_autoWalkDestination = destinationWorld;
|
||||
|
|
@ -438,35 +438,28 @@ public sealed class PlayerMovementController
|
|||
_autoWalkDistanceToObject = distanceToObject;
|
||||
_autoWalkMoveTowards = moveTowards;
|
||||
|
||||
// 2026-05-16 (retail-faithful) — one-shot walk/run decision
|
||||
// using retail's get_command formula (decomp 0x0052aa00):
|
||||
// running = (initialDist - distance_to_object) >= walk_run_threshhold
|
||||
// Subtract the use-radius from the raw distance so the
|
||||
// discriminator is "distance LEFT to walk" — same shape as
|
||||
// retail's `arg2 - distance_to_object` term. Held for the
|
||||
// rest of the auto-walk so the body runs all the way to the
|
||||
// target (or walks all the way), matching observed retail
|
||||
// behaviour.
|
||||
// Issue #77 fix (2026-05-18) — retail-faithful walk-vs-run.
|
||||
//
|
||||
// The wire-supplied walkRunThreshold defaults to 15m at
|
||||
// retail's MovementParameters constructor (0x005243b5) AND at
|
||||
// ACE's wire-layer MoveToParameters.cs:51 — but the user-
|
||||
// observed retail behaviour is "only walk when very close,"
|
||||
// run from any non-trivial distance. ACE's own physics layer
|
||||
// confirms: MovementParameters.cs:49-50 has the 15.0f default
|
||||
// commented out with `Default_WalkRunThreshold = 1.0f`
|
||||
// active, and Creature.cs:312 comments "default 15 distance
|
||||
// seems too far" before halving it to 7.5f for CanCharge.
|
||||
// We override the wire value with a retail-faithful 1.0m
|
||||
// constant; the wire value (typically 15m from ACE) is
|
||||
// ignored for the run/walk decision. _walkRunThreshold is in
|
||||
// METERS of remaining-distance-to-use-radius.
|
||||
const float RetailWalkRunThresholdMeters = 1.0f;
|
||||
float dx = destinationWorld.X - _body.Position.X;
|
||||
float dy = destinationWorld.Y - _body.Position.Y;
|
||||
float initialDist = MathF.Sqrt(dx * dx + dy * dy);
|
||||
float remainingAtStart = initialDist - distanceToObject;
|
||||
_autoWalkInitiallyRunning = remainingAtStart >= RetailWalkRunThresholdMeters;
|
||||
// Retail's MovementParameters::get_command (decomp 0x0052aa00)
|
||||
// gates run on the CanCharge flag (bit 0x10 of
|
||||
// MovementParameters). Cleared → fall through to the inner
|
||||
// walk_run_threshold check, which ACE's 15 m wire default +
|
||||
// 0.6 m use-radius makes practically always walk for any
|
||||
// chase under 15.6 m. Set → unconditional HoldKey_Run.
|
||||
//
|
||||
// ACE's Creature.SetWalkRunThreshold sets CanCharge when
|
||||
// (server-side player→target distance) >= WalkRunThreshold /
|
||||
// 2 (= 7.5 m for the 15 m default), and clears it otherwise.
|
||||
// The CanCharge bit IS the wire-side walk-vs-run answer; we
|
||||
// just relay it.
|
||||
//
|
||||
// Previously we hardcoded a 1.0 m threshold against
|
||||
// initialDist - distanceToObject, which forced run at any
|
||||
// chase past ~1.6 m — including the 3-5 m "walk range" the
|
||||
// user expected to walk in (issue #77 reproduction). Honoring
|
||||
// CanCharge restores the retail bucket: walk under ~7.5 m,
|
||||
// run beyond.
|
||||
_autoWalkInitiallyRunning = canCharge;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
|
@ -698,12 +691,36 @@ public sealed class PlayerMovementController
|
|||
|
||||
if (!moveForward)
|
||||
{
|
||||
// Turn-in-place phase. The motion state needs to be Ready
|
||||
// (or at least not RunForward) so we're not pretending to
|
||||
// run while standing. _motion stays at whatever the user-
|
||||
// input pipeline last set it to (typically Ready/null since
|
||||
// the user isn't pressing W). Don't touch body velocity
|
||||
// either — physics will handle gravity-only behaviour.
|
||||
// Turn-in-place phase. Two sub-cases land here:
|
||||
// (a) initial turn — body must rotate to face the target
|
||||
// before we drive forward (walkAligned == false at chain
|
||||
// start, body is stationary).
|
||||
// (b) overshoot recovery — body crossed the destination, so
|
||||
// desiredYaw flipped ~180° and walkAligned dropped to
|
||||
// false; body needs to turn around before walking back.
|
||||
// (c) settling — body is within use-radius but not aligned
|
||||
// enough to fire arrival (withinArrival == true,
|
||||
// !aligned); body holds position while finishing rotation
|
||||
// so the arrival predicate fires on the next tick.
|
||||
//
|
||||
// Issue #77 fix: explicitly zero horizontal velocity. Without
|
||||
// this, in case (b) the body keeps the prior frame's running
|
||||
// velocity (RunAnimSpeed × runRate ≈ 11 m/s) and slides past
|
||||
// the destination by several meters before the turn-around
|
||||
// rotation completes — the "runs and slides away, runs back,
|
||||
// picks up" symptom reported in issue #77 / bug B. Cases (a)
|
||||
// and (c) zero a velocity that's already zero, so the change
|
||||
// is a no-op there.
|
||||
//
|
||||
// The motion-interpreter state also has to step out of
|
||||
// WalkForward so get_state_velocity (used downstream) reports
|
||||
// standing-velocity, not the prior frame's run-speed.
|
||||
_motion.DoMotion(MotionCommand.Ready, 1.0f);
|
||||
if (_body.OnWalkable)
|
||||
{
|
||||
float savedWorldVz = _body.Velocity.Z;
|
||||
_body.set_local_velocity(new Vector3(0f, 0f, savedWorldVz));
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -3396,16 +3396,17 @@ public sealed class GameWindow : IDisposable
|
|||
pathData.OriginZ,
|
||||
_liveCenterX,
|
||||
_liveCenterY);
|
||||
bool canCharge = update.MotionState.CanCharge;
|
||||
_playerController.BeginServerAutoWalk(
|
||||
destWorld,
|
||||
pathData.MinDistance,
|
||||
pathData.DistanceToObject,
|
||||
update.MotionState.MoveTowards,
|
||||
pathData.WalkRunThreshold);
|
||||
canCharge);
|
||||
if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled)
|
||||
{
|
||||
Console.WriteLine(System.FormattableString.Invariant(
|
||||
$"[autowalk-begin] dest=({destWorld.X:F2},{destWorld.Y:F2},{destWorld.Z:F2}) minDist={pathData.MinDistance:F2} objDist={pathData.DistanceToObject:F2} walkRunThresh={pathData.WalkRunThreshold:F2} towards={update.MotionState.MoveTowards}"));
|
||||
$"[autowalk-begin] dest=({destWorld.X:F2},{destWorld.Y:F2},{destWorld.Z:F2}) minDist={pathData.MinDistance:F2} objDist={pathData.DistanceToObject:F2} canCharge={canCharge} towards={update.MotionState.MoveTowards}"));
|
||||
}
|
||||
}
|
||||
// Note: do NOT cancel auto-walk on a non-MoveTo motion
|
||||
|
|
@ -9412,21 +9413,27 @@ public sealed class GameWindow : IDisposable
|
|||
if ((odf & LargeFlatMask) != 0) useRadius = 2.0f;
|
||||
}
|
||||
|
||||
// Issue #77 fix (2026-05-18) — predict ACE's CanCharge bit
|
||||
// from local distance so the speculative auto-walk uses the
|
||||
// same walk/run as the wire-triggered overwrite that arrives
|
||||
// moments later. ACE's Creature.SetWalkRunThreshold sets
|
||||
// CanCharge when player→target distance >= WalkRunThreshold /
|
||||
// 2 = 7.5 m (the 15 m wire default halved). Match exactly so
|
||||
// the speculative install doesn't flip walk↔run when ACE's
|
||||
// MoveToObject broadcast overwrites it.
|
||||
const float AceCanChargeDistance = 7.5f;
|
||||
var bodyPos = _playerController.Position;
|
||||
float ddx = entity.Position.X - bodyPos.X;
|
||||
float ddy = entity.Position.Y - bodyPos.Y;
|
||||
float distToTarget = MathF.Sqrt(ddx * ddx + ddy * ddy);
|
||||
bool speculativeCanCharge = distToTarget >= AceCanChargeDistance;
|
||||
|
||||
_playerController.BeginServerAutoWalk(
|
||||
destinationWorld: entity.Position,
|
||||
minDistance: 0f,
|
||||
distanceToObject: useRadius,
|
||||
moveTowards: true,
|
||||
// 15 m matches ACE's MoveToParameters.SetDefaults
|
||||
// WalkRunThreshold for non-combat Use/PickUp paths.
|
||||
// Using 9999 here forced walk-mode for the brief window
|
||||
// between this speculative install and ACE's MovementType=6
|
||||
// overwrite — far targets briefly walked before switching
|
||||
// to run, which the user observed as "we only walk, not
|
||||
// running from the correct threshold". 15.0 lines up with
|
||||
// what ACE will send anyway, so the initial decision and
|
||||
// the overwrite agree.
|
||||
walkRunThreshold: 15.0f);
|
||||
canCharge: speculativeCanCharge);
|
||||
}
|
||||
|
||||
private uint? SelectClosestCombatTarget(bool showToast)
|
||||
|
|
|
|||
|
|
@ -203,6 +203,28 @@ public static class CreateObject
|
|||
/// </summary>
|
||||
public bool MoveTowards => MoveToParameters.HasValue
|
||||
&& (MoveToParameters.Value & 0x200u) != 0;
|
||||
|
||||
/// <summary>
|
||||
/// MovementParameters bit 4 (mask 0x10) — set when the mover should
|
||||
/// charge (run) rather than walk. ACE's
|
||||
/// <c>Creature.SetWalkRunThreshold</c> sets this when the player-to-
|
||||
/// target distance is at least <c>WalkRunThreshold / 2</c> (7.5 m
|
||||
/// for the 15 m default), and clears it for shorter chases — so this
|
||||
/// bit IS the wire-side walk-vs-run decision.
|
||||
/// <para>
|
||||
/// Retail's <c>MovementParameters::get_command</c>
|
||||
/// (<c>0x0052aa00</c>) gates the run path on this bit: cleared →
|
||||
/// fall through to the inner walk_run_threshold check (which ACE's
|
||||
/// 15 m default + 0.6 m use-radius makes practically always walk for
|
||||
/// any < 15.6 m chase); set → unconditional <c>HoldKey_Run</c>.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Cross-ref: ACE <c>MovementParams.CanCharge = 0x10</c>
|
||||
/// (<c>ACE.Entity/Enum/MovementParams.cs:12</c>).
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public bool CanCharge => MoveToParameters.HasValue
|
||||
&& (MoveToParameters.Value & 0x10u) != 0;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue