diff --git a/src/Playerbots.cpp b/src/Playerbots.cpp index 83df732a..51865e34 100644 --- a/src/Playerbots.cpp +++ b/src/Playerbots.cpp @@ -25,6 +25,7 @@ #include "Metric.h" #include "PlayerScript.h" #include "PlayerbotAIConfig.h" +#include "PlayerbotSpellCache.h" #include "PlayerbotWorldThreadProcessor.h" #include "RandomPlayerbotMgr.h" #include "ScriptMgr.h" @@ -347,6 +348,9 @@ public: LOG_INFO("server.loading", ">> Loaded playerbots config in {} ms", GetMSTimeDiffToNow(oldMSTime)); LOG_INFO("server.loading", " "); + + sPlayerbotSpellCache->Initialize(); + LOG_INFO("server.loading", "Playerbots World Thread Processor initialized"); } diff --git a/src/database/PlayerbotSpellCache.cpp b/src/database/PlayerbotSpellCache.cpp new file mode 100644 index 00000000..02efcbd9 --- /dev/null +++ b/src/database/PlayerbotSpellCache.cpp @@ -0,0 +1,45 @@ +#include "PlayerbotSpellCache.h" + +void PlayerbotSpellCache::Initialize() +{ + LOG_INFO("playerbots", + "Playerbots: ListSpellsAction caches initialized"); + for (uint32 j = 0; j < sSkillLineAbilityStore.GetNumRows(); ++j) + { + if (SkillLineAbilityEntry const* skillLine = sSkillLineAbilityStore.LookupEntry(j)) + skillSpells[skillLine->Spell] = skillLine; + } + + // Fill the vendorItems cache once from the world database. + QueryResult results = WorldDatabase.Query("SELECT item FROM npc_vendor WHERE maxcount = 0"); + if (results) + { + do + { + Field* fields = results->Fetch(); + int32 entry = fields[0].Get(); + if (entry <= 0) + continue; + + vendorItems.insert(static_cast(entry)); + } + while (results->NextRow()); + } + + LOG_DEBUG("playerbots", + "ListSpellsAction: initialized caches (skillSpells={}, vendorItems={}).", + skillSpells.size(), vendorItems.size()); +} + +SkillLineAbilityEntry const* PlayerbotSpellCache::GetSkillLine(uint32 spellId) const +{ + auto itr = skillSpells.find(spellId); + if (itr != skillSpells.end()) + return itr->second; + return nullptr; +} + +bool PlayerbotSpellCache::IsItemBuyable(uint32 itemId) const +{ + return vendorItems.find(itemId) != vendorItems.end(); +} diff --git a/src/database/PlayerbotSpellCache.h b/src/database/PlayerbotSpellCache.h new file mode 100644 index 00000000..15502768 --- /dev/null +++ b/src/database/PlayerbotSpellCache.h @@ -0,0 +1,34 @@ +/* + * Copyright (C) 2016+ AzerothCore , released under GNU AGPL v3 license, you may redistribute it + * and/or modify it under version 3 of the License, or (at your option), any later version. + */ + +#ifndef _PLAYERBOT_PLAYERBOTSPELLCACHE_H +#define _PLAYERBOT_PLAYERBOTSPELLCACHE_H + +#include "Playerbots.h" + +class PlayerbotSpellCache +{ +public: + static PlayerbotSpellCache* Instance() + { + static PlayerbotSpellCache instance; + return &instance; + } + + void Initialize(); // call once on startup + + SkillLineAbilityEntry const* GetSkillLine(uint32 spellId) const; + bool IsItemBuyable(uint32 itemId) const; + +private: + PlayerbotSpellCache() = default; + + std::map skillSpells; + std::set vendorItems; +}; + +#define sPlayerbotSpellCache PlayerbotSpellCache::Instance() + +#endif diff --git a/src/strategy/actions/ListSpellsAction.cpp b/src/strategy/actions/ListSpellsAction.cpp index 3a3ad7f6..6d774115 100644 --- a/src/strategy/actions/ListSpellsAction.cpp +++ b/src/strategy/actions/ListSpellsAction.cpp @@ -7,90 +7,43 @@ #include "Event.h" #include "Playerbots.h" +#include "PlayerbotSpellCache.h" -std::map ListSpellsAction::skillSpells; -std::set ListSpellsAction::vendorItems; +using SpellListEntry = std::pair; -bool CompareSpells(const std::pair& s1, const std::pair& s2) +// CHANGE: Simplified and cheap comparator used in MapUpdater worker thread. +// It now avoids scanning the entire SkillLineAbilityStore for each comparison +// and only relies on spell school and spell name to keep sorting fast and bounded. +// lhs = the left element, rhs = the right element. +static bool CompareSpells(SpellListEntry const& lhSpell, SpellListEntry const& rhSpell) { - SpellInfo const* si1 = sSpellMgr->GetSpellInfo(s1.first); - SpellInfo const* si2 = sSpellMgr->GetSpellInfo(s2.first); - if (!si1 || !si2) + SpellInfo const* lhSpellInfo = sSpellMgr->GetSpellInfo(lhSpell.first); + SpellInfo const* rhSpellInfo = sSpellMgr->GetSpellInfo(rhSpell.first); + + if (!lhSpellInfo || !rhSpellInfo) { - LOG_ERROR("playerbots", "SpellInfo missing. {} {}", s1.first, s2.first); - return false; - } - uint32 p1 = si1->SchoolMask * 20000; - uint32 p2 = si2->SchoolMask * 20000; - - uint32 skill1 = 0, skill2 = 0; - uint32 skillValue1 = 0, skillValue2 = 0; - for (uint32 j = 0; j < sSkillLineAbilityStore.GetNumRows(); ++j) - { - if (SkillLineAbilityEntry const* skillLine = sSkillLineAbilityStore.LookupEntry(j)) - { - if (skillLine->Spell == s1.first) - { - skill1 = skillLine->SkillLine; - skillValue1 = skillLine->TrivialSkillLineRankLow; - } - - if (skillLine->Spell == s2.first) - { - skill2 = skillLine->SkillLine; - skillValue2 = skillLine->TrivialSkillLineRankLow; - } - } - - if (skill1 && skill2) - break; + LOG_ERROR("playerbots", "SpellInfo missing for spell {} or {}", lhSpell.first, rhSpell.first); + // Fallback: order by spell id to keep comparator strict and deterministic. + return lhSpell.first < rhSpell.first; } - p1 += skill1 * 500; - p2 += skill2 * 500; + uint32 lhsKey = lhSpellInfo->SchoolMask; + uint32 rhsKey = rhSpellInfo->SchoolMask; - p1 += skillValue1; - p2 += skillValue2; - - if (p1 == p2) + if (lhsKey == rhsKey) { - return strcmp(si1->SpellName[0], si2->SpellName[0]) > 0; - } + // Defensive check: if DBC data is broken and spell names are nullptr, + // fall back to id ordering instead of risking a crash in std::strcmp. + if (!lhSpellInfo->SpellName[0] || !rhSpellInfo->SpellName[0]) + return lhSpell.first < rhSpell.first; - return p1 > p2; + return std::strcmp(lhSpellInfo->SpellName[0], rhSpellInfo->SpellName[0]) > 0; + } + return lhsKey > rhsKey; } std::vector> ListSpellsAction::GetSpellList(std::string filter) { - if (skillSpells.empty()) - { - for (uint32 j = 0; j < sSkillLineAbilityStore.GetNumRows(); ++j) - { - if (SkillLineAbilityEntry const* skillLine = sSkillLineAbilityStore.LookupEntry(j)) - skillSpells[skillLine->Spell] = skillLine; - } - } - - if (vendorItems.empty()) - { - QueryResult results = WorldDatabase.Query("SELECT item FROM npc_vendor WHERE maxcount = 0"); - if (results) - { - do - { - Field* fields = results->Fetch(); - int32 entry = fields[0].Get(); - if (entry <= 0) - continue; - - vendorItems.insert(entry); - } while (results->NextRow()); - } - } - - std::ostringstream posOut; - std::ostringstream negOut; - uint32 skill = 0; std::vector ss = split(filter, ' '); @@ -99,13 +52,15 @@ std::vector> ListSpellsAction::GetSpellList(std:: skill = chat->parseSkill(ss[0]); if (skill != SKILL_NONE) { - filter = ss.size() > 1 ? filter = ss[1] : ""; + filter = ss.size() > 1 ? ss[1] : ""; } - if (ss[0] == "first" && ss[1] == "aid") + // Guard access to ss[1]/ss[2] to avoid out-of-bounds + // when the player only types "first" without "aid". + if (ss[0] == "first" && ss.size() > 1 && ss[1] == "aid") { skill = SKILL_FIRST_AID; - filter = ss.size() > 2 ? filter = ss[2] : ""; + filter = ss.size() > 2 ? ss[2] : ""; } } @@ -115,26 +70,57 @@ std::vector> ListSpellsAction::GetSpellList(std:: uint32 minLevel = 0; uint32 maxLevel = 0; - if (filter.find("-") != std::string::npos) + if (filter.find('-') != std::string::npos) { std::vector ff = split(filter, '-'); - minLevel = atoi(ff[0].c_str()); - maxLevel = atoi(ff[1].c_str()); - filter = ""; + if (ff.size() >= 2) + { + minLevel = std::atoi(ff[0].c_str()); + maxLevel = std::atoi(ff[1].c_str()); + if (minLevel > maxLevel) + std::swap(minLevel, maxLevel); + } + filter.clear(); } - bool craftableOnly = false; - if (filter.find("+") != std::string::npos) + bool canCraftNow = false; + if (filter.find('+') != std::string::npos) { - craftableOnly = true; + canCraftNow = true; + + // Support "+" syntax (e.g. "spells +tailoring" or "spells tailoring+"). + // If no explicit skill was detected yet, try to parse the filter (without '+') + // as a profession/skill name so that craftable-only filters still work with skills. + if (skill == SKILL_NONE) + { + std::string skillFilter = filter; + + // Remove '+' before trying to interpret the first token as a skill name. + skillFilter.erase(remove(skillFilter.begin(), skillFilter.end(), '+'), skillFilter.end()); + + std::vector skillTokens = split(skillFilter, ' '); + if (!skillTokens.empty()) + { + uint32 parsedSkill = chat->parseSkill(skillTokens[0]); + if (parsedSkill != SKILL_NONE) + { + skill = parsedSkill; + + // Any remaining text after the skill token becomes the "name" filter + // (e.g. "spells +tailoring cloth" -> skill = tailoring, filter = "cloth"). + filter = skillTokens.size() > 1 ? skillTokens[1] : ""; + } + } + } + // Finally remove '+' from the filter that will be used for name/range parsing. filter.erase(remove(filter.begin(), filter.end(), '+'), filter.end()); } uint32 slot = chat->parseSlot(filter); if (slot != EQUIPMENT_SLOT_END) - filter = ""; + filter.clear(); - std::vector> spells; + std::vector spells; for (PlayerSpellMap::iterator itr = bot->GetSpellMap().begin(); itr != bot->GetSpellMap().end(); ++itr) { if (itr->second->State == PLAYERSPELL_REMOVED || !itr->second->Active) @@ -150,7 +136,7 @@ std::vector> ListSpellsAction::GetSpellList(std:: if (spellInfo->IsPassive()) continue; - SkillLineAbilityEntry const* skillLine = skillSpells[itr->first]; + SkillLineAbilityEntry const* skillLine = sPlayerbotSpellCache->GetSkillLine(itr->first); if (skill != SKILL_NONE && (!skillLine || skillLine->SkillLine != skill)) continue; @@ -162,7 +148,7 @@ std::vector> ListSpellsAction::GetSpellList(std:: continue; bool first = true; - int32 craftCount = -1; + int32 craftsPossible = -1; std::ostringstream materials; for (uint32 x = 0; x < MAX_SPELL_REAGENTS; ++x) { @@ -189,12 +175,12 @@ std::vector> ListSpellsAction::GetSpellList(std:: FindItemByIdVisitor visitor(itemid); uint32 reagentsInInventory = InventoryAction::GetItemCount(&visitor); - bool buyable = (vendorItems.find(itemid) != vendorItems.end()); + bool buyable = sPlayerbotSpellCache->IsItemBuyable(itemid); if (!buyable) { uint32 craftable = reagentsInInventory / reagentsRequired; - if (craftCount < 0 || craftCount > craftable) - craftCount = craftable; + if (craftsPossible < 0 || craftsPossible > static_cast(craftable)) + craftsPossible = static_cast(craftable); } if (reagentsInInventory) @@ -205,8 +191,8 @@ std::vector> ListSpellsAction::GetSpellList(std:: } } - if (craftCount < 0) - craftCount = 0; + if (craftsPossible < 0) + craftsPossible = 0; std::ostringstream out; bool filtered = false; @@ -218,8 +204,8 @@ std::vector> ListSpellsAction::GetSpellList(std:: { if (ItemTemplate const* proto = sObjectMgr->GetItemTemplate(spellInfo->Effects[i].ItemType)) { - if (craftCount) - out << "|cffffff00(x" << craftCount << ")|r "; + if (craftsPossible) + out << "|cffffff00(x" << craftsPossible << ")|r "; out << chat->FormatItem(proto); @@ -246,7 +232,7 @@ std::vector> ListSpellsAction::GetSpellList(std:: if (filtered) continue; - if (craftableOnly && !craftCount) + if (canCraftNow && !craftsPossible) continue; out << materials.str(); @@ -275,10 +261,9 @@ std::vector> ListSpellsAction::GetSpellList(std:: continue; if (itr->first == 0) - { LOG_ERROR("playerbots", "?! {}", itr->first); - } - spells.push_back(std::pair(itr->first, out.str())); + + spells.emplace_back(itr->first, out.str()); alreadySeenList += spellInfo->SpellName[0]; alreadySeenList += ","; } @@ -294,25 +279,28 @@ bool ListSpellsAction::Execute(Event event) std::string const filter = event.getParam(); - std::vector> spells = GetSpellList(filter); + std::vector spells = GetSpellList(filter); + + if (spells.empty()) + { + // CHANGE: Give early feedback when no spells match the filter. + botAI->TellMaster("No spells found."); + return true; + } botAI->TellMaster("=== Spells ==="); std::sort(spells.begin(), spells.end(), CompareSpells); - uint32 count = 0; - for (std::vector>::iterator i = spells.begin(); i != spells.end(); ++i) - { + // CHANGE: Send the full spell list again so client-side addons + // (e.g. Multibot / Unbot) can reconstruct the + // complete spellbook for configuration. The heavy part that caused + // freezes before was the old CompareSpells implementation scanning + // the entire SkillLineAbility DBC on every comparison. With the new + // cheap comparator above, sending all lines here is safe and keeps + // behaviour compatible with existing addons. + for (std::vector::const_iterator i = spells.begin(); i != spells.end(); ++i) botAI->TellMasterNoFacing(i->second); - // if (++count >= 50) - // { - // std::ostringstream msg; - // msg << (spells.size() - 50) << " more..."; - // botAI->TellMasterNoFacing(msg.str()); - // break; - // } - } - return true; -} +} \ No newline at end of file diff --git a/src/strategy/actions/ListSpellsAction.h b/src/strategy/actions/ListSpellsAction.h index 5fa44bc1..9e674bf3 100644 --- a/src/strategy/actions/ListSpellsAction.h +++ b/src/strategy/actions/ListSpellsAction.h @@ -18,9 +18,8 @@ public: bool Execute(Event event) override; virtual std::vector> GetSpellList(std::string filter = ""); -private: - static std::map skillSpells; - static std::set vendorItems; + static void InitSpellCaches(); + }; #endif