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

1.20.2 Support #1088

Merged
merged 42 commits into from
Oct 10, 2023
Merged

1.20.2 Support #1088

merged 42 commits into from
Oct 10, 2023

Conversation

Paul19988
Copy link
Contributor

Adds support for 1.20.2 clients.
Only caveat at the moment is events that run before the player has reached the play state may need to be queued in future.

Copy link
Contributor

@Joo200 Joo200 left a comment

Choose a reason for hiding this comment

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

Looks nice overall. However it could be nice if you reduce the reformatting of this PR to minify the diff.

@powercasgamer
Copy link
Contributor

There are a lot of unnecessary code style changes

@Paul19988
Copy link
Contributor Author

Some of the reformats are from the IDE attempting to change it to what it believes to be right & not manual intervention

Copy link
Contributor

@astei astei left a comment

Choose a reason for hiding this comment

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

Please reformat these changes to use the Velocity code style (~Google code style). Left a few other comments.

@NEZNAMY
Copy link

NEZNAMY commented Sep 25, 2023

Is there a pre-compiled binary available for testing?

@Attack8
Copy link

Attack8 commented Sep 25, 2023

Is there a pre-compiled binary available for testing?

You should compile it yourself since there is no official build
I could give you a comiled jar, but you shouldn't trust unofficial builds

@IgnitingIce
Copy link

Tried out this PR and experiencing this error on join:
image

@Vislo
Copy link

Vislo commented Sep 25, 2023

Tried out this PR and experiencing this error on join:
image

I’m not sure how to reproduce it but for some reason only happens randomly if you disconnect and try to reconnect, if you get this error connecting you will keep getting this error until you restart the Minecraft client. Hope this information helps.

@totKing
Copy link

totKing commented Oct 3, 2023

https://mclo.gs/EI1AL4a
With the last push (1b07659)

Beginning it works. After a timeout, the error occurs.

@Paul19988
Copy link
Contributor Author

https://mclo.gs/EI1AL4a With the last push (1b07659)

Beginning it works. After a timeout, the error occurs.

Looks like you have a plugin on the proxy attempting to send the player a message before they've finished the configuration stage

@totKing
Copy link

totKing commented Oct 3, 2023

https://mclo.gs/EI1AL4a Mit dem letzten Push ( 1b07659 )
Von Anfang an klappt es. Nach einer Zeitüberschreitung tritt der Fehler auf.

Es sieht so aus, als ob Sie ein Plugin auf dem Proxy haben, das versucht, dem Player eine Nachricht zu senden, bevor er die Konfigurationsphase abgeschlossen hat

It worked yes, after last commit from you.
If the player gets timeout, he can not enter until the proxy restart.

@MVanilla
Copy link

MVanilla commented Oct 3, 2023

If the player moves to a different backend server during the configuration phase, an error will occur. 1 Velocity logs

[22:43:53 INFO]: [connected player] MyName (/127.0.0.1:49436) has connected
[22:43:53 INFO]: [server connection] MyName -> survival has connected
> send MyName survival2
[22:43:54 INFO]: [server connection] MyName -> survival2 has connected
[22:43:54 INFO]: Attempting to send 0 players to survival2
[22:43:54 INFO]: [server connection] MyName -> survival has disconnected
[22:43:55 INFO]: [connected player] MyName (/127.0.0.1:49436) has disconnected
[22:43:55 INFO]: [server connection] MyName -> survival2 has disconnecte

Client logs

[22:43:55] [Render thread/WARN]: Client disconnected with reason: Internal Exception: io.netty.handler.codec.EncoderException: java.io.IOException: Can't serialize unregistered packet

Plugins used: https://github.com/Elytrium/VelocityTools

Solution: 2 When a player is in the configuration phase, Velocity should either deny the player from moving to another backend server or use a queue.

Are you planning on fixing this? This error occurs when using the server send or join queue plugin.
https://github.com/Elytrium/VelocityTools/releases
https://www.spigotmc.org/resources/ajqueue.78266/

@yinzixie
Copy link

yinzixie commented Oct 6, 2023

Any progress ?

@Leguan16 Leguan16 mentioned this pull request Oct 6, 2023
@p0t4t0sandwich
Copy link

Just need to be patient, if you really need it now and can't live without it, follow the readme on how to compile it.
Asking if there's any progress won't make it go faster.

@Strahilchu
Copy link

Come on Velocity you can do this

@Phoenix616
Copy link
Contributor

Phoenix616 commented Oct 6, 2023

Please stop spamming this issue if you have nothing to improve it or feedback from your testing. If you just want updates on the progress then follow the issue/specify what notifications you want on the right, if you want to show your support then react to the posts, and if you want to test the 1.20.2 integration then build the PR (seeing as it uses gradle's wrapper it should be extremely easy to build it)

On an actual relevant note: Has the resource pack API issue been addressed/discussed @Paul19988? (See #1088 (comment)) Because I don't see anything about that in the PR. (But it's extremely hard to review with all the formatting changes :S)

@DeprecatedLuke
Copy link

DeprecatedLuke commented Oct 7, 2023

If you run ViaVersion on the backend servers make sure you upgrade it to 4.8.1 before use if you attempt to use this branch.

Stuff working:

  • Previous versions
  • Geyser

Possible issues:

  • Boss bar does not get cleared when switching servers?
  • Slow connections have trouble switching servers ( on 1.20.2? ). No errors in console except for the normal timeout messages.
  • Plugin compatibility. Network I oversee only uses 1 custom-made plugin (managing backend servers, kick and send handling as well as motd) alongside Geyser.

There is no error spam and is quite usable in production.

@Gerrygames
Copy link
Contributor

Gerrygames commented Oct 7, 2023

I implemented support for the backend server switching back to config state.

I also implemented a fix for the issue with the resource pack being cleared on server switch. The previously applied resource pack just gets reapplied after the server switch is done.
For some users it might be useful for the resource pack to be cleared on server switch. So we might need to find a better solution for this. However I would like to make sure we don't break the API. So this approach is the best solution for now. Also we don't know if Mojang will fix this issue with 1.20.3.

I created a PR for my changes here: Paul19988#2

@MVanilla

This comment was marked as spam.

froobynooby added a commit to FroobWorld/Velocity that referenced this pull request Oct 8, 2023
@rayanbzd
Copy link

rayanbzd commented Oct 8, 2023

I implemented support for the backend server switching back to config state.

I also implemented a fix for the issue with the resource pack being cleared on server switch. The previously applied resource pack just gets reapplied after the server switch is done. For some users it might be useful for the resource pack to be cleared on server switch. So we might need to find a better solution for this. However I would like to make sure we don't break the API. So this approach is the best solution for now. Also we don't know if Mojang will fix this issue with 1.20.3.

I created a PR for my changes here: Paul19988#2

Here is a bug report about this ressource pack issue: https://bugs.mojang.com/browse/MC-265673

@bluegreensea
Copy link

This PR Paul19988#3 shall recover all only had formatting codes

@Phoenix616
Copy link
Contributor

Just to make sure it doesn't get lost: Added a comment regarding resource pack sending after switch here

Copy link
Member

@kennytv kennytv left a comment

Choose a reason for hiding this comment

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

Thanks for the hard work, everyone!

@kennytv kennytv merged commit 768ecdb into PaperMC:dev/3.0.0 Oct 10, 2023
1 check passed
@Xernium Xernium mentioned this pull request Oct 12, 2023
BlackBaroness added a commit to PvPClashnet/Velocity-CommandExecuteEvent that referenced this pull request Oct 24, 2023
* Add version information for 1.20.1 (PaperMC#1021)

* bump adventure to 4.14.0 (PaperMC#1034)

* check if a plugin has a executor service (PaperMC#1038)

* check if a plugin has an executor service

* checkstyle

* feat: add TabList#addEntries (PaperMC#987)

* [ci skip] Replaced weired i with i in javadocs (PaperMC#1057)

In this little patch I replaced an i which caused my build process to crash with an i

* Do not track plugin channels registered per-player on the proxy (PaperMC#591)

We don't need to track this information since Velocity uses the JoinGame packet, which is about as good of a server rejoin mechanism we're likely to get in vanilla Minecraft.

* fix PaperMC#1062

* 1.20.2 Support (PaperMC#1088)

Co-authored-by: RednedEpic <[email protected]>
Co-authored-by: Gero <[email protected]>

* Actually send plugin message registration to backend servers

I don't see where this was ever done, and don't see how plugin messaging
could of ever worked, at least within the confines of CB and co, given
the fact that we never seemed to be sending this to the backend?

* appease checkstyle, move back to older fix placement

* Change packet decode logging prompt

* Catch throwables when loading plugins (PaperMC#1098)

* Several improvements and fixes for 1.20.2 (PaperMC#1097)

* Send LoginAcknowledged immediately

* Resend player list header/footer after backend server switched to config state

* Fix clearHeaderAndFooter not clearing fields in ConnectedPlayer

* Clear boss bars, header/footer, tab list when switching client to config state

* Send client settings in config state

* Fix unsigned commands detected as signed (PaperMC#1082)

* fix: commands flagged as signed without signed arguments

* feat: improve error message for illegal protocol state.

* acknowledge seen messages to server when running proxy commands (PaperMC#1100)

* Implement ComponentLogger (PaperMC#1022)

* Log the protocol phase in case of trying to obtain a packet id not existing in the phase (PaperMC#1107)

* Bump netty to 4.1.100.Final (PaperMC#1067)

---------

Co-authored-by: Pantera (Mad_Daniel) <[email protected]>
Co-authored-by: chris <[email protected]>
Co-authored-by: Aaron <[email protected]>
Co-authored-by: powercas_gamer <[email protected]>
Co-authored-by: Groldi <[email protected]>
Co-authored-by: Andrew Steinborn <[email protected]>
Co-authored-by: Emmanuel Lampe <[email protected]>
Co-authored-by: Paul <[email protected]>
Co-authored-by: RednedEpic <[email protected]>
Co-authored-by: Gero <[email protected]>
Co-authored-by: Shane Freeder <[email protected]>
Co-authored-by: Joo200 <[email protected]>
Co-authored-by: Adrian <[email protected]>
BlackBaroness added a commit to influct/Velocity that referenced this pull request Oct 24, 2023
* 1.20.2 Support (PaperMC#1088)

Co-authored-by: RednedEpic <[email protected]>
Co-authored-by: Gero <[email protected]>

* Actually send plugin message registration to backend servers

I don't see where this was ever done, and don't see how plugin messaging
could of ever worked, at least within the confines of CB and co, given
the fact that we never seemed to be sending this to the backend?

* appease checkstyle, move back to older fix placement

* Change packet decode logging prompt

* Catch throwables when loading plugins (PaperMC#1098)

* Several improvements and fixes for 1.20.2 (PaperMC#1097)

* Send LoginAcknowledged immediately

* Resend player list header/footer after backend server switched to config state

* Fix clearHeaderAndFooter not clearing fields in ConnectedPlayer

* Clear boss bars, header/footer, tab list when switching client to config state

* Send client settings in config state

* Fix unsigned commands detected as signed (PaperMC#1082)

* fix: commands flagged as signed without signed arguments

* feat: improve error message for illegal protocol state.

* acknowledge seen messages to server when running proxy commands (PaperMC#1100)

* Implement ComponentLogger (PaperMC#1022)

* Log the protocol phase in case of trying to obtain a packet id not existing in the phase (PaperMC#1107)

* Bump netty to 4.1.100.Final (PaperMC#1067)

---------

Co-authored-by: Paul <[email protected]>
Co-authored-by: RednedEpic <[email protected]>
Co-authored-by: Gero <[email protected]>
Co-authored-by: Shane Freeder <[email protected]>
Co-authored-by: Joo200 <[email protected]>
Co-authored-by: Adrian <[email protected]>
Co-authored-by: Pantera (Mad_Daniel) <[email protected]>
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.