From 32d10080a423ffdccbcc7652cb579602ca68a07f Mon Sep 17 00:00:00 2001 From: Crow Date: Sat, 30 May 2026 13:12:34 -0500 Subject: [PATCH] Improve bot trinket usage and fix related bugs (#2425) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Pull Request Description This PR makes three changes to UseTrinketAction: 1. It adds health and mana gating (based on existing config thresholds) for bots to activate mana recovery, mana efficiency, and defensive trinkets. The thresholds are mediumMana (default 40%) for mana recovery trinkets, highMana (default 65%) for mana efficiency trinkets, and lowHealth (default 45%) for defensive trinkets. 2. It removes the old overinclusive procflag/specproc gate introduced by PR 1385, which prevents bots from using dozens of valid trinkets, including some extremely powerful ones (such as Skull of Gul’dan, the iconic TBC expansion BiS trinket for all casters), and replaces it with a narrower exclusion that still addresses the original issue. - Regarding PR 1385, focusing on liyunfan’s post specifically, I interpet the issue to be relating to trinkets where the item data is screwed up such that a passive effect was implemented as an on-use spell. I had AI do a scan, and it seems that issue impacts only 2 trinkets in the game, Oracle Talisman of Ablution and Frenzyheart Insignia of Fury, and specifically only the versions of those items that are not legitimately obtainable (unclear why they exist at all). I’ve excluded those trinkets from bots via the config now, and this PR maintains the guards against those trinkets regardless but does so in a narrower fashion. PR 1385's approach of using ProcFlags != 0 (i.e., excluding all on-use trinkets with non-zero ProcFlags) works only if proc metadata can be used to distinguish between active/passive trinkets, and that’s not even close to being the case. AI came up with 44 false positives, including many significant trinkets beyond Skull of Gul’dan such as the ZG Hakkar quest trinkets, Badge of the Swarmguard, Petrified Scarab, Scarab Brooch, Eye of the Dead, Essence of the Martyr, Abacus of Violent Odds, Ribbon of Sacrifice, and Pendant of the Violet Eye (and I’m not mentioning WotLK trinkets only because I don’t know anything about what is relevant for that expansion). 3. It fixes an issue where bots were not respecting trinket cooldowns in some cases. This resulted because trinkets with shared cooldown categories (i.e., those that are not stackable) would substitute the individual trinket cooldowns with shared category cooldowns, which let bots spam usages of trinkets, by tracking per-item and per-category trinket cooldowns locally. The root of this is based in AC; I don't know if it should be considered a bug or not, but regardless it doesn't impact players, presumably because cooldowns are enforced on the client side. - Here’s an illustration to explain the issue in practice. Skull of Gul’dan has a 240s cooldown and Shifting Naaru Sliver has a 180s cooldown, and they share a cooldown category so their usages cannot be stacked. The shared cooldown matches the length of the on-use effect (so 20s for Skull and 15s for Sliver). The below is what can happen without this PR (and is what I observed in testing). - t = 0s: Shifting Naaru Sliver used, writes its 90s personal cooldown and 15s shared category cooldown. - t = 15s: Skull of Gul’dan used, writes its 120s personal cooldown and 20s shared category cooldown. Skull’s shared category cooldown overwrites Sliver’s spell-cooldown entry, giving Sliver 20s left on its personal cooldown instead of 75s. - t = 35s: Sliver incorrectly appears ready and can be used again (55s sooner than should be possible). Then Sliver’s shared category cooldown in turn overwrites Skull’s longer personal cooldown. - t = 50s: Skull incorrectly appears ready and can be used again (85s sooner than should be possible). - Repeat. ## Feature Evaluation - Describe the **minimum logic** required to achieve the intended behavior. - Describe the **processing cost** when this logic executes across many bots. The logic runs through the existing trinket use code. Specifically: - Helpers are used to classify on-use trinket effects into mana restoration, mana efficiency, and defensive/tank categories using spell-effect checks - Existing configured mana and health thresholds are applied to those trinkets - The old procflag gate is replaced with a one-time cached set of mixed ON_USE/ON_EQUIP trinket spell ids - Two small cooldown maps per bot are tracked in UseTrinketAction: one for item cooldowns and one for shared category cooldowns ## How to Test the Changes I did all of these things, but verification is always good. Overall, I think it is worth running with this PR merged into test-staging if/when that happens and keeping an eye on overall performance. 1. Equip a bot with a trinket referenced above, such as Skull of Gul'dan, and confirm it is now used in combat. 2. Test mana-based classes with mana recovery and mana-efficiency trinkets and confirm they are used only when below the applicable configured mana threshold. An easy one to check is Glimmering Naaru Sliver because it is a channel. 3. Test defensive on-use trinkets and confirm they are used only when health is below the configured low-health threshold. Something like Shadowmoon Insignia, which increases maximum health, is pretty obvious. 4. Confirm that the error versions of Oracle/Frenzyheart don’t stack auras on bots (i.e., the bug addressed in PR 1385 has not returned). You can do this by having a bot kill mobs and check .listauras, though I checked through logging in the code because auras are noisy as hell. 5. Equip a bot with two trinkets that have shared cooldowns. I used Skull of Gul’dan and Shifting Naaru Sliver. Go fight a mostly tank-and-spank boss, such as Gruul. Use an add-on like Skada that tracks buffs. You should see that before this PR, bots will use the trinkets multiple times in one cooldown period, and after, they observe the actual cooldowns. ## Impact Assessment - Does this change increase per-bot/per-tick processing or risk scaling poorly with thousands of bots? - - [ ] No, not at all - - [x] Minimal impact (**explain below**) - - [ ] Moderate impact (**explain below**) Any additional impact is confined to UseTrinketAction. The exclusion of the busted trinkets uses a cache that is built once per server process followed by constant-time lookups. The per-bot trinket cooldown maps also use constant-time lookups and store only a few timestamp entries per bot. - Does this change modify default bot behavior? - - [x] No - - [ ] Yes (**explain why**) - Does this change add new decision branches or increase maintenance complexity? - - [ ] No - - [x] Yes (**explain below**) Sort of--trinkets previously didn't have any consideration for effects with respect to usage so that is new. I think it is necessary though to have half-decent bot trinket usage, and there could be further refinement for how bots decide to use trinkets based on this structure. ## AI Assistance Was AI assistance used while working on this change? - - [ ] No - - [x] Yes (**explain below**) GPT-5.4 was used for investigation and root-cause analysis and assistance with drafting. All resulting code and the PR rationale was validated through in-game testing. ## 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 Bots also suck at using DPS trinkets properly, but I don't think there's a simple way to address that unlike with mana recovery or defensive trinkets. So that's to consider another day. --------- Co-authored-by: Keleborn <22352763+Celandriel@users.noreply.github.com> --- conf/playerbots.conf.dist | 5 +- src/Ai/Base/Actions/GenericSpellActions.cpp | 216 ++++++++++++++++++-- src/Ai/Base/Actions/GenericSpellActions.h | 4 + src/PlayerbotAIConfig.cpp | 2 +- 4 files changed, 209 insertions(+), 18 deletions(-) diff --git a/conf/playerbots.conf.dist b/conf/playerbots.conf.dist index 886cc8cd5..c12049d66 100644 --- a/conf/playerbots.conf.dist +++ b/conf/playerbots.conf.dist @@ -865,8 +865,9 @@ AiPlayerbot.LimitGearExpansion = 1 AiPlayerbot.RandomGearLoweringChance = 0 # Unobtainable or unusable items (comma-separated list of item IDs) -# Default: Chilton Wand (12468), Totem of the Earthen Ring (46978) -AiPlayerbot.UnobtainableItems = 12468,46978 +# Defaults: Chilton Wand (12468), Frenzyheart Insignia of Fury test/on-use row (44869), +# Oracle Talisman of Ablution test/on-use row (44870), Totem of the Earthen Ring (46978) +AiPlayerbot.UnobtainableItems = 12468,44869,44870,46978 # Randombots check player's gearscore level and deny the group invitation if it's too low # Default: 0 (disabled) diff --git a/src/Ai/Base/Actions/GenericSpellActions.cpp b/src/Ai/Base/Actions/GenericSpellActions.cpp index d4b54f16f..657fda7cd 100644 --- a/src/Ai/Base/Actions/GenericSpellActions.cpp +++ b/src/Ai/Base/Actions/GenericSpellActions.cpp @@ -6,6 +6,7 @@ #include "GenericSpellActions.h" #include +#include #include "Event.h" #include "ItemTemplate.h" @@ -23,6 +24,116 @@ using ai::buff::MakeAuraQualifierForBuff; using ai::spell::HasSpellOrCategoryCooldown; +namespace +{ + std::unordered_set const& GetMixedTriggerTrinketSpellIds() + { + static std::unordered_set const mixedTriggerSpellIds = []() + { + std::unordered_set onUseSpellIds; + std::unordered_set onEquipSpellIds; + std::unordered_set mixedSpellIds; + + auto const* itemTemplates = sObjectMgr->GetItemTemplateStore(); + if (!itemTemplates) + return mixedSpellIds; + + auto const markSpellId = [&](int32 spellId, uint8 spellTrigger) + { + if (spellId <= 0) + return; + + if (spellTrigger == ITEM_SPELLTRIGGER_ON_USE) + { + if (onEquipSpellIds.find(spellId) != onEquipSpellIds.end()) + mixedSpellIds.insert(spellId); + + onUseSpellIds.insert(spellId); + } + else if (spellTrigger == ITEM_SPELLTRIGGER_ON_EQUIP) + { + if (onUseSpellIds.find(spellId) != onUseSpellIds.end()) + mixedSpellIds.insert(spellId); + + onEquipSpellIds.insert(spellId); + } + }; + + for (auto const& itr : *itemTemplates) + { + ItemTemplate const& proto = itr.second; + if (proto.InventoryType != INVTYPE_TRINKET) + continue; + + for (uint8 spellIndex = 0; spellIndex < MAX_ITEM_PROTO_SPELLS; ++spellIndex) + { + auto const& spellData = proto.Spells[spellIndex]; + markSpellId(spellData.SpellId, spellData.SpellTrigger); + } + } + + return mixedSpellIds; + }(); + + return mixedTriggerSpellIds; + } + + bool IsManaRestoreEffect(SpellEffectInfo const& effectInfo) + { + return (effectInfo.Effect == SPELL_EFFECT_ENERGIZE && + effectInfo.MiscValue == POWER_MANA) || + (effectInfo.Effect == SPELL_EFFECT_APPLY_AURA && + effectInfo.ApplyAuraName == SPELL_AURA_PERIODIC_ENERGIZE && + effectInfo.MiscValue == POWER_MANA); + } + + bool IsManaEfficiencyEffect(SpellEffectInfo const& effectInfo) + { + return effectInfo.Effect == SPELL_EFFECT_APPLY_AURA && + (((effectInfo.ApplyAuraName == SPELL_AURA_MOD_POWER_REGEN || + effectInfo.ApplyAuraName == SPELL_AURA_MOD_POWER_REGEN_PERCENT) && + effectInfo.MiscValue == POWER_MANA) || + effectInfo.ApplyAuraName == SPELL_AURA_MOD_POWER_COST_SCHOOL || + effectInfo.ApplyAuraName == SPELL_AURA_MOD_POWER_COST_SCHOOL_PCT || + effectInfo.ApplyAuraName == SPELL_AURA_MOD_MANA_REGEN_INTERRUPT); + } + + bool IsDefensiveTankEffect(SpellEffectInfo const& effectInfo) + { + if (effectInfo.Effect != SPELL_EFFECT_APPLY_AURA) + return false; + + uint32 const tankRatingsMask = + (1u << CR_DEFENSE_SKILL) | + (1u << CR_DODGE) | + (1u << CR_PARRY) | + (1u << CR_BLOCK) | + (1u << CR_HIT_TAKEN_MELEE) | + (1u << CR_HIT_TAKEN_RANGED) | + (1u << CR_HIT_TAKEN_SPELL) | + (1u << CR_CRIT_TAKEN_MELEE) | + (1u << CR_CRIT_TAKEN_RANGED) | + (1u << CR_CRIT_TAKEN_SPELL); + + switch (effectInfo.ApplyAuraName) + { + case SPELL_AURA_MOD_RESISTANCE: + return (effectInfo.MiscValue & SPELL_SCHOOL_MASK_NORMAL) != 0; + case SPELL_AURA_MOD_RATING: + return (effectInfo.MiscValue & tankRatingsMask) != 0; + case SPELL_AURA_MOD_INCREASE_HEALTH: + case SPELL_AURA_MOD_INCREASE_HEALTH_PERCENT: + case SPELL_AURA_MOD_PARRY_PERCENT: + case SPELL_AURA_MOD_DODGE_PERCENT: + case SPELL_AURA_MOD_BLOCK_PERCENT: + case SPELL_AURA_MOD_DAMAGE_PERCENT_TAKEN: + return true; + default: + return false; + } + } +} + CastSpellAction::CastSpellAction(PlayerbotAI* botAI, std::string const spell) : Action(botAI, spell), range(botAI->GetRange("spell")), spell(spell) {} @@ -429,52 +540,109 @@ bool UseTrinketAction::UseTrinket(Item* item) uint8 bagIndex = item->GetBagSlot(); uint8 slot = item->GetSlot(); - // uint8 spell_index = 0; //not used, line marked for removal. uint8 cast_count = 1; ObjectGuid item_guid = item->GetGUID(); uint32 glyphIndex = 0; uint8 castFlags = 0; uint32 targetFlag = TARGET_FLAG_NONE; uint32 spellId = 0; + int32 itemSpellCooldown = 0; + uint32 itemSpellCategory = 0; + int32 itemSpellCategoryCooldown = 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) { spellId = item->GetTemplate()->Spells[i].SpellId; - const SpellInfo* spellInfo = sSpellMgr->GetSpellInfo(spellId); + itemSpellCooldown = item->GetTemplate()->Spells[i].SpellCooldown; + itemSpellCategory = item->GetTemplate()->Spells[i].SpellCategory; + itemSpellCategoryCooldown = item->GetTemplate()->Spells[i].SpellCategoryCooldown; + uint64 const itemCooldownKey = (static_cast(item->GetEntry()) << 32) | spellId; + uint32 const now = getMSTime(); + if (itemSpellCooldown > 0) + { + auto const itemCooldownItr = trinketItemCooldownExpiries.find(itemCooldownKey); + if (itemCooldownItr != trinketItemCooldownExpiries.end()) + { + if (itemCooldownItr->second > now) + return false; + + trinketItemCooldownExpiries.erase(itemCooldownItr); + } + } + + if (itemSpellCategory && itemSpellCategoryCooldown > 0) + { + auto const categoryCooldownItr = trinketCategoryCooldownExpiries.find(itemSpellCategory); + if (categoryCooldownItr != trinketCategoryCooldownExpiries.end()) + { + if (categoryCooldownItr->second > now) + return false; + + trinketCategoryCooldownExpiries.erase(categoryCooldownItr); + } + } + + const SpellInfo* spellInfo = sSpellMgr->GetSpellInfo(spellId); if (!spellInfo || !spellInfo->IsPositive()) return false; bool applyAura = false; + bool restoresMana = false; + bool improvesManaEfficiency = false; + bool defensiveTankEffect = false; for (int i = 0; i < MAX_SPELL_EFFECTS; i++) { const SpellEffectInfo& effectInfo = spellInfo->Effects[i]; if (effectInfo.Effect == SPELL_EFFECT_APPLY_AURA) - { applyAura = true; - break; + + restoresMana = restoresMana || IsManaRestoreEffect(effectInfo); + improvesManaEfficiency = improvesManaEfficiency || IsManaEfficiencyEffect(effectInfo); + defensiveTankEffect = defensiveTankEffect || IsDefensiveTankEffect(effectInfo); + } + + if (!applyAura && !restoresMana) + return false; + + if (restoresMana || improvesManaEfficiency) + { + if (!AI_VALUE2(bool, "has mana", "self target")) + return false; + + uint8 const manaPct = AI_VALUE2(uint8, "mana", "self target"); + if ((restoresMana && manaPct >= sPlayerbotAIConfig.mediumMana) || + manaPct >= sPlayerbotAIConfig.highMana) + { + return false; } } - if (!applyAura) + if (defensiveTankEffect) + { + uint8 const healthPct = AI_VALUE2(uint8, "health", "self target"); + if (healthPct > sPlayerbotAIConfig.lowHealth) + return false; + } + + auto const& mixedTriggerTrinketSpellIds = GetMixedTriggerTrinketSpellIds(); + // Exclude trinkets that expose the same spell as both ON_EQUIP and ON_USE across + // item templates. Those are equip/proc effects leaking into the active-use path, + // as seen with the error versions of Oracle Talisman of Ablution (44870) and + // Frenzyheart Insignia of Fury (44869). + if (mixedTriggerTrinketSpellIds.find(spellId) != mixedTriggerTrinketSpellIds.end()) return false; - uint32 spellProcFlag = spellInfo->ProcFlags; - - // Handle items with procflag "if you kill a target that grants honor or experience" - // Bots will "learn" the trinket proc, so CanCastSpell() will be true - // e.g. on Item https://www.wowhead.com/wotlk/item=44074/oracle-talisman-of-ablution leading to - // constant casting of the proc spell onto themselfes https://www.wowhead.com/wotlk/spell=59787/oracle-ablutions - // This will lead to multiple hundreds of entries in m_appliedAuras -> Once killing an enemy -> Big diff time spikes - if (spellProcFlag != 0) return false; - - if (!botAI->CanCastSpell(spellId, bot, false)) + if (!botAI->CanCastSpell(spellId, bot, false, nullptr, item)) return false; + break; } } + if (!spellId) return false; @@ -483,7 +651,25 @@ bool UseTrinketAction::UseTrinket(Item* item) targetFlag = TARGET_FLAG_NONE; packet << targetFlag << bot->GetPackGUID(); + bot->GetSession()->HandleUseItemOpcode(packet); + + uint32 const now = getMSTime(); + uint32 const cooldownDelay = bot->GetSpellCooldownDelay(spellId); + if (cooldownDelay > 0) + { + if (itemSpellCooldown > 0) + { + uint64 const itemCooldownKey = (static_cast(item->GetEntry()) << 32) | spellId; + trinketItemCooldownExpiries[itemCooldownKey] = now + static_cast(itemSpellCooldown); + } + + if (itemSpellCategory && itemSpellCategoryCooldown > 0) + { + trinketCategoryCooldownExpiries[itemSpellCategory] = now + static_cast(itemSpellCategoryCooldown); + } + } + return true; } diff --git a/src/Ai/Base/Actions/GenericSpellActions.h b/src/Ai/Base/Actions/GenericSpellActions.h index 5c52b7fd9..c17d96907 100644 --- a/src/Ai/Base/Actions/GenericSpellActions.h +++ b/src/Ai/Base/Actions/GenericSpellActions.h @@ -334,6 +334,10 @@ public: protected: bool UseTrinket(Item* trinket); + +private: + std::unordered_map trinketItemCooldownExpiries; + std::unordered_map trinketCategoryCooldownExpiries; }; class CastSpellOnEnemyHealerAction : public CastSpellAction diff --git a/src/PlayerbotAIConfig.cpp b/src/PlayerbotAIConfig.cpp index 06e88a67b..c01769eb8 100644 --- a/src/PlayerbotAIConfig.cpp +++ b/src/PlayerbotAIConfig.cpp @@ -215,7 +215,7 @@ bool PlayerbotAIConfig::Initialize() attunementQuests); LoadSet>( - sConfigMgr->GetOption("AiPlayerbot.UnobtainableItems", "12468,46978"), + sConfigMgr->GetOption("AiPlayerbot.UnobtainableItems", "12468,44869,44870,46978"), unobtainableItems); botAutologin = sConfigMgr->GetOption("AiPlayerbot.BotAutologin", false);