Core/Entities: Kick engagement logic upstairs to Unit (from ThreatManager), since all Units with AI need it (not just those with threat list). Fixes #17981.

8 files changed, 53 insertions, 52 deletions

diff --git a/src/server/game/Combat/CombatManager.cpp b/src/server/game/Combat/CombatManager.cpp
index 2d124b9b560..9e5a8f5fb02 100644
--- a/src/server/game/Combat/CombatManager.cpp
+++ b/src/server/game/Combat/CombatManager.cpp
@@ -337,11 +337,15 @@ bool CombatManager::UpdateOwnerCombatState() const
     {
         _owner->SetFlag(UNIT_FIELD_FLAGS, UNIT_FLAG_IN_COMBAT);
         _owner->AtEnterCombat();
+        if (!_owner->CanHaveThreatList() && !_owner->IsEngaged())
+            _owner->AtEngage(GetAnyTarget());
     }
     else
     {
         _owner->RemoveFlag(UNIT_FIELD_FLAGS, UNIT_FLAG_IN_COMBAT);
         _owner->AtExitCombat();
+        if (_owner->IsEngaged() && !(_owner->ToCreature() && _owner->ToCreature()->IsAIEnabled()))
+            _owner->AtDisengage();
     }
 
     if (Unit* master = _owner->GetCharmerOrOwner())
diff --git a/src/server/game/Combat/ThreatManager.cpp b/src/server/game/Combat/ThreatManager.cpp
index ce5a2ac58d3..83c86f59e1a 100644
--- a/src/server/game/Combat/ThreatManager.cpp
+++ b/src/server/game/Combat/ThreatManager.cpp
@@ -166,7 +166,7 @@ void ThreatReference::UnregisterAndFree()
     return true;
 }
 
-ThreatManager::ThreatManager(Unit* owner) : _owner(owner), _ownerCanHaveThreatList(false), _ownerEngaged(false), _needClientUpdate(false), _updateTimer(THREAT_UPDATE_INTERVAL), _currentVictimRef(nullptr), _fixateRef(nullptr)
+ThreatManager::ThreatManager(Unit* owner) : _owner(owner), _ownerCanHaveThreatList(false), _needClientUpdate(false), _updateTimer(THREAT_UPDATE_INTERVAL), _currentVictimRef(nullptr), _fixateRef(nullptr)
 {
     for (int8 i = 0; i < MAX_SPELL_SCHOOL; ++i)
         _singleSchoolModifiers[i] = 1.0f;
@@ -186,7 +186,7 @@ void ThreatManager::Initialize()
 
 void ThreatManager::Update(uint32 tdiff)
 {
-    if (!CanHaveThreatList() || !IsEngaged())
+    if (!CanHaveThreatList() || IsThreatListEmpty())
         return;
     if (_updateTimer <= tdiff)
     {
@@ -292,13 +292,6 @@ void ThreatManager::EvaluateSuppressed(bool canExpire)
     }
 }
 
-static void SaveCreatureHomePositionIfNeed(Creature* c)
-{
-    MovementGeneratorType const movetype = c->GetMotionMaster()->GetCurrentMovementGeneratorType();
-    if (movetype == WAYPOINT_MOTION_TYPE || movetype == POINT_MOTION_TYPE || (c->IsAIEnabled() && c->AI()->IsEscorted()))
-        c->SetHomePosition(c->GetPosition());
-}
-
 void ThreatManager::AddThreat(Unit* target, float amount, SpellInfo const* spell, bool ignoreModifiers, bool ignoreRedirects)
 {
     // step 1: we can shortcut if the spell has one of the NO_THREAT attrs set - nothing will happen
@@ -398,18 +391,10 @@ void ThreatManager::AddThreat(Unit* target, float amount, SpellInfo const* spell
     if (ref->IsOnline()) // ...and if the ref is online it also gets the threat it should have
         ref->AddThreat(amount);
 
-    if (!_ownerEngaged)
+    if (!_owner->IsEngaged())
     {
-        Creature* cOwner = ASSERT_NOTNULL(_owner->ToCreature()); // if we got here the owner can have a threat list, and must be a creature!
-        _ownerEngaged = true;
-
+        _owner->AtEngage(target);
         UpdateVictim();
-
-        SaveCreatureHomePositionIfNeed(cOwner);
-        if (CreatureAI* ownerAI = cOwner->AI())
-            ownerAI->JustEngagedWith(target);
-        if (CreatureGroup* formation = cOwner->GetFormation())
-            formation->MemberEngagingTarget(cOwner, target);
     }
 }
 
@@ -486,14 +471,17 @@ void ThreatManager::ClearThreat(ThreatReference* ref)
 
 void ThreatManager::ClearAllThreat()
 {
-    _ownerEngaged = false;
-    if (_myThreatListEntries.empty())
-        return;
-
-    SendClearAllThreatToClients();
-    do
-        _myThreatListEntries.begin()->second->UnregisterAndFree();
-    while (!_myThreatListEntries.empty());
+    if (!_myThreatListEntries.empty())
+    {
+        SendClearAllThreatToClients();
+        do
+            _myThreatListEntries.begin()->second->UnregisterAndFree();
+        while (!_myThreatListEntries.empty());
+    }
+    // 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)
diff --git a/src/server/game/Combat/ThreatManager.h b/src/server/game/Combat/ThreatManager.h
index cf49f149506..12beb54c486 100644
--- a/src/server/game/Combat/ThreatManager.h
+++ b/src/server/game/Combat/ThreatManager.h
@@ -43,11 +43,6 @@ class SpellInfo;
  *  - Both owner and victim are valid units, which are currently in the world. Neither can be nullptr.                                                  *
  *  - There is an active combat reference between owner and victim.                                                                                     *
  *                                                                                                                                                      *
- * ThreatManager also keeps track of whether its owner is engaged (a boolean flag).                                                                     *
- *  - If a (non-offline) threat list entry is added to a not-yet-engaged ThreatManager, it calls JustEngagedWith on its owner's AI.                     *
- *  - The engaged state is cleared in ClearAllThreat (which is invoked on evade).                                                                       *
- *  - This flag can be accessed through the IsEngaged method. For creatures that can have a threat list, this is equal to Unit::IsEngaged.              *
- *                                                                                                                                                      *
  * Note that (threat => combat) is a strong guarantee provided in conjunction with CombatManager. Thus:                                                 *
  *  - Adding threat will also create a combat reference between the units if one doesn't exist yet (even if the owner can't have a threat list!)        *
  *  - Ending combat between two units will also delete any threat references that may exist between them.                                               *
@@ -112,7 +107,6 @@ class TC_GAME_API ThreatManager
         // returns an arbitrary non-offline victim from owner's threat list if one exists, nullptr otherwise
         Unit* GetAnyTarget() const;
 
-        bool IsEngaged() const { return _ownerEngaged; }
         // are there any entries in owner's threat list?
         bool IsThreatListEmpty(bool includeOffline = false) const;
         // is there a threat list entry on owner's threat list with victim == who?
@@ -188,7 +182,6 @@ class TC_GAME_API ThreatManager
     private:
         Unit* const _owner;
         bool _ownerCanHaveThreatList;
-        bool _ownerEngaged;
 
         static const CompareThreatLessThan CompareThreat;
         static bool CompareReferencesLT(ThreatReference const* a, ThreatReference const* b, float aWeight);
diff --git a/src/server/game/Entities/Creature/Creature.cpp b/src/server/game/Entities/Creature/Creature.cpp
index 27b792afd7e..7585c37db81 100644
--- a/src/server/game/Entities/Creature/Creature.cpp
+++ b/src/server/game/Entities/Creature/Creature.cpp
@@ -3205,9 +3205,9 @@ bool Creature::CanGiveExperience() const
         && !(GetCreatureTemplate()->flags_extra & CREATURE_FLAG_EXTRA_NO_XP_AT_KILL);
 }
 
-void Creature::AtEnterCombat()
+void Creature::AtEngage(Unit* target)
 {
-    Unit::AtEnterCombat();
+    Unit::AtEngage(target);
 
     if (!(GetCreatureTemplate()->type_flags & CREATURE_TYPE_FLAG_MOUNTED_COMBAT_ALLOWED))
         Dismount();
@@ -3218,11 +3218,20 @@ void Creature::AtEnterCombat()
         UpdateSpeed(MOVE_SWIM);
         UpdateSpeed(MOVE_FLIGHT);
     }
+
+    MovementGeneratorType const movetype = GetMotionMaster()->GetCurrentMovementGeneratorType();
+    if (movetype == WAYPOINT_MOTION_TYPE || movetype == POINT_MOTION_TYPE || (IsAIEnabled() && AI()->IsEscorted()))
+        SetHomePosition(GetPosition());
+
+    if (CreatureAI* ai = AI())
+        ai->JustEngagedWith(target);
+    if (CreatureGroup* formation = GetFormation())
+        formation->MemberEngagingTarget(this, target);
 }
 
-void Creature::AtExitCombat()
+void Creature::AtDisengage()
 {
-    Unit::AtExitCombat();
+    Unit::AtDisengage();
 
     ClearUnitState(UNIT_STATE_ATTACK_PLAYER);
     if (HasFlag(UNIT_DYNAMIC_FLAGS, UNIT_DYNFLAG_TAPPED))
diff --git a/src/server/game/Entities/Creature/Creature.h b/src/server/game/Entities/Creature/Creature.h
index 189e7db0d9c..5246e2c6284 100644
--- a/src/server/game/Entities/Creature/Creature.h
+++ b/src/server/game/Entities/Creature/Creature.h
@@ -359,8 +359,8 @@ class TC_GAME_API Creature : public Unit, public GridObject<Creature>, public Ma
 
         bool CanGiveExperience() const;
 
-        void AtEnterCombat() override;
-        void AtExitCombat() override;
+        void AtEngage(Unit* target) override;
+        void AtDisengage() override;
 
         std::string GetDebugInfo() const override;
 
diff --git a/src/server/game/Entities/Unit/Unit.cpp b/src/server/game/Entities/Unit/Unit.cpp
index 5dc4e71572d..b06ef11d312 100644
--- a/src/server/game/Entities/Unit/Unit.cpp
+++ b/src/server/game/Entities/Unit/Unit.cpp
@@ -293,7 +293,7 @@ Unit::Unit(bool isWorldObject) :
     movespline(new Movement::MoveSpline()), m_ControlledByPlayer(false), m_AutoRepeatFirstCast(false),
     m_procDeep(0), m_transformSpell(0), m_removedAurasCount(0), m_charmer(nullptr), m_charmed(nullptr),
     i_motionMaster(new MotionMaster(this)), m_regenTimer(0), m_vehicle(nullptr), m_vehicleKit(nullptr),
-    m_unitTypeMask(UNIT_MASK_NONE), m_Diminishing(), m_combatManager(this), m_threatManager(this),
+    m_unitTypeMask(UNIT_MASK_NONE), m_Diminishing(), m_isEngaged(false), m_combatManager(this), m_threatManager(this),
     m_aiLocked(false), m_comboTarget(nullptr), m_comboPoints(0), m_spellHistory(new SpellHistory(this))
 {
     m_objectType |= TYPEMASK_UNIT;
diff --git a/src/server/game/Entities/Unit/Unit.h b/src/server/game/Entities/Unit/Unit.h
index 19e7ab5e11a..f8149a48d70 100644
--- a/src/server/game/Entities/Unit/Unit.h
+++ b/src/server/game/Entities/Unit/Unit.h
@@ -1023,10 +1023,10 @@ class TC_GAME_API Unit : public WorldObject
 
         /// ====================== THREAT & COMBAT ====================
         bool CanHaveThreatList() const { return m_threatManager.CanHaveThreatList(); }
-        // For NPCs with threat list: Whether there are any enemies on our threat list
-        // For other units: Whether we're in combat
-        // This value is different from IsInCombat when a projectile spell is midair (combat on launch - threat+aggro on impact)
-        bool IsEngaged() const { return CanHaveThreatList() ? m_threatManager.IsEngaged() : IsInCombat(); }
+        // This value can be different from IsInCombat:
+        // - when a projectile spell is midair against a creature (combat on launch - threat+aggro on impact)
+        // - when the creature has no targets left, but the AI has not yet ceased engaged logic
+        bool IsEngaged() const { return m_isEngaged; }
         bool IsEngagedBy(Unit const* who) const { return CanHaveThreatList() ? IsThreatenedBy(who) : IsInCombatWith(who); }
         void EngageWithTarget(Unit* who); // Adds target to threat list if applicable, otherwise just sets combat state
         // Combat handling
@@ -1766,6 +1766,9 @@ class TC_GAME_API Unit : public WorldObject
         virtual void AtEnterCombat() { }
         virtual void AtExitCombat();
 
+        virtual void AtEngage(Unit* /*target*/) { m_isEngaged = true; }
+        virtual void AtDisengage() { m_isEngaged = false; }
+
     private:
 
         void UpdateSplineMovement(uint32 t_diff);
@@ -1789,7 +1792,9 @@ class TC_GAME_API Unit : public WorldObject
         TimeTrackerSmall m_movesplineTimer;
 
         DiminishingReturn m_Diminishing[DIMINISHING_MAX];
-        // Manage all Units that are threatened by us
+
+        // Threat+combat management
+        bool m_isEngaged;
         friend class CombatManager;
         CombatManager m_combatManager;
         friend class ThreatManager;
diff --git a/src/server/scripts/Commands/cs_debug.cpp b/src/server/scripts/Commands/cs_debug.cpp
index 9e54c1986e7..ac1c9570e80 100644
--- a/src/server/scripts/Commands/cs_debug.cpp
+++ b/src/server/scripts/Commands/cs_debug.cpp
@@ -863,17 +863,17 @@ public:
         ThreatManager& mgr = target->GetThreatManager();
         if (!target->IsAlive())
         {
-            handler->PSendSysMessage("%s (guid %u) is not alive.", target->GetName().c_str(), target->GetGUID().GetCounter());
+            handler->PSendSysMessage("%s (GUID %u) is not alive.", target->GetName().c_str(), target->GetGUID().GetCounter());
             return true;
         }
 
         uint32 count = 0;
         auto const& threatenedByMe = target->GetThreatManager().GetThreatenedByMeList();
         if (threatenedByMe.empty())
-            handler->PSendSysMessage("%s (guid %u) does not threaten any units.", target->GetName().c_str(), target->GetGUID().GetCounter());
+            handler->PSendSysMessage("%s (GUID %u) does not threaten any units.", target->GetName().c_str(), target->GetGUID().GetCounter());
         else
         {
-            handler->PSendSysMessage("List of units threatened by %s (guid %u)", target->GetName().c_str(), target->GetGUID().GetCounter());
+            handler->PSendSysMessage("List of units threatened by %s (GUID %u)", target->GetName().c_str(), target->GetGUID().GetCounter());
             for (auto const& pair : threatenedByMe)
             {
                 Unit* unit = pair.second->GetOwner();
@@ -886,7 +886,7 @@ public:
         {
             if (!mgr.IsThreatListEmpty(true))
             {
-                if (mgr.IsEngaged())
+                if (target->IsEngaged())
                     handler->PSendSysMessage("Threat list of %s (GUID %u, SpawnID %u):", target->GetName().c_str(), target->GetGUID().GetCounter(), target->GetTypeId() == TYPEID_UNIT ? target->ToCreature()->GetSpawnId() : 0);
                 else
                     handler->PSendSysMessage("%s (GUID %u, SpawnID %u) is not engaged, but still has a threat list? Well, here it is:", target->GetName().c_str(), target->GetGUID().GetCounter(), target->GetTypeId() == TYPEID_UNIT ? target->ToCreature()->GetSpawnId() : 0);
@@ -927,13 +927,15 @@ public:
                 }
                 handler->SendSysMessage("End of threat list.");
             }
-            else if (!mgr.IsEngaged())
+            else if (!target->IsEngaged())
                 handler->PSendSysMessage("%s (GUID %u, SpawnID %u) is not currently engaged.", target->GetName().c_str(), target->GetGUID().GetCounter(), target->GetTypeId() == TYPEID_UNIT ? target->ToCreature()->GetSpawnId() : 0);
             else
                 handler->PSendSysMessage("%s (GUID %u, SpawnID %u) seems to be engaged, but does not have a threat list??", target->GetName().c_str(), target->GetGUID().GetCounter(), target->GetTypeId() == TYPEID_UNIT ? target->ToCreature()->GetSpawnId() : 0);
         }
+        else if (target->IsEngaged())
+            handler->PSendSysMessage("%s (GUID %u) is currently engaged. (This unit cannot have a threat list.)", target->GetName().c_str(), target->GetGUID().GetCounter());
         else
-            handler->PSendSysMessage("%s (GUID %u) cannot have a threat list.", target->GetName().c_str(), target->GetGUID().GetCounter());
+            handler->PSendSysMessage("%s (GUID %u) is not currently engaged. (This unit cannot have a threat list.)", target->GetName().c_str(), target->GetGUID().GetCounter());
         return true;
     }