Update windows_build.yml

Adding a guard against bots that may be registered in `playerBots`
variable as selfbot have appeared to crash here.

Edit: The exact circumstances that caused the incorrect registration are
under investigation as it seems to be 'atypical' behavior.

.github/workflows/windows_build.yml

@@ -25,21 +25,25 @@ jobs:
         with:
           repository: 'mod-playerbots/azerothcore-wotlk'
           ref: 'Playerbot'
+          path: 'ac'
       - name: Checkout Playerbot Module
         uses: actions/checkout@v3
         with:
           repository: 'mod-playerbots/mod-playerbots'
-          path: 'modules/mod-playerbots'
+          #path: 'modules/mod-playerbots'
+          path: ac/modules/mod-playerbots
       - name: ccache
         uses: hendrikmuhs/ccache-action@v1.2.13
       - name: Configure OS
         shell: bash
+        working-directory: ac
         env:
           CONTINUOUS_INTEGRATION: true
         run: |
           ./acore.sh install-deps
       - name: Build
         shell: bash
+        working-directory: ac
         run: |
           export CTOOLS_BUILD=all
           ./acore.sh compiler build

src/RandomPlayerbotMgr.cpp

@@ -3214,6 +3214,12 @@ void RandomPlayerbotMgr::PrintStats()
         lvlPerRace[bot->getRace()] += bot->GetLevel();
 
         PlayerbotAI* botAI = GET_PLAYERBOT_AI(bot);
+        if (!botAI)
+        {
+            LOG_ERROR("playerbots", "Player/Bot {} is registered in sRandomPlayerbotMgr playerBots and has no bot AI!", bot->GetName().c_str());
+            continue;
+        }
+
         if (botAI->AllowActivity())
             ++active;
 

src/factory/PlayerbotFactory.cpp

@@ -288,17 +288,17 @@ void PlayerbotFactory::Randomize(bool incremental)
             pmo->finish();
     }
 
-    pmo = sPerformanceMonitor->start(PERF_MON_RNDBOT, "PlayerbotFactory_Spells1");
-    LOG_DEBUG("playerbots", "Initializing spells (step 1)...");
+    LOG_DEBUG("playerbots", "Initializing skills (step 1)...");
+    pmo = sPerformanceMonitor->start(PERF_MON_RNDBOT, "PlayerbotFactory_Skills1");
     bot->LearnDefaultSkills();
-    InitClassSpells();
-    InitAvailableSpells();
+    InitSkills();
     if (pmo)
         pmo->finish();
 
-    LOG_DEBUG("playerbots", "Initializing skills (step 1)...");
-    pmo = sPerformanceMonitor->start(PERF_MON_RNDBOT, "PlayerbotFactory_Skills1");
-    InitSkills();
+    pmo = sPerformanceMonitor->start(PERF_MON_RNDBOT, "PlayerbotFactory_Spells1");
+    LOG_DEBUG("playerbots", "Initializing spells (step 1)...");
+    InitClassSpells();
+    InitAvailableSpells();
     if (pmo)
         pmo->finish();
 
@@ -506,9 +506,9 @@ void PlayerbotFactory::Refresh()
     InitPotions();
     InitPet();
     InitPetTalents();
+    InitSkills();
     InitClassSpells();
     InitAvailableSpells();
-    InitSkills();
     InitReputation();
     InitSpecialSpells();
     InitMounts();
@@ -2550,13 +2550,19 @@ void PlayerbotFactory::InitAvailableSpells()
 
         for (auto& spell : trainer->GetSpells())
         {
-            if (!trainer->CanTeachSpell(bot, trainer->GetSpell(spell.SpellId)))
+            // simplified version of Trainer::TeachSpell method
+
+            Trainer::Spell const* trainerSpell = trainer->GetSpell(spell.SpellId);
+            if (!trainerSpell)
                 continue;
 
-            if (spell.IsCastable())
-                bot->CastSpell(bot, spell.SpellId, true);
+            if (!trainer->CanTeachSpell(bot, trainerSpell))
+                continue;
+
+            if (trainerSpell->IsCastable())
+                bot->CastSpell(bot, trainerSpell->SpellId, true);
             else
-                bot->learnSpell(spell.SpellId, false);
+                bot->learnSpell(trainerSpell->SpellId, false);
         }
     }
 }

src/strategy/Engine.cpp

@@ -606,7 +606,7 @@ void Engine::LogAction(char const* format, ...)
 
     va_list ap;
     va_start(ap, format);
-    vsprintf(buf, format, ap);
+    vsnprintf(buf, sizeof(buf), format, ap);
     va_end(ap);
 
     lastAction += "|";

src/strategy/actions/AutoMaintenanceOnLevelupAction.cpp

@@ -75,36 +75,79 @@ void AutoMaintenanceOnLevelupAction::LearnSpells(std::ostringstream* out)
 void AutoMaintenanceOnLevelupAction::LearnTrainerSpells(std::ostringstream* out)
 {
     PlayerbotFactory factory(bot, bot->GetLevel());
+    factory.InitSkills();
     factory.InitClassSpells();
     factory.InitAvailableSpells();
-    factory.InitSkills();
     factory.InitPet();
 }
 
 void AutoMaintenanceOnLevelupAction::LearnQuestSpells(std::ostringstream* out)
 {
-    // CreatureTemplate const* co = sCreatureStorage.LookupEntry<CreatureTemplate>(id);
     ObjectMgr::QuestMap const& questTemplates = sObjectMgr->GetQuestTemplates();
     for (ObjectMgr::QuestMap::const_iterator i = questTemplates.begin(); i != questTemplates.end(); ++i)
     {
-        //uint32 questId = i->first; //not used, line marked for removal.
         Quest const* quest = i->second;
 
-        if (!quest->GetRequiredClasses() || quest->IsRepeatable() || quest->GetMinLevel() < 10)
+        // only process class-specific quests to learn class-related spells, cuz
+        // we don't want all these bunch of entries to be handled!
+        if (!quest->GetRequiredClasses())
             continue;
 
-        if (!bot->SatisfyQuestClass(quest, false) || quest->GetMinLevel() > bot->GetLevel() ||
-            !bot->SatisfyQuestRace(quest, false))
+        // skip quests that are repeatable, too low level, or above bots' level
+        if (quest->IsRepeatable() || quest->GetMinLevel() < 10 || quest->GetMinLevel() > bot->GetLevel())
             continue;
 
-        if (quest->GetRewSpellCast() > 0)
+        // skip if bot doesnt satisfy class, race, or skill requirements
+        if (!bot->SatisfyQuestClass(quest, false) || !bot->SatisfyQuestRace(quest, false) ||
+            !bot->SatisfyQuestSkill(quest, false))
+            continue;
+
+        // use the same logic and impl from Player::learnQuestRewardedSpells
+
+        int32 spellId = quest->GetRewSpellCast();
+        if (!spellId)
+            continue;
+
+        SpellInfo const* spellInfo = sSpellMgr->GetSpellInfo(spellId);
+        if (!spellInfo)
+            continue;
+
+        // xinef: find effect with learn spell and check if we have this spell
+        bool found = false;
+        for (uint8 i = 0; i < MAX_SPELL_EFFECTS; ++i)
         {
-            LearnSpell(quest->GetRewSpellCast(), out);
+            if (spellInfo->Effects[i].Effect == SPELL_EFFECT_LEARN_SPELL && spellInfo->Effects[i].TriggerSpell &&
+                !bot->HasSpell(spellInfo->Effects[i].TriggerSpell))
+            {
+                // pusywizard: don't re-add profession specialties!
+                if (SpellInfo const* triggeredInfo = sSpellMgr->GetSpellInfo(spellInfo->Effects[i].TriggerSpell))
+                    if (triggeredInfo->Effects[0].Effect == SPELL_EFFECT_TRADE_SKILL)
+                        break; // pussywizard: break and not cast the spell (found is false)
+
+                found = true;
+                break;
+            }
         }
-        else if (quest->GetRewSpell() > 0)
+
+        // xinef: we know the spell, continue
+        if (!found)
+            continue;
+
+        bot->CastSpell(bot, spellId, true);
+
+        // Check if RewardDisplaySpell is set to output the proper spell learned
+        // after processing quests. Output the original RewardSpell otherwise.
+        uint32 rewSpellId = quest->GetRewSpell();
+        if (rewSpellId)
         {
-            LearnSpell(quest->GetRewSpell(), out);
+            if (SpellInfo const* rewSpellInfo = sSpellMgr->GetSpellInfo(rewSpellId))
+            {
+                *out << FormatSpell(rewSpellInfo) << ", ";
+                continue;
+            }
         }
+
+        *out << FormatSpell(spellInfo) << ", ";
     }
 }
 
@@ -121,39 +164,6 @@ std::string const AutoMaintenanceOnLevelupAction::FormatSpell(SpellInfo const* s
     return out.str();
 }
 
-void AutoMaintenanceOnLevelupAction::LearnSpell(uint32 spellId, std::ostringstream* out)
-{
-    SpellInfo const* proto = sSpellMgr->GetSpellInfo(spellId);
-    if (!proto)
-        return;
-
-    bool learned = false;
-    for (uint8 j = 0; j < 3; ++j)
-    {
-        if (proto->Effects[j].Effect == SPELL_EFFECT_LEARN_SPELL)
-        {
-            uint32 learnedSpell = proto->Effects[j].TriggerSpell;
-
-            if (!bot->HasSpell(learnedSpell))
-            {
-                bot->learnSpell(learnedSpell);
-                *out << FormatSpell(sSpellMgr->GetSpellInfo(learnedSpell)) << ", ";
-            }
-
-            learned = true;
-        }
-    }
-
-    if (!learned)
-    {
-        if (!bot->HasSpell(spellId))
-        {
-            bot->learnSpell(spellId);
-            *out << FormatSpell(proto) << ", ";
-        }
-    }
-}
-
 void AutoMaintenanceOnLevelupAction::AutoUpgradeEquip()
 {
     if (!sPlayerbotAIConfig->autoUpgradeEquip || !sRandomPlayerbotMgr->IsRandomBot(bot))

src/strategy/actions/AutoMaintenanceOnLevelupAction.h

@@ -28,7 +28,6 @@ protected:
     void LearnSpells(std::ostringstream* out);
     void LearnTrainerSpells(std::ostringstream* out);
     void LearnQuestSpells(std::ostringstream* out);
-    void LearnSpell(uint32 spellId, std::ostringstream* out);
     std::string const FormatSpell(SpellInfo const* sInfo);
 };
 

src/strategy/actions/FishingAction.cpp

@@ -261,7 +261,9 @@ bool MoveNearWaterAction::isUseful()
         return false;
     FishingSpotValue* fishingSpotValueObject = (FishingSpotValue*)context->GetValue<WorldPosition>("fishing spot");
     WorldPosition pos = fishingSpotValueObject->Get();
-    return !pos.IsValid() || fishingSpotValueObject->IsStale(FISHING_LOCATION_TIMEOUT) || pos != bot->GetPosition();
+    return !pos.IsValid() || fishingSpotValueObject->IsStale(FISHING_LOCATION_TIMEOUT) ||
+           bot->GetExactDist(&pos) < 0.1f;
+
 }
 
 bool MoveNearWaterAction::isPossible()
@@ -299,6 +301,17 @@ bool MoveNearWaterAction::isPossible()
             }
         }
     }
+    // Can the bot fish from current position?
+    WorldPosition waterAtCurrentPos =
+        FindWaterRadial(bot, bot->GetPositionX(), bot->GetPositionY(), bot->GetPositionZ(), bot->GetMap(),
+                        bot->GetPhaseMask(), MIN_DISTANCE_TO_WATER, MAX_DISTANCE_TO_WATER, SEARCH_INCREMENT, true);
+    if (waterAtCurrentPos.IsValid())
+    {
+        SET_AI_VALUE(WorldPosition, "fishing spot",
+                     WorldPosition(WorldPosition(bot->GetMapId(), bot->GetPositionX(), bot->GetPositionY(),
+                                                 bot->GetPositionZ())));
+        return false;
+    }
     // Lets find some water where we can fish.
     WorldPosition water = FindWaterRadial(
         bot, bot->GetPositionX(), bot->GetPositionY(), bot->GetPositionZ(),
@@ -435,10 +448,14 @@ bool FishingAction::isUseful()
 {
     if (!AI_VALUE(bool, "can fish"))
         return false;
+
     FishingSpotValue* fishingSpotValueObject = (FishingSpotValue*)context->GetValue<WorldPosition>("fishing spot");
     WorldPosition pos = fishingSpotValueObject->Get();
 
-    return pos.IsValid() && !fishingSpotValueObject->IsStale(FISHING_LOCATION_TIMEOUT) && pos == bot->GetPosition();
+    if (!pos.IsValid() || fishingSpotValueObject->IsStale(FISHING_LOCATION_TIMEOUT))
+        return false;
+
+    return bot->GetExactDist(&pos) < 0.1f;
 }
 
 bool UseBobberAction::isUseful()

src/strategy/actions/TrainerAction.cpp

@@ -178,9 +178,9 @@ bool MaintenanceAction::Execute(Event event)
         factory.InitTalentsTree(true);
         factory.InitPet();
         factory.InitPetTalents();
+        factory.InitSkills();
         factory.InitClassSpells();
         factory.InitAvailableSpells();
-        factory.InitSkills();
         factory.InitReputation();
         factory.InitSpecialSpells();
         factory.InitMounts();
@@ -221,15 +221,15 @@ bool MaintenanceAction::Execute(Event event)
         if (sPlayerbotAIConfig->altMaintenancePetTalents)
             factory.InitPetTalents();
 
+        if (sPlayerbotAIConfig->altMaintenanceSkills)
+            factory.InitSkills();
+
         if (sPlayerbotAIConfig->altMaintenanceClassSpells)
             factory.InitClassSpells();
 
         if (sPlayerbotAIConfig->altMaintenanceAvailableSpells)
             factory.InitAvailableSpells();
 
-        if (sPlayerbotAIConfig->altMaintenanceSkills)
-            factory.InitSkills();
-
         if (sPlayerbotAIConfig->altMaintenanceReputation)
             factory.InitReputation();
 

src/strategy/rpg/NewRpgBaseAction.cpp

@@ -269,7 +269,7 @@ bool NewRpgBaseAction::CanInteractWithQuestGiver(Object* questGiver)
             if (!guid)
                 return false;
 
-            if (!bot->IsInWorld())
+            if (!bot->IsInWorld() || bot->IsDuringRemoveFromWorld())
                 return false;
 
             if (bot->IsInFlight())

src/strategy/values/EnemyPlayerValue.cpp

@@ -11,10 +11,6 @@
 
 bool NearestEnemyPlayersValue::AcceptUnit(Unit* unit)
 {
-    // Apply parent's filtering first (includes level difference checks)
-    if (!PossibleTargetsValue::AcceptUnit(unit))
-        return false;
-
     bool inCannon = botAI->IsInVehicle(false, true);
     Player* enemy = dynamic_cast<Player*>(unit);
     if (enemy && botAI->IsOpposing(enemy) && enemy->IsPvP() &&
@@ -23,14 +19,7 @@ bool NearestEnemyPlayersValue::AcceptUnit(Unit* unit)
         ((inCannon || !enemy->HasFlag(UNIT_FIELD_FLAGS, UNIT_FLAG_NOT_SELECTABLE))) &&
         /*!enemy->HasStealthAura() && !enemy->HasInvisibilityAura()*/ enemy->CanSeeOrDetect(bot) &&
         !(enemy->HasSpiritOfRedemptionAura()))
-    {
-        // If with master, only attack if master is PvP flagged
-        Player* master = botAI->GetMaster();
-        if (master && !master->IsPvP() && !master->IsFFAPvP())
-            return false;
-
         return true;
-    }
 
     return false;
 }

src/strategy/values/PossibleTargetsValue.cpp

@@ -16,32 +16,6 @@
 #include "SpellAuraEffects.h"
 #include "SpellMgr.h"
 #include "Unit.h"
-#include "AreaDefines.h"
-#include <unordered_map>
-
-// Level difference thresholds for attack probability
-constexpr int32 EXTREME_LEVEL_DIFF = 5;  // Don't attack if enemy is this much higher
-constexpr int32 HIGH_LEVEL_DIFF = 4;     // 25% chance at +/- this difference
-constexpr int32 MID_LEVEL_DIFF = 3;      // 50% chance at +/- this difference
-constexpr int32 LOW_LEVEL_DIFF = 2;      // 75% chance at +/- this difference
-
-// Cache duration before reconsidering attack decision, and old cache cleanup interval
-constexpr uint32 ATTACK_DECISION_CACHE_DURATION = 2 * MINUTE;
-constexpr uint32 ATTACK_DECISION_CACHE_CLEANUP_INTERVAL = 10 * MINUTE;
-
-// Custom hash function for (botGUID, targetGUID) pairs
-struct PairGuidHash
-{
-    std::size_t operator()(const std::pair<ObjectGuid, ObjectGuid>& pair) const
-    {
-        return std::hash<uint64>()(pair.first.GetRawValue()) ^
-               (std::hash<uint64>()(pair.second.GetRawValue()) << 1);
-    }
-};
-
-// Cache for probability-based attack decisions (Per-bot: non-global)
-// Map: (botGUID, targetGUID) -> (should attack decision, timestamp)
-static std::unordered_map<std::pair<ObjectGuid, ObjectGuid>, std::pair<bool, time_t>, PairGuidHash> attackDecisionCache;
 
 void PossibleTargetsValue::FindUnits(std::list<Unit*>& targets)
 {
@@ -50,117 +24,7 @@ void PossibleTargetsValue::FindUnits(std::list<Unit*>& targets)
     Cell::VisitObjects(bot, searcher, range);
 }
 
-static void CleanupAttackDecisionCache()
-{
-    time_t currentTime = time(nullptr);
-    for (auto it = attackDecisionCache.begin(); it != attackDecisionCache.end();)
-    {
-        if (currentTime - it->second.second >= ATTACK_DECISION_CACHE_DURATION)
-            it = attackDecisionCache.erase(it);
-        else
-            ++it;
-    }
-}
-
-bool PossibleTargetsValue::AcceptUnit(Unit* unit)
-{
-    // attackDecisionCache cleanup
-    static time_t lastCleanup = 0;
-    time_t currentTime = time(nullptr);
-    if (currentTime - lastCleanup > ATTACK_DECISION_CACHE_CLEANUP_INTERVAL)
-    {
-        CleanupAttackDecisionCache();
-        lastCleanup = currentTime;
-    }
-
-    if (!AttackersValue::IsPossibleTarget(unit, bot, range))
-        return false;
-
-    // Level-based PvP restrictions
-    if (unit->IsPlayer())
-    {
-        // Self-defense - always allow fighting back
-        if (bot->IsInCombat() && bot->GetVictim() == unit)
-            return true;  // Already fighting
-
-        Unit* botAttacker = bot->getAttackerForHelper();
-        if (botAttacker)
-        {
-            if (botAttacker == unit)
-                return true;  // Enemy attacking
-
-            if (botAttacker->IsPet())
-            {
-                Unit* petOwner = botAttacker->GetOwner();
-                if (petOwner && petOwner == unit)
-                    return true;  // Enemy's pet attacking
-            }
-        }
-
-        // Skip restrictions in BG/Arena
-        if (bot->InBattleground() || bot->InArena())
-            return true;
-
-        // Skip restrictions if in duel with this player
-        if (bot->duel && bot->duel->Opponent == unit)
-            return true;
-
-        // Capital cities - no restrictions
-        uint32 zoneId = bot->GetZoneId();
-        bool inCapitalCity = (zoneId == AREA_STORMWIND_CITY ||
-                            zoneId == AREA_IRONFORGE        ||
-                            zoneId == AREA_DARNASSUS        ||
-                            zoneId == AREA_THE_EXODAR       ||
-                            zoneId == AREA_ORGRIMMAR        ||
-                            zoneId == AREA_THUNDER_BLUFF    ||
-                            zoneId == AREA_UNDERCITY        ||
-                            zoneId == AREA_SILVERMOON_CITY);
-
-        if (inCapitalCity)
-            return true;
-
-        // Level difference check
-        int32 levelDifference = unit->GetLevel() - bot->GetLevel();
-        int32 absLevelDifference = std::abs(levelDifference);
-
-        // Extreme difference - do not attack
-        if (levelDifference >= EXTREME_LEVEL_DIFF)
-            return false;
-
-        // Calculate attack chance based on level difference
-        uint32 attackChance = 100; // Default 100%: Bot and target's levels are very close
-
-        // There's a chance a bot might gank on an extremly low target
-        if ((absLevelDifference < EXTREME_LEVEL_DIFF && absLevelDifference >= HIGH_LEVEL_DIFF) ||
-            levelDifference <= -EXTREME_LEVEL_DIFF)
-            attackChance = 25;
-
-        else if (absLevelDifference < HIGH_LEVEL_DIFF && absLevelDifference >= MID_LEVEL_DIFF)
-            attackChance = 50;
-
-        else if (absLevelDifference < MID_LEVEL_DIFF && absLevelDifference >= LOW_LEVEL_DIFF)
-            attackChance = 75;
-
-        // If probability check needed, use cache
-        if (attackChance < 100)
-        {
-            std::pair<ObjectGuid, ObjectGuid> cacheKey = std::make_pair(bot->GetGUID(), unit->GetGUID());
-
-            auto it = attackDecisionCache.find(cacheKey);
-            if (it != attackDecisionCache.end())
-            {
-                if (currentTime - it->second.second < ATTACK_DECISION_CACHE_DURATION)
-                    return it->second.first;
-            }
-
-            bool shouldAttack = (urand(1, 100) <= attackChance);
-            attackDecisionCache[cacheKey] = std::make_pair(shouldAttack, currentTime);
-            return shouldAttack;
-        }
-    }
-
-    return true;
-}
+bool PossibleTargetsValue::AcceptUnit(Unit* unit) { return AttackersValue::IsPossibleTarget(unit, bot, range); }
 
 void PossibleTriggersValue::FindUnits(std::list<Unit*>& targets)
 {
@@ -172,8 +36,9 @@ void PossibleTriggersValue::FindUnits(std::list<Unit*>& targets)
 bool PossibleTriggersValue::AcceptUnit(Unit* unit)
 {
     if (!unit->HasUnitFlag(UNIT_FLAG_NOT_SELECTABLE))
+    {
         return false;
-
+    }
     Unit::AuraEffectList const& aurasPeriodicTriggerSpell =
         unit->GetAuraEffectsByType(SPELL_AURA_PERIODIC_TRIGGER_SPELL);
     Unit::AuraEffectList const& aurasPeriodicTriggerWithValueSpell =
@@ -193,7 +58,9 @@ bool PossibleTriggersValue::AcceptUnit(Unit* unit)
             for (int j = 0; j < MAX_SPELL_EFFECTS; j++)
             {
                 if (triggerSpellInfo->Effects[j].Effect == SPELL_EFFECT_SCHOOL_DAMAGE)
+                {
                     return true;
+                }
             }
         }
     }