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

improve support for custom payloads in 1.20.2 #2553

Merged
merged 2 commits into from
Oct 25, 2023

Conversation

derklaro
Copy link
Contributor

@derklaro derklaro commented Oct 5, 2023

Adds a full test for custom payloads to be serialized and deserialized correctly by:

  • creating a clientbound payload packet with data
  • serializing the payload packet
  • deserializing it to a serverbound packet
  • reading the custom payload & validating it

This test now properly ensures that (de-) serializing a custom payload works as expected over the wire. Previous tests only captured deserialize and cloning.

@derklaro
Copy link
Contributor Author

derklaro commented Oct 6, 2023

Gonna put this into draft until I implemented a way to read data from every CustomPayload implementation, not only the Spigot specific "UnknownPayload"

@derklaro derklaro marked this pull request as draft October 6, 2023 07:00
@derklaro
Copy link
Contributor Author

derklaro commented Oct 6, 2023

Now any kind of custom payload can be read using getCustomPacketPayloads() and it's no longer limited to ServerboundCustomPayloadPacket.UnknownPayload

@derklaro derklaro marked this pull request as ready for review October 6, 2023 10:23
@derklaro derklaro changed the title add actual full test for custom payload packets improve support for custom payloads in 1.20.2 Oct 6, 2023
@Potothingi
Copy link

@dmulloy2 When will this be merged? Should I directly build and use this PR?

@dmulloy2 dmulloy2 enabled auto-merge (squash) October 25, 2023 00:58
@dmulloy2 dmulloy2 merged commit a7aa31a into dmulloy2:master Oct 25, 2023
3 checks passed
@molor molor mentioned this pull request Dec 18, 2023
1 task
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