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.

(cherry picked from commit dbe3bbefe734f8824e8f07c302e256f0d446b974)

8 files changed, 50 insertions, 49 deletions

diff --git a/src/server/game/Combat/CombatManager.cpp b/src/server/game/Combat/CombatManager.cpp
index 68233d45b64..67ebb233fa4 100644
--- a/src/server/game/Combat/CombatManager.cpp
+++ b/src/server/game/Combat/CombatManager.cpp
@@ -336,11 +336,15 @@ bool CombatManager::UpdateOwnerCombatState() const
     {
         _owner->AddUnitFlag(UNIT_FLAG_IN_COMBAT);
         _owner->AtEnterCombat();
+        if (!_owner->CanHaveThreatList() && !_owner->IsEngaged())
+            _owner->AtEngage(GetAnyTarget());
     }
     else
     {
         _owner->RemoveUnitFlag(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 d6d6378373e..b52a67bb95b 100644
--- a/src/server/game/Combat/ThreatManager.cpp
+++ b/src/server/game/Combat/ThreatManager.cpp
@@ -164,7 +164,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;
@@ -184,7 +184,7 @@ void ThreatManager::Initialize()
 
 void ThreatManager::Update(uint32 tdiff)
 {
-    if (!CanHaveThreatList() || !IsEngaged())
+    if (!CanHaveThreatList() || IsThreatListEmpty())
         return;
     if (_updateTimer <= tdiff)
     {
@@ -290,13 +290,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
@@ -406,18 +399,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);
     }
 }
 
@@ -494,14 +479,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 1060e41fe50..7d7fda27e90 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 bcda4c972f1..2e260347a0a 100644
--- a/src/server/game/Entities/Creature/Creature.cpp
+++ b/src/server/game/Entities/Creature/Creature.cpp
@@ -3368,9 +3368,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();
@@ -3381,11 +3381,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 (HasDynamicFlag(UNIT_DYNFLAG_TAPPED))
diff --git a/src/server/game/Entities/Creature/Creature.h b/src/server/game/Entities/Creature/Creature.h
index 51f3252796c..b4d2228bb2e 100644
--- a/src/server/game/Entities/Creature/Creature.h
+++ b/src/server/game/Entities/Creature/Creature.h
@@ -367,8 +367,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 982341baedc..4a7bee546fa 100644
--- a/src/server/game/Entities/Unit/Unit.cpp
+++ b/src/server/game/Entities/Unit/Unit.cpp
@@ -304,7 +304,7 @@ Unit::Unit(bool isWorldObject) :
     m_interruptMask(SpellAuraInterruptFlags::None), m_interruptMask2(SpellAuraInterruptFlags2::None),
     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_vehicleKit(nullptr), m_unitTypeMask(UNIT_MASK_NONE), m_Diminishing(), m_isEngaged(false), m_combatManager(this),
     m_threatManager(this), m_aiLocked(false), _playHoverAnim(false), _aiAnimKitId(0), _movementAnimKitId(0), _meleeAnimKitId(0),
     _spellHistory(new SpellHistory(this))
 {
diff --git a/src/server/game/Entities/Unit/Unit.h b/src/server/game/Entities/Unit/Unit.h
index 1229461f673..4ad89f1ce6e 100644
--- a/src/server/game/Entities/Unit/Unit.h
+++ b/src/server/game/Entities/Unit/Unit.h
@@ -1098,10 +1098,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
@@ -1946,6 +1946,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);
@@ -1971,7 +1974,9 @@ class TC_GAME_API Unit : public WorldObject
         TimeTrackerSmall m_movesplineTimer;
 
         Diminishing m_Diminishing;
-        // 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 ab1aec347ba..3bba570eb33 100644
--- a/src/server/scripts/Commands/cs_debug.cpp
+++ b/src/server/scripts/Commands/cs_debug.cpp
@@ -891,7 +891,7 @@ public:
         {
             if (!mgr.IsThreatListEmpty(true))
             {
-                if (mgr.IsEngaged())
+                if (target->IsEngaged())
                     handler->PSendSysMessage("Threat list of %s (%s, SpawnID " UI64FMTD "):", target->GetName().c_str(), target->GetGUID().ToString().c_str(), target->GetTypeId() == TYPEID_UNIT ? target->ToCreature()->GetSpawnId() : 0);
                 else
                     handler->PSendSysMessage("%s (%s, SpawnID " UI64FMTD ") is not engaged, but still has a threat list? Well, here it is:", target->GetName().c_str(), target->GetGUID().ToString().c_str(), target->GetTypeId() == TYPEID_UNIT ? target->ToCreature()->GetSpawnId() : 0);
@@ -932,13 +932,15 @@ public:
                 }
                 handler->SendSysMessage("End of threat list.");
             }
-            else if (!mgr.IsEngaged())
+            else if (!target->IsEngaged())
                 handler->PSendSysMessage("%s (%s, SpawnID " UI64FMTD ") is not currently engaged.", target->GetName().c_str(), target->GetGUID().ToString().c_str(), target->GetTypeId() == TYPEID_UNIT ? target->ToCreature()->GetSpawnId() : 0);
             else
                 handler->PSendSysMessage("%s (%s, SpawnID " UI64FMTD ") seems to be engaged, but does not have a threat list??", target->GetName().c_str(), target->GetGUID().ToString().c_str(), target->GetTypeId() == TYPEID_UNIT ? target->ToCreature()->GetSpawnId() : 0);
         }
+        else if (target->IsEngaged())
+            handler->PSendSysMessage("%s (%s) is currently engaged. (This unit cannot have a threat list.)", target->GetName().c_str(), target->GetGUID().ToString().c_str());
         else
-            handler->PSendSysMessage("%s (%s) cannot have a threat list.", target->GetName().c_str(), target->GetGUID().ToString().c_str());
+            handler->PSendSysMessage("%s (%s) is not currently engaged. (This unit cannot have a threat list.)", target->GetName().c_str(), target->GetGUID().ToString().c_str());
         return true;
     }