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 pull request implements raid strategies for Serpentshrine Cavern (SSC), a Burning Crusade raid instance. The PR adds comprehensive bot AI behavior for all bosses and trash encounters in SSC.
Changes:
- Added SSC strategy implementation with triggers, actions, and multipliers for all encounters
- Integrated SSC strategy into the existing raid strategy framework
- Updated configuration documentation to reflect SSC raid cheat support
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Bot/PlayerbotAI.cpp | Registered SSC map ID (548) to enable strategy activation |
| src/Bot/Engine/AiObjectContext.cpp | Added SSC action and trigger contexts to the AI framework |
| src/Ai/Raid/SerpentshrineCavern/Trigger/RaidSSCTriggers.h | Defined trigger classes for SSC encounters |
| src/Ai/Raid/SerpentshrineCavern/Trigger/RaidSSCTriggers.cpp | Implemented trigger logic for encounter mechanics |
| src/Ai/Raid/SerpentshrineCavern/Strategy/RaidSSCStrategy.h | Defined main SSC strategy class |
| src/Ai/Raid/SerpentshrineCavern/Strategy/RaidSSCStrategy.cpp | Implemented trigger-action bindings and multiplier setup |
| src/Ai/Raid/SerpentshrineCavern/RaidSSCTriggerContext.h | Registered all SSC triggers with creator functions |
| src/Ai/Raid/SerpentshrineCavern/RaidSSCHelpers.h | Declared helper functions, constants, and shared state |
| src/Ai/Raid/SerpentshrineCavern/RaidSSCHelpers.cpp | Implemented helper functions for encounter mechanics |
| src/Ai/Raid/SerpentshrineCavern/RaidSSCActionContext.h | Registered all SSC actions with creator functions |
| src/Ai/Raid/SerpentshrineCavern/Multiplier/RaidSSCMultipliers.h | Defined multiplier classes for action priority adjustment |
| src/Ai/Raid/SerpentshrineCavern/Multiplier/RaidSSCMultipliers.cpp | Implemented multiplier logic for encounter-specific behavior |
| src/Ai/Raid/SerpentshrineCavern/Action/RaidSSCActions.h | Defined action classes for all SSC encounters |
| src/Ai/Raid/RaidStrategyContext.h | Registered SSC strategy in the global strategy context |
| conf/playerbots.conf.dist | Updated raid cheat documentation to include SSC |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class LeotherasTheBlindHumanFormEngagedByMainTankTrigger : public Trigger | ||
| { | ||
| public: | ||
| LeotherasTheBlindHumanFormEngagedByMainTankTrigger( | ||
| PlayerbotAI* botAI) : Trigger(botAI, "leotheras the blind human form engaged by main tank") {} | ||
| bool IsActive() override; | ||
| }; | ||
|
|
There was a problem hiding this comment.
This trigger class is declared but never implemented or used. The IsActive method is not defined in RaidSSCTriggers.cpp, and it's not registered in RaidSSCTriggerContext.h. This appears to be dead code that should be removed.
| class LeotherasTheBlindHumanFormEngagedByMainTankTrigger : public Trigger | |
| { | |
| public: | |
| LeotherasTheBlindHumanFormEngagedByMainTankTrigger( | |
| PlayerbotAI* botAI) : Trigger(botAI, "leotheras the blind human form engaged by main tank") {} | |
| bool IsActive() override; | |
| }; |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class LeotherasTheBlindHumanFormEngagedByMainTankTrigger : public Trigger | ||
| { | ||
| public: | ||
| LeotherasTheBlindHumanFormEngagedByMainTankTrigger( | ||
| PlayerbotAI* botAI) : Trigger(botAI, "leotheras the blind human form engaged by main tank") {} | ||
| bool IsActive() override; | ||
| }; | ||
|
|
There was a problem hiding this comment.
The class LeotherasTheBlindHumanFormEngagedByMainTankTrigger is declared but never implemented or registered in the trigger context. This will cause a linker error when the code is compiled, as the IsActive() method has no implementation. Either implement and register this trigger, or remove the unused class declaration.
| class LeotherasTheBlindHumanFormEngagedByMainTankTrigger : public Trigger | |
| { | |
| public: | |
| LeotherasTheBlindHumanFormEngagedByMainTankTrigger( | |
| PlayerbotAI* botAI) : Trigger(botAI, "leotheras the blind human form engaged by main tank") {} | |
| bool IsActive() override; | |
| }; |
| # "power" (bots have infinite energy, rage, and runic power) | ||
| # "taxi" (bots may use all flight paths, though they will not actually learn them) | ||
| # "raid" (bots use cheats implemented into raid strategies (currently only for Ulduar)) | ||
| # "raid" (bots use cheats implemented into raid strategies (currently only for SSC and Ulduar)) |
There was a problem hiding this comment.
The updated comment states that raid cheats are "currently only for SSC and Ulduar", but based on the codebase context, there may be other raids that also use the raid cheat functionality. Verify that this comment accurately reflects all raids that use the raid cheat feature, or update it to be more general (e.g., "currently for certain raid encounters").
| # "raid" (bots use cheats implemented into raid strategies (currently only for SSC and Ulduar)) | |
| # "raid" (bots use cheats implemented into certain raid encounter strategies) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Unit* GetNearestActiveShieldGeneratorTriggerByEntry(Unit* reference) | ||
| { | ||
| if (!reference) | ||
| return nullptr; | ||
|
|
||
| std::list<Creature*> triggers; | ||
| constexpr float searchRange = 150.0f; | ||
| reference->GetCreatureListWithEntryInGrid( | ||
| triggers, NPC_WORLD_INVISIBLE_TRIGGER, searchRange); | ||
|
|
||
| Creature* nearest = nullptr; | ||
| float minDist = std::numeric_limits<float>::max(); |
There was a problem hiding this comment.
RaidSSCHelpers.cpp uses std::list and std::numeric_limits but does not include or . Relying on transitive includes is fragile and can break builds; add the missing standard headers here.
| // If raid cheat (which enables bot looting of the core) is not enabled | ||
| // Bots should not loot the core | ||
| float LadyVashjDoNotLootTheTaintedCoreMultiplier::GetValue(Action* action) | ||
| { | ||
| if (!botAI->HasCheat(BotCheatMask::raid)) | ||
| return 1.0f; | ||
|
|
||
| if (AI_VALUE2(Unit*, "find target", "lady vashj")) | ||
| { | ||
| if (dynamic_cast<LootAction*>(action)) | ||
| return 0.0f; | ||
| } |
There was a problem hiding this comment.
LadyVashjDoNotLootTheTaintedCoreMultiplier logic is inverted relative to the comment: when the raid cheat is NOT enabled it currently returns 1.0f (allowing normal LootAction), and when the cheat IS enabled it disables LootAction. If the intent is to prevent bots from looting the core unless the cheat-driven core handling is active, invert the HasCheat check (or update the comment/logic so they match).
| if (vashjRangedPositions.erase(guid) > 0) | ||
| erased = true; | ||
| if (hasReachedVashjRangedPosition.erase(guid) > 0) | ||
| erased = true; |
There was a problem hiding this comment.
SerpentShrineCavernEraseTimersAndTrackersAction clears some Vashj state (ranged positions) when Lady Vashj is not present, but it does not clear instance-level core-passing trackers (nearestTriggerGuid, lastImbueAttempt, lastCoreInInventoryTime) or per-bot intendedLineup. After a wipe/leave (when "lady vashj" target is null) this can leave stale state that affects subsequent attempts; clear those maps here too when Vashj is not found.
| erased = true; | |
| erased = true; | |
| if (nearestTriggerGuid.erase(instanceId) > 0) | |
| erased = true; | |
| if (lastImbueAttempt.erase(instanceId) > 0) | |
| erased = true; | |
| if (lastCoreInInventoryTime.erase(instanceId) > 0) | |
| erased = true; | |
| if (intendedLineup.erase(guid) > 0) | |
| erased = true; |
| dynamic_cast<CastKillingSpreeAction*>(action) || | ||
| dynamic_cast<CastReachTargetSpellAction*>(action)) |
There was a problem hiding this comment.
Duplicate condition: CastReachTargetSpellAction is checked twice in the same if-chain, which is redundant and makes the list harder to maintain. Remove the duplicate entry.
| dynamic_cast<CastKillingSpreeAction*>(action) || | |
| dynamic_cast<CastReachTargetSpellAction*>(action)) | |
| dynamic_cast<CastKillingSpreeAction*>(action)) |
| struct GeneratorInfo { ObjectGuid guid; float x, y, z; }; | ||
| extern const std::vector<uint32> SHIELD_GENERATOR_DB_GUIDS; | ||
| std::vector<GeneratorInfo> GetAllGeneratorInfosByDbGuids( | ||
| Map* map, const std::vector<uint32>& generatorDbGuids); | ||
| Unit* GetNearestActiveShieldGeneratorTriggerByEntry(Unit* reference); | ||
| const GeneratorInfo* GetNearestGeneratorToBot( | ||
| Player* bot, const std::vector<GeneratorInfo>& generators); |
There was a problem hiding this comment.
RaidSSCHelpers.h uses std::vector in public declarations (e.g., SHIELD_GENERATOR_DB_GUIDS / GetAllGeneratorInfosByDbGuids) but does not include . This can break compilation depending on include order; include in this header (or avoid exposing std::vector in the header).
2 participants
No description provided.