Unit/AI: Move CharmedPlayerAI logic from the SetCharmedBy/RemoveCharmedBy hooks to Player::Update -> Unit::UpdateCharmAI to guarantee thread safety and prevent race condition crashes.

(cherry picked from commit 5354b585e3d8eb89b77b932986a3fbf3b53706c4)

2 files changed, 66 insertions, 39 deletions

diff --git a/src/server/game/Entities/Player/Player.cpp b/src/server/game/Entities/Player/Player.cpp
index 0d4c8bcda94..43ec01e953e 100644
--- a/src/server/game/Entities/Player/Player.cpp
+++ b/src/server/game/Entities/Player/Player.cpp
@@ -1066,6 +1066,12 @@ void Player::Update(uint32 p_time)
 
     if (IsAIEnabled && GetAI())
         GetAI()->UpdateAI(p_time);
+    else if (NeedChangeAI)
+    {
+        UpdateCharmAI();
+        NeedChangeAI = false;
+        IsAIEnabled = !!GetAI();
+    }
 
     // Update items that have just a limited lifetime
     if (now > m_Last_tick)
diff --git a/src/server/game/Entities/Unit/Unit.cpp b/src/server/game/Entities/Unit/Unit.cpp
index ae0b55352a6..d983e307f10 100644
--- a/src/server/game/Entities/Unit/Unit.cpp
+++ b/src/server/game/Entities/Unit/Unit.cpp
@@ -7552,7 +7552,7 @@ Unit* Unit::GetCharmer() const
     if (!charmerGUID.IsEmpty())
         return ObjectAccessor::GetUnit(*this, charmerGUID);
 
-    return NULL;
+    return nullptr;
 }
 
 Player* Unit::GetCharmerOrOwnerPlayerOrPlayerItself() const
@@ -11806,28 +11806,63 @@ void Unit::CleanupsBeforeDelete(bool finalCleanup)
 
 void Unit::UpdateCharmAI()
 {
-    if (GetTypeId() == TYPEID_PLAYER)
-        return;
-
-    if (i_disabledAI) // disabled AI must be primary AI
+    switch (GetTypeId())
     {
-        if (!IsCharmed())
-        {
-            delete i_AI;
-            i_AI = i_disabledAI;
-            i_disabledAI = NULL;
-        }
-    }
-    else
-    {
-        if (IsCharmed())
+        case TYPEID_UNIT:
+            if (i_disabledAI) // disabled AI must be primary AI
+            {
+                if (!IsCharmed())
+                {
+                    delete i_AI;
+                    i_AI = i_disabledAI;
+                    i_disabledAI = nullptr;
+                }
+            }
+            else
+            {
+                if (IsCharmed())
+                {
+                    i_disabledAI = i_AI;
+                    if (isPossessed() || IsVehicle())
+                        i_AI = new PossessedAI(ToCreature());
+                    else
+                        i_AI = new PetAI(ToCreature());
+                }
+            }
+            break;
+        case TYPEID_PLAYER:
         {
-            i_disabledAI = i_AI;
-            if (isPossessed() || IsVehicle())
-                i_AI = new PossessedAI(ToCreature());
+            if (Unit* charmer = GetCharmer()) // if we are currently being charmed, then we should apply charm AI
+            {
+                if (Creature* creatureCharmer = charmer->ToCreature()) // this should only ever happen for creature charmers
+                {
+                    i_disabledAI = i_AI;
+                    // first, we check if the creature's own AI specifies an override playerai for its owned players
+                    if (PlayerAI* charmAI = creatureCharmer->IsAIEnabled ? creatureCharmer->AI()->GetAIForCharmedPlayer(ToPlayer()) : nullptr)
+                        i_AI = charmAI;
+                    else // otherwise, we default to the generic one
+                        i_AI = new SimpleCharmedPlayerAI(ToPlayer());
+                }
+                else
+                {
+                    TC_LOG_ERROR("misc", "Attempt to assign charm AI to player %s who is charmed by non-creature %s.", GetGUID().ToString().c_str(), charmer->GetGUID().ToString().c_str());
+                }
+            }
             else
-                i_AI = new PetAI(ToCreature());
+            {
+                // we allow the charmed PlayerAI to clean up
+                i_AI->OnCharmed(false);
+                // then delete it
+                delete i_AI;
+                // and restore our previous PlayerAI (if we had one)
+                i_AI = i_disabledAI;
+                i_disabledAI = nullptr;
+                // IsAIEnabled gets handled in the caller
+            }
+            break;
         }
+        default:
+            TC_LOG_ERROR("misc", "Attempt to update charm AI for unit %s, which is neither player nor creature.", GetGUID().ToString().c_str());
     }
 }
 
@@ -14106,15 +14141,11 @@ bool Unit::SetCharmedBy(Unit* charmer, CharmType type, AuraApplication const* au
         if (player->isAFK())
             player->ToggleAFK();
 
-        if (Creature* creatureCharmer = charmer->ToCreature()) // we are charmed by a creature
+        if (charmer->GetTypeId() == TYPEID_UNIT) // we are charmed by a creature
         {
-            IsAIEnabled = true;
-            i_disabledAI = i_AI;
-            // set our AI to the charmer's custom charm AI if applicable
-            if (PlayerAI* charmAI = creatureCharmer->AI()->GetAIForCharmedPlayer(player))
-                i_AI = charmAI;
-            else // otherwise use the default charmed player AI
-                i_AI = new SimpleCharmedPlayerAI(player);
+            // change AI to charmed AI on next Update tick
+            NeedChangeAI = true;
+            IsAIEnabled = false;
         }
         player->SetClientControl(this, false);
     }
@@ -14276,18 +14307,8 @@ void Unit::RemoveCharmedBy(Unit* charmer)
     {
         if (charmer->GetTypeId() == TYPEID_UNIT) // charmed by a creature, this means we had PlayerAI
         {
-            if (i_AI)
-            {
-                // allow charmed player AI to clean up
-                i_AI->OnCharmed(false);
-                // then delete it
-                delete i_AI;
-                // and restore our previous playerAI (if we had one)
-                if ((i_AI = i_disabledAI))
-                    i_disabledAI = nullptr;
-                else
-                    IsAIEnabled = false;
-            }
+            NeedChangeAI = true;
+            IsAIEnabled = false;
         }
         player->SetClientControl(this, true);
     }