From 34022e9329f69774b1763e718a20bdf0110f03e1 Mon Sep 17 00:00:00 2001 From: Tecc Date: Fri, 3 Jul 2026 22:15:14 +0000 Subject: [PATCH 01/10] fix: Never hand out a stale master pointer after master logout Bots hold a raw Player* master that dangles when the master logs out and the Player object is destroyed between AI ticks on the map-update threads. FollowAction::isUseful then crashes on fTarget->GetGUID() (reproduced twice, cores captured; same UAF family as #2474; the crashing branch was introduced in #2462). SetMaster() now records the master GUID and GetMaster() re-validates it through ObjectAccessor::FindPlayer, which never dereferences the stored pointer, returning nullptr once the master is gone. HasRealPlayerMaster, HasActivePlayerMaster and the GetGroupLeader fallback go through the validated accessor, UpdateAIGroupMaster clears a stale master at entry, and GossipHelloAction handles the now-possible null master. --- src/Ai/Base/Actions/GossipHelloAction.cpp | 12 ++++- src/Bot/PlayerbotAI.cpp | 62 +++++++++++++++++++---- src/Bot/PlayerbotAI.h | 6 ++- 3 files changed, 66 insertions(+), 14 deletions(-) diff --git a/src/Ai/Base/Actions/GossipHelloAction.cpp b/src/Ai/Base/Actions/GossipHelloAction.cpp index 2eb5e527907..ad277942251 100644 --- a/src/Ai/Base/Actions/GossipHelloAction.cpp +++ b/src/Ai/Base/Actions/GossipHelloAction.cpp @@ -59,7 +59,11 @@ void GossipHelloAction::TellGossipMenus() if (!bot->PlayerTalkClass) return; - Creature* pCreature = bot->GetNPCIfCanInteractWith(GetMaster()->GetTarget(), UNIT_NPC_FLAG_NONE); + Player* master = GetMaster(); + if (!master) + return; + + Creature* pCreature = bot->GetNPCIfCanInteractWith(master->GetTarget(), UNIT_NPC_FLAG_NONE); GossipMenu& menu = bot->PlayerTalkClass->GetGossipMenu(); if (pCreature) { @@ -87,9 +91,13 @@ bool GossipHelloAction::ProcessGossip(int32 menuToSelect, bool silent) return false; } + Player* master = GetMaster(); + if (!master) + return false; + WorldPacket p; std::string code; - p << GetMaster()->GetTarget(); + p << master->GetTarget(); p << menu.GetMenuId() << menuToSelect; p << code; bot->GetSession()->HandleGossipSelectOptionOpcode(p); diff --git a/src/Bot/PlayerbotAI.cpp b/src/Bot/PlayerbotAI.cpp index da952e2c329..fac71fc5140 100644 --- a/src/Bot/PlayerbotAI.cpp +++ b/src/Bot/PlayerbotAI.cpp @@ -114,6 +114,7 @@ PlayerbotAI::PlayerbotAI() : PlayerbotAIBase(true), bot(nullptr), master(nullptr), + masterGuid(), accountId(0), aiObjectContext(nullptr), currentEngine(nullptr), @@ -137,6 +138,7 @@ PlayerbotAI::PlayerbotAI(Player* bot) forceRebuff(bot), bot(bot), master(nullptr), + masterGuid(), chatHelper(this), chatFilter(this), security(bot) // reorder args - whipowill @@ -418,6 +420,11 @@ void PlayerbotAI::UpdateAIGroupMaster() if (!botAI) return; + // Drop a stale master pointer (master logged out and got destroyed between AI ticks) + // before anything below dereferences it + if (master && !GetMaster()) + SetMaster(nullptr); + Group* group = bot->GetGroup(); // If bot is not in group verify that for is RandomBot before clearing master and resetting. @@ -446,7 +453,7 @@ void PlayerbotAI::UpdateAIGroupMaster() Player* newMaster = FindNewMaster(); if (newMaster) { - master = newMaster; + SetMaster(newMaster); botAI->SetMaster(newMaster); botAI->ResetStrategies(); @@ -1059,8 +1066,11 @@ void PlayerbotAI::HandleCommand(uint32 type, std::string const text, Player* fro if (bot->GetSession()->isLogingOut()) return; - // Verify the command came from this bot's master. Also handles nullptr - if (fromPlayer != master) + // Verify the command came from this bot's master. Also handles nullptr. + // Use the validated accessor: this runs from the chat packet handler, outside + // the AI tick, where a raw master pointer can be stale after master logout. + Player* validMaster = GetMaster(); + if (!validMaster || fromPlayer != validMaster) { if (type == CHAT_MSG_WHISPER) { @@ -1071,7 +1081,7 @@ void PlayerbotAI::HandleCommand(uint32 type, std::string const text, Player* fro return; } - PlayerbotMgr* masterBotMgr = GET_PLAYERBOT_MGR(master); + PlayerbotMgr* masterBotMgr = GET_PLAYERBOT_MGR(validMaster); if (!masterBotMgr) return; @@ -3066,7 +3076,10 @@ bool PlayerbotAI::TellMaster(std::ostringstream& stream, PlayerbotSecurityLevel bool PlayerbotAI::TellMaster(std::string const text, PlayerbotSecurityLevel securityLevel) { - if (!master) + // Use the validated accessor - this runs from packet-driven paths too, where a raw + // master pointer can be stale after master logout + Player* validMaster = GetMaster(); + if (!validMaster) { if (sPlayerbotAIConfig.randomBotSayWithoutMaster) return TellMasterNoFacing(text, securityLevel); @@ -3076,11 +3089,11 @@ bool PlayerbotAI::TellMaster(std::string const text, PlayerbotSecurityLevel secu if (!TellMasterNoFacing(text, securityLevel)) return false; - if (!bot->isMoving() && !bot->IsInCombat() && bot->GetMapId() == master->GetMapId() && + if (!bot->isMoving() && !bot->IsInCombat() && bot->GetMapId() == validMaster->GetMapId() && !bot->HasUnitState(UNIT_STATE_IN_FLIGHT) && !bot->IsFlying()) { - if (!bot->HasInArc(EMOTE_ANGLE_IN_FRONT, master, sPlayerbotAIConfig.sightDistance)) - bot->SetFacingToObject(master); + if (!bot->HasInArc(EMOTE_ANGLE_IN_FRONT, validMaster, sPlayerbotAIConfig.sightDistance)) + bot->SetFacingToObject(validMaster); bot->HandleEmoteCommand(EMOTE_ONESHOT_TALK); } @@ -4477,7 +4490,36 @@ Player* PlayerbotAI::FindNewMaster() bool PlayerbotAI::IsAltBot() { return HasGameClientMaster() && !sRandomPlayerbotMgr.IsRandomBot(bot) && !IsSelfBot(bot); } // True when the bot's master is driven by a player with a game client: a regular player (no bot AI) or a selfbot player. -bool PlayerbotAI::HasGameClientMaster() { return IsRealPlayer(master) || IsSelfBot(master); } +bool PlayerbotAI::HasGameClientMaster() +{ + // Go through GetMaster() - the raw pointer can be stale after master logout, and + // IsRealPlayer/IsSelfBot dereference it + Player* validMaster = GetMaster(); + return IsRealPlayer(validMaster) || IsSelfBot(validMaster); +} + +Player* PlayerbotAI::GetMaster() +{ + if (!master) + return nullptr; + + // Never hand out a stale pointer: the master Player can be destroyed (logout) between + // AI ticks on the map-update threads while bots still hold the raw pointer. Re-validate + // through the ObjectAccessor by GUID, which never dereferences the stored pointer. + // FindConnectedPlayer (not FindPlayer): the master must still count as present while + // merely loading/teleporting between maps, otherwise every zone transition makes bots + // transiently masterless (rejected whisper commands, spurious master resets). + if (master != bot && (!masterGuid || !ObjectAccessor::FindConnectedPlayer(masterGuid))) + return nullptr; + + return master; +} + +void PlayerbotAI::SetMaster(Player* newMaster) +{ + master = newMaster; + masterGuid = newMaster ? newMaster->GetGUID() : ObjectGuid::Empty; +} Player* PlayerbotAI::GetGroupLeader() { @@ -4486,7 +4528,7 @@ Player* PlayerbotAI::GetGroupLeader() if (Player* player = ObjectAccessor::FindPlayer(group->GetLeaderGUID())) return player; - return master; + return GetMaster(); } uint32 PlayerbotAI::GetFixedBotNumber(uint32 maxNum) diff --git a/src/Bot/PlayerbotAI.h b/src/Bot/PlayerbotAI.h index 2142673bff5..9bc571e5511 100644 --- a/src/Bot/PlayerbotAI.h +++ b/src/Bot/PlayerbotAI.h @@ -15,6 +15,7 @@ #include "Item.h" #include "NewRpgInfo.h" #include "NewRpgStrategy.h" +#include "ObjectGuid.h" #include "PlayerbotAIBase.h" #include "PlayerbotAIConfig.h" #include "PlayerbotSecurity.h" @@ -537,7 +538,7 @@ class PlayerbotAI : public PlayerbotAIBase float GetRange(std::string const type); Player* GetBot() { return bot; } - Player* GetMaster() { return master; } + Player* GetMaster(); Player* FindNewMaster(); // Get the group leader or the master of the bot. @@ -570,7 +571,7 @@ class PlayerbotAI : public PlayerbotAIBase BotCheatMask GetCheat() { return cheatMask; } void SetCheat(BotCheatMask mask) { cheatMask = mask; } - void SetMaster(Player* newMaster) { master = newMaster; } + void SetMaster(Player* newMaster); AiObjectContext* GetAiObjectContext() { return aiObjectContext; } ChatHelper* GetChatHelper() { return &chatHelper; } bool IsOpposing(Player* player); @@ -631,6 +632,7 @@ class PlayerbotAI : public PlayerbotAIBase protected: Player* bot; Player* master; + ObjectGuid masterGuid; uint32 accountId; AiObjectContext* aiObjectContext; Engine* currentEngine; From 9695df2d723f1490028d8091e5934bef10d8ebb1 Mon Sep 17 00:00:00 2001 From: Tecc Date: Thu, 2 Jul 2026 22:29:37 +0000 Subject: [PATCH 02/10] feat: Stay mounted or mount up to close distance to master Reattempt of PR #1760 (reverted in #1855). Bots now stay mounted when the master dismounts far away, and mount up to reach a distant master, dismounting at assist range when the master is in combat. Unlike the original PR, the shared target-based mount/dismount logic is untouched and the feature is scoped entirely to the master-following branch, which battleground bots never enter. This removes the BG mounted-loop regression that caused the revert. Both helpers also require the follow strategy, so bots told to stay are unaffected. --- src/Ai/Base/Actions/CheckMountStateAction.cpp | 57 +++++++++++++++++++ src/Ai/Base/Actions/CheckMountStateAction.h | 2 + 2 files changed, 59 insertions(+) diff --git a/src/Ai/Base/Actions/CheckMountStateAction.cpp b/src/Ai/Base/Actions/CheckMountStateAction.cpp index 74faf8cd41f..597cd167055 100644 --- a/src/Ai/Base/Actions/CheckMountStateAction.cpp +++ b/src/Ai/Base/Actions/CheckMountStateAction.cpp @@ -125,10 +125,23 @@ bool CheckMountStateAction::Execute(Event /*event*/) else if (ShouldDismountForMaster(master) && bot->IsMounted()) { + // If master dismounted, stay mounted until close enough to assist - but only while + // the bot itself is safe. A bot in combat (or with attackers) always falls through + // to the normal dismount, so it can never get stuck mounted while being attacked. + if (noAttackers && !bot->IsInCombat() && botAI->GetState() != BOT_STATE_COMBAT && + StayMountedToCloseDistance()) + return false; + Dismount(); return true; } + // Mount up to close the distance to master if beneficial - allow mounting even if master + // is in combat, as long as the bot itself is not in combat and has no attackers + else if (!bot->IsMounted() && noAttackers && !bot->IsInCombat() && + botAI->GetState() != BOT_STATE_COMBAT && ShouldMountToCloseDistance()) + return Mount(); + return false; } @@ -455,6 +468,50 @@ bool CheckMountStateAction::TryRandomMountFiltered(const std::mapHasStrategy("follow", BOT_STATE_NON_COMBAT)) + return false; + + float distToMaster = ServerFacade::instance().GetDistance2d(bot, master); + + // If master is in combat, dismount at combat assist range to help immediately + if (master->IsInCombat()) + return distToMaster > CalculateDismountDistance(); + + // If master is not in combat, use smaller proximity range for general following + float masterProximityRange = 10.0f; // Close enough to be near master but not attack range + return distToMaster > masterProximityRange; +} + +bool CheckMountStateAction::ShouldMountToCloseDistance() const +{ + // Mount up to close the distance to master if beneficial. + // Uses CalculateMountDistance(), which already considers the mount cast time, so the bot + // only mounts when riding is actually faster than running. This also covers the case where + // the master is in combat but the bot is not, and the bot needs to mount to reach the master. + + if (!master) + return false; + + // Only mount to close distance when actively following + if (!botAI->HasStrategy("follow", BOT_STATE_NON_COMBAT)) + return false; + + float distToMaster = ServerFacade::instance().GetDistance2d(bot, master); + return distToMaster > CalculateMountDistance(); +} + float CheckMountStateAction::CalculateDismountDistance() const { // Warrior bots should dismount far enough to charge (because it's important for generating some initial rage), diff --git a/src/Ai/Base/Actions/CheckMountStateAction.h b/src/Ai/Base/Actions/CheckMountStateAction.h index 19adf86f752..dcf38177faf 100644 --- a/src/Ai/Base/Actions/CheckMountStateAction.h +++ b/src/Ai/Base/Actions/CheckMountStateAction.h @@ -63,6 +63,8 @@ class CheckMountStateAction : public UseItemAction bool TryPreferredMount(Player* master) const; uint32 GetMountType(Player* master) const; bool TryRandomMountFiltered(const std::map>& spells, int32 masterSpeed) const; + bool StayMountedToCloseDistance() const; + bool ShouldMountToCloseDistance() const; }; #endif From 40ecb76eee6d70e529feb37c0aea3e63037ea555 Mon Sep 17 00:00:00 2001 From: Tecc Date: Fri, 3 Jul 2026 18:03:28 +0000 Subject: [PATCH 03/10] fix: Address in-game test findings for mount-to-assist behavior - Gate the whole master mount-mirror block on the follow strategy: a bot told to stay is parked and no longer mounts when the master mounts - Clamp the in-combat dismount range to at least 18yd: for non-warrior melee CalculateDismountDistance() is ~3yd, which made the bot ride to the master's exact position and hover at follow distance without engaging - Reduce the out-of-combat proximity range to 5yd so the bot follows until near the master and then mirrors their mount state --- src/Ai/Base/Actions/CheckMountStateAction.cpp | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/Ai/Base/Actions/CheckMountStateAction.cpp b/src/Ai/Base/Actions/CheckMountStateAction.cpp index 597cd167055..46ee4489671 100644 --- a/src/Ai/Base/Actions/CheckMountStateAction.cpp +++ b/src/Ai/Base/Actions/CheckMountStateAction.cpp @@ -120,6 +120,11 @@ bool CheckMountStateAction::Execute(Event /*event*/) // If there is a master and bot not in BG, follow master's mount state regardless of group leader if (!noRealMaster && !inBattleground) { + // Only react to the master's mount state while actively following - a bot told to + // stay is parked and keeps its current mount state instead of mirroring the master + if (!botAI->HasStrategy("follow", BOT_STATE_NON_COMBAT)) + return false; + if (ShouldFollowMasterMountState(master, noAttackers, shouldMount)) return Mount(); @@ -478,19 +483,20 @@ bool CheckMountStateAction::StayMountedToCloseDistance() const if (!master) return false; - // Only applies while actively following - a bot told to stay should mirror the - // master's mount state as before instead of reacting to master distance - if (!botAI->HasStrategy("follow", BOT_STATE_NON_COMBAT)) - return false; - float distToMaster = ServerFacade::instance().GetDistance2d(bot, master); - // If master is in combat, dismount at combat assist range to help immediately + // If master is in combat, dismount at assist range. CalculateDismountDistance() alone is + // ~3yd for non-warrior melee, which reads as riding into the master's face and then + // hovering around the follow distance without ever engaging - clamp to a proper approach + // range so the bot dismounts clearly before the fight and closes the rest on foot. if (master->IsInCombat()) - return distToMaster > CalculateDismountDistance(); + { + float assistRange = std::max(18.0f, CalculateDismountDistance()); + return distToMaster > assistRange; + } - // If master is not in combat, use smaller proximity range for general following - float masterProximityRange = 10.0f; // Close enough to be near master but not attack range + // If master is not in combat, stay mounted until near the master, then mirror their state + float masterProximityRange = 5.0f; return distToMaster > masterProximityRange; } @@ -504,10 +510,6 @@ bool CheckMountStateAction::ShouldMountToCloseDistance() const if (!master) return false; - // Only mount to close distance when actively following - if (!botAI->HasStrategy("follow", BOT_STATE_NON_COMBAT)) - return false; - float distToMaster = ServerFacade::instance().GetDistance2d(bot, master); return distToMaster > CalculateMountDistance(); } From 12e2a027294286603207eef505ddde30adbf4acf Mon Sep 17 00:00:00 2001 From: Tecc Date: Fri, 3 Jul 2026 18:31:43 +0000 Subject: [PATCH 04/10] fix: Make the follow branch assist-aware A mob fighting the master resolves as the bot's dps target well before the bot itself is in combat, but the master-follow branch decided mount state only against the master. The bot rode past the fight to the master's exact position and could oscillate between assisting and re-mounting without attacking. When an assist target exists, dismount at engage range of the target (dismount distance + combat reach) and stop touching the mount state while next to it, so the dps/tank assist trigger can take over. Also drop the 18yd assist-range clamp again: dismount happens at the class combat distance. --- src/Ai/Base/Actions/CheckMountStateAction.cpp | 38 +++++++++++++++---- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/src/Ai/Base/Actions/CheckMountStateAction.cpp b/src/Ai/Base/Actions/CheckMountStateAction.cpp index 46ee4489671..9bf7d9c360b 100644 --- a/src/Ai/Base/Actions/CheckMountStateAction.cpp +++ b/src/Ai/Base/Actions/CheckMountStateAction.cpp @@ -125,6 +125,34 @@ bool CheckMountStateAction::Execute(Event /*event*/) if (!botAI->HasStrategy("follow", BOT_STATE_NON_COMBAT)) return false; + // Assist-aware mount handling: when the bot has an assist target (a mob fighting the + // master resolves as "dps target" long before the bot itself is in combat), decide + // against the target instead of the master. Without this the bot rides past the fight + // to the master's position and oscillates between assisting and re-mounting. + Unit* assistTarget = AI_VALUE(Unit*, "dps target"); + if (!assistTarget) + assistTarget = AI_VALUE(Unit*, "enemy player target"); + + if (assistTarget) + { + float reach = bot->GetCombatReach() + assistTarget->GetCombatReach(); + float distToTarget = bot->GetExactDist(assistTarget); + + // Close enough to engage: dismount so the assist strategies can take over + if (distToTarget <= CalculateDismountDistance() + reach) + { + if (bot->IsMounted()) + { + Dismount(); + return true; + } + + // Unmounted next to the assist target: leave mount state alone so the + // dps/tank assist trigger can act instead of re-mounting for the master + return false; + } + } + if (ShouldFollowMasterMountState(master, noAttackers, shouldMount)) return Mount(); @@ -485,15 +513,9 @@ bool CheckMountStateAction::StayMountedToCloseDistance() const float distToMaster = ServerFacade::instance().GetDistance2d(bot, master); - // If master is in combat, dismount at assist range. CalculateDismountDistance() alone is - // ~3yd for non-warrior melee, which reads as riding into the master's face and then - // hovering around the follow distance without ever engaging - clamp to a proper approach - // range so the bot dismounts clearly before the fight and closes the rest on foot. + // If master is in combat, stay mounted until combat reach, then dismount to assist if (master->IsInCombat()) - { - float assistRange = std::max(18.0f, CalculateDismountDistance()); - return distToMaster > assistRange; - } + return distToMaster > CalculateDismountDistance(); // If master is not in combat, stay mounted until near the master, then mirror their state float masterProximityRange = 5.0f; From f4af6947d0a4154d6bb087908ae409307215f515 Mon Sep 17 00:00:00 2001 From: Tecc Date: Fri, 3 Jul 2026 19:39:02 +0000 Subject: [PATCH 05/10] fix: Mirror the master's mount state only when near The mirror path had no distance condition, so a bot at 15yd mounted the moment the master did. Now: mirror within 5yd, walk between 5 and the mount threshold (21+ yd, where the cast time isn't worth it), and mount to close distance beyond that. Shared proximity constant for the mirror gate and the stay-mounted dismount range. --- src/Ai/Base/Actions/CheckMountStateAction.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Ai/Base/Actions/CheckMountStateAction.cpp b/src/Ai/Base/Actions/CheckMountStateAction.cpp index 9bf7d9c360b..35c05671476 100644 --- a/src/Ai/Base/Actions/CheckMountStateAction.cpp +++ b/src/Ai/Base/Actions/CheckMountStateAction.cpp @@ -19,6 +19,10 @@ static constexpr uint32 SPELL_COLD_WEATHER_FLYING = 54197; static constexpr float PARACHUTE_LAND_THRESHOLD = 15.0f; +// Range around the master inside which a following bot mirrors the master's mount state. +// Between this and CalculateMountDistance() (21+ yd) the bot just walks - mounting there +// costs more time (cast) than it saves. +static constexpr float MASTER_PROXIMITY_RANGE = 5.0f; // Define the static map / init bool for caching bot preferred mount data globally std::unordered_map CheckMountStateAction::mountCache; @@ -153,7 +157,13 @@ bool CheckMountStateAction::Execute(Event /*event*/) } } - if (ShouldFollowMasterMountState(master, noAttackers, shouldMount)) + float distToMaster = ServerFacade::instance().GetDistance2d(bot, master); + + // Mirror the master's mount state only when near: farther out the bot either walks + // (5-21 yd, mounting wouldn't pay for its cast time) or mounts to close a real gap + // (ShouldMountToCloseDistance, 21+ yd) + if (distToMaster <= MASTER_PROXIMITY_RANGE && + ShouldFollowMasterMountState(master, noAttackers, shouldMount)) return Mount(); else if (ShouldDismountForMaster(master) && bot->IsMounted()) @@ -518,8 +528,7 @@ bool CheckMountStateAction::StayMountedToCloseDistance() const return distToMaster > CalculateDismountDistance(); // If master is not in combat, stay mounted until near the master, then mirror their state - float masterProximityRange = 5.0f; - return distToMaster > masterProximityRange; + return distToMaster > MASTER_PROXIMITY_RANGE; } bool CheckMountStateAction::ShouldMountToCloseDistance() const From c3617814101ad14f05cfcdd5d5762adfe4c16d7d Mon Sep 17 00:00:00 2001 From: Tecc Date: Fri, 3 Jul 2026 19:47:06 +0000 Subject: [PATCH 06/10] refactor: Use TooCloseDistance config for the near-master mirror range Replaces the hardcoded proximity constant with the existing (previously unused) AiPlayerbot.TooCloseDistance option, default 5. --- src/Ai/Base/Actions/CheckMountStateAction.cpp | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/Ai/Base/Actions/CheckMountStateAction.cpp b/src/Ai/Base/Actions/CheckMountStateAction.cpp index 35c05671476..90aff1ccd73 100644 --- a/src/Ai/Base/Actions/CheckMountStateAction.cpp +++ b/src/Ai/Base/Actions/CheckMountStateAction.cpp @@ -19,10 +19,6 @@ static constexpr uint32 SPELL_COLD_WEATHER_FLYING = 54197; static constexpr float PARACHUTE_LAND_THRESHOLD = 15.0f; -// Range around the master inside which a following bot mirrors the master's mount state. -// Between this and CalculateMountDistance() (21+ yd) the bot just walks - mounting there -// costs more time (cast) than it saves. -static constexpr float MASTER_PROXIMITY_RANGE = 5.0f; // Define the static map / init bool for caching bot preferred mount data globally std::unordered_map CheckMountStateAction::mountCache; @@ -159,10 +155,10 @@ bool CheckMountStateAction::Execute(Event /*event*/) float distToMaster = ServerFacade::instance().GetDistance2d(bot, master); - // Mirror the master's mount state only when near: farther out the bot either walks - // (5-21 yd, mounting wouldn't pay for its cast time) or mounts to close a real gap - // (ShouldMountToCloseDistance, 21+ yd) - if (distToMaster <= MASTER_PROXIMITY_RANGE && + // Mirror the master's mount state only when near (TooCloseDistance, default 5 yd): + // farther out the bot either walks (mounting wouldn't pay for its cast time) or + // mounts to close a real gap (ShouldMountToCloseDistance, 21+ yd) + if (distToMaster <= sPlayerbotAIConfig.tooCloseDistance && ShouldFollowMasterMountState(master, noAttackers, shouldMount)) return Mount(); @@ -528,7 +524,7 @@ bool CheckMountStateAction::StayMountedToCloseDistance() const return distToMaster > CalculateDismountDistance(); // If master is not in combat, stay mounted until near the master, then mirror their state - return distToMaster > MASTER_PROXIMITY_RANGE; + return distToMaster > sPlayerbotAIConfig.tooCloseDistance; } bool CheckMountStateAction::ShouldMountToCloseDistance() const From 5bb3698b3bbed53753936de58ef9d78285c8155d Mon Sep 17 00:00:00 2001 From: Tecc Date: Sat, 4 Jul 2026 16:34:44 +0000 Subject: [PATCH 07/10] fix: Return the freshly-resolved master, not the cached raw pointer GetMaster() validated the master GUID via ObjectAccessor::FindConnectedPlayer but then returned the cached raw pointer, which is exactly the stale pointer the validation was meant to guard against: after a same-GUID relog the lookup finds the new Player object yet the old (freed) pointer was handed back. Return the resolved object instead - a logged-out master yields nullptr, a relogged one yields the fresh Player. --- src/Bot/PlayerbotAI.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Bot/PlayerbotAI.cpp b/src/Bot/PlayerbotAI.cpp index fac71fc5140..ca1714dbcef 100644 --- a/src/Bot/PlayerbotAI.cpp +++ b/src/Bot/PlayerbotAI.cpp @@ -4503,16 +4503,18 @@ Player* PlayerbotAI::GetMaster() if (!master) return nullptr; + // A real player is its own master; that pointer is never validated here (out of scope). + if (master == bot) + return master; + // Never hand out a stale pointer: the master Player can be destroyed (logout) between - // AI ticks on the map-update threads while bots still hold the raw pointer. Re-validate - // through the ObjectAccessor by GUID, which never dereferences the stored pointer. + // AI ticks on the map-update threads while bots still hold the raw pointer. Re-resolve + // through the ObjectAccessor by GUID and return THAT object, never the cached raw pointer + // - a fast same-GUID relog yields a fresh Player, a logged-out master yields nullptr. // FindConnectedPlayer (not FindPlayer): the master must still count as present while // merely loading/teleporting between maps, otherwise every zone transition makes bots // transiently masterless (rejected whisper commands, spurious master resets). - if (master != bot && (!masterGuid || !ObjectAccessor::FindConnectedPlayer(masterGuid))) - return nullptr; - - return master; + return masterGuid ? ObjectAccessor::FindConnectedPlayer(masterGuid) : nullptr; } void PlayerbotAI::SetMaster(Player* newMaster) From 150d7e9cee5f489e57286ab08179e6f1408a6281 Mon Sep 17 00:00:00 2001 From: Tecc Date: Sat, 4 Jul 2026 16:34:44 +0000 Subject: [PATCH 08/10] refactor: Pass distToMaster into the mount helpers instead of recomputing Execute() already computes distToMaster once per tick; StayMountedToCloseDistance and ShouldMountToCloseDistance each recomputed the same GetDistance2d, so a mounted bot near a dismounted master ran the distance calc three times per tick. Thread the value through - no behaviour change. --- src/Ai/Base/Actions/CheckMountStateAction.cpp | 11 ++++------- src/Ai/Base/Actions/CheckMountStateAction.h | 4 ++-- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Ai/Base/Actions/CheckMountStateAction.cpp b/src/Ai/Base/Actions/CheckMountStateAction.cpp index 90aff1ccd73..9d43d02f5c6 100644 --- a/src/Ai/Base/Actions/CheckMountStateAction.cpp +++ b/src/Ai/Base/Actions/CheckMountStateAction.cpp @@ -168,7 +168,7 @@ bool CheckMountStateAction::Execute(Event /*event*/) // the bot itself is safe. A bot in combat (or with attackers) always falls through // to the normal dismount, so it can never get stuck mounted while being attacked. if (noAttackers && !bot->IsInCombat() && botAI->GetState() != BOT_STATE_COMBAT && - StayMountedToCloseDistance()) + StayMountedToCloseDistance(distToMaster)) return false; Dismount(); @@ -178,7 +178,7 @@ bool CheckMountStateAction::Execute(Event /*event*/) // Mount up to close the distance to master if beneficial - allow mounting even if master // is in combat, as long as the bot itself is not in combat and has no attackers else if (!bot->IsMounted() && noAttackers && !bot->IsInCombat() && - botAI->GetState() != BOT_STATE_COMBAT && ShouldMountToCloseDistance()) + botAI->GetState() != BOT_STATE_COMBAT && ShouldMountToCloseDistance(distToMaster)) return Mount(); return false; @@ -507,7 +507,7 @@ bool CheckMountStateAction::TryRandomMountFiltered(const std::mapIsInCombat()) return distToMaster > CalculateDismountDistance(); @@ -527,7 +525,7 @@ bool CheckMountStateAction::StayMountedToCloseDistance() const return distToMaster > sPlayerbotAIConfig.tooCloseDistance; } -bool CheckMountStateAction::ShouldMountToCloseDistance() const +bool CheckMountStateAction::ShouldMountToCloseDistance(float distToMaster) const { // Mount up to close the distance to master if beneficial. // Uses CalculateMountDistance(), which already considers the mount cast time, so the bot @@ -537,7 +535,6 @@ bool CheckMountStateAction::ShouldMountToCloseDistance() const if (!master) return false; - float distToMaster = ServerFacade::instance().GetDistance2d(bot, master); return distToMaster > CalculateMountDistance(); } diff --git a/src/Ai/Base/Actions/CheckMountStateAction.h b/src/Ai/Base/Actions/CheckMountStateAction.h index dcf38177faf..268e077a69b 100644 --- a/src/Ai/Base/Actions/CheckMountStateAction.h +++ b/src/Ai/Base/Actions/CheckMountStateAction.h @@ -63,8 +63,8 @@ class CheckMountStateAction : public UseItemAction bool TryPreferredMount(Player* master) const; uint32 GetMountType(Player* master) const; bool TryRandomMountFiltered(const std::map>& spells, int32 masterSpeed) const; - bool StayMountedToCloseDistance() const; - bool ShouldMountToCloseDistance() const; + bool StayMountedToCloseDistance(float distToMaster) const; + bool ShouldMountToCloseDistance(float distToMaster) const; }; #endif From e49da6060f99916545d1265dbdb7ed38db960924 Mon Sep 17 00:00:00 2001 From: Tecc Date: Tue, 8 Sep 2026 11:30:24 +0000 Subject: [PATCH 09/10] fix: Re-sync the cached master on any change, not only on loss UpdateAIGroupMaster only cleared the raw master pointer when the master resolved to nullptr. A same-GUID relog yields a fresh Player object, so GetMaster() returns the new one while the cached raw pointer still points at the destroyed one, and every raw master dereference further down the function is a use-after-free until PlayerbotMgr happens to call SetMaster again. Compare against the re-resolved master instead. --- src/Bot/PlayerbotAI.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Bot/PlayerbotAI.cpp b/src/Bot/PlayerbotAI.cpp index ca1714dbcef..dc8e795b40f 100644 --- a/src/Bot/PlayerbotAI.cpp +++ b/src/Bot/PlayerbotAI.cpp @@ -420,10 +420,13 @@ void PlayerbotAI::UpdateAIGroupMaster() if (!botAI) return; - // Drop a stale master pointer (master logged out and got destroyed between AI ticks) - // before anything below dereferences it - if (master && !GetMaster()) - SetMaster(nullptr); + // Re-sync the cached master pointer before anything below dereferences it: the master + // Player can be destroyed between AI ticks (logout, or a same-GUID relog that yields a + // fresh object). Compare against the re-resolved master, not just against nullptr, so a + // swapped-out object is picked up too and not only a lost one. + Player* revalidatedMaster = GetMaster(); + if (master != revalidatedMaster) + SetMaster(revalidatedMaster); Group* group = bot->GetGroup(); From c4c328fe7ea3fbe4f75754d2ee989094d3aebcc4 Mon Sep 17 00:00:00 2001 From: Tecc Date: Tue, 8 Sep 2026 13:03:24 +0000 Subject: [PATCH 10/10] fix: Mirror the master's mount state whenever the master is riding, not only when near The near-only mirror gate left a dead band between TooCloseDistance and CalculateMountDistance() where neither the mirror branch nor the mount-to-close branch fires: 5-21 yd for melee, and 5-38.5 yd for casters, since CalculateMountDistance() is max(21, SpellDistance + 10) and SpellDistance defaults to 28.5. CalculateMountDistance() is the break-even for closing a FIXED gap, so it is the wrong test against a master who is actively opening one. And a ground mount matches the master's speed rather than beating it, so a bot that waits for the gap to cross the threshold does not claw it back - it holds that distance for the rest of the ride. A caster trailing 38 yd behind is the visible symptom. Mirror when near or when the master is moving. ShouldFollowMasterMountState already requires the master mounted, the bot unmounted, no attackers and the bot out of combat, so the added clause only fires for a safe bot on foot behind a moving mounted master. A stationary master keeps the deliberate near-only rule. --- src/Ai/Base/Actions/CheckMountStateAction.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Ai/Base/Actions/CheckMountStateAction.cpp b/src/Ai/Base/Actions/CheckMountStateAction.cpp index 9d43d02f5c6..4deb1a752d1 100644 --- a/src/Ai/Base/Actions/CheckMountStateAction.cpp +++ b/src/Ai/Base/Actions/CheckMountStateAction.cpp @@ -155,10 +155,15 @@ bool CheckMountStateAction::Execute(Event /*event*/) float distToMaster = ServerFacade::instance().GetDistance2d(bot, master); - // Mirror the master's mount state only when near (TooCloseDistance, default 5 yd): - // farther out the bot either walks (mounting wouldn't pay for its cast time) or - // mounts to close a real gap (ShouldMountToCloseDistance, 21+ yd) - if (distToMaster <= sPlayerbotAIConfig.tooCloseDistance && + // Mirror the master's mount state when near (TooCloseDistance, default 5 yd), or + // whenever the master is actually riding away. Without the second clause there is a + // dead band between TooCloseDistance and CalculateMountDistance() where neither rule + // fires: 5-21 yd for melee, 5-38.5 yd for casters (max(21, SpellDistance + 10), and + // SpellDistance defaults to 28.5). CalculateMountDistance() is a break-even for a + // FIXED gap, so it is the wrong test against a master who is opening the gap - and + // because a ground mount matches the master's speed rather than beating it, a bot + // that waits to cross it then holds that whole distance until the master stops. + if ((distToMaster <= sPlayerbotAIConfig.tooCloseDistance || master->isMoving()) && ShouldFollowMasterMountState(master, noAttackers, shouldMount)) return Mount();