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

Fix config phase packets getting lost #1316

Closed

Conversation

valaphee
Copy link

@valaphee valaphee commented May 5, 2024

When the config-phase is initiated externally, packets are getting lost, as there is no connectionInFlight.

This is most noticeable with PluginMessagePacket as some client-only mods initiate their state in the config phase. The servers request is receipt, but the client has no way to acknowledge the pm.

One common scenario where it would occur is when you chain multiple Velocity servers. But it should be the expected behavior nonetheless.

@R00tB33rMan
Copy link
Contributor

R00tB33rMan commented May 11, 2024

When the config-phase is initiated externally, packets are getting lost, as there is no connectionInFlight.

This is most noticeable with PluginMessagePacket as some client-only mods initiate their state in the config phase. The servers request is receipt, but the client has no way to acknowledge the pm.

One common scenario where it would occur is when you chain multiple Velocity servers. But it should be the expected behavior nonetheless.

Does this theoretically resolve #1251?

@valaphee
Copy link
Author

valaphee commented May 12, 2024

When the config-phase is initiated externally, packets are getting lost, as there is no connectionInFlight.
This is most noticeable with PluginMessagePacket as some client-only mods initiate their state in the config phase. The servers request is receipt, but the client has no way to acknowledge the pm.
One common scenario where it would occur is when you chain multiple Velocity servers. But it should be the expected behavior nonetheless.

Does this theoretically resolve #1251?

Shouldn't impact this issue. At first this PR only impacts client -> server packets, and only normally unrelevant packets. (at least when you don't use modded Minecraft)

if (serverConn != null) {
serverConn.ensureConnected().write(packet);
} else {
player.getConnection().write(packet);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this sending data to the wrong thing? you either want to get the connection in flight, or the connected server?

@electronicboy
Copy link
Member

Closed in favour of #1326 as this is sending packets in the wrong direction

@valaphee
Copy link
Author

connection is the clientbound connection, and this is the proxy handler acting as client? But the other PR is probably a better solution, I'll try.

But this definitely works, as we use a client mod and the handshake worked correctly afterwards.

@electronicboy
Copy link
Member

ClientConfigSessionHandler is receiving packets from the client, hence why it has logic to write to the connection in-flight, so I'm honestly not sure how this would fix anything scratches head

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

Successfully merging this pull request may close these issues.

None yet

3 participants