Trinitycore: Core/Unit: Guardians stop follow the player after a combat

[dr-j]

Description:
A new issue has arisen with charmed guardians, where when the NPC has entered into combat, then after combat it will not move from its spot once combat has finished, On further investigation, it looks like unit flag 2048 UNIT_FLAG_PET_IN_COMBAT is not getting cleared when the pet leaves combat.

Current behaviour: Tell us what happens
This has been confirmed with all Sholazar Guardians and Taunka'le Evacuee (26159 26167) taken Across Transborea in Borean Tundra or Gods like Shiny Things in Sholazar Basin, and summon pet (for Across Transborea you must take quest from NPC 26158) and take pet and let it enter into combat, kill the attacking NPCs and then notice how the NPC just stands there and wont move again. If you type .npc info you will notice NPC still has unit flags 2048 pet in combat even though is not in combat.

Expected behaviour: Tell us what should happen instead
Once combat has finished, the guardian should once again follow player and in combat, unit flags should be removed.

For Across Transborea, you will need the following, as current script no longer works at all:

UPDATE `smart_scripts` SET `event_flags`=`event_flags`|0x200 WHERE `entryorguid`IN(2616700,2616701,2616702,2616703) AND `source_type`=9;

UPDATE `smart_scripts` SET `target_type`=12, `target_param1`=1 WHERE  `entryorguid`=2616703 AND `source_type`=9 AND `id`=6 AND `link`=0;

DELETE FROM `smart_scripts` WHERE `entryorguid`=26167 AND `source_type`=0;
INSERT INTO `smart_scripts` (`entryorguid`, `source_type`, `id`, `link`, `event_type`, `event_phase_mask`, `event_chance`, `event_flags`, `event_param1`, `event_param2`, `event_param3`, `event_param4`, `action_type`, `action_param1`, `action_param2`, `action_param3`, `action_param4`, `action_param5`, `action_param6`, `target_type`, `target_param1`, `target_param2`, `target_param3`, `target_x`, `target_y`, `target_z`, `target_o`, `comment`) VALUES 
(26167, 0, 0, 0, 25, 0, 100, 512, 0, 0, 0, 0, 11, 46669, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 'Taunka\'le Evacuee - On Spawn - Cast Evacuee Random Action'),
(26167, 0, 1, 0, 8, 0, 100, 512, 46663, 0, 0, 0, 87, 2616700, 2616701, 2616702, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 'Taunka\'le Evacuee - On Spellhit (Evacuee Random Action) - Run Random Script'),
(26167, 0, 2, 0, 8, 0, 100, 513, 46677, 0, 0, 0, 80, 2616703, 2, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 'Taunka\'le Evacuee - On Spellhit (Evacuee Reaches Dragonblight) - Run Script'),
(26167, 0, 3, 0, 1, 0, 100, 512, 0, 0, 0, 0, 29, 0, 0, 0, 0, 0, 0, 12, 1, 0, 0, 0, 0, 0, 0, 'Taunka\'le Evacuee - On Reset - Follow Target'),
(26167, 0, 4, 0, 54, 0, 100, 512, 0, 0, 0, 0, 64, 1, 0, 0, 0, 0, 0, 7, 0, 0, 0, 0, 0, 0, 0, 'Taunka\'le Evacuee - On Just Summoned Store Target');

With the above SQL, once NPC enters into combat, it will not move from its spot.

now add this line

(26167, 0, 5, 0, 0, 0, 100, 512, 30000, 30000, 30000, 30000, 19, 2048, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 'Taunka\'le Evacuee - IC - Remove unit flag Pet In Combat');

Now you will notice the guardian follows all the time but combat fails (obviously)

Branch(es): Tested on 3.3.5, can't confirm on 6.x.x

TC hash/commit:
https://github.com/TrinityCore/TrinityCore/commit/8d2f17b2a8b20bbb2728c5d0a8fc9eed4e398205

[Killyana]

It wasn't fixed by https://github.com/TrinityCore/TrinityCore/commit/e69a483aa942c1c6c4bd9b5fa47139f89643b8d5 ?

[ghost]

I would not think that commit https://github.com/TrinityCore/TrinityCore/commit/e69a483aa942c1c6c4bd9b5fa47139f89643b8d5 helps in this case, because e69a483 is about NPC stuck in Evade, unit_flags 4 (UNIT_FLAG_REMOVE_CLIENT_CONTROL) and event_flags 0x200 for charmed creatures.

If I read dr-j's post correctly, unit flag 2048 should be removed after combat (OOC / reset).

[dr-j]

yes flags cause guardian to just stand in spot where left combat and stop following player has soon as it enters and leaves combat once it gets stuck with unit flags 2048 which it did not have before entering in combat if you script to remove flags it starts moving again but this should not need scripting as pet in combat flags should be removed as soon as guardian leaves combat.

[ghost]

OK, so if I get what you're saying, this should be handled in Core, because core sets the flag 2048 on the Pet (guardian) NPC upon entering combat. (Maybe replace [Comp-Database] with [Comp-Core] ?).

[ariel-]

18906 does some modifications to UNIT_FLAG_PET_IN_COMBAT, is the issue still valid?

[Killyana]

Still happen on rev. a891eb364e94
Just .cast 46657 and start a combat, once the combat ended the guardian will stop follow, there's no unit_flag that remains from the combat.
I removed also the SAI and the creature_template_addon to be sure it's not relates to that.

[Langerz82]

https://github.com/TrinityCore/TrinityCore/issues/22063#issuecomment-401805674

[Olion17]

guardians.txt
This diff, taken against the rev. 59edf6b, was tested on a several months old core.

Hunter pets get informed on the player killing a target in Unit::Kill(). It looks weird a bit to have that functionality hooked on the PetAI::KilledUnit() since it is rather common than custom pet behaviour. This way is unacceptable for generic creatures due to different AI types allowed, thus a new target is looked for straight in Unit::Kill(). The guardian AI may prevent evading as usual either by overriding CreatureAI::EnterEvadeMode(), or by disabling evade for SAI.

[jackpoz]

@Olion17 could you make a PR (or anyone else who has spare time to open a PR from that patch) ? Could anyone confirm the patch above fixes the issue ?

[Sorikoff]

Fixes problem (tested on The Gift That Keeps On Giving).

[illfated]

You mean this code snippet, or more than the following?

diff --git a/src/server/game/Entities/Unit/Unit.cpp b/src/server/game/Entities/Unit/Unit.cpp
index 7cf57adaca..506eb134eb 100644
--- a/src/server/game/Entities/Unit/Unit.cpp
+++ b/src/server/game/Entities/Unit/Unit.cpp
@@ -10944,18 +10944,23 @@ bool Unit::InitTamedPet(Pet* pet, uint8 level, uint32 spell_id)

     // Inform pets (if any) when player kills target)
     // MUST come after victim->setDeathState(JUST_DIED); or pet next target
     // selection will get stuck on same target and break pet react state
     if (player)
     {
         Pet* pet = player->GetPet();
         if (pet && pet->IsAlive() && pet->isControlled())
             ASSERT_NOTNULL(pet->AI())->KilledUnit(victim);
+
+        // inform uncontrollable guardians if any
+        for (auto minion : player->m_Controlled)
+            if (minion->IsGuardian() && !minion->HasUnitTypeMask(UNIT_MASK_CONTROLABLE_GUARDIAN) && minion->IsAlive() && minion->ToCreature())
+                minion->ToCreature()->SelectVictim();
     }

     // 10% durability loss on death
     if (Player* plrVictim = victim->ToPlayer())
     {
         // remember victim PvP death for corpse type and corpse reclaim delay
         // at original death (not at SpiritOfRedemtionTalent timeout)
         plrVictim->SetPvPDeath(player != nullptr);

[Sorikoff]

This snippet.

[illfated]

Thank you for answering my question.
Can you verify if it solves the issue and open a PR for it?

[Killyana]

If you test this diff without a script for the ghoul you will get a weird behavior
``sql UPDATEcreature_templateSET ScriptName="" WHEREentry` IN (28845);

[Killyana]

Here an interesting thing:
.cast 19096
Start a combat then .gm on and don't kill the creature, notice the summoned guardians will follow the npc and not your char, if you kill the creature they will remain in the position where the creature was killed.

you can also reproduce it with .go c id 11878 start the combat and wait until he summon the guardians then gm on, the summoned guardians will follow you char instead the summoner.

[Killyana]

@ccrs any idea why the chasing movement is not cleared and the follow not resumed after a combat ?

[jackpoz]

https://github.com/TrinityCore/TrinityCore/commit/7fff83d67526efff63867d41b9e036a19a9287b3#diff-703f1f16a955ae7ddc7caf673a75d85aR611 is deleting the FollowMovementGenerator when chasing an enemy so it cannot be resumed when the combat is finished

[jackpoz]

@ccrs could you take a look at this and fix it ?

[Riztazz]

But SAI's enterevademode do call MoveFollow again after combat so issue is elsewhere, they never truly evade i think?

edit: https://github.com/TrinityCore/TrinityCore/blob/3.3.5/src/server/game/AI/SmartScripts/SmartAI.cpp#L434

[Riztazz]

Anyone knows whether guardians should evade when combat ends(and obviously follow owner again) or just return to following?

[jackpoz]

Riztazz: let's just call Evade and see what happens

[jackpoz]

Btw https://github.com/TrinityCore/TrinityCore/pull/23187#discussion_r275946530 explains why evade is not called

[illfated]

Wow... to see that an evade bug has been used as a follow mechanic for so long. 😞

[jackpoz]

CreatureAI::UpdateVictim() checks if the creature is engaged and if not, it returns; later in the function is checks if the creature is in combat and if not, it evades.
https://github.com/TrinityCore/TrinityCore/blob/8163ea60c8d517ebe3a88f595dd8291df8338811/src/server/game/AI/CreatureAI.cpp#L180

If the owner of the creature is a player, me->IsEngaged() will return IsInCombat() which will be false if a creature just finished fighting, even before calling EnterEvadeMode(EVADE_REASON_NO_HOSTILES) in this function.

[jackpoz]

Actually in case of Creature 26167 https://wotlk.evowow.com/npc=26167 summoned with ".cast 46657" , the react state REACT_AGGRESSIVE causes EnterEvadeMode() to never be called because SelectVictim() is called instead

https://github.com/TrinityCore/TrinityCore/blob/8163ea60c8d517ebe3a88f595dd8291df8338811/src/server/game/AI/CreatureAI.cpp#L187

[jackpoz]

Callstack of UNIT_FLAG_IN_COMBAT being removed from the Guardian:

worldserver.exe!CombatManager::UpdateOwnerCombatState() Line 343 C++
worldserver.exe!CombatReference::EndCombat() Line 70 C++
worldserver.exe!CombatManager::EndAllPvECombat() Line 286 C++
worldserver.exe!CombatManager::EndAllCombat() Line 125 C++
worldserver.exe!Unit::ClearInCombat() Line 1050 C++
worldserver.exe!Unit::CombatStop(bool includingCast, bool mutualPvP) Line 5606 C++
worldserver.exe!Unit::setDeathState(DeathState s) Line 8637 C++
worldserver.exe!Creature::setDeathState(DeathState s) Line 1893 C++
worldserver.exe!Unit::Kill(Unit * attacker, Unit * victim, bool durabilityLoss) Line 10902 C++
worldserver.exe!Unit::DealDamage(Unit * attacker, Unit * victim, unsigned int damage, const CleanDamage * cleanDamage, DamageEffectType damagetype, SpellSchoolMask damageSchoolMask, const SpellInfo * spellProto, bool durabilityLoss) Line 823 C++
worldserver.exe!Unit::DealMeleeDamage(CalcDamageInfo * damageInfo, bool durabilityLoss) Line 1416 C++
worldserver.exe!Unit::AttackerStateUpdate(Unit * victim, WeaponAttackType attType, bool extra) Line 2039 C++
worldserver.exe!UnitAI::DoMeleeAttackIfReady() Line 76 C++
worldserver.exe!SmartAI::UpdateAI(unsigned int diff) Line 293 C++
worldserver.exe!Unit::AIUpdateTick(unsigned int diff) Line 9386 C++
worldserver.exe!Creature::Update(unsigned int diff) Line 789 C++
worldserver.exe!TempSummon::Update(unsigned int diff) Line 68 C++

[jackpoz]

@Treeston are REACT_AGGRESSIVE TempSummons with a Player as owner supposed to call EnterEvadeMode() ? Because that's currently not possible but it's needed to call SmartAI::EnterEvadeMode() that will callGetMotionMaster()->MoveFollow(owner)

[jackpoz]

I opened another Pull Request, please test it and provide feedback https://github.com/TrinityCore/TrinityCore/pull/23466

[Treeston]

If these do not have a threat list, then EnterEvadeMode will not be called on them.

Why not just use JustExitedCombat?