Bug - Selfbot can trigger crash in printstats (#2013)

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.

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 skills (step 1)...");
-    LOG_DEBUG("playerbots", "Initializing spells (step 1)...");
+    pmo = sPerformanceMonitor->start(PERF_MON_RNDBOT, "PlayerbotFactory_Skills1");
     bot->LearnDefaultSkills();
-    InitClassSpells();
+    InitSkills();
-    InitAvailableSpells();
     if (pmo)
         pmo->finish();
 
-    LOG_DEBUG("playerbots", "Initializing skills (step 1)...");
+    pmo = sPerformanceMonitor->start(PERF_MON_RNDBOT, "PlayerbotFactory_Spells1");
-    pmo = sPerformanceMonitor->start(PERF_MON_RNDBOT, "PlayerbotFactory_Skills1");
+    LOG_DEBUG("playerbots", "Initializing spells (step 1)...");
-    InitSkills();
+    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())
+            if (!trainer->CanTeachSpell(bot, trainerSpell))
-                bot->CastSpell(bot, spell.SpellId, true);
+                continue;
+
+            if (trainerSpell->IsCastable())
+                bot->CastSpell(bot, trainerSpell->SpellId, true);
             else
-                bot->learnSpell(spell.SpellId, false);
+                bot->learnSpell(trainerSpell->SpellId, false);
         }
     }
 }

src/strategy/actions/AutoMaintenanceOnLevelupAction.cpp

@@ -75,19 +75,17 @@ 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;
 
         // only process class-specific quests to learn class-related spells, cuz
@@ -104,14 +102,52 @@ void AutoMaintenanceOnLevelupAction::LearnQuestSpells(std::ostringstream* out)
             !bot->SatisfyQuestSkill(quest, false))
             continue;
 
-        if (quest->GetRewSpellCast() > 0) // RewardSpell - expected route
+        // 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) // RewardDisplaySpell - fallback
+
+        // 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) << ", ";
     }
 }
 
@@ -128,41 +164,6 @@ std::string const AutoMaintenanceOnLevelupAction::FormatSpell(SpellInfo const* s
     return out.str();
 }
 
-void AutoMaintenanceOnLevelupAction::LearnSpell(uint32 spellId, std::ostringstream* out)
-{
-    SpellInfo const* spellInfo = sSpellMgr->GetSpellInfo(spellId);
-    if (!spellInfo)
-        return;
-
-    SpellInfo const* triggeredInfo;
-
-    // find effect with learn spell and check if we have this spell
-    bool found = false;
-    for (uint8 i = 0; i < MAX_SPELL_EFFECTS; ++i)
-    {
-        if (spellInfo->Effects[i].Effect == SPELL_EFFECT_LEARN_SPELL && spellInfo->Effects[i].TriggerSpell &&
-            !bot->HasSpell(spellInfo->Effects[i].TriggerSpell))
-        {
-            triggeredInfo = sSpellMgr->GetSpellInfo(spellInfo->Effects[i].TriggerSpell);
-
-            // do not learn profession specialties!
-            if (!triggeredInfo || triggeredInfo->Effects[0].Effect == SPELL_EFFECT_TRADE_SKILL)
-                break;
-
-            found = true;
-            break;
-        }
-    }
-
-    if (!found)
-        return;
-
-    // NOTE: When rewarding quests, core casts spells instead of learning them,
-    // but we sacrifice safe cast checks here in favor of performance/speed
-    bot->learnSpell(triggeredInfo->Id);
-    *out << FormatSpell(triggeredInfo) << ", ";
-}
-
 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/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();