Fix bug with GetCreature + GetGameObject = use ObjectAccessor's methods instead (#2105)

# Pull Request

https://en.cppreference.com/w/cpp/algorithm/equal_range.html
> second is an iterator to the first element of the range [first, last)
ordered after value (or last if no such element is found).

The original code uses `return bounds.second->second`, which causes the
wrong creature/gameobject to be returned. Instead, both methods
(`GetCreature` and `GetGameObject`) now utilize ObjectAccessor's methods
to retrieve the correct entities. These built-in methods offer a safer
way to access objects. Additionally, `GetUnit` no longer includes
redundant creature processing before checks and now has the same logic
as the `ObjectAccessor::GetUnit` method.

Furthermore, `GuidPosition::isDead` method has been renamed to
`GuidPosition::IsCreatureOrGOAccessible` and updated, as it is used only
for creatures (NOT units) and gameobjects.

---

## Design Philosophy

We prioritize **stability, performance, and predictability** over
behavioral realism.
Complex player-mimicking logic is intentionally limited due to its
negative impact on scalability, maintainability, and
long-term robustness.

Excessive processing overhead can lead to server hiccups, increased CPU
usage, and degraded performance for all
participants. Because every action and
decision tree is executed **per bot and per trigger**, even small
increases in logic complexity can scale poorly and
negatively affect both players and
world (random) bots. Bots are not expected to behave perfectly, and
perfect simulation of human decision-making is not a
project goal. Increased behavioral
realism often introduces disproportionate cost, reduced predictability,
and significantly higher maintenance overhead.

Every additional branch of logic increases long-term responsibility. All
decision paths must be tested, validated, and
maintained continuously as the system evolves.
If advanced or AI-intensive behavior is introduced, the **default
configuration must remain the lightweight decision
model**. More complex behavior should only be
available as an **explicit opt-in option**, clearly documented as having
a measurable performance cost.

Principles:

- **Stability before intelligence**  
  A stable system is always preferred over a smarter one.

- **Performance is a shared resource**  
  Any increase in bot cost affects all players and all bots.

- **Simple logic scales better than smart logic**  
Predictable behavior under load is more valuable than perfect decisions.

- **Complexity must justify itself**  
  If a feature cannot clearly explain its cost, it should not exist.

- **Defaults must be cheap**  
  Expensive behavior must always be optional and clearly communicated.

- **Bots should look reasonable, not perfect**  
  The goal is believable behavior, not human simulation.

Before submitting, confirm that this change aligns with those
principles.

---

## How to Test the Changes

The behavior has not changed after all.

## Complexity & Impact

- Does this change add new decision branches?
    - [x] No
    - [ ] Yes (**explain below**)

- Does this change increase per-bot or per-tick processing?
    - [x] No
    - [ ] Yes (**describe and justify impact**)

- Could this logic scale poorly under load?
    - [x] No
    - [ ] Yes (**explain why**)

---

## Defaults & Configuration

- Does this change modify default bot behavior?
    - [x] No
    - [ ] Yes (**explain why**)

If this introduces more advanced or AI-heavy logic:

- [ ] Lightweight mode remains the default
- [ ] More complex behavior is optional and thereby configurable

---

## AI Assistance

- Was AI assistance (e.g. ChatGPT or similar tools) used while working
on this change?
    - [x] No
    - [ ] Yes (**explain below**)

If yes, please specify:

- AI tool or model used (e.g. ChatGPT, GPT-4, Claude, etc.)
- Purpose of usage (e.g. brainstorming, refactoring, documentation, code
generation)
- Which parts of the change were influenced or generated
- Whether the result was manually reviewed and adapted

AI assistance is allowed, but all submitted code must be fully
understood, reviewed, and owned by the contributor.
Any AI-influenced changes must be verified against existing CORE and PB
logic. We expect contributors to be honest
about what they do and do not understand.

---

## Final Checklist

- [x] Stability is not compromised
- [x] Performance impact is understood, tested, and acceptable
- [x] Added logic complexity is justified and explained
- [x] Documentation updated if needed

---

## Notes for Reviewers

Anything that significantly improves realism at the cost of stability or
performance should be carefully discussed
before merging.

---------

Co-authored-by: bashermens <31279994+hermensbas@users.noreply.github.com>

src/Ai/Base/Value/QuestValues.cpp

@@ -173,7 +173,7 @@ std::vector<GuidPosition> ActiveQuestGiversValue::Calculate()
                     continue;
             }
 
-            if (guidp.isDead())
+            if (!guidp.IsCreatureOrGOAccessible())
                 continue;
 
             retQuestGivers.push_back(guidp);
@@ -231,7 +231,7 @@ std::vector<GuidPosition> ActiveQuestTakersValue::Calculate()
 
             for (auto& guidp : entry.second)
             {
-                if (guidp.isDead())
+                if (!guidp.IsCreatureOrGOAccessible())
                     continue;
 
                 retQuestTakers.push_back(guidp);
@@ -298,7 +298,7 @@ std::vector<GuidPosition> ActiveQuestObjectivesValue::Calculate()
             {
                 for (auto& guidp : entry.second)
                 {
-                    if (guidp.isDead())
+                    if (!guidp.IsCreatureOrGOAccessible())
                         continue;
 
                     retQuestObjectives.push_back(guidp);

src/Ai/Class/Warlock/Action/WarlockActions.h

@@ -41,6 +41,18 @@ public:
     std::string const GetTargetName() override { return "pet target"; }
 };
 
+class CastUnendingBreathAction : public CastBuffSpellAction
+{
+public:
+    CastUnendingBreathAction(PlayerbotAI* botAI) : CastBuffSpellAction(botAI, "unending breath") {}
+};
+
+class CastUnendingBreathOnPartyAction : public BuffOnPartyAction
+{
+public:
+    CastUnendingBreathOnPartyAction(PlayerbotAI* botAI) : BuffOnPartyAction(botAI, "unending breath") {}
+};
+
 class CreateSoulShardAction : public Action
 {
 public:

src/Ai/Class/Warlock/Strategy/GenericWarlockNonCombatStrategy.cpp

@@ -85,6 +85,8 @@ void GenericWarlockNonCombatStrategy::InitTriggers(std::vector<TriggerNode*>& tr
     triggers.push_back(new TriggerNode("too many soul shards", { NextAction("destroy soul shard", 60.0f) }));
     triggers.push_back(new TriggerNode("soul link", { NextAction("soul link", 28.0f) }));
     triggers.push_back(new TriggerNode("demon armor", { NextAction("fel armor", 27.0f) }));
+    triggers.push_back(new TriggerNode("unending breath", { NextAction("unending breath", 12.0f) }));
+    triggers.push_back(new TriggerNode("unending breath on party", { NextAction("unending breath on party", 11.0f) }));
     triggers.push_back(new TriggerNode("no healthstone", { NextAction("create healthstone", 26.0f) }));
     triggers.push_back(new TriggerNode("no soulstone", { NextAction("create soulstone", 25.0f) }));
     triggers.push_back(new TriggerNode("life tap", { NextAction("life tap", 23.0f) }));

src/Ai/Class/Warlock/Trigger/WarlockTriggers.cpp

@@ -79,6 +79,16 @@ bool SoulLinkTrigger::IsActive()
     return !botAI->HasAura("soul link", target);
 }
 
+bool UnendingBreathTrigger::IsActive()
+{
+    return BuffTrigger::IsActive() && AI_VALUE2(bool, "swimming", "self target");
+}
+
+bool UnendingBreathOnPartyTrigger::IsActive()
+{
+    return BuffOnPartyTrigger::IsActive() && AI_VALUE2(bool, "swimming", "self target");
+}
+
 bool DemonicEmpowermentTrigger::IsActive()
 {
     Pet* pet = bot->GetPet();

src/Ai/Class/Warlock/Trigger/WarlockTriggers.h

@@ -32,6 +32,20 @@ public:
     bool IsActive() override;
 };
 
+class UnendingBreathTrigger : public BuffTrigger
+{
+public:
+    UnendingBreathTrigger(PlayerbotAI* botAI) : BuffTrigger(botAI, "unending breath", 5 * 2000) {}
+    bool IsActive() override;
+};
+
+class UnendingBreathOnPartyTrigger : public BuffOnPartyTrigger
+{
+public:
+    UnendingBreathOnPartyTrigger(PlayerbotAI* botAI) : BuffOnPartyTrigger(botAI, "unending breath on party", 2 * 2000) {}
+    bool IsActive() override;
+};
+
 class OutOfSoulShardsTrigger : public Trigger
 {
 public:

src/Ai/Class/Warlock/WarlockAiObjectContext.cpp

@@ -143,6 +143,8 @@ public:
         creators["shadow trance"] = &WarlockTriggerFactoryInternal::shadow_trance;
         creators["demon armor"] = &WarlockTriggerFactoryInternal::demon_armor;
         creators["soul link"] = &WarlockTriggerFactoryInternal::soul_link;
+        creators["unending breath"] = &WarlockTriggerFactoryInternal::unending_breath;
+        creators["unending breath on party"] = &WarlockTriggerFactoryInternal::unending_breath_on_party;
         creators["no soul shard"] = &WarlockTriggerFactoryInternal::no_soul_shard;
         creators["too many soul shards"] = &WarlockTriggerFactoryInternal::too_many_soul_shards;
         creators["no healthstone"] = &WarlockTriggerFactoryInternal::HasHealthstone;
@@ -189,6 +191,8 @@ private:
     static Trigger* shadow_trance(PlayerbotAI* botAI) { return new ShadowTranceTrigger(botAI); }
     static Trigger* demon_armor(PlayerbotAI* botAI) { return new DemonArmorTrigger(botAI); }
     static Trigger* soul_link(PlayerbotAI* botAI) { return new SoulLinkTrigger(botAI); }
+    static Trigger* unending_breath(PlayerbotAI* botAI) { return new UnendingBreathTrigger(botAI); }
+    static Trigger* unending_breath_on_party(PlayerbotAI* botAI) { return new UnendingBreathOnPartyTrigger(botAI); }
     static Trigger* no_soul_shard(PlayerbotAI* botAI) { return new OutOfSoulShardsTrigger(botAI); }
     static Trigger* too_many_soul_shards(PlayerbotAI* botAI) { return new TooManySoulShardsTrigger(botAI); }
     static Trigger* HasHealthstone(PlayerbotAI* botAI) { return new HasHealthstoneTrigger(botAI); }
@@ -240,6 +244,8 @@ public:
         creators["demon armor"] = &WarlockAiObjectContextInternal::demon_armor;
         creators["demon skin"] = &WarlockAiObjectContextInternal::demon_skin;
         creators["soul link"] = &WarlockAiObjectContextInternal::soul_link;
+        creators["unending breath"] = &WarlockAiObjectContextInternal::unending_breath;
+        creators["unending breath on party"] = &WarlockAiObjectContextInternal::unending_breath_on_party;
         creators["create soul shard"] = &WarlockAiObjectContextInternal::create_soul_shard;
         creators["destroy soul shard"] = &WarlockAiObjectContextInternal::destroy_soul_shard;
         creators["create healthstone"] = &WarlockAiObjectContextInternal::create_healthstone;
@@ -313,6 +319,8 @@ private:
     static Action* demon_armor(PlayerbotAI* botAI) { return new CastDemonArmorAction(botAI); }
     static Action* demon_skin(PlayerbotAI* botAI) { return new CastDemonSkinAction(botAI); }
     static Action* soul_link(PlayerbotAI* botAI) { return new CastSoulLinkAction(botAI); }
+    static Action* unending_breath(PlayerbotAI* botAI) { return new CastUnendingBreathAction(botAI); }
+    static Action* unending_breath_on_party(PlayerbotAI* botAI) { return new CastUnendingBreathOnPartyAction(botAI); }
     static Action* create_soul_shard(PlayerbotAI* botAI) { return new CreateSoulShardAction(botAI); }
     static Action* destroy_soul_shard(PlayerbotAI* botAI) { return new DestroySoulShardAction(botAI); }
     static Action* create_healthstone(PlayerbotAI* botAI) { return new CastCreateHealthstoneAction(botAI); }

src/Ai/Dungeon/Oculus/Action/OculusActions.cpp

@@ -62,7 +62,7 @@ bool MountDrakeAction::Execute(Event event)
             break;
     }
 
-    std::vector<Player*> players = botAI->GetPlayersInGroup();
+    std::vector<Player*> players = botAI->GetAllPlayersInGroup();
     for (Player* player : players)
     {
         if (!player || !player->IsInWorld() || player->IsDuringRemoveFromWorld())

src/Bot/PlayerbotAI.cpp

@@ -2584,7 +2584,7 @@ std::string PlayerbotAI::GetLocalizedGameObjectName(uint32 entry)
     return name;
 }
 
-std::vector<Player*> PlayerbotAI::GetPlayersInGroup()
+std::vector<Player*> PlayerbotAI::GetRealPlayersInGroup()
 {
     std::vector<Player*> members;
 
@@ -2611,6 +2611,30 @@ std::vector<Player*> PlayerbotAI::GetPlayersInGroup()
     return members;
 }
 
+std::vector<Player*> PlayerbotAI::GetAllPlayersInGroup()
+{
+    std::vector<Player*> members;
+
+    Group* group = bot->GetGroup();
+
+    if (!group)
+        return members;
+
+    for (GroupReference* ref = group->GetFirstMember(); ref; ref = ref->next())
+    {
+        Player* member = ref->GetSource();
+
+        if (!member)
+        {
+            continue;
+        }
+
+        members.push_back(ref->GetSource());
+    }
+
+    return members;
+}
+
 bool PlayerbotAI::SayToGuild(const std::string& msg)
 {
     if (msg.empty())
@@ -2719,9 +2743,9 @@ bool PlayerbotAI::SayToParty(const std::string& msg)
     ChatHandler::BuildChatPacket(data, CHAT_MSG_PARTY, msg.c_str(), LANG_UNIVERSAL, CHAT_TAG_NONE, bot->GetGUID(),
                                  bot->GetName());
 
-    for (auto reciever : GetPlayersInGroup())
+    for (auto receiver : GetRealPlayersInGroup())
     {
-        ServerFacade::instance().SendPacket(reciever, &data);
+        ServerFacade::instance().SendPacket(receiver, &data);
     }
 
     return true;
@@ -2736,9 +2760,9 @@ bool PlayerbotAI::SayToRaid(const std::string& msg)
     ChatHandler::BuildChatPacket(data, CHAT_MSG_RAID, msg.c_str(), LANG_UNIVERSAL, CHAT_TAG_NONE, bot->GetGUID(),
                                  bot->GetName());
 
-    for (auto reciever : GetPlayersInGroup())
+    for (auto receiver : GetRealPlayersInGroup())
     {
-        ServerFacade::instance().SendPacket(reciever, &data);
+        ServerFacade::instance().SendPacket(receiver, &data);
     }
 
     return true;

src/Bot/PlayerbotAI.h

@@ -446,7 +446,8 @@ public:
     GameObject* GetGameObject(ObjectGuid guid);
     // static GameObject* GetGameObject(GameObjectData const* gameObjectData);
     WorldObject* GetWorldObject(ObjectGuid guid);
-    std::vector<Player*> GetPlayersInGroup();
+    std::vector<Player*> GetAllPlayersInGroup();
+    std::vector<Player*> GetRealPlayersInGroup();
     const AreaTableEntry* GetCurrentArea();
     const AreaTableEntry* GetCurrentZone();
     static std::string GetLocalizedAreaName(const AreaTableEntry* entry);

src/Mgr/Travel/TravelMgr.cpp

@@ -881,107 +881,6 @@ std::vector<GameObjectData const*> WorldPosition::getGameObjectsNear(float radiu
     return worker.GetResult();
 }
 
-Creature* GuidPosition::GetCreature()
-{
-    if (!*this)
-        return nullptr;
-
-    if (loadedFromDB)
-    {
-        auto creatureBounds = getMap()->GetCreatureBySpawnIdStore().equal_range(GetCounter());
-        if (creatureBounds.first != creatureBounds.second)
-            return creatureBounds.second->second;
-
-        return nullptr;
-    }
-
-    return getMap()->GetCreature(*this);
-}
-
-Unit* GuidPosition::GetUnit()
-{
-    if (!*this)
-        return nullptr;
-
-    if (loadedFromDB)
-    {
-        auto creatureBounds = getMap()->GetCreatureBySpawnIdStore().equal_range(GetCounter());
-        if (creatureBounds.first != creatureBounds.second)
-            return creatureBounds.second->second;
-
-        return nullptr;
-    }
-
-    if (IsPlayer())
-        return ObjectAccessor::FindPlayer(*this);
-
-    if (IsPet())
-        return getMap()->GetPet(*this);
-
-    return GetCreature();
-}
-
-GameObject* GuidPosition::GetGameObject()
-{
-    if (!*this)
-        return nullptr;
-
-    if (loadedFromDB)
-    {
-        auto gameobjectBounds = getMap()->GetGameObjectBySpawnIdStore().equal_range(GetCounter());
-        if (gameobjectBounds.first != gameobjectBounds.second)
-            return gameobjectBounds.second->second;
-
-        return nullptr;
-    }
-
-    return getMap()->GetGameObject(*this);
-}
-
-Player* GuidPosition::GetPlayer()
-{
-    if (!*this)
-        return nullptr;
-
-    if (IsPlayer())
-        return ObjectAccessor::FindPlayer(*this);
-
-    return nullptr;
-}
-
-bool GuidPosition::isDead()
-{
-    if (!getMap())
-        return false;
-
-    if (!getMap()->IsGridLoaded(getX(), getY()))
-        return false;
-
-    if (IsUnit() && GetUnit() && GetUnit()->IsInWorld() && GetUnit()->IsAlive())
-        return false;
-
-    if (IsGameObject() && GetGameObject() && GetGameObject()->IsInWorld())
-        return false;
-
-    return true;
-}
-
-GuidPosition::GuidPosition(WorldObject* wo) : ObjectGuid(wo->GetGUID()), WorldPosition(wo), loadedFromDB(false) {}
-
-GuidPosition::GuidPosition(CreatureData const& creData)
-    : ObjectGuid(HighGuid::Unit, creData.id1, creData.spawnId),
-      WorldPosition(creData.mapid, creData.posX, creData.posY, creData.posZ, creData.orientation)
-{
-    loadedFromDB = true;
-}
-
-GuidPosition::GuidPosition(GameObjectData const& goData)
-    : ObjectGuid(HighGuid::GameObject, goData.id),
-      WorldPosition(goData.mapid, goData.posX, goData.posY, goData.posZ, goData.orientation)
-{
-    loadedFromDB = true;
-}
-
 CreatureTemplate const* GuidPosition::GetCreatureTemplate()
 {
     return IsCreature() ? sObjectMgr->GetCreatureTemplate(GetEntry()) : nullptr;
@@ -1000,7 +899,7 @@ WorldObject* GuidPosition::GetWorldObject()
     switch (GetHigh())
     {
         case HighGuid::Player:
-            return ObjectAccessor::FindPlayer(*this);
+            return GetPlayer();
         case HighGuid::Transport:
         case HighGuid::Mo_Transport:
         case HighGuid::GameObject:
@@ -1021,8 +920,93 @@ WorldObject* GuidPosition::GetWorldObject()
     return nullptr;
 }
 
+GameObject* GuidPosition::GetGameObject()
+{
+    if (!*this)
+        return nullptr;
+
+    if (loadedFromDB)
+        return ObjectAccessor::GetSpawnedGameObjectByDBGUID(GetMapId(), GetCounter());
+
+    return getMap()->GetGameObject(*this); // fallback
+}
+
+Unit* GuidPosition::GetUnit()
+{
+    if (!*this)
+        return nullptr;
+
+    if (IsPlayer())
+        return GetPlayer();
+
+    if (IsPet())
+        return getMap()->GetPet(*this);
+
+    return GetCreature();
+}
+
+Creature* GuidPosition::GetCreature()
+{
+    if (!*this)
+        return nullptr;
+
+    if (loadedFromDB)
+        return ObjectAccessor::GetSpawnedCreatureByDBGUID(GetMapId(), GetCounter());
+
+    return getMap()->GetCreature(*this); // fallback
+}
+
+Player* GuidPosition::GetPlayer()
+{
+    if (!*this)
+        return nullptr;
+
+    if (IsPlayer())
+        return ObjectAccessor::FindPlayer(*this);
+
+    return nullptr;
+}
+
 bool GuidPosition::HasNpcFlag(NPCFlags flag) { return IsCreature() && GetCreatureTemplate()->npcflag & flag; }
 
+bool GuidPosition::IsCreatureOrGOAccessible()
+{
+    Map* map = getMap();
+    if (!map || !map->IsGridLoaded(GetPositionX(), GetPositionY()))
+        return false;
+
+    if (IsCreature())
+    {
+        Creature* creature = GetCreature();
+        if (creature && creature->IsInWorld() && creature->IsAlive())
+            return true;
+    }
+    else if (IsGameObject())
+    {
+        GameObject* go = GetGameObject();
+        if (go && go->IsInWorld())
+            return true;
+    }
+
+    return false;
+}
+
+GuidPosition::GuidPosition(WorldObject* wo) : ObjectGuid(wo->GetGUID()), WorldPosition(wo), loadedFromDB(false) {}
+
+GuidPosition::GuidPosition(CreatureData const& creData)
+    : ObjectGuid(HighGuid::Unit, creData.id1, creData.spawnId),
+      WorldPosition(creData.mapid, creData.posX, creData.posY, creData.posZ, creData.orientation)
+{
+    loadedFromDB = true;
+}
+
+GuidPosition::GuidPosition(GameObjectData const& goData)
+    : ObjectGuid(HighGuid::GameObject, goData.id),
+      WorldPosition(goData.mapid, goData.posX, goData.posY, goData.posZ, goData.orientation)
+{
+    loadedFromDB = true;
+}
+
 std::vector<WorldPosition*> TravelDestination::getPoints(bool ignoreFull)
 {
     if (ignoreFull)

src/Mgr/Travel/TravelMgr.h

@@ -416,14 +416,13 @@ public:
     GameObjectTemplate const* GetGameObjectTemplate();
 
     WorldObject* GetWorldObject();
-    Creature* GetCreature();
-    Unit* GetUnit();
     GameObject* GetGameObject();
+    Unit* GetUnit();
+    Creature* GetCreature();
     Player* GetPlayer();
 
     bool HasNpcFlag(NPCFlags flag);
-
-    bool isDead();  // For loaded grids check if the unit/object is unloaded/dead.
+    bool IsCreatureOrGOAccessible(); // For loaded grids check if the creature/gameobject is in world + alive
 
     operator bool() const { return !IsEmpty(); }
     bool operator==(ObjectGuid const& guid) const { return GetRawValue() == guid.GetRawValue(); }