Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces PvP behavior constraints for bots, including level-difference-based target filtering and alignment of bot aggression with the master’s PvP state, plus a small cleanup in trigger targeting.
Changes:
- Extend
PossibleTargetsValue::AcceptUnitto add PvP-specific rules: self‑defense exceptions, BG/arena and duel exemptions, capital-city handling, level-difference–based attack probabilities with a per-(bot,target) cache guarded by a mutex. - Make
NearestEnemyPlayersValue::AcceptUnitdelegate toPossibleTargetsValue::AcceptUnitfor common filtering (including the new level rules) and add a master PvP-flag check so bots only engage enemy players when their master is flagged. - Simplify
PossibleTriggersValue::AcceptUnitby removing unnecessary braces around single-line conditionals.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/Ai/Base/Value/PossibleTargetsValue.cpp |
Adds PvP-aware acceptance logic for potential targets, including level-difference probability and a thread-safe decision cache, and introduces a capital-city exception and minor style changes in trigger selection. |
src/Ai/Base/Value/EnemyPlayerValue.cpp |
Reuses PossibleTargetsValue’s filtering (including level rules) for nearest enemy players and ensures bots only attack enemy players when their master is PvP/FFA flagged. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| uint32 zoneId = bot->GetZoneId(); | ||
| bool inCapitalCity = (zoneId == AREA_STORMWIND_CITY || | ||
| zoneId == AREA_IRONFORGE || | ||
| zoneId == AREA_DARNASSUS || | ||
| zoneId == AREA_THE_EXODAR || | ||
| zoneId == AREA_ORGRIMMAR || | ||
| zoneId == AREA_THUNDER_BLUFF || | ||
| zoneId == AREA_UNDERCITY || | ||
| zoneId == AREA_SILVERMOON_CITY); |
There was a problem hiding this comment.
The AREA_* constants used in this capital city check (e.g. AREA_STORMWIND_CITY, AREA_ORGRIMMAR, etc.) are not defined anywhere in the current codebase, and the only reference to AreaDefines.h is this new include. This will cause compilation failures due to undefined identifiers unless AreaDefines.h and these macros are added elsewhere in the project. Either include an existing header that already defines these IDs, or switch to using AreaTableEntry flags (e.g. AREA_FLAG_CAPITAL) consistently with the rest of the codebase.
| uint32 zoneId = bot->GetZoneId(); | ||
| bool inCapitalCity = (zoneId == AREA_STORMWIND_CITY || | ||
| zoneId == AREA_IRONFORGE || | ||
| zoneId == AREA_DARNASSUS || | ||
| zoneId == AREA_THE_EXODAR || | ||
| zoneId == AREA_ORGRIMMAR || | ||
| zoneId == AREA_THUNDER_BLUFF || | ||
| zoneId == AREA_UNDERCITY || | ||
| zoneId == AREA_SILVERMOON_CITY); |
There was a problem hiding this comment.
Hard-coding a list of capital city zone IDs here diverges from the established convention elsewhere in the codebase, where capital status is determined via AreaTableEntry flags (for example, see src/Ai/Base/Actions/GuildCreateActions.cpp:288-293 and 323-329, and src/Ai/World/Rpg/Action/NewRpgBaseAction.cpp:878-883, 930-936, 1025-1030). To keep things maintainable and compatible with future content changes, consider reusing the same AREA_FLAG_CAPITAL-based check instead of maintaining a separate list of explicit zone IDs.
| // Calculate attack chance based on level difference | ||
| uint32 attackChance = 100; // Default 100%: Bot and target's levels are very close | ||
|
|
||
| // There's a chance a bot might gank on an extremly low target |
There was a problem hiding this comment.
In the comment "There's a chance a bot might gank on an extremly low target" the word "extremly" is misspelled. It should be "extremely" for correct spelling.
| // There's a chance a bot might gank on an extremly low target | |
| // There's a chance a bot might gank on an extremely low target |
No description provided.