Skip to content

Commit a76f2ca

Browse files
authored
Refactor HasSpell/HasAura and convert spellIds to constants (mod-playerbots#2435)
<!-- Thank you for contributing to mod-playerbots, please make sure that you... 1. Submit your PR to the test-staging branch, not master. 2. Read the guidelines below before submitting. 3. Don't delete parts of this template. DESIGN PHILOSOPHY: We prioritize STABILITY, PERFORMANCE, AND PREDICTABILITY over behavioral realism. Every action and decision executes PER BOT AND PER TRIGGER. Small increases in logic complexity scale poorly across thousands of bots and negatively affect all. We prioritize a stable system over a smarter one. Bots don't need to behave perfectly; believable behavior is the goal, not human simulation. Default behavior must be cheap in processing; expensive behavior must be opt-in. Before submitting, make sure your changes aligns with these principles. --> ## Pull Request Description <!-- Describe what this change does and why it is needed --> This is a PR to follow-up on a discussion with @kadeshar during the recent mage armor PR. Main changes: - Add new PlayerbotAI::HasSpell() overload that accepts a string for the spell name as an argument, in order to replace repeated bot->HasSpell(spellId) checks for places where multiple ranks of a spell are checked, and to replace a few calls to AI_VALUE(uint32, "spell id", ...) where it existed only to check for the spell being known. - Removed PlayerbotAI::HasAura() overload that takes a spell Id as an argument. It is rarely used, and the few instances of its usage are easily replaced with AC's Unit::HasAura(), which is the predominant usage for spell Id aura checks already anyway. - When modifying DruidPullStrategy to use the new HasSpell() method, I went ahead and also simplified the file overall to pare down duplication (for example, the first check in CanCastSpell is for the spell Id so we don't need to separately check it before calling CanCastSpell()). I then did the same for other pull strategies so they can be consistent. - Changed many magic numbers for spell Ids to be defined as compile-time constants. I didn't do this throughout the code, but I did in the files where I was making changes anyway due to the HasSpell() and/or HasAura() changes. ## Feature Evaluation <!-- If your PR is very minimal (comment typo, wrong ID reference, etc), and it is very obvious it will not have any impact on performance, you may skip these question. If necessary, a maintainer may ask you for them later. --> <!-- Please answer the following: --> - Describe the **minimum logic** required to achieve the intended behavior. - Describe the **processing cost** when this logic executes across many bots. - Minimum logic is described above. - Processing cost shouldn't change much. See Impact Assessment below. ## How to Test the Changes <!-- - Step-by-step instructions to test the change. - Any required setup (e.g. multiple players, number of bots, specific configuration). - Expected behavior and how to verify it. --> ## Impact Assessment <!-- As a generic test, before and after measure of pmon (playerbot pmon tick) can help you here. --> - Does this change increase per-bot/per-tick processing or risk scaling poorly with thousands of bots? - - [ ] No, not at all - - [x] Minimal impact (**explain below**) - - [ ] Moderate impact (**explain below**) - Passing a string for the spell to check for it being known or the aura being present is more taxing than looking up a spell Id, although the usefulness of the string-based overload is for situations with spells that have multiple ranks. - Does this change modify default bot behavior? - - [x] No - - [ ] Yes (**explain why**) - Does this change add new decision branches or increase maintenance complexity? - - [x] No - - [ ] Yes (**explain below**) - I think these changes should simplify maintenance complexity. ## AI Assistance <!-- AI assistance is allowed, but all submitted code must be fully understood, reviewed, and owned by the contributor. We expect contributors to be honest about what they do and do not understand. --> Was AI assistance used while working on this change? - - [ ] No - - [x] Yes (**explain below**) <!-- If yes, please specify: - Purpose of usage (e.g. brainstorming, refactoring, documentation, code generation). - Which parts of the change were influenced or generated, and whether it was thoroughly reviewed. --> - I had AI make many of the changes and then review since they were pretty mundane (for example, I'd tell it to just take all the magic numbers from a file and define them in an anonymous namespace, then replace the magic numbers with the spell constants). There isn't anything complicated in this PR; it was mostly busywork. <!-- TRANSLATIONS: Anything new that the bots say in chat must be in a translatable format. This is done using GetBotTextOrDefault, which you can search for in the codebase to find examples. Your code needs to have English as the default fallback, while the full translations need to be in an SQL update file. The languages in the file are the nine language options supported by AzerothCore: English, Korean, French, German, Chinese, Taiwanese, Spanish, Spanish Mexico, and Russian. See data/sql/playerbots/updates/2025_12_27_ai_playerbot_fishing_text.sql as an example of a translation SQL update, whose content are called within the codebase at src/strategy/actions/FishingAction.cpp --> ## Final Checklist - - [x] Stability is not compromised. - - [x] Performance impact is understood, tested, and acceptable. - - [x] Added logic complexity is justified and explained. - - [x] Any new bot dialogue lines are translated. - - [x] Documentation updated if needed (Conf comments, WiKi commands). ## Notes for Reviewers <!-- Anything else that's helpful to review or test your pull request. -->
1 parent dda9ff0 commit a76f2ca

34 files changed

Lines changed: 329 additions & 289 deletions

src/Ai/Base/Actions/TameAction.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -422,10 +422,9 @@ bool TameAction::RenamePet(const std::string& newName)
422422

423423
// Remove the current pet and (re-)cast Call Pet spell if the bot is a hunter
424424
bot->RemovePet(nullptr, PET_SAVE_AS_CURRENT, true);
425-
if (bot->getClass() == CLASS_HUNTER && bot->HasSpell(883))
426-
{
427-
bot->CastSpell(bot, 883, true);
428-
}
425+
constexpr uint32 SPELL_CALL_PET = 883;
426+
if (bot->getClass() == CLASS_HUNTER && bot->HasSpell(SPELL_CALL_PET))
427+
bot->CastSpell(bot, SPELL_CALL_PET, true);
429428

430429
return true;
431430
}

src/Ai/Base/Actions/TradeStatusExtendedAction.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,10 @@ bool TradeStatusExtendedAction::Execute(Event event)
6767
return false;
6868
}
6969

70-
if (bot->getClass() == CLASS_ROGUE && bot->HasSpell(1804) && lockbox->IsLocked()) // Pick Lock spell
70+
constexpr uint32 SPELL_PICK_LOCK = 1804;
71+
if (bot->getClass() == CLASS_ROGUE && bot->HasSpell(SPELL_PICK_LOCK) && lockbox->IsLocked())
7172
{
72-
// botAI->CastSpell(1804, bot, lockbox); // Attempt to cast Pick Lock on the lockbox
73+
// botAI->CastSpell(SPELL_PICK_LOCK, bot, lockbox); // Attempt to cast Pick Lock on the lockbox
7374
botAI->DoSpecificAction("unlock traded item");
7475
botAI->SetNextCheckDelay(4000); // Delay before accepting trade
7576
}

src/Ai/Base/Trigger/BossAuraTriggers.cpp

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,7 @@ bool BossFireResistanceTrigger::IsActive()
2323
return false;
2424

2525
// Check if bot have fire resistance aura
26-
if (bot->HasAura(SPELL_FIRE_RESISTANCE_AURA_RANK_5) || bot->HasAura(SPELL_FIRE_RESISTANCE_AURA_RANK_4) ||
27-
bot->HasAura(SPELL_FIRE_RESISTANCE_AURA_RANK_3) || bot->HasAura(SPELL_FIRE_RESISTANCE_AURA_RANK_2) ||
28-
bot->HasAura(SPELL_FIRE_RESISTANCE_AURA_RANK_1))
26+
if (botAI->HasAura("fire resistance aura", bot))
2927
return false;
3028

3129
// Check if bot dont have already have fire resistance strategy
@@ -76,9 +74,7 @@ bool BossFrostResistanceTrigger::IsActive()
7674
return false;
7775

7876
// Check if bot have frost resistance aura
79-
if (bot->HasAura(SPELL_FROST_RESISTANCE_AURA_RANK_5) || bot->HasAura(SPELL_FROST_RESISTANCE_AURA_RANK_4) ||
80-
bot->HasAura(SPELL_FROST_RESISTANCE_AURA_RANK_3) || bot->HasAura(SPELL_FROST_RESISTANCE_AURA_RANK_2) ||
81-
bot->HasAura(SPELL_FROST_RESISTANCE_AURA_RANK_1))
77+
if (botAI->HasAura("frost resistance aura", bot))
8278
return false;
8379

8480
// Check if bot dont have already have frost resistance strategy
@@ -133,8 +129,7 @@ bool BossNatureResistanceTrigger::IsActive()
133129
return false;
134130

135131
// Check if bot have nature resistance aura
136-
if (bot->HasAura(SPELL_ASPECT_OF_THE_WILD_RANK_4) || bot->HasAura(SPELL_ASPECT_OF_THE_WILD_RANK_3) ||
137-
bot->HasAura(SPELL_ASPECT_OF_THE_WILD_RANK_2) || bot->HasAura(SPELL_ASPECT_OF_THE_WILD_RANK_1))
132+
if (botAI->HasAura("aspect of the wild", bot))
138133
return false;
139134

140135
// Check if bot dont have already setted nature resistance aura
@@ -184,11 +179,7 @@ bool BossShadowResistanceTrigger::IsActive()
184179
return false;
185180

186181
// Check if bot have shadow resistance aura
187-
if (bot->HasAura(SPELL_SHADOW_RESISTANCE_AURA_RANK_5) ||
188-
bot->HasAura(SPELL_SHADOW_RESISTANCE_AURA_RANK_4) ||
189-
bot->HasAura(SPELL_SHADOW_RESISTANCE_AURA_RANK_3) ||
190-
bot->HasAura(SPELL_SHADOW_RESISTANCE_AURA_RANK_2) ||
191-
bot->HasAura(SPELL_SHADOW_RESISTANCE_AURA_RANK_1))
182+
if (botAI->HasAura("shadow resistance aura", bot))
192183
return false;
193184

194185
// Check if bot dont have already have shadow resistance strategy

src/Ai/Class/Dk/Strategy/DeathKnightPullStrategy.cpp

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,38 +5,26 @@
55

66
#include "DeathKnightPullStrategy.h"
77

8-
#include "AiObjectContext.h"
98
#include "Player.h"
109
#include "PlayerbotAI.h"
1110
#include "Playerbots.h"
1211

1312
std::string DeathKnightPullStrategy::GetPullActionName() const
1413
{
15-
Player* bot = botAI->GetBot();
1614
Unit* target = GetTarget();
17-
if (!bot || !target ||
15+
if (!target ||
1816
(!botAI->HasStrategy("blood", BOT_STATE_COMBAT) && !botAI->HasStrategy("blood", BOT_STATE_NON_COMBAT)))
1917
{
2018
return PullStrategy::GetPullActionName();
2119
}
2220

23-
uint32 const deathGripSpellId = botAI->GetAiObjectContext()->GetValue<uint32>("spell id", "death grip")->Get();
24-
if (deathGripSpellId && bot->HasSpell(deathGripSpellId) &&
25-
botAI->CanCastSpell(deathGripSpellId, target))
26-
{
21+
if (botAI->CanCastSpell("death grip", target))
2722
return "death grip";
28-
}
2923

30-
uint32 const icyTouchSpellId = botAI->GetAiObjectContext()->GetValue<uint32>("spell id", "icy touch")->Get();
31-
if (!icyTouchSpellId || !bot->HasSpell(icyTouchSpellId) ||
32-
!botAI->CanCastSpell(icyTouchSpellId, target))
24+
if (!botAI->CanCastSpell("icy touch", target) &&
25+
botAI->CanCastSpell("dark command", target))
3326
{
34-
uint32 const darkCommandSpellId = botAI->GetAiObjectContext()->GetValue<uint32>("spell id", "dark command")->Get();
35-
if (darkCommandSpellId && bot->HasSpell(darkCommandSpellId) &&
36-
botAI->CanCastSpell(darkCommandSpellId, target))
37-
{
38-
return "dark command";
39-
}
27+
return "dark command";
4028
}
4129

4230
return PullStrategy::GetPullActionName();

src/Ai/Class/Druid/Action/DruidShapeshiftActions.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,10 @@ bool CastCancelDruidAction::Execute(Event /*event*/)
5050
return true;
5151
}
5252

53-
bool CastCancelDruidAction::isUseful() { return botAI->HasAura(auraId, bot); }
53+
bool CastCancelDruidAction::isUseful() { return bot->HasAura(auraId); }
5454

5555
bool CastTreeFormAction::isUseful()
5656
{
57-
return GetTarget() && CastSpellAction::isUseful() && !botAI->HasAura(33891, bot);
57+
constexpr uint32 SPELL_TREE_OF_LIFE = 33891;
58+
return GetTarget() && CastSpellAction::isUseful() && !bot->HasAura(SPELL_TREE_OF_LIFE);
5859
}

src/Ai/Class/Druid/DruidTriggers.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,11 @@ bool ThornsTrigger::IsActive() { return BuffTrigger::IsActive() && !botAI->HasAu
2828

2929
bool BearFormTrigger::IsActive() { return !botAI->HasAnyAuraOf(bot, "bear form", "dire bear form", nullptr); }
3030

31-
bool TreeFormTrigger::IsActive() { return !botAI->HasAura(33891, bot); }
31+
bool TreeFormTrigger::IsActive()
32+
{
33+
constexpr uint32 SPELL_TREE_OF_LIFE = 33891;
34+
return !bot->HasAura(SPELL_TREE_OF_LIFE);
35+
}
3236

3337
bool CatFormTrigger::IsActive() { return !botAI->HasAura("cat form", bot); }
3438

@@ -43,8 +47,11 @@ bool ProwlTrigger::IsActive()
4347
if (botAI->HasAura("prowl", bot) || bot->IsInCombat())
4448
return false;
4549

46-
uint32 prowlId = botAI->GetAiObjectContext()->GetValue<uint32>("spell id", "prowl")->Get();
47-
if (!prowlId || !bot->HasSpell(prowlId) || bot->HasSpellCooldown(prowlId))
50+
if (!botAI->HasSpell("prowl"))
51+
return false;
52+
53+
uint32 const prowlId = AI_VALUE2(uint32, "spell id", "prowl");
54+
if (bot->HasSpellCooldown(prowlId))
4855
return false;
4956

5057
float distance = 30.f;

src/Ai/Class/Druid/DruidTriggers.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,14 +393,14 @@ class FerociousBiteTimeTrigger : public Trigger
393393
class FerociousBiteExecuteTrigger : public Trigger
394394
{
395395
public:
396-
FerociousBiteExecuteTrigger(PlayerbotAI* ai) : Trigger(ai, "ferocious bite execute") {}
396+
FerociousBiteExecuteTrigger(PlayerbotAI* botAI) : Trigger(botAI, "ferocious bite execute") {}
397397
bool IsActive() override
398398
{
399399
Unit* target = AI_VALUE(Unit*, "current target");
400400
if (!target || !target->IsAlive())
401401
return false;
402402

403-
if (!AI_VALUE2(uint32, "spell id", "ferocious bite"))
403+
if (!botAI->HasSpell("ferocious bite"))
404404
return false;
405405

406406
if (AI_VALUE2(uint8, "combo", "current target") < 1)

src/Ai/Class/Druid/Strategy/DruidPullStrategy.cpp

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,34 +5,23 @@
55

66
#include "DruidPullStrategy.h"
77

8-
#include "AiObjectContext.h"
9-
#include "Player.h"
108
#include "PlayerbotAI.h"
119
#include "Playerbots.h"
1210

1311
std::string DruidPullStrategy::GetPullActionName() const
1412
{
15-
Player* bot = botAI->GetBot();
16-
std::string actionName = PullStrategy::GetPullActionName();
17-
if (!bot)
18-
return actionName;
19-
20-
uint32 const faerieFireFeralId = botAI->GetAiObjectContext()->GetValue<uint32>("spell id", "faerie fire (feral)")->Get();
21-
if (faerieFireFeralId && bot->HasSpell(faerieFireFeralId) &&
22-
(botAI->HasStrategy("bear", BOT_STATE_COMBAT) || botAI->HasStrategy("cat", BOT_STATE_COMBAT)))
23-
{
24-
actionName = "faerie fire (feral)";
25-
}
13+
std::string const pullActionName = PullStrategy::GetPullActionName();
14+
std::string const actionName =
15+
botAI->HasSpell("faerie fire (feral)") &&
16+
(botAI->HasStrategy("bear", BOT_STATE_COMBAT) || botAI->HasStrategy("cat", BOT_STATE_COMBAT))
17+
? "faerie fire (feral)" : pullActionName;
2618

2719
Unit* target = GetTarget();
28-
uint32 const faerieFireSpellId = botAI->GetAiObjectContext()->GetValue<uint32>("spell id", actionName)->Get();
29-
if (target && (!faerieFireSpellId || !bot->HasSpell(faerieFireSpellId) ||
30-
!botAI->CanCastSpell(faerieFireSpellId, target)))
31-
{
32-
uint32 const growlSpellId = botAI->GetAiObjectContext()->GetValue<uint32>("spell id", "growl")->Get();
33-
if (growlSpellId && bot->HasSpell(growlSpellId) && botAI->CanCastSpell(growlSpellId, target))
34-
return "growl";
35-
}
20+
if (!target)
21+
return actionName;
22+
23+
if (!botAI->CanCastSpell(actionName, target) && botAI->CanCastSpell("growl", target))
24+
return "growl";
3625

3726
return actionName;
3827
}

src/Ai/Class/Hunter/HunterActions.cpp

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,13 @@ bool CastViperStingAction::isUseful()
1919
bool CastAspectOfTheHawkAction::isUseful()
2020
{
2121
Unit* target = GetTarget();
22-
if (!target)
23-
return false;
24-
25-
if (bot->HasSpell(61846) || bot->HasSpell(61847)) // Aspect of the Dragonhawk spell IDs
26-
return false;
27-
28-
return true;
22+
return target && !botAI->HasSpell("aspect of the dragonhawk");
2923
}
3024

3125
bool CastArcaneShotAction::isUseful()
3226
{
3327
Unit* target = GetTarget();
34-
if (!target)
35-
return false;
36-
37-
if (bot->HasSpell(53301) || bot->HasSpell(60051) ||
38-
bot->HasSpell(60052) || bot->HasSpell(60053)) // Explosive Shot spell IDs
28+
if (!target || !botAI->HasSpell("explosive shot"))
3929
return false;
4030

4131
// Armor Penetration rating check - will not cast Arcane Shot above 435 ArP
@@ -50,14 +40,7 @@ bool CastArcaneShotAction::isUseful()
5040
bool CastImmolationTrapAction::isUseful()
5141
{
5242
Unit* target = GetTarget();
53-
if (!target)
54-
return false;
55-
56-
if (bot->HasSpell(13813) || bot->HasSpell(14316) || bot->HasSpell(14317) || bot->HasSpell(27025) ||
57-
bot->HasSpell(49066) || bot->HasSpell(49067)) // Explosive Trap spell IDs
58-
return false;
59-
60-
return true;
43+
return target && !botAI->HasSpell("explosive trap");
6144
}
6245

6346
Value<Unit*>* CastFreezingTrap::GetTargetValue()

src/Ai/Class/Mage/MageActions.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,15 @@
1313

1414
std::vector<NextAction> CastMoltenArmorAction::getAlternatives()
1515
{
16-
if (!AI_VALUE2(uint32, "spell id", "molten armor"))
16+
if (!botAI->HasSpell("molten armor"))
1717
return NextAction::merge({ NextAction("mage armor") }, CastBuffSpellAction::getAlternatives());
1818

1919
return CastBuffSpellAction::getAlternatives();
2020
}
2121

2222
std::vector<NextAction> CastMageArmorAction::getAlternatives()
2323
{
24-
if (!AI_VALUE2(uint32, "spell id", "mage armor"))
24+
if (!botAI->HasSpell("mage armor"))
2525
return NextAction::merge({ NextAction("ice armor") }, CastBuffSpellAction::getAlternatives());
2626

2727
return CastBuffSpellAction::getAlternatives();

0 commit comments

Comments
 (0)