From cff9fbda2945faad632bae84c51f73933c750729 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Sat, 6 Jun 2026 10:20:52 -0700 Subject: [PATCH] fix(ai): creatures pathfind around concave obstacles instead of oscillating (#2461) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Creatures (pets following, monsters chasing, NPCs approaching) would **oscillate — "pace back and forth really fast"** at concave obstacles (reported at the Britain Inn L-desk: a pet at `(1493,1614,20)` never reaching its master at `(1494,1605,21)`) instead of routing around them. **Root cause** (the A* pathfinder itself was correct): all goal-seeking funnels through `MoveTo` and `WalkMobileRange` → `MoveTowardsOrAwayFrom`, which step greedily via `DoMove(dir, badStateOk:true)`. `DoMove` returns `true` even when the direct step was blocked and the creature merely **auto-turned and sidestepped** (`MoveResult.SuccessAutoTurn`), and the caller then set `Path = null`, discarding the `PathFollower`. So at a concave obstacle a non-progressing sidestep was mistaken for progress and the creature never committed to a route. (AOS pet-follow runs at `CurrentSpeed = 0.1`, hence the "really fast" shuffle.) ## What changed - **New centralized `BaseAI.ApproachTarget(target, run, range)` primitive.** A greedy step is committed only when it **fully succeeds (`MoveResult.Success`) and actually gets closer**; otherwise the creature commits to a **persistent `PathFollower`** that routes around the obstacle and is never discarded by a greedy step. The open-terrain fast path (one greedy step, no pathfinding) is preserved. `MoveTo`, `MoveTowardsOrAwayFrom`, and `MoveToWithCollisionAvoidance` all delegate to it — public signatures unchanged, so no AI-class call site changes. - **Best-distance give-up + idle.** A creature that cannot reach a **stationary** in-range goal stops shuffling and idles after `ApproachGiveUpTicks` (40) ticks without lowering its closest-ever distance; a **moving** goal (active chase) never gives up. It resumes the moment the goal moves. - **Pathfinder fix (required):** `BitmapAStarAlgorithm.IsBlockedByDynamic` now skips the dynamic mobile-block check **at the goal cell only** (`MoveImpl.Goal`). Previously A* returned `null` whenever the target mobile stood on the goal cell, so creatures could never pathfind *toward* another mobile — only toward empty ground. The follower stops within `range` short of it. Static/item blocking and all non-goal mobile blocking are unchanged. ## Tests New AI-loop integration tests in `ApproachTargetTests.cs` drive the real `BaseAI` primitives against live Britain Inn map statics: exact-repro pet follow, open-terrain (asserts zero pathfinding), `MoveTo` chase (static + walking-away target), route-around-a-dynamic-wall, and walled-off give-up-and-idle. - Pathfinding + AI subset: **52/52** pass. - Full `UOContent.Tests`: **301/301** pass. (Note: the test host lingers on shutdown — a pre-existing infra quirk unrelated to this change; all tests complete and pass.) - Full solution build: clean (0 warnings / 0 errors). ## Notes - Branched off `main`; independent of the in-flight step-cache work. - Out of scope (future work): proactive "SmartAI" look-ahead pathfinding so clever creatures plan a route before walking into the obstacle, rather than reacting after they hit it. ## Test Plan - [X] In-game: order a pet to `follow`/`come` across the Britain Inn L-desk; confirm it routes around and reaches you instead of pacing. - [X] Aggro a monster and kite it around a building/treeline; confirm it chases around obstacles. - [X] Confirm open-terrain following/chasing feels unchanged (no extra latency). - [X] Confirm a creature with a genuinely unreachable target idles rather than shuffling forever. --- .../Fixtures/PathfindingTestFixture.cs | 1 + .../Tests/Mobiles/AI/ApproachTargetTests.cs | 283 ++++++++++++++++++ .../Engines/Pathing/BitmapAStarAlgorithm.cs | 24 +- .../UOContent/Mobiles/AI/BaseAI/AIMovement.cs | 217 ++++++++++---- 4 files changed, 460 insertions(+), 65 deletions(-) create mode 100644 Projects/UOContent.Tests/Tests/Mobiles/AI/ApproachTargetTests.cs diff --git a/Projects/UOContent.Tests/Fixtures/PathfindingTestFixture.cs b/Projects/UOContent.Tests/Fixtures/PathfindingTestFixture.cs index 598aac653..ee761d1c0 100644 --- a/Projects/UOContent.Tests/Fixtures/PathfindingTestFixture.cs +++ b/Projects/UOContent.Tests/Fixtures/PathfindingTestFixture.cs @@ -35,6 +35,7 @@ public class PathfindingTestFixture : ICollectionFixture Timer.Init(0); RaceDefinitions.Configure(); MovementImpl.Configure(); + PathFollower.Configure(); World.Load(); World.ExitSerializationThreads(); DecayScheduler.Configure(); diff --git a/Projects/UOContent.Tests/Tests/Mobiles/AI/ApproachTargetTests.cs b/Projects/UOContent.Tests/Tests/Mobiles/AI/ApproachTargetTests.cs new file mode 100644 index 000000000..fa3eb23bd --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Mobiles/AI/ApproachTargetTests.cs @@ -0,0 +1,283 @@ +using System.Collections.Generic; +using Server.Engines.Pathing.Cache; +using Server.Mobiles; +using Xunit; + +namespace Server.Tests.Mobiles.AI; + +// AI movement tests drive the real BaseAI primitives against live map statics. The +// pathfinder uses non-reentrant static buffers, so we share the pathfinding sequential +// collection to avoid parallel interference. +[Collection("Sequential Pathfinding Tests")] +public class ApproachTargetTests +{ + private sealed class FollowerStub : BaseCreature + { + // Serial ctor + DefaultMobileInit bypasses NPCSpeeds JSON (absent in tests). + public FollowerStub(Serial serial) : base(serial) => Body = 0xC9; + } + + private sealed class TargetStub : Mobile + { + public TargetStub() => Body = 0xC9; + } + + private static (FollowerStub bc, BaseAI ai) NewFollower(Map map, Point3D loc) + { + var bc = new FollowerStub(World.NewMobile); + bc.DefaultMobileInit(); + bc.MoveToWorld(loc, map); + BaseAI ai = new AnimalAI(bc); + ai.AITimer?.Stop(); // we drive movement manually; no background timer + return (bc, ai); + } + + // Drives the pet follow primitive once per tick; returns true once it reaches within + // arriveDist tiles. Uses InRange (Chebyshev) to match the game's own range semantics — + // a diagonal neighbor at Euclidean ~2.24 is "within 2" to the engine. + private static bool DriveFollow(BaseAI ai, Mobile bc, Mobile target, int arriveDist, int maxTicks) + { + for (var i = 0; i < maxTicks; i++) + { + ai.NextMove = 0; + ai.WalkMobileRange(target, 1, false, 1, 2); + if (bc.InRange(target, arriveDist)) + { + return true; + } + } + return false; + } + + private static ushort FirstImpassableItemId() + { + for (ushort id = 1; id < TileData.MaxItemValue; id++) + { + if (TileData.ItemTable[id].ImpassableSurface) + { + return id; + } + } + return 0; + } + + [Fact] + public void OpenTerrain_ReachesTarget_WithoutPathfinding() + { + var map = Map.Maps[1]; + Assert.NotNull(map); + map.GetAverageZ(1500, 1600, out _, out var z, out _); + + var (bc, ai) = NewFollower(map, new Point3D(1500, 1600, (sbyte)z)); + var target = new TargetStub(); + target.MoveToWorld(new Point3D(1495, 1600, (sbyte)z), map); // 5 tiles west, open + + StepCache.Instance.Clear(); + var buildsBefore = StepCache.Instance.GetStats().BuildsTotal; + + var arrived = DriveFollow(ai, bc, target, 2, 40); + + var buildsAfter = StepCache.Instance.GetStats().BuildsTotal; + bc.Delete(); + target.Delete(); + + Assert.True(arrived, "creature should reach an open-terrain target by stepping"); + Assert.Equal(buildsBefore, buildsAfter); // greedy fast path: no chunk builds + } + + [Fact] + public void BritainInnDesk_PetReachesMaster() + { + var map = Map.Maps[1]; + Assert.NotNull(map); + + // Exact in-game repro: pet south of the L-desk, master north of it. + var (bc, ai) = NewFollower(map, new Point3D(1493, 1614, 20)); + var target = new TargetStub(); + target.MoveToWorld(new Point3D(1494, 1605, 21), map); + + StepCache.Instance.Clear(); + + // 200 ticks is generous for the ~17-step detour at one step per tick. + var arrived = DriveFollow(ai, bc, target, 2, 200); + + bc.Delete(); + target.Delete(); + + Assert.True(arrived, "pet must navigate around the L-desk to reach the master"); + } + + [Fact] + public void MoveTo_ReachesTargetAroundDesk() + { + var map = Map.Maps[1]; + Assert.NotNull(map); + + var (bc, ai) = NewFollower(map, new Point3D(1493, 1614, 20)); + var target = new TargetStub(); + target.MoveToWorld(new Point3D(1494, 1605, 21), map); + + StepCache.Instance.Clear(); + + var arrived = false; + for (var i = 0; i < 200; i++) + { + ai.NextMove = 0; + ai.MoveTo(target, false, 1); + if (bc.InRange(target, 1)) + { + arrived = true; + break; + } + } + + bc.Delete(); + target.Delete(); + + Assert.True(arrived, "MoveTo chaser must reach the target around the desk"); + } + + [Fact] + public void MoveTo_CatchesTargetWalkingAway_OpenTerrain() + { + var map = Map.Maps[1]; + Assert.NotNull(map); + map.GetAverageZ(1500, 1600, out _, out var z, out _); + + var (bc, ai) = NewFollower(map, new Point3D(1500, 1600, (sbyte)z)); + var target = new TargetStub(); + target.MoveToWorld(new Point3D(1498, 1600, (sbyte)z), map); + + StepCache.Instance.Clear(); + + var caught = false; + for (var i = 0; i < 60; i++) + { + ai.NextMove = 0; + ai.MoveTo(target, true, 1); + + // Target walks west every other tick for its first several steps, then stops, + // so a same-speed chaser eventually closes the gap. + if (i % 2 == 0 && i < 16 && target.X > 1490) + { + target.MoveToWorld(new Point3D(target.X - 1, target.Y, target.Z), map); + } + + if (bc.InRange(target, 1)) + { + caught = true; + break; + } + } + + bc.Delete(); + target.Delete(); + + Assert.True(caught, "chaser must catch a target that walks away then stops"); + } + + [Fact] + public void UnreachableTarget_GivesUp_AndIdles() + { + var map = Map.Maps[1]; + Assert.NotNull(map); + map.GetAverageZ(1500, 1601, out _, out var z, out _); + + var (bc, ai) = NewFollower(map, new Point3D(1500, 1601, (sbyte)z)); + var target = new TargetStub(); + target.MoveToWorld(new Point3D(1500, 1596, (sbyte)z), map); + + // Full impassable ring around the target cell -> no path to within range 1. + var id = FirstImpassableItemId(); + Assert.NotEqual(0, id); + var ring = new List(); + for (var x = 1499; x <= 1501; x++) + { + for (var y = 1595; y <= 1597; y++) + { + if (x == 1500 && y == 1596) + { + continue; // leave the target's own cell + } + map.GetAverageZ(x, y, out _, out var rz, out _); + ring.Add(new Item(World.NewItem) + { + ItemID = id, Map = map, Location = new Point3D(x, y, (sbyte)rz) + }); + } + } + + StepCache.Instance.Clear(); + + // Run well past the stuck window so give-up engages. + for (var i = 0; i < 120; i++) + { + ai.NextMove = 0; + ai.MoveTo(target, false, 1); + } + + // After giving up, the creature must idle (not oscillate) while the goal is still. + var idleStart = bc.Location; + var stayedIdle = true; + for (var i = 0; i < 20; i++) + { + ai.NextMove = 0; + ai.MoveTo(target, false, 1); + if (bc.Location != idleStart) + { + stayedIdle = false; + break; + } + } + + var reached = bc.InRange(target, 1); + + bc.Delete(); + target.Delete(); + foreach (var it in ring) + { + it.Delete(); + } + + Assert.False(reached, "walled-off target must not be reached"); + Assert.True(stayedIdle, "after giving up, the creature must idle, not shuffle"); + } + + [Fact] + public void WallBetween_RoutesAround_ReachesTarget() + { + var map = Map.Maps[1]; + Assert.NotNull(map); + map.GetAverageZ(1500, 1600, out _, out var z, out _); + + var (bc, ai) = NewFollower(map, new Point3D(1500, 1600, (sbyte)z)); + var target = new TargetStub(); + target.MoveToWorld(new Point3D(1500, 1595, (sbyte)z), map); // 5 tiles north + + // Impassable wall on Y=1598 from X=1497..1503: blocks the straight line; ends open. + var id = FirstImpassableItemId(); + Assert.NotEqual(0, id); + var wall = new List(); + for (var x = 1497; x <= 1503; x++) + { + map.GetAverageZ(x, 1598, out _, out var wz, out _); + wall.Add(new Item(World.NewItem) + { + ItemID = id, Map = map, Location = new Point3D(x, 1598, (sbyte)wz) + }); + } + + StepCache.Instance.Clear(); + + var arrived = DriveFollow(ai, bc, target, 2, 200); + + bc.Delete(); + target.Delete(); + foreach (var it in wall) + { + it.Delete(); + } + + Assert.True(arrived, "creature must route around the wall to reach the target"); + } +} diff --git a/Projects/UOContent/Engines/Pathing/BitmapAStarAlgorithm.cs b/Projects/UOContent/Engines/Pathing/BitmapAStarAlgorithm.cs index 8357040ba..af4a91dc0 100644 --- a/Projects/UOContent/Engines/Pathing/BitmapAStarAlgorithm.cs +++ b/Projects/UOContent/Engines/Pathing/BitmapAStarAlgorithm.cs @@ -459,16 +459,24 @@ public class BitmapAStarAlgorithm : PathAlgorithm } } - foreach (var mob in map.GetMobilesAt(x, y)) - { - if (mob == m) - { - continue; - } + // A* must be able to plan a path to the goal cell even when the target mobile is + // standing on it (the follower stops within range short of it). Skip the mob-block + // check at the goal cell ONLY; everywhere else dynamic mobiles still block. + var skipMobCheck = x == MoveImpl.Goal.X && y == MoveImpl.Goal.Y; - if (mob.Z + MobileHeight > z && z + MobileHeight > mob.Z && !CanMoveOver(m, mob)) + if (!skipMobCheck) + { + foreach (var mob in map.GetMobilesAt(x, y)) { - return true; + if (mob == m) + { + continue; + } + + if (mob.Z + MobileHeight > z && z + MobileHeight > mob.Z && !CanMoveOver(m, mob)) + { + return true; + } } } diff --git a/Projects/UOContent/Mobiles/AI/BaseAI/AIMovement.cs b/Projects/UOContent/Mobiles/AI/BaseAI/AIMovement.cs index 12fa56352..1063dd70a 100644 --- a/Projects/UOContent/Mobiles/AI/BaseAI/AIMovement.cs +++ b/Projects/UOContent/Mobiles/AI/BaseAI/AIMovement.cs @@ -22,6 +22,21 @@ namespace Server.Mobiles; public abstract partial class BaseAI { + // --- Centralized progress-based approach state (see ApproachTarget) --------------- + // Consecutive move-eligible ticks a creature may fail to improve its best distance to a + // STATIONARY goal before it gives up and idles. A moving goal (an active chase) never + // triggers give-up. Must exceed the longest no-improvement stretch of a valid detour + // (the Britain Inn detour's is ~13 ticks), with margin; this also bounds the largest + // concave detour a creature will navigate before idling on a stationary goal. + private const int ApproachGiveUpTicks = 40; + + private Mobile _approachGoal; + private Point3D _approachGoalLoc; + private double _approachBestDist; + private int _approachStallTicks; + private bool _approachGaveUp; + private Point3D _approachGaveUpGoalLoc; + public static double BadlyHurtMoveDelay(BaseCreature bc) { var statMin = Core.HS ? bc.Stam : bc.Hits; @@ -279,6 +294,138 @@ public abstract partial class BaseAI } } + /// + /// Centralized "move toward until within + /// " decision shared by MoveTo and WalkMobileRange. A greedy + /// step is taken only when it actually gets the creature closer; a blocked step or an + /// auto-turn sidestep that made no progress falls through to a persistent PathFollower + /// that routes around the obstacle and is never discarded by a greedy step. A + /// best-distance stall counter idles the creature if an in-range goal is genuinely + /// unreachable, without ever abandoning a real chase or detour. + /// + protected bool ApproachTarget(Mobile target, bool run, int range) + { + if (Mobile.Deleted || Mobile.DisallowAllMoves || target?.Deleted != false) + { + return false; + } + + if (Mobile.InRange(target, range)) + { + ResetApproach(); + return true; + } + + // Already gave up on this exact (unreachable) goal: idle until it moves. + if (_approachGaveUp && _approachGoal == target) + { + if (target.Location == _approachGaveUpGoalLoc) + { + return false; + } + + ResetApproach(); // target moved — try again fresh + } + + // FAST PATH: greedy step toward the target, counted as success ONLY when the move + // fully succeeded (not an auto-turn sidestep) and actually got us closer. An + // auto-turn sidestep can reduce Euclidean distance while moving in the wrong + // direction (e.g., east when the true route requires going south-first around a + // concave obstacle); treating it as progress would discard a PathFollower that is + // the only way to navigate. A blocked step or a non-Success result falls through to + // the planner immediately. + if (Path == null && Mobile.InLOS(target)) + { + var distBefore = Mobile.GetDistanceToSqrt(target); + var res = DoMoveImpl(Mobile.GetDirectionTo(target, run), true); + + if (res == MoveResult.BadState) + { + return false; // not allowed to move this tick; not a stall + } + + if (res == MoveResult.Success && Mobile.GetDistanceToSqrt(target) < distBefore) + { + ResetApproach(); + return Mobile.InRange(target, range); + } + // else: fall through; let the PathFollower route around the obstacle. + } + + // PLANNING PATH: a persistent PathFollower, never discarded by a greedy step. + if (Path == null || Path.Goal != target) + { + Path = new PathFollower(Mobile, target) { Mover = DoMoveImpl }; + } + + if (Path.Follow(run, range)) + { + ResetApproach(); + return true; + } + + TrackApproachProgress(target); + return false; + } + + /// + /// Best-distance stuck detection. A creature making real headway keeps lowering its + /// closest-ever distance to the goal (a detour's outbound leg pauses that, but it + /// resumes once the creature rounds the obstacle). A creature that cannot reach a + /// STATIONARY goal never lowers it and, after ticks, + /// gives up and idles. A MOVING goal (an active chase) resets the baseline every tick, + /// so chases never give up even when the gap holds constant. + /// + private void TrackApproachProgress(Mobile target) + { + if (!CanMoveNow(out _)) + { + return; // a not-yet-due move (stun) is not a stall + } + + var dist = Mobile.GetDistanceToSqrt(target); + var goalLoc = target.Location; + + // New goal, or the goal moved (active chase): reset the stall baseline. Clearing the + // give-up flag here prevents a prior goal's give-up state from leaking onto a new one. + if (_approachGoal != target || goalLoc != _approachGoalLoc) + { + _approachGoal = target; + _approachGoalLoc = goalLoc; + _approachBestDist = dist; + _approachStallTicks = 0; + _approachGaveUp = false; + return; + } + + // Stationary goal: getting closer than ever resets the stall. + if (dist < _approachBestDist) + { + _approachBestDist = dist; + _approachStallTicks = 0; + return; + } + + if (++_approachStallTicks >= ApproachGiveUpTicks) + { + _approachGaveUp = true; + _approachGaveUpGoalLoc = goalLoc; + Path = null; + } + } + + /// Clears all approach state (called on arrival, real greedy progress, or when + /// a given-up goal moves). + private void ResetApproach() + { + Path = null; + _approachGoal = null; + _approachGoalLoc = Point3D.Zero; + _approachBestDist = 0; + _approachStallTicks = 0; + _approachGaveUp = false; + } + public virtual bool MoveTo(Mobile m, bool run, int range) { if (Mobile.Deleted || Mobile.DisallowAllMoves || m?.Deleted != false) @@ -293,7 +440,7 @@ public abstract partial class BaseAI if (Mobile.InRange(m, range)) { - Path = null; + ResetApproach(); return true; } @@ -302,23 +449,7 @@ public abstract partial class BaseAI return MoveToWithGroup(this, m, shouldRun, range); } - if (Path == null && Mobile.InLOS(m) && DoMove(Mobile.GetDirectionTo(m), true)) - { - return true; - } - - if (Path?.Goal != m) - { - Path = new PathFollower(Mobile, m) { Mover = DoMoveImpl }; - } - - if (Path.Follow(shouldRun, 1)) - { - Path = null; - return true; - } - - return false; + return ApproachTarget(m, shouldRun, range); } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -358,18 +489,9 @@ public abstract partial class BaseAI } } - if (Path?.Goal != target) - { - Path = new PathFollower(Mobile, target) { Mover = DoMoveImpl }; - } - - if (Path.Follow(shouldRun, 1)) - { - Path = null; - return true; - } - - return false; + // Tactical sidesteps exhausted — route around the obstacle via the centralized + // approach primitive (persistent PathFollower, no oscillation). + return ApproachTarget(target, shouldRun, range); } public virtual bool WalkMobileRange(Mobile m, int iSteps, bool run, int iWantDistMin, int iWantDistMax) @@ -405,36 +527,17 @@ public abstract partial class BaseAI { var shouldRun = run && iCurrDist > 5; - var needCloser = iCurrDist > iWantDistMax; - - if (needCloser && m != null && Path?.Goal == m) + if (iCurrDist > iWantDistMax) { - if (Path.Follow(shouldRun, 1)) - { - Path = null; - return true; - } + // Too far: approach via the centralized progress-based primitive. + return ApproachTarget(m, shouldRun, iWantDistMax); } - else + + // Too close: back away. Retreat keeps the simple greedy behavior (out of scope). + if (DoMove(m.GetDirectionTo(Mobile, shouldRun), true)) { - var dirTo = needCloser ? Mobile.GetDirectionTo(m, shouldRun) : m.GetDirectionTo(Mobile, shouldRun); - - if (DoMove(dirTo, true)) - { - Path = null; - return true; - } - - if (needCloser) - { - Path = new PathFollower(Mobile, m) { Mover = DoMoveImpl }; - - if (Path.Follow(shouldRun, 1)) - { - Path = null; - return true; - } - } + Path = null; + return true; } return false;