Instance respawn, fix suggestion

[Elron]

Hey there.

As we know there are currently some problems with the instance/id system, since the creatures DO even respawn if the players have an ID with that instance.

I have tried something to stop that respawning of creatures in instances with an ID and since this is the section for discussing code, I would like to know what you think of my try to fix it.

First of all, some revisions I am currently using:

MaNGOS: 7739

ScriptDev2: 1035

UDB: 371

ACID: 28

How I tested the respawning of creatures in instances with an ID:

I used 2 accounts to test it. I just created a raid group with two characters and entered the instance (in my example Zul'Aman). GM mode was OFF and my characters were visible. Then I spawned an instance boss (so I do not need to walk the whole way ). I killed the boss using the ".die" command and my characters were bound to the instance.

After that I left the group with one of my characters and the group was disbanded. Then I shut down the server and restarted it. The boss was respawned again.

How I got the (following) idea to fix it:

I put some debug lines near all places where the SQL command "DELETE FROM creature_respawn" is used. So I noticed, that the function "ObjectMgr:: DeleteRespawnTimeForInstance(instanceId)" is that function that removed the remaining respawntime of my creature.

After that I searched all places, where that function is called and I used debug lines again, to get the right function call. The function "InstanceMap::UnloadAll(bool pForce)" called the function for the deleting of the respawntime.

And this is the place where my fix is located...

The fix:

First of all here a link to the patch:

New version (SQL):

http://paste2.org/p/199545

BUGGED OLD VERSION 2:

http://paste2.org/p/199313

BUGGED OLD VERSION:

http://paste2.org/p/197700

As you can see, there is a variable called "m_resetAfterUnload" that needs to be true in order to be able to call the deleting function. I added a new variable here "bool instanceHasId". This variable needs to be false, so the deleting function can be called.

This variable is first set to false. But it can be set to true, if a player on the instance map has an ID for the instance map he is in.

=> So the instance's respawntimes will NOT be deleted, if a player on the instance map is bound to the instance. <=

I think this could be a correct solution and I would like to know, what you think about this try to fix it.

Happy about any responses.

[phelpsben]

I just want to let you know from experience with raid on retail. The only thing that doesn't respawn are bosses.

Trash will respawn on w/e timer it may be on. Doing Tempest Keep on retail, we would have to clear some trash respawns on the way out after clearing all the bosses. Serpentshrine Cavern is the best example I have, trash has a really short respawn in the raid, about 45 minutes.

Make sure only the bosses dont respawn, trash respawns as normal if your keeping it offical.

[Elron]

First, thank you for your answer.

Hm that's a quite interesting point, the trash respawn.

I think (I'm not sure) that the "usual" respawn should still be handled by the usual respawn routine. That fix here should only affect the (wrong) deleting of the respawntimes in an instance with an ID.

I was just told, that some trashes do not respawn if you have killed a specific boss. For example in Tempest Keep : If you have killed Al'ar, all trashs on the way to that boss should not respawn on retail. Do you maybe know anything concerning this "trash-not-respawn"? If it is that way, I think this needs to be handled by that part of code where the fix is in.

Edit:

I just thought about the idea mentioned above. If it is actually that way on retail, we might need a new field in the `creature`table, where the trash is linked to the "boss", that needs to be killed so the trash will not respawn again. So this might need a bigger patch if it is really that way on retail.

[chanty]

First, thank you for your answer.

Hm that's a quite interesting point, the trash respawn.

I think (I'm not sure) that the "usual" respawn should still be handled by the usual respawn routine. That fix here should only affect the (wrong) deleting of the respawntimes in an instance with an ID.

I was just told, that some trashes do not respawn if you have killed a specific boss. For example in Tempest Keep : If you have killed Al'ar, all trashs on the way to that boss should not respawn on retail. Do you maybe know anything concerning this "trash-not-respawn"? If it is that way, I think this needs to be handled by that part of code where the fix is in.

Edit:

I just thought about the idea mentioned above. If it is actually that way on retail, we might need a new field in the `creature`table, where the trash is linked to the "boss", that needs to be killed so the trash will not respawn again. So this might need a bigger patch if it is really that way on retail.

Right!

only trash respawn if the boss is not dead...

[przemratajczak]

yup, this is pain in as.. you can see raiding epic dungeons (like BT). would be very glad to test patch enabling binding trash mobs to bosses.

i can even offer help in filling DB data from mangos-0.12 (UDB365) for tests

[Skinwalker]

Yes, currently you would need to have the spawns done by the instance script to ensure no trashes are spawned in the areas where the boss is already done. This is really the way it is on retail.

[Elron]

Ah okay. Then we might need a bigger patch.

So as it seems the (wrong) deleting of respawntimes of bosses (and also trash) in instances with an ID should be handled by the patch above...

I will try to test it a bit more, and if it works, we could think about the correction for the trash-non-respawn in case a boss has been killed.

If someone is already testing the patch, I would be pleasured about responses, if it worked or not.

[danbst]

10x for trying fix the instance system!

[phelpsben]

Well, whether to spawn the trash depending on what bosses are down is somewhat incorrect. Some trash will respawn even if a boss is down.

I know the first hallway in Tempest Keep, that all respawns. The trash around Alar doesn't. I don't think the trash around VR or Solarian respawns.

For this to be blizzlike, its going to require alot of research or contact with endgame raid leaders.

[subhuman_bob]

we might need a new field in the `creature`table, where the trash is linked to the "boss", that needs to be killed so the trash will not respawn again

I would suggest making use of `pool_creature` table for this instead of adding a new field to `creature` table.

[RammboNr5]

I would suggest making use of `pool_creature` table for this instead of adding a new field to `creature` table.

I don't think this is a good idea. pool-system has a totally different purpose. Mixing it with this link-system would be much more confusing than a new table or field

[fullburned]

WIth this patch server crashes every 5min.

BT: http://paste2.org/p/198687

The first visible error in the code: "bool instanceHasId" is defined twice

[subhuman_bob]

I don't think this is a good idea. pool-system has a totally different purpose. Mixing it with this link-system would be much more confusing than a new table or field

I disagree...

1 ) pools already link mobs to other mobs, this is a logical extension of the current function

2 ) pools are already checked before a mob is respawned, so this is logical

3 ) pool_creature.chance could be altered to be pool_creature.ChanceOrMaster (or something similar) where a negative value indicates the GUID of the mob that must be spawned in order for linked entries to spawn

4 ) no new fields are needed in DB this way (just change an existing field)

[Elron]

WIth this patch server crashes every 5min.

BT: http://paste2.org/p/198687

The first visible error in the code: "bool instanceHasId" is defined twice

Oh yeah, stupid mistake .

Thanks, I will correct it and create a new paste.

New paste in first post.

[fullburned]

Oh yeah, stupid mistake .

Thanks, I will correct it and create a new paste.

New paste in first post.

Oh, and I forgot to mention - it crashes even without that mistake... and with this check too:

if(plr && plr->GetBoundInstance(GetId(), GetSpawnMode()))

Backtrace is mostly the same...

[Elron]

Hm okay.

Thank you for your testing until now fullburned.

Maybe...this function is also called, to reset instances on the usual way. So it is possible that there are no players inside (so the calling of itr->getSource might be the problem), and that might be a reason for crashing.

I will try to create a new patch, with more checks to prevent a crash.

Edit:

Or maybe we could just use an SQL query inside the code....that would be the most accurate solution for our problem I think.

Edit 2:

New patch is now using SQL query. It should select the instance from DB and if there is an entry for this instance, the instance should not be respawned due to the issue mentioned above. I did not test it by myself yet. Updated first post.

Btw:

Yeah the pool system seems to be a good idea for being able to link trashs => boss.

[Elron]

Okay I have tested the patch using SQL query now.

At the moment it's working for me, if anyone else has tested it, please report.

Tested it as I said above in the first post.