From 8a5df83f4fd947e52458f0aacb6913e9483bc1fb Mon Sep 17 00:00:00 2001 From: bash Date: Sat, 2 May 2026 18:01:51 +0200 Subject: [PATCH] feat(Core/RPG): Refactor MoveFarTo decision tree (single 75y gate + chained probe) Single 75y long-path gate, 40-step chained mmap probe via WorldPosition::getPathFromPath, spellDistance shortcut to skip the node graph when mmap can reach the destination, single-waypoint spline fallback when both fail. MoveWorldObjectTo delegates to MoveFarTo so quest approaches use the same routing hierarchy. Removes the stuck-counter teleport recovery from MoveFarTo; the UnstuckAction layer (planned) is the eventual safety net. Chain probe fix included: getPathStepFrom uses the explicit-start CalculatePath overload so each chain step actually advances from the previous endpoint instead of restarting from the bot's real position. --- src/Ai/Base/Actions/MovementActions.cpp | 13 ++ src/Ai/World/Rpg/Action/NewRpgBaseAction.cpp | 224 +++++++++---------- src/Ai/World/Rpg/Action/NewRpgBaseAction.h | 17 +- src/Ai/World/Rpg/NewRpgInfo.cpp | 8 - src/Ai/World/Rpg/NewRpgInfo.h | 6 - src/Mgr/Travel/TravelMgr.cpp | 12 +- 6 files changed, 140 insertions(+), 140 deletions(-) diff --git a/src/Ai/Base/Actions/MovementActions.cpp b/src/Ai/Base/Actions/MovementActions.cpp index 39bfe69c6..dcb34e7c4 100644 --- a/src/Ai/Base/Actions/MovementActions.cpp +++ b/src/Ai/Base/Actions/MovementActions.cpp @@ -3231,6 +3231,19 @@ bool MovementAction::LaunchWalkSpline(TravelPlan& state) return true; } + // Re-clamp cached waypoints to current valid Z. Rows in + // playerbots_travelnode_path store absolute coords baked at offline + // generation; if the live navmesh has shifted since (mmap regen, + // terrain change, vmap update), the stored z can be above ground — + // MoveSplinePath plays back coords verbatim and the bot looks like + // it's walking through the air. UpdateAllowedPositionZ is the same + // per-waypoint snap cmangos-playerbots applies in DispatchMovement + // (MovementActions.cpp:1006); it factors mmap polygon Z, water + // surface, swimming, flying and transport state, so cave floors + // above the terrain plane snap correctly. + for (auto& pt : state.walkPoints) + bot->UpdateAllowedPositionZ(pt.x, pt.y, pt.z); + // Mount up if (!bot->IsMounted() && !bot->IsInCombat() && bot->IsOutdoors() && bot->IsAlive()) botAI->DoSpecificAction("check mount state", Event(), true); diff --git a/src/Ai/World/Rpg/Action/NewRpgBaseAction.cpp b/src/Ai/World/Rpg/Action/NewRpgBaseAction.cpp index 80ce6758b..b0e061c35 100644 --- a/src/Ai/World/Rpg/Action/NewRpgBaseAction.cpp +++ b/src/Ai/World/Rpg/Action/NewRpgBaseAction.cpp @@ -1,5 +1,7 @@ #include "NewRpgBaseAction.h" +#include + #include "BroadcastHelper.h" #include "ChatHelper.h" #include "Creature.h" @@ -36,17 +38,12 @@ #include "Timer.h" #include "TravelMgr.h" + bool NewRpgBaseAction::MoveFarTo(WorldPosition dest) { if (dest == WorldPosition()) return false; - if (dest != botAI->rpgInfo.moveFarPos) - { - // clear stuck information if it's a new dest - botAI->rpgInfo.SetMoveFarTo(dest); - } - // performance optimization if (IsWaitingForLastMove(MovementPriority::MOVEMENT_NORMAL)) { @@ -62,15 +59,12 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest) // Without this gate, DoMovePoint would call mm->Clear() and // reissue MovePoint from the new bot position — and from a new // position mmap's partial-path endpoint often differs, so the - // bot gets clobbered mid-walk and ends up oscillating (e.g. - // cave entrance -> inside cave -> cave entrance -> mountain - // base -> cave entrance...) around an unreachable destination. + // bot gets clobbered mid-walk and ends up oscillating around an + // unreachable destination. // // If the bot is still actively walking toward its last // committed point on the same map, just let the current spline - // finish. The stuck counter below continues to track real - // progress toward dest and triggers teleport recovery if the - // committed paths genuinely aren't closing the gap. + // finish. { LastMovement& lastMove = AI_VALUE(LastMovement&, "last movement"); if (bot->isMoving() && lastMove.lastMoveToMapId == bot->GetMapId()) @@ -81,118 +75,107 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest) } } - // stuck check float disToDest = bot->GetDistance(dest); - // Require a meaningful improvement (5yd) to reset the stuck counter. - // The old 1yd threshold was small enough that bots oscillating back - // and forth around an obstacle would keep "making progress" forever - // and never trigger the teleport recovery below. - if (disToDest + 5.0f < botAI->rpgInfo.nearestMoveFarDis) - { - botAI->rpgInfo.nearestMoveFarDis = disToDest; - botAI->rpgInfo.stuckTs = getMSTime(); - botAI->rpgInfo.stuckAttempts = 0; - } - else if (++botAI->rpgInfo.stuckAttempts >= 5 && GetMSTimeDiffToNow(botAI->rpgInfo.stuckTs) >= stuckTime) - { - // No meaningful progress toward dest for `stuckTime`: fall - // back to teleporting directly so the bot can get on with - // its RPG objective instead of oscillating indefinitely. - botAI->rpgInfo.stuckTs = getMSTime(); - botAI->rpgInfo.stuckAttempts = 0; - const AreaTableEntry* entry = sAreaTableStore.LookupEntry(bot->GetZoneId()); - std::string zone_name = PlayerbotAI::GetLocalizedAreaName(entry); - LOG_DEBUG("playerbots","[New RPG] Teleport {} from ({},{},{},{}) to ({},{},{},{}) as it stuck when moving far - Zone: {} ({})", - bot->GetName(), bot->GetPositionX(), bot->GetPositionY(), bot->GetPositionZ(), bot->GetMapId(), - dest.GetPositionX(), dest.GetPositionY(), dest.GetPositionZ(), dest.GetMapId(), bot->GetZoneId(), zone_name); - botAI->TeleportTo(dest); - return true; - } - float dis = bot->GetExactDist(dest); - // Long distance + travel nodes enabled: use the pre-computed node graph - // (A*, flight paths, transports) instead of repeated mmap hops. - if (dis > MAX_PATHFINDING_DISTANCE && sPlayerbotAIConfig.enableTravelNodes) - { - if (!botAI->rpgInfo.HasActiveTravelPlan()) - StartTravelPlan(dest); + // Mirrors cmangos-playerbots' ResolveMovePath / getFullPath flow: + // + // 1. Active node plan? Ride it. The plan executor owns its own + // per-step transitions (walk/flight/transport/teleport). + // + // 2. Otherwise, run the same 40-step chained mmap probe that + // cmangos uses everywhere (TravelMgr.cpp:760, ported in PR + // #2312 onto WorldPosition). It chains PathGenerator calls + // across navmesh tiles so it reaches destinations far beyond + // a single PathGenerator's ~296y smooth-path cap. + // + // 3. Probe lands within spellDistance of dest AND move is "long" + // (>= nodeFirstDis): use mmap, skip the node graph. This is + // cmangos's TravelNode.cpp:1894 short-circuit — and the fix + // for the cave-quest case (when mmap CAN route into the + // cave from current position, prefer that over the cached + // surface-node detour). + // + // 4. Probe didn't reach AND move is long: commit to the + // travel-node plan (graph A* + flights + transports). + // + // 5. Otherwise: walk to the probe's furthest reachable point. + // Empty / non-progressing probe falls back to a best-effort + // spline at the destination (cmangos line 720: addPoint). + // Stuck-recovery (above) handles oscillation. + // + // No cone / random-direction sampling — cmangos doesn't do it and + // it tends to walk bots into geometry on the way to "stepping + // stones" that aren't on the actual route. + bool tryNodes = (dis >= nodeFirstDis && sPlayerbotAIConfig.enableTravelNodes); + // If a node plan is already active, ride it. The plan executor + // owns its own per-step transitions. + if (tryNodes && botAI->rpgInfo.HasActiveTravelPlan()) return UpdateTravelPlan(); - } - // Crossed below the travel-node threshold — clear any leftover plan - if (botAI->rpgInfo.HasActiveTravelPlan()) + // 40-step chained mmap probe (cmangos getPathFromPath, ported in + // PR #2312 at TravelMgr.cpp:760). Heavier than a single + // GeneratePath call but matches cmangos's "same effort" baseline + // — chains PathGenerator calls across multiple navmesh tiles so + // it can reach destinations far beyond a single PathGenerator's + // ~296y smooth-path cap. + WorldPosition botPos(bot); + std::vector probe = botPos.getPathTo(dest, bot); + bool probeReachesDest = dest.isPathTo(probe, sPlayerbotAIConfig.spellDistance); + + if (tryNodes && !probeReachesDest) + { + // Long-distance move and mmap couldn't get within spellDistance + // of the destination — commit to the travel-node graph + // (cmangos TravelNode.cpp:1907 buildPath branch). + StartTravelPlan(dest); + if (botAI->rpgInfo.HasActiveTravelPlan()) + { + // Fire once on plan start so the user sees nodetravel as + // the chosen strategy. Per-step labels + // (TravelPlan:walk/segment/...) continue from the executor. + EmitDebugMove("MoveFar:nodetravel", + dest.GetPositionX(), dest.GetPositionY(), dest.GetPositionZ()); + return UpdateTravelPlan(); + } + // else: graph returned no plan — fall through to mmap best-effort + } + 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. botAI->rpgInfo.ClearTravel(); - - // Short range: close enough for a single mmap call - if (dis < pathFinderDis) - { - return MoveTo(dest.GetMapId(), dest.GetPositionX(), dest.GetPositionY(), dest.GetPositionZ(), false, false, - false, true); } - // Primary strategy: ask mmap for a route to the TRUE destination. - // If mmap can reach it directly (PATHFIND_NORMAL) or partially - // (PATHFIND_INCOMPLETE — destinations beyond the smooth-path cap - // of ~296 yards, or where local geometry blocks the final step), - // walk to the furthest reachable waypoint mmap computed. This - // lets bots follow the real route around obstacles (mountains, - // cave walls, cliffs) instead of trying to cut straight through. - // The spline system walks the whole returned path smoothly, so - // subsequent ticks early-out via IsWaitingForLastMove and no - // further PathGenerator calls fire until the bot arrives. + + // Walk to the chained probe's furthest reachable point if it + // makes meaningful progress toward the destination. cmangos + // dispatches the full waypoint list via MovePath; we hand the + // endpoint to MoveTo and let the motion master plan its own + // spline. Functionally equivalent across multiple ticks + // (incremental progress). + if (!probe.empty()) { - PathResult path = GeneratePath(dest.GetPositionX(), dest.GetPositionY(), - dest.GetPositionZ(), RELAXED_PATH_ACCEPT_MASK); - if (path.reachable) + WorldPosition stepDest = probe.back(); + float endDistToDest = dest.GetExactDist(stepDest.GetPositionX(), + stepDest.GetPositionY(), stepDest.GetPositionZ()); + if (endDistToDest + 5.0f < disToDest) { - // Only commit if the mmap endpoint actually makes progress - // toward the destination. For pathological INCOMPLETE - // results (e.g. disconnected polys that still report - // INCOMPLETE) the endpoint can land right under the bot; - // fall through to cone sampling in that case. - float endDistToDest = dest.GetExactDist(path.actualEnd.x, path.actualEnd.y, path.actualEnd.z); - if (endDistToDest + 5.0f < disToDest) - return MoveTo(bot->GetMapId(), path.actualEnd.x, path.actualEnd.y, path.actualEnd.z, false, false, false, true); + EmitDebugMove("MoveFar:mmap", + stepDest.GetPositionX(), stepDest.GetPositionY(), stepDest.GetPositionZ()); + return MoveTo(bot->GetMapId(), stepDest.GetPositionX(), stepDest.GetPositionY(), + stepDest.GetPositionZ(), false, false, false, true); } } - // Fallback: mmap couldn't route to the destination. Sample the - // forward cone for a reachable stepping stone so the bot keeps - // moving and can try again from a new vantage point. Cap at 2 - // samples — we already spent one PathGenerator call above and at - // 3000 bots every extra CalculatePath matters. - float minDelta = M_PI; - const float x = bot->GetPositionX(); - const float y = bot->GetPositionY(); - const float z = bot->GetPositionZ(); - const float baseAngle = bot->GetAngle(&dest); - float rx, ry, rz; - bool found = false; - for (int attempt = 0; attempt < 2; ++attempt) - { - float delta = (rand_norm() - 0.5f) * static_cast(M_PI); // ±π/2, forward cone - float sampleDis = (0.5f + rand_norm() * 0.5f) * pathFinderDis; - float angle = baseAngle + delta; - float dx = x + cos(angle) * sampleDis; - float dy = y + sin(angle) * sampleDis; - float dz = z + 0.5f; - PathResult path = GeneratePath(dx, dy, dz, RELAXED_PATH_ACCEPT_MASK); - - if (path.reachable && fabs(delta) <= minDelta) - { - found = true; - rx = path.actualEnd.x; - ry = path.actualEnd.y; - rz = path.actualEnd.z; - minDelta = fabs(delta); - } - } - if (found) - { - return MoveTo(bot->GetMapId(), rx, ry, rz, false, false, false, true); - } - return false; + // 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. + EmitDebugMove("MoveFar:spline", + dest.GetPositionX(), dest.GetPositionY(), dest.GetPositionZ()); + return MoveTo(dest.GetMapId(), dest.GetPositionX(), dest.GetPositionY(), dest.GetPositionZ(), + false, false, false, true); } void NewRpgBaseAction::StartTravelPlan(WorldPosition dest) @@ -218,16 +201,13 @@ bool NewRpgBaseAction::UpdateTravelPlan() bool NewRpgBaseAction::MoveWorldObjectTo(ObjectGuid guid, float distance) { - if (IsWaitingForLastMove(MovementPriority::MOVEMENT_NORMAL)) - return false; - WorldObject* object = botAI->GetWorldObject(guid); if (!object) return false; + float x = object->GetPositionX(); float y = object->GetPositionY(); float z = object->GetPositionZ(); - float mapId = object->GetMapId(); float angle = 0.f; if (!object->ToUnit() || !object->ToUnit()->isMoving()) @@ -246,7 +226,14 @@ bool NewRpgBaseAction::MoveWorldObjectTo(ObjectGuid guid, float distance) y = object->GetPositionY(); z = object->GetPositionZ(); } - return MoveTo(mapId, x, y, z, false, false, false, true); + // Delegate to MoveFarTo so every approach gets the cmangos-aligned + // chained mmap probe + spellDistance shortcut + travel-node + // fallback, instead of a single direct MoveTo. The debug-move + // trace then labels the actual mechanism (spline / mmap / + // nodetravel) rather than a generic "MoveWorldObjectTo:spline". + // (cmangos: refactor(Move): Route MoveWorldObjectTo through + // MoveFarTo — 4cb3abab.) + return MoveFarTo(WorldPosition(object->GetMapId(), x, y, z)); } bool NewRpgBaseAction::MoveRandomNear(float moveStep, MovementPriority priority, WorldObject* center) @@ -284,7 +271,10 @@ bool NewRpgBaseAction::MoveRandomNear(float moveStep, MovementPriority priority, bool moved = MoveTo(bot->GetMapId(), dx, dy, dz, false, false, false, true, priority); if (moved) + { + EmitDebugMove("MoveRandomNear:spline", dx, dy, dz); return true; + } } return false; @@ -316,6 +306,8 @@ bool NewRpgBaseAction::TakeFlight(std::vector const& taxiNodes, Creature LOG_DEBUG("playerbots", "[New RPG] Bot {} taking flight ({} nodes, {} to {})", bot->GetName(), taxiNodes.size(), taxiNodes.front(), taxiNodes.back()); + EmitDebugMove("TravelPlan:flight", flightMaster->GetPositionX(), flightMaster->GetPositionY(), + flightMaster->GetPositionZ()); return true; } diff --git a/src/Ai/World/Rpg/Action/NewRpgBaseAction.h b/src/Ai/World/Rpg/Action/NewRpgBaseAction.h index e33d3760a..d98625275 100644 --- a/src/Ai/World/Rpg/Action/NewRpgBaseAction.h +++ b/src/Ai/World/Rpg/Action/NewRpgBaseAction.h @@ -78,15 +78,14 @@ protected: protected: /* FOR MOVE FAR */ - const float pathFinderDis = 70.0f; - // Time without real progress toward dest before MoveFarTo - // falls back to teleport recovery. Kept short enough that a - // bot truly oscillating around an unreachable destination - // (mmap returning non-progressing partial paths, or NOPATH + - // cone fallback wandering) doesn't spin for 5 minutes before - // the teleport fires, but long enough that a genuine long - // walk that is slowly making progress never triggers it. - const uint32 stuckTime = 90 * 1000; + // Distance at which MoveFarTo considers the travel-node graph as + // a routing option. Below this, the move is short enough that + // mmap handles it directly. Above this, mmap is *still probed + // first* via the 40-step chained pathfinder; the node graph only + // takes over if mmap can't get within spellDistance of the + // destination. Matches cmangos-playerbots' sightDistance gate + // (75y) — the only long-path threshold they use. + const float nodeFirstDis = 75.0f; private: void StartTravelPlan(WorldPosition dest); diff --git a/src/Ai/World/Rpg/NewRpgInfo.cpp b/src/Ai/World/Rpg/NewRpgInfo.cpp index 150421752..59eaca93f 100644 --- a/src/Ai/World/Rpg/NewRpgInfo.cpp +++ b/src/Ai/World/Rpg/NewRpgInfo.cpp @@ -80,14 +80,6 @@ void NewRpgInfo::Reset() ClearTravel(); } -void NewRpgInfo::SetMoveFarTo(WorldPosition pos) -{ - nearestMoveFarDis = FLT_MAX; - stuckTs = 0; - stuckAttempts = 0; - moveFarPos = pos; -} - NewRpgStatus NewRpgInfo::GetStatus() { return std::visit([](auto&& arg) -> NewRpgStatus { diff --git a/src/Ai/World/Rpg/NewRpgInfo.h b/src/Ai/World/Rpg/NewRpgInfo.h index 894f1302b..caf94f012 100644 --- a/src/Ai/World/Rpg/NewRpgInfo.h +++ b/src/Ai/World/Rpg/NewRpgInfo.h @@ -76,11 +76,6 @@ struct NewRpgInfo uint32 startT{0}; // start timestamp of the current status - // MOVE_FAR - float nearestMoveFarDis{FLT_MAX}; - uint32 stuckTs{0}; - uint32 stuckAttempts{0}; - WorldPosition moveFarPos; // Travel Node System TravelPlan travelPlan; bool HasActiveTravelPlan() const { return travelPlan.IsActive(); } @@ -112,7 +107,6 @@ struct NewRpgInfo void ChangeToIdle(); bool CanChangeTo(NewRpgStatus status); void Reset(); - void SetMoveFarTo(WorldPosition pos); std::string ToString(); }; diff --git a/src/Mgr/Travel/TravelMgr.cpp b/src/Mgr/Travel/TravelMgr.cpp index 12fad2245..94c5c1152 100644 --- a/src/Mgr/Travel/TravelMgr.cpp +++ b/src/Mgr/Travel/TravelMgr.cpp @@ -721,8 +721,18 @@ std::vector WorldPosition::getPathStepFrom(WorldPosition startPos map->EnsureGridCreated(Acore::ComputeGridCoord(GetPositionX(), GetPositionY())); } + // Explicit-start overload (PathGenerator.h:67). Without this, + // CalculatePath(destX,destY,destZ) defaults to the unit's CURRENT + // position as start — which means every iteration of + // getPathFromPath's "chain" begins from the bot's same real + // location and produces the same ~296y partial path. The chain + // never advances. With explicit start, each step extends from the + // previous step's endpoint, giving the 40-attempt walker its + // intended multi-tile reach. Cmangos passes start explicitly too + // (WorldPosition.cpp:962 — pathfinder->calculate(start, end)). PathGenerator path(pathUnit); - path.CalculatePath(GetPositionX(), GetPositionY(), GetPositionZ()); + path.CalculatePath(startPos.GetPositionX(), startPos.GetPositionY(), startPos.GetPositionZ(), + GetPositionX(), GetPositionY(), GetPositionZ(), false); Movement::PointsArray points = path.GetPath(); PathType type = path.GetPathType();