mirror of
https://github.com/liyunfan1223/mod-playerbots.git
synced 2026-06-20 15:39:25 +02:00
14 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Crow
|
db3cd7a93b
|
Delete Empty ActionNodes (#2445)
<!--
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 -->
No functional changes in this PR--just cleaning up a bunch of unused
code.
Engine::CreateActionNode creates an empty node when one is not supplied
by an ActionNodeFactory. That means that all ActionNodes in class
strategies that are empty (no prerequisite, alternative, or
continuation) are unnecessary. I've already deleted some in past PRs.
This one gets rid of all that remains.
I also deleted DeathKnightBuffStrategyFactoryInternal, which provides
the "bdps" strategy for DKs, which is not associated with any actions.
It's just a dead factory and inoperative strategy.
## 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.
## 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.
-->
## 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.
-->
I had GPT-5.4 run a check for any empty ActionNodes that I missed after
I was done, and it caught a few.
<!--
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.
-->
|
||
|
Crow
|
a76f2ca268
|
Refactor HasSpell/HasAura and convert spellIds to constants (#2435)
<!--
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 is a PR to follow-up on a discussion with @kadeshar during the
recent mage armor PR. Main changes:
- Add new PlayerbotAI::HasSpell() overload that accepts a string for the
spell name as an argument, in order to replace repeated
bot->HasSpell(spellId) checks for places where multiple ranks of a spell
are checked, and to replace a few calls to AI_VALUE(uint32, "spell id",
...) where it existed only to check for the spell being known.
- Removed PlayerbotAI::HasAura() overload that takes a spell Id as an
argument. It is rarely used, and the few instances of its usage are
easily replaced with AC's Unit::HasAura(), which is the predominant
usage for spell Id aura checks already anyway.
- When modifying DruidPullStrategy to use the new HasSpell() method, I
went ahead and also simplified the file overall to pare down duplication
(for example, the first check in CanCastSpell is for the spell Id so we
don't need to separately check it before calling CanCastSpell()). I then
did the same for other pull strategies so they can be consistent.
- Changed many magic numbers for spell Ids to be defined as compile-time
constants. I didn't do this throughout the code, but I did in the files
where I was making changes anyway due to the HasSpell() and/or HasAura()
changes.
## 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.
- Minimum logic is described above.
- Processing cost shouldn't change much. See Impact Assessment below.
## 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.
-->
## 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**)
- Passing a string for the spell to check for it being known or the aura
being present is more taxing than looking up a spell Id, although the
usefulness of the string-based overload is for situations with spells
that have multiple ranks.
- 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**)
- I think these changes should simplify maintenance complexity.
## 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 AI make many of the changes and then review since they were
pretty mundane (for example, I'd tell it to just take all the magic
numbers from a file and define them in an anonymous namespace, then
replace the magic numbers with the spell constants). There isn't
anything complicated in this PR; it was mostly busywork.
<!--
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.
-->
|
||
|
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>
|
||
|
Crow
|
cd2fe2f9a1
|
Use Stones for Prot Paladins & Reduce Oils (#2405)
<!--
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 -->
Back when I did the Stones/Oils PR, I gave Wizard Oil to Prot Paladins.
I was operating based on their mechanics in TBC and have since realized
that their damage is much more physical-based in WotLK as opposed to
spell-based in TBC. So I've changed them to now get stones upon
maintenance and apply stones.
I also reduced weapon oils given during maintenance from 4 to 2 stacks.
I did 4 originally because oils stack to 5 so it would align with the
amount given of stones and poisons (which stack to 20). But 4 inventory
slots taken up by oils is excessive, and 2 still last far longer than
buffing reagents in a raid context. So this is just to reduce inventory
clutter.
Other stuff is just little code stuff like reformatting or reordering
things to be more efficient.
## 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.
There's nothing new--essentially with respect to consumables, prot
paladins now are lumped with ret.
## 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 a prot paladin, turn on selfbot. Use maintenance and the paladin
should be given stones and no oils. The paladin should then apply the
stone to its weapon (if it is equipping a level-eligible weapon).
Create a caster, turn on selfbot. Use maintenance and they should be
given 2 stacks of oils.
## 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**)
Prot paladins will now use stones in their inventory and will not use
oils in their inventories. This is better for their mechanics.
- 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?
- - [x] No
- - [ ] 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.
-->
<!--
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.
-->
|
||
|
Keleborn
|
866a73dfbf
|
Clean up unused variables (#2268)
<!--
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 -->
Clean up a bunch of additional unused variable warnings.
## 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.
## 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.
-->
## 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**)
## 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.
-->
Claude reviewed the warnings log from a build and suggested a series of
changes. I focused just on these warnings for now. Every line was
reviewed. Some sections need to be reviewed by author for intent.
## 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.
-->
|
||
|
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>
|
||
|
Keleborn
|
5e2f2823ec
|
fix(Core/Paladin): Remove duplicate trigger registrations (#2301)
<!--
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 -->
The "lay on hands", "lay on hands on party", "blessing of protection on
party", and "divine plea" triggers were registered twice with
conflicting priorities, causing double-firing. Removes the old block
while preserving the new "hand of freedom on party" trigger.
@kadeshar Can you confirm this matches your intent?
## 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.
## 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.
-->
## 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**)
## 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.
-->
Claude spotted the error on review and ran ahead with a fix, but I
confirmed that it is infact duplicate.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
## 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: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
||
|
kadeshar
|
d07ddb14d0
|
Paladin use bubble heal strategy (#2244)
<!--
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 support for bubble heal strategy.
Adjusted emergency action order
## 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.
-->
- invite paladin bot to party
- check that bot dont have `healer dps` strategy
- start combat (with for example dummy)
- use `.damage 20000` where 20000 is near max health of bot
- use `.unaura 25771` until bot use Divine Shield
- bot should heal himself
## 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**)
Paladin bot heal himself without `healer dps` while Divine Shield
- 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?
- - [x] No
- - [ ] 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.
-->
## 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="297" height="142" alt="obraz"
src="https://github.com/user-attachments/assets/5e35341b-2b23-4d34-b0c6-dc228823850f"
/>
|
||
|
kadeshar
|
76dd91c4fa
|
Hand of Freedom support (#2233)
<!-- 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 Added Hand of Freedom action for paladin. Related with: #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. --> - invite paladin bot to party - start fight (can use dummy target) - apply some snare effect to bot or yourself (for example `.aura 1715`) - bot should use hand of freedom ## 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**) Yes, paladin bots start using Hand of Freedom - 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, as helper to create and review code ## 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="424" height="93" alt="obraz" src="https://github.com/user-attachments/assets/3cac4454-35af-474d-8ea0-67c462973c79" /> |
||
|
dillyns
|
32af1b95de
|
Paladin Seal of wisdom fallback fix for Ret/Prot Paladins (#2147)
# Pull Request This PR removes the fallback for Seal of Wisdom action from generic Paladin strategy and moves it to Holy Paladin only. This is necessary because a paladin who does not have Seal of Wisdom yet who goes low mana and triggers the seal of wisdom action will end up falling back to Seal of Righteousness, even though a better seal may be available, such as Seal of Command. The fallback is added to Holy Paladin so that low level holy paladins still use Righteousness until they get Wisdom --- ## Feature Evaluation Please answer the following: - Describe the **minimum logic** required to achieve the intended behavior? Ret paladin without Seal of Wisdom shouldn't change seals on low mana. - Describe the **cheapest implementation** that produces an acceptable result? Cheapest implementation is to remove seal of wisdom fallback for non-holy paladins. - Describe the **runtime cost** when this logic executes across many bots? No difference in cost compared to existing logic. --- ## How to Test the Changes Use a ret paladin bot who has Seal of Command but who does not have Seal of Wisdom. A paladin under level 38 will do. Order them to attack something, like a test dummy, until they eventually run low on mana. Before this change: The paladin will switch to Seal of Righteousness when they get low mana. After this change: The paladin leaves Seal of Command on when they get low mana. ## 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: - - [x] 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. |
||
|
Crow
|
35a0282ca6
|
Add Sense Undead for Paladins (#2200)
# Pull Request This PR adds the sense undead ability for Paladins, which they will keep active at all times. This is mildly useful because the associated minor glyph provides a 1% damage increase against undead while the ability is active. Sense undead is also added to InitClassSpells(). I understand that it is a trainer spell so would normally be covered by InitAvailableSpells(), but those playing with mod-individual-progression will not receive the spell through InitAvailableSpells() because it is removed from trainers by the mod (in TBC, a quest was required to obtain the spell). Finally, the minor glyph of sense undead is now added to the config as a default glyph for all PvE specs. It is not added for PvP specs because Forsaken do not count as undead so the glyph is useless in PvP. I also made some other tweaks to Paladin default minor glyphs that are not worth spending any time talking about. Edit: I also did some minor reformatting of code and replaced some numbers with existing constants. --- ## 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? The implementation just checks if a Paladin has the sense undead aura, and if not, the Paladin will activate sense undead. It is simple and cheap. --- ## 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? - - [ ] No - - [x] Yes (**describe and justify impact**) Infinitesimally 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**) Paladin bots will by default have sense undead enabled. There is no disadvantage to this. If this introduces more advanced or AI-heavy logic: - - [x] 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. --------- 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> |
||
|
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>
|
||
|
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>
|
||
|
bashermens
|
41c53365ae
|
[HOT FIX] MS build issues regarding folder / command lenght usage or rc.exe (#2038) |