11 Commits

Author SHA1 Message Date
Crow
32d10080a4
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>
2026-05-30 11:12:34 -07:00
Crow
9bba4b78dd
Overhaul party buff/greater blessing system (#2358)
## Pull Request Description

These changes I originally made for myself because as a person who
really likes to raid with bots, I felt like the current group buff
system is fundamentally broken, and I needed something more consistent
and optimal. I debated a lot whether to PR this because it's such an
extensive overhaul that was almost entirely reliant on AI, and I know
that wishmaster still has a PR open regarding the greater blessings. I
decided to after a couple of conversations so at least people can look
at it and see if it's something that they want.

The tl;dr version is that this PR overhauls buff handling in two related
areas:
1. It adds a dedicated greater blessing assignment system.
2. It generalizes party/raid reagent-buff handling for Paladins, Druids,
Mages, and Priests.

Under this PR, greater blessings are determined by assignments for the
current group, and those assignments are determined based on:

1. a hardcoded priority list of blessings for each spec;
2. the number of Paladins in the group; and 
3. whether any Paladins have talents for Blessing of Sanctuary, Improved
Blessing of Might, or Improved Blessing of Wisdom.

Assignment determinations are cached in a value to avoid constant
reevaluation.

The exact priority list is:

- All casters: Kings, Wisdom, Sanctuary, Might
- Physical-only DPS (Rogues, Warriors, DKs): Might, Kings, Sanctuary,
N/A
- Hybrid DPS (Enh, Ret, Hunters, Cats): Might, Kings, Wisdom, Sanctuary
- Druid tanks: Kings, Might, Sanctuary, Wisdom
- Warrior and DK tanks: Kings, Might, Sanctuary, N/A
- Paladin tank: Sanctuary, Might, Wisdom, Kings

Note that Sanctuary is preferred over Kings for Paladin tanks because of
the mana regen component but deprioritized for other tanks because Kings
provides Agility. The extra 3% damage reduction from Sanctuary does not
stack with Disc Priests’ Renewed Hope, which will have 100% uptime.

For group buffs, logic is centralized so that class triggers use the
same gating and upgrade rules for Gift of the Wild, Arcane Brilliance,
Prayer of Fortitude, Prayer of Spirit, and Prayer of Shadow Protection.
Also, Shadow Protection is now a default strategy for Priests (rshadow,
which existed before but wasn’t added by default).

I’ve added a config setting for the greater blessing system and adjusted
the current config setting for group buffs. In each case, you can pick
whether to disable the feature entirely, use it in all groups, or use it
only in raid groups. The default is raid only for greater blessings and
all groups for group buffs. Note that for group buffs, even if the
config is enabled, they will be used only if at least 3 group/raid
members on the same map are missing the buff family. This is mainly to
stop group buff spamming during wipe recovery as bots are revived
one-by-one.

I renamed the Paladin buff strategies to align them with the actual
blessing names:
- `bhealth` -> `bsanc`
- `bmana` -> `bwisdom`
- `bdps` -> `bmight`
- `bstats` -> `bkings`

This is an intentional breaking change for saved strategy strings. Bots
will need a one-time strategy reset after update.

I removed bots telling you when they are out of reagents for greater
blessings. If people like that though, I can add it back.

A small cleanup is also included in TankPaladinStrategy: Holy Shield was
subject to three overlapping health triggers with the same priority; I
removed the two lower health thresholds which have no purpose.

## Feature Evaluation

- Describe the **minimum logic** required to achieve the intended
behavior.

I’m going to let the AI answer this one.
> The minimum logic is:
> - a shared config-gated check for whether group/raid buff variants are
allowed
> - a shared way to treat single and group variants as equivalent aura
families
> - a shared upgrade path from single-target buff to group buff when the
group variant is appropriate
> - a Paladin-only cached assignment model that decides which blessing
family each Paladin should cover for the current group
> - trigger/action wiring that only attempts casts when a group member
is actually missing the assigned buff
>
> This avoids scattering separate per-class heuristics across many
triggers and actions.

- Describe the **processing cost** when this logic executes across many
bots.

Processing cost should be minimal but non-zero. The general party buff
changes are limited to existing buff trigger paths and mostly replace
duplicated checks with shared helpers. They do not add expensive default
per-tick behavior outside those existing trigger evaluations.

The Paladin greater blessing logic does add extra decision-making, but
it is limited to Paladins, gated by config and group eligibility,
subject to a delayed trigger evaluation of only once per 4s, and cached
per group assignment set instead of recomputing the full assignment
model on every action attempt.

This PR also increases the throttle duration for group buff triggers to
limit performance impact; I’m open to adjustments to these durations:
- Mark of the Wild triggers were increased from 4s to 8s
- Arcane Intellect triggers were increased from 4s to 8s
- Priest buff triggers were increased to 8s (previously, Fortitude was
6s, Spirit was 4s, and Shadow Protection had no throttle)
- There is now a 5s delay on buffing (greater blessings and group buffs)
after bots log in—I was getting bots spamming buffs as soon as they
logged in even when it was not necessary

I’ve tested with pmon, and the impact is minimal—these are very cheap
triggers even compared to standard bot rotational ability triggers.

## How to Test the Changes

1. Try different config settings to confirm that they work to
enable/disable greater blessings/group buffs in the configured scenarios

2. For greater blessing changes:
   - test with one Paladin in a party/raid
   - test with multiple Paladins in a party/raid
- confirm the Paladins divide blessing coverage instead of repeatedly
overwriting each other
- include at least one Paladin with Improved Blessing of Might and make
sure it casts Might over Paladins without the talent; check the same
with a Paladin with Improved Blessing of Wisdom
- do not include a Paladin that knows Sanctuary, confirm any Paladin
tank receives Kings instead (you’ll need a low-level Paladin for this
since Sanctuary is a prot talent)
- confirm bots cast blessings only when a member is actually missing the
relevant blessing family
   - confirm there is a 5s delay on buffing when bots log in

3. For group buff changes:
   - confirm there is a 5s delay on buffing when bots log in
- confirm that single buffs are used when there aren’t at least three
unbuffed members in the same map, even if group buffs are enabled in the
config

4. For all buffs, test with reagents missing to confirm fallback to
single-target buffs and single blessings

5. Confirm the Paladin buff strategy names are changed after resetting
AI

## 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**)

Discussed above in processing costs.

- Does this change modify default bot behavior?
    - - [ ] No
    - - [x] Yes (**explain why**)

Yes—that is the purpose of this PR, to change default buffing behavior.

- Does this change add new decision branches or increase maintenance
complexity?
    - - [ ] No
    - - [x] Yes (**explain below**)

Yes, but I think it’s inevitable to add complexity to get greater
blessings to function consistently, given the challenges brought by
their mechanic of applying across each class.

## AI Assistance

Was AI assistance used while working on this change?
- - [ ] No
- - [x] Yes (**explain below**)

I used GPT-5.4 extensively for this overhaul. It’s much more complicated
than I could handle on my own. I’ve done a lot of testing and have
reviewed the code and provided plenty of revisions, but I cannot say I
can perfectly explain each addition and how it works, not even close.

## 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

---------

Co-authored-by: Keleborn <22352763+Celandriel@users.noreply.github.com>
Co-authored-by: bash <hermensb@gmail.com>
Co-authored-by: Revision <tkn963@gmail.com>
Co-authored-by: kadeshar <kadeshar@gmail.com>
2026-05-29 23:08:21 -07:00
Crow
826887133d
Exclude Invalid Weapons from Shaman Enchants & Refactor Temporary Enchant Spellcasting (#2345)
<!--
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 -->
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
<!--
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 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
<!--
- 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.
-->

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
<!-- 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?
    - - [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
<!--
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.
-->

I kicked around some ideas with GPT-5.4 with respect to the refactoring
aspect of the PR after I had fixed the bug.
<!--
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.
-->
2026-05-08 22:40:35 -07:00
kadeshar
19249e90a0
Pull strategy migration (#2310)
<!--
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 -->

Pull strategy migration from cmangos for tank specializations

## 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.
-->

1. Invite bot tank
2. Use `reset boAI` or `nc +pull,+pull back` + `co +pull,+pull back`
3. Order bot to pull using command `pull my target` or `pull rti target`
4. Bot should run to mob, use ranged skill and back to point where he
started pull
Without `pull back` strategy bot run to mob, use ranged skill and wait
on mob until he come to bot

## 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?
    - - [x] No, not at all
    - - [ ] Minimal impact (**explain below**)
    - - [ ] Moderate impact (**explain below**)



- Does this change modify default bot behavior?
    - - [x] No
    - - [ ] Yes (**explain why**)



- Does this change add new decision branches or increase maintenance
complexity?
    - - [x] No
    - - [ ] Yes (**explain below**)



## 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.
-->

Help with migration and solving some problems

<!--
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.
- - [ ] Documentation updated if needed (Conf comments, WiKi commands).

## Notes for Reviewers
<!-- Anything else that's helpful to review or test your pull request.
-->

Stability test after randomize new bots
<img width="465" height="172" alt="obraz"
src="https://github.com/user-attachments/assets/6e39a8c0-f23b-47cc-852a-71fa98044a31"
/>

---------

Co-authored-by: Keleborn <22352763+Celandriel@users.noreply.github.com>
2026-04-17 14:07:47 -07:00
Crow
15bf0ab427
Fix Shaman Weapon Enchants & Cure Toxins/Cleanse Spirit (#2234)
<!--
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 -->

1. I've been having persistent issues with Enhancement Shamans sometimes
applying Rockbiter to both weapons instead of MH Windfury and OH
Flametongue. Rockbiter is the alternative for Flametongue and, through
Flametongue, the alternative for Windfury. But there seemed to be no
obvious reason why a Shaman that had all three abilities would ever use
Rockbiter, which costs more mana than Windfury and Flametongue. Claude's
take on it is that there is instability from ItemForSpellValue related
to its poor way of distinguishing handedness, in addition to it having a
1-second cache, which can cause in some scenarios stale caches for the
action running on each hand back-to-back. I still can't say I fully
understand why the issue exists, but the most straightforward fix that
should prevent this from happening is to just have separate mainhand and
offhand actions for each enchant. So that's what this PR does. The
relevant ActionNodes are now:
 
- The MH-specific chain for Enhancement is WF -> FT -> RB. In practice,
Enhancement should never apply RB because all Shamans under level 10
(when FT is learned) are considered Elemental. The FT -> RB node is just
for Elemental.
- The MH-specific Resto chain (not that Resto can dual-wield) is EL ->
FT -> RB. Againt, FT -> RB is just for Elemental.
- OH for Enhancement is only FT. You cannot be Enhancement before level
10, nor can Enhancement dual-wield before level 40, so no alternative is
needed.

3. I commented out Frostbrand Weapon actions/triggers because the
ability is not included in any strategy. I didn't delete the code
because in the future somebody might want to implement it as I
understand it can be useful for Enhancement Shamans in PvP.
4. Shamans are coded to use "cure poison" and "cure disease", which do
not exist in WotLK, having been combined into Cure Toxins. Wishmaster
has PR #1844 that has been open on this for a long time, but I decided
to correct the abilities here anyway as he was going for a more limited
approach, and I decided to rename all the actions and redo the structure
to rely on ActionNode alternatives, which is pretty much the exact
framework that should be used for this type of situation w/r/t bots.
Now, Shamans prefer Cleanse Spirit (Resto talent, which costs the same
as Cure Toxins and also dispels curses), with an alternative of Cure
Toxins (for poisons and disease only). I tested this and it seems to
work well.
5. I deleted empty ActionNodes.
6. I did some cleanup of formatting and such, but this is not intended
to be a comprehensive refactor.


## 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.

I've followed the general intended structure of class strategies with
triggers and actions. The same triggers exist, just different actions
are called based on the trigger that fires, so I don't think there
should be any impact on performance.

## 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.
-->

The best way to get a grasp on if things work is probably to just do
group play for a while with Shamans and make sure they apply the right
enchants and properly cast Cleanse Spirit and Cure Toxins.

## 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?
    - - [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**)

Shamans previously did not cure poisons or disease at all, and now they
do with the default "cure" strategy applied.

- Does this change add new decision branches or increase maintenance
complexity?
    - - [ ] No
    - - [x] Yes (**explain below**)

One might say having separate actions per hand for enchants is somewhat
more complex, but ultimately I think it is less confusing to keep those
paths separate.

## Messages to Translate
<!--
Bot messages have to be translatable, but you don't need to do the
translations here. You only need to make sure
the message is in a translatable format, and list in the table the
message_key and the default English message.
Search for GetBotTextOrDefault in the codebase for examples.
-->
Does this change add bot messages to translate?
    - - [x] No
    - - [ ] Yes (**list messages in the table**)

| Message key  | Default message |
| --------------- | ------------------ |
|			 |			      |
|			 |			      |

## 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.
-->

I had Claude try to diagnose the weapon enchant issue. It proposed and
provided the separate MH/OH WF/FT actions. The other things were easy
enough for me to do.

## Final Checklist

- - [x] Stability is not compromised.
- - [x] Performance impact is understood, tested, and acceptable.
- - [x] Added logic complexity is justified and explained.
- - [x] Documentation updated if needed (Conf comments, WiKi commands).

## Notes for Reviewers
<!-- Anything else that's helpful to review or test your pull request.
-->

---------

Co-authored-by: Keleborn <22352763+Celandriel@users.noreply.github.com>
Co-authored-by: bash <hermensb@gmail.com>
Co-authored-by: Revision <tkn963@gmail.com>
Co-authored-by: kadeshar <kadeshar@gmail.com>
2026-04-03 13:25:40 -07:00
kadeshar
6db44b5296
Will of the forsaken (#2231)
<!--
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 -->
Added Will of the Forsaken support. 
Made structure fix for affected files.

Partially resolves: #2002 

## 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.
-->
- create/invite undead bot
- start fight (you can use dummy target)
- use `.aura 6215` to fear bot
- bot should use "Will of the Forsaken" to remove debuff


## 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?
    - - [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**)

Undead bot use now "Will of the Forsaken" to remove charm, fear or
sleep.

- Does this change add new decision branches or increase maintenance
complexity?
    - - [x] No
    - - [ ] Yes (**explain below**)



## Messages to Translate
<!--
Bot messages have to be translatable, but you don't need to do the
translations here. You only need to make sure
the message is in a translatable format, and list in the table the
message_key and the default English message.
Search for GetBotTextOrDefault in the codebase for examples.
-->
- Does this change add bot messages to translate?
    - - [x] No
    - - [ ] Yes (**list messages in the table**)

| Message key  | Default message |
| --------------- | ------------------ |
|			 |			      |
|			 |			      |

## 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.
-->

OpenCode to review changes


## Final Checklist

- - [x] Stability is not compromised.
- - [x] Performance impact is understood, tested, and acceptable.
- - [x] Added logic complexity is justified and explained.
- - [x] Documentation updated if needed (Conf comments, WiKi commands).

## Notes for Reviewers
<!-- Anything else that's helpful to review or test your pull request.
-->

<img width="325" height="77" alt="obraz"
src="https://github.com/user-attachments/assets/729cfdf8-3742-457c-9278-7fcce824e6d0"
/>

---------

Co-authored-by: Keleborn <22352763+Celandriel@users.noreply.github.com>
2026-03-27 11:53:53 -07:00
kadeshar
c6a07ad012
Every Man for Himself racial support (#2198)
# Pull Request

Added Every Man for Himself racial support
Partially resolves:
https://github.com/mod-playerbots/mod-playerbots/issues/2002

---

## How to Test the Changes

- when human bot is in combat apply aura via command `.aura 20066`
- bot should use "Every Man for Himself" to remove aura

## Complexity & Impact

Does this change add new decision branches?
- - [x] No
- - [ ] Yes (**explain below**)

Does this change increase per-bot or per-tick processing?
- - [x] No
- - [ ] Yes (**describe and justify impact**)

Could this logic scale poorly under load?
- - [x] No
- - [ ] Yes (**explain why**)
---

## Defaults & Configuration

Does this change modify default bot behavior?
- - [ ] No
- - [x] Yes (**explain why**)

Human bots now using "Every Man for Himself" by default where in combat

If this introduces more advanced or AI-heavy logic:
- - [x] Lightweight mode remains the default
- - [x] More complex behavior is optional and thereby configurable
---

## AI Assistance

Was AI assistance (e.g. ChatGPT or similar tools) used while working on
this change?
- - [ ] No
- - [x] Yes (**explain below**)

Copilot CLI to review changes

---

## Final Checklist

- - [x] Stability is not compromised
- - [x] Performance impact is understood, tested, and acceptable
- - [x] Added logic complexity is justified and explained
- - [x] Documentation updated if needed

---

## Notes for Reviewers

Test result:
<img width="358" height="97" alt="obraz"
src="https://github.com/user-attachments/assets/66044a93-d73b-4706-ae2f-ea8ae6e25438"
/>
2026-03-20 20:41:22 +01:00
Keleborn
441f9f7552
Warnings PR 1: Event warnings and headers (#2106)
# Pull Request

This is the first in a series of PRs intended to eliminate warnings in
the module. The design intent is to eliminate the calling event when not
needed in the body of the function. Based off of SmashingQuasars work.

---

## How to Test the Changes

- Step-by-step instructions to test the change
- Any required setup (e.g. multiple players, bots, specific
configuration)
- Expected behavior and how to verify it

## Complexity & Impact

- Does this change add new decision branches?
    - [x] No
    - [ ] Yes (**explain below**)

- Does this change increase per-bot or per-tick processing?
    - [x] No
    - [ ] Yes (**describe and justify impact**)

- Could this logic scale poorly under load?
    - [x] No
    - [ ] Yes (**explain why**)

---

## Defaults & Configuration

- Does this change modify default bot behavior?
    - [x] No
    - [ ] Yes (**explain why**)

If this introduces more advanced or AI-heavy logic:

- [ ] Lightweight mode remains the default
- [ ] More complex behavior is optional and thereby configurable

---

## AI Assistance

- Was AI assistance (e.g. ChatGPT or similar tools) used while working
on this change?
    - [x] No
    - [ ] Yes (**explain below**)

---

## Final Checklist

- [x] Stability is not compromised
- [x] Performance impact is understood, tested, and acceptable
- [x] Added logic complexity is justified and explained
- [x] Documentation updated if needed

---

## Notes for Reviewers

Anything that significantly improves realism at the cost of stability or
performance should be carefully discussed
before merging.

---------

Co-authored-by: bashermens <31279994+hermensbas@users.noreply.github.com>
2026-02-14 20:55:10 +01:00
privatecore
a0a50204ec
Fix action validation checks: isUseful -> isPossible + codestyle fixes and corrections (#2125)
# Pull Request

Fix the incorrect logic flaw when processing actions from different
sources. It should be: `isUseful` -> `isPossible`. The original logic is
based on the Mangosbot code and the impl presented inside
`Engine::DoNextAction`. This should fix all wrong validation orders for
triggers and direct/specific actions.

Code style is based on the AzerothCore style guide + clang-format.

---

## Design Philosophy

We prioritize **stability, performance, and predictability** over
behavioral realism.
Complex player-mimicking logic is intentionally limited due to its
negative impact on scalability, maintainability, and
long-term robustness.

Excessive processing overhead can lead to server hiccups, increased CPU
usage, and degraded performance for all
participants. Because every action and
decision tree is executed **per bot and per trigger**, even small
increases in logic complexity can scale poorly and
negatively affect both players and
world (random) bots. Bots are not expected to behave perfectly, and
perfect simulation of human decision-making is not a
project goal. Increased behavioral
realism often introduces disproportionate cost, reduced predictability,
and significantly higher maintenance overhead.

Every additional branch of logic increases long-term responsibility. All
decision paths must be tested, validated, and
maintained continuously as the system evolves.
If advanced or AI-intensive behavior is introduced, the **default
configuration must remain the lightweight decision
model**. More complex behavior should only be
available as an **explicit opt-in option**, clearly documented as having
a measurable performance cost.

Principles:

- **Stability before intelligence**  
  A stable system is always preferred over a smarter one.

- **Performance is a shared resource**  
  Any increase in bot cost affects all players and all bots.

- **Simple logic scales better than smart logic**  
Predictable behavior under load is more valuable than perfect decisions.

- **Complexity must justify itself**  
  If a feature cannot clearly explain its cost, it should not exist.

- **Defaults must be cheap**  
  Expensive behavior must always be optional and clearly communicated.

- **Bots should look reasonable, not perfect**  
  The goal is believable behavior, not human simulation.

Before submitting, confirm that this change aligns with those
principles.

---

## Feature Evaluation

Please answer the following:

- Describe the **minimum logic** required to achieve the intended
behavior?
- Describe the **cheapest implementation** that produces an acceptable
result?
- Describe the **runtime cost** when this logic executes across many
bots?

---

## How to Test the Changes

- Step-by-step instructions to test the change
- Any required setup (e.g. multiple players, bots, specific
configuration)
- Expected behavior and how to verify it

## Complexity & Impact

Does this change add new decision branches?
- - [x] No
- - [ ] Yes (**explain below**)

Does this change increase per-bot or per-tick processing?
- - [x] No
- - [ ] Yes (**describe and justify impact**)

Could this logic scale poorly under load?
- - [x] No
- - [ ] Yes (**explain why**)
---

## Defaults & Configuration

Does this change modify default bot behavior?
- - [x] No
- - [ ] Yes (**explain why**)

If this introduces more advanced or AI-heavy logic:
- - [ ] Lightweight mode remains the default
- - [ ] More complex behavior is optional and thereby configurable
---

## AI Assistance

Was AI assistance (e.g. ChatGPT or similar tools) used while working on
this change?
- - [x] No
- - [ ] Yes (**explain below**)

If yes, please specify:

- AI tool or model used (e.g. ChatGPT, GPT-4, Claude, etc.)
- Purpose of usage (e.g. brainstorming, refactoring, documentation, code
generation)
- Which parts of the change were influenced or generated
- Whether the result was manually reviewed and adapted

AI assistance is allowed, but all submitted code must be fully
understood, reviewed, and owned by the contributor.
Any AI-influenced changes must be verified against existing CORE and PB
logic. We expect contributors to be honest
about what they do and do not understand.

---

## Final Checklist

- - [x] Stability is not compromised
- - [x] Performance impact is understood, tested, and acceptable
- - [x] Added logic complexity is justified and explained
- - [x] Documentation updated if needed

---

## Notes for Reviewers

Anything that significantly improves realism at the cost of stability or
performance should be carefully discussed
before merging.
2026-02-13 09:24:11 -08:00
bashermens
13fff46fa0
Improper singletons migration to clean Meyer's singletons (cherry-pick) (#2082)
# Pull Request

- Applies the clean and corrected singletons, Meyer pattern. (cherry
picked from @SmashingQuasar )

Testing by just playing the game in various ways. Been tested by myself
@Celandriel and @SmashingQuasar
---

## Complexity & Impact

- Does this change add new decision branches?
    - [x] No
    - [ ] Yes (**explain below**)

- Does this change increase per-bot or per-tick processing?
    - [x] No
    - [ ] Yes (**describe and justify impact**)

- Could this logic scale poorly under load?
    - [x] No
    - [ ] Yes (**explain why**)

---

## Defaults & Configuration

- Does this change modify default bot behavior?
    - [x] No
    - [ ] Yes (**explain why**)

---

## AI Assistance

- Was AI assistance (e.g. ChatGPT or similar tools) used while working
on this change?
    - [x] No
    - [ ] Yes (**explain below**)
---

## Final Checklist

- [x] Stability is not compromised
- [x] Performance impact is understood, tested, and acceptable
- [x] Added logic complexity is justified and explained
- [x] Documentation updated if needed

---

## Notes for Reviewers

Anything that significantly improves realism at the cost of stability or
performance should be carefully discussed
before merging.

---------

Co-authored-by: Nicolas Lebacq <nicolas.cordier@outlook.com>
Co-authored-by: Keleborn <22352763+Celandriel@users.noreply.github.com>
2026-01-30 21:49:37 +01:00
bashermens
41c53365ae
[HOT FIX] MS build issues regarding folder / command lenght usage or rc.exe (#2038) 2026-01-19 22:45:28 +01:00