Trinitycore: Creature/Combat: npc_blackfathom_deeps_event

[Killyana]

Creature with the script: npc_blackfathom_deeps_event will enter combat upon summoned but without threat and without aggro the player.
https://github.com/TrinityCore/TrinityCore/blob/3.3.5/src/server/scripts/Kalimdor/BlackfathomDeeps/blackfathom_deeps.cpp#L106

Steps to reproduce the problem:

1. .go xyz -811.81829, -154.5005, -25.887489 48
2. kill the boss
3. click on of the Fire of Aku'mai
4. notice you are in combat with the summoned npc but no aggro
5. hit one of the npc and notice it will react passive and even regen health

.deb combat
Combat refs: (Combat state: 1 | Manager state: 1)
[PvE] Testitiit (SpawnID 0)

.deb threat
Aku'mai Snapjaw (GUID 230) does not threaten any units.
Aku'mai Snapjaw (GUID 230, SpawnID 0) is not engaged, but still has a threat list? Well, here it is:

1. Testitiit (GUID 52) - threat 0.000000
   End of threat list.

https://imgur.com/a/Id70Iyd

Branch(es): 3.3.5

TC rev. hash/commit: rev. 49e77d7c9e4c

[Killyana]

Don't try to fix the script it self, we need to understand why this happen and what cause the creature to behave like this.

[dr-j]

Do these by any chance follow waypoints in between spawn and combat?

[jackpoz]

Don't try to fix the script it self

but the script itself has a custom implementation of AttackPlayer()
https://github.com/TrinityCore/TrinityCore/blob/889b6caf931cbb96cb8c21edcddd87f6c98ba214/src/server/scripts/Kalimdor/BlackfathomDeeps/blackfathom_deeps.cpp#L148-L153

[Killyana]

Yes, but we have other similar cases like: https://github.com/TrinityCore/TrinityCore/issues/23909 and probably many others.
We must know which part cause this issue and if we must fix it core side or update all scripts.

[Killyana]

I found out if you add any amount of threat to a non engaged unit it will bug.
AddThreat(player, 0.0f);
Also me->SetInCombatWith(player); will set the npc in combat without being engaged

[Rushor]

was there any change for AddThreat or SetInCombatWith recently?

[Killyana]

This one: https://github.com/TrinityCore/TrinityCore/pull/19930

[Rushor]

hmm if you remove the whole code there it still bugs

[Rushor]

fix

diff --git a/src/server/scripts/Kalimdor/BlackfathomDeeps/blackfathom_deeps.cpp b/src/server/scripts/Kalimdor/BlackfathomDeeps/blackfathom_deeps.cpp
index 516f3e894c..fbda387440 100644
--- a/src/server/scripts/Kalimdor/BlackfathomDeeps/blackfathom_deeps.cpp
+++ b/src/server/scripts/Kalimdor/BlackfathomDeeps/blackfathom_deeps.cpp
@@ -101,10 +101,7 @@ public:
         {
             Initialize();
             if (creature->IsSummon())
-            {
                 creature->SetHomePosition(HomePosition);
-                AttackPlayer();
-            }

             instance = creature->GetInstanceScript();
         }
@@ -129,6 +126,7 @@ public:
         void Reset() override
         {
             Initialize();
+            AttackPlayer();
         }

         void AttackPlayer()

soo the reason is if you handle the attack void directly in the constructor it will bug out - adding it to the reset call it is fine again. i guess we need to check all cases with SetInCombatWith

[jackpoz]

why are there still scripts that do stuff in the constructor ? what has "attack player" to do with constructing a C++ object ? ....

[Aokromes]

why are there still scripts that do stuff in the constructor ? what has "attack player" to do with constructing a C++ object ? ....

ancient sd2 scripts no one cared to update xd

[jackpoz]

ancient sd2 scripts no one cared to update xd

https://github.com/TrinityCore/TrinityCore/commit/8d051d9645bcac4035d1bee9e87b00fd00923b3c#diff-f8130d4be7c940473830e89d2b29934eR55-R61

10 years old :P

[Rushor]

I think the homeposition check there is also not needed anymore?

[illfated]

if the home position is handled somewhere else (or really not needed), that sounds OK to me.

[jackpoz]

https://github.com/TrinityCore/TrinityCore/commit/71f53104c367bda2094bee042eddc609dabaecd5