Core/AI: Finally move the "is creature engaged" flag to be a property of the creature AI, where it honestly always belonged. Fixes #17981 and #23602 for real this time.

(cherry picked from commit 0e7c66cb4c7ff7d44e232d0b50703a48605ffd24)

3 files changed, 35 insertions, 34 deletions

diff --git a/src/server/game/Combat/CombatManager.cpp b/src/server/game/Combat/CombatManager.cpp
index d6f7a55665b..a43bb72eca4 100644
--- a/src/server/game/Combat/CombatManager.cpp
+++ b/src/server/game/Combat/CombatManager.cpp
@@ -293,13 +293,8 @@ void CombatManager::EndAllPvPCombat()
 
 /*static*/ void CombatManager::NotifyAICombat(Unit* me, Unit* other)
 {
-    if (!me->IsAIEnabled())
-        return;
-    me->GetAI()->JustEnteredCombat(other);
-
-    if (Creature* cMe = me->ToCreature())
-        if (!cMe->CanHaveThreatList())
-            cMe->AI()->JustEngagedWith(other);
+    if (UnitAI* ai = me->GetAI())
+        ai->JustEnteredCombat(other);
 }
 
 void CombatManager::PutReference(ObjectGuid const& guid, CombatReference* ref)
@@ -336,14 +331,14 @@ bool CombatManager::UpdateOwnerCombatState() const
     {
         _owner->AddUnitFlag(UNIT_FLAG_IN_COMBAT);
         _owner->AtEnterCombat();
-        if (!_owner->CanHaveThreatList() && !_owner->IsEngaged())
+        if (_owner->GetTypeId() == TYPEID_UNIT)
             _owner->AtEngage(GetAnyTarget());
     }
     else
     {
         _owner->RemoveUnitFlag(UNIT_FLAG_IN_COMBAT);
         _owner->AtExitCombat();
-        if (_owner->IsEngaged() && !(_owner->ToCreature() && _owner->CanHaveThreatList() && _owner->ToCreature()->IsAIEnabled()))
+        if (_owner->GetTypeId() != TYPEID_UNIT)
             _owner->AtDisengage();
     }
 
diff --git a/src/server/game/Combat/ThreatManager.cpp b/src/server/game/Combat/ThreatManager.cpp
index 183142f1cd7..6ad8de0aabb 100644
--- a/src/server/game/Combat/ThreatManager.cpp
+++ b/src/server/game/Combat/ThreatManager.cpp
@@ -71,6 +71,7 @@ void ThreatReference::UpdateOffline()
     {
         _online = ShouldBeSuppressed() ? ONLINE_STATE_SUPPRESSED : ONLINE_STATE_ONLINE;
         HeapNotifyIncreased();
+        _mgr.RegisterForAIUpdate(this);
     }
 }
 
@@ -199,10 +200,11 @@ Unit* ThreatManager::GetCurrentVictim()
 {
     if (!_currentVictimRef || _currentVictimRef->ShouldBeOffline())
         UpdateVictim();
-    return const_cast<ThreatManager const*>(this)->GetCurrentVictim();
+    ASSERT(!_currentVictimRef || _currentVictimRef->IsAvailable());
+    return _currentVictimRef ? _currentVictimRef->GetVictim() : nullptr;
 }
 
-Unit* ThreatManager::GetCurrentVictim() const
+Unit* ThreatManager::GetLastVictim() const
 {
     if (_currentVictimRef && !_currentVictimRef->ShouldBeOffline())
         return _currentVictimRef->GetVictim();
@@ -244,7 +246,7 @@ float ThreatManager::GetThreat(Unit const* who, bool includeOffline) const
     return (includeOffline || it->second->IsAvailable()) ? it->second->GetThreat() : 0.0f;
 }
 
-std::vector<ThreatReference*> ThreatManager::GetModifiableThreatList() const
+std::vector<ThreatReference*> ThreatManager::GetModifiableThreatList()
 {
     std::vector<ThreatReference*> list;
     list.reserve(_myThreatListEntries.size());
@@ -394,16 +396,14 @@ void ThreatManager::AddThreat(Unit* target, float amount, SpellInfo const* spell
     PutThreatListRef(target->GetGUID(), ref);
     target->GetThreatManager().PutThreatenedByMeRef(_owner->GetGUID(), ref);
 
-    // afterwards, we evaluate whether this is an online reference (it might not be an acceptable target, but we need to add it to our threat list before we check!)
     ref->UpdateOffline();
-    if (ref->IsOnline()) // ...and if the ref is online it also gets the threat it should have
+    if (ref->IsOnline()) // we only add the threat if the ref is currently available
         ref->AddThreat(amount);
 
-    if (!_owner->IsEngaged())
-    {
-        _owner->AtEngage(target);
+    if (!_currentVictimRef)
         UpdateVictim();
-    }
+    else
+        ProcessAIUpdates();
 }
 
 void ThreatManager::ScaleThreat(Unit* target, float factor)
@@ -488,14 +488,6 @@ void ThreatManager::ClearAllThreat()
     }
 }
 
-void ThreatManager::NotifyDisengaged()
-{
-    // note: i don't really like having this here
-    // (maybe engage flag should be in creature ai? it's inherently an AI property...)
-    if (_owner->IsEngaged())
-        _owner->AtDisengage();
-}
-
 void ThreatManager::FixateTarget(Unit* target)
 {
     if (target)
@@ -530,6 +522,7 @@ void ThreatManager::UpdateVictim()
         _needClientUpdate = false;
     }
 
+    ProcessAIUpdates();
 }
 
 ThreatReference const* ThreatManager::ReselectVictim()
@@ -538,7 +531,7 @@ ThreatReference const* ThreatManager::ReselectVictim()
         return nullptr;
 
     for (auto const& pair : _myThreatListEntries)
-        pair.second->UpdateOffline();
+        pair.second->UpdateOffline(); // AI notifies are processed in ::UpdateVictim caller
 
     // fixated target is always preferred
     if (_fixateRef && _fixateRef->IsAvailable())
@@ -587,6 +580,16 @@ ThreatReference const* ThreatManager::ReselectVictim()
     return nullptr;
 }
 
+void ThreatManager::ProcessAIUpdates()
+{
+    CreatureAI* ai = ASSERT_NOTNULL(_owner->ToCreature())->AI();
+    std::vector<ThreatReference const*> v(std::move(_needsAIUpdate)); // _needClientUpdate is now empty in case this triggers a recursive call
+    if (!ai)
+        return;
+    for (ThreatReference const* ref : v)
+        ai->JustStartedThreateningMe(ref->GetVictim());
+}
+
 // returns true if a is LOWER on the threat list than b
 /*static*/ bool ThreatManager::CompareReferencesLT(ThreatReference const* a, ThreatReference const* b, float aWeight)
 {
diff --git a/src/server/game/Combat/ThreatManager.h b/src/server/game/Combat/ThreatManager.h
index 6e4f20c55a9..8e722707f91 100644
--- a/src/server/game/Combat/ThreatManager.h
+++ b/src/server/game/Combat/ThreatManager.h
@@ -103,7 +103,7 @@ class TC_GAME_API ThreatManager
         bool CanHaveThreatList() const { return _ownerCanHaveThreatList; }
         // returns the current victim - this can be nullptr if owner's threat list is empty, or has only offline targets
         Unit* GetCurrentVictim();
-        Unit* GetCurrentVictim() const;
+        Unit* GetLastVictim() const;
         // returns an arbitrary non-offline victim from owner's threat list if one exists, nullptr otherwise
         Unit* GetAnyTarget() const;
 
@@ -121,10 +121,10 @@ class TC_GAME_API ThreatManager
         Trinity::IteratorPair<ThreatListIterator> GetUnsortedThreatList() const { return { _myThreatListEntries.begin(), _myThreatListEntries.end() }; }
         // slightly slower than GetUnsorted, but, well...sorted - only use it if you need the sorted property, of course
         // this iterator pair will invalidate on any modification (even indirect) of the threat list; spell casts and similar can all induce this!
-        // note: current tank is NOT guaranteed to be the first entry in this list - check GetCurrentVictim separately if you want that!
+        // note: current tank is NOT guaranteed to be the first entry in this list - check GetLastVictim separately if you want that!
         Trinity::IteratorPair<threat_list_heap::ordered_iterator> GetSortedThreatList() const { return { _sortedThreatList.ordered_begin(), _sortedThreatList.ordered_end() }; }
         // slowest of the three threat list getters (by far), but lets you modify the threat references - this is also sorted
-        std::vector<ThreatReference*> GetModifiableThreatList() const;
+        std::vector<ThreatReference*> GetModifiableThreatList();
 
         // does any unit have a threat list entry with victim == this.owner?
         bool IsThreateningAnyone(bool includeOffline = false) const;
@@ -154,9 +154,6 @@ class TC_GAME_API ThreatManager
         void ClearThreat(ThreatReference* ref);
         // Removes all targets from the threat list (will cause evade in UpdateVictim if called)
         void ClearAllThreat();
-        // THIS SHOULD ONLY BE CALLED FROM A CREATURE'S AI (typically in EnterEvadeMode)
-        // notify the unit that the AI has disengaged
-        void NotifyDisengaged();
 
         // Fixate on the passed target; this target will always be selected until the fixate is cleared
         // (if the target is not in the threat list, does nothing)
@@ -204,6 +201,12 @@ class TC_GAME_API ThreatManager
         threat_list_heap _sortedThreatList;
         std::unordered_map<ObjectGuid, ThreatReference*> _myThreatListEntries;
 
+        // AI notifies are delayed to ensure we are in a consistent state before we call out to arbitrary logic
+        // threat references might register themselves here when ::UpdateOffline() is called - MAKE SURE THIS IS PROCESSED JUST BEFORE YOU EXIT THREATMANAGER LOGIC
+        void ProcessAIUpdates();
+        void RegisterForAIUpdate(ThreatReference const* ref) { _needsAIUpdate.push_back(ref); }
+        std::vector<ThreatReference const*> _needsAIUpdate;
+
         // picks a new victim - called from ::Update periodically
         void UpdateVictim();
         ThreatReference const* ReselectVictim();
@@ -278,7 +281,7 @@ class TC_GAME_API ThreatReference
             _owner(reinterpret_cast<Creature*>(mgr->_owner)), _mgr(*mgr), _victim(victim),
             _baseAmount(0.0f), _tempModifier(0), _taunted(TAUNT_STATE_NONE)
         {
-            _online = ShouldBeSuppressed() ? ONLINE_STATE_SUPPRESSED : ONLINE_STATE_ONLINE;
+            _online = ONLINE_STATE_OFFLINE;
         }
 
         void UnregisterAndFree();