BattleGrounds: Refactor and Fix bots re-spawning when graveyard meanwhile gets lost (#937)

* Update ReleaseSpiritAction.cpp

* Update ReleaseSpiritAction.cpp

* Update ReleaseSpiritAction.cpp

* Update ReleaseSpiritAction.cpp

* Update ReleaseSpiritAction.h

* Update ReleaseSpiritAction.h

* Update ReleaseSpiritAction.cpp

* Update ReleaseSpiritAction.cpp

* Update ReleaseSpiritAction.cpp

* Refactor (#68)

* Refactored ReleaseSpiritAction.cpp

* Ref ReleaseSpiritAction.h

* Restore comment

src/strategy/actions/ReleaseSpiritAction.cpp

@@ -7,12 +7,14 @@
 
 #include "Event.h"
 #include "GameGraveyard.h"
+#include "NearestNpcsValue.h"
 #include "ObjectDefines.h"
 #include "ObjectGuid.h"
 #include "Playerbots.h"
 #include "ServerFacade.h"
 #include "Corpse.h"
 
+// ReleaseSpiritAction implementation
 bool ReleaseSpiritAction::Execute(Event event)
 {
     if (bot->IsAlive())
@@ -28,214 +30,212 @@ bool ReleaseSpiritAction::Execute(Event event)
         return false;
     }
 
-    WorldPacket& p = event.getPacket();
+    const WorldPacket& packet = event.getPacket();
-    if (!p.empty() && p.GetOpcode() == CMSG_REPOP_REQUEST)
+    const std::string message = !packet.empty() && packet.GetOpcode() == CMSG_REPOP_REQUEST 
-        botAI->TellMasterNoFacing("Releasing...");
+                                ? "Releasing..." 
-    else
+                                : "Meet me at the graveyard";
-        botAI->TellMasterNoFacing("Meet me at the graveyard");
+    botAI->TellMasterNoFacing(message);
 
-    // Death Count to prevent skeleton piles
+    IncrementDeathCount();
-    Player* master = GetMaster();
+    LogRelease("released");
-    if (!master || (master && GET_PLAYERBOT_AI(master)))
-    {
-        uint32 dCount = AI_VALUE(uint32, "death count");
-        context->GetValue<uint32>("death count")->Set(dCount + 1);
-    }
 
-    LOG_INFO("playerbots", "Bot {} {}:{} <{}> released", bot->GetGUID().ToString().c_str(),
+    WorldPacket releasePacket(CMSG_REPOP_REQUEST);
-             bot->GetTeamId() == TEAM_ALLIANCE ? "A" : "H", bot->GetLevel(), bot->GetName().c_str());
+    releasePacket << uint8(0);
-
+    bot->GetSession()->HandleRepopRequestOpcode(releasePacket);
-    WorldPacket packet(CMSG_REPOP_REQUEST);
-    packet << uint8(0);
-    bot->GetSession()->HandleRepopRequestOpcode(packet);
-
-    // // add waiting for ress aura
-    // if (bot->InBattleground() && !botAI->HasAura(SPELL_WAITING_FOR_RESURRECT, bot) && !bot->IsAlive())
-    // {
-    //     // cast Waiting for Resurrect
-    //     GuidVector npcs = AI_VALUE(GuidVector, "nearest npcs");
-    //     ObjectGuid guid;
-    //     Unit* unit;
-    //     for (GuidVector::iterator i = npcs.begin(); i != npcs.end(); i++)
-    //     {
-    //         unit = botAI->GetUnit(*i);
-    //         if (unit && unit->IsSpiritService())
-    //         {
-    //             guid = unit->GetGUID();
-    //             break;
-    //         }
-    //     }
-    //     if (!guid) {
-    //         return true;
-    //     }
-    //     if (bot->GetDistance(unit) >= INTERACTION_DISTANCE) {
-    //         bot->GetMotionMaster()->MoveChase(unit);
-    //     } else {
-    //         WorldPacket packet(CMSG_GOSSIP_HELLO);
-    //         packet << guid;
-    //         bot->GetSession()->HandleGossipHelloOpcode(packet);
-    //     }
-    // }
 
     return true;
 }
 
-bool AutoReleaseSpiritAction::Execute(Event event)
+void ReleaseSpiritAction::IncrementDeathCount() const
 {
     // Death Count to prevent skeleton piles
-    Player* master = GetMaster();
+    Player* master = botAI->GetMaster();
-    if (!master || (master && GET_PLAYERBOT_AI(master)))
+    if (!master || GET_PLAYERBOT_AI(master))
     {
-        uint32 dCount = AI_VALUE(uint32, "death count");
+        uint32 deathCount = AI_VALUE(uint32, "death count");
-        context->GetValue<uint32>("death count")->Set(dCount + 1);
+        context->GetValue<uint32>("death count")->Set(deathCount + 1);
     }
+}
 
-    LOG_DEBUG("playerbots", "Bot {} {}:{} <{}> auto released", bot->GetGUID().ToString().c_str(),
+void ReleaseSpiritAction::LogRelease(const std::string& releaseMsg, bool isAutoRelease) const
-              bot->GetTeamId() == TEAM_ALLIANCE ? "A" : "H", bot->GetLevel(), bot->GetName().c_str());
+{
+    const std::string teamPrefix = bot->GetTeamId() == TEAM_ALLIANCE ? "A" : "H";
+
+    LOG_DEBUG("playerbots", "Bot {} {}:{} <{}> {}",
+        bot->GetGUID().ToString().c_str(),
+        teamPrefix,
+        bot->GetLevel(),
+        bot->GetName().c_str(),
+        releaseMsg.c_str());
+}
+
+// AutoReleaseSpiritAction implementation
+bool AutoReleaseSpiritAction::Execute(Event event)
+{
+    IncrementDeathCount();
+    LogRelease("auto released", true);
 
     WorldPacket packet(CMSG_REPOP_REQUEST);
     packet << uint8(0);
     bot->GetSession()->HandleRepopRequestOpcode(packet);
 
-    LOG_DEBUG("playerbots", "Bot {} {}:{} <{}> releases spirit", bot->GetGUID().ToString().c_str(),
+    LogRelease("releases spirit", true);
-              bot->GetTeamId() == TEAM_ALLIANCE ? "A" : "H", bot->GetLevel(), bot->GetName().c_str());
     
-    if (bot->InBattleground() && (time(NULL) - bg_gossip_time >= 15 || !bot->HasAura(SPELL_WAITING_FOR_RESURRECT)))
+    if (bot->InBattleground())
     {
-        GuidVector npcs = AI_VALUE(GuidVector, "nearest npcs");
+        return HandleBattlegroundSpiritHealer();
-        ObjectGuid guid;
-        Unit* unit;
-        for (GuidVector::iterator i = npcs.begin(); i != npcs.end(); i++)
-        {
-            unit = botAI->GetUnit(*i);
-            if (unit && unit->IsSpiritService())
-            {
-                guid = unit->GetGUID();
-                break;
-            }
-        }
-        if (!guid)
-        {
-            return true;
-        }
-        if (bot->GetDistance(unit) >= INTERACTION_DISTANCE)
-        {
-            // bot needs to actually click spirit-healer in BG to get res timer going
-            // and in IOC it's not within clicking range when they res in own base
-            MotionMaster& mm = *bot->GetMotionMaster();
-            mm.Clear();
-            mm.MovePoint(bot->GetMapId(), unit->GetPositionX(), unit->GetPositionY(), unit->GetPositionZ(), true);
-        }
-        else if (!botAI->IsRealPlayer()) // below doesnt work properly on realplayer, but its also not needed
-        {
-            bg_gossip_time = time(NULL);
-            WorldPacket packet(CMSG_GOSSIP_HELLO);
-            packet << guid;
-            bot->GetSession()->HandleGossipHelloOpcode(packet);
-        }
     }
+
     botAI->SetNextCheckDelay(1000);
     return true;
 }
 
 bool AutoReleaseSpiritAction::isUseful()
 {
-    if (!bot->isDead())
+    if (!bot->isDead() || bot->InArena())
         return false;
 
-    if (bot->InArena())
-        return false;
-
-    // if (bot->InBattleground())
-    //     return true;
-
-    // When bot dies in BG, wait a couple of seconds before release.
-    // This prevents currently casted (ranged) spells to be re-directed to the bot's ghost.
-    // Use a static map to track release times for each bot.
     if (bot->InBattleground())
-    {
+        return ShouldDelayBattlegroundRelease();
-        auto botId = bot->GetGUID().GetRawValue();
-
-        // If the bot is not a ghost yet, delay release some.
-        if (!bot->HasPlayerFlag(PLAYER_FLAGS_GHOST))
-        {
-            time_t now = time(nullptr);
-
-            // If this bot has no recorded release time yet, set it to now.
-            if (botReleaseTimes.find(botId) == botReleaseTimes.end())
-                botReleaseTimes[botId] = now;
-
-            // Wait 6 seconds before releasing.
-            if (now - botReleaseTimes[botId] < 6)
-                return false;
-        }
-        // Erase the release time for this bot.
-        botReleaseTimes.erase(botId);
-        return true;
-    }
 
     if (bot->HasPlayerFlag(PLAYER_FLAGS_GHOST))
         return false;
 
+    return ShouldAutoRelease();
+}
+
+bool AutoReleaseSpiritAction::HandleBattlegroundSpiritHealer()
+{
+    constexpr uint32_t RESURRECT_DELAY = 15;
+    const time_t now = time(nullptr);
+    
+    if ((now - m_bgGossipTime < RESURRECT_DELAY) && 
+        bot->HasAura(SPELL_WAITING_FOR_RESURRECT))
+    {
+        return false;
+    }
+
+    float bgRange = 2000.0f;
+    GuidVector npcs = NearestNpcsValue(botAI, bgRange);
+    Unit* spiritHealer = nullptr;
+
+    for (const auto& guid : npcs)
+    {
+        Unit* unit = botAI->GetUnit(guid);
+        if (unit && unit->IsFriendlyTo(bot) && unit->IsSpiritService())
+        {
+            spiritHealer = unit;
+            break;
+        }
+    }
+
+    if (!spiritHealer)
+        return false;
+
+    if (bot->GetDistance(spiritHealer) >= INTERACTION_DISTANCE)
+    {
+        // Bot needs to actually click spirit-healer in BG to get res timer going
+        // 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->TeleportTo(bot->GetMapId(), spiritHealer->GetPositionX(), spiritHealer->GetPositionY(), spiritHealer->GetPositionZ(), 0.f);
+        RESET_AI_VALUE(bool, "combat::self target");
+        RESET_AI_VALUE(WorldPosition, "current position");
+    }
+    else if (!botAI->IsRealPlayer())
+    {
+        m_bgGossipTime = now;
+        WorldPacket packet(CMSG_GOSSIP_HELLO);
+        packet << spiritHealer->GetGUID();
+        bot->GetSession()->HandleGossipHelloOpcode(packet);
+    }
+
+    return true;
+}
+
+bool AutoReleaseSpiritAction::ShouldAutoRelease() const
+{
     if (!bot->GetGroup())
         return true;
 
-    if (!botAI->GetGroupMaster())
+    Player* groupMaster = botAI->GetGroupMaster();
-        return true;
+    if (!groupMaster || groupMaster == bot)
-
-    if (botAI->GetGroupMaster() == bot)
         return true;
 
     if (!botAI->HasActivePlayerMaster())
         return true;
 
-    if (botAI->HasActivePlayerMaster() && botAI->GetGroupMaster()->GetMapId() == bot->GetMapId() && bot->GetMap() &&
+    if (botAI->HasActivePlayerMaster() && 
+        groupMaster->GetMapId() == bot->GetMapId() &&
+        bot->GetMap() && 
         (bot->GetMap()->IsRaid() || bot->GetMap()->IsDungeon()))
+    {
+        return false;
+    }
+
+    return sServerFacade->IsDistanceGreaterThan(
+        AI_VALUE2(float, "distance", "master target"),
+        sPlayerbotAIConfig->sightDistance);
+}
+
+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 (bot->HasPlayerFlag(PLAYER_FLAGS_GHOST))
+    {
+        m_botReleaseTimes.erase(botId);
+        return true;
+    }
+
+    // Delay release to spirit.
+    const time_t now = time(nullptr);
+    constexpr time_t RELEASE_DELAY = 6;
+
+    auto& lastReleaseTime = m_botReleaseTimes[botId];
+    if (lastReleaseTime == 0)
+        lastReleaseTime = now;
+
+    if (now - lastReleaseTime < RELEASE_DELAY)
         return false;
 
-    if (botAI->GetGroupMaster()->isDead())
+    m_botReleaseTimes.erase(botId);
     return true;
-
-    if (sServerFacade->IsDistanceGreaterThan(AI_VALUE2(float, "distance", "master target"),
-                                             sPlayerbotAIConfig->sightDistance))
-        return true;
-
-    return false;
 }
 
 bool RepopAction::Execute(Event event)
 {
-    LOG_DEBUG("playerbots", "Bot {} {}:{} <{}> repops at graveyard", bot->GetGUID().ToString().c_str(),
+    const GraveyardStruct* graveyard = GetGrave(
-              bot->GetTeamId() == TEAM_ALLIANCE ? "A" : "H", bot->GetLevel(), bot->GetName().c_str());
+        AI_VALUE(uint32, "death count") > 10 || 
+        CalculateDeadTime() > 30 * MINUTE
+    );
 
-    int64 deadTime;
+    if (!graveyard)
-
-    Corpse* corpse = bot->GetCorpse();
-    if (corpse)
-        deadTime = time(nullptr) - corpse->GetGhostTime();
-    else if (bot->isDead())
-        deadTime = 0;
-    else
-        deadTime = 60 * MINUTE;
-
-    uint32 dCount = AI_VALUE(uint32, "death count");
-
-    GraveyardStruct const* ClosestGrave = GetGrave(dCount > 10 || deadTime > 30 * MINUTE);
-    if (!ClosestGrave)
         return false;
 
-    bot->TeleportTo(ClosestGrave->Map, ClosestGrave->x, ClosestGrave->y, ClosestGrave->z, 0.f);
+    PerformGraveyardTeleport(graveyard);
-
-    RESET_AI_VALUE(bool, "combat::self target");
-    RESET_AI_VALUE(WorldPosition, "current position");
-
     return true;
 }
 
 bool RepopAction::isUseful()
 {
-    if (bot->InBattleground())
+    return !bot->InBattleground();
-        return false;
+}
-
+
-    return true;
+int64 RepopAction::CalculateDeadTime() const
+{
+    if (Corpse* corpse = bot->GetCorpse())
+        return time(nullptr) - corpse->GetGhostTime();
+    
+    return bot->isDead() ? 0 : 60 * MINUTE;
+}
+
+void RepopAction::PerformGraveyardTeleport(const GraveyardStruct* graveyard) const
+{
+    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

@@ -3,10 +3,8 @@
  * and/or modify it under version 2 of the License, or (at your option), any later version.
  */
 
-#ifndef _PLAYERBOT_RELEASESPIRITACTION_H
+#ifndef PLAYERBOT_RELEASESPIRITACTION_H
-#define _PLAYERBOT_RELEASESPIRITACTION_H
+#define PLAYERBOT_RELEASESPIRITACTION_H
-
-#include <cstdint>
 
 #include "Action.h"
 #include "ReviveFromCorpseAction.h"
@@ -16,34 +14,46 @@ class PlayerbotAI;
 class ReleaseSpiritAction : public Action
 {
 public:
-    ReleaseSpiritAction(PlayerbotAI* botAI, std::string const name = "release") : Action(botAI, name) {}
+    ReleaseSpiritAction(PlayerbotAI* botAI, const std::string& name = "release")
+        : Action(botAI, name) {}
 
     bool Execute(Event event) override;
+    void LogRelease(const std::string& releaseType, bool isAutoRelease = false) const;
+
+protected:
+    void IncrementDeathCount() const;
 };
 
 class AutoReleaseSpiritAction : public ReleaseSpiritAction
 {
 public:
-    AutoReleaseSpiritAction(PlayerbotAI* botAI, std::string const name = "auto release")
+    AutoReleaseSpiritAction(PlayerbotAI* botAI, const std::string& name = "auto release")
-        : ReleaseSpiritAction(botAI, name)
+        : ReleaseSpiritAction(botAI, name) {}
-    {
-    }
 
     bool Execute(Event event) override;
     bool isUseful() override;
 
 private:
-    inline static std::unordered_map<uint32_t, time_t> botReleaseTimes;
+    bool HandleBattlegroundSpiritHealer();
-    uint32_t bg_gossip_time = 0;
+    bool ShouldAutoRelease() const;
+    bool ShouldDelayBattlegroundRelease() const;
+
+    inline static std::unordered_map<uint32_t, time_t> m_botReleaseTimes;
+    time_t m_bgGossipTime = 0;
 };
 
 class RepopAction : public SpiritHealerAction
 {
 public:
-    RepopAction(PlayerbotAI* botAI, std::string const name = "repop") : SpiritHealerAction(botAI, name) {}
+    RepopAction(PlayerbotAI* botAI, const std::string& name = "repop")
+        : SpiritHealerAction(botAI, name) {}
 
     bool Execute(Event event) override;
     bool isUseful() override;
+
+private:
+    int64 CalculateDeadTime() const;
+    void PerformGraveyardTeleport(const GraveyardStruct* graveyard) const;
 };
 
 #endif