Login event should be called after disconnect event, when we have multiple connections for the same player

[MinecraftiUndCo]

Currently (Version: 1.16.4 / #393) when a player connects to the proxy, when another connection for the same player is still online (out of whichever reasons), it can happen that the login event for the new connection is called before the old connection has their disconnect.

This situation can lead to some problems for custom plugins / development with the order of events and you can't make sure that the order is always disconnect and than login. So you maybe have to do some manual checks in the login event.

As @electronicboy stated in Discord it "would need look into making it happen as a callback in netty if it lets us". It would therefore be good if this is checked so that you can always maintain a defined order of events for a player. Alternatively, one could generally think about whether a new connection should be allowed at all as long as an old connection still exists.

[Xernium]

It seems like this the disconnect that happens when the player is already online is an intentionally detached/async event that detaches here:

https://github.com/SpigotMC/BungeeCord/blob/master/proxy/src/main/java/net/md_5/bungee/netty/ChannelWrapper.java#L83

From:
https://github.com/SpigotMC/BungeeCord/blob/c987ee199d3ec93ce13469deefb1e2787d97147c/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java#L485
refers to:
https://github.com/SpigotMC/BungeeCord/blob/c987ee199d3ec93ce13469deefb1e2787d97147c/proxy/src/main/java/net/md_5/bungee/UserConnection.java#L391

[Xernium]

This probably can’t/won’t be fixed by paper. You should open an issue upstream on Bungeecord as this is a design change we probably won’t accommodate

[MinecraftiUndCo]

I already found an open issue in Bungeecord #SpigotMC/BungeeCord#2815 for that which aims at my problem described. But this has been open and unprocessed for a long time.

A user has already added his draft there. Maybe the paper team could take a look at it. This wrong order of the events can lead to the fact that custom user data from plugins could be first loaded and then removed again by the subsequent disconnect, which is very annoying and certainly so many developers will not be aware of it.

[Xernium]

The draft mentioned in this issue doesn’t take a lot of channel logic into account. Fact of the matter is that to actually get this to follow the right order a completeable future would have to be issued from the Channel disconnect future of the existing client. That isn’t the greatest of ideas as this part of the login sequence is rather time-critical and it can take several seconds or more to complete the disconnect event as you want to wait on that. The player should be identifiable by session, though, that’s still not the best solution.

[MinecraftiUndCo]

Yes you are right. The design is probably not the best. But I don't think handling user data on the basis of the session is a good idea either, as this complicates the whole thing very much.

I can understand the point with the time-critical behavior to a limited extent. In general, it should be the exceptional case that a player tries to connect with a second connection and if this happens, then the time span of the login does not really matter.

Due to this fact, I currently regulate it in such a way that I then unfortunately terminate or break off both connections so that there are no inconsistencies in my user data, which are stored using the UUID. So it would be nice if some good solution could be found for that. But when it comes to Netty, I'm unfortunately not the expert myself. 😅