fix(Scripts/Pet): Port TrinityCore snake trap AI to use CombatManager (#25232)

Co-authored-by: blinkysc <blinkysc@users.noreply.github.com>
Co-authored-by: Treeston <treeston.mmoc@gmail.com>

src/server/scripts/Pet/pet_hunter.cpp

@@ -20,12 +20,15 @@
  * Scriptnames of files in this file should be prefixed with "npc_pet_hun_".
  */
 
+#include "CombatManager.h"
+#include "Containers.h"
 #include "CreatureScript.h"
 #include "PetDefines.h"
 #include "ScriptedCreature.h"
 #include "SpellAuraEffects.h"
 #include "SpellScript.h"
 #include "SpellScriptLoader.h"
+#include "TemporarySummon.h"
 
 enum HunterSpells
 {
@@ -50,99 +53,94 @@ enum PetSpellsMisc
 
 struct npc_pet_hunter_snake_trap : public ScriptedAI
 {
-    npc_pet_hunter_snake_trap(Creature* creature) : ScriptedAI(creature) { _init = false; }
+    npc_pet_hunter_snake_trap(Creature* creature) : ScriptedAI(creature), _isViper(false), _spellTimer(0) { }
 
-    void Reset() override
+    void JustEngagedWith(Unit* /*who*/) override { }
+
+    void InitializeAI() override
     {
-        _spellTimer = urand(1500, 3000);
+        _isViper = me->GetEntry() == NPC_VIPER;
 
-        // Start attacking attacker of owner on first ai update after spawn - move in line of sight may choose better target
+        me->SetMaxHealth(uint32(107 * (me->GetLevel() - 40) * 0.025f));
-        if (!me->GetVictim())
+        // Add delta to make them not all hit the same time
-            if (Unit* tgt = me->SelectNearestTarget(10.0f))
+        me->SetAttackTime(BASE_ATTACK, me->GetAttackTime(BASE_ATTACK) + urandms(0, 6));
-            {
+
-                me->AddThreat(tgt, 100000.0f);
+        if (!_isViper && !me->HasAura(SPELL_HUNTER_DEADLY_POISON_PASSIVE))
-                AttackStart(tgt);
+            DoCast(me, SPELL_HUNTER_DEADLY_POISON_PASSIVE, true);
-            }
+
+        // Glyph of Snake Trap — apply AoE damage reduction scaling
+        if (Unit* owner = me->GetOwner())
+            if (owner->GetAuraEffectDummy(SPELL_HUNTER_GLYPH_OF_SNAKE_TRAP))
+                me->CastSpell(me, SPELL_HUNTER_PET_SCALING, true);
     }
 
-    void EnterEvadeMode(EvadeReason /*why*/) override
+    // Redefined for random target selection:
-    {
+    void MoveInLineOfSight(Unit* /*who*/) override { }
-        // _EnterEvadeMode();
-        me->AddUnitState(UNIT_STATE_EVADE);
-        me->GetThreatMgr().ClearAllThreat();
-        me->CombatStop(true);
-        me->LoadCreaturesAddon(true);
-        me->SetLootRecipient(nullptr);
-        me->ResetPlayerDamageReq();
-        me->ClearLastLeashExtensionTimePtr();
-        me->GetMotionMaster()->MoveTargetedHome();
-
-        Reset();
-    }
-
-    //Redefined for random target selection:
-    void MoveInLineOfSight(Unit* who) override
-    {
-        if (!me->GetVictim() && who->isTargetableForAttack() && (me->IsHostileTo(who)) && who->isInAccessiblePlaceFor(me))
-        {
-            if (me->GetDistanceZ(who) > CREATURE_Z_ATTACK_RANGE)
-                return;
-
-            if (me->IsWithinDistInMap(who, 10.0f))
-            {
-                me->AddThreat(who, 100000.0f);
-                AttackStart(who);
-            }
-        }
-    }
 
     void UpdateAI(uint32 diff) override
     {
+        if (me->GetVictim() && me->GetVictim()->HasBreakableByDamageCrowdControlAura(me))
+        {
+            me->GetThreatMgr().ClearFixate();
+            me->InterruptNonMeleeSpells(false);
+            me->AttackStop();
+            return;
+        }
+
+        if (me->IsSummon() && !me->GetThreatMgr().GetFixateTarget())
+        {
+            Unit* summoner = me->ToTempSummon()->GetSummonerUnit();
+            if (summoner)
+            {
+                std::vector<Unit*> targets;
+
+                auto addTargetIfValid = [this, &targets, summoner](CombatReference* ref) mutable
+                {
+                    Unit* enemy = ref->GetOther(summoner);
+                    if (!enemy->HasBreakableByDamageCrowdControlAura(me) && me->CanCreatureAttack(enemy) && me->IsWithinDistInMap(enemy, me->GetAttackDistance(enemy)))
+                        targets.push_back(enemy);
+                };
+
+                for (auto const& [guid, ref] : summoner->GetCombatManager().GetPvPCombatRefs())
+                    addTargetIfValid(ref);
+
+                if (targets.empty())
+                    for (auto const& [guid, ref] : summoner->GetCombatManager().GetPvECombatRefs())
+                        addTargetIfValid(ref);
+
+                for (Unit* target : targets)
+                    me->EngageWithTarget(target);
+
+                if (!targets.empty())
+                {
+                    Unit* target = Acore::Containers::SelectRandomContainerElement(targets);
+                    me->GetThreatMgr().FixateTarget(target);
+                }
+            }
+        }
+
         if (!UpdateVictim())
             return;
 
-        if (me->GetVictim()->HasBreakableByDamageCrowdControlAura(me))
+        // Viper
+        if (_isViper)
         {
-            me->InterruptNonMeleeSpells(false);
+            if (_spellTimer <= diff)
-            return;
+            {
-        }
+                if (!urand(0, 2)) // 33% chance to cast
+                    DoCastVictim(RAND(SPELL_HUNTER_MIND_NUMBING_POISON, SPELL_HUNTER_CRIPPLING_POISON));
 
-        if (!_init)
+                _spellTimer = 3000;
-        {
+            }
-            _init = true;
+            else
-
+                _spellTimer -= diff;
-            uint32 health = uint32(107 * (me->GetLevel() - 40) * 0.025f);
-            me->SetCreateHealth(health);
-            me->SetStatFlatModifier(UNIT_MOD_HEALTH, BASE_VALUE, (float)health);
-            me->SetMaxHealth(health);
-
-            //Add delta to make them not all hit the same time
-            uint32 delta = urand(0, 700);
-            me->SetAttackTime(BASE_ATTACK, me->GetAttackTime(BASE_ATTACK) + delta);
-
-            if (me->GetEntry() == NPC_VENOMOUS_SNAKE)
-                DoCastSelf(SPELL_HUNTER_DEADLY_POISON_PASSIVE, true);
-
-            // Glyph of Snake Trap
-            if (Unit* owner = me->GetOwner())
-                if (owner->GetAuraEffectDummy(SPELL_HUNTER_GLYPH_OF_SNAKE_TRAP))
-                    me->CastSpell(me, SPELL_HUNTER_PET_SCALING, true);
-        }
-
-        _spellTimer += diff;
-        if (_spellTimer >= 3000)
-        {
-            if (urand(0, 2) == 0) // 33% chance to cast
-                DoCastVictim(RAND(SPELL_HUNTER_MIND_NUMBING_POISON, SPELL_HUNTER_CRIPPLING_POISON));
-
-            _spellTimer = 0;
         }
 
         DoMeleeAttackIfReady();
     }
 
 private:
-    bool _init;
+    bool _isViper;
     uint32 _spellTimer;
 };
 

src/test/server/game/Combat/SnakeTrapEvadeTest.cpp

@@ -0,0 +1,197 @@
+/*
+ * This file is part of the AzerothCore Project. See AUTHORS file for Copyright information
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License as published by the Free Software
+ * Foundation; either version 2 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "CombatManager.h"
+#include "ThreatManager.h"
+#include "CreatureAI.h"
+#include "DBCStores.h"
+#include "TestCreature.h"
+#include "TestMap.h"
+#include "WorldMock.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace testing;
+
+namespace
+{
+
+// ============================================================================
+// Regression test for snake trap evade recursion crash.
+//
+// The old npc_pet_hunter_snake_trap::EnterEvadeMode called CombatStop(true)
+// which triggers ClearInCombat -> EndAllCombat -> CombatReference::EndCombat
+// -> JustExitedCombat -> EnterEvadeMode, causing deep recursion and a
+// freeze-detector crash during Battleground::EndBattleground.
+//
+// The fix removes the custom EnterEvadeMode override entirely, using the base
+// CreatureAI::EnterEvadeMode which properly guards against recursion via
+// UNIT_STATE_EVADE before calling CombatStop.
+//
+// This test verifies that ending combat on a creature with multiple PvE refs
+// does not cause unbounded recursion or leave stale combat state.
+// ============================================================================
+class SnakeTrapEvadeTest : public ::testing::Test
+{
+protected:
+    void SetUp() override
+    {
+        _previousWorld = std::move(sWorld);
+        _worldMock = new NiceMock<WorldMock>();
+
+        ON_CALL(*_worldMock, getIntConfig(_)).WillByDefault(Return(0));
+        ON_CALL(*_worldMock, getFloatConfig(_)).WillByDefault(Return(1.0f));
+        ON_CALL(*_worldMock, getBoolConfig(_)).WillByDefault(Return(false));
+        static std::string emptyString;
+        ON_CALL(*_worldMock, GetDataPath()).WillByDefault(ReturnRef(emptyString));
+
+        sWorld.reset(_worldMock);
+
+        // Create two mutually hostile factions
+        auto* factionA = new FactionTemplateEntry{};
+        factionA->ID = 90001;
+        factionA->faction = 90001;
+        factionA->factionFlags = 0;
+        factionA->ourMask = 1;
+        factionA->friendlyMask = 0;
+        factionA->hostileMask = 2;
+        for (auto& e : factionA->enemyFaction) e = 0;
+        for (auto& f : factionA->friendFaction) f = 0;
+        sFactionTemplateStore.SetEntry(90001, factionA);
+
+        auto* factionB = new FactionTemplateEntry{};
+        factionB->ID = 90002;
+        factionB->faction = 90002;
+        factionB->factionFlags = 0;
+        factionB->ourMask = 2;
+        factionB->friendlyMask = 0;
+        factionB->hostileMask = 1;
+        for (auto& e : factionB->enemyFaction) e = 0;
+        for (auto& f : factionB->friendFaction) f = 0;
+        sFactionTemplateStore.SetEntry(90002, factionB);
+
+        TestMap::EnsureDBC();
+        _map = new TestMap();
+
+        // Simulate a "snake trap snake" — a creature with multiple combat refs
+        _snake = new TestCreature();
+        _snake->SetupForCombatTest(_map, 1, 19833); // NPC_VENOMOUS_SNAKE entry
+        _snake->SetFaction(90001);
+
+        // Simulate two enemy targets (e.g., players in a BG)
+        _targetA = new TestCreature();
+        _targetA->SetupForCombatTest(_map, 2, 50001);
+        _targetA->SetFaction(90002);
+
+        _targetB = new TestCreature();
+        _targetB->SetupForCombatTest(_map, 3, 50002);
+        _targetB->SetFaction(90002);
+    }
+
+    void TearDown() override
+    {
+        _snake->CleanupCombatState();
+        _targetA->CleanupCombatState();
+        _targetB->CleanupCombatState();
+        delete _snake;
+        delete _targetA;
+        delete _targetB;
+        delete _map;
+        sWorld = std::move(_previousWorld);
+    }
+
+    std::unique_ptr<IWorld> _previousWorld;
+    NiceMock<WorldMock>* _worldMock = nullptr;
+    TestMap* _map = nullptr;
+    TestCreature* _snake = nullptr;
+    TestCreature* _targetA = nullptr;
+    TestCreature* _targetB = nullptr;
+};
+
+// Verify that ending all combat on a creature with multiple refs completes
+// without hanging or leaving stale state (regression: recursive EnterEvadeMode)
+// cppcheck-suppress syntaxError
+TEST_F(SnakeTrapEvadeTest, EndAllCombat_WithMultipleRefs_DoesNotRecurse)
+{
+    // Put the snake in combat with both targets
+    _snake->TestGetCombatMgr().SetInCombatWith(_targetA);
+    _snake->TestGetCombatMgr().SetInCombatWith(_targetB);
+
+    ASSERT_TRUE(_snake->TestGetCombatMgr().HasCombat());
+    ASSERT_TRUE(_snake->TestGetCombatMgr().IsInCombatWith(_targetA));
+    ASSERT_TRUE(_snake->TestGetCombatMgr().IsInCombatWith(_targetB));
+
+    // This is the call path that caused the crash:
+    // EndAllCombat -> EndCombat -> JustExitedCombat -> EnterEvadeMode -> CombatStop -> EndAllCombat
+    // With the old custom EnterEvadeMode, this would recurse unboundedly.
+    _snake->TestGetCombatMgr().EndAllPvECombat();
+
+    // All combat state should be cleanly resolved
+    EXPECT_FALSE(_snake->TestGetCombatMgr().HasCombat());
+    EXPECT_FALSE(_targetA->TestGetCombatMgr().HasCombat());
+    EXPECT_FALSE(_targetB->TestGetCombatMgr().HasCombat());
+}
+
+// Verify that CombatStop on a target also clears the snake's refs cleanly
+TEST_F(SnakeTrapEvadeTest, TargetCombatStop_ClearsSnakeRefs)
+{
+    _snake->TestGetCombatMgr().SetInCombatWith(_targetA);
+    _snake->TestGetCombatMgr().SetInCombatWith(_targetB);
+
+    ASSERT_TRUE(_snake->TestGetCombatMgr().HasCombat());
+
+    // Simulate what Battleground::EndBattleground does: CombatStop on the target
+    // This triggers ClearInCombat -> EndAllCombat on _targetA, which calls
+    // EndCombat on the ref(targetA, snake), which triggers JustExitedCombat
+    // on the snake if it's the snake's last ref.
+    _targetA->TestGetCombatMgr().EndAllPvECombat();
+
+    // targetA should be out of combat
+    EXPECT_FALSE(_targetA->TestGetCombatMgr().HasCombat());
+    // Snake should still be in combat with targetB
+    EXPECT_TRUE(_snake->TestGetCombatMgr().HasCombat());
+    EXPECT_TRUE(_snake->TestGetCombatMgr().IsInCombatWith(_targetB));
+
+    // Now end targetB's combat too
+    _targetB->TestGetCombatMgr().EndAllPvECombat();
+
+    // Everything clean
+    EXPECT_FALSE(_snake->TestGetCombatMgr().HasCombat());
+    EXPECT_FALSE(_targetB->TestGetCombatMgr().HasCombat());
+}
+
+// Verify that adding threat during evade is rejected (guards against
+// the old Reset() -> AddThreat -> re-enter combat pattern)
+TEST_F(SnakeTrapEvadeTest, AddThreat_DuringEvade_IsRejected)
+{
+    _snake->TestGetCombatMgr().SetInCombatWith(_targetA);
+    _snake->TestGetThreatMgr().AddThreat(_targetA, 100.0f);
+
+    ASSERT_TRUE(_snake->TestGetThreatMgr().IsThreatenedBy(_targetA));
+
+    // Enter evade state
+    _snake->AddUnitState(UNIT_STATE_EVADE);
+
+    // AddThreat should be rejected while in evade
+    _snake->TestGetThreatMgr().ClearAllThreat();
+    _snake->AddThreat(_targetB, 100000.0f);
+
+    // Should NOT have threat on targetB because UNIT_STATE_EVADE blocks AddThreat
+    EXPECT_FALSE(_snake->TestGetThreatMgr().IsThreatenedBy(_targetB));
+
+    _snake->ClearUnitState(UNIT_STATE_EVADE);
+}
+
+} // namespace