search

SpigotMC / BungeeCord

BungeeCord, the 6th in a generation of server portal suites. Efficiently proxies and maintains connections and transport between multiple Minecraft servers.
https://www.spigotmc.org/go/bungeecord
Other
1.56k stars 1.1k forks source link

Login packet is sent after ServerSwitchEvent is called for 1.20.2 players #3542

Open NEZNAMY opened 11 months ago

NEZNAMY commented 11 months ago

Bungeecord version

Build 1751 (edited to add debug messages & stack traces)

Server version

Spigot 1.20.2

Client version

1.20.2

Bungeecord plugins

A plugin to confirm this issue

The bug

Unlike with 1.20.1 and lower, on 1.20.2 the Login packet is sent after ServerSwitchEvent is called. The issue with this is that Login packet clears the client (most notably scoreboard teams and objectives), including those sent during the event. For 1.20.1 and lower the event is called after login packet is sent, allowing to use the event to send data to players. For 1.20.2, I would either need to listen to the Login packet or delay my task when the event is called, both of which are suboptimal solutions.

After tons of debugging I found out that on 1.20.2 the handleLogin method which sends the packet is called from DownstreamBridge, which is after ServerSwitchEvent was called. On 1.20.1 and lower, the method is called in ServerConnector, before ServerSwitchEvent was called, allowing to send data immediately.

I have also noticed that calling sendPacketQueued during the event does not queue the packets, but sends them immediately.

I'm not really sure if this is a bug or intended, so I'm looking for any kind of solution to this problem.

Log output (links)

Debug stack trace for sending Login packet on 1.20.2 (after ServerSwitchEvent was called)

at net.md_5.bungee.UserConnection$1.sendPacket(UserConnection.java:152)
at net.md_5.bungee.ServerConnector.handleLogin(ServerConnector.java:246)
at net.md_5.bungee.connection.DownstreamBridge.handle(DownstreamBridge.java:769)
at net.md_5.bungee.protocol.packet.Login.handle(Login.java:283)
at net.md_5.bungee.netty.HandlerBoss.channelRead(HandlerBoss.java:124)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
at io.netty.handler.timeout.IdleStateHandler.channelRead(IdleStateHandler.java:286)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:103)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:103)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:346)
at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:333)
at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:454)
at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:290)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:788)
at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:724)
at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:650)
at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:562)
at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
at java.base/java.lang.Thread.run(Thread.java:833)

Debug stack trace for sending Login packet on 1.20.1 (before ServerSwitchEvent was called):

at net.md_5.bungee.UserConnection$1.sendPacket(UserConnection.java:152)
at net.md_5.bungee.ServerConnector.handleLogin(ServerConnector.java:246)
at net.md_5.bungee.ServerConnector.handle(ServerConnector.java:194)
at net.md_5.bungee.protocol.packet.Login.handle(Login.java:283)
at net.md_5.bungee.netty.HandlerBoss.channelRead(HandlerBoss.java:124)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
at io.netty.handler.timeout.IdleStateHandler.channelRead(IdleStateHandler.java:286)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:103)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:103)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:346)
at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:333)
at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:454)
at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:290)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:788)
at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:724)
at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:650)
at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:562)
at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
at java.base/java.lang.Thread.run(Thread.java:833)

Checking

md-5 commented 11 months ago

This is unavoidable due to Mojang's addition of an extra step in the protocol. What exactly is the bug you think needs addressing?

NEZNAMY commented 11 months ago

I am unable to use the event to send stuff, because it gets cleared right away. So I need a workaround. I came up with 2 ideas on my plugin side, both of which are highly questionable and I'm looking for some confirmation if it's up to me or something can be done on bungeecord side about this. I have no way to know if player received the packet after switching or not yet.

Janmm14 commented 11 months ago

Maybe a suitable solution could be to move ServerConnectedEvent call in ServerConnector to after we sent Login packets in that method and NEZNAMY should then use ServerConnectedEvent.

But to be honest with this change I feel even more lost inside bungee's code. What is called in which state of the protocol, what will happen afterwards etc. I think there is a need of some diagrams detailing what methods will be called and which packets are sent/expected to be recieved for first connection / server switches per mc version.

0utplay commented 11 months ago

I think I found a similar problem with the events in the 1.20.1 / 1.20.2. While the ServerConnectedEvent is called on an initial login in 1.20.1 without a server (see figure 1), in 1.20.2 a server is already assigned to the player (see figure 2).

Running version git:BungeeCord-Bootstrap:1.20-R0.2-SNAPSHOT:1ef4d27:1754 Fig. 1 (connecting with 1.20.1) idea64_IT2mrZnsNk Fig. 2 (connecting with 1.20.2) idea64_oWO3kl76HT

This causes problems with plugins that rely on the event behavior remaining the same especially when the bungeecord version is the same

md-5 commented 11 months ago

I think something has to give, it is impossible for 1.20.1 and 1.20.2 to have the same behaviour. If not this (minor) difference you get a much bigger difference of the connection not being in a state to send most packets.

0utplay commented 11 months ago

Do you think we can somehow distinguish between an initial connect or server switch in the ServerConnectedEvent? This would at least the problem I am facing with the changes in 1.20.2.

NEZNAMY commented 11 months ago

Adding an event when login packet is sent would also solve this.

MattTheTekie commented 11 months ago

Is there any progress to this?

md-5 commented 10 months ago

What progress / fix are you expecting given the above comments?

Codixer commented 10 months ago

Presumably? Provide a solution for players who use your software. Bungeecord is one of the most important pieces of networking software right next to Velocity.

While there are forks who try to patch these issues, it would be nice if stuff gets solved downstream. As plugin developers have to take the blame when a user goes "WAAA, your plugin doesnt work!!!! WHAAAA, I will review you 1 star because bungeecord doesn't work, WHAAAA". While this isn't even the fault of the developers of a plugin, but the software the plugin runs on.

md-5 commented 10 months ago

While there are forks who try to patch these issues, it would be nice if stuff gets solved downstream.

They're welcome to provide a pull request upstream.

As per the ticket above it's very unclear what the fix for this is or if it's even a bug. Please explain what about BungeeCord is not working. You haven't contributed anything helpful to this discussion.

Owen1212055 commented 9 months ago

Some food for thought, it would be beneficial if there was some event that could be fired when the configuration stage is completed on the client and they are now in the GAME protocol state.

I guess it's fair that this event firing here would cause these errors, but I think it's most reasonable to assume that the current functionality of the event doesn't really serve the best purpose-wise. Most notably previous code that communicated with the backend server in this stage now errors due to the player not yet being in the world. (Bukkit#getPlayer returns null)

I now add a ~500 MS delay after this event's invocation before trying to communicate with the backend (java server)... however, this is not the perfect solution as the client can potentially take longer than 500ms to ack the actual configuration stage.