-
-
Notifications
You must be signed in to change notification settings - Fork 256
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 support for 1.20.2+ clients to older servers on bungeecord #3534
Conversation
I don't think I'll have time to review/investigate this before 1.20.3, but I'll get to to it at some point, thank you |
We are on 1.20.4 so is this PR considered for review or on hold atm? |
20c4f33
to
9c764b5
Compare
Rebased onto the latest development version |
@@ -73,16 +84,63 @@ public class BungeeServerHandler implements Listener { | |||
setProtocol = Class.forName("net.md_5.bungee.protocol.packet.Handshake").getDeclaredMethod("setProtocolVersion", int.class); | |||
getEntityMap = Class.forName("net.md_5.bungee.entitymap.EntityMap").getDeclaredMethod("getEntityMap", int.class); | |||
setVersion = Class.forName("net.md_5.bungee.netty.ChannelWrapper").getDeclaredMethod("setVersion", int.class); | |||
|
|||
Class<?> clazzProtocol = Class.forName("net.md_5.bungee.protocol.Protocol"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can access this class directly
Class<?> clazzProtocol = Class.forName("net.md_5.bungee.protocol.Protocol"); | ||
setEncodeProtocol = Class.forName("net.md_5.bungee.netty.ChannelWrapper").getDeclaredMethod("setEncodeProtocol", clazzProtocol); | ||
setDecodeProtocol = Class.forName("net.md_5.bungee.netty.ChannelWrapper").getDeclaredMethod("setDecodeProtocol", clazzProtocol); | ||
GAME = clazzProtocol.getField("GAME").get(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This constant as well
event.getPlayer().getUniqueId()); | ||
userConnection.getProtocolInfo().setUsername( | ||
event.getPlayer().getName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the line breaks within the setters
Channel c; | ||
try { | ||
Object ch = channelWrapper.get(event.getPlayer()); | ||
c = (Channel) channelWrapperChannel.get(ch); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
channel, channelWrapper
Same below
I feel like the only long-term solution to this is to PR backend/frontend injection points to Waterfall so we can remove all of that hacked code, and to drop Bungee (assuming md would not accept such a PR; https://github.com/PaperMC/Velocity/pull/294/files for both the server and client connection) |
Does this mean we have to tell people to migrate from bungeecord to waterfall in viaversion code or how can this be done in terms of support? |
Dropping support for BungeeCord doesn't seem like a good solution for this to me, BungeeCord is still one of the most used Proxy softwares and I don't think we'll ever have a reason to drop support for it except the code quality of the bungeecord implementation, which I don't think is important anyways. On the other hand, maybe we can ask md_5 about an API before we just assume he wouldn't accept it. |
Any news? |
Since waterfall is essentially end-of-life, there is currently no reason to drop bungeecord in favor of software that is basically EOL - as for the rest of this PR; currently unsure what the status of this is at the moment. |
BungeeCord support has been moved out into a separate bootstrapping plugin starting with v5 versions, see https://github.com/ViaVersion/ViaBungee. If you wish to continue this PR, you should reopen it on the new repository together with a rebase for V5 changes + the requested changes from kenny. |
This should work around the way BungeeCord handles the first connection on 1.20.2+. Bungeecord keeps the player in the 'login' state, so that the first backend the player connects to can complete the handshake. However, without this patch that does prevent ViaVersion from working properly, as it relies on the login state being completed to initialize its user object, and changes the version of the connection without adjusting the game state or sending out a packet to complete the login.
NOTE: This will need some (minor?) adjustments before this can be merged, that go beyond my experience with ViaVersion (which was none before this patch). Sending out the 'GAME_PROFILE' packet also (incorrectly) invokes the base protocol handler, which expects a string UUID, instead of the two longs. To be able to test this, an early return was added to the base protocol, but this will probably break non-bungeecord versions.
This was locally tested with bungeecord and two (custom) 1.12.2 backends, and is currently running in a more conventional bungeecord production setup with several (modified) paper backends. No testing was done on anything non-bungeecord.
Any feedback or help with that remaining issue would be appreciated.
Fixes ViaVersion/ViaBungee#2