Trinitycore: [3.3.5] The Violet Hold

[Ghost-R]

Description:
When players wipe inside the Violet Hold dungeon, the script will loop and summon the NPCs again. (Sometimes crash or freeze occurs, crash might not happen always.)

Steps to reproduce the problem:

1. Speak with Lieutenant Sinclari to start.
2. When the first wave starts, let the players die.
3. The door will be open, the dungeon will continue and will summon a lot of NPCs non-stop.

Branch(es):
3.3.5

TC hash/commit:
c0de28b

Operating system:
Debian 64bit

Screen Shot:

[jackpoz]

after 15min server will be crash

invalid crash report, follow the guidelines to report a proper crash log

[Ghost-R]

@jackpoz I don't have crash log. When we now how reproduce why we need it !
I just like to report for loop in script, and you can check it by your self, see screen shot.

[jackpoz]

take your time to get a crashlog and post a complete issue with all the details required

[jackpoz]

still waiting for a proper crash report to start working on the issue

[Ghost-R]

@jackpoz I try 10 time more on my test server, but no crash just loop in script and not end like image.

[jackpoz]

updated the issue description stating that the crash might not happen always

[Aokromes]

this needs one mini-event when door is broken

[jackpoz]

summoning @mik1893 since this relates to https://github.com/TrinityCore/TrinityCore/pull/14860

Edit: or maybe it relates to @joschiwald https://github.com/TrinityCore/TrinityCore/commit/df21162fe44d2ff29c201a9004586f560789c38b ? So many "Full Rewrites" for this single instance

[ccrs]

its the second rewrite, there is almost nothing left from the first
the conditions for the instance to know if its been a wipe are missing (there are only basic checks)

[Keader]

Something like "CheckPlayer"? (https://github.com/Keader/TrinityCore/blob/4d98c95bd6423403ce5e0e012fe2e2ff25b5c0a5/src/server/scripts/Outland/BlackTemple/boss_shade_of_akama.cpp#L335)

[ccrs]

thats still incomplete
example: 3 people die -> rogue decides to vanish to avoid death or shammy reinc -> instace is stuck forever till there is actually no player alive inside

[Treeston]

If no player stops them, mobs should kill door and then instance should reset, no?

[Aokromes]

That's what i said yesterday, one mini-event is missing.

[ccrs]

still what I've described can happen when fighting any of the bosses

[Keader]

still valid?

[Aokromes]

yes

[Keader]

Someone with Retail access can record a video about it?
Just to know what we're missing here.

I forgot my character inside of Violet Hold today for 15-20 min. After it my worldserver was using 3 GB RAM ._.

[jackpoz]

Joined LFD in Violet Hold with 5 clients on my pc while testing some stuff, teleported out, after 15 minutes I teleported back in and noticed a few mobs waiting in line at the entrances with more coming over and over. This should be fixed too (no need to wipe or anything)

[Jildor]

We have a bit of issues here:
1- The guards have less damage, so, the NPCs summoned by intro portals (when event != in progress) kill the guards and ... fuck NPCs everywere
(guards are despawned when event is in progress, so increase the damage seems fine, but I don't have data about guards damage from blizzard)

2 - This checks seems wrong for me:
https://github.com/TrinityCore/TrinityCore/blob/3.3.5/src/server/scripts/Northrend/VioletHold/instance_violet_hold.cpp#L855-L856

Maybe can be changed by:

            void Update(uint32 diff) override
            {
+                // if we don't have any player in the instance
                if (!instance->HavePlayers())
-                    return;
+                {
+                    if (EventState == IN_PROGRESS) // if event is in progress, mark as fail
+                        EventState = FAIL;
+                    else  // We don't have players and event isn't in progress, so return
+                        return;
+                }

                // if main event is in progress and players have wiped then reset instance
                if ((EventState == IN_PROGRESS && CheckWipe()) || EventState == FAIL)

3 - When npc_sinclari_vh do the Reset(), despawn your summons (the portals)
https://github.com/TrinityCore/TrinityCore/blob/3.3.5/src/server/scripts/Northrend/VioletHold/violet_hold.cpp#L328
But summos of portals doesn't despawn when portals are despawned, so we need despawn summons of portals before despawn summons of npc_sinclari_vh (the portals).
Maybe killing portals and do JustDied _summons.DespawnAll(); in common script of portals

[jackpoz]

But summos of portals doesn't despawn

imo summon.DespawnAll() should call summonsummon.DespawnAll() (as in recursive call. we could add an optional parameter int32 recurseLevel = 0 incremented at every call so we can avoid infinite loops).

Edit: maybe adding a virtual GetSummons() in ScriptedAI() returning an empty container by default, or moving SummonList down in the hierarchy to ScriptedAI()

[Jildor]

Well, I decide make a Pull Request with a minor changes: #23961
the part of despawn portals I don't know what is the best way, so I decide don't touch this xD
and the damage of guards I don't have data

[Aokromes]

@Keader https://mega.nz/#!QngXASLC!6vNod4--JSkm4xdWI3rabSG1MImOPz9MmnsMY-ZKryo

[Killyana]

In the video we see https://www.wowhead.com/npc=30665/veteran-mage-hunter it' not used at all in our script.

[Killyana]

Still not fix, the only change is npcs no longer channeling after a wipe but npcs keep spawning:

Portals and npcs must be despawn in this case, and probably friendly npc back to their position

[Jildor]

My PR isn't try to fix this issue, only minor improvements.
When wipe occurs (wipe or door 0%) the NPCs sumoned by portals should run to the entrance of the instance and despawn, portals despawn ok, after that should respawn sinclari and guards.
And when event isn't in progress guards are killed by NPCs because guards do less damage than blizz I think, so intro NPCs everywhere after 10-15min

[Rushor]

adding a despawn of the portals if there is noone in the instance shouls solve this?

[Aokromes]

adding a despawn of the portals if there is noone in the instance shouls solve this?

the blizzlike way is on reach 0% break door and move all adds to entrance and despawn.

[Rushor]

despawn added on fail encounter - maybe there is a better function to get all adds

+++ b/src/server/scripts/Northrend/VioletHold/instance_violet_hold.cpp
@@ -896,7 +896,36 @@ class instance_violet_hold : public InstanceMapScript
                     Scheduler.CancelAll();

                     if (Creature* sinclari = GetCreature(DATA_SINCLARI))
+                    {
                         sinclari->AI()->EnterEvadeMode();
+                        std::list<Creature*> HelperList;
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_TELEPORTATION_PORTAL,       1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_TELEPORTATION_PORTAL_ELITE, 1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_TELEPORTATION_PORTAL_INTRO, 1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_PORTAL_GUARDIAN,            1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_PORTAL_KEEPER,              1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_INVADER_1,            1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_SPELLBREAKER_1,       1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_BINDER_1,             1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_MAGE_SLAYER_1,        1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_VETERAN_MAGE_HUNTER,        1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_CAPTAIN_1,            1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_SORCEROR_1,           1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_RAIDER_1,             1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_BINDER_2,             1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_INVADER_2,            1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_SPELLBREAKER_2,       1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_MAGE_SLAYER_2,        1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_BINDER_3,             1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_INVADER_3,            1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_SPELLBREAKER_3,       1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_MAGE_SLAYER_3,        1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_RAIDER_2,             1000.0f);
+                        sinclari->GetCreatureListWithEntryInGrid(HelperList, NPC_AZURE_STALKER_1,            1000.0f);
+                        if (!HelperList.empty())
+                            for (std::list<Creature*>::iterator itr = HelperList.begin(); itr != HelperList.end(); itr++)
+                                (*itr)->DespawnOrUnsummon();
+                    }
                 }
             }

[Aokromes]

https://github.com/TrinityCore/TrinityCore/issues/17880#issuecomment-564002479

[jackpoz]

@Rushor just get the whole list of creatures once, checking if the entry matches the list (or if it's a tempsummon maybe with creature as owner ?)

[Rushor]

yeah we could also do: despawn the portals and despawn the tempsummons on despawn of portals

[Killyana]

Yes, it's much cleaner

[jackpoz]

npc_sinclariAI:.Reset() calls _summons.DespawnAll() but if the instance was stopped with a server restart the mobs keep spawning as _summons is empty

[jackpoz]

With https://github.com/TrinityCore/TrinityCore/commit/54e30d4ebae11e0c8da77af6104a3cc0a6a11365 the currently summoned mobs will still not be despawned but the intro portals spawned after the wipe will spawn max 9 mobs in total (3 mobs for each of the 3 portals), avoiding the "infinite spawn" issue.

[jackpoz]

Remaining issues (that should be handled in another ticket):

• Mobs are not despawned after a wipe (if that's needed)
• Guards should kill the intro waves so their DPS should be increased

[Keader]

I think that commit is wrong @jackpoz
i dont remember to see any limit in retail.
But... guards damage is low, they should quick kill npcs there.

[Shauren]

There is a limit, I checked it today