Trinitycore: [3.3.5] Spell/Hunter: Bestial Wrath needs validations.

[Noryad]

Description:
Bestial Wrath needs validations when the pet is dead or is not on line of sight with the hunter.

Current behaviour:
When the corpse of the pet is visible the hunter can use Bestial Wrath, the buff not appear and the ability is put into cooldown.
When the hunter is not on line of sight with the pet can use Bestial Wrath, the buff not appear and the ability is put into cooldown.

Expected behaviour:
With the pet is dead or out of line of sight with the hunter the ability can not be used.

Steps to reproduce the problem:

1. Attack and kill the hunter's pet. At the moment the hunter can use Bestial Wrath and should not.
2. The hunter relives the pet and orders him to stay. The hunter goes behind a mountain or a tree (out of line of sight) and can use Bestial Wrath and should not.

Branch(es): 3.3.5

TC rev. hash/commit: 674ac56

TDB version: TDB_full_world_335.62_2016_10_17 + updates

Operating system: Windows

[MrSmite]

This is actually a bug that was fixed in 5.2

https://worldofwarcraft.com/en-us/news/8896363
Bestial Wrath may now be activated when there is no line of sight to the Hunter's pet.

My vote is to leave it alone ;)

[ariel-]

At least add dead pet checks. In fact I think it would be better to generically check dead pet for all the spells that require having a pet in one way or another. (eg Focused Fire)

[Goatform]

It is not just Beastial Wrath, I actually made these reports for my core few days ago:

BUGS: Bestial Wrath, Kill Command, Intimidation, Feed Pet and Eyes of the Beast are castable while pet is dead (but spawned as corpse)

I am not sure if these things are blizzlike however. Also, I am not talking about LoS part since I am not familiar with that at all.

[MrSmite]

+1 for what @ariel- suggests (github isn't offering me the "thumbs up" button for your post).

@Goatform, those are indeed bugs. The core should report "your pet is dead" or "you don't have a pet" if the pet is dismissed.

As for LOS, it's a bit tricky because spells that effect both the Hunter and pet should technically be cast on the hunter then "shared" with the pet through an aura. The Hunter can never be out of LOS of him/herself so they should never fail for that.

[robinsch]

diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp
index cc3831a..3c2555d 100644
--- a/src/server/game/Spells/Spell.cpp
+++ b/src/server/game/Spells/Spell.cpp
@@ -4869,7 +4869,12 @@ SpellCastResult Spell::CheckCast(bool strict)
     {
         if (m_spellInfo->Effects[j].TargetA.GetTarget() == TARGET_UNIT_PET)
         {
-            if (!m_caster->GetGuardianPet())
+            if (Unit* pet = m_caster->GetGuardianPet())
+            {
+                if (!m_spellInfo->HasAttribute(SPELL_ATTR2_CAN_TARGET_NOT_IN_LOS) && !DisableMgr::IsDisabledFor(DISABLE_TYPE_SPELL, m_spellInfo->Id, NULL, SPELL_DISABLE_LOS) && !m_caster->IsWithinLOSInMap(pet))
+                    return SPELL_FAILED_LINE_OF_SIGHT;
+            }
+            else
             {
                 if (m_triggeredByAuraSpell)              // not report pet not existence for triggered spells
                     return SPELL_FAILED_DONT_REPORT;

Pretty simple check for LOS. The pet is dead message is sent by SMSG_PET_TAME_FAILURE which is not implemented yet.

diff --git a/src/server/game/Entities/Player/Player.cpp b/src/server/game/Entities/Player/Player.cpp
index 61c12c9..6ceee84 100644
--- a/src/server/game/Entities/Player/Player.cpp
+++ b/src/server/game/Entities/Player/Player.cpp
@@ -26470,3 +26470,10 @@ Creature* Player::GetCritter() const

     return nullptr;
 }
+
+void Player::SendPetTameFailureReason(PetTameFailureReason reason)
+{
+    WorldPacket data(SMSG_PET_TAME_FAILURE, 4);
+    data << uint32(reason);
+    SendDirectMessage(&data);
+}

diff --git a/src/server/game/Entities/Player/Player.h b/src/server/game/Entities/Player/Player.h
index 48ac0a6..a6d1731 100644
--- a/src/server/game/Entities/Player/Player.h
+++ b/src/server/game/Entities/Player/Player.h
@@ -2144,6 +2144,8 @@ class Player : public Unit, public GridObject<Player>
         uint32 GetLastPetNumber() const { return m_lastpetnumber; }
         void SetLastPetNumber(uint32 petnumber) { m_lastpetnumber = petnumber; }

+        void SendPetTameFailureReason(PetTameFailureReason reason);
+
         /*********************************************************/
         /***                   GROUP SYSTEM                    ***/
         /*********************************************************/

diff --git a/src/server/game/Miscellaneous/SharedDefines.h b/src/server/game/Miscellaneous/SharedDefines.h
index 8e649a2..a13cd0f 100644
--- a/src/server/game/Miscellaneous/SharedDefines.h
+++ b/src/server/game/Miscellaneous/SharedDefines.h
@@ -3579,4 +3579,22 @@ enum SpellCooldownFlags
     SPELL_COOLDOWN_FLAG_INCLUDE_EVENT_COOLDOWNS = 0x2   ///< Starts GCD for spells that should start their cooldown on events, requires SPELL_COOLDOWN_FLAG_INCLUDE_GCD set
 };

+enum PetTameFailureReason
+{
+    PET_TAME_FAILURE_UNK1 = 0,
+    PET_TAME_FAILURE_INVALID_CREATURE,
+    PET_TAME_FAILURE_TOO_MANY,
+    PET_TAME_FAILURE_CREATURE_ALREADY_OWNED,
+    PET_TAME_FAILURE_NOT_TAMEBLE,
+    PET_TAME_FAILURE_ANOTHER_SUMMON_ACTIVE,
+    PET_TAME_FAILURE_UNITS_CANNOT_TAME,
+    PET_TAME_FAILURE_NO_PET_AVAILABLE,
+    PET_TAME_FAILURE_INTERNAL_ERROR,
+    PET_TAME_FAILURE_TOO_HIGH_LEVEL,
+    PET_TAME_FAILURE_DEAD,
+    PET_TAME_FAILURE_NOT_DEAD,
+    PET_TAME_FAILURE_CANNOT_CONTROL_EXOTIC,
+    PET_TAME_FAILURE_INVALID_SLOT,
+};
+
 #endif
diff --git a/src/server/scripts/Spells/spell_hunter.cpp b/src/server/scripts/Spells/spell_hunter.cpp
index ccc566d..63fa745 100644
--- a/src/server/scripts/Spells/spell_hunter.cpp
+++ b/src/server/scripts/Spells/spell_hunter.cpp
@@ -858,18 +858,16 @@ class spell_hun_tame_beast : public SpellScriptLoader

                 if (Creature* target = GetExplTargetUnit()->ToCreature())
                 {
-                    if (target->getLevel() > caster->getLevel())
-                        return SPELL_FAILED_HIGHLEVEL;
-
-                    // use SMSG_PET_TAME_FAILURE?
-                    if (!target->GetCreatureTemplate()->IsTameable(caster->ToPlayer()->CanTameExoticPets()))
-                        return SPELL_FAILED_BAD_TARGETS;
+                    Player* player = caster->ToPlayer();

-                    if (caster->GetPetGUID())
-                        return SPELL_FAILED_ALREADY_HAVE_SUMMON;
+                    if (target->getLevel() > caster->getLevel())
+                        player->SendPetTameFailureReason(PET_TAME_FAILURE_TOO_HIGH_LEVEL);
+                    else if (!target->GetCreatureTemplate()->IsTameable(caster->ToPlayer()->CanTameExoticPets()))
+                        player->SendPetTameFailureReason(PET_TAME_FAILURE_CANNOT_CONTROL_EXOTIC);
+                    else if (caster->GetPetGUID() || caster->GetCharmGUID())
+                        player->SendPetTameFailureReason(PET_TAME_FAILURE_ANOTHER_SUMMON_ACTIVE);

-                    if (caster->GetCharmGUID())
-                        return SPELL_FAILED_ALREADY_HAVE_CHARM;
+                    return SPELL_FAILED_DONT_REPORT;
                 }
                 else
                     return SPELL_FAILED_BAD_IMPLICIT_TARGETS;

Also a dead pet should not be summoned by Call Pet for reference see:
https://github.com/cmangos/mangos-tbc/pull/139/files#diff-4f14a67c343c24da132bdf3c8706b7c4R92

[ariel-]

Duplicated #10198

[Noryad]

@robinsch #10198 is from 2013, it is possible to make a merge with your contribution?

[ghost]

@Noryad : better post in issue #10198 (since it is open) and even re-post content from this issue if needed.