diff --git a/src/Ai/Base/Actions/MovementActions.cpp b/src/Ai/Base/Actions/MovementActions.cpp index 04eb83353..1be2a82ea 100644 --- a/src/Ai/Base/Actions/MovementActions.cpp +++ b/src/Ai/Base/Actions/MovementActions.cpp @@ -3233,15 +3233,14 @@ bool MovementAction::LaunchWalkSpline(TravelPlan& state) } // 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. + // 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 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); @@ -3308,8 +3307,7 @@ bool MovementAction::LaunchWalkSpline(TravelPlan& state) bot->GetOrientation(), delay, MovementPriority::MOVEMENT_NORMAL); // Cache the dispatched waypoint chain so MoveFarTo's 10% - // lastPath reuse (cmangos MovementActions.cpp:687) and the - // "no worse" reuse (line 716) can pick it up next tick. + // lastPath reuse and "no worse" reuse can pick it up next tick. std::vector wpts; wpts.reserve(state.walkPoints.size()); for (auto const& pt : state.walkPoints) @@ -3477,9 +3475,8 @@ bool MovementAction::ExecuteTravelPlan(TravelPlan& state) } // Too far from first point — abort the plan and let the - // caller's stuck-recovery decide what to do. (cmangos - // doesn't teleport here either; an abandoned plan is - // recovered by the next MoveFarTo cycle.) + // caller's stuck-recovery decide what to do. An abandoned + // plan is recovered by the next MoveFarTo cycle. if (state.walkPoints.size() >= 2) { G3D::Vector3 const& first = state.walkPoints.front(); @@ -3528,8 +3525,7 @@ bool MovementAction::ExecuteTravelPlan(TravelPlan& state) // At portal but didn't cross — natural collision missed. // Abort the plan; stuck-recovery in MoveFarTo will decide - // whether to retry or teleport the bot. (cmangos doesn't - // teleport here either.) + // whether to retry or teleport the bot. state.Reset(); return false; } @@ -3665,10 +3661,10 @@ bool MovementAction::ExecuteTravelPlan(TravelPlan& state) case PathNodeType::NODE_FLYING_MOUNT: { - // Flying-mount node not implemented — abort. cmangos - // produces these nodes during graph generation but their - // execution is server-specific; we treat them as - // unreachable rather than papering over with a teleport. + // Flying-mount node not implemented — abort. The graph + // generator produces these but their execution is + // server-specific; we treat them as unreachable rather + // than papering over with a teleport. state.Reset(); return false; } diff --git a/src/Ai/World/Rpg/Action/NewRpgBaseAction.cpp b/src/Ai/World/Rpg/Action/NewRpgBaseAction.cpp index 0e7c43fd6..722dcab15 100644 --- a/src/Ai/World/Rpg/Action/NewRpgBaseAction.cpp +++ b/src/Ai/World/Rpg/Action/NewRpgBaseAction.cpp @@ -112,12 +112,11 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest) } } - // 10% lastPath reuse (cmangos MovementActions.cpp:687-689). If - // the cached path's endpoint is within 10% of the new dest's - // distance AND the bot is still mid-flight toward it, skip the - // chained-probe recompute entirely. The 10y guard ensures we - // don't reuse a finished path (where the bot has already - // arrived at the cached endpoint). + // 10% lastPath reuse. If the cached path's endpoint is within + // 10% of the new dest's distance AND the bot is still mid-flight + // toward it, skip the chained-probe recompute entirely. The 10y + // guard ensures we don't reuse a finished path (where the bot + // has already arrived at the cached endpoint). { LastMovement& lastMove = AI_VALUE(LastMovement&, "last movement"); if (!lastMove.lastPath.empty()) @@ -137,35 +136,31 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest) float disToDest = bot->GetDistance(dest); float dis = bot->GetExactDist(dest); - // Mirrors cmangos-playerbots' ResolveMovePath / getFullPath flow: + // Decision tree: // // 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. + // 2. Otherwise, run the 40-step chained mmap probe. 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). + // 3. Probe lands within spellDistance of dest AND move is + // "long" (>= nodeFirstDis): use mmap, skip the node graph. + // Fixes cases where mmap CAN route to the destination and + // we'd otherwise commit to a 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. + // spline at the destination. // - // 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. + // No cone / random-direction sampling — 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); // Loop-breaker: count recent attempts of each strategy to this @@ -206,12 +201,11 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest) if (tryNodes && !forceMmapOverNodes && !bothExhausted && botAI->rpgInfo.HasActiveTravelPlan()) return UpdateTravelPlan(); - // 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. + // 40-step chained mmap probe (WorldPosition::getPathFromPath in + // TravelMgr.cpp). Heavier than a single GeneratePath call but + // chains PathGenerator across multiple navmesh tiles so it can + // reach destinations far beyond one PathGenerator's ~296y + // smooth-path cap. WorldPosition botPos(bot); std::vector probe = botPos.getPathTo(dest, bot); bool probeReachesDest = dest.isPathTo(probe, sPlayerbotAIConfig.spellDistance); @@ -222,8 +216,7 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest) { // 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). + // loops — commit to the travel-node graph. StartTravelPlan(dest); if (botAI->rpgInfo.HasActiveTravelPlan()) { @@ -252,13 +245,12 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest) botAI->rpgInfo.ClearTravel(); } - // Walk the chained probe's full waypoint chain via MoveSplinePath - // — cmangos's DispatchMovement pattern (MovementActions.cpp:1014: - // mm.MovePath(pointPath, moveMode, false)). Handing the FULL - // waypoint vector to the motion master removes its discretion - // to introduce a straight-line shortcut between intermediate - // points (which is what produced the diagonal-through-air bug - // when we used MoveTo(endpoint) and let the motion master replan). + // Walk the chained probe's full waypoint chain via MoveSplinePath. + // Handing the full waypoint vector to the motion master removes + // its discretion to introduce a straight-line shortcut between + // intermediate points — that shortcut produced the diagonal- + // through-air bug when we used MoveTo(endpoint) and let the + // motion master replan. // Skip when both routing strategies have failed 3 times each. if (!probe.empty() && !bothExhausted && probe.size() >= 2) { @@ -273,7 +265,7 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest) for (auto const& wp : probe) points.emplace_back(wp.GetPositionX(), wp.GetPositionY(), wp.GetPositionZ()); - // Per-waypoint Z-snap (cmangos DispatchMovement:1006). + // Per-waypoint Z-snap to current ground. for (auto& pt : points) bot->UpdateAllowedPositionZ(pt.x, pt.y, pt.z); @@ -301,12 +293,12 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest) if (points.size() >= 2) { - // No-worse lastPath reuse (cmangos MovementActions.cpp:716-717). - // If the cached path's endpoint is no further from - // dest than this new probe's, prefer cached to - // prevent path-swapping mid-walk. Same 10y guard as - // the top-of-MoveFarTo reuse to avoid reusing a - // finished path the bot already arrived at. + // No-worse lastPath reuse. If the cached path's + // endpoint is no further from dest than this new + // probe's, prefer cached to prevent path-swapping + // mid-walk. Same 10y guard as the top-of-MoveFarTo + // reuse to avoid reusing a finished path the bot + // already arrived at. { LastMovement& lastMove = AI_VALUE(LastMovement&, "last movement"); if (!lastMove.lastPath.empty()) @@ -363,8 +355,7 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest) MovementPriority::MOVEMENT_NORMAL); // Cache dispatched waypoints so the next MoveFarTo - // tick can satisfy the 10% / no-worse reuse checks - // (cmangos MovementActions.cpp:687, 716). + // tick can satisfy the 10% / no-worse reuse checks. std::vector wpts; wpts.reserve(points.size()); for (auto const& pt : points) @@ -376,10 +367,10 @@ 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; UnstuckAction (5/10 min) is the eventual - // catch if this loops forever. + // Empty / non-progressing path falls back to dispatching the + // destination as a single waypoint. Best-effort spline; + // UnstuckAction (5/10 min) is the eventual catch if this loops + // forever. LOG_INFO("playerbots", "[MoveFar] {} spline | dest=({:.0f},{:.0f},{:.0f}) | dis={:.0f} | probe.empty={} | mmapFails={} nodeFails={} | flags={}{}{}", bot->GetName(), dest.GetPositionX(), dest.GetPositionY(), dest.GetPositionZ(), dis, probe.empty() ? "y" : "n", @@ -445,13 +436,11 @@ bool NewRpgBaseAction::MoveWorldObjectTo(ObjectGuid guid, float distance) y = object->GetPositionY(); z = object->GetPositionZ(); } - // 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.) + // Delegate to MoveFarTo so every approach gets the 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". return MoveFarTo(WorldPosition(object->GetMapId(), x, y, z)); } diff --git a/src/Ai/World/Rpg/Action/NewRpgBaseAction.h b/src/Ai/World/Rpg/Action/NewRpgBaseAction.h index d98625275..2a9df1692 100644 --- a/src/Ai/World/Rpg/Action/NewRpgBaseAction.h +++ b/src/Ai/World/Rpg/Action/NewRpgBaseAction.h @@ -81,10 +81,9 @@ protected: // 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. + // first* via the 40-step chained pathfinder; the node graph + // only takes over if mmap can't get within spellDistance of + // the destination. const float nodeFirstDis = 75.0f; private: diff --git a/src/Ai/World/Rpg/NewRpgInfo.h b/src/Ai/World/Rpg/NewRpgInfo.h index 57196137a..8436b9567 100644 --- a/src/Ai/World/Rpg/NewRpgInfo.h +++ b/src/Ai/World/Rpg/NewRpgInfo.h @@ -83,13 +83,12 @@ 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. + // 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. struct MoveFarAttempt { WorldPosition dest; // requested destination diff --git a/src/Bot/PlayerbotAI.cpp b/src/Bot/PlayerbotAI.cpp index 9dd26395e..916b7c838 100644 --- a/src/Bot/PlayerbotAI.cpp +++ b/src/Bot/PlayerbotAI.cpp @@ -824,7 +824,7 @@ void PlayerbotAI::HandleTeleportAck() bot->StopMoving(); } - // simulate far teleport latency (cmangos-style) + // simulate far teleport latency SetNextCheckDelay(urand(2000, 5000)); return; } diff --git a/src/Mgr/Travel/TravelMgr.cpp b/src/Mgr/Travel/TravelMgr.cpp index 47e6df615..b3bf6b608 100644 --- a/src/Mgr/Travel/TravelMgr.cpp +++ b/src/Mgr/Travel/TravelMgr.cpp @@ -722,14 +722,13 @@ std::vector WorldPosition::getPathStepFrom(WorldPosition startPos } // 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 + // 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)). + // never advances. With explicit start, each step extends from + // the previous step's endpoint, giving the 40-attempt walker + // its intended multi-tile reach. PathGenerator path(pathUnit); path.CalculatePath(startPos.GetPositionX(), startPos.GetPositionY(), startPos.GetPositionZ(), GetPositionX(), GetPositionY(), GetPositionZ(), false); diff --git a/src/Mgr/Travel/TravelMgr.h b/src/Mgr/Travel/TravelMgr.h index 41cc4ecf8..877b0744b 100644 --- a/src/Mgr/Travel/TravelMgr.h +++ b/src/Mgr/Travel/TravelMgr.h @@ -292,12 +292,12 @@ public: std::vector getPathTo(WorldPosition endPos, Unit* bot) { return endPos.getPathFrom(*this, bot); } - // Cmangos-aligned (WorldPosition.h:317): the path "reaches" this - // position when its last point is on the same map, within - // maxDistance horizontally, and within maxZDistance vertically. - // 3D Euclidean distance would falsely accept paths that end the - // right horizontal distance from us but on a roof/floor below. - // maxDistance == 0 falls back to targetPosRecalcDistance (0.1y). + // The path "reaches" this position when its last point is on + // the same map, within maxDistance horizontally, and within + // maxZDistance vertically. 3D Euclidean distance would falsely + // accept paths that end the right horizontal distance from us + // but on a roof/floor below. maxDistance == 0 falls back to + // targetPosRecalcDistance (0.1y). bool isPathTo(std::vector const& path, float const maxDistance = 0.0f, float const maxZDistance = 2.0f) const {