From 3be700020b82a7be55da00eeac3e6ff18ce07778 Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 18 May 2026 09:33:33 +0200 Subject: [PATCH] =?UTF-8?q?fix(physics):=20close=20#77=20=E2=80=94=20auto-?= =?UTF-8?q?walk=20honors=20ACE=20CanCharge=20bit;=20zero=20velocity=20in?= =?UTF-8?q?=20turn-in-place?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../Input/PlayerMovementController.cs | 87 +++++++++++-------- src/AcDream.App/Rendering/GameWindow.cs | 31 ++++--- src/AcDream.Core.Net/Messages/CreateObject.cs | 22 +++++ 3 files changed, 93 insertions(+), 47 deletions(-) diff --git a/src/AcDream.App/Input/PlayerMovementController.cs b/src/AcDream.App/Input/PlayerMovementController.cs index dab9138..8d4ab44 100644 --- a/src/AcDream.App/Input/PlayerMovementController.cs +++ b/src/AcDream.App/Input/PlayerMovementController.cs @@ -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; } /// @@ -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; } diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index d1b0bef..e35a85e 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -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) diff --git a/src/AcDream.Core.Net/Messages/CreateObject.cs b/src/AcDream.Core.Net/Messages/CreateObject.cs index 971c726..48b678d 100644 --- a/src/AcDream.Core.Net/Messages/CreateObject.cs +++ b/src/AcDream.Core.Net/Messages/CreateObject.cs @@ -203,6 +203,28 @@ public static class CreateObject /// public bool MoveTowards => MoveToParameters.HasValue && (MoveToParameters.Value & 0x200u) != 0; + + /// + /// MovementParameters bit 4 (mask 0x10) — set when the mover should + /// charge (run) rather than walk. ACE's + /// Creature.SetWalkRunThreshold sets this when the player-to- + /// target distance is at least WalkRunThreshold / 2 (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. + /// + /// Retail's MovementParameters::get_command + /// (0x0052aa00) 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 HoldKey_Run. + /// + /// + /// Cross-ref: ACE MovementParams.CanCharge = 0x10 + /// (ACE.Entity/Enum/MovementParams.cs:12). + /// + /// + public bool CanCharge => MoveToParameters.HasValue + && (MoveToParameters.Value & 0x10u) != 0; } ///