mangosArchives / Mangos-One-scripts-old

* Current Repo: mangosone/scripts * Replacement for the Script Library that comes with MaNGOS, written in C++ and is compatible with both Windows and Linux. SQL supports MySQL and PGSql.
http://www.getmangos.co.uk/
GNU General Public License v2.0
40 stars 19 forks source link

Prince Malchezaar server crash #2

Closed fr1nge closed 12 years ago

fr1nge commented 12 years ago

Function DoMeleeAttacksIfReady() doesn't have required target checks. If main-hand attack kills the player then the off-hand attack tries to do damage to a dead target, for some reason it's not properly handled by m_creature->AttackerStateUpdate() and it crashes the server.

Method to reproduce: One player tanks, the other dpses, once it gets to phase 2 tank should die, boss goes for squishy player, one-shots him and server crashes.

Nighoo commented 12 years ago

I can confirm this crash, but AttackerStateUpdate already has a check against dead targets.

    if (!pVictim->isAlive())
        return;

... or am I missing something?

fr1nge commented 12 years ago

EDIT: Nvm misread your comment, idk why it fails to check but doesnt crashes with the fix I linked anymore, so it has smth to do with it.

Schmoozerd commented 12 years ago

Hmm, I think we should remove the DoMeleeAttacksIfReady function from the boss script at all.

However there might be a possible problem: The problem (in MaNGOS - bool Unit::UpdateMeleeAttackingState() ) could be that: If the enemy is killed, getVictim is set to NULL (not sure here). This would then crash on the offhand swing.

possible solutions: 1) add a getVictim() check 2) use an "else" to prevent doing main and offhand attack in one tick (but this is kind of random solution..)

fr1nge commented 12 years ago

Can the Trash ability be implemented in some other way if DoMeleeAttacksIfReady() is removed? (I guess that's what this function is for)

Schmoozerd commented 12 years ago

This function is from old times when mangos did not support npc-offhand attack. But this has changed by now. So provided the weapon is set proper, there should no additional code be required for offhand attack.

NoReturn commented 12 years ago

Any updates on this on how is the best way to handle it?

xfurry commented 12 years ago

This boss will be updated on SD2 master, and probably there will be some changes. I'll let you know when this is done so you can retest.

xfurry commented 12 years ago

Fixed in [s2609].