Add Viewer#playTotemOfUndyingEffect

[MrHell228]

SpongeAPI | Sponge

With introduction of DEATH_PROTECTION item component it's now possible to play death protection effect with any ItemStack

[aromaa]

Calling this just sendDeathProtection is going to be very vague. It should at least imply that its just visual effect. Also I think I would rather just call it totemOfUndying because thats very discoverable and understandable. Something like playConsumedTotemOfUndyingEffect, its a bit wordy and lengthy but at least the intent is clear.

[MrHell228]

I believe the fact that it's the method in Viewer interface implies visual-only effect? If not, I can add client-only words to docs, some other methods there has this as well. But I don't really want to blend item name into method name.
Also there will be Keys.DEATH_PROTECTION_EFFECTS from other PR and I don't think you would like it to be called TOTEM_OF_UNDYING_EFFECTS? Maybe rename to playDeathProtectionEffect would be enough

[aromaa]

Effects aren't abstract thing, you want to play a specific one. Abstracting things out works well in the Data API because its a common entrypoint for basically anything. Its much easier to describe the actual effects based on from where they originate. I'm just not buying it that "death protection" is specific enough to distinct itself from any other possible options (Vanilla only has one for now).

[aromaa]

I should also add that the "death protection" term itself is also a very abstract concept. It could mean various things like the player is actively being protected from death, like invincibility, and many games display this by making the player semi transparent or distort the player model in different ways.

[MrHell228]

Yeah, effects aren't abstract thing, I meant that it would be nice to have some consistency between these two things and if you find one, you will likely search for the same name because they originate from the same thing.
I have really no idea if someone is typing player.totem and then autocomplete. If you just find this method on player, I think it's the docs who should say you that in vanilla this is what totem usually does (And even in vanilla with command you can give yourself item that behaves like totem and on death it will pop on the screen as an actual item, not totem)

[MrHell228]

I guess I can agree that it's abstract enough. But playConsumedTotemOfUndyingEffect... eh, too long and inconvenient. But apparently playTotemOfUndyingEffect is shorter than playDeathProtectionEffect. Still don't like item name being involved, but will change to it

[MrHell228]

I believe that the naming concern was resolved the day it appeared as I renamed the method to playTotemOfUndyingEffect.
So maybe this PR could be merged?

[Yeregorix]

Just to clarify, this method will only show the totem of undying animation? It doesn't actually run any of the actions specified in component DEATH_PROTECTION, like removing or adding potion effects or teleporting?

[MrHell228]

Right, it's just fancy flying item on the player's screen

[Yeregorix]

Alright so playDeathProtection indeed would have been a bad name since it doesn't perform DEATH_PROTECTION actions.
Still I agree with you that playTotemOfUndyingEffect is a strange name for something that can work with any item type/texture.
I took a look at internals and the animation is simply named "item activation", so I guess playItemActivation would be a good item-agnostic name. It's also more future-proof in case Mojang decides to reuse the animation for something else.

@aromaa Any opinion on this?

[aromaa]

I'm not really against making something more generic but we run really fast to the situation at WHAT is an item activation? Can you play the shield animation? Open a book? Draw a bow? Ripple? And so on, and we should review all possible item interactions in that case and what this API should do. The other thing is can we guarantee that providing ItemStackSnapshot is valid for all possible activations? This can all be fixed with documentation but there are concerns whatever this will end up being pit of failure.

Isolating this to just totem makes this at least straightforward.

[Yeregorix]

You can give any item type with the death_protection component and it will play the animation with the corresponding texture on screen. It's not limited to the totem of undying. That's why I have been looking for an item-agnostic name, but yeah the "activation" term is quite vague.

[aromaa]

If we are going to append the component implicitly, I think it makes the most sense to be very explicit about this or the API is going to be ambiguous on what it actually does now or in the future when Mojang makes changes.

[Yeregorix]

No it doesn't alter the item server-side. This is all client-side only. The implementation fakes an item in the hand of the player, plays the animation, then removes the item. All in a bundle packet.

[aromaa]

My point was that if the API was in the shape of playItemActivation I would expect it to perform the item specific action, like open a book. But being explicit about this with playTotemOfUndyingEffect we can tell the consumer that this is the behavior you get and we can internally fake whatever is needed to perform the action on the client.

[Yeregorix]

I see. I agree it makes sense to go with playTotemOfUndyingEffect for clarity. The javadoc should state that it works with any item type, to avoid confusion.

[Yeregorix]

Merged by 5dd330e