Sponge: Unable to properly set vanish on login

[ImMorpheus]

I am currently running

• SpongeForge version:
• SpongeVanilla version: 1.12 from github (SpongeCommon to this commit)

Issue Description
This was discovered while trying to fix a few vanish-related issues with Nucleus without using hacky solution.
This is the current situation when you try to login on a server:

• A ClientConnectionEvent.Login is fired here: https://github.com/SpongePowered/SpongeCommon/blob/bleeding/src/main/java/org/spongepowered/common/mixin/core/server/MixinPlayerList.java#L271 (You can't offer keys to the player during this event, as there's no "player")
• The NetHandler is assigned to the player and the required packets are sent.
• Then playerLoggedIn is called, but it is injected here before it sends the SPacketPlayerListItem packet to all players to modify the tablist (so that we can support Keys.VANISH https://github.com/SpongePowered/SpongeCommon/blob/bleeding/src/main/java/org/spongepowered/common/mixin/core/server/MixinPlayerList.java#L789)
• The canSee method should check if this entity can see the provided entity, however this check is always true as the only point where you can offer Keys.VANISH is during ClientConnectionEvent.Join which is fired way after the tab check.

Pinging @dualspiral, not sure if you found more to this while working on Nucleus

TLDR: a player is added to the tablist before we can offer any keys, so there's no way to properly vanish on login.

[Aaron1011]

I didn't test my commit thoroughly enough before pushing it. It turns out that the client relies on having the tab list packets sent at a certain time. Resolving this issue will be somewhat tricky.

[dualspiral]

If someone reminds me over the weekend, it should be easy to make a test plugin that can be used to simulate what I was doing, or add a hidden switch into Nucleus with this old behaviour.

I'm sure you know this, this is more for the lay person, but effectively, this isn't really to do with vanish, so much as it is to do with altering the tab list. I was making tab list changes as well, which ended up with us having the unfortunate situation where some users during SoS XV appeared to have Steve/Alex skins to some people. I had just added a feature to alter the tab list based on vanish status, but with different tablists based on who should be able to see who. So, it wasn't picked up by me or SoS staff because they had the "vanish see" permission - but it causes problems to both logging in players and players that have already logged in.

Sorry I missed the ping, @ImMorpheus, but hopefully that gives a bit more context as to why Steve was promenent in SoS!

[ryantheleach]

@CodeCrafter47 do you have any insight, as someone who manages a Bungee plugin for tab lists with Sponge compatibility?

Research on other sites recommend experimenting with sending remove player packet if they are vanished, or alternatively, at least blanking the display name of the player on login.

In order to get players to render, (could be outdated info) the playerlist add packet must be sent before spawning the player.

So some ideas.

1. Queueing the packet actions until the player unvanishes?
2. Immediately sending a remove packet?
3. blanking the players display name in the tab list?

,.

1. 
   
   
   
   • Pro's - hacked clients won't know they joined.
   
   

2. Con's - Complicates the code, could it cause mod issues?

3. • Pro's - Suspect this has a better mod compatibility story.

4. Con's - Would this also mess with other plugins that need to customize the tab list?

5. • Pro's - Super low maintenance, just need to make sure to send the player with the correct name at some point.
6. Con's - Kinda hacky, Could it mess with the tablist or mods?

[CodeCrafter47]

In order to get players to render, (could be outdated info) the playerlist add packet must be sent before spawning the player.

That's correct, the player list entry must be there when spawning the player entity.

Queueing the packet actions until the player unvanishes?

You actually don't need a queue.
I'm by far not an expert for vanish plugins, however the way I understand it, if a player is vanished you probably don't want to spawn a player entity (that's the point of vanish). As long as you don't spawn a player entity (on the client) there's no need for the tab list entry, so you can just not send it and everything will be fine. Only thing to pay attention to is to send the tab list entry before spawning the player entity when the player unvanishes. I think that's how it's done in some popular Bukkit plugins, though I could be wrong.
I don't have a lot of experience using mods so I can't tell whether it'll cause any issues.

Immediately sending a remove packet?

There are actually use cases for this, though I don't think it's used in vanish plugins. If you send a remove packet a short time after spawning the player entity, chances are that the client will render the player just fine. If I'm not mistaken Citizens uses this trick to avoid spamming the tab list with NPCs.

blanking the players display name in the tab list?

That's lame. Also the player icon will still be on the tab list. My plugin does this in some cases (i.e. if the vanish plugin isn't removing the player list entry). Though it's easy to implement.