[DO NOT MERGE] Mount behavior v2 - testing branch, destined for upstream

src/Ai/Base/Actions/CheckMountStateAction.cpp

@@ -120,15 +120,72 @@ 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)
     {
-        if (ShouldFollowMasterMountState(master, noAttackers, shouldMount))
+        // 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;
+
+        // 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;
+            }
+        }
+
+        float distToMaster = ServerFacade::instance().GetDistance2d(bot, master);
+
+        // 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();
 
         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(distToMaster))
+                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(distToMaster))
+            return Mount();
+
         return false;
     }
 
@@ -455,6 +512,37 @@ bool CheckMountStateAction::TryRandomMountFiltered(const std::map<int32, std::ve
     return false;
 }
 
+bool CheckMountStateAction::StayMountedToCloseDistance(float distToMaster) const
+{
+    // Keep the bot mounted while closing distance to a recently dismounted master.
+    // Rationale: if the master dismounts far away, immediately dismounting slows the bot down
+    // and delays assistance. Instead, remain mounted until within reasonable proximity
+    // of the master, then dismount to help.
+
+    if (!master)
+        return false;
+
+    // If master is in combat, stay mounted until combat reach, then dismount to assist
+    if (master->IsInCombat())
+        return distToMaster > CalculateDismountDistance();
+
+    // If master is not in combat, stay mounted until near the master, then mirror their state
+    return distToMaster > sPlayerbotAIConfig.tooCloseDistance;
+}
+
+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
+    // 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;
+
+    return distToMaster > CalculateMountDistance();
+}
+
 float CheckMountStateAction::CalculateDismountDistance() const
 {
     // Warrior bots should dismount far enough to charge (because it's important for generating some initial rage),

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<int32, std::vector<uint32>>& spells, int32 masterSpeed) const;
+    bool StayMountedToCloseDistance(float distToMaster) const;
+    bool ShouldMountToCloseDistance(float distToMaster) const;
 };
 
 #endif

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);

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,14 @@ void PlayerbotAI::UpdateAIGroupMaster()
     if (!botAI)
         return;
 
+    // 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();
 
     // If bot is not in group verify that for is RandomBot before clearing  master and resetting.
@@ -446,7 +456,7 @@ void PlayerbotAI::UpdateAIGroupMaster()
         Player* newMaster = FindNewMaster();
         if (newMaster)
         {
-            master = newMaster;
+            SetMaster(newMaster);
             botAI->SetMaster(newMaster);
             botAI->ResetStrategies();
 
@@ -1059,8 +1069,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 +1084,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 +3079,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 +3092,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 +4493,38 @@ 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;
+
+    // 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-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).
+    return masterGuid ? ObjectAccessor::FindConnectedPlayer(masterGuid) : nullptr;
+}
+
+void PlayerbotAI::SetMaster(Player* newMaster)
+{
+    master = newMaster;
+    masterGuid = newMaster ? newMaster->GetGUID() : ObjectGuid::Empty;
+}
 
 Player* PlayerbotAI::GetGroupLeader()
 {
@@ -4486,7 +4533,7 @@ Player* PlayerbotAI::GetGroupLeader()
             if (Player* player = ObjectAccessor::FindPlayer(group->GetLeaderGUID()))
                 return player;
 
-    return master;
+    return GetMaster();
 }
 
 uint32 PlayerbotAI::GetFixedBotNumber(uint32 maxNum)

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;