chore: Tighten inline comments

This commit is contained in:
bash 2026-05-02 18:20:40 +02:00
parent 3f9640334e
commit 1a593a6a3c
7 changed files with 84 additions and 102 deletions

View File

@ -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<WorldPosition> 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;
}

View File

@ -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<WorldPosition> 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<WorldPosition> 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));
}

View File

@ -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:

View File

@ -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

View File

@ -824,7 +824,7 @@ void PlayerbotAI::HandleTeleportAck()
bot->StopMoving();
}
// simulate far teleport latency (cmangos-style)
// simulate far teleport latency
SetNextCheckDelay(urand(2000, 5000));
return;
}

View File

@ -722,14 +722,13 @@ std::vector<WorldPosition> 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);

View File

@ -292,12 +292,12 @@ public:
std::vector<WorldPosition> 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<WorldPosition> const& path, float const maxDistance = 0.0f,
float const maxZDistance = 2.0f) const
{