From b6f882886da3c14fa2a1678288ce0d3975ea4faa Mon Sep 17 00:00:00 2001 From: Keleborn <22352763+Celandriel@users.noreply.github.com> Date: Fri, 19 Dec 2025 14:08:01 -0800 Subject: [PATCH] FixListSpellsWithCache (#1931) This is based off of Wishmasters rewrite of spell PR. #1912 and #1843, and partial #1918 I created a new cache singleton with the required code to reference as needed. I clarified some variable names for additional clarity in how they functioned. This requires a wiki update to better describe the functionality that was already defined in code. Commands Spells - Returns all spells Spells Returns only the spells in that profession + Returns only the recipies that the bot has materials to craft `x - y` Craftable items within those levels e.g. Chest, returns craftable items within that slot. Its messy whether what combinations work for commands, but fixing that will come when bot professions are enabled. Edit: To test you can teach a bot various professions by going to a trainer with them. Using gm command .setskill you increase their skill level and with maintenance teach the commands. From wishmasters PR he detailed various commands to test spells spells first aid spells tailoring spells 20-40 spells +tailoring spells head --------- Co-authored-by: Wishmaster117 <140754794+Wishmaster117@users.noreply.github.com> --- src/Playerbots.cpp | 4 + src/database/PlayerbotSpellCache.cpp | 45 +++++ src/database/PlayerbotSpellCache.h | 34 ++++ src/strategy/actions/ListSpellsAction.cpp | 208 ++++++++++------------ src/strategy/actions/ListSpellsAction.h | 5 +- 5 files changed, 183 insertions(+), 113 deletions(-) create mode 100644 src/database/PlayerbotSpellCache.cpp create mode 100644 src/database/PlayerbotSpellCache.h 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