Issue with Data Keys.INVISIBILITY

[clucky]

I'm encountering an issue when offering the following data to an ArmorStand Entity that I created within my plugin:

• Keys.INVISIBLE: true
• Keys.CUSTOM_NAME_VISIBLE: true
• Keys.DISPLAY_NAME: ████████████████

The issue is that the name is not visible. In fact, I can't even change the armor stand to not be invisible using the Minecraft /entitydata command, nor does manually changing CustomNameVisible with the /entitydata command resolve my issue. I would like to note though, after a server restart, the names do appear finally.

I'm not certain if this issue is due to Sponge or Forge, but it is certainly not Minecraft. I know this because I have placed armor stands by hand and set all of these values and it worked fine.

[clucky]

I've created a gist highlighting my code, for relevance. Note: this is not a complete class file, so there may be some methods/fields missing, but I attempted to include all relevant ones.

https://gist.github.com/clucky/51e5c946844452051d2d

[clucky]

It is appearing that Keys.INVISIBLE is creating complete invisibility. Upon spawning of the invisible entity, there is no increase on the Entity counter in f3, even though the entity does exist on the server (/kill @e displays death message for ArmorStand).

[Lunaphied]

This is rather strange, I will look into this and see what I can dig up.

[Lunaphied]

Ok I've found the issue, I need some comments about how to handle this though. Basically the problem is that invisibility has two components. There's "invisibility" which is a property of an entity, and "vanished" which is a component of Sponge.

Minecraft uses invisibility to handle entities that are marked invisibile, which are still sent to the client, just marked with a flag so the game doesn't render them. Sponge uses vanished to prevent the clients from being sent packets pertaining to vanished entities so that players/entities are hid completely from the connected players.

This in most cases works out fine, since players can go invisible and be hidden from even hacked clients, which a NBT flag wouldn't accomplish. However when using this property with ArmorStands, the matter is confused, because the plugin wants only to hide the entity from rendering, not completely "delete" it from the client.

I see three solutions, the first is simply to modify the code that handles the invisibility flag to only set the invisibility of the ArmorStand, not changing it to vanished, causing the intended result. However this method will prevent the possibility of a valid use of wholly vanished ArmorStands. Option 2 is to leave the functionality of Keys.INVISIBLE as is, but modify the ArmorStand entity to support a setInvisible and isInvisible pair of methods that specifically handle the NBT property on ArmorStands. The third option is to do both, although I don't really see why this would be desired it is an option.

@SpongePowered/developers thoughts? The implementation of InvisibilityData is really strange as is and should probably be cleaned up regardless.

[clucky]

My thought would be to have the functionality of Keys.INVISIBLE to serve the purpose of the Minecraft NBT Invisible data, and then have methods setVanished() and isVanished() for entities, which would provide the Sponge packet filtering. When I think of the DataApi, I think of actually accessing the Minecraft NBT, not Sponge methods. It wouldn't make sense for all of the DataAPI to purpose only Minecraft NBT, with the one exception of Keys.INVISIBLE.

[ryantheleach]

DataAPI is available for custom features by plugins and mods as well, I don't see why sponge should be singled out as not being allowed to have data.

I agree however that the naming for vanishing is confusing, and should be a separate key/value to invisibility.

[Lunaphied]

I can implement another key for this. Another thing to consider is the fact that setting invisibility currently automatically seems to activate unbreakable and disable hitbox, unless they are previously set. However, that functionality seems unexpected especially as I can't see any documentation on the API or docs informing the difference.

[felixoi]

What's about adding a Keys.VANISHED and modify the current Keys.INVISIBLE

[Lunaphied]

That was exactly my plan. Working on it now. On Mar 18, 2016 10:08 AM, "Felix K." notifications@github.com wrote:

What's about adding a Keys.VANISHED and modify the current Keys.INVISIBLE

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/SpongePowered/SpongeCommon/issues/588#issuecomment-198401828

[gabizou]

I haven't had a chance to carefully reply to this issue until now.

A few things to state:

• Keys.INVISIBLE is meant to render the entity completely invisible. Completely invisible such that packets are not sent to players for the invisible entity, sounds are not played, particles aren't spawned, etc. The reasoning why it calls the internal setInvisible(boolean) is to maintain compatibility with not only vanilla functionality, but also mod compatibility.
• Mojang's invisibility is a boolean toggle, but it should be treated separately in my opinion, not where it's called INVISIBLE but some other functional word that will relate that the entity is not completely invisible, but only rendered invisible.
• Changing the functionality where Keys.INVISIBILITY doesn't completely render the entity invisible with packet/particle/sound handling is a breaking change, even though the API doesn't actually break, the intended behavior does break with various plugins, such as @mbax's Vanish plugin. So, if anything, the choice of changing the implementation of Keys.INVISIBILTY to only call the internal method, and Keys.VANISHED performs the current functionality would have to go into the bleeding branch of the API and implementation.
• @modwizcode There's no real issue, it's intended functionality as the javadoc states, and as Bukkit's previous implementation used.

I'm open to suggestions on whether a change of the key's name is decided or whether an addition of an alternative "simple invisibility" key is added.

I'm closing this as it's not an implementation issue, but an API usage issue.

[kashike]

API issue: https://github.com/SpongePowered/SpongeAPI/issues/1151

[simon816]

I would just like to point out that complete invisibility was never the intention of InvisibilityData. I can only assume that at some point the definition was changed (not in writing? I don't remember this) Before the data API it was simply a boolean render toggle https://github.com/SpongePowered/SpongeAPI/blob/2046c95cb1fb689133f9bd7f2d110cce112d5546/src/main/java/org/spongepowered/api/entity/living/Living.java#L285-L316 Which I believe would toggle a particular bit in the entity metadata (http://wiki.vg/Entities#Entity_Metadata_Format)

Then when DataAPI 1.0 came out InvisibilityData still seemed to reference 'rendering' invisible only https://github.com/SpongePowered/SpongeAPI/blob/c2da2f690f590f7c1c428d8bfc3fc22279fdb9a0/src/main/java/org/spongepowered/api/data/manipulators/InvisibilityData.java

Even today in dataapi 2.0 it still mentions rendering https://github.com/SpongePowered/SpongeAPI/blob/6a975e7d97d8f523905a880ef85cdb53f4d25761/src/main/java/org/spongepowered/api/data/manipulator/mutable/entity/InvisibilityData.java#L34

I assume that one day it was changed and no proper replacement for the existing functionality was added

[gabizou]

@simon816 refer to the API issue. Forewarning: I wrote the initial Entity API, and I know what I wanted to do, but didn't implement at the time for the sake of time. So what the "original API" intended is nothing near what I had intended when I initially wrote it.