EbonJaeger / perworldinventory-kt

Multi-world inventory plugin for Spigot written in Kotlin.
MIT License
46 stars 35 forks source link

Modifying player base health causes exception on inventory switch #172

Open Alphaesia opened 4 years ago

Alphaesia commented 4 years ago

Describe the bug When switching gamemodes while the player (initially) has a max health of less than 20, an exception will be thrown.

To Reproduce Steps to reproduce the behavior:

  1. Set your game mode to survival.
  2. Run /give @s minecraft:diamond_chestplate{AttributeModifiers:[{Slot:"chest", AttributeName:"generic.maxHealth", Name:"generic.maxHealth", Amount:-5, Operation:0, UUIDLeast:1, UUIDMost:1}]}.
  3. Equip the chestplate.
  4. Set your game mode to creative.

Expected behavior The player's health to be updated successfully (no exception thrown).

Plugin version

Server version CraftBukkit version git-Spigot-c574e08-807a677 (MC: 1.15.2) (Implementing API version 1.15.2-R0.1-SNAPSHOT)

Additional context and logs Server logs: https://gist.github.com/Alphaesia/b61ef0ac6bd306c78d3dbb1470119aa9 (attached full logs to provide context).

Note that the exception is thrown in two different places (the first exception has a different stack trace from the others):

Caused by: java.lang.IllegalArgumentException: Health must be between 0 and 15.0(20.0)
    at org.bukkit.craftbukkit.v1_15_R1.entity.CraftLivingEntity.setHealth(CraftLivingEntity.java:108) ~[spigot-1.15.2.jar:git-Spigot-c574e08-807a677]
    at me.ebonjaeger.perworldinventory.data.ProfileManager.applyDefaults(ProfileManager.kt:226) ~[?:?]

and

Caused by: java.lang.IllegalArgumentException: Health must be between 0 and 15.0(20.0)
    at org.bukkit.craftbukkit.v1_15_R1.entity.CraftLivingEntity.setHealth(CraftLivingEntity.java:108) ~[spigot-1.15.2.jar:git-Spigot-c574e08-807a677]
    at me.ebonjaeger.perworldinventory.data.ProfileManager.transferHealth(ProfileManager.kt:160) ~[?:?]

Some areas you may wish to investigate:

Clearing the player's inventory will not instantly remove the relevant modifiers to their GENERIC_MAX_HEALTH attribute. Clearing a player's inventory (and health-reducing armour), then immediately setting their health to something higher than their previous max (e.g. 20) will throw an exception. Clearing their inventory, then clearing all of the attribute's modifiers, then setting the health will work. Adding player.getAttribute(Attribute.GENERIC_MAX_HEALTH)!!.modifiers.forEach { player.getAttribute(Attribute.GENERIC_MAX_HEALTH)!!.removeModifier(it) } right after line 157 in ProfileManager seemed to fix one exception, though this may potentially cause some issues when the inventory being equipped has modified (and may be inefficient/inelegant, I don't know Kotlin).

I also discovered earlier today the player's reported health may not always be considered a legal argument by setHealth(). For instance, if the player equips health-lowering armour, their health value will stay as 20 until they take damage. As a result, setting the player's health value to their current (or previous) health value (which may be 20) could result in an IllegalArgumentException.