MSA Improvement Part 2 (#66) (#933)

Reduce iterations.

src/strategy/actions/CheckMountStateAction.cpp

@@ -12,8 +12,57 @@
 #include "ServerFacade.h"
 #include "SpellAuraEffects.h"
 
+MountData CollectMountData(const Player* bot)
+{
+    MountData data;
+    // Iterate once over the spell map. Finally, we won't torture your CPU as before.
+    for (auto& entry : bot->GetSpellMap())
+    {
+        uint32 spellId = entry.first;
+        SpellInfo const* spellInfo = sSpellMgr->GetSpellInfo(spellId);
+        if (!spellInfo || spellInfo->Effects[0].ApplyAuraName != SPELL_AURA_MOUNTED)
+            continue;
+
+        if (entry.second->State == PLAYERSPELL_REMOVED || !entry.second->Active || spellInfo->IsPassive())
+            continue;
+
+        int32 effect1 = spellInfo->Effects[1].BasePoints;
+        int32 effect2 = spellInfo->Effects[2].BasePoints;
+        int32 speed = std::max(effect1, effect2);
+
+        // Check for swift mount criteria.
+        // Note: if the aura isn't the swift one, then a high speed qualifies; otherwise, we check the alternate threshold.
+        if ((speed > 59 && spellInfo->Effects[1].ApplyAuraName != SPELL_AURA_MOD_INCREASE_MOUNTED_FLIGHT_SPEED) ||
+            (speed > 149 && spellInfo->Effects[1].ApplyAuraName == SPELL_AURA_MOD_INCREASE_MOUNTED_FLIGHT_SPEED))
+        {
+            data.swiftMount = true;
+        }
+
+        // Update max speed if appropriate.
+        if (speed > data.maxSpeed)
+        {
+            // In BG, clamp max speed to 99 later; here we just store the maximum found.
+            data.maxSpeed = speed;
+        }
+
+        // Determine index: flight if either effect has flight aura or specific mount ID.
+        uint32 index = (spellInfo->Effects[1].ApplyAuraName == SPELL_AURA_MOD_INCREASE_MOUNTED_FLIGHT_SPEED ||
+                        spellInfo->Effects[2].ApplyAuraName == SPELL_AURA_MOD_INCREASE_MOUNTED_FLIGHT_SPEED ||
+                        // Winged Steed of the Ebon Blade
+                        // This mount is meant to autoscale from a 150% flyer
+                        // up to a 280% as you train your flying skill up.
+                        // This incorrectly gets categorised as a ground mount, force this to flyer only.
+                        // TODO: Add other scaling mounts here if they have the same issue, or adjust above
+                        // checks so that they are all correctly detected.
+                        spellInfo->Id == 54729) ? 1 : 0;
+        data.allSpells[index][speed].push_back(spellId);
+    }
+    return data;
+}
+
 bool CheckMountStateAction::Execute(Event event)
 {
+    // Determine if there are no attackers
     bool noAttackers = !AI_VALUE2(bool, "combat", "self target") || !AI_VALUE(uint8, "attacker count");
     bool enemy = AI_VALUE(Unit*, "enemy player target");
     bool dps = AI_VALUE(Unit*, "dps target");
@@ -26,10 +75,11 @@ bool CheckMountStateAction::Execute(Event event)
         float dismountDistance = CalculateDismountDistance();
         float mountDistance = CalculateMountDistance();
 
+        // Cache combat reach and distance calculations
         float combatReach = bot->GetCombatReach() + currentTarget->GetCombatReach();
         float distanceToTarget = bot->GetExactDist(currentTarget);
-        shouldDismount = distanceToTarget <= dismountDistance + combatReach;
+        shouldDismount = (distanceToTarget <= dismountDistance + combatReach);
-        shouldMount = distanceToTarget > mountDistance + combatReach;
+        shouldMount = (distanceToTarget > mountDistance + combatReach);
     }
     else
     {
@@ -44,13 +94,15 @@ bool CheckMountStateAction::Execute(Event event)
 
     // If there is a master and bot not in BG
     Player* master = GetMaster();
-    if (master && !bot->InBattleground())
+    bool inBattleground = bot->InBattleground();
+    if (master && !inBattleground)
     {
-        masterInShapeshiftForm = master->GetShapeshiftForm();
+        Group* group = bot->GetGroup();
-
+        if (!group || group->GetLeaderGUID() != master->GetGUID())
-        if (!bot->GetGroup() || bot->GetGroup()->GetLeaderGUID() != master->GetGUID())
             return false;
 
+        masterInShapeshiftForm = master->GetShapeshiftForm();
+
         if (ShouldFollowMasterMountState(master, noAttackers, shouldMount))
             return Mount();
 
@@ -64,29 +116,27 @@ bool CheckMountStateAction::Execute(Event event)
     }
 
     // If there is no master and bot not in BG
-    if (!master && !bot->InBattleground())
+    if (!master && !inBattleground)
     {
         if (!bot->IsMounted() && noAttackers && shouldMount && !bot->IsInCombat())
-        {
             return Mount();
     }
-    }
 
     // If the bot is in BG
-    if (bot->InBattleground() && shouldMount && noAttackers && !bot->IsInCombat() && !bot->IsMounted())
+    if (inBattleground && shouldMount && noAttackers && !bot->IsInCombat() && !bot->IsMounted())
     {
         // WSG Specific - Do not mount when carrying the flag
         if (bot->GetBattlegroundTypeId() == BATTLEGROUND_WS)
         {
             if (bot->HasAura(23333) || bot->HasAura(23335))
-            {
                 return false;
         }
-        }
+
         return Mount();
     }
 
-    if (!bot->IsFlying() && shouldDismount && bot->IsMounted() && (enemy || dps || (!noAttackers && bot->IsInCombat())))
+    if (!bot->IsFlying() && shouldDismount && bot->IsMounted() &&
+        (enemy || dps || (!noAttackers && bot->IsInCombat())))
     {
         Dismount();
         return true;
@@ -97,13 +147,16 @@ bool CheckMountStateAction::Execute(Event event)
 
 bool CheckMountStateAction::isUseful()
 {
-    if (botAI->IsInVehicle() || bot->isDead() || bot->HasUnitState(UNIT_STATE_IN_FLIGHT) || !bot->IsOutdoors() || bot->InArena())
+    if (botAI->IsInVehicle() || bot->isDead() || bot->HasUnitState(UNIT_STATE_IN_FLIGHT) ||
+        !bot->IsOutdoors() || bot->InArena())
         return false;
 
     // In addition to checking IsOutdoors, also check whether bot is clipping below floor slightly because that will
     // cause bot to falsly indicate they are outdoors. This fixes bug where bot tries to mount indoors (which seems
     // to mostly be an issue in tunnels of WSG and AV)
-    if (!bot->IsMounted() && bot->GetPositionZ() < bot->GetMapWaterOrGroundLevel(bot->GetPositionX(), bot->GetPositionY(), bot->GetPositionZ()))
+    float posZ = bot->GetPositionZ();
+    float groundLevel = bot->GetMapWaterOrGroundLevel(bot->GetPositionX(), bot->GetPositionY(), posZ);
+    if (!bot->IsMounted() && posZ < groundLevel)
         return false;
 
     if (!GET_PLAYERBOT_AI(bot)->HasStrategy("mount", BOT_STATE_NON_COMBAT) && !bot->IsMounted())
@@ -137,26 +190,27 @@ bool CheckMountStateAction::Mount()
     if (bot->isMoving())
         bot->StopMoving();
 
+    // If there is a master, we'll need to clear a few things before mounting.
     Player* master = GetMaster();
     botAI->RemoveShapeshift();
     botAI->RemoveAura("tree of life");
 
     int32 masterSpeed = CalculateMasterMountSpeed(master);
-    bool hasSwiftMount = CheckForSwiftMount();
+    MountData mountData = CollectMountData(bot);  // One pass for all mount info.
-
+    bool hasSwiftMount = mountData.swiftMount;
-    auto allSpells = GetAllMountSpells();
-    int32 masterMountType = GetMountType(master);
 
     if (TryPreferredMount(master))
         return true;
 
-    auto& spells = allSpells[masterMountType];
+    // Cache the spells for the mount type using our freshly collected data.
-    if (hasSwiftMount)
+    int32 masterMountType = GetMountType(master);
-        FilterMountsBySpeed(spells, masterSpeed);
+    auto spellsIt = mountData.allSpells.find(masterMountType);
-
+    if (spellsIt != mountData.allSpells.end())
-    // No preferred mount found (or invalid), continue with random mount selection
+    {
-    if (TryRandomMount(spells))
+        auto& spells = spellsIt->second;
+        if (TryRandomMountFiltered(spells, masterSpeed))
             return true;
+    }
 
     std::vector<Item*> items = AI_VALUE2(std::vector<Item*>, "inventory items", "mount");
     if (!items.empty())
@@ -167,8 +221,8 @@ bool CheckMountStateAction::Mount()
 
 float CheckMountStateAction::CalculateDismountDistance() const
 {
-    // Warrior bots should dismount far enough to charge (because its important for generating some initial rage),
+    // Warrior bots should dismount far enough to charge (because it's important for generating some initial rage),
-    // a real player would be riding toward enemy mashing the charge key but the bots wont cast charge while mounted
+    // a real player would be riding toward enemy mashing the charge key but the bots won't cast charge while mounted.
     bool isMelee = PlayerbotAI::IsMelee(bot);
     float dismountDistance = isMelee ? sPlayerbotAIConfig->meleeDistance + 2.0f : sPlayerbotAIConfig->spellDistance + 2.0f;
     return bot->getClass() == CLASS_WARRIOR ? std::max(18.0f, dismountDistance) : dismountDistance;
@@ -181,7 +235,8 @@ float CheckMountStateAction::CalculateMountDistance() const
     // seconds:
     // 21 / 7  =  21 / 14 + 1.5  =  3   (7 = dismounted speed  14 = epic-mount speed  1.5 = mount-spell cast time)
     bool isMelee = PlayerbotAI::IsMelee(bot);
-    return std::max(21.0f, isMelee ? sPlayerbotAIConfig->meleeDistance + 10.0f : sPlayerbotAIConfig->spellDistance + 10.0f);
+    float baseDistance = isMelee ? sPlayerbotAIConfig->meleeDistance + 10.0f : sPlayerbotAIConfig->spellDistance + 10.0f;
+    return std::max(21.0f, baseDistance);
 }
 
 void CheckMountStateAction::Dismount()
@@ -192,113 +247,60 @@ void CheckMountStateAction::Dismount()
 
 bool CheckMountStateAction::ShouldFollowMasterMountState(Player* master, bool noAttackers, bool shouldMount) const
 {
-    bool isMasterMounted = master->IsMounted() || masterInShapeshiftForm == FORM_FLIGHT || masterInShapeshiftForm == FORM_FLIGHT_EPIC || masterInShapeshiftForm == FORM_TRAVEL;
+    bool isMasterMounted = master->IsMounted() || (masterInShapeshiftForm == FORM_FLIGHT ||
-    return isMasterMounted && !bot->IsMounted() && noAttackers && shouldMount && !bot->IsInCombat() && botAI->GetState() != BOT_STATE_COMBAT;
+                                                    masterInShapeshiftForm == FORM_FLIGHT_EPIC ||
+                                                    masterInShapeshiftForm == FORM_TRAVEL);
+    return isMasterMounted && !bot->IsMounted() && noAttackers &&
+           shouldMount && !bot->IsInCombat() && botAI->GetState() != BOT_STATE_COMBAT;
 }
 
 bool CheckMountStateAction::ShouldDismountForMaster(Player* master) const
 {
-    bool isMasterMounted = master->IsMounted() || masterInShapeshiftForm == FORM_FLIGHT || masterInShapeshiftForm == FORM_FLIGHT_EPIC || masterInShapeshiftForm == FORM_TRAVEL;
+    bool isMasterMounted = master->IsMounted() || (masterInShapeshiftForm == FORM_FLIGHT ||
+                                                    masterInShapeshiftForm == FORM_FLIGHT_EPIC ||
+                                                    masterInShapeshiftForm == FORM_TRAVEL);
     return !isMasterMounted && bot->IsMounted();
 }
 
 int32 CheckMountStateAction::CalculateMasterMountSpeed(Player* master) const
 {
-    if (bot->GetPureSkillValue(SKILL_RIDING) <= 75 && bot->GetLevel() < sPlayerbotAIConfig->useFastGroundMountAtMinLevel)
+    int32 ridingSkill = bot->GetPureSkillValue(SKILL_RIDING);
+    int32 botLevel = bot->GetLevel();
+
+    // Check riding skill and level requirements
+    if (ridingSkill <= 75 && botLevel < static_cast<int32>(sPlayerbotAIConfig->useFastGroundMountAtMinLevel))
         return 59;
 
-    // If there ia a master and bot not in BG
+    // If there is a master and bot not in BG, use master's aura effects.
-    if (master != nullptr && !bot->InBattleground())
+    if (master && !bot->InBattleground())
     {
         auto auraEffects = master->GetAuraEffectsByType(SPELL_AURA_MOUNTED);
         if (!auraEffects.empty())
         {
             SpellInfo const* masterSpell = auraEffects.front()->GetSpellInfo();
-            return std::max(masterSpell->Effects[1].BasePoints, masterSpell->Effects[2].BasePoints);
+            int32 effect1 = masterSpell->Effects[1].BasePoints;
+            int32 effect2 = masterSpell->Effects[2].BasePoints;
+            return std::max(effect1, effect2);
         }
-
         else if (masterInShapeshiftForm == FORM_FLIGHT_EPIC)
             return 279;
-
         else if (masterInShapeshiftForm == FORM_FLIGHT)
             return 149;
     }
     else
-    // Bots on their own
     {
-        for (PlayerSpellMap::iterator itr = bot->GetSpellMap().begin(); itr != bot->GetSpellMap().end(); ++itr)
+        // Bots on their own: use our cached mount data to avoid iterating again.
-        {
+        MountData data = CollectMountData(bot);
-            uint32 spellId = itr->first;
+        int32 speed = data.maxSpeed;
-            SpellInfo const* spellInfo = sSpellMgr->GetSpellInfo(spellId);
+        // Ensure max speed of 99 in BG if necessary.
-            if (!spellInfo || spellInfo->Effects[0].ApplyAuraName != SPELL_AURA_MOUNTED)
+        if (bot->InBattleground() && speed > 99)
-                continue;
+            return 99;
-
-            if (itr->second->State == PLAYERSPELL_REMOVED || !itr->second->Active || spellInfo->IsPassive())
-                continue;
-
-            int32 speed = std::max(spellInfo->Effects[1].BasePoints, spellInfo->Effects[2].BasePoints);
-
-            if (speed > 59)
-            {
-                // Ensure max speed of 99 in BG
-                if (bot->InBattleground())
-                    return (speed > 99) ? 99 : speed;
-
         return speed;
     }
-        }
-    }
 
     return 59;
 }
 
-bool CheckMountStateAction::CheckForSwiftMount() const
-{
-    for (auto& [spellId, spellState] : bot->GetSpellMap())
-    {
-        SpellInfo const* spellInfo = sSpellMgr->GetSpellInfo(spellId);
-        if (!spellInfo || spellInfo->Effects[0].ApplyAuraName != SPELL_AURA_MOUNTED)
-            continue;
-
-        if (spellState->State == PLAYERSPELL_REMOVED || !spellState->Active || spellInfo->IsPassive())
-            continue;
-
-        int32 effect = std::max(spellInfo->Effects[1].BasePoints, spellInfo->Effects[2].BasePoints);
-        if ((effect > 59 && spellInfo->Effects[1].ApplyAuraName != SPELL_AURA_MOD_INCREASE_MOUNTED_FLIGHT_SPEED) ||
-            (effect > 149 && spellInfo->Effects[1].ApplyAuraName == SPELL_AURA_MOD_INCREASE_MOUNTED_FLIGHT_SPEED))
-            return true;
-    }
-    return false;
-}
-
-std::map<uint32, std::map<int32, std::vector<uint32>>> CheckMountStateAction::GetAllMountSpells() const
-{
-    std::map<uint32, std::map<int32, std::vector<uint32>>> allSpells;
-    for (auto& [spellId, spellState] : bot->GetSpellMap())
-    {
-        SpellInfo const* spellInfo = sSpellMgr->GetSpellInfo(spellId);
-        if (!spellInfo || spellInfo->Effects[0].ApplyAuraName != SPELL_AURA_MOUNTED)
-            continue;
-
-        if (spellState->State == PLAYERSPELL_REMOVED || !spellState->Active || spellInfo->IsPassive())
-            continue;
-
-        int32 effect = std::max(spellInfo->Effects[1].BasePoints, spellInfo->Effects[2].BasePoints);
-        uint32 index = (spellInfo->Effects[1].ApplyAuraName == SPELL_AURA_MOD_INCREASE_MOUNTED_FLIGHT_SPEED ||
-                        spellInfo->Effects[2].ApplyAuraName == SPELL_AURA_MOD_INCREASE_MOUNTED_FLIGHT_SPEED ||
-                        // Winged Steed of the Ebon Blade
-                        // This mount is meant to autoscale from a 150% flyer
-                        // up to a 280% as you train your flying skill up.
-                        // This incorrectly gets categorised as a ground mount, force this to flyer only.
-                        // TODO: Add other scaling mounts here if they have the same issue, or adjust above
-                        // checks so that they are all correctly detected.
-                        spellInfo->Id == 54729) ? 1 : 0;
-
-        allSpells[index][effect].push_back(spellId);
-    }
-    return allSpells;
-}
-
 bool CheckMountStateAction::TryPreferredMount(Player* master) const
 {
     static bool tableExists = false;
@@ -330,11 +332,14 @@ bool CheckMountStateAction::TryPreferredMount(Player* master) const
 
             // Validate spell ID
             // TODO: May want to do checks for 'bot riding skill > skill required to ride the mount'
+            if (!mounts.empty())
+            {
                 uint32 index = urand(0, mounts.size() - 1);
                 if (index < mounts.size() && sSpellMgr->GetSpellInfo(mounts[index]))
                     return botAI->CastSpell(mounts[index], bot);
             }
         }
+    }
     return false;
 }
 
@@ -357,35 +362,22 @@ uint32 CheckMountStateAction::GetMountType(Player* master) const
     return 0;
 }
 
-void CheckMountStateAction::FilterMountsBySpeed(std::map<int32, std::vector<uint32>>& spells, int32 masterSpeed) const
+bool CheckMountStateAction::TryRandomMountFiltered(const std::map<int32, std::vector<uint32>>& spells, int32 masterSpeed) const
 {
-    for (auto& [speed, ids] : spells)
+    // Iterate over each speed group once.
+    for (const auto& pair : spells)
     {
-        for (auto& id : ids)
+        int32 currentSpeed = pair.first;
-        {
+        if ((masterSpeed > 59 && currentSpeed < 99) || (masterSpeed > 149 && currentSpeed < 279))
-            SpellInfo const* spellInfo = sSpellMgr->GetSpellInfo(id);
-            if (!spellInfo)
             continue;
 
-            if (masterSpeed > 59 && std::max(spellInfo->Effects[1].BasePoints, spellInfo->Effects[2].BasePoints) < 99)
+        const auto& ids = pair.second;
-                spells[59].clear();
+        if (!ids.empty())
-
-            if (masterSpeed > 149 && std::max(spellInfo->Effects[1].BasePoints, spellInfo->Effects[2].BasePoints) < 279)
-                spells[149].clear();
-        }
-    }
-}
-
-bool CheckMountStateAction::TryRandomMount(const std::map<int32, std::vector<uint32>>& spells) const
-{
-    for (const auto& [speed, ids] : spells)
         {
-        if (ids.empty())
+            // Pick a random mount from the candidate group.
-            continue;
-
             uint32 index = urand(0, ids.size() - 1);
-        if (index < ids.size())
             return botAI->CastSpell(ids[index], bot);
         }
+    }
     return false;
 }

src/strategy/actions/CheckMountStateAction.h

@@ -8,6 +8,15 @@
 
 #include "UseItemAction.h"
 
+struct MountData
+{
+    bool swiftMount = false;
+    // Outer map: index (0 for ground, 1 for flight), inner map: effect speed -> vector of spell IDs.
+    std::map<uint32, std::map<int32, std::vector<uint32>>> allSpells;
+    // Default mount speed.
+    int32 maxSpeed = 59;
+};
+
 class PlayerbotAI;
 
 class CheckMountStateAction : public UseItemAction
@@ -32,8 +41,7 @@ private:
     std::map<uint32, std::map<int32, std::vector<uint32>>> GetAllMountSpells() const;
     bool TryPreferredMount(Player* master) const;
     uint32 GetMountType(Player* master) const;
-    void FilterMountsBySpeed(std::map<int32, std::vector<uint32>>& spells, int32 masterSpeed) const;
+    bool TryRandomMountFiltered(const std::map<int32, std::vector<uint32>>& spells, int32 masterSpeed) const;
-    bool TryRandomMount(const std::map<int32, std::vector<uint32>>& spells) const;
 };
 
 #endif