Trinitycore: [3.3.5] Bosses respawning

[Keader]

Description: Bosses can respawn after you kill him.

Expected behaviour: Boss should never respawn after you kill him (after state = DONE).

Steps to reproduce the problem:

1. Kill some boss
2. Wait respawn

It was fixed by this commit: f826ecb4b2e8cc1fa54419947dff041a580d9c03
But reverted in : b3d44d6c366c7e4f12a6ce30ee5cf6b9bff68a06

Now need a new solution to fix Bosses without entry in linked_respawn.

Branch(es): 3.3.5

TC rev. hash/commit: a17e3673b2d775db12ff16cba928648200679889

[joschiwald]

how is it even possible that a boss can respawn?

[Treeston]

Instance extension works now, they have a 1 week respawn timer (most of them) in the DB.

Core doesn't have any option for "never respawn".

[joschiwald]

maybe that will be a better solution to implement a never respawn option like -1 in creature.spawntimesecs

[sirikfoll]

The -1 in creature.spawntimesecs may not be effective because of the _DespawnAtEvade funcionality, it calls a respawn that should be allowed in that case, checking bossState is the simpler way I can think of.

[joschiwald]

that are 2 different respawn triggers, db respawntimer triggers on death, _DespawnAtEvade is a manuell respawn trigger

[Treeston]

What @joschiwald says is theoretically correct, but would require us to rewrite respawn logic.

I don't know if the instance boss case is widespread enough for that.

[joschiwald]

or maybe spawntime 0 because gameobject uses negativ spawntime to determine despawned by default (see loot chests)

[Treeston]

spawntime 0 is instant respawn iirc

ps: keep in mind the fix in #18827 is just a temporary fix, it'll be unnecessary once we have dynspawn (spawn groups for each boss + its trash toggled on boss state update by instance script).

[Keader]

So why remove the old fix if dynspawn is not ready? This modification just broken things and i cant see reason...
That modification should be done, when dynspawn is ready.

ps: keep in mind the fix in #18827 is just a temporary fix

Much work for a temp fix, no?
Why not restore the old behavior until dynamic spawn be ready?

[Treeston]

Because adding more and more differences that need to be fixed before dynspawn is ready will not lead to dynspawn ever being ready.

PS: "much work"? what? This is a matter of replacing two elements per boss (inherit BossCreatureScript instead of CreatureScript, and change the ctor to pass the boss ID).

[Keader]

yes, we need change it in every single script that dont have entrys in linked_spawn, and use it for every single new scripts... instead be inside of BossAI (that dont need change nothing).
I realy cant see any benefit in b3d44d6c366c7e4f12a6ce30ee5cf6b9bff68a06, because now need change more stuffs, so to me you still be "adding more and more differences that need to be fixed before dynspawn is ready "

[Treeston]

The existing system (CanSpawn in AI) is simply not compatible with dynspawn. It's that simple.

The system needs to be removed for dynspawn to work.

[Noryad]

Confirmed. We defeated Lord Marrowgar, Lady Deathwhisper, Gunship, Deathbringer Saurfang, Festergut and Rotface in the early hours of 15/01/2017. Monday morning Lady Deathwhisper was alive, not the rest of the mentioned bosses. The same situation is present at the moment (17/01/2017).

[Keader]

Only Lady Deathwhisper and Blood Prince Council should respawn in ICC (caused by this bug), because other bosses has entry in linked_respawn.
I disagree with fix proposed in #18827, is much work to a "temp fix".
Maybe implement a "DB flag" like Joschiwald suggest...
Btw, i cant do anything before finish illidan.

[Treeston]

@Keader I wonder...should all creatures with bind flag never respawn naturally (unless spawn time override)?

[Keader]

@Treeston I think so

[Treeston]

hrm, don't we have dungeon boss flag in there somewhere

if so we could just not schedule respawn if the respawn timer isn't specifically provided (in death logic)

[ariel-]

The existing system (CanSpawn in AI) is simply not compatible with dynspawn. It's that simple.

Why not? Can't it be made to call AI to check spawn possibility?

[Treeston]

There is no AI object before the creature is spawned, because there is no creature object before it's spawned.

[Noryad]

ps: keep in mind the fix in #18827 is just a temporary fix

18827 does not work, Lady Deathwhisper reappears after 24 hours.

[Keader]

@Noryad you need make some changes in lady script to make that pr works.
Try add it:

diff --git a/src/server/scripts/Northrend/IcecrownCitadel/boss_lady_deathwhisper.cpp b/src/server/scripts/Northrend/IcecrownCitadel/boss_lady_deathwhisper.cpp
index f178498..57abd43 100644
--- a/src/server/scripts/Northrend/IcecrownCitadel/boss_lady_deathwhisper.cpp
+++ b/src/server/scripts/Northrend/IcecrownCitadel/boss_lady_deathwhisper.cpp
@@ -185,10 +185,10 @@ class DaranavanMoveEvent : public BasicEvent
         Creature& _darnavan;
 };

-class boss_lady_deathwhisper : public CreatureScript
+class boss_lady_deathwhisper : public BossCreatureScript
 {
     public:
-        boss_lady_deathwhisper() : CreatureScript("boss_lady_deathwhisper") { }
+        boss_lady_deathwhisper() : BossCreatureScript("boss_lady_deathwhisper", DATA_LADY_DEATHWHISPER) { }

         struct boss_lady_deathwhisperAI : public BossAI
         {

But again, this solution dont looks good to me :/

[Treeston]

i think easiest proper fix is a test when scheduling respawn (same method called by despawnonevade). if dungeon boss flag + no override respawn time -> never respawn.

will look into it.

[ariel-]

There is no AI object before the creature is spawned, because there is no creature object before it's spawned.

Strange then, how did the prev system work?

[Treeston]

One creature object per GUID, recycled on respawn. Which is a horrible hack.

[Noryad]

@Keader now it works ok

[Noryad]

How would be for boss_blood_council? The following does not work for me...

class boss_blood_council_controller : public BossCreatureScript
{
    public:        
    boss_blood_council_controller() : BossCreatureScript("boss_blood_council_controller", DATA_BLOOD_PRINCE_COUNCIL) { }

[Treeston]

IDK if princes have linked respawn with their controller.

[Keader]

I removed princes spawn (perma spawn), they are summoned by controller now (like illidari council in black temple).
So, if they respawn is because controller respawn. Controller has BossAI and using that code should never respawn.

diff --git a/src/server/scripts/Northrend/IcecrownCitadel/boss_blood_prince_council.cpp b/src/server/scripts/Northrend/IcecrownCitadel/boss_blood_prince_council.cpp
index a6ec97c..fbab3f9 100644
--- a/src/server/scripts/Northrend/IcecrownCitadel/boss_blood_prince_council.cpp
+++ b/src/server/scripts/Northrend/IcecrownCitadel/boss_blood_prince_council.cpp
@@ -208,10 +208,10 @@ uint32 const PrincesData[] =
     DATA_PRINCE_VALANAR
 };

-class boss_blood_council_controller : public CreatureScript
+class boss_blood_council_controller : public BossCreatureScript
 {
     public:
-        boss_blood_council_controller() : CreatureScript("boss_blood_council_controller") { }
+        boss_blood_council_controller() : BossCreatureScript("boss_blood_council_controller", DATA_BLOOD_PRINCE_COUNCIL) { }

         struct boss_blood_council_controllerAI : public BossAI
         {

@Noryad BossState = 3 (DONE) when you build this code ?

btw @Treeston any news about https://github.com/TrinityCore/TrinityCore/issues/18824#issuecomment-274260407 ? xd

[Keader]

@Treeston Why not link this "NO-RESPAWN" to flags like: CREATURE_FLAG_EXTRA_DUNGEON_BOSS or CREATURE_FLAG_EXTRA_INSTANCE_BIND ?
All bosses should has this flags

[Noryad]

@Keader when i tested it, Lady Deathwhisper and Blood Prince Council had been defeated and reappeared after 24 hours. with the temporary fix Lady Deathwhisper does not appear in the room, the opposite of Blood Prince Council. Maybe if we defeat the Blood Prince Councile with the patch from the beginning it works ok.

[Keader]

My suggestion about this issue is:

https://gist.github.com/Keader/70286275ce21fc6f4530f42f121b4bf7

[Treeston]

That breaks DespawnAtEvade

[Treeston]

Re-tagging #18827 for fix

[Noryad]

18827 merged in 94a6dd7

At this time it works ok for 10 man, not for 25 man, current revision 76f3e0e

[Treeston]

What is the output of .npc info

[Noryad]

[Treeston]

Interesting, she doesn't get instance boss flag. ~No entry in instance_encounters?~ Entry is there. Investigating.

[Keader]

Thank you @Noryad