Conversation
All bosses
including removing Tainted Elementals from general target priority logic
Also took Sporebats off of prio list in Vashj P3. Bots are going in midair to kill them--it looks stupid and is basically cheating, and I don't know how to stop it.
also require tanks to be alive to maintain roles
There was a problem hiding this comment.
Pull request overview
This PR introduces a Serpentshrine Cavern (“ssc”, map 548) raid strategy for Playerbots, wiring it into the existing raid-strategy selection/registration flow and adding the related SSC triggers, actions, helpers, and multipliers.
Changes:
- Add SSC instance strategy selection (mapId 548 →
ssc) and register SSC strategy/context objects in the AI engine. - Implement SSC-specific triggers/multipliers/helpers/strategy scaffolding to drive raid behaviors (Hydross, Lurker, Leotheras, Karathress, Tidewalker, Vashj).
- Update playerbots config documentation to reflect SSC raid-cheat usage.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Bot/PlayerbotAI.cpp | MapId 548 now maps to the ssc instance strategy. |
| src/Bot/Engine/AiObjectContext.cpp | Registers SSC shared action/trigger contexts. |
| src/Ai/Raid/SerpentshrineCavern/Util/RaidSSCHelpers.h / .cpp | SSC constants, timers/trackers, helper queries for encounter logic. |
| src/Ai/Raid/SerpentshrineCavern/Trigger/RaidSSCTriggers.h / .cpp | SSC trigger set controlling encounter behaviors. |
| src/Ai/Raid/SerpentshrineCavern/Strategy/RaidSSCStrategy.h / .cpp | Strategy wiring: trigger nodes and multipliers for SSC. |
| src/Ai/Raid/SerpentshrineCavern/RaidSSCTriggerContext.h | Named trigger factory registrations for SSC triggers. |
| src/Ai/Raid/SerpentshrineCavern/RaidSSCActionContext.h | Named action factory registrations for SSC actions. |
| src/Ai/Raid/SerpentshrineCavern/Multiplier/RaidSSCMultipliers.h / .cpp | SSC multipliers that suppress/shape default behaviors during mechanics. |
| src/Ai/Raid/SerpentshrineCavern/Action/RaidSSCActions.h | Declares SSC action implementations used by the strategy. |
| src/Ai/Raid/RaidStrategyContext.h | Registers the ssc raid strategy. |
| conf/playerbots.conf.dist | Updates raid-cheat comment to include SSC. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!botAI->HasCheat(BotCheatMask::raid)) | ||
| return 1.0f; | ||
|
|
||
| if (AI_VALUE2(Unit*, "find target", "lady vashj")) | ||
| { | ||
| if (dynamic_cast<LootAction*>(action)) |
There was a problem hiding this comment.
LadyVashjDoNotLootTheTaintedCoreMultiplier has its raid-cheat condition inverted: the comment says bots should NOT loot the tainted core when the raid cheat is NOT enabled, but the code currently disables LootAction only when the cheat IS enabled. This will allow bots to loot the core in non-cheat mode (likely breaking the intended core-passing flow) and may also unnecessarily disable looting when cheat mode is on. Flip the condition so generic looting is blocked during the Vashj encounter unless raid cheat is enabled (or otherwise align code + comment).
| if (!botAI->HasCheat(BotCheatMask::raid)) | |
| return 1.0f; | |
| if (AI_VALUE2(Unit*, "find target", "lady vashj")) | |
| { | |
| if (dynamic_cast<LootAction*>(action)) | |
| if (AI_VALUE2(Unit*, "find target", "lady vashj")) | |
| { | |
| // Without the raid cheat, prevent generic looting of the core during the Vashj encounter | |
| if (!botAI->HasCheat(BotCheatMask::raid) && | |
| dynamic_cast<LootAction*>(action)) |
| if (Group* group = bot->GetGroup()) | ||
| { | ||
| for (GroupReference* ref = group->GetFirstMember(); ref; ref = ref->next()) | ||
| { | ||
| Player* member = ref->GetSource(); | ||
| if (member && member->HasAura(SPELL_STATIC_CHARGE)) | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; |
There was a problem hiding this comment.
LadyVashjBotHasStaticChargeTrigger name/behavior mismatch: it currently returns true if ANY group member has Static Charge, which will make every bot run the "static charge move away from group" action. This should likely check whether this bot has SPELL_STATIC_CHARGE (or rename the trigger + adjust actions if the intent is raid-wide behavior).
| if (Group* group = bot->GetGroup()) | |
| { | |
| for (GroupReference* ref = group->GetFirstMember(); ref; ref = ref->next()) | |
| { | |
| Player* member = ref->GetSource(); | |
| if (member && member->HasAura(SPELL_STATIC_CHARGE)) | |
| return true; | |
| } | |
| } | |
| return false; | |
| return bot->HasAura(SPELL_STATIC_CHARGE); |
| if (Group* group = bot->GetGroup()) | ||
| { | ||
| for (GroupReference* ref = group->GetFirstMember(); ref; ref = ref->next()) | ||
| { | ||
| Player* member = ref->GetSource(); | ||
| if (!member || !member->HasAura(SPELL_ENTANGLE)) | ||
| continue; | ||
|
|
||
| if (botAI->IsMelee(member)) | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; |
There was a problem hiding this comment.
LadyVashjBotIsEntangledInToxicSporesOrStaticChargeTrigger also appears to be raid-wide instead of per-bot: it scans the group and triggers if any (melee) member has SPELL_ENTANGLE, causing all bots to execute "use free action abilities". This should check the current bot's relevant auras (and/or include Static Charge if that’s intended by the trigger name) so only affected bots react.
| if (Group* group = bot->GetGroup()) | |
| { | |
| for (GroupReference* ref = group->GetFirstMember(); ref; ref = ref->next()) | |
| { | |
| Player* member = ref->GetSource(); | |
| if (!member || !member->HasAura(SPELL_ENTANGLE)) | |
| continue; | |
| if (botAI->IsMelee(member)) | |
| return true; | |
| } | |
| } | |
| return false; | |
| // Trigger only when this bot is actually affected, not when any group member is. | |
| return bot->HasAura(SPELL_ENTANGLE); |
2 participants
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.