Merge pull request #1708 from hermensbas/feature/removeFromGroup_replaced_with_worldpackets

[fix crash] Crash on removeFromGroup

src/PlayerbotAI.cpp

@@ -378,10 +378,7 @@ void PlayerbotAI::UpdateAIGroupMembership()
             PlayerbotAI* leaderAI = GET_PLAYERBOT_AI(leader);
             if (leaderAI && !leaderAI->IsRealPlayer())
             {
-                WorldPacket* packet = new WorldPacket(CMSG_GROUP_DISBAND);
-                bot->GetSession()->QueuePacket(packet);
-                // bot->RemoveFromGroup();
-                ResetStrategies();
+                LeaveOrDisbandGroup();
             }
         }
     }
@@ -405,10 +402,7 @@ void PlayerbotAI::UpdateAIGroupMembership()
         }
         if (!hasRealPlayer)
         {
-            WorldPacket* packet = new WorldPacket(CMSG_GROUP_DISBAND);
-            bot->GetSession()->QueuePacket(packet);
-            // bot->RemoveFromGroup();
-            ResetStrategies();
+            LeaveOrDisbandGroup();
         }
     }
 }
@@ -791,6 +785,16 @@ void PlayerbotAI::Reset(bool full)
     }
 }
 
+void PlayerbotAI::LeaveOrDisbandGroup()
+{
+    if (!bot || !bot->GetGroup() || IsRealPlayer())
+        return;
+
+    WorldPacket* packet = new WorldPacket(CMSG_GROUP_DISBAND);
+    bot->GetSession()->QueuePacket(packet);
+    ResetStrategies();
+}
+
 bool PlayerbotAI::IsAllowedCommand(std::string const text)
 {
     if (unsecuredCommands.empty())

src/PlayerbotAI.h

@@ -415,6 +415,7 @@ public:
     void ResetStrategies(bool load = false);
     void ReInitCurrentEngine();
     void Reset(bool full = false);
+    void LeaveOrDisbandGroup();
     static bool IsTank(Player* player, bool bySpec = false);
     static bool IsHeal(Player* player, bool bySpec = false);
     static bool IsDps(Player* player, bool bySpec = false);
@@ -601,6 +602,7 @@ public:
     NewRpgInfo rpgInfo;
     NewRpgStatistic rpgStatistic;
     std::unordered_set<uint32> lowPriorityQuest;
+    time_t bgReleaseAttemptTime = 0;
 
     // Schedules a callback to run once after <delayMs> milliseconds.
     void AddTimedEvent(std::function<void()> callback, uint32 delayMs);

src/PlayerbotMgr.cpp

@@ -514,7 +514,7 @@ void PlayerbotHolder::OnBotLogin(Player* const bot)
 
         if (!groupValid)
         {
-            bot->RemoveFromGroup();
+            botAI->LeaveOrDisbandGroup();
         }
     }
 

src/Playerbots.cpp

@@ -87,7 +87,8 @@ public:
         PLAYERHOOK_ON_BEFORE_CRITERIA_PROGRESS,
         PLAYERHOOK_ON_BEFORE_ACHI_COMPLETE,
         PLAYERHOOK_CAN_PLAYER_USE_PRIVATE_CHAT,
-        PLAYERHOOK_ON_GIVE_EXP
+        PLAYERHOOK_ON_GIVE_EXP,
+        PLAYERHOOK_ON_BEFORE_TELEPORT
     }) {}
 
     void OnPlayerLogin(Player* player) override
@@ -121,6 +122,26 @@ public:
         }
     }
 
+    bool OnPlayerBeforeTeleport(Player* player, uint32 mapid, float /*x*/, float /*y*/, float /*z*/, float /*orientation*/, uint32 /*options*/, Unit* /*target*/) override
+    {
+        // Only apply to bots to prevent affecting real players
+        if (!player || !player->GetSession()->IsBot())
+            return true;
+
+        // If changing maps, proactively clean visibility references to prevent
+        // stale pointers in other players' visibility maps during the teleport.
+        // This fixes a race condition where:
+        // 1. Bot A teleports and its visible objects start getting cleaned up
+        // 2. Bot B is simultaneously updating visibility and tries to access objects in Bot A's old visibility map
+        // 3. Those objects may already be freed, causing a segmentation fault
+        if (player->GetMapId() != mapid && player->IsInWorld())
+        {
+            player->GetObjectVisibilityContainer().CleanVisibilityReferences();
+        }
+
+        return true;  // Allow teleport to continue
+    }
+
     void OnPlayerAfterUpdate(Player* player, uint32 diff) override
     {
         if (PlayerbotAI* botAI = GET_PLAYERBOT_AI(player))

src/RandomPlayerbotMgr.cpp

@@ -1517,33 +1517,38 @@ bool RandomPlayerbotMgr::ProcessBot(uint32 bot)
     return false;
 }
 
-bool RandomPlayerbotMgr::ProcessBot(Player* player)
+bool RandomPlayerbotMgr::ProcessBot(Player* bot)
 {
-    uint32 bot = player->GetGUID().GetCounter();
 
-    if (player->InBattleground())
+    PlayerbotAI* botAI = GET_PLAYERBOT_AI(bot);
+    if (!botAI)
         return false;
 
-    if (player->InBattlegroundQueue())
+    if (bot->InBattleground())
         return false;
 
+    if (bot->InBattlegroundQueue())
+        return false;
+
+     uint32 botId = bot->GetGUID().GetCounter();
+
     // if death revive
-    if (player->isDead())
+    if (bot->isDead())
     {
-        if (!GetEventValue(bot, "dead"))
+        if (!GetEventValue(botId, "dead"))
         {
             uint32 randomTime =
                 urand(sPlayerbotAIConfig->minRandomBotReviveTime, sPlayerbotAIConfig->maxRandomBotReviveTime);
-            LOG_DEBUG("playerbots", "Mark bot {} as dead, will be revived in {}s.", player->GetName().c_str(),
+            LOG_DEBUG("playerbots", "Mark bot {} as dead, will be revived in {}s.", bot->GetName().c_str(),
                       randomTime);
-            SetEventValue(bot, "dead", 1, sPlayerbotAIConfig->maxRandomBotInWorldTime);
-            SetEventValue(bot, "revive", 1, randomTime);
+            SetEventValue(botId, "dead", 1, sPlayerbotAIConfig->maxRandomBotInWorldTime);
+            SetEventValue(botId, "revive", 1, randomTime);
             return false;
         }
 
-        if (!GetEventValue(bot, "revive"))
+        if (!GetEventValue(botId, "revive"))
         {
-            Revive(player);
+            Revive(bot);
             return true;
         }
 
@@ -1551,34 +1556,31 @@ bool RandomPlayerbotMgr::ProcessBot(Player* player)
     }
 
     // leave group if leader is rndbot
-    Group* group = player->GetGroup();
+    Group* group = bot->GetGroup();
     if (group && !group->isLFGGroup() && IsRandomBot(group->GetLeader()))
     {
-        player->RemoveFromGroup();
-        LOG_INFO("playerbots", "Bot {} remove from group since leader is random bot.", player->GetName().c_str());
+        botAI->LeaveOrDisbandGroup();
+        LOG_INFO("playerbots", "Bot {} remove from group since leader is random bot.", bot->GetName().c_str());
     }
 
     // only randomize and teleport idle bots
     bool idleBot = false;
-    PlayerbotAI* botAI = GET_PLAYERBOT_AI(player);
-    if (botAI)
+    if (TravelTarget* target = botAI->GetAiObjectContext()->GetValue<TravelTarget*>("travel target")->Get())
     {
-        if (TravelTarget* target = botAI->GetAiObjectContext()->GetValue<TravelTarget*>("travel target")->Get())
-        {
-            if (target->getTravelState() == TravelState::TRAVEL_STATE_IDLE)
-            {
-                idleBot = true;
-            }
-        }
-        else
+        if (target->getTravelState() == TravelState::TRAVEL_STATE_IDLE)
         {
             idleBot = true;
         }
     }
+    else
+    {
+        idleBot = true;
+    }
+    
     if (idleBot)
     {
         // randomize
-        uint32 randomize = GetEventValue(bot, "randomize");
+        uint32 randomize = GetEventValue(botId, "randomize");
         if (!randomize)
         {
             // bool randomiser = true;
@@ -1602,12 +1604,12 @@ bool RandomPlayerbotMgr::ProcessBot(Player* player)
             // }
             // if (randomiser)
             // {
-            Randomize(player);
-            LOG_DEBUG("playerbots", "Bot #{} {}:{} <{}>: randomized", bot,
-                      player->GetTeamId() == TEAM_ALLIANCE ? "A" : "H", player->GetLevel(), player->GetName());
+            Randomize(bot);
+            LOG_DEBUG("playerbots", "Bot #{} {}:{} <{}>: randomized", botId,
+                      bot->GetTeamId() == TEAM_ALLIANCE ? "A" : "H", bot->GetLevel(), bot->GetName());
             uint32 randomTime =
                 urand(sPlayerbotAIConfig->minRandomBotRandomizeTime, sPlayerbotAIConfig->maxRandomBotRandomizeTime);
-            ScheduleRandomize(bot, randomTime);
+            ScheduleRandomize(botId, randomTime);
             return true;
         }
 
@@ -1619,15 +1621,15 @@ bool RandomPlayerbotMgr::ProcessBot(Player* player)
         //     return true;
         // }
 
-        uint32 teleport = GetEventValue(bot, "teleport");
+        uint32 teleport = GetEventValue(botId, "teleport");
         if (!teleport)
         {
-            LOG_DEBUG("playerbots", "Bot #{} <{}>: teleport for level and refresh", bot, player->GetName());
-            Refresh(player);
-            RandomTeleportForLevel(player);
+            LOG_DEBUG("playerbots", "Bot #{} <{}>: teleport for level and refresh", botId, bot->GetName());
+            Refresh(bot);
+            RandomTeleportForLevel(bot);
             uint32 time = urand(sPlayerbotAIConfig->minRandomBotTeleportInterval,
                                 sPlayerbotAIConfig->maxRandomBotTeleportInterval);
-            ScheduleTeleport(bot, time);
+            ScheduleTeleport(botId, time);
             return true;
         }
     }
@@ -1771,6 +1773,7 @@ void RandomPlayerbotMgr::RandomTeleport(Player* bot, std::vector<WorldLocation>&
         PlayerbotAI* botAI = GET_PLAYERBOT_AI(bot);
         if (botAI)
             botAI->Reset(true);
+        bot->RemoveAurasWithInterruptFlags(AURA_INTERRUPT_FLAG_TELEPORTED | AURA_INTERRUPT_FLAG_CHANGE_MAP);
         bot->TeleportTo(loc.GetMapId(), x, y, z, 0);
         bot->SendMovementFlagUpdate();
 
@@ -2376,6 +2379,10 @@ void RandomPlayerbotMgr::IncreaseLevel(Player* bot)
 
 void RandomPlayerbotMgr::RandomizeFirst(Player* bot)
 {
+    PlayerbotAI* botAI = GET_PLAYERBOT_AI(bot);
+    if (!botAI)
+        return;
+
     uint32 maxLevel = sPlayerbotAIConfig->randomBotMaxLevel;
     if (maxLevel > sWorld->getIntConfig(CONFIG_MAX_PLAYER_LEVEL))
         maxLevel = sWorld->getIntConfig(CONFIG_MAX_PLAYER_LEVEL);
@@ -2433,7 +2440,6 @@ void RandomPlayerbotMgr::RandomizeFirst(Player* bot)
     }
 
     SetValue(bot, "level", level);
-
     PlayerbotFactory factory(bot, level);
     factory.Randomize(false);
 
@@ -2455,11 +2461,10 @@ void RandomPlayerbotMgr::RandomizeFirst(Player* bot)
     PlayerbotsDatabase.Execute(stmt);
 
     // teleport to a random inn for bot level
-    if (GET_PLAYERBOT_AI(bot))
-        GET_PLAYERBOT_AI(bot)->Reset(true);
+    botAI->Reset(true);
 
     if (bot->GetGroup())
-        bot->RemoveFromGroup();
+        botAI->LeaveOrDisbandGroup();
 
     if (pmo)
         pmo->finish();
@@ -2469,12 +2474,13 @@ void RandomPlayerbotMgr::RandomizeFirst(Player* bot)
 
 void RandomPlayerbotMgr::RandomizeMin(Player* bot)
 {
+    PlayerbotAI* botAI = GET_PLAYERBOT_AI(bot);
+    if (!botAI)
+        return;
+
     PerformanceMonitorOperation* pmo = sPerformanceMonitor->start(PERF_MON_RNDBOT, "RandomizeMin");
-
     uint32 level = sPlayerbotAIConfig->randomBotMinLevel;
-
     SetValue(bot, "level", level);
-
     PlayerbotFactory factory(bot, level);
     factory.Randomize(false);
 
@@ -2496,11 +2502,10 @@ void RandomPlayerbotMgr::RandomizeMin(Player* bot)
     PlayerbotsDatabase.Execute(stmt);
 
     // teleport to a random inn for bot level
-    if (GET_PLAYERBOT_AI(bot))
-        GET_PLAYERBOT_AI(bot)->Reset(true);
+    botAI->Reset(true);
 
     if (bot->GetGroup())
-        bot->RemoveFromGroup();
+        botAI->LeaveOrDisbandGroup();
 
     if (pmo)
         pmo->finish();
@@ -2582,7 +2587,7 @@ void RandomPlayerbotMgr::Refresh(Player* bot)
     bot->SetMoney(money + 500 * sqrt(urand(1, bot->GetLevel() * 5)));
 
     if (bot->GetGroup())
-        bot->RemoveFromGroup();
+        botAI->LeaveOrDisbandGroup();
 
     if (pmo)
         pmo->finish();
@@ -3090,6 +3095,7 @@ void RandomPlayerbotMgr::OnPlayerLogin(Player* player)
             } while (true);
         }
 
+        player->RemoveAurasWithInterruptFlags(AURA_INTERRUPT_FLAG_TELEPORTED | AURA_INTERRUPT_FLAG_CHANGE_MAP);
         player->TeleportTo(botPos);
 
         // player->Relocate(botPos.getX(), botPos.getY(), botPos.getZ(), botPos.getO());

src/strategy/actions/BattleGroundJoinAction.cpp

@@ -176,6 +176,7 @@ bool BGJoinAction::gatherArenaTeam(ArenaType type)
             continue;
 
         memberBotAI->Reset();
+        member->RemoveAurasWithInterruptFlags(AURA_INTERRUPT_FLAG_TELEPORTED | AURA_INTERRUPT_FLAG_CHANGE_MAP);
         member->TeleportTo(bot->GetMapId(), bot->GetPositionX(), bot->GetPositionY(), bot->GetPositionZ(), 0);
 
         LOG_INFO("playerbots", "Bot {} <{}>: Member of <{}>", member->GetGUID().ToString().c_str(),

src/strategy/actions/BattleGroundTactics.cpp

@@ -4289,9 +4289,15 @@ bool ArenaTactics::moveToCenter(Battleground* bg)
             {
                 // they like to hang around at the tip of the pipes doing nothing, so we just teleport them down
                 if (bot->GetDistance(1333.07f, 817.18f, 13.35f) < 4)
+                {
+                    bot->RemoveAurasWithInterruptFlags(AURA_INTERRUPT_FLAG_TELEPORTED | AURA_INTERRUPT_FLAG_CHANGE_MAP);
                     bot->TeleportTo(bg->GetMapId(), 1330.96f, 816.75f, 3.2f, bot->GetOrientation());
+                }
                 if (bot->GetDistance(1250.13f, 764.79f, 13.34f) < 4)
+                {
+                    bot->RemoveAurasWithInterruptFlags(AURA_INTERRUPT_FLAG_TELEPORTED | AURA_INTERRUPT_FLAG_CHANGE_MAP);
                     bot->TeleportTo(bg->GetMapId(), 1252.19f, 765.41f, 3.2f, bot->GetOrientation());
+                }
             }
             break;
         case BATTLEGROUND_RV:

src/strategy/actions/ChatShortcutActions.cpp

@@ -106,6 +106,7 @@ bool FollowChatShortcutAction::Execute(Event event)
         else
             botAI->TellMaster("You are too far away from me! I will there soon.");
 
+        bot->RemoveAurasWithInterruptFlags(AURA_INTERRUPT_FLAG_TELEPORTED | AURA_INTERRUPT_FLAG_CHANGE_MAP);
         bot->TeleportTo(master->GetMapId(), master->GetPositionX(), master->GetPositionY(), master->GetPositionZ(),
     master->GetOrientation()); return true;
     }

src/strategy/actions/LeaveGroupAction.cpp

@@ -72,8 +72,10 @@ bool UninviteAction::Execute(Event event)
 
 bool LeaveGroupAction::Leave(Player* player)
 {
-    if (player && !GET_PLAYERBOT_AI(player) &&
+    if (player &&
+        !botAI &&
         !botAI->GetSecurity()->CheckLevelFor(PLAYERBOT_SECURITY_INVITE, false, player))
+
         return false;
 
     bool aiMaster = GET_PLAYERBOT_AI(botAI->GetMaster()) != nullptr;
@@ -84,7 +86,7 @@ bool LeaveGroupAction::Leave(Player* player)
     bool shouldStay = randomBot && bot->GetGroup() && player == bot;
     if (!shouldStay)
     {
-        bot->RemoveFromGroup();
+        botAI->LeaveOrDisbandGroup();
     }
 
     if (randomBot)

src/strategy/actions/MovementActions.cpp

@@ -1148,6 +1148,7 @@ bool MovementAction::Follow(Unit* target, float distance, float angle)
             if ((target->GetMap() && target->GetMap()->IsBattlegroundOrArena()) || (bot->GetMap() &&
     bot->GetMap()->IsBattlegroundOrArena())) return false;
 
+            bot->RemoveAurasWithInterruptFlags(AURA_INTERRUPT_FLAG_TELEPORTED | AURA_INTERRUPT_FLAG_CHANGE_MAP);
             bot->TeleportTo(target->GetMapId(), x, y, z, bot->GetOrientation());
         }
         else
@@ -1175,6 +1176,7 @@ bool MovementAction::Follow(Unit* target, float distance, float angle)
 
         bot->CombatStop(true);
         botAI->TellMasterNoFacing("I will there soon.");
+        bot->RemoveAurasWithInterruptFlags(AURA_INTERRUPT_FLAG_TELEPORTED | AURA_INTERRUPT_FLAG_CHANGE_MAP);
         bot->TeleportTo(target->GetMapId(), target->GetPositionX(), target->GetPositionY(), target->GetPositionZ(),
     target->GetOrientation()); return false;
     }

src/strategy/actions/ReleaseSpiritAction.cpp

@@ -147,6 +147,7 @@ bool AutoReleaseSpiritAction::HandleBattlegroundSpiritHealer()
         // and in IOC it's not within clicking range when they res in own base
 
         // Teleport to nearest friendly Spirit Healer when not currently in range of one.
+        bot->RemoveAurasWithInterruptFlags(AURA_INTERRUPT_FLAG_TELEPORTED | AURA_INTERRUPT_FLAG_CHANGE_MAP);
         bot->TeleportTo(bot->GetMapId(), spiritHealer->GetPositionX(), spiritHealer->GetPositionY(), spiritHealer->GetPositionZ(), 0.f);
         RESET_AI_VALUE(bool, "combat::self target");
         RESET_AI_VALUE(WorldPosition, "current position");
@@ -191,12 +192,11 @@ bool AutoReleaseSpiritAction::ShouldDelayBattlegroundRelease() const
 {
     // The below delays release to spirit with 6 seconds.
     // This prevents currently casted (ranged) spells to be re-directed to the died bot's ghost.
-    const int32_t botId = bot->GetGUID().GetRawValue();
 
-    // If the bot already is a spirit, erase release time and return true
+    // If the bot already is a spirit, reset release time and return true
     if (bot->HasPlayerFlag(PLAYER_FLAGS_GHOST))
     {
-        m_botReleaseTimes.erase(botId);
+        botAI->bgReleaseAttemptTime = 0;
         return true;
     }
 
@@ -204,14 +204,13 @@ bool AutoReleaseSpiritAction::ShouldDelayBattlegroundRelease() const
     const time_t now = time(nullptr);
     constexpr time_t RELEASE_DELAY = 6;
 
-    auto& lastReleaseTime = m_botReleaseTimes[botId];
-    if (lastReleaseTime == 0)
-        lastReleaseTime = now;
+    if (botAI->bgReleaseAttemptTime == 0)
+        botAI->bgReleaseAttemptTime = now;
 
-    if (now - lastReleaseTime < RELEASE_DELAY)
+    if (now - botAI->bgReleaseAttemptTime < RELEASE_DELAY)
         return false;
 
-    m_botReleaseTimes.erase(botId);
+    botAI->bgReleaseAttemptTime = 0;
     return true;
 }
 
@@ -244,6 +243,7 @@ int64 RepopAction::CalculateDeadTime() const
 
 void RepopAction::PerformGraveyardTeleport(const GraveyardStruct* graveyard) const
 {
+    bot->RemoveAurasWithInterruptFlags(AURA_INTERRUPT_FLAG_TELEPORTED | AURA_INTERRUPT_FLAG_CHANGE_MAP);
     bot->TeleportTo(graveyard->Map, graveyard->x, graveyard->y, graveyard->z, 0.f);
     RESET_AI_VALUE(bool, "combat::self target");
     RESET_AI_VALUE(WorldPosition, "current position");

src/strategy/actions/ReleaseSpiritAction.h

@@ -38,7 +38,6 @@ private:
     bool ShouldAutoRelease() const;
     bool ShouldDelayBattlegroundRelease() const;
 
-    inline static std::unordered_map<uint32_t, time_t> m_botReleaseTimes;
     time_t m_bgGossipTime = 0;
 };
 

src/strategy/actions/ReviveFromCorpseAction.cpp

@@ -169,6 +169,7 @@ bool FindCorpseAction::Execute(Event event)
         if (deadTime > delay)
         {
             bot->GetMotionMaster()->Clear();
+            bot->RemoveAurasWithInterruptFlags(AURA_INTERRUPT_FLAG_TELEPORTED | AURA_INTERRUPT_FLAG_CHANGE_MAP);
             bot->TeleportTo(moveToPos.getMapId(), moveToPos.getX(), moveToPos.getY(), moveToPos.getZ(), 0);
         }
 
@@ -350,6 +351,7 @@ bool SpiritHealerAction::Execute(Event event)
     // if (!botAI->HasActivePlayerMaster())
     // {
     context->GetValue<uint32>("death count")->Set(dCount + 1);
+    bot->RemoveAurasWithInterruptFlags(AURA_INTERRUPT_FLAG_TELEPORTED | AURA_INTERRUPT_FLAG_CHANGE_MAP);
     return bot->TeleportTo(ClosestGrave->Map, ClosestGrave->x, ClosestGrave->y, ClosestGrave->z, 0.f);
     // }
 

src/strategy/actions/UseMeetingStoneAction.cpp

@@ -225,6 +225,7 @@ bool SummonAction::Teleport(Player* summoner, Player* player)
 
                 player->GetMotionMaster()->Clear();
                 AI_VALUE(LastMovement&, "last movement").clear();
+                player->RemoveAurasWithInterruptFlags(AURA_INTERRUPT_FLAG_TELEPORTED | AURA_INTERRUPT_FLAG_CHANGE_MAP);
                 player->TeleportTo(mapId, x, y, z, 0);
 
                 if (botAI->HasStrategy("stay", botAI->GetState()))

src/strategy/rpg/NewRpgBaseAction.cpp

@@ -67,6 +67,7 @@ bool NewRpgBaseAction::MoveFarTo(WorldPosition dest)
             bot->GetName(), bot->GetPositionX(), bot->GetPositionY(), bot->GetPositionZ(), bot->GetMapId(),
             dest.GetPositionX(), dest.GetPositionY(), dest.GetPositionZ(), dest.getMapId(), bot->GetZoneId(),
             zone_name);
+        bot->RemoveAurasWithInterruptFlags(AURA_INTERRUPT_FLAG_TELEPORTED | AURA_INTERRUPT_FLAG_CHANGE_MAP);
         return bot->TeleportTo(dest);
     }