forked from mod-playerbots/mod-playerbots
-
Notifications
You must be signed in to change notification settings - Fork 0
Smart loot system refactored 2026 #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
hermensbas
wants to merge
74
commits into
hermensbas:master
Choose a base branch
from
Wishmaster117:Smart-Loot-System-Refactored-2026
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Smart loot system refactored 2026 #6
hermensbas
wants to merge
74
commits into
hermensbas:master
from
Wishmaster117:Smart-Loot-System-Refactored-2026
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ing grey (normal or lower quality)
# Pull Request Describe what this change does and why it is needed... --- ## 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? - - [ ] No - - [ ] Yes (**explain below**) Does this change increase per-bot or per-tick processing? - - [ ] No - - [ ] Yes (**describe and justify impact**) Could this logic scale poorly under load? - - [ ] No - - [ ] Yes (**explain why**) --- ## Defaults & Configuration Does this change modify default bot behavior? - - [ ] 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? - - [ ] 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 - - [ ] Stability is not compromised - - [ ] Performance impact is understood, tested, and acceptable - - [ ] Added logic complexity is justified and explained - - [ ] 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: Crow <pengchengw@me.com>
# Pull Request The purposes of this PR are to (1) establish a general raid helper framework for the benefit of future raid strategies and (2) make some improvements to problematic areas of the raid strategy code. List of changes: 1. Added new RaidBossHelpers.cpp and RaidBossHelpers.h files in the Raid folder. 3. Moved reused helpers from Karazhan, Gruul, and Magtheridon strategies to the new helper files. 4. Modified the prior function that assigned a DPS bot to store and erase timers and trackers in associative containers--the function now includes parameters for mapId (so a bot that is not in the instance will not be assigned) and for the ability to exclude a bot (useful for excluding particular important roles, such as a Warlock tank, so they are not bogged down by these extra tasks at critical moments). I also renamed it from IsInstanceTimerManager to IsMechanicTrackerBot. 5. Moved all helper files in raid strategies to Util folders (was needed for ICC, MC, and Ulduar). 6. Renamed and reordered includes of Ulduar files in AiObjectContext.cpp to match other raid strategies. a. This initially caused compile errors which made me realize that the existing code had several problems with missing includes and was compiling only due to the prior ordering in AiObjectContext.cpp. Therefore, I added the missing includes to Molten Core, Ulduar, and Vault of Archavon strategies. b. Ulduar and Old Kingdom were also using the same constant name for a spell--the reordering caused a compile error here as well, which just highlighted an existing problem that was being hidden. I renamed the constant for Ulduar to fix this, but I think the better approach going forward would be to use a namespace or enum class. But that is for another time and probably another person. 7. Several changes with respect to Ulduar files: a. The position constants and enums for spells and NPCs and such were in the trigger header file. I did not think that made sense so moved them to existing helper files. b. Since the strategy does not use multipliers, I removed all files and references to multipliers in it. c. I removed some unneeded includes. I did not do a detailed review to determine what else could be removed--I just took some out that I could tell right away were not needed. d. I renamed the ingame strategy name from "uld" to "ulduar," which I think is clearer and is still plenty short. 8. Partial refactor of Gruul and Magtheridon strategies: a. I did not due a full refactoring but made some quick changes to things I did previously that were rather stupid like repeating calculations, having useless logic like pointless IsAlive() checks for creatures already on the hostile references list, and not using the existing Position class for coordinates. b. There were a few substantive changes, such as allowing players to pick Maulgar mage and moonkin tanks with the assistant flag, but a greater refactoring of the strategies themselves is beyond this PR. c. I was clearing some containers used for Gruul and Magtheridon strategies; the methods are now fixed to erase only the applicable keys so that in the unlikely event that one server has multiple groups running Gruul or Magtheridon at the same time, there won't be timer or position tracker conflicts. ## How to Test the Changes 1. Enter any raid instance that has any code impacted by this PR 2. Engage bosses and observe if any strategies are now broken I personally tested Maulgar, Gruul, and Magtheridon and confirmed that they still work as intended. ## Complexity & Impact I do not expect this PR to have any relevant changes to in-game performance, but I will defer to those more knowledgeable than I if there are concerns in this area. As I've mentioned before, you can consider me to be like a person who has taken half an intro C++ course at best. ## AI Assistance None beyond autocomplete of repetitive changes. --------- Co-authored-by: bashermens <31279994+hermensbas@users.noreply.github.com>
# Pull Request Added Chilton wand to excluded to equipment items for bots and unified 2 exclusion lists to single one. Resolves: mod-playerbots#2093 --- ## How to Test the Changes Couldnt reproduce Chilton wand bug then testing sound impossible. Someone can try getting this items on shaman. ## Complexity & Impact Does this change add new decision branches? - - [x] No - - [ ] Yes Does this change increase per-bot or per-tick processing? - - [x] No - - [ ] Yes Could this logic scale poorly under load? - - [x] No - - [ ] Yes --- ## Defaults & Configuration Does this change modify default bot behavior? - - [x] No - - [ ] Yes 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 --- ## 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 ---
…ing grey (normal or lower quality)
…m/Wishmaster117/mod-playerbots into Smart-Loot-System-Refactored-2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.