Fix: Stop pets from fighting in PVP prohibited zones (#1829)

Stripped down version of #1818. No new features. Refactors
IsPossibleTarget in AttackersValue.cpp to a better style and makes sure
pets don't attack in prohibited zones.

Testing:
Confirmed that aggro pets no longer attack in PVP prohibited areas, but
still do outside them. Zim'Torga in Zul'Drak is a good example to test
this (ID 4323). Lookout for death knights with a Risen Ally
(uncontrolled and naturally aggro) now they respect PVP prohibition like
their master.

Note: If you manually teleport a bot that is in mid combat to a PVP
prohibited area, its aggro pet might still attack, because its master is
still in combat strategy. Otherwise the pet will not attack if its
master has switched to non-combat.

src/strategy/actions/AttackAction.cpp

@@ -84,9 +84,10 @@ bool AttackAction::Attack(Unit* target, bool with_pet /*true*/)
         return false;
     }
 
-   if ((sPlayerbotAIConfig->IsInPvpProhibitedZone(bot->GetZoneId()) ||
-     sPlayerbotAIConfig->IsInPvpProhibitedArea(bot->GetAreaId()))
-        && (target->IsPlayer() || target->IsPet()))
+    // Check if bot OR target is in prohibited zone/area
+    if ((target->IsPlayer() || target->IsPet()) &&
+        (sPlayerbotAIConfig->IsPvpProhibited(bot->GetZoneId(), bot->GetAreaId()) ||
+        sPlayerbotAIConfig->IsPvpProhibited(target->GetZoneId(), target->GetAreaId())))
     {
         if (verbose)
             botAI->TellError("I cannot attack other players in PvP prohibited areas.");

src/strategy/actions/PetsAction.cpp

@@ -25,7 +25,9 @@ bool PetsAction::Execute(Event event)
     if (param.empty())
     {
         // If no parameter is provided, show usage instructions and return.
-        botAI->TellError("Usage: pet <aggressive|defensive|passive|stance|attack|follow|stay>");
+        std::string text = sPlayerbotTextMgr->GetBotTextOrDefault(
+            "pet_usage_error", "Usage: pet <aggressive|defensive|passive|stance|attack|follow|stay>", {});
+        botAI->TellError(text);
         return false;
     }
 
@@ -52,7 +54,9 @@ bool PetsAction::Execute(Event event)
     // If no pets or guardians are found, notify and return.
     if (targets.empty())
     {
-        botAI->TellError("You have no pet or guardian pet.");
+        std::string text = sPlayerbotTextMgr->GetBotTextOrDefault(
+            "pet_no_pet_error", "You have no pet or guardian pet.", {});
+        botAI->TellError(text);
         return false;
     }
 
@@ -63,42 +67,54 @@ bool PetsAction::Execute(Event event)
     if (param == "aggressive")
     {
         react = REACT_AGGRESSIVE;
-        stanceText = "aggressive";
+        stanceText = sPlayerbotTextMgr->GetBotTextOrDefault(
+            "pet_stance_aggressive", "aggressive", {});
     }
     else if (param == "defensive")
     {
         react = REACT_DEFENSIVE;
-        stanceText = "defensive";
+        stanceText = sPlayerbotTextMgr->GetBotTextOrDefault(
+            "pet_stance_defensive", "defensive", {});
     }
     else if (param == "passive")
     {
         react = REACT_PASSIVE;
-        stanceText = "passive";
+        stanceText = sPlayerbotTextMgr->GetBotTextOrDefault(
+            "pet_stance_passive", "passive", {});
     }
     // The "stance" command simply reports the current stance of each pet/guardian.
     else if (param == "stance")
     {
         for (Creature* target : targets)
         {
-            std::string type = target->IsPet() ? "pet" : "guardian";
+            std::string type = target->IsPet() ?
+                sPlayerbotTextMgr->GetBotTextOrDefault("pet_type_pet", "pet", {}) :
+                sPlayerbotTextMgr->GetBotTextOrDefault("pet_type_guardian", "guardian", {});
             std::string name = target->GetName();
             std::string stance;
             switch (target->GetReactState())
             {
                 case REACT_AGGRESSIVE:
-                    stance = "aggressive";
+                    stance = sPlayerbotTextMgr->GetBotTextOrDefault(
+                        "pet_stance_aggressive", "aggressive", {});
                     break;
                 case REACT_DEFENSIVE:
-                    stance = "defensive";
+                    stance = sPlayerbotTextMgr->GetBotTextOrDefault(
+                        "pet_stance_defensive", "defensive", {});
                     break;
                 case REACT_PASSIVE:
-                    stance = "passive";
+                    stance = sPlayerbotTextMgr->GetBotTextOrDefault(
+                        "pet_stance_passive", "passive", {});
                     break;
                 default:
-                    stance = "unknown";
+                    stance = sPlayerbotTextMgr->GetBotTextOrDefault(
+                        "pet_stance_unknown", "unknown", {});
                     break;
             }
-            botAI->TellMaster("Current stance of " + type + " \"" + name + "\": " + stance + ".");
+            std::string text = sPlayerbotTextMgr->GetBotTextOrDefault(
+                "pet_stance_report", "Current stance of %type \"%name\": %stance.",
+                {{"type", type}, {"name", name}, {"stance", stance}});
+            botAI->TellMaster(text);
         }
         return true;
     }
@@ -121,17 +137,31 @@ bool PetsAction::Execute(Event event)
         // If no valid target is selected, show an error and return.
         if (!targetUnit)
         {
-            botAI->TellError("No valid target selected by master.");
+            std::string text = sPlayerbotTextMgr->GetBotTextOrDefault(
+                "pet_no_target_error", "No valid target selected by master.", {});
+            botAI->TellError(text);
             return false;
         }
         if (!targetUnit->IsAlive())
         {
-            botAI->TellError("Target is not alive.");
+            std::string text = sPlayerbotTextMgr->GetBotTextOrDefault(
+                "pet_target_dead_error", "Target is not alive.", {});
+            botAI->TellError(text);
             return false;
         }
         if (!bot->IsValidAttackTarget(targetUnit))
         {
-            botAI->TellError("Target is not a valid attack target for the bot.");
+            std::string text = sPlayerbotTextMgr->GetBotTextOrDefault(
+                "pet_invalid_target_error", "Target is not a valid attack target for the bot.", {});
+            botAI->TellError(text);
+            return false;
+        }
+        if (sPlayerbotAIConfig->IsPvpProhibited(bot->GetZoneId(), bot->GetAreaId())
+            && (targetUnit->IsPlayer() || targetUnit->IsPet()))
+        {
+            std::string text = sPlayerbotTextMgr->GetBotTextOrDefault(
+                "pet_pvp_prohibited_error", "I cannot command my pet to attack players in PvP prohibited areas.", {});
+            botAI->TellError(text);
             return false;
         }
 
@@ -182,9 +212,17 @@ bool PetsAction::Execute(Event event)
         }
         // Inform the master if the command succeeded or failed.
         if (didAttack && sPlayerbotAIConfig->petChatCommandDebug == 1)
-            botAI->TellMaster("Pet commanded to attack your target.");
+        {
+            std::string text = sPlayerbotTextMgr->GetBotTextOrDefault(
+                "pet_attack_success", "Pet commanded to attack your target.", {});
+            botAI->TellMaster(text);
+        }
         else if (!didAttack)
-            botAI->TellError("Pet did not attack. (Already attacking or unable to attack target)");
+        {
+            std::string text = sPlayerbotTextMgr->GetBotTextOrDefault(
+                "pet_attack_failed", "Pet did not attack. (Already attacking or unable to attack target)", {});
+            botAI->TellError(text);
+        }
         return didAttack;
     }
     // The "follow" command makes all pets/guardians follow the bot.
@@ -192,7 +230,11 @@ bool PetsAction::Execute(Event event)
     {
         botAI->PetFollow();
         if (sPlayerbotAIConfig->petChatCommandDebug == 1)
-            botAI->TellMaster("Pet commanded to follow.");
+        {
+            std::string text = sPlayerbotTextMgr->GetBotTextOrDefault(
+                "pet_follow_success", "Pet commanded to follow.", {});
+            botAI->TellMaster(text);
+        }
         return true;
     }
     // The "stay" command causes all pets/guardians to stop and stay in place.
@@ -229,14 +271,20 @@ bool PetsAction::Execute(Event event)
             }
         }
         if (sPlayerbotAIConfig->petChatCommandDebug == 1)
-            botAI->TellMaster("Pet commanded to stay.");
+        {
+            std::string text = sPlayerbotTextMgr->GetBotTextOrDefault(
+                "pet_stay_success", "Pet commanded to stay.", {});
+            botAI->TellMaster(text);
+        }
         return true;
     }
     // Unknown command: show usage instructions and return.
     else
     {
-        botAI->TellError("Unknown pet command: " + param +
-                         ". Use: pet <aggressive|defensive|passive|stance|attack|follow|stay>");
+        std::string text = sPlayerbotTextMgr->GetBotTextOrDefault(
+            "pet_unknown_command_error", "Unknown pet command: %param. Use: pet <aggressive|defensive|passive|stance|attack|follow|stay>",
+            {{"param", param}});
+        botAI->TellError(text);
         return false;
     }
 
@@ -251,7 +299,12 @@ bool PetsAction::Execute(Event event)
 
     // Inform the master of the new stance if debug is enabled.
     if (sPlayerbotAIConfig->petChatCommandDebug == 1)
-        botAI->TellMaster("Pet stance set to " + stanceText + ".");
+    {
+        std::string text = sPlayerbotTextMgr->GetBotTextOrDefault(
+            "pet_stance_set_success", "Pet stance set to %stance.",
+            {{"stance", stanceText}});
+        botAI->TellMaster(text);
+    }
 
     return true;
-}
+}

src/strategy/values/AttackersValue.cpp

@@ -107,7 +107,6 @@ void AttackersValue::AddAttackersOf(Player* player, std::unordered_set<Unit*>& t
     {
         ThreatMgr* threatMgr = ref->GetSource();
         Unit* attacker = threatMgr->GetOwner();
-        Unit* victim = attacker->GetVictim();
 
         if (player->IsValidAttackTarget(attacker) &&
             player->GetDistance2d(attacker) < sPlayerbotAIConfig->sightDistance)
@@ -142,57 +141,107 @@ bool AttackersValue::hasRealThreat(Unit* attacker)
     (attacker->GetThreatMgr().getCurrentVictim() || dynamic_cast<Player*>(attacker));
 }
 
-bool AttackersValue::IsPossibleTarget(Unit* attacker, Player* bot, float range)
+bool AttackersValue::IsPossibleTarget(Unit* attacker, Player* bot, float /*range*/)
 {
-    Creature* c = attacker->ToCreature();
-    bool rti = false;
-    if (attacker && bot->GetGroup())
-        rti = bot->GetGroup()->GetTargetIcon(7) == attacker->GetGUID();
-
     PlayerbotAI* botAI = GET_PLAYERBOT_AI(bot);
+    if (!botAI)
+        return false;
 
-    bool leaderHasThreat = false;
-    if (attacker && bot->GetGroup() && botAI->GetMaster())
-        leaderHasThreat = attacker->GetThreatMgr().GetThreat(botAI->GetMaster());
-
-    bool isMemberBotGroup = false;
-    if (bot->GetGroup() && botAI->GetMaster())
-    {
-        PlayerbotAI* masterBotAI = GET_PLAYERBOT_AI(botAI->GetMaster());
-        if (masterBotAI && !masterBotAI->IsRealPlayer())
-            isMemberBotGroup = true;
-    }
+    // Basic check
+    if (!attacker)
+        return false;
 
     // bool inCannon = botAI->IsInVehicle(false, true);
     // bool enemy = botAI->GetAiObjectContext()->GetValue<Unit*>("enemy player target")->Get();
 
-    return attacker && attacker->IsVisible() && attacker->IsInWorld() && attacker->GetMapId() == bot->GetMapId() &&
-           !attacker->isDead() &&
-           !attacker->HasFlag(UNIT_FIELD_FLAGS, UNIT_FLAG_NON_ATTACKABLE | UNIT_FLAG_NON_ATTACKABLE_2) &&
-           // (inCannon || !attacker->HasFlag(UNIT_FIELD_FLAGS, UNIT_FLAG_NOT_SELECTABLE)) &&
-           // attacker->CanSeeOrDetect(bot) &&
-           // !(attacker->HasUnitState(UNIT_STATE_STUNNED) && botAI->HasAura("shackle undead", attacker)) &&
-           // !((attacker->IsPolymorphed() || botAI->HasAura("sap", attacker) || /*attacker->IsCharmed() ||*/
-           // attacker->isFeared()) && !rti) &&
-           /*!sServerFacade->IsInRoots(attacker) &&*/
-           !attacker->IsFriendlyTo(bot) && !attacker->HasSpiritOfRedemptionAura() &&
-           // !(attacker->GetGUID().IsPet() && enemy) &&
-           !(attacker->GetCreatureType() == CREATURE_TYPE_CRITTER && !attacker->IsInCombat()) &&
-           !attacker->HasUnitFlag(UNIT_FLAG_IMMUNE_TO_PC) && !attacker->HasUnitFlag(UNIT_FLAG_NOT_SELECTABLE) &&
-           bot->CanSeeOrDetect(attacker) &&
-           !(sPlayerbotAIConfig->IsPvpProhibited(attacker->GetZoneId(), attacker->GetAreaId()) &&
-             (attacker->GetGUID().IsPlayer() || attacker->GetGUID().IsPet())) &&
-           !(attacker->IsPlayer() && !attacker->IsPvP() && !attacker->IsFFAPvP() &&
-             (!bot->duel || bot->duel->Opponent != attacker)) &&
-           (!c ||
-            (!c->IsInEvadeMode() &&
-             ((!isMemberBotGroup && botAI->HasStrategy("attack tagged", BOT_STATE_NON_COMBAT)) || leaderHasThreat ||
-              (!c->hasLootRecipient() &&
-               (!c->GetVictim() ||
-                (c->GetVictim() &&
-                 ((!c->GetVictim()->IsPlayer() || bot->IsInSameGroupWith(c->GetVictim()->ToPlayer())) ||
-                  (botAI->GetMaster() && c->GetVictim() == botAI->GetMaster()))))) ||
-              c->isTappedBy(bot))));
+    // Validity checks
+    if (!attacker->IsVisible() || !attacker->IsInWorld() || attacker->GetMapId() != bot->GetMapId())
+        return false;
+
+    if (attacker->isDead() || attacker->HasSpiritOfRedemptionAura())
+        return false;
+
+    // Flag checks
+    if (attacker->HasFlag(UNIT_FIELD_FLAGS, UNIT_FLAG_NON_ATTACKABLE | UNIT_FLAG_NON_ATTACKABLE_2))
+        return false;
+
+    if (attacker->HasUnitFlag(UNIT_FLAG_IMMUNE_TO_PC) || attacker->HasUnitFlag(UNIT_FLAG_NOT_SELECTABLE))
+        return false;
+
+    // Relationship checks
+    if (attacker->IsFriendlyTo(bot))
+        return false;
+
+    // Critter exception
+    if (attacker->GetCreatureType() == CREATURE_TYPE_CRITTER && !attacker->IsInCombat())
+        return false;
+
+    // Visibility check
+    if (!bot->CanSeeOrDetect(attacker))
+        return false;
+
+    // PvP prohibition checks
+    if ((attacker->GetGUID().IsPlayer() || attacker->GetGUID().IsPet()) &&
+        (sPlayerbotAIConfig->IsPvpProhibited(attacker->GetZoneId(), attacker->GetAreaId()) ||
+        sPlayerbotAIConfig->IsPvpProhibited(bot->GetZoneId(), bot->GetAreaId())))
+    {
+        // This will stop aggresive pets from starting an attack.
+        // This will stop currently attacking pets from continuing their attack.
+        // This will first require the bot to change from a combat strat. It will
+        // not be reached if the bot only switches targets, including NPC targets.
+        for (Unit::ControlSet::const_iterator itr = bot->m_Controlled.begin();
+            itr != bot->m_Controlled.end(); ++itr)
+        {
+            Creature* creature = dynamic_cast<Creature*>(*itr);
+            if (creature && creature->GetVictim() == attacker)
+            {
+                creature->AttackStop();
+                if (CharmInfo* charmInfo = creature->GetCharmInfo())
+                    charmInfo->SetIsCommandAttack(false);
+            }
+        }
+
+        return false;
+    }
+
+    // Unflagged player check
+    if (attacker->IsPlayer() && !attacker->IsPvP() && !attacker->IsFFAPvP() &&
+        (!bot->duel || bot->duel->Opponent != attacker))
+        return false;
+
+    // Creature-specific checks
+    Creature* c = attacker->ToCreature();
+    if (c)
+    {
+        if (c->IsInEvadeMode())
+            return false;
+
+        bool leaderHasThreat = false;
+        if (bot->GetGroup() && botAI->GetMaster())
+            leaderHasThreat = attacker->GetThreatMgr().GetThreat(botAI->GetMaster());
+
+        bool isMemberBotGroup = false;
+        if (bot->GetGroup() && botAI->GetMaster())
+        {
+            PlayerbotAI* masterBotAI = GET_PLAYERBOT_AI(botAI->GetMaster());
+            if (masterBotAI && !masterBotAI->IsRealPlayer())
+                isMemberBotGroup = true;
+        }
+
+        bool canAttack = (!isMemberBotGroup && botAI->HasStrategy("attack tagged", BOT_STATE_NON_COMBAT)) ||
+            leaderHasThreat ||
+            (!c->hasLootRecipient() &&
+                (!c->GetVictim() ||
+                    (c->GetVictim() &&
+                        ((!c->GetVictim()->IsPlayer() || bot->IsInSameGroupWith(c->GetVictim()->ToPlayer())) ||
+                            (botAI->GetMaster() && c->GetVictim() == botAI->GetMaster()))))) ||
+            c->isTappedBy(bot);
+
+        if (!canAttack)
+            return false;
+    }
+
+    return true;
 }
 
 bool AttackersValue::IsValidTarget(Unit* attacker, Player* bot)