Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
3 of 4 tasks
NEZNAMY opened this issue Sep 28, 2023 · 13 comments
Open
3 of 4 tasks

Comments

@NEZNAMY
Copy link

NEZNAMY commented Sep 28, 2023

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

  • I am using BungeeCord and not a fork. Issues with forks should not be reported here.
  • I think this is not an issue with a bungeecord plugin.
  • I have not read these checkboxes and therefore I just ticked them all.
  • This is not a question or plugin creation help request.
@md-5
Copy link
Member

md-5 commented Sep 28, 2023

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
Copy link
Author

NEZNAMY commented Sep 28, 2023

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
Copy link
Contributor

Janmm14 commented Sep 29, 2023

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
Copy link

0utplay commented Oct 1, 2023

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
Copy link
Member

md-5 commented Oct 1, 2023

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
Copy link

0utplay commented Oct 1, 2023

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
Copy link
Author

NEZNAMY commented Oct 1, 2023

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

@MattTheTekie
Copy link

Is there any progress to this?

@LucidAPs
Copy link

Will this be fixed?

@md-5
Copy link
Member

md-5 commented Oct 14, 2023

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

@Codixer
Copy link

Codixer commented Nov 5, 2023

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
Copy link
Member

md-5 commented Nov 5, 2023

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
Copy link

Owen1212055 commented Dec 5, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants