Merge pull request liyunfan1223#625 from Bobblybook/master

Dungeon code cleanup

src/strategy/dungeons/wotlk/hallsofstone/HallsOfStoneActions.cpp

@@ -12,14 +12,14 @@ bool ShatterSpreadAction::Execute(Event event)
     GuidVector members = AI_VALUE(GuidVector, "group members");
     for (auto& member : members)
     {
-        if (bot->GetGUID() == member)
+        Unit* unit = botAI->GetUnit(member);
+        if (!unit || bot->GetGUID() == member)
         {
             continue;
         }
-        if (!closestMember ||
-            bot->GetExactDist2d(botAI->GetUnit(member)) < bot->GetExactDist2d(closestMember))
+        if (!closestMember || bot->GetExactDist2d(unit) < bot->GetExactDist2d(closestMember))
         {
-            closestMember = botAI->GetUnit(member);
+            closestMember = unit;
         }
     }
 

src/strategy/dungeons/wotlk/hallsofstone/HallsOfStoneStrategy.cpp

@@ -26,4 +26,5 @@ void WotlkDungeonHoSStrategy::InitTriggers(std::vector<TriggerNode*> &triggers)
 void WotlkDungeonHoSStrategy::InitMultipliers(std::vector<Multiplier*> &multipliers)
 {
     multipliers.push_back(new KrystallusMultiplier(botAI));
+    multipliers.push_back(new SjonnirMultiplier(botAI));
 }

src/strategy/dungeons/wotlk/nexus/NexusActions.cpp

@@ -33,11 +33,12 @@ bool MoveFromWhirlwindAction::Execute(Event event)
         default:
             break;
     }
-    if (!boss || bot->GetExactDist2d(boss->GetPosition()) > targetDist)
+    float bossDistance = bot->GetExactDist2d(boss->GetPosition());
+    if (!boss || bossDistance > targetDist)
     {
         return false;
     }
-    return MoveAway(boss, targetDist - bot->GetExactDist2d(boss->GetPosition()));
+    return MoveAway(boss, targetDist - bossDistance);
 }
 
 bool FirebombSpreadAction::Execute(Event event)
@@ -50,16 +51,8 @@ bool FirebombSpreadAction::Execute(Event event)
     GuidVector members = AI_VALUE(GuidVector, "group members");
     for (auto& member : members)
     {
-        if (bot->GetGUID() == member)
-        {
-            continue;
-        }
-
         Unit* unit = botAI->GetUnit(member);
-        if (!unit) 
-        {
-            continue;
-        }
+        if (!unit || bot->GetGUID() == member) { continue; }
 
         if (bot->GetExactDist2d(unit) < targetDist)
         {
@@ -77,24 +70,22 @@ bool TelestraSplitTargetAction::Execute(Event event)
 
     for (auto& attacker : attackers)
     {
-        Unit* npc = botAI->GetUnit(attacker);
-        if (!npc)
-        {
-            continue;
-        }
-        switch (npc->GetEntry())
+        Unit* unit = botAI->GetUnit(attacker);
+        if (!unit) { continue; }
+
+        switch (unit->GetEntry())
         {
             // Focus arcane clone first
             case NPC_ARCANE_MAGUS:
-                splitTargets[0] = npc;
+                splitTargets[0] = unit;
                 break;
             // Then the frost clone
             case NPC_FROST_MAGUS:
-                splitTargets[1] = npc;
+                splitTargets[1] = unit;
                 break;
             // Fire clone last
             case NPC_FIRE_MAGUS:
-                splitTargets[2] = npc;
+                splitTargets[2] = unit;
                 break;
         }
     }
@@ -146,11 +137,15 @@ bool ChaoticRiftTargetAction::Execute(Event event)
 bool DodgeSpikesAction::isUseful()
 {
     Unit* boss = AI_VALUE2(Unit*, "find target", "ormorok the tree-shaper");
+    if (!boss) { return false; }
+
     return bot->GetExactDist2d(boss) > 0.5f;
 }
 bool DodgeSpikesAction::Execute(Event event)
 {
     Unit* boss = AI_VALUE2(Unit*, "find target", "ormorok the tree-shaper");
+    if (!boss) { return false; }
+
     return Move(bot->GetAngle(boss), bot->GetExactDist2d(boss) - 0.3f);
 }
 

src/strategy/dungeons/wotlk/nexus/NexusActions.h

@@ -7,22 +7,6 @@
 #include "Playerbots.h"
 #include "NexusTriggers.h"
 
-#define ANGLE_45_DEG (static_cast<float>(M_PI) / 4.f)
-#define ANGLE_90_DEG M_PI_2
-#define ANGLE_120_DEG (2.f * static_cast<float>(M_PI) / 3.f)
-
-// Slice of the circle that we want melee dps to attack from.
-// Measured from boss orientation, on one side.
-
-// Even though the breath cone is not the full 180 degrees,
-// avoid melee dps from the front due to parry potential.
-// Bots should learn good dps etiquette :)
-#define DRAGON_MELEE_MIN_ANGLE ANGLE_90_DEG
-// This leaves a danger zone of 60 degrees at the tail end on both sides.
-// This is a total of 120 degrees tail arc that bots will avoid -
-// number just happens to be the same in this case, but this is always measured from the front.
-#define DRAGON_MELEE_MAX_ANGLE ANGLE_120_DEG
-
 class MoveFromWhirlwindAction : public MovementAction
 {
 public:

src/strategy/dungeons/wotlk/nexus/NexusMultipliers.cpp

@@ -77,10 +77,8 @@ float AnomalusMultiplier::GetValue(Action* action)
 float OrmorokMultiplier::GetValue(Action* action)
 {
     Unit* boss = AI_VALUE2(Unit*, "find target", "ormorok the tree-shaper");
-    if (!boss)
-    {
-        return 1.0f;
-    }
+    if (!boss) { return 1.0f; }
+
     // These are used for auto ranged repositioning, need to suppress so ranged dps don't ping-pong
     if (dynamic_cast<FleeAction*>(action))
     {

src/strategy/dungeons/wotlk/nexus/NexusMultipliers.h

@@ -39,13 +39,4 @@ class OrmorokMultiplier : public Multiplier
         virtual float GetValue(Action* action);
 };
 
-class KeristraszaMultiplier : public Multiplier
-{
-    public:
-        KeristraszaMultiplier(PlayerbotAI* ai) : Multiplier(ai, "keristrasza") {}
-
-    public:
-        virtual float GetValue(Action* action);
-};
-
 #endif

src/strategy/dungeons/wotlk/nexus/NexusStrategy.cpp

@@ -50,5 +50,4 @@ void WotlkDungeonNexStrategy::InitMultipliers(std::vector<Multiplier*> &multipli
     multipliers.push_back(new TelestraMultiplier(botAI));
     multipliers.push_back(new AnomalusMultiplier(botAI));
     multipliers.push_back(new OrmorokMultiplier(botAI));
-    // multipliers.push_back(new KeristraszaMultiplier(botAI));
 }

src/strategy/dungeons/wotlk/oldkingdom/OldKingdomActions.cpp

@@ -42,22 +42,22 @@ bool AvoidShadowCrashAction::Execute(Event event)
 {
     // Could check all enemy units in range as it's possible to pull multiple of these mobs.
     // They should really be killed 1 by 1, multipulls are messy so we just handle singles for now
-    Unit* npc = AI_VALUE2(Unit*, "find target", "forgotten one");
+    Unit* unit = AI_VALUE2(Unit*, "find target", "forgotten one");
     Unit* victim = nullptr;
     float radius = 10.0f;
     float targetDist = radius + 2.0f;
-    if (!npc) { return false; }
+    if (!unit) { return false; }
 
     // Actively move if targeted by a shadow crash.
     // Spell check not needed, they don't have any other non-instant casts
-    if (npc->HasUnitState(UNIT_STATE_CASTING)) // && npc->FindCurrentSpellBySpellId(SPELL_SHADOW_CRASH))
+    if (unit->HasUnitState(UNIT_STATE_CASTING)) // && unit->FindCurrentSpellBySpellId(SPELL_SHADOW_CRASH))
     {
         // This doesn't seem to avoid casts very well, perhaps because this isn't checked while allies are casting.
         // TODO: Revisit if this is an issue in heroics, otherwise ignore shadow crashes for the most part.
-        victim = botAI->GetUnit(npc->GetTarget());
+        victim = botAI->GetUnit(unit->GetTarget());
         if (victim && bot->GetExactDist2d(victim) < radius)
         {
-            return MoveAway(victim, targetDist - bot->GetExactDist2d(victim));
+            return MoveAway(victim, targetDist - bot->GetExactDist2d(victim->GetPosition()));
         }
     }
 

src/strategy/dungeons/wotlk/oldkingdom/OldKingdomMultipliers.cpp

@@ -9,9 +9,10 @@
 float ElderNadoxMultiplier::GetValue(Action* action)
 {
     Unit* boss = AI_VALUE2(Unit*, "find target", "elder nadox");
-    Unit* guardian = AI_VALUE2(Unit*, "find target", "ahn'kahar guardian");
+    if (!boss) { return 1.0f; }
 
-    if (boss && guardian)
+    Unit* guardian = AI_VALUE2(Unit*, "find target", "ahn'kahar guardian");
+    if (guardian)
     {
         if (dynamic_cast<DpsAssistAction*>(action))
         {
@@ -24,7 +25,7 @@ float ElderNadoxMultiplier::GetValue(Action* action)
 float JedogaShadowseekerMultiplier::GetValue(Action* action)
 {
     Unit* boss = AI_VALUE2(Unit*, "find target", "jedoga shadowseeker");
-    // Unit* volunteer = AI_VALUE2(Unit*, "find target", "twilight volunteer");
+    if (!boss) { return 1.0f; }
 
     Unit* volunteer = nullptr;
     // Target is not findable from threat table using AI_VALUE2(),
@@ -41,7 +42,7 @@ float JedogaShadowseekerMultiplier::GetValue(Action* action)
         }
     }
 
-    if (boss && volunteer)
+    if (volunteer)
     {
         if (dynamic_cast<DpsAssistAction*>(action))
         {
@@ -53,9 +54,10 @@ float JedogaShadowseekerMultiplier::GetValue(Action* action)
 
 float ForgottenOneMultiplier::GetValue(Action* action)
 {
-    Unit* npc = AI_VALUE2(Unit*, "find target", "forgotten one");
+    Unit* unit = AI_VALUE2(Unit*, "find target", "forgotten one");
+    if (!unit) { return 1.0f; }
 
-    if (npc && bot->isMoving())
+    if (bot->isMoving())
     {
         if (dynamic_cast<MovementAction*>(action))
         {

src/strategy/dungeons/wotlk/oldkingdom/OldKingdomTriggers.cpp

@@ -17,9 +17,9 @@ bool NadoxGuardianTrigger::IsActive()
 bool JedogaVolunteerTrigger::IsActive()
 {
     Unit* boss = AI_VALUE2(Unit*, "find target", "jedoga shadowseeker");
-    // Unit* volunteer = AI_VALUE2(Unit*, "find target", "twilight volunteer");
-    Unit* volunteer = nullptr;
-    // Target is not findable from threat table using AI_VALUE2(),
+    if (!boss) { return false; }
+
+    // Volunteer is not findable from threat table using AI_VALUE2(),
     // therefore need to search manually for the unit name
     GuidVector targets = AI_VALUE(GuidVector, "possible targets no los");
 
@@ -28,16 +28,16 @@ bool JedogaVolunteerTrigger::IsActive()
         Unit* unit = botAI->GetUnit(*i);
         if (unit && unit->GetEntry() == NPC_TWILIGHT_VOLUNTEER)
         {
-            volunteer = unit;
-            break;
+            return true;
         }
     }
-
-    return boss && volunteer;
+    return false;
 }
 
 bool ShadowCrashTrigger::IsActive()
 {
-    if (botAI->IsMelee(bot)) { return false; }
-    return !botAI->IsMelee(bot) && AI_VALUE2(Unit*, "find target", "forgotten one");
+    Unit* unit = AI_VALUE2(Unit*, "find target", "forgotten one");
+    if (!unit) { return false; }
+
+    return !botAI->IsMelee(bot);
 }

src/strategy/dungeons/wotlk/utgardekeep/UtgardeKeepActions.cpp

@@ -14,7 +14,7 @@ bool AttackFrostTombAction::Execute(Event event)
     for (auto i = targets.begin(); i != targets.end(); ++i)
     {
         Unit* unit = botAI->GetUnit(*i);
-        if (unit && unit->GetName() == "Frost Tomb")
+        if (unit && unit->GetEntry() == NPC_FROST_TOMB)
         {
             frostTomb = unit;
             break;
@@ -31,7 +31,9 @@ bool AttackFrostTombAction::Execute(Event event)
 bool AttackDalronnAction::Execute(Event event)
 {
     Unit* boss = AI_VALUE2(Unit*, "find target", "dalronn the controller");
-    if (!boss || AI_VALUE(Unit*, "current target") == boss)
+    if (!boss) { return false; }
+
+    if (AI_VALUE(Unit*, "current target") == boss)
     {
         return false;
     }
@@ -42,10 +44,7 @@ bool IngvarStopCastingAction::Execute(Event event)
 {
     // Doesn't work, this action gets queued behind the current spell instead of interrupting it
     Unit* boss = AI_VALUE2(Unit*, "find target", "ingvar the plunderer");
-    if (!boss)
-    {
-        return false;
-    }
+    if (!boss) { return false; }
 
     int32 my_spell_id = AI_VALUE(uint32, "active spell");
     if (!my_spell_id || my_spell_id == 0)
@@ -54,10 +53,7 @@ bool IngvarStopCastingAction::Execute(Event event)
     }
 
     Spell* spell = bot->FindCurrentSpellBySpellId(my_spell_id);
-    if (!spell)
-    {
-        return false;
-    }
+    if (!spell) { return false; }
 
     // bot->Yell("cancelling spell="+std::to_string(my_spell_id), LANG_UNIVERSAL);
     bot->InterruptSpell(spell->GetCurrentContainer(), false, true, true);
@@ -71,10 +67,7 @@ bool IngvarDodgeSmashAction::isUseful() { return !AI_VALUE2(bool, "behind", "cur
 bool IngvarDodgeSmashAction::Execute(Event event)
 {
     Unit* boss = AI_VALUE2(Unit*, "find target", "ingvar the plunderer");
-    if (!boss)
-    {
-        return false;
-    }
+    if (!boss) { return false; }
 
     float distance = bot->GetExactDist2d(boss->GetPosition());
     // Extra units to move into the boss, instead of being just 1 pixel past his midpoint.
@@ -88,10 +81,7 @@ bool IngvarSmashReturnAction::isUseful() { return AI_VALUE2(bool, "behind", "cur
 bool IngvarSmashReturnAction::Execute(Event event)
 {
     Unit* boss = AI_VALUE2(Unit*, "find target", "ingvar the plunderer");
-    if (!boss)
-    {
-        return false;
-    }
+    if (!boss) { return false; }
 
     float distance = bot->GetExactDist2d(boss->GetPosition());
     return Move(bot->GetAngle(boss), distance + bot->GetMeleeReach());