Core/Unit: Spell focusing now no longer nonsensical. HasSpellFocus always const. Error logs for various stuff.

9 files changed, 82 insertions, 93 deletions

diff --git a/src/server/game/AI/CreatureAI.cpp b/src/server/game/AI/CreatureAI.cpp
index c03d848275d..f084fe547d5 100644
--- a/src/server/game/AI/CreatureAI.cpp
+++ b/src/server/game/AI/CreatureAI.cpp
@@ -249,7 +249,7 @@ bool CreatureAI::UpdateVictim()
     if (!me->HasReactState(REACT_PASSIVE))
     {
         if (Unit* victim = me->SelectVictim())
-            if (!me->HandleSpellFocus(nullptr, true) && victim != me->GetVictim())
+            if (!me->HasSpellFocus() && victim != me->GetVictim())
                 AttackStart(victim);
 
         return me->GetVictim() != nullptr;
@@ -308,7 +308,7 @@ bool CreatureAI::_EnterEvadeMode(EvadeReason /*why*/)
     me->ResetPlayerDamageReq();
     me->SetLastDamagedTime(0);
     me->SetCannotReachTarget(false);
-    me->DoNotReacquireTarget();
+    me->DoNotReacquireSpellFocusTarget();
     EngagementOver();
 
     return true;
diff --git a/src/server/game/Entities/Creature/Creature.cpp b/src/server/game/Entities/Creature/Creature.cpp
index 819d19b4f09..1d81cc1a044 100644
--- a/src/server/game/Entities/Creature/Creature.cpp
+++ b/src/server/game/Entities/Creature/Creature.cpp
@@ -250,7 +250,7 @@ Creature::Creature(bool isWorldObject): Unit(isWorldObject), MapObject(), m_grou
     m_corpseRemoveTime(0), m_respawnTime(0), m_respawnDelay(300), m_corpseDelay(60), m_respawnradius(0.0f), m_boundaryCheckTime(2500), m_combatPulseTime(0), m_combatPulseDelay(0), m_reactState(REACT_AGGRESSIVE),
     m_defaultMovementType(IDLE_MOTION_TYPE), m_spawnId(0), m_equipmentId(0), m_originalEquipmentId(0), m_AlreadyCallAssistance(false), m_AlreadySearchedAssistance(false), m_cannotReachTarget(false), m_cannotReachTimer(0),
     m_meleeDamageSchoolMask(SPELL_SCHOOL_MASK_NORMAL), m_originalEntry(0), m_homePosition(), m_transportHomePosition(), m_creatureInfo(nullptr), m_creatureData(nullptr), _waypointPathId(0), _currentWaypointNodeInfo(0, 0),
-    m_formation(nullptr), m_triggerJustAppeared(true), m_respawnCompatibilityMode(false), _focusSpell(nullptr), _spellFocusDelay(0), _shouldReacquireSpellFocusTarget(false), _suppressedSpellFocusOrientation(0.0f), _lastDamagedTime(0),
+    m_formation(nullptr), m_triggerJustAppeared(true), m_respawnCompatibilityMode(false), _lastDamagedTime(0),
     _regenerateHealth(true), _regenerateHealthLock(false)
 {
     m_regenTimer = CREATURE_REGEN_INTERVAL;
@@ -734,22 +734,12 @@ void Creature::Update(uint32 diff)
                 break;
 
             GetThreatManager().Update(diff);
-
-            if (_shouldReacquireSpellFocusTarget && !HandleSpellFocus(nullptr, true))
+            if (_spellFocusInfo.delay)
             {
-                SetTarget(_suppressedSpellFocusTarget);
-
-                if (!HasFlag(UNIT_FIELD_FLAGS_2, UNIT_FLAG2_DISABLE_TURN))
-                {
-                    if (_suppressedSpellFocusTarget)
-                    {
-                        if (WorldObject const* objTarget = ObjectAccessor::GetWorldObject(*this, _suppressedSpellFocusTarget))
-                            SetFacingToObject(objTarget, false);
-                    }
-                    else
-                        SetFacingTo(_suppressedSpellFocusOrientation, false);
-                }
-                _shouldReacquireSpellFocusTarget = false;
+                if (_spellFocusInfo.delay <= diff)
+                    ReacquireSpellFocusTarget();
+                else
+                    _spellFocusInfo.delay -= 0;
             }
 
             // periodic check to see if the creature has passed an evade boundary
@@ -1120,7 +1110,7 @@ Unit* Creature::SelectVictim()
 
     if (target && _IsTargetAcceptable(target) && CanCreatureAttack(target))
     {
-        if (!HandleSpellFocus(nullptr, true))
+        if (!HasSpellFocus())
             SetInFront(target);
         return target;
     }
@@ -1952,8 +1942,8 @@ void Creature::setDeathState(DeathState s)
         SaveRespawnTime();
 
         ReleaseSpellFocus(nullptr, false); // remove spellcast focus
-        DoNotReacquireTarget(); // cancel delayed re-target
-        SetTarget(ObjectGuid::Empty); // drop target - dead mobs shouldn't ever target things
+        DoNotReacquireSpellFocusTarget();  // cancel delayed re-target
+        SetTarget(ObjectGuid::Empty);      // drop target - dead mobs shouldn't ever target things
 
         SetUInt32Value(UNIT_NPC_FLAGS, UNIT_NPC_FLAG_NONE);
 
@@ -3032,16 +3022,16 @@ void Creature::SetDisplayId(uint32 modelId)
 
 void Creature::SetTarget(ObjectGuid guid)
 {
-    if (HandleSpellFocus(nullptr, true))
-        _suppressedSpellFocusTarget = guid;
+    if (HasSpellFocus())
+        _spellFocusInfo.target = guid;
     else
         SetGuidValue(UNIT_FIELD_TARGET, guid);
 }
 
-void Creature::SetSpellFocusTarget(Spell const* focusSpell, WorldObject const* target)
+void Creature::SetSpellFocus(Spell const* focusSpell, WorldObject const* target)
 {
     // already focused
-    if (_focusSpell)
+    if (_spellFocusInfo.spell)
         return;
 
     // Prevent dead/feigning death creatures from setting a focus target, so they won't turn
@@ -3066,14 +3056,17 @@ void Creature::SetSpellFocusTarget(Spell const* focusSpell, WorldObject const* t
         return;
 
     // store pre-cast values for target and orientation (used to later restore)
-    if (!HandleSpellFocus(nullptr, true))
+    if (!_spellFocusInfo.delay)
     { // only overwrite these fields if we aren't transitioning from one spell focus to another
-        _suppressedSpellFocusTarget = GetGuidValue(UNIT_FIELD_TARGET);
-        _suppressedSpellFocusOrientation = GetOrientation();
+        _spellFocusInfo.target = GetGuidValue(UNIT_FIELD_TARGET);
+        _spellFocusInfo.orientation = GetOrientation();
     }
+    else // don't automatically reacquire target for the previous spellcast
+        _spellFocusInfo.delay = 0;
 
-    _focusSpell = focusSpell;
+    _spellFocusInfo.spell = focusSpell;
 
+    bool const noTurnDuringCast = spellInfo->HasAttribute(SPELL_ATTR5_DONT_TURN_DURING_CAST);
     // set target, then force send update packet to players if it changed to provide appropriate facing
     ObjectGuid newTarget = target ? target->GetGUID() : ObjectGuid::Empty;
     if (GetGuidValue(UNIT_FIELD_TARGET) != newTarget)
@@ -3086,7 +3079,7 @@ void Creature::SetSpellFocusTarget(Spell const* focusSpell, WorldObject const* t
                 spellInfo->SpellVisual[1]
             ) && (
                 !focusSpell->GetCastTime() || // if the spell is instant cast
-                spellInfo->HasAttribute(SPELL_ATTR5_DONT_TURN_DURING_CAST) // client gets confused if we attempt to turn at the regularly scheduled update packet
+                noTurnDuringCast // client gets confused if we attempt to turn at the regularly scheduled update packet
             )
         )
         {
@@ -3101,8 +3094,6 @@ void Creature::SetSpellFocusTarget(Spell const* focusSpell, WorldObject const* t
         }
     }
 
-    bool const noTurnDuringCast = spellInfo->HasAttribute(SPELL_ATTR5_DONT_TURN_DURING_CAST);
-
     if (!HasFlag(UNIT_FIELD_FLAGS_2, UNIT_FLAG2_DISABLE_TURN))
     {
         // Face the target - we need to do this before the unit state is modified for no-turn spells
@@ -3117,61 +3108,65 @@ void Creature::SetSpellFocusTarget(Spell const* focusSpell, WorldObject const* t
         AddUnitState(UNIT_STATE_FOCUSING);
 }
 
-bool Creature::HandleSpellFocus(Spell const* focusSpell, bool withDelay)
+bool Creature::HasSpellFocus(Spell const* focusSpell) const
 {
     if (!IsAlive()) // dead creatures cannot focus
     {
-        ReleaseSpellFocus(nullptr, false);
-        return false;
-    }
-
-    if (focusSpell && (focusSpell != _focusSpell))
-        return false;
-
-    if (!_focusSpell)
-    {
-        if (!withDelay || !_spellFocusDelay)
-            return false;
-        if (GetMSTimeDiffToNow(_spellFocusDelay) > 1000) // @todo figure out if we can get rid of this magic number somehow
+        if (_spellFocusInfo.spell || _spellFocusInfo.delay)
         {
-            _spellFocusDelay = 0; // save checks in the future
-            return false;
+            TC_LOG_WARN("entities.unit", "Creature '%s' (entry %u) has spell focus (spell id %u, delay %ums) despite being dead.",
+                        GetName().c_str(), GetEntry(), _spellFocusInfo.spell ? _spellFocusInfo.spell->GetSpellInfo()->Id : 0, _spellFocusInfo.delay);
         }
+        return false;
     }
 
-    return true;
+    if (focusSpell)
+        return (focusSpell == _spellFocusInfo.spell);
+    else
+        return (_spellFocusInfo.spell || _spellFocusInfo.delay);
 }
 
 void Creature::ReleaseSpellFocus(Spell const* focusSpell, bool withDelay)
 {
-    if (!_focusSpell)
+    if (!_spellFocusInfo.spell)
         return;
 
     // focused to something else
-    if (focusSpell && focusSpell != _focusSpell)
+    if (focusSpell && focusSpell != _spellFocusInfo.spell)
         return;
 
-    if (IsPet() && !HasFlag(UNIT_FIELD_FLAGS_2, UNIT_FLAG2_DISABLE_TURN)) // player pets do not use delay system
+    if (_spellFocusInfo.spell->GetSpellInfo()->HasAttribute(SPELL_ATTR5_DONT_TURN_DURING_CAST))
+        ClearUnitState(UNIT_STATE_FOCUSING);
+
+    if (IsPet()) // player pets do not use delay system
     {
-        SetGuidValue(UNIT_FIELD_TARGET, _suppressedSpellFocusTarget);
-        if (_suppressedSpellFocusTarget)
+        if (!HasFlag(UNIT_FIELD_FLAGS_2, UNIT_FLAG2_DISABLE_TURN))
+            ReacquireSpellFocusTarget();
+    }
+    else // don't allow re-target right away to prevent visual bugs
+        _spellFocusInfo.delay = withDelay ? 1000 : 1;
+
+    _spellFocusInfo.spell = nullptr;
+}
+
+void Creature::ReacquireSpellFocusTarget()
+{
+    if (!HasSpellFocus())
+        return;
+
+    SetTarget(_spellFocusInfo.target);
+
+    if (!HasFlag(UNIT_FIELD_FLAGS_2, UNIT_FLAG2_DISABLE_TURN))
+    {
+        if (_spellFocusInfo.target)
         {
-            if (WorldObject const* objTarget = ObjectAccessor::GetWorldObject(*this, _suppressedSpellFocusTarget))
+            if (WorldObject const* objTarget = ObjectAccessor::GetWorldObject(*this, _spellFocusInfo.target))
                 SetFacingToObject(objTarget, false);
         }
         else
-            SetFacingTo(_suppressedSpellFocusOrientation, false);
+            SetFacingTo(_spellFocusInfo.orientation, false);
     }
-    else
-        // tell the creature that it should reacquire its actual target after the delay expires (this is handled in ::Update)
-        // player pets don't need to do this, as they automatically reacquire their target on focus release
-        MustReacquireTarget();
-
-    if (_focusSpell->GetSpellInfo()->HasAttribute(SPELL_ATTR5_DONT_TURN_DURING_CAST))
-        ClearUnitState(UNIT_STATE_FOCUSING);
-
-    _focusSpell = nullptr;
-    _spellFocusDelay = (!IsPet() && withDelay) ? GameTime::GetGameTimeMS() : 0; // don't allow re-target right away to prevent visual bugs
+    _spellFocusInfo.delay = 0;
 }
 
 bool Creature::IsMovementPreventedByCasting() const
@@ -3184,7 +3179,7 @@ bool Creature::IsMovementPreventedByCasting() const
                 return false;
     }
 
-    if (HasSpellFocusTarget())
+    if (HasSpellFocus())
         return true;
 
     if (HasUnitState(UNIT_STATE_CASTING))
diff --git a/src/server/game/Entities/Creature/Creature.h b/src/server/game/Entities/Creature/Creature.h
index 41170334aac..ed4b8fce669 100644
--- a/src/server/game/Entities/Creature/Creature.h
+++ b/src/server/game/Entities/Creature/Creature.h
@@ -340,18 +340,11 @@ class TC_GAME_API Creature : public Unit, public GridObject<Creature>, public Ma
 
         // Handling caster facing during spellcast
         void SetTarget(ObjectGuid guid) override;
-        void MustReacquireTarget() { _shouldReacquireSpellFocusTarget = true; } // flags the Creature for forced (client displayed) target reacquisition in the next ::Update call
-        void DoNotReacquireTarget()
-        {
-            _shouldReacquireSpellFocusTarget = false;
-            _suppressedSpellFocusTarget = ObjectGuid::Empty;
-            SetGuidValue(UNIT_FIELD_TARGET, ObjectGuid::Empty);
-            _suppressedSpellFocusOrientation = 0.0f;
-        }
-        void SetSpellFocusTarget(Spell const* focusSpell, WorldObject const* target);
-        bool HandleSpellFocus(Spell const* focusSpell = nullptr, bool withDelay = false) override;
+        void ReacquireSpellFocusTarget();
+        void DoNotReacquireSpellFocusTarget() { _spellFocusInfo.delay = 0; }
+        void SetSpellFocus(Spell const* focusSpell, WorldObject const* target);
+        bool HasSpellFocus(Spell const* focusSpell = nullptr) const override;
         void ReleaseSpellFocus(Spell const* focusSpell = nullptr, bool withDelay = true);
-        bool HasSpellFocusTarget() const override { return IsAlive() && (_focusSpell || _spellFocusDelay); }
 
         bool IsMovementPreventedByCasting() const override;
 
@@ -438,11 +431,13 @@ class TC_GAME_API Creature : public Unit, public GridObject<Creature>, public Ma
         bool m_respawnCompatibilityMode;
 
         /* Spell focus system */
-        Spell const* _focusSpell; // Locks the target during spell cast for proper facing
-        uint32 _spellFocusDelay;
-        bool _shouldReacquireSpellFocusTarget;
-        ObjectGuid _suppressedSpellFocusTarget; // Stores the creature's "real" target while casting
-        float _suppressedSpellFocusOrientation; // Stores the creature's "real" orientation while casting
+        struct
+        {
+            Spell const* spell = nullptr;
+            uint32 delay = 0;         // ms until the creature's target should snap back (0 = no snapback scheduled)
+            ObjectGuid target;        // the creature's "real" target while casting
+            float orientation = 0.0f; // the creature's "real" orientation while casting
+        } _spellFocusInfo;
 
         time_t _lastDamagedTime; // Part of Evade mechanics
         CreatureTextRepeatGroup m_textRepeat;
diff --git a/src/server/game/Entities/Unit/Unit.h b/src/server/game/Entities/Unit/Unit.h
index 4b8a1da607a..f92c80c9b55 100644
--- a/src/server/game/Entities/Unit/Unit.h
+++ b/src/server/game/Entities/Unit/Unit.h
@@ -1357,8 +1357,7 @@ class TC_GAME_API Unit : public WorldObject
         Spell* FindCurrentSpellBySpellId(uint32 spell_id) const;
         int32 GetCurrentSpellCastTime(uint32 spell_id) const;
 
-        virtual bool HandleSpellFocus(Spell const* /*focusSpell*/ = nullptr, bool /*withDelay*/ = false) { return false; }
-        virtual bool HasSpellFocusTarget() const { return false; }
+        virtual bool HasSpellFocus(Spell const* /*focusSpell*/ = nullptr) const { return false; }
         virtual bool IsMovementPreventedByCasting() const;
 
         SpellHistory* GetSpellHistory() { return m_spellHistory; }
diff --git a/src/server/game/Handlers/PetHandler.cpp b/src/server/game/Handlers/PetHandler.cpp
index 7ca145b9428..c2cf331e4af 100644
--- a/src/server/game/Handlers/PetHandler.cpp
+++ b/src/server/game/Handlers/PetHandler.cpp
@@ -327,14 +327,14 @@ void WorldSession::HandlePetActionHelper(Unit* pet, ObjectGuid guid1, uint32 spe
             {
                 if (unit_target)
                 {
-                    if (!pet->HandleSpellFocus())
+                    if (!pet->HasSpellFocus())
                         pet->SetInFront(unit_target);
                     if (Player* player = unit_target->ToPlayer())
                         pet->SendUpdateToPlayer(player);
                 }
                 else if (Unit* unit_target2 = spell->m_targets.GetUnitTarget())
                 {
-                    if (!pet->HandleSpellFocus())
+                    if (!pet->HasSpellFocus())
                         pet->SetInFront(unit_target2);
                     if (Player* player = unit_target2->ToPlayer())
                         pet->SendUpdateToPlayer(player);
diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp
index 42fb5e2eab1..5e05fd2f911 100644
--- a/src/server/game/Spells/Spell.cpp
+++ b/src/server/game/Spells/Spell.cpp
@@ -3066,9 +3066,9 @@ SpellCastResult Spell::prepare(SpellCastTargets const& targets, AuraEffect const
         if (!(m_spellInfo->IsNextMeleeSwingSpell() || IsAutoRepeat()))
         {
             if (m_targets.GetObjectTarget() && m_caster != m_targets.GetObjectTarget())
-                m_caster->ToCreature()->SetSpellFocusTarget(this, m_targets.GetObjectTarget());
+                m_caster->ToCreature()->SetSpellFocus(this, m_targets.GetObjectTarget());
             else if (m_spellInfo->HasAttribute(SPELL_ATTR5_DONT_TURN_DURING_CAST))
-                m_caster->ToCreature()->SetSpellFocusTarget(this, nullptr);
+                m_caster->ToCreature()->SetSpellFocus(this, nullptr);
         }
     }
 
@@ -4554,8 +4554,8 @@ void Spell::SendChannelStart(uint32 duration)
 
         if (channelTarget != unitCaster->GetGUID())
             if (Creature* creatureCaster = unitCaster->ToCreature())
-                if (!creatureCaster->HandleSpellFocus(this))
-                    creatureCaster->SetSpellFocusTarget(this, ObjectAccessor::GetWorldObject(*creatureCaster, channelTarget));
+                if (!creatureCaster->HasSpellFocus(this))
+                    creatureCaster->SetSpellFocus(this, ObjectAccessor::GetWorldObject(*creatureCaster, channelTarget));
     }
 
     unitCaster->SetUInt32Value(UNIT_CHANNEL_SPELL, m_spellInfo->Id);
diff --git a/src/server/scripts/Northrend/FrozenHalls/HallsOfReflection/halls_of_reflection.cpp b/src/server/scripts/Northrend/FrozenHalls/HallsOfReflection/halls_of_reflection.cpp
index ce71046433c..243020e4d4c 100644
--- a/src/server/scripts/Northrend/FrozenHalls/HallsOfReflection/halls_of_reflection.cpp
+++ b/src/server/scripts/Northrend/FrozenHalls/HallsOfReflection/halls_of_reflection.cpp
@@ -1363,7 +1363,7 @@ class npc_the_lich_king_escape_hor : public CreatureScript
                 if (!me->HasReactState(REACT_PASSIVE))
                 {
                     if (Unit* victim = me->SelectVictim())
-                        if (!me->HandleSpellFocus(nullptr, true) && victim != me->GetVictim())
+                        if (!me->HasSpellFocus() && victim != me->GetVictim())
                             AttackStart(victim);
                     return me->GetVictim() != nullptr;
                 }
diff --git a/src/server/scripts/Northrend/IcecrownCitadel/boss_icecrown_gunship_battle.cpp b/src/server/scripts/Northrend/IcecrownCitadel/boss_icecrown_gunship_battle.cpp
index 2f2793b13bd..c663f696f46 100644
--- a/src/server/scripts/Northrend/IcecrownCitadel/boss_icecrown_gunship_battle.cpp
+++ b/src/server/scripts/Northrend/IcecrownCitadel/boss_icecrown_gunship_battle.cpp
@@ -630,7 +630,7 @@ protected:
         if (!me->HasReactState(REACT_PASSIVE))
         {
             if (Unit* victim = me->SelectVictim())
-                if (!me->HandleSpellFocus(nullptr, true) && victim != me->GetVictim())
+                if (!me->HasSpellFocus() && victim != me->GetVictim())
                     AttackStart(victim);
 
             return me->GetVictim() != nullptr;
diff --git a/src/server/scripts/Northrend/Ulduar/Ulduar/boss_thorim.cpp b/src/server/scripts/Northrend/Ulduar/Ulduar/boss_thorim.cpp
index 2ddc9af56f3..672fe283892 100644
--- a/src/server/scripts/Northrend/Ulduar/Ulduar/boss_thorim.cpp
+++ b/src/server/scripts/Northrend/Ulduar/Ulduar/boss_thorim.cpp
@@ -1821,7 +1821,7 @@ class spell_thorim_lightning_charge : public SpellScriptLoader
             {
                 /// @workaround: focus target is not working because spell is triggered and instant
                 if (Creature* creature = GetCaster()->ToCreature())
-                    creature->SetSpellFocusTarget(GetSpell(), GetExplTargetWorldObject());
+                    creature->SetSpellFocus(GetSpell(), GetExplTargetWorldObject());
             }
 
             void HandleCharge()