From 826887133d01405e1021b731f19d213bdabc0772 Mon Sep 17 00:00:00 2001 From: Crow Date: Sat, 9 May 2026 00:40:35 -0500 Subject: [PATCH] Exclude Invalid Weapons from Shaman Enchants & Refactor Temporary Enchant Spellcasting (#2345) ## Pull Request Description Fix for issue #2343 I excluded the MISC and FISHING POLE weapon subclasses from weapon enchants. MISC includes the entry profession "weapons" (skinning knife, mining pick, blacksmithing hammer, arclight spanner) and some other crap that I suspect is not enchantable, but even if it is there's no good reason to do so (like Brewfest steins). The subclass doesn't include weapons that can be used for professions but you might actually want to use for fighting (like Finkle's Skinner). To clean things up overall, I removed the intermediate class CastEnchantItemAction between CastSpellAction and CastEnchantItemMainHandAction and CastEnchantItemOffHandAction. CastEnchantItemAction is not doing anything helpful that can't easily be replicated in the MH/OH classes, and I can't think of any future reason for keeping CastEnchantItemAction. I also brought the CanCastSpell check into the MH/OH classes--previously it just wasn't run for the weapon enchant spells, and I can't think of any good reason why it shouldn't be. I also added Execute functions to both CastEnchantItemMainHandAction and CastEnchantItemOffHandAction so they actually directly cast the enchant on the specified hand instead of running through CastSpellAction's Execute (and thus going through item for spell). I wasn't having problems with the wrong hand being applied under the prior approach, but this is a more direct and better approach anyway. Other changes are just formatting. ## Feature Evaluation - Describe the **minimum logic** required to achieve the intended behavior. - Describe the **processing cost** when this logic executes across many bots. The new path is very similar to the old one but just adds a check that is common to all spells and early returns to avoid invalid results. ## How to Test the Changes 1. Log into a Shaman and activate selfbot 2. Check to make sure the correct enchantments are applied (e.g., MH Windfury and OH Flametongue for a dual-wielding Enhancement Shaman) 3. Equip a profession weapon such as a skinning knife and make sure the Shaman does not attempt to enchant it ## Impact Assessment - Does this change increase per-bot/per-tick processing or risk scaling poorly with thousands of bots? - - [x] No, not at all - - [ ] Minimal impact (**explain below**) - - [ ] Moderate impact (**explain below**) - Does this change modify default bot behavior? - - [ ] No - - [x] Yes (**explain why**) This just stops Shamans from trying to enchant stuff that they can't. - Does this change add new decision branches or increase maintenance complexity? - - [x] No - - [ ] Yes (**explain below**) ## AI Assistance Was AI assistance used while working on this change? - - [ ] No - - [x] Yes (**explain below**) I kicked around some ideas with GPT-5.4 with respect to the refactoring aspect of the PR after I had fixed the bug. ## Final Checklist - - [x] Stability is not compromised. - - [x] Performance impact is understood, tested, and acceptable. - - [x] Added logic complexity is justified and explained. - - [x] Any new bot dialogue lines are translated. - - [x] Documentation updated if needed (Conf comments, WiKi commands). ## Notes for Reviewers --- src/Ai/Base/Actions/GenericSpellActions.cpp | 83 ++++++++++----------- src/Ai/Base/Actions/GenericSpellActions.h | 17 ++--- 2 files changed, 46 insertions(+), 54 deletions(-) diff --git a/src/Ai/Base/Actions/GenericSpellActions.cpp b/src/Ai/Base/Actions/GenericSpellActions.cpp index c81aca214..587862a29 100644 --- a/src/Ai/Base/Actions/GenericSpellActions.cpp +++ b/src/Ai/Base/Actions/GenericSpellActions.cpp @@ -17,7 +17,7 @@ #include "WorldPacket.h" #include "Group.h" #include "Chat.h" -#include "Ai/Base/Util/GenericBuffUtils.h" +#include "GenericBuffUtils.h" #include "PlayerbotAI.h" using ai::buff::MakeAuraQualifierForBuff; @@ -134,7 +134,8 @@ bool CastSpellAction::isPossible() return botAI->CanCastSpell(spell, GetTarget()); } -CastMeleeSpellAction::CastMeleeSpellAction(PlayerbotAI* botAI, std::string const spell) : CastSpellAction(botAI, spell) +CastMeleeSpellAction::CastMeleeSpellAction( + PlayerbotAI* botAI, std::string const spell) : CastSpellAction(botAI, spell) { range = ATTACK_DISTANCE; } @@ -182,56 +183,47 @@ bool CastAuraSpellAction::isUseful() return false; } -CastEnchantItemAction::CastEnchantItemAction(PlayerbotAI* botAI, std::string const spell) - : CastSpellAction(botAI, spell) +CastEnchantItemMainHandAction::CastEnchantItemMainHandAction( + PlayerbotAI* botAI, std::string const spell) : CastSpellAction(botAI, spell) {} + +bool CastEnchantItemMainHandAction::Execute(Event /*event*/) { - range = botAI->GetRange("spell"); + Item* item = bot->GetItemByPos(INVENTORY_SLOT_BAG_0, EQUIPMENT_SLOT_MAINHAND); + return item && botAI->CastSpell(spell, bot, item); } -bool CastEnchantItemAction::isPossible() -{ - // if (!CastSpellAction::isPossible()) - // { - // botAI->TellMasterNoFacing("Impossible: " + spell); - // return false; - // } - - uint32 spellId = AI_VALUE2(uint32, "spell id", spell); - - // bool ok = AI_VALUE2(Item*, "item for spell", spellId); - // Item* item = AI_VALUE2(Item*, "item for spell", spellId); - // botAI->TellMasterNoFacing("spell: " + spell + ", spell id: " + std::to_string(spellId) + " item for spell: " + - // std::to_string(ok)); - return spellId && AI_VALUE2(Item*, "item for spell", spellId); -} - -CastEnchantItemMainHandAction::CastEnchantItemMainHandAction(PlayerbotAI* botAI, std::string const spell) - : CastEnchantItemAction(botAI, spell) {} - bool CastEnchantItemMainHandAction::isPossible() { - if (!CastEnchantItemAction::isPossible()) - return false; - Item* item = bot->GetItemByPos(INVENTORY_SLOT_BAG_0, EQUIPMENT_SLOT_MAINHAND); - return item && !item->GetEnchantmentId(TEMP_ENCHANTMENT_SLOT) && - item->GetTemplate()->Class == ITEM_CLASS_WEAPON; + if (!item || item->GetTemplate()->SubClass == ITEM_SUBCLASS_WEAPON_MISC || + item->GetTemplate()->SubClass == ITEM_SUBCLASS_WEAPON_FISHING_POLE || + item->GetEnchantmentId(TEMP_ENCHANTMENT_SLOT)) + { + return false; + } + + return botAI->CanCastSpell(spell, bot, item); } -CastEnchantItemOffHandAction::CastEnchantItemOffHandAction(PlayerbotAI* botAI, std::string const spell) - : CastEnchantItemAction(botAI, spell) {} +CastEnchantItemOffHandAction::CastEnchantItemOffHandAction( + PlayerbotAI* botAI, std::string const spell) : CastSpellAction(botAI, spell) {} + +bool CastEnchantItemOffHandAction::Execute(Event /*event*/) +{ + Item* item = bot->GetItemByPos(INVENTORY_SLOT_BAG_0, EQUIPMENT_SLOT_OFFHAND); + return item && botAI->CastSpell(spell, bot, item); +} bool CastEnchantItemOffHandAction::isPossible() { - if (!CastEnchantItemAction::isPossible()) - return false; - Item* item = bot->GetItemByPos(INVENTORY_SLOT_BAG_0, EQUIPMENT_SLOT_OFFHAND); - if (!item || item->GetEnchantmentId(TEMP_ENCHANTMENT_SLOT)) + if (!item || item->GetTemplate()->SubClass == ITEM_SUBCLASS_WEAPON_MISC || + item->GetEnchantmentId(TEMP_ENCHANTMENT_SLOT)) + { return false; + } - uint32 invType = item->GetTemplate()->InventoryType; - return invType == INVTYPE_WEAPON || invType == INVTYPE_WEAPONOFFHAND; + return botAI->CanCastSpell(spell, bot, item); } CastHealingSpellAction::CastHealingSpellAction(PlayerbotAI* botAI, std::string const spell, uint8 estAmount, @@ -245,7 +237,8 @@ bool CastHealingSpellAction::isUseful() { return CastAuraSpellAction::isUseful() bool CastAoeHealSpellAction::isUseful() { return CastSpellAction::isUseful(); } -CastCureSpellAction::CastCureSpellAction(PlayerbotAI* botAI, std::string const spell) : CastSpellAction(botAI, spell) +CastCureSpellAction::CastCureSpellAction( + PlayerbotAI* botAI, std::string const spell) : CastSpellAction(botAI, spell) { range = botAI->GetRange("heal"); } @@ -267,13 +260,15 @@ bool BuffOnPartyAction::Execute(Event /*event*/) std::string castName = spell; // default = mono auto SendGroupRP = ai::chat::MakeGroupAnnouncer(bot); - castName = ai::buff::UpgradeToGroupIfAppropriate(bot, botAI, castName, /*announceOnMissing=*/true, SendGroupRP); + castName = ai::buff::UpgradeToGroupIfAppropriate( + bot, botAI, castName, /*announceOnMissing=*/true, SendGroupRP); return botAI->CastSpell(castName, GetTarget()); } // End greater buff fix -CastShootAction::CastShootAction(PlayerbotAI* botAI) : CastSpellAction(botAI, "shoot"), shootSpellId(0) +CastShootAction::CastShootAction( + PlayerbotAI* botAI) : CastSpellAction(botAI, "shoot"), shootSpellId(0) { if (Item* const pItem = bot->GetItemByPos(INVENTORY_SLOT_BAG_0, EQUIPMENT_SLOT_RANGED)) { @@ -327,7 +322,8 @@ Value* CastDebuffSpellOnMeleeAttackerAction::GetTargetValue() return context->GetValue("melee attacker without aura", spell); } -CastBuffSpellAction::CastBuffSpellAction(PlayerbotAI* botAI, std::string const spell, bool checkIsOwner, uint32 beforeDuration) +CastBuffSpellAction::CastBuffSpellAction( + PlayerbotAI* botAI, std::string const spell, bool checkIsOwner, uint32 beforeDuration) : CastAuraSpellAction(botAI, spell, checkIsOwner, false, beforeDuration) { range = botAI->GetRange("spell"); @@ -448,7 +444,8 @@ bool UseTrinketAction::UseTrinket(Item* item) uint32 spellId = 0; for (uint8 i = 0; i < MAX_ITEM_PROTO_SPELLS; ++i) { - if (item->GetTemplate()->Spells[i].SpellId > 0 && item->GetTemplate()->Spells[i].SpellTrigger == ITEM_SPELLTRIGGER_ON_USE) + if (item->GetTemplate()->Spells[i].SpellId > 0 && + item->GetTemplate()->Spells[i].SpellTrigger == ITEM_SPELLTRIGGER_ON_USE) { spellId = item->GetTemplate()->Spells[i].SpellId; const SpellInfo* spellInfo = sSpellMgr->GetSpellInfo(spellId); diff --git a/src/Ai/Base/Actions/GenericSpellActions.h b/src/Ai/Base/Actions/GenericSpellActions.h index e9dacb7d2..b87bd0a1f 100644 --- a/src/Ai/Base/Actions/GenericSpellActions.h +++ b/src/Ai/Base/Actions/GenericSpellActions.h @@ -121,26 +121,21 @@ public: std::string const GetTargetName() override { return "self target"; } }; -class CastEnchantItemAction : public CastSpellAction -{ -public: - CastEnchantItemAction(PlayerbotAI* botAI, std::string const spell); - - bool isPossible() override; - std::string const GetTargetName() override { return "self target"; } -}; - -class CastEnchantItemMainHandAction : public CastEnchantItemAction +class CastEnchantItemMainHandAction : public CastSpellAction { public: CastEnchantItemMainHandAction(PlayerbotAI* botAI, std::string const spell); + std::string const GetTargetName() override { return "self target"; } + bool Execute(Event event) override; bool isPossible() override; }; -class CastEnchantItemOffHandAction : public CastEnchantItemAction +class CastEnchantItemOffHandAction : public CastSpellAction { public: CastEnchantItemOffHandAction(PlayerbotAI* botAI, std::string const spell); + std::string const GetTargetName() override { return "self target"; } + bool Execute(Event event) override; bool isPossible() override; };