Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to make bot gathering behavior reliable by preventing selection of unusable nodes, stopping gathering casts when combat starts, and moving dungeon/distance gating into isUseful() for the gathering loot action.
Changes:
- Skip gathering gameobjects unless they are in
GO_READYloot state to avoid repeatedly targeting non-ready nodes. - Adjust AI update behavior to interrupt the bot’s current cast when the master enters combat (to avoid continuing gathering while the group is fighting).
- Add
AddGatheringLootAction::isUseful()to gate gathering by instance type and master distance, and delegate remaining usefulness checks to the base class.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/modules/Bots/playerbot/strategy/actions/AddLootAction.h |
Declares AddGatheringLootAction::isUseful() so gathering-specific gating can live at the action level. |
src/modules/Bots/playerbot/strategy/actions/AddLootAction.cpp |
Moves dungeon/distance gating into isUseful() and simplifies AddLoot() to just add eligible gathering loot. |
src/modules/Bots/playerbot/PlayerbotAI.cpp |
Attempts to interrupt bot spellcasting when master is in combat to avoid continuing gathering during fights. |
src/modules/Bots/playerbot/LootObjectStack.cpp |
Filters gameobjects by GO_READY loot state when building loot targets to avoid “infinite gather” loops on non-ready nodes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (nextAICheckDelay > sPlayerbotAIConfig.maxWaitForMove && | ||
| !bot->GetCurrentSpell(CURRENT_CHANNELED_SPELL)) | ||
| { | ||
| nextAICheckDelay = sPlayerbotAIConfig.maxWaitForMove; | ||
| if (bot->IsInCombat()) | ||
| nextAICheckDelay = sPlayerbotAIConfig.maxWaitForMove; | ||
| else | ||
| { | ||
| Player* master = GetMaster(); | ||
| if (master && master->IsInCombat()) | ||
| { | ||
| InterruptSpell(); | ||
| nextAICheckDelay = sPlayerbotAIConfig.maxWaitForMove; |
There was a problem hiding this comment.
This change intends to interrupt gathering "channels" when the master enters combat, but the new condition explicitly excludes CURRENT_CHANNELED_SPELL, and InterruptSpell() also returns early when a channeled spell is active. If gathering (or other relevant actions) can be implemented as channeled spells, this will not interrupt them; consider handling the channeled case explicitly (or adjusting InterruptSpell()/the condition) so the behavior matches the PR intent.
There was a problem hiding this comment.
Gathering spells aren't channeled, and the channel guard was inherited, so I can't say why. InterruptSpell is for cancelling casting times, not chanelling.
There was a problem hiding this comment.
IOW, Copilot is just wrong here.
I have fought and fought with bot gathering, and this is it-- three fixes that together make bot gathering actually work:
This change is