Improve bot trinket usage and fix related bugs (#2425)

<!--
Thank you for contributing to mod-playerbots, please make sure that
you...
1. Submit your PR to the test-staging branch, not master.
2. Read the guidelines below before submitting.
3. Don't delete parts of this template.

DESIGN PHILOSOPHY: We prioritize STABILITY, PERFORMANCE, AND
PREDICTABILITY over behavioral realism.

Every action and decision executes PER BOT AND PER TRIGGER. Small
increases in logic complexity scale
poorly across thousands of bots and negatively affect all. We prioritize
a stable system over a smarter
one. Bots don't need to behave perfectly; believable behavior is the
goal, not human simulation.
Default behavior must be cheap in processing; expensive behavior must be
opt-in.

Before submitting, make sure your changes aligns with these principles.
-->

## Pull Request Description
<!-- Describe what this change does and why it is needed -->

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
<!--
If your PR is very minimal (comment typo, wrong ID reference, etc), and
it is very obvious it will not have
any impact on performance, you may skip these question. If necessary, a
maintainer may ask you for them later.
-->

<!-- Please answer the following: -->
- 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
<!--
- Step-by-step instructions to test the change.
- Any required setup (e.g. multiple players, number of bots, specific
configuration).
- Expected behavior and how to verify it.
-->

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
<!-- As a generic test, before and after measure of pmon (playerbot pmon
tick) can help you here. -->
- 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
<!--
AI assistance is allowed, but all submitted code must be fully
understood, reviewed, and owned by the contributor.
We expect contributors to be honest about what they do and do not
understand.
-->
Was AI assistance used while working on this change?
- - [ ] No
- - [x] Yes (**explain below**)
<!--
If yes, please specify:
- Purpose of usage (e.g. brainstorming, refactoring, documentation, code
generation).
- Which parts of the change were influenced or generated, and whether it
was thoroughly reviewed.
-->

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.

<!--
TRANSLATIONS:
Anything new that the bots say in chat must be in a translatable format.
This is done using GetBotTextOrDefault,
which you can search for in the codebase to find examples. Your code
needs to have English as the default fallback,
while the full translations need to be in an SQL update file. The
languages in the file are the nine language
options supported by AzerothCore: English, Korean, French, German,
Chinese, Taiwanese, Spanish, Spanish Mexico, and
Russian. See
data/sql/playerbots/updates/2025_12_27_ai_playerbot_fishing_text.sql as
an example of a translation SQL
update, whose content are called within the codebase at
src/strategy/actions/FishingAction.cpp
-->

## 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
<!-- Anything else that's helpful to review or test your pull request.
-->
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

@@ -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)
+# Defaults: Chilton Wand (12468), Frenzyheart Insignia of Fury test/on-use row (44869),
-AiPlayerbot.UnobtainableItems = 12468,46978
+#           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)

src/Ai/Base/Actions/GenericSpellActions.cpp

@@ -6,6 +6,7 @@
 #include "GenericSpellActions.h"
 
 #include <ctime>
+#include <unordered_set>
 
 #include "Event.h"
 #include "ItemTemplate.h"
@@ -23,6 +24,116 @@
 using ai::buff::MakeAuraQualifierForBuff;
 using ai::spell::HasSpellOrCategoryCooldown;
 
+namespace
+{
+    std::unordered_set<uint32> const& GetMixedTriggerTrinketSpellIds()
+    {
+        static std::unordered_set<uint32> const mixedTriggerSpellIds = []()
+        {
+            std::unordered_set<uint32> onUseSpellIds;
+            std::unordered_set<uint32> onEquipSpellIds;
+            std::unordered_set<uint32> 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<uint64>(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;
+
+                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 (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;
+
+            if (!botAI->CanCastSpell(spellId, bot, false, nullptr, item))
+                return false;
+
             break;
         }
     }
 
-            if (!applyAura)
-                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))
-                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<uint64>(item->GetEntry()) << 32) | spellId;
+            trinketItemCooldownExpiries[itemCooldownKey] = now + static_cast<uint32>(itemSpellCooldown);
+        }
+
+        if (itemSpellCategory && itemSpellCategoryCooldown > 0)
+        {
+            trinketCategoryCooldownExpiries[itemSpellCategory] = now + static_cast<uint32>(itemSpellCategoryCooldown);
+        }
+    }
+
     return true;
 }
 

src/Ai/Base/Actions/GenericSpellActions.h

@@ -334,6 +334,10 @@ public:
 
 protected:
     bool UseTrinket(Item* trinket);
+
+private:
+    std::unordered_map<uint64, uint32> trinketItemCooldownExpiries;
+    std::unordered_map<uint32, uint32> trinketCategoryCooldownExpiries;
 };
 
 class CastSpellOnEnemyHealerAction : public CastSpellAction

src/PlayerbotAIConfig.cpp

@@ -215,7 +215,7 @@ bool PlayerbotAIConfig::Initialize()
         attunementQuests);
 
     LoadSet<std::set<uint32>>(
-        sConfigMgr->GetOption<std::string>("AiPlayerbot.UnobtainableItems", "12468,46978"),
+        sConfigMgr->GetOption<std::string>("AiPlayerbot.UnobtainableItems", "12468,44869,44870,46978"),
         unobtainableItems);
 
     botAutologin = sConfigMgr->GetOption<bool>("AiPlayerbot.BotAutologin", false);