fix(Core/AI): more AI factory checks (#12402)

Co-authored-by: ariel- <ariel-@users.noreply.github.com>
Co-authored-by: Treeston <treeston@users.noreply.github.com>
Co-authored-by: Aokromes <aokromes@users.noreply.github.com>

data/sql/updates/pending_db_world/rev_1657988086857948400.sql

@@ -0,0 +1,7 @@
+--
+UPDATE `creature` SET `MovementType` = 1, `wander_distance` = 5 WHERE `guid` IN (51879,51914);
+UPDATE `creature` SET `MovementType` = 0 WHERE `guid` = 132313;
+
+UPDATE `creature_template` SET `AIName` = 'NullCreatureAI' WHERE `AIName` IN ('NullCreatureAi', 'NullAI');
+UPDATE `creature_template` SET `AIName` = 'AggressorAI' WHERE `AIName` = 'AgressorAI';
+UPDATE `creature_template` SET `AIName` = '' WHERE `AIName` = 'OutdoorPvPObjectiveAI';

src/common/Dynamic/ObjectRegistry.h

@@ -65,6 +65,12 @@ public:
         return true;
     }
 
+    /// Returns true if registry contains an item
+    bool HasItem(Key const& key) const
+    {
+        return (_registeredObjects.count(key) > 0);
+    }
+
     /// Return the map of registered items
     RegistryMapType const& GetRegisteredItems() const
     {

src/common/Utilities/Timer.h

@@ -231,7 +231,7 @@ private:
 struct TimeTrackerSmall
 {
 public:
-    TimeTrackerSmall(uint32 expiry = 0)
+    TimeTrackerSmall(int32 expiry = 0)
         : i_expiryTime(expiry)
     {
     }
@@ -246,7 +246,7 @@ public:
         return i_expiryTime <= 0;
     }
 
-    void Reset(uint32 interval)
+    void Reset(int32 interval)
     {
         i_expiryTime = interval;
     }

src/server/game/AI/CoreAI/CombatAI.cpp

@@ -30,7 +30,7 @@ int32 AggressorAI::Permissible(Creature const* creature)
 {
     // have some hostile factions, it will be selected by IsHostileTo check at MoveInLineOfSight
     if (!creature->IsCivilian() && !creature->IsNeutralToAll())
-        return PERMIT_BASE_PROACTIVE;
+        return PERMIT_BASE_REACTIVE;
 
     return PERMIT_BASE_NO;
 }

src/server/game/AI/CoreAI/PetAI.cpp

@@ -30,9 +30,6 @@
 
 int32 PetAI::Permissible(Creature const* creature)
 {
-    if (creature->IsPet())
-        return PERMIT_BASE_SPECIAL;
-
     if (creature->HasUnitTypeMask(UNIT_MASK_CONTROLABLE_GUARDIAN))
     {
         if (reinterpret_cast<Guardian const*>(creature)->GetOwner()->GetTypeId() == TYPEID_PLAYER)

src/server/game/AI/CreatureAISelector.cpp

@@ -28,6 +28,13 @@
 
 namespace FactorySelector
 {
+    template <class T, class Value>
+    inline int32 GetPermitFor(T const* obj, Value const& value)
+    {
+        Permissible<T> const* const p = ASSERT_NOTNULL(dynamic_cast<Permissible<T> const*>(value.second.get()));
+        return p->Permit(obj);
+    }
+
     template <class T>
     struct PermissibleOrderPred
     {
@@ -37,50 +44,57 @@ namespace FactorySelector
         template <class Value>
         bool operator()(Value const& left, Value const& right) const
         {
-            Permissible<T> const* leftPermit = ASSERT_NOTNULL(dynamic_cast<Permissible<T> const*>(left.second.get()));
+            return GetPermitFor(_obj, left) < GetPermitFor(_obj, right);
-            Permissible<T> const* rightPermit = ASSERT_NOTNULL(dynamic_cast<Permissible<T> const*>(right.second.get()));
-
-            return leftPermit->Permit(_obj) < rightPermit->Permit(_obj);
         }
 
     private:
         T const* const _obj;
     };
 
-    CreatureAI* SelectAI(Creature* creature)
+    template <class AI, class T>
+    inline FactoryHolder<AI, T> const* SelectFactory(T* obj)
     {
-        CreatureAICreator const* ai_factory = nullptr;
+        static_assert(std::is_same<AI, CreatureAI>::value || std::is_same<AI, GameObjectAI>::value, "Invalid template parameter");
+        static_assert(std::is_same<AI, CreatureAI>::value == std::is_same<T, Creature>::value, "Incompatible AI for type");
+        static_assert(std::is_same<AI, GameObjectAI>::value == std::is_same<T, GameObject>::value, "Incompatible AI for type");
 
-        // scriptname in db
+        using AIRegistry = typename FactoryHolder<AI, T>::FactoryHolderRegistry;
-        if (!ai_factory)
-            if (CreatureAI* scriptedAI = sScriptMgr->GetCreatureAI(creature))
-                return scriptedAI;
 
-        // AIname in db
+        // AIName in db
-        std::string const& aiName = creature->GetAIName();
+        std::string const& aiName = obj->GetAIName();
-        if (!ai_factory && !aiName.empty())
+        if (!aiName.empty())
-            ai_factory = sCreatureAIRegistry->GetRegistryItem(aiName);
+            return AIRegistry::instance()->GetRegistryItem(aiName);
 
         // select by permit check
-        if (!ai_factory)
+        typename AIRegistry::RegistryMapType const& items = AIRegistry::instance()->GetRegisteredItems();
-        {
+        auto itr = std::max_element(items.begin(), items.end(), PermissibleOrderPred<T>(obj));
-            CreatureAIRegistry::RegistryMapType const& items = sCreatureAIRegistry->GetRegisteredItems();
+        if (itr != items.end() && GetPermitFor(obj, *itr) >= 0)
-            auto itr = std::max_element(items.begin(), items.end(), PermissibleOrderPred<Creature>(creature));
+            return itr->second.get();
-            if (itr != items.end())
-                ai_factory = itr->second.get();
-        }
 
-        if (!ai_factory)
+        // should _never_ happen, Null AI types defined as PERMIT_BASE_IDLE, it must've been found
-            ai_factory = sCreatureAIRegistry->GetRegistryItem("NullCreatureAI");
+        ABORT();
+        return nullptr;
+    }
 
-        return ASSERT_NOTNULL(ai_factory)->Create(creature);
+    CreatureAI* SelectAI(Creature* creature)
+    {
+        // special pet case, if a tamed creature uses AIName (example SmartAI) we need to override it
+        if (creature->IsPet())
+            return ASSERT_NOTNULL(sCreatureAIRegistry->GetRegistryItem("PetAI"))->Create(creature);
+
+        // scriptname in db
+        if (CreatureAI* scriptedAI = sScriptMgr->GetCreatureAI(creature))
+            return scriptedAI;
+
+        return SelectFactory<CreatureAI>(creature)->Create(creature);
     }
 
     MovementGenerator* SelectMovementGenerator(Unit* unit)
     {
         MovementGeneratorType type = IDLE_MOTION_TYPE;
-        if (unit->GetTypeId() == TYPEID_UNIT)
+        if (Creature* creature = unit->ToCreature())
-            type = unit->ToCreature()->GetDefaultMovementType();
+            if (!creature->GetCharmerOrOwnerPlayerOrPlayerItself())
+                type = creature->GetDefaultMovementType();
 
         MovementGeneratorCreator const* mv_factory = sMovementGeneratorRegistry->GetRegistryItem(type);
         return ASSERT_NOTNULL(mv_factory)->Create(unit);
@@ -88,29 +102,10 @@ namespace FactorySelector
 
     GameObjectAI* SelectGameObjectAI(GameObject* go)
     {
-        GameObjectAICreator const* ai_factory = nullptr;
-
         // scriptname in db
         if (GameObjectAI* scriptedAI = sScriptMgr->GetGameObjectAI(go))
             return scriptedAI;
 
-        // AIname in db
+        return SelectFactory<GameObjectAI>(go)->Create(go);
-        std::string const& aiName = go->GetAIName();
-        if (!ai_factory && !aiName.empty())
-            ai_factory = sGameObjectAIRegistry->GetRegistryItem(aiName);
-
-        // select by permit check
-        if (!ai_factory)
-        {
-            GameObjectAIRegistry::RegistryMapType const& items = sGameObjectAIRegistry->GetRegisteredItems();
-            auto itr = std::max_element(items.begin(), items.end(), PermissibleOrderPred<GameObject>(go));
-            if (itr != items.end())
-                ai_factory = itr->second.get();
-        }
-
-        if (!ai_factory)
-            ai_factory = sGameObjectAIRegistry->GetRegistryItem("NullGameObjectAI");
-
-        return ASSERT_NOTNULL(ai_factory)->Create(go);
     }
 }

src/server/game/Entities/Player/Player.cpp

@@ -12561,8 +12561,13 @@ void Player::SetMover(Unit* target)
         LOG_INFO("misc", "Player::SetMover (B2) - {}, {}, {}, {}, {}, {}, {}, {}", target->GetGUID().ToString(), target->GetMapId(), target->GetInstanceId(), target->FindMap()->GetId(), target->IsInWorld() ? 1 : 0, target->IsDuringRemoveFromWorld() ? 1 : 0, (target->ToPlayer() && target->ToPlayer()->IsBeingTeleported() ? 1 : 0), target->isBeingLoaded() ? 1 : 0);
     }
     m_mover->m_movedByPlayer = nullptr;
+    if (m_mover->GetTypeId() == TYPEID_UNIT)
+        m_mover->GetMotionMaster()->Initialize();
+
     m_mover = target;
     m_mover->m_movedByPlayer = this;
+    if (m_mover->GetTypeId() == TYPEID_UNIT)
+        m_mover->GetMotionMaster()->Initialize();
 }
 
 uint32 Player::GetCorpseReclaimDelay(bool pvp) const

src/server/game/Globals/ObjectMgr.cpp

@@ -22,10 +22,12 @@
 #include "CharacterCache.h"
 #include "Chat.h"
 #include "Common.h"
+#include "CreatureAIFactory.h"
 #include "Config.h"
 #include "Containers.h"
 #include "DatabaseEnv.h"
 #include "DisableMgr.h"
+#include "GameObjectAIFactory.h"
 #include "GameEventMgr.h"
 #include "GameTime.h"
 #include "GossipDef.h"
@@ -979,6 +981,12 @@ void ObjectMgr::CheckCreatureTemplate(CreatureTemplate const* cInfo)
         ok = true;
     }
 
+    if (!cInfo->AIName.empty() && !sCreatureAIRegistry->HasItem(cInfo->AIName))
+    {
+        LOG_ERROR("sql.sql", "Creature (Entry: {}) has non-registered `AIName` '{}' set, removing", cInfo->Entry, cInfo->AIName);
+        const_cast<CreatureTemplate*>(cInfo)->AIName.clear();
+    }
+
     FactionTemplateEntry const* factionTemplate = sFactionTemplateStore.LookupEntry(cInfo->faction);
     if (!factionTemplate)
         LOG_ERROR("sql.sql", "Creature (Entry: {}) has non-existing faction template ({}).", cInfo->Entry, cInfo->faction);
@@ -2162,6 +2170,11 @@ void ObjectMgr::LoadCreatures()
                 LOG_ERROR("sql.sql", "Table `creature` have creature (SpawnId: {} Entries: {}, {}, {}) with a `creature_template`.`flags_extra` in one or more entries including CREATURE_FLAG_EXTRA_INSTANCE_BIND but creature are not in instance.",
                     spawnId, data.id1, data.id2, data.id3);
         }
+        if (data.movementType >= MAX_DB_MOTION_TYPE)
+        {
+            LOG_ERROR("sql.sql", "Table `creature` has creature (SpawnId: {} Entries: {}, {}, {}) with wrong movement generator type ({}), ignored and set to IDLE.", spawnId, data.id1, data.id2, data.id3, data.movementType);
+            data.movementType = IDLE_MOTION_TYPE;
+        }
         if (data.wander_distance < 0.0f)
         {
             LOG_ERROR("sql.sql", "Table `creature` have creature (SpawnId: {} Entries: {}, {}, {}) with `wander_distance`< 0, set to 0.", spawnId, data.id1, data.id2, data.id3);
@@ -7040,6 +7053,10 @@ void ObjectMgr::LoadGameObjectTemplate()
         got.IsForQuests = false;
 
         // Checks
+        if (!got.AIName.empty() && !sGameObjectAIRegistry->HasItem(got.AIName))
+        {
+            LOG_ERROR("sql.sql", "GameObject (Entry: {}) has non-registered `AIName` '{}' set, removing", got.entry, got.AIName);
+        }
 
         switch (got.type)
         {

src/server/game/World/World.cpp

@@ -1567,6 +1567,9 @@ void World::SetInitialWorldSettings()
     LOG_INFO("server.loading", "Initializing PlayerDump tables...");
     PlayerDump::InitializeTables();
 
+    ///- Initilize static helper structures
+    AIRegistry::Initialize();
+
     LOG_INFO("server.loading", "Loading SpellInfo store...");
     sSpellMgr->LoadSpellInfoStore();
 
@@ -2035,9 +2038,6 @@ void World::SetInitialWorldSettings()
 
     mail_expire_check_timer = GameTime::GetGameTime() + 6h;
 
-    ///- Initilize static helper structures
-    AIRegistry::Initialize();
-
     ///- Initialize MapMgr
     LOG_INFO("server.loading", "Starting Map System");
     LOG_INFO("server.loading", " ");