fix(ai): creatures pathfind around concave obstacles instead of oscillating (#2461)

## 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.

Projects/UOContent.Tests/Fixtures/PathfindingTestFixture.cs

@@ -35,6 +35,7 @@ public class PathfindingTestFixture : ICollectionFixture<PathfindingTestFixture>
         Timer.Init(0);
         RaceDefinitions.Configure();
         MovementImpl.Configure();
+        PathFollower.Configure();
         World.Load();
         World.ExitSerializationThreads();
         DecayScheduler.Configure();

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<ushort>(0, id);
+        var ring = new List<Item>();
+        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<ushort>(0, id);
+        var wall = new List<Item>();
+        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");
+    }
+}

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;
+                }
             }
         }
 

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
         }
     }
 
+    /// <summary>
+    /// Centralized "move toward <paramref name="target"/> until within
+    /// <paramref name="range"/>" 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.
+    /// </summary>
+    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;
+    }
+
+    /// <summary>
+    /// 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 <see cref="ApproachGiveUpTicks"/> 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.
+    /// </summary>
+    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;
+        }
+    }
+
+    /// <summary>Clears all approach state (called on arrival, real greedy progress, or when
+    /// a given-up goal moves).</summary>
+    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;