mod-playerbots

veitR/mod-playerbots

Fork 0

mirror of https://github.com/liyunfan1223/mod-playerbots.git synced 2026-06-20 15:39:25 +02:00

Commit Graph

Author	SHA1	Message	Date
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
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
bashermens	41c53365ae	[HOT FIX] MS build issues regarding folder / command lenght usage or rc.exe (#2038 )	2026-01-19 22:45:28 +01:00

Author

SHA1

Message

Date

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

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

bashermens

41c53365ae

[HOT FIX] MS build issues regarding folder / command lenght usage or rc.exe (#2038 )

2026-01-19 22:45:28 +01:00

3 Commits