Closed
Conversation
# 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 ---
There was a problem hiding this comment.
Pull request overview
This PR primarily focuses on reducing compiler warnings and tightening compilation dependencies across the playerbots module, while also introducing shared raid helper utilities and a small set of raid/strategy naming cleanups.
Changes:
- Suppress unused-parameter warnings by updating many
Execute(Event ...)implementations to ignore theEventargument explicitly. - Reduce/adjust header dependencies across many compilation units (include cleanup, replacing umbrella includes in several places).
- Add shared raid helper utilities (RTI marking / mechanic-tracker selection) and rename Ulduar strategy identifier from
uldtoulduarin multiple call sites.
Reviewed changes
Copilot reviewed 220 out of 223 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Script/WorldThr/PlayerbotWorldThreadProcessor.h | Minor include cleanup |
| src/Script/Playerbots.cpp | Braces/style cleanup in scripts |
| src/Mgr/Travel/TravelMgr.cpp | Minor control-flow/style cleanup |
| src/Mgr/Talent/Talentspec.h | Adjust includes for types used |
| src/Mgr/Talent/Talentspec.cpp | Adjust includes for types used |
| src/Mgr/Item/StatsCollector.cpp | Remove unused includes |
| src/Mgr/Item/RandomItemMgr.cpp | Filter unobtainable items; minor cleanup |
| src/Db/PlayerbotRepository.cpp | Replace broad include with narrower include |
| src/Bot/RandomPlayerbotMgr.cpp | Remove unused include; comment fix; style cleanup |
| src/Bot/PlayerbotMgr.cpp | Config access cleanup; signature cleanup; minor refactors |
| src/Bot/PlayerbotAI.h | Include cleanup |
| src/Bot/PlayerbotAI.cpp | Rename Ulduar strategy key usage (uld → ulduar) |
| src/Bot/Factory/RandomPlayerbotFactory.cpp | Include cleanup |
| src/Bot/Factory/PlayerbotFactory.cpp | Config singleton usage in a couple spots; include cleanup |
| src/Bot/Factory/AiFactory.h | Minor include whitespace cleanup |
| src/Bot/Engine/Trigger/Trigger.h | Include cleanup |
| src/Bot/Engine/Trigger/Trigger.cpp | Include cleanup (remove broad includes) |
| src/Bot/Engine/Strategy/CustomStrategy.cpp | Loop braces/style cleanup |
| src/Bot/Engine/AiObjectContext.cpp | Move Ulduar contexts to new path include |
| src/Bot/Engine/Action/Action.h | Include cleanup |
| src/Bot/Cmd/PlayerbotCommandServer.cpp | Switch to RandomPlayerbotMgr::instance(); include cleanup |
| src/Ai/World/Rpg/Strategy/NewRpgStrategy.cpp | Remove unused include; unused param suppression |
| src/Ai/World/Rpg/Action/NewRpgAction.cpp | Include cleanup; unused param suppression |
| src/Ai/Raid/VaultOfArchavon/RaidVoATriggerContext.h | Add boss-aura trigger header include |
| src/Ai/Raid/VaultOfArchavon/RaidVoAActionContext.h | Add boss-aura action header include |
| src/Ai/Raid/VaultOfArchavon/Action/RaidVoAActions.cpp | Unused param suppression |
| src/Ai/Raid/Ulduar/Util/RaidUlduarScripts.h | New script include wrapper header |
| src/Ai/Raid/Ulduar/Util/RaidUlduarBossHelper.cpp | Add many Position constants; include cleanup |
| src/Ai/Raid/Ulduar/Strategy/RaidUlduarStrategy.h | Rename strategy key to ulduar; remove multipliers hook |
| src/Ai/Raid/Ulduar/Strategy/RaidUlduarStrategy.cpp | Remove multipliers wiring |
| src/Ai/Raid/Ulduar/RaidUlduarBossHelper.h | Deleted (moved/refactored) |
| src/Ai/Raid/Ulduar/Multiplier/RaidUlduarMultipliers.h | Deleted (multipliers removed) |
| src/Ai/Raid/Ulduar/Multiplier/RaidUlduarMultipliers.cpp | Deleted (multipliers removed) |
| src/Ai/Raid/RaidStrategyContext.h | Rename Ulduar strategy factory key (uld → ulduar) |
| src/Ai/Raid/RaidBossHelpers.h | New shared raid helper declarations |
| src/Ai/Raid/RaidBossHelpers.cpp | New shared raid helper implementations |
| src/Ai/Raid/Onyxia/Action/RaidOnyxiaActions.cpp | Unused param suppression |
| src/Ai/Raid/ObsidianSanctum/Action/RaidOsActions.cpp | Unused param suppression |
| src/Ai/Raid/MoltenCore/Util/RaidMcHelpers.h | New helper constants header |
| src/Ai/Raid/MoltenCore/RaidMcTriggerContext.h | Add boss-aura trigger header include |
| src/Ai/Raid/MoltenCore/RaidMcActionContext.h | Add boss-aura action header include |
| src/Ai/Raid/MoltenCore/Action/RaidMcActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Raid/Magtheridon/Util/RaidMagtheridonHelpers.h | Refactor helpers/constants, remove duplicated RTI helpers |
| src/Ai/Raid/Magtheridon/Util/RaidMagtheridonHelpers.cpp | Refactor helpers/constants; remove duplicated RTI helpers |
| src/Ai/Raid/Magtheridon/Trigger/RaidMagtheridonTriggers.cpp | Logic simplification around channeler liveness checks |
| src/Ai/Raid/Magtheridon/Multiplier/RaidMagtheridonMultipliers.cpp | Add WipeAction handling; adjust multipliers |
| src/Ai/Raid/Magtheridon/Action/RaidMagtheridonActions.h | Fix namespace qualification usage |
| src/Ai/Raid/Karazhan/Util/RaidKarazhanHelpers.h | Add NPC constants; convert consts to constexpr; remove duplicated helpers |
| src/Ai/Raid/Karazhan/Util/RaidKarazhanHelpers.cpp | Remove duplicated RTI/timer helpers; minor cleanup |
| src/Ai/Raid/Karazhan/Trigger/RaidKarazhanTriggers.cpp | Switch instance timer ownership logic to shared mechanic-tracker helper |
| src/Ai/Raid/Karazhan/Multiplier/RaidKarazhanMultipliers.cpp | Add shared helper include; refine Enfeeble suppression |
| src/Ai/Raid/Icecrown/Util/RaidIccScripts.h | New script include wrapper header |
| src/Ai/Raid/Icecrown/Trigger/RaidIccTriggers.cpp | Include cleanup |
| src/Ai/Raid/Icecrown/Multiplier/RaidIccMultipliers.cpp | Include cleanup |
| src/Ai/Raid/GruulsLair/Util/RaidGruulsLairHelpers.h | Refactor constants; remove duplicated RTI helpers |
| src/Ai/Raid/GruulsLair/Util/RaidGruulsLairHelpers.cpp | Refactor tank-selection logic; remove duplicated RTI helpers |
| src/Ai/Raid/GruulsLair/Trigger/RaidGruulsLairTriggers.h | Rename triggers (main tank → tanks, range → ranged) |
| src/Ai/Raid/GruulsLair/Trigger/RaidGruulsLairTriggers.cpp | Update trigger logic to match renamed triggers |
| src/Ai/Raid/GruulsLair/Strategy/RaidGruulsLairStrategy.cpp | Update trigger/action names to new identifiers |
| src/Ai/Raid/GruulsLair/RaidGruulsLairTriggerContext.h | Update trigger creators to new identifiers |
| src/Ai/Raid/GruulsLair/RaidGruulsLairActionContext.h | Update action creators to new identifiers |
| src/Ai/Raid/GruulsLair/Multiplier/RaidGruulsLairMultipliers.cpp | Replace charge-action helper with reach-target suppression |
| src/Ai/Raid/GruulsLair/Action/RaidGruulsLairActions.h | Rename action class to match trigger rename |
| src/Ai/Raid/EyeOfEternity/Action/RaidEoEActions.cpp | Unused param suppression; remove unused locals |
| src/Ai/Raid/BlackwingLair/Action/RaidBwlActions.cpp | Unused param suppression; expand one-liner for consistency |
| src/Ai/Raid/Aq20/Trigger/RaidAq20Triggers.cpp | Include cleanup |
| src/Ai/Raid/Aq20/Action/RaidAq20Actions.cpp | Unused param suppression; include cleanup |
| src/Ai/Dungeon/VioletHold/Action/VioletHoldActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Dungeon/UtgardePinnacle/Action/UtgardePinnacleActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Dungeon/UtgardeKeep/Action/UtgardeKeepActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Dungeon/TrialOfTheChampion/Strategy/TrialOfTheChampionStrategy.cpp | Include cleanup |
| src/Ai/Dungeon/TrialOfTheChampion/Action/TrialOfTheChampionActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Dungeon/PitOfSaron/Action/PitOfSaronActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Dungeon/OldKingdom/Action/OldKingdomActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Dungeon/Oculus/Multiplier/OculusMultipliers.cpp | Include added; unused param suppression; planar-shift logic touched |
| src/Ai/Dungeon/Oculus/Action/OculusActions.h | Include cleanup |
| src/Ai/Dungeon/Oculus/Action/OculusActions.cpp | Include ordering cleanup; unused param suppression; type tweak in loop |
| src/Ai/Dungeon/Nexus/Action/NexusActions.cpp | Unused param suppression; control-flow braces cleanup |
| src/Ai/Dungeon/HallsOfStone/Action/HallsOfStoneActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Dungeon/HallsOfLightning/Action/HallsOfLightningActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Dungeon/Gundrak/Action/GundrakActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Dungeon/ForgeOfSouls/Action/ForgeOfSoulsActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Dungeon/DraktharonKeep/Action/DrakTharonKeepActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Dungeon/CullingOfStratholme/Action/CullingOfStratholmeActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Dungeon/AzjolNerub/Action/AzjolNerubActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Class/Warrior/Strategy/TankWarriorStrategy.cpp | Include cleanup |
| src/Ai/Class/Warrior/Strategy/FuryWarriorStrategy.cpp | Include cleanup |
| src/Ai/Class/Warrior/Strategy/ArmsWarriorStrategy.cpp | Include cleanup |
| src/Ai/Class/Warrior/Action/WarriorActions.cpp | Unused param suppression |
| src/Ai/Class/Warlock/Strategy/TankWarlockStrategy.cpp | Include cleanup |
| src/Ai/Class/Warlock/Strategy/GenericWarlockStrategy.cpp | Include cleanup |
| src/Ai/Class/Warlock/Action/WarlockActions.cpp | Unused param suppression |
| src/Ai/Class/Shaman/Trigger/ShamanTriggers.h | Include cleanup |
| src/Ai/Class/Shaman/Strategy/TotemsShamanStrategy.h | Include cleanup |
| src/Ai/Class/Shaman/Action/ShamanActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Class/Rogue/Action/RogueActions.cpp | Unused param suppression |
| src/Ai/Class/Priest/Action/PriestActions.cpp | Unused param suppression |
| src/Ai/Class/Paladin/Strategy/GenericPaladinStrategy.cpp | Include cleanup |
| src/Ai/Class/Paladin/Action/PaladinActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Class/Hunter/Strategy/GenericHunterStrategy.cpp | Include cleanup |
| src/Ai/Class/Hunter/Action/HunterActions.cpp | Unused param suppression |
| src/Ai/Class/Druid/Action/DruidShapeshiftActions.cpp | Unused param suppression |
| src/Ai/Class/Druid/Action/DruidActions.cpp | Unused param suppression |
| src/Ai/Class/Dk/Strategy/GenericDKNonCombatStrategy.cpp | Include cleanup |
| src/Ai/Base/Value/TankTargetValue.cpp | Replace umbrella include with narrower includes |
| src/Ai/Base/Value/StatsValues.cpp | Replace umbrella include with narrower includes; config singleton usage in one place |
| src/Ai/Base/Value/SpellIdValue.cpp | Remove unused include |
| src/Ai/Base/Value/SnareTargetValue.cpp | Replace umbrella include with narrower includes |
| src/Ai/Base/Value/PossibleRpgTargetsValue.cpp | Include reshuffle; clarify travel-target acceptance condition |
| src/Ai/Base/Value/PartyMemberWithoutItemValue.h | Remove config include; simplify ctor |
| src/Ai/Base/Value/PartyMemberValue.cpp | Replace umbrella include with narrower includes |
| src/Ai/Base/Value/PartyMemberToDispel.cpp | Replace umbrella include with narrower include |
| src/Ai/Base/Value/NearestCorpsesValue.cpp | Remove umbrella include |
| src/Ai/Base/Value/LogLevelValue.h | Add missing log-common include |
| src/Ai/Base/Value/LeastHpTargetValue.cpp | Replace umbrella include with narrower include |
| src/Ai/Base/Value/LastMovementValue.cpp | Remove umbrella include |
| src/Ai/Base/Value/ItemCountValue.cpp | Replace umbrella include with narrower include; minor loop style |
| src/Ai/Base/Value/DpsTargetValue.cpp | Remove unused params/locals; signature cleanup |
| src/Ai/Base/Value/CurrentCcTargetValue.cpp | Replace umbrella include with narrower include |
| src/Ai/Base/Value/CcTargetValue.cpp | Replace umbrella include with narrower includes |
| src/Ai/Base/Value/Arrow.cpp | Replace umbrella include with narrower includes |
| src/Ai/Base/Trigger/RangeTriggers.cpp | Remove unused local |
| src/Ai/Base/Trigger/GenericTriggers.cpp | Include cleanup |
| src/Ai/Base/Strategy/UsePotionsStrategy.cpp | Include cleanup |
| src/Ai/Base/Strategy/RacialsStrategy.cpp | Include cleanup |
| src/Ai/Base/Strategy/RTSCStrategy.cpp | Include cleanup |
| src/Ai/Base/Strategy/NonCombatStrategy.cpp | Include cleanup |
| src/Ai/Base/Strategy/GuardStrategy.cpp | Include cleanup |
| src/Ai/Base/Strategy/FollowMasterStrategy.cpp | Include cleanup |
| src/Ai/Base/Strategy/DuelStrategy.cpp | Include cleanup |
| src/Ai/Base/Strategy/CombatStrategy.cpp | Include cleanup |
| src/Ai/Base/Strategy/ChatCommandHandlerStrategy.cpp | Include cleanup |
| src/Ai/Base/Actions/VehicleActions.cpp | Unused param suppression |
| src/Ai/Base/Actions/UseMeetingStoneAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/UseItemAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/UnlockTradedItemAction.cpp | Replace macro with constexpr; unused param suppression; include cleanup |
| src/Ai/Base/Actions/UnlockItemAction.cpp | Replace macro with constexpr; unused param suppression; include cleanup |
| src/Ai/Base/Actions/TravelAction.cpp | Unused param suppression; add braces around group check |
| src/Ai/Base/Actions/TrainerAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/TradeStatusAction.cpp | Minor braces/style cleanup |
| src/Ai/Base/Actions/TradeAction.cpp | Replace umbrella include with narrower include; minor style |
| src/Ai/Base/Actions/TellTargetAction.cpp | Replace umbrella include with narrower includes; unused param suppression |
| src/Ai/Base/Actions/TellReputationAction.cpp | Replace umbrella include with narrower include; unused param suppression |
| src/Ai/Base/Actions/TellMasterAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/TellLosAction.cpp | Include cleanup; unused param suppression |
| src/Ai/Base/Actions/TeleportAction.cpp | Replace umbrella include; switch to SpellMgr::instance() accessor |
| src/Ai/Base/Actions/TameAction.cpp | Include cleanup |
| src/Ai/Base/Actions/SuggestWhatToDoAction.cpp | Remove duplicated includes; include cleanup; unused param suppression |
| src/Ai/Base/Actions/StayActions.cpp | Unused param suppression; minor conditional braces cleanup |
| src/Ai/Base/Actions/StatsAction.cpp | Replace umbrella include; unused param suppression |
| src/Ai/Base/Actions/SetHomeAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/SetCraftAction.cpp | Minor braces/style cleanup |
| src/Ai/Base/Actions/SeeSpellAction.cpp | Include cleanup |
| src/Ai/Base/Actions/SecurityCheckAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/SayAction.cpp | Include cleanup; unused param suppression |
| src/Ai/Base/Actions/RtiAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/RpgSubActions.cpp | Unused param suppression across many sub-actions |
| src/Ai/Base/Actions/RpgAction.cpp | Unused param suppression; include cleanup |
| src/Ai/Base/Actions/ReviveFromCorpseAction.cpp | Include cleanup; unused param suppression |
| src/Ai/Base/Actions/RevealGatheringItemAction.cpp | Replace umbrella include; unused param suppression |
| src/Ai/Base/Actions/ResetInstancesAction.cpp | Replace umbrella include; unused param suppression; remove master message |
| src/Ai/Base/Actions/RepairAllAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/RememberTaxiAction.cpp | Replace umbrella include; include cleanup |
| src/Ai/Base/Actions/ReleaseSpiritAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/ReachTargetActions.cpp | Unused param suppression (one-liner) |
| src/Ai/Base/Actions/RandomBotUpdateAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/QuestAction.cpp | Include cleanup; unused param suppression; small formatting change |
| src/Ai/Base/Actions/QueryQuestAction.cpp | Replace umbrella include; include cleanup |
| src/Ai/Base/Actions/PositionAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/PetsAction.h | Include cleanup |
| src/Ai/Base/Actions/PassLeadershipToMasterAction.cpp | Unused param suppression; include cleanup |
| src/Ai/Base/Actions/OpenItemAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/MoveToTravelTargetAction.cpp | Unused param suppression; include cleanup |
| src/Ai/Base/Actions/MoveToRpgTargetAction.cpp | Unused param suppression; remove unused local |
| src/Ai/Base/Actions/LootRollAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/LfgActions.cpp | Switch several sRandomPlayerbotMgr uses to singleton; unused param suppression |
| src/Ai/Base/Actions/LeaveGroupAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/InviteToGroupAction.cpp | Config singleton usage; random-bot singleton usage; unused param suppression |
| src/Ai/Base/Actions/InventoryAction.cpp | Minor loop braces/style cleanup |
| src/Ai/Base/Actions/ImbueAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/HireAction.cpp | Switch to RandomPlayerbotMgr::instance(); unused param suppression |
| src/Ai/Base/Actions/HelpAction.cpp | Replace umbrella include; unused param suppression |
| src/Ai/Base/Actions/GuildManagementActions.cpp | Unused param suppression; remove unused local |
| src/Ai/Base/Actions/GuildCreateActions.cpp | Unused param suppression; comment/include cleanup |
| src/Ai/Base/Actions/GuildBankAction.cpp | Replace umbrella include; include cleanup |
| src/Ai/Base/Actions/GreetAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/GiveItemAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/GenericSpellActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Base/Actions/GenericActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Base/Actions/FollowActions.cpp | Unused param suppression; include cleanup |
| src/Ai/Base/Actions/FishingAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/EquipAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/EmoteAction.cpp | Unused param suppression; include cleanup |
| src/Ai/Base/Actions/DestroyItemAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/DelayAction.cpp | Replace config global with singleton; unused param suppression |
| src/Ai/Base/Actions/DebugAction.cpp | Replace umbrella include; add narrower includes |
| src/Ai/Base/Actions/ChooseTravelTargetAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/ChooseTargetActions.cpp | Unused param suppression |
| src/Ai/Base/Actions/ChooseRpgTargetAction.cpp | Unused param suppression; include cleanup |
| src/Ai/Base/Actions/CheckValuesAction.cpp | Replace umbrella include; unused param suppression; include cleanup |
| src/Ai/Base/Actions/CheckMailAction.cpp | Config singleton usage; unused param suppression |
| src/Ai/Base/Actions/ChatShortcutActions.cpp | Unused param suppression across shortcuts |
| src/Ai/Base/Actions/ChangeTalentsAction.cpp | Unused param suppression; config/random-bot singleton usage |
| src/Ai/Base/Actions/CastCustomSpellAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/CancelChannelAction.cpp | Unused param suppression; minor formatting |
| src/Ai/Base/Actions/BossAuraActions.cpp | Unused param suppression |
| src/Ai/Base/Actions/BattleGroundTactics.cpp | Unused param suppression |
| src/Ai/Base/Actions/BattleGroundJoinAction.cpp | Unused param suppression; include cleanup |
| src/Ai/Base/Actions/AutoMaintenanceOnLevelupAction.cpp | Include cleanup; unused param suppression |
| src/Ai/Base/Actions/AreaTriggerAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/AddLootAction.cpp | Unused param suppression |
| src/Ai/Base/Actions/AcceptBattlegroundInvitationAction.cpp | Replace umbrella include; unused param suppression |
| conf/playerbots.conf.dist | Fix zone comment label (Duskwood) |
| PULL_REQUEST_TEMPLATE.md | Update checklist formatting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
96
to
97
| if (boss->HasAura(SPELL_PLANAR_SHIFT && dynamic_cast<OccDrakeAttackAction*>(action))) | ||
| { | ||
| return 0.0f; |
There was a problem hiding this comment.
HasAura is being called with the result of (SPELL_PLANAR_SHIFT && dynamic_cast<OccDrakeAttackAction*>(action)), which evaluates to 0/1 instead of a spell ID and effectively breaks the intended planar-shift check. Split this into two conditions: check boss->HasAura(SPELL_PLANAR_SHIFT) and separately check whether action is an OccDrakeAttackAction (or invert the logic accordingly).
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
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:
How to Test the Changes
Complexity & Impact
Does this change add new decision branches?
Does this change increase per-bot or per-tick processing?
Could this logic scale poorly under load?
Defaults & Configuration
Does this change modify default bot behavior?
If this introduces more advanced or AI-heavy logic:
AI Assistance
Was AI assistance (e.g. ChatGPT or similar tools) used while working on this change?
If yes, please specify:
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
Notes for Reviewers
Anything that significantly improves realism at the cost of stability or performance should be carefully discussed
before merging.