ichttt / FirstAid

A Minecraft Mod that changes the vanilla health system
GNU General Public License v3.0
27 stars 28 forks source link

Bug with player revive 1.18.2 #167

Closed TheMenethil closed 2 years ago

TheMenethil commented 2 years ago

Hello !

There is a bug with the mod player revive. When you are revived by command or by an allie, you still die when you get up.

To reproduce this :

  1. Die
  2. Be revived
  3. Automatically die again
ichttt commented 2 years ago

Huh, player revive got updated, didn't notice. Yeah, the First Aid side of the player revive compat layer was thrown out as for a long time PlayerRevive didn't update to modern versions.

TheMenethil commented 2 years ago

Lol, I saw that you had hard times with this mod.. So will you be able to update it ? These two mods are just the best gameplay changer mods, it could be awesome.

CreativeMD commented 2 years ago

If there is any change necessary to PlayerRevive necessary to make it work, feel free to let me know.

ichttt commented 2 years ago

Hi @CreativeMD - Sorry it took so long, but I could now finally take a look at this. Unfortunally, the time at which the PlayerRevivedEvent is fired has changed. In 1.18 (and above I guess) it calls setHealth AFTER firing the event, while in 1.12 it calls setHealth and then fires the event. (See https://github.com/CreativeMD/PlayerRevive/blob/10dd92b53e902c775cb076463891c8fed27d33a9/src/main/java/com/creativemd/playerrevive/server/ReviveEventServer.java#L88 - this is the 1.12 code, which shows setHealth being called before calling the method that fires the event. See https://github.com/CreativeMD/PlayerRevive/blob/569dfda6c033da606921d41fd3a770544dd62524/src/main/java/team/creative/playerrevive/server/PlayerReviveServer.java#L46 for the current code). It would be the best for me if you could move the event right after the health has been set to zero. This allows me to still intercept this setHealth and react in my own way to set the health to the correct amount.

CreativeMD commented 2 years ago

@ichttt I assume you don't mean set to zero but instead set health to PlayerRevive.CONFIG.revive.healthAfter. Sure I can change that, but the only problem is I have dropped support for 1.18. So it will only be changed for 1.19. Hope that is ok. (Commit)

ichttt commented 2 years ago

Yeah thats what I meant. Thanks @CreativeMD.

ichttt commented 2 years ago

Fixed in Version 1.11.1 for 1.18.2, 1.19 version should also work once it comes out. For 1.18.2 to work I did a horrible hack in https://github.com/ichttt/FirstAid/blob/c9b1eda101f78cc631d99f160bbc4097476ad449/src/main/java/ichttt/mods/firstaid/common/compat/playerrevive/PRPresentCompatHandler.java#L76 . Yes, it's not very performant, and it relies on internals of PlayerRevive, but since it is EOL for 1.18 that should be fine. And these workarounds can be removed in 1.19 as they moved the event there.