SpongePowered / SpongeAPI

A Minecraft plugin API
http://www.spongepowered.org/
MIT License
1.14k stars 343 forks source link

Entity pre-death event #1545

Open blablubbabc opened 7 years ago

blablubbabc commented 7 years ago

This is basically the same idea as behind this 2 year old issue: https://github.com/SpongePowered/SpongeAPI/issues/898

However, with minecraft 1.11 and the addition of the 'totem of undying' some justification behind the decision of the linked ticket might have changed by now. A pre-death event might make sense in order for plugins to be able to implement features similar to that of the totem of undying.

The compromise of the linked ticket was to let the DamageEntityEvent provide a willCauseDeath method, which simply checks if the damage of the event is sufficient to kill the player. Plugins are then able to cancel the damage event and handle the death somehow differently.

However, due to the addition of the totem of undying, it is no longer possible for the damage event to know whether or not the entity is actually going to die as result of the damage.

And there are other issues with this proposed 'solution' as well: By cancelling the damage event all the other consequences of damage (besides the entity dying) are getting skipped as well. For example: Armor not getting damaged, exhaustion not getting applied, the damage not getting tracked by the DamageTracker (influences the death message for example), .. Also there might be other sources of an entity dying. I haven't checked, but: If a plugin directly sets an entity's health to zero, I assume that will cause the entity to die as well? If so, plugins are probably not able to react/prevent those deaths currently.

A cancellable pre-death event (or a pre-death event with a 'resurrectWithHealth(health)'-method) would only be called when an entity is really about to die. And this might be called regardless of whether or not the death is caused by damage or something else. This would prevent the issues mentioned above and give plugins more control over how to handle an entity's death, including being able to resurrect the entity similiar to the totem of undying.

gabizou commented 7 years ago

However, due to the addition of the totem of undying, it is no longer possible for the damage event to know whether or not the entity is actually going to die as result of the damage.

The totem checks are still performed after damage events are handled, so the event will still know whether the damage dealt will cause death.

The compromise of the linked ticket was to let the DamageEntityEvent provide a willCauseDeath method, which simply checks if the damage of the event is sufficient to kill the player. Plugins are then able to cancel the damage event and handle the death somehow differently.

It's very much the same as how vanilla handles it currently, it sets the entity health to 1.0D and then applies some potion effects, and removes the now "used" totem from inventory.

However, due to the addition of the totem of undying, it is no longer possible for the damage event to know whether or not the entity is actually going to die as result of the damage.

It's still possible, you just have to do the same check as vanilla, check if the item in hand is an ItemTypes.TOTEM and bing bang boom, you know if it's going to apply or not.

And there are other issues with this proposed 'solution' as well: By cancelling the damage event all the other consequences of damage (besides the entity dying) are getting skipped as well. For example: Armor not getting damaged, exhaustion not getting applied, the damage not getting tracked by the DamageTracker (influences the death message for example), .. Also there might be other sources of an entity dying. I haven't checked, but: If a plugin directly sets an entity's health to zero, I assume that will cause the entity to die as well? If so, plugins are probably not able to react/prevent those deaths currently.

Other sources are going to be plugin based, and even then, setting health to 0 is an invalid argument to the Data implementation. And if plugins are actively bypassing systems in place to throw events to make other plugins work, that's the plugin's fault, not the fault of the API when the API clearly exposes everything available to make it work.

A cancellable pre-death event (or a pre-death event with a 'resurrectWithHealth(health)'-method) would only be called when an entity is really about to die. And this might be called regardless of whether or not the death is caused by damage or something else. This would prevent the issues mentioned above and give plugins more control over how to handle an entity's death, including being able to resurrect the entity similiar to the totem of undying.

There's no real way to hook this in without potentially breaking mods in various ways. What is possible is to use the API as intended, and trust that the systems in place will do everything they're supposed to do.

If you want to propose something separate with corrections to these points, I'm all ears, however, the API as it stands is suiting the purpose as explained in my responses, as well as explaining how the API is interacting with the implementation and vice versa.

blablubbabc commented 7 years ago

The totem checks are still performed after damage events are handled, so the event will still know whether the damage dealt will cause death.

I was refering to the willCauseDeath-method here: https://github.com/SpongePowered/SpongeAPI/blob/766022b54cd5a5db9ef87d29855887dda3f9ae7f/src/main/java/org/spongepowered/api/event/impl/AbstractDamageEntityEvent.java#L178 As you pointed out, plugins currently have to additionally reimplement minecraft's logic: Checking if the player is holding a totem somewhere in their hand slots. This might not be that big of a deal for this simple check, but would still no longer be needed with the proposed addition of a pre-death event.

It's very much the same as how vanilla handles it currently, it sets the entity health to 1.0D and then applies some potion effects, and removes the now "used" totem from inventory.

Minecraft doesn't cancel the damage though. Its letting it getting handled like normal (including armor damage, damage tracking for later death messages, etc.). The only way for plugins to try to recreate this would probably be to not cancel the event and instead temporarily increase the player's health in order to prevent the death. Though, there seem to be limits on the max health the player can have, while there are no limits on the amount of damage he can receive. So this is more of an unstable workaround. Reacting to the actual (pre-)death would be a lot cleaner way to handle all this.

There's no real way to hook this in without potentially breaking mods in various ways.

How would adding a separate pre-death event break mods? This would be called in between damage being handled and after minecraft having checked if the player is getting reveived, and the call to the entity's death. I would assume mods are currently reacting to the actual death. So this wouldn't be any more breaking than minecraft's addition of the totem of undying, which is placed right before death as well.

ryantheleach commented 7 years ago

@blablubbabc Thing is, the damage does "cause death" (in the extreme abstract) otherwise the totem wouldn't activate and intervene. at that stage I think of it as multiple systems fighting for priority.

If you can describe a Usecase / User Story for the API, that can't be achieved using the current API I'm sure people will be more sympathetic.

*if it's fast respawning, without the death screen, I'm pretty sure people would be happier including an implementation of that, rather then having multiple plugins handle it.

blablubbabc commented 7 years ago

If you can describe a Usecase / User Story for the API, that can't be achieved using the current API I'm sure people will be more sympathetic.

I see the chain of events to be like this:

There is happening a lot between the damage event and the entity's actual death. The last chance for plugins to prevent the death is at the damage event currently. However, cancelling the damage event skips the 'damage gets applied phase' and plugins would additionally have to copy minecraft's 'check: has totem' logic inside their damage handling. And not cancelling the damage event would require plugins to still copy the totem check and then to temporarily give the entity additional health to survive the damage event (which, as pointed out above already, doesn't even work in all cases, so it's more of a workaround currently).

The proposed pre-death event would be placed right before 'handle death'. At this point plugins can fully ignore whether or not minecraft has some 'totem-like feature' in place right before that. Actually, the pre-death event could even incorporate minecraft's totem-revive check, so that plugins could trigger the revive, even though the player has no totem in his hand. All that is needed for plugins to be able to revive the player here is that event, and an additional 'check: health <= 0' after the pre-death event, before proceeding to the 'handle death' stage. And their decision of whether or not the entity is getting revived here wouldn't interfere with minecraft's damage handling, as that has already been handled before. And the impact on mods to prevent the death at this stage would be the same as minecraft's totem preventing the death.

ryantheleach commented 7 years ago

Sorry if I wasn't clear.

What I was interested in reading was a hard example of a plugin that wouldn't be possible (or incredibly hard to implement) without the proposed changes.

e.g.

"The plugin I am developing is an arena plugin, in order to make combat fast, I want to automatically respawn the player without them ever hitting the death screen, making it similar to twitch arena shooters, in order to facillitate this, I'd like to be able to bypass the death screen by automatically respawning / moving the player before the death event even fires."

or

"I am creating a plugin that creates custom totem of undying like items, that take you to the closest graveyard waypoint by a guardian angel. In order to be more compatible with mods and other plugins, I need the last and final say over when the player dies, in order to correctly position them in the world and cancel the death. I've tried using the damage event, but it's a pain to check for the totem of undying and other forge added items that interact with death."

This issue is old enough I'm sure most people commenting are aware of what's being requested. it's the why, and whether there is a more optimal way of solving it, rather then interfering with the death logic that I think people are interested in.

If there is no other clean way to solve the issue, it's more likely that they will change the design of the current death events in sponge

blablubbabc commented 7 years ago

You already gave 2 usecases with this. Though the 'custom totem' usecase might be more suitable here to make my point, because 'fast respawn' might indicate that the player first has to die and gets respawned afterwards. This however is about intervening right before the death is actually handled, just like minecraft's totem does it.

I already tried to explain what issues there are with trying to do this with the currently available damage event.

rather then interfering with the death logic that I think people are interested in

And I already tried to point out that this is not interfering with death logic anymore than minecraft's totem is.

it's the why, and whether there is a more optimal way of solving it

Well, it has already been pointed out that having this as part of the death event would break mods, and I am trying to point out that having it as part of the damage event is not an equivalent solution due to the issues I mentioned above. So there is not much possible in between. That's why I am so nitpicking about making the point for an additional pre-death event here. If you or anyone has a better idea to this than I am all open to hear about it.

If there is no other clean way to solve the issue, it's more likely that they will change the design of the current death events in sponge

I wouldn't have a problem if this would be possible as part of the death handling. But in the old, linked ticket, and the first comment to this ticket, the consens seems to be that messing with the death event might be an issue due to interference with mods. That's why I assumed that this might not be an option here.

ryantheleach commented 5 years ago

Another thing that I've run into recently, whilst investigating mini-games.

A lot of mini-games will prevent death just for the performance benefit, and other gameplay comes as a side effect.

Respawning the player requires re-syncing the world and resending chunks, where if you just change them to spectator, or teleport them nearby, it can reduce the amount of chunks that need to be resent.

In this usecase, They really want death to look, and appear to all other plugins etc to look like real death. Count for stats, etc. But use their own simulated death/corpse state. Whether that's spectator, teleporting to a nearby death area until they respawn etc.