From 553c7739e823fe35b97385a0fb29837b831cc5b0 Mon Sep 17 00:00:00 2001 From: bash Date: Sat, 2 May 2026 18:02:06 +0200 Subject: [PATCH] feat(Core/RPG): MoveFarTo loop detection with strategy flip + grinding throttle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 | 27 ++++----- src/Ai/World/Rpg/Action/NewRpgBaseAction.cpp | 59 +++++++++++++++++--- src/Ai/World/Rpg/NewRpgInfo.cpp | 31 ++++++++++ src/Ai/World/Rpg/NewRpgInfo.h | 19 +++++++ 4 files changed, 111 insertions(+), 25 deletions(-) diff --git a/src/Ai/World/Rpg/Action/NewRpgAction.cpp b/src/Ai/World/Rpg/Action/NewRpgAction.cpp index 6762da77c..c06eb205e 100644 --- a/src/Ai/World/Rpg/Action/NewRpgAction.cpp +++ b/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; diff --git a/src/Ai/World/Rpg/Action/NewRpgBaseAction.cpp b/src/Ai/World/Rpg/Action/NewRpgBaseAction.cpp index 150a60f48..23225adc0 100644 --- a/src/Ai/World/Rpg/Action/NewRpgBaseAction.cpp +++ b/src/Ai/World/Rpg/Action/NewRpgBaseAction.cpp @@ -110,9 +110,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 @@ -125,10 +158,13 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest) std::vector 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()) @@ -138,6 +174,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 @@ -145,7 +182,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(); } @@ -155,7 +193,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(), @@ -164,6 +205,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); } @@ -171,10 +213,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); } diff --git a/src/Ai/World/Rpg/NewRpgInfo.cpp b/src/Ai/World/Rpg/NewRpgInfo.cpp index 59eaca93f..5c5af71f8 100644 --- a/src/Ai/World/Rpg/NewRpgInfo.cpp +++ b/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() diff --git a/src/Ai/World/Rpg/NewRpgInfo.h b/src/Ai/World/Rpg/NewRpgInfo.h index caf94f012..57196137a 100644 --- a/src/Ai/World/Rpg/NewRpgInfo.h +++ b/src/Ai/World/Rpg/NewRpgInfo.h @@ -1,6 +1,8 @@ #ifndef _PLAYERBOT_NEWRPGINFO_H #define _PLAYERBOT_NEWRPGINFO_H +#include + #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 recentMoveFarAttempts; + void RecordMoveFarAttempt(WorldPosition const& dest, bool wasNodeTravel); + int CountRecentAttempts(WorldPosition const& dest, bool wasNodeTravel) const; + using RpgData = std::variant< Idle, GoGrind,