ronancpl / HeavenMS

An improved server based on MapleSolaxia (v83 MapleStory private server)
GNU Affero General Public License v3.0
1.03k stars 742 forks source link

Boss HP bar announcement sometimes crashes client #521

Closed Riizade closed 5 years ago

Riizade commented 5 years ago

Can consistently reproduce the client crash with 9001001/Grendel the Really Old's Clone Can sometimes reproduce the issue with ghost-type bosses with the warping ability Have not encountered the issue with the fox boss around Korean Folk Town - Fox Ridge

I think the bug is in MaplePacketCreator.showBossHP(), but I'm not 100% sure. It might be callers to that function passing in invalid arguments. At the very least, the function should maybe be refactored to not be able to produce a malformed packet. It should check for erroneous values and throw an exception or return null if an invalid packet would be created. The alternative is crashing the client, which is much worse, since some crashes are unrecoverable without GM help, since the affected character can no longer log in, since they'll crash on every map spawn.

ronancpl commented 5 years ago

An excerpt from my statement on the group chat:

"Ronan LanaToday at 8:29 PM you may want to review for which bosses that packet is being sent whether the client-side files has a canvas for the bosshp, that packet should only be sent for bosses having a HPbar (informations are gathered through the XMLs)"

That packet itself is supposed to be functional, however:

    public boolean hasBossHPBar() {
        return isBoss() && getTagColor() > 0;
    }

It's possible there's an ongoing check being missed, as the current hasBossHPBar method may not be fulfilling as it is, currently.

davidlafriniere commented 5 years ago

Grendel the Really Old (mob 9001001) doesn't have a Boss HP bar in v83 or even v90. If it was added as an extra feature on your server, you have to add an image into UI/UIWindow/MobGage/mob for 9001001 otherwise the client will continuously crash whenever that boss' HP bar attempts to display.

ronancpl commented 5 years ago

Well, hopefully, the explanations here are enough to close this case. To avoid further unexpected crashes, an image check at UI/UIWindow/MobGage/mob needs to be done, to make hasBossHPBar() recognize whether the HPbar should be forwarded to the player.

ronancpl commented 5 years ago

Verification has been added to this. Bosses that don't have a canvas available shouldn't show an HPBar anymore, closing this...