From e2c203a35e2fa77bd37711f058d12c0c02479bf8 Mon Sep 17 00:00:00 2001 From: privatecore Date: Thu, 15 Jan 2026 00:42:19 +0100 Subject: [PATCH] Fix the duplicate spells issue during randomization (#2014) **Original issue:** Bots/Characters received duplicate spells during randomization process. **Root cause:** When `PlayerbotFactory::Randomize` is processed, `InitSkills` is called AFTER `InitAvailableSpells`, which causes the duplicate spells issue because the skillline ability spell is already learned when processing spells from trainers (`InitAvailableSpells`). We simply need to change the order of the randomization process: skills should be handled first, then spells. An alternative approach would be to adjust the skillline abilities and check each spell for every skill, but that seems redundant since we already have checks for the trainer's spells. `InitSkills` -> `SetRandomSkill` -> `SetSkill` -> `learnSkillRewardedSpells` -> `learnSpell` -> duplicate error... **Steps to reproduce:** 1. create common character and login with it 2. set level for this character eq. 80 (`.set level 79`) 3. create and run macro: ``` /g .playerbots bot initself /g .saveall ``` 4. logout -> login and run macro again **Note:** Also added checks for the trainer's spells since `GetSpell` can return nullptr. Updated `LearnQuestSpells` after recent changes and used the same logic and implementation from `Player::learnQuestRewardedSpells`. Yes, we need to cast spells, or we should handle all spell effects that quests/trainers have (for ex.: `SPELL_EFFECT_SKILL_STEP`, `SPELL_EFFECT_UNLEARN_SPECIALIZATION`, `SPELL_EFFECT_BIND`). --- src/factory/PlayerbotFactory.cpp | 30 ++++--- .../AutoMaintenanceOnLevelupAction.cpp | 85 ++++++++++--------- .../actions/AutoMaintenanceOnLevelupAction.h | 1 - src/strategy/actions/TrainerAction.cpp | 8 +- 4 files changed, 65 insertions(+), 59 deletions(-) diff --git a/src/factory/PlayerbotFactory.cpp b/src/factory/PlayerbotFactory.cpp index 21f09381..af2975a5 100644 --- a/src/factory/PlayerbotFactory.cpp +++ b/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); } } } diff --git a/src/strategy/actions/AutoMaintenanceOnLevelupAction.cpp b/src/strategy/actions/AutoMaintenanceOnLevelupAction.cpp index 5f7d7435..6939e5c9 100644 --- a/src/strategy/actions/AutoMaintenanceOnLevelupAction.cpp +++ b/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(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)) diff --git a/src/strategy/actions/AutoMaintenanceOnLevelupAction.h b/src/strategy/actions/AutoMaintenanceOnLevelupAction.h index 314d44bb..5399973c 100644 --- a/src/strategy/actions/AutoMaintenanceOnLevelupAction.h +++ b/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); }; diff --git a/src/strategy/actions/TrainerAction.cpp b/src/strategy/actions/TrainerAction.cpp index cfa2f504..bca74060 100644 --- a/src/strategy/actions/TrainerAction.cpp +++ b/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();