feat(Core/RPG): MoveFarTo loop detection with strategy flip + grinding throttle

Per-bot ring buffer of last 3 path attempts on RpgInfo. When 3 mmap or 3 nodetravel attempts to the same dest fail, force the alternative routing strategy on the next tick. When both strategies have failed 3 times each (bothExhausted), fall through to MoveFar:spline rather than flip-flopping forever. Also drops the 10%-per-tick opportunistic combat engage during do-quest travel — the multiplier (0.20x) is the right knob; the random yield was overriding it and producing the 'still grinding too much while traveling' symptom.

src/Ai/World/Rpg/Action/NewRpgAction.cpp

@@ -335,23 +335,16 @@ bool NewRpgDoQuestAction::DoIncompleteQuest(NewRpgInfo::DoQuest& data)
         if (HasNearbyQuestMob(15.0f))
             return false;
 
-        // Occasional yield so attack-anything can pick off a passing
-        // hostile. Gated on "hostile actually in range" so we don't
-        // burn ticks yielding into nothing, and rate-limited so we
-        // don't fight every mob we walk past — multiplier still
-        // dominates, this just opens an occasional window.
-        GuidVector nearbyTargets = AI_VALUE(GuidVector, "possible targets");
-        for (ObjectGuid guid : nearbyTargets)
-        {
-            Unit* u = botAI->GetUnit(guid);
-            if (!u || !u->IsAlive())
-                continue;
-            if (bot->GetDistance(u) > 25.0f)
-                continue;
-            if (urand(0, 9) == 0)  // 10% per tick when a hostile is in range
-                return false;
-            break;
-        }
+        // Note: previously yielded ~10%/tick when any hostile was
+        // within 25y. That overrode the do-quest multiplier in
+        // practice (combined with bots getting aggroed on the way,
+        // which ALSO bypasses the multiplier via combat engine) and
+        // bots ended up grinding their way to POIs instead of
+        // travelling. Quest-mob exception above is kept so we don't
+        // walk past a quest target while gathering. Anything else
+        // hostile is the multiplier's job to throttle — and bots
+        // that DO get aggroed switch to combat engine where the
+        // class strategy handles it.
 
         if (MoveFarTo(data.pos))
             return true;

src/Ai/World/Rpg/Action/NewRpgBaseAction.cpp

@@ -109,9 +109,42 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest)
     // stones" that aren't on the actual route.
     bool tryNodes = (dis >= nodeFirstDis && sPlayerbotAIConfig.enableTravelNodes);
 
+    // Loop-breaker: count recent attempts of each strategy to this
+    // dest. If 3 of one strategy → flip to the other. If both have
+    // failed 3 times each → both exhausted; fall through to
+    // MoveFar:spline and rely on UnstuckAction (5/10 min) for the
+    // eventual hearthstone-out. Without the "both exhausted" branch
+    // we'd flip-flop forever as the buffer evicts.
+    bool forceMmapOverNodes = false;  // 3 nodes failed -> try mmap
+    bool forceNodesOverMmap = false;  // 3 mmap failed -> try nodes
+    bool bothExhausted = false;
+    if (tryNodes)
+    {
+        int nodeFails = botAI->rpgInfo.CountRecentAttempts(dest, /*wasNodeTravel=*/true);
+        int mmapFails = botAI->rpgInfo.CountRecentAttempts(dest, /*wasNodeTravel=*/false);
+
+        if (nodeFails >= 3 && mmapFails >= 3)
+            bothExhausted = true;  // give up, spline at dest
+        else if (nodeFails >= 3)
+            forceMmapOverNodes = true;
+        else if (mmapFails >= 3)
+            forceNodesOverMmap = true;
+
+        if (forceMmapOverNodes || forceNodesOverMmap || bothExhausted)
+        {
+            // Drop the in-flight plan if any; we're about to flip
+            // (or give up). Buffer is intentionally NOT cleared so
+            // we remember which strategies have already been tried
+            // — otherwise we'd flip-flop indefinitely as the buffer
+            // evicts old entries.
+            if (botAI->rpgInfo.HasActiveTravelPlan())
+                botAI->rpgInfo.ClearTravel();
+        }
+    }
+
     // If a node plan is already active, ride it. The plan executor
     // owns its own per-step transitions.
-    if (tryNodes && botAI->rpgInfo.HasActiveTravelPlan())
+    if (tryNodes && !forceMmapOverNodes && !bothExhausted && botAI->rpgInfo.HasActiveTravelPlan())
         return UpdateTravelPlan();
 
     // 40-step chained mmap probe (cmangos getPathFromPath, ported in
@@ -124,10 +157,13 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest)
     std::vector<WorldPosition> probe = botPos.getPathTo(dest, bot);
     bool probeReachesDest = dest.isPathTo(probe, sPlayerbotAIConfig.spellDistance);
 
-    if (tryNodes && !probeReachesDest)
+    bool wantNodes = (tryNodes && !forceMmapOverNodes && !bothExhausted)
+                     && (!probeReachesDest || forceNodesOverMmap);
+    if (wantNodes)
     {
-        // Long-distance move and mmap couldn't get within spellDistance
-        // of the destination — commit to the travel-node graph
+        // Long-distance move and either mmap couldn't get within
+        // spellDistance OR we're forcing nodes after 3 failed mmap
+        // loops — commit to the travel-node graph
         // (cmangos TravelNode.cpp:1907 buildPath branch).
         StartTravelPlan(dest);
         if (botAI->rpgInfo.HasActiveTravelPlan())
@@ -137,6 +173,7 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest)
             // (TravelPlan:walk/segment/...) continue from the executor.
             EmitDebugMove("MoveFar:nodetravel",
                           dest.GetPositionX(), dest.GetPositionY(), dest.GetPositionZ());
+            botAI->rpgInfo.RecordMoveFarAttempt(dest, /*wasNodeTravel=*/true);
             return UpdateTravelPlan();
         }
         // else: graph returned no plan — fall through to mmap best-effort
@@ -144,7 +181,8 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest)
     else if (botAI->rpgInfo.HasActiveTravelPlan())
     {
         // mmap probe is now close enough OR we crossed below the
-        // node-first threshold — drop any leftover plan from a prior tick.
+        // node-first threshold OR we're forcing mmap — drop any
+        // leftover plan from a prior tick.
         botAI->rpgInfo.ClearTravel();
     }
 
@@ -154,7 +192,10 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest)
     // endpoint to MoveTo and let the motion master plan its own
     // spline. Functionally equivalent across multiple ticks
     // (incremental progress).
-    if (!probe.empty())
+    // Skip when both routing strategies have failed 3 times each —
+    // the probe is deterministic so it'd just lead back to the same
+    // dead end. Fall through to spline at the dest.
+    if (!probe.empty() && !bothExhausted)
     {
         WorldPosition stepDest = probe.back();
         float endDistToDest = dest.GetExactDist(stepDest.GetPositionX(),
@@ -163,6 +204,7 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest)
         {
             EmitDebugMove("MoveFar:mmap",
                           stepDest.GetPositionX(), stepDest.GetPositionY(), stepDest.GetPositionZ());
+            botAI->rpgInfo.RecordMoveFarAttempt(dest, /*wasNodeTravel=*/false);
             return MoveTo(bot->GetMapId(), stepDest.GetPositionX(), stepDest.GetPositionY(),
                           stepDest.GetPositionZ(), false, false, false, true);
         }
@@ -170,10 +212,11 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest)
 
     // cmangos MovementActions.cpp:720 — empty / non-progressing path
     // falls back to dispatching the destination as a single waypoint.
-    // Best-effort spline; stuck-recovery teleport (above) takes over
-    // if this oscillates.
+    // Best-effort spline; UnstuckAction (5/10 min) is the eventual
+    // catch if this loops forever.
     EmitDebugMove("MoveFar:spline",
                   dest.GetPositionX(), dest.GetPositionY(), dest.GetPositionZ());
+    botAI->rpgInfo.RecordMoveFarAttempt(dest, /*wasNodeTravel=*/false);
     return MoveTo(dest.GetMapId(), dest.GetPositionX(), dest.GetPositionY(), dest.GetPositionZ(),
                   false, false, false, true);
 }

src/Ai/World/Rpg/NewRpgInfo.cpp

@@ -78,6 +78,37 @@ void NewRpgInfo::Reset()
     data = Idle{};
     startT = getMSTime();
     ClearTravel();
+    recentMoveFarAttempts.clear();
+}
+
+void NewRpgInfo::RecordMoveFarAttempt(WorldPosition const& dest, bool wasNodeTravel)
+{
+    if (recentMoveFarAttempts.size() >= 3)
+        recentMoveFarAttempts.pop_front();
+    MoveFarAttempt a;
+    a.dest = dest;
+    a.wasNodeTravel = wasNodeTravel;
+    a.timestamp = getMSTime();
+    recentMoveFarAttempts.push_back(a);
+}
+
+int NewRpgInfo::CountRecentAttempts(WorldPosition const& dest, bool wasNodeTravel) const
+{
+    int count = 0;
+    for (auto const& a : recentMoveFarAttempts)
+    {
+        if (a.wasNodeTravel != wasNodeTravel)
+            continue;
+        // Treat destinations within 10y as "same dest" — small jitter
+        // from quest objective re-resolution shouldn't reset the loop
+        // detector.
+        if (a.dest.GetMapId() != dest.GetMapId())
+            continue;
+        if (a.dest.GetExactDist2dSq(&dest) > 10.0f * 10.0f)
+            continue;
+        ++count;
+    }
+    return count;
 }
 
 NewRpgStatus NewRpgInfo::GetStatus()

src/Ai/World/Rpg/NewRpgInfo.h

@@ -1,6 +1,8 @@
 #ifndef _PLAYERBOT_NEWRPGINFO_H
 #define _PLAYERBOT_NEWRPGINFO_H
 
+#include <deque>
+
 #include "Define.h"
 #include "ObjectGuid.h"
 #include "ObjectMgr.h"
@@ -81,6 +83,23 @@ struct NewRpgInfo
     bool HasActiveTravelPlan() const { return travelPlan.IsActive(); }
     void ClearTravel() { travelPlan.Reset(); }
 
+    // MoveFar attempt history. Records the last 3 path commits (node
+    // plan or mmap) so MoveFarTo can detect when the same dest +
+    // strategy has failed repeatedly and force the alternative
+    // routing this tick. Breaks deterministic-loop scenarios where
+    // the chained probe (or node graph) keeps returning the same
+    // dead-end path. Cmangos doesn't do this — they wait 5+ minutes
+    // for UnstuckAction. We're more aggressive here for UX.
+    struct MoveFarAttempt
+    {
+        WorldPosition dest;            // requested destination
+        bool wasNodeTravel{false};     // true=node plan, false=mmap/spline
+        uint32 timestamp{0};
+    };
+    std::deque<MoveFarAttempt> recentMoveFarAttempts;
+    void RecordMoveFarAttempt(WorldPosition const& dest, bool wasNodeTravel);
+    int CountRecentAttempts(WorldPosition const& dest, bool wasNodeTravel) const;
+
     using RpgData = std::variant<
         Idle,
         GoGrind,