Support for 1.18+ ClientboundLevelChunkWithLightPacket

[Etrayed]

Name says it all.

Other changes:

• Added equals and hashCode method to MinecraftKey to allow better comparison

This PR also contains tests for all the newly added stuff.

Tested using the latest 1.18.2 paper 1.19 spigot.

[Etrayed]

Also dont be confused with the branch name. Let's just say I underestimated this topic a bit :)

[Etrayed]

@dmulloy2 is there anything I need to change in order for this PR to be merged?

[Aurelien30000]

Thanks for this PR, we would appreciate a merge to address some issues with the PlotHider plugin!

[dmulloy2]

@dmulloy2 is there anything I need to change in order for this PR to be merged?

looks good to me! just needs a few merge conflicts to be resolved

[Etrayed]

@dmulloy2 is there anything I need to change in order for this PR to be merged?

looks good to me! just needs a few merge conflicts to be resolved

Should be good now, tested it again on a spigot 1.19 server

[Etrayed]

@dmulloy2 friendly reminder 🙂

[derklaro]

Just a few nits here:

• Please replace the usage of guava classes with the java.util classes (if possible), for example guava.Objects.hashCode -> java.util.Objects.hashCode
• Please remove the usages of streams, they are slower than normal for-loops (that normally doesn't matter, but most users are using sync packet listeners and blocking the main thread longer than needed shouldn't be a goal)
• Please replace the duplicate field definitions for the same type (private int x, y, z) with explicit ones.

Everything else kinda looks fine. I would really appreciate if you could expand the test cases, as these wrappers (especially this one as it uses data serialisation directly) are outdated quickly when updating to newer minecraft versions, which is a pain if we cannot detect these changes during testing (and maybe event release an update which has a broken wrapper 😢 )

[Etrayed]

Just a few nits here:

• Please replace the usage of guava classes with the java.util classes (if possible), for example guava.Objects.hashCode -> java.util.Objects.hashCode
• Please remove the usages of streams, they are slower than normal for-loops (that normally doesn't matter, but most users are using sync packet listeners and blocking the main thread longer than needed shouldn't be a goal)
• Please replace the duplicate field definitions for the same type (private int x, y, z) with explicit ones.

Everything else kinda looks fine. I would really appreciate if you could expand the test cases, as these wrappers (especially this one as it uses data serialisation directly) are outdated quickly when updating to newer minecraft versions, which is a pain if we cannot detect these changes during testing (and maybe event release an update which has a broken wrapper 😢 )

Hope this is fine, will do the tests later 👍

[derklaro]

We can currently bypass final (and trusted final) fields, yes. That's why I was confused about your serialisation :)

[Etrayed]

We can currently bypass final (and trusted final) fields, yes. That's why I was confused about your serialisation :)

Well thats great. Im going to modify the wrappers again and then im ready to merge.

[Etrayed]

Did the modification, im ready from my side

[derklaro]

Will look at it later / tomorrow 👍

[NoJokeFNA]

Could you please reformat your Code?

[Etrayed]

Could you please reformat your Code?

Did the if spaces now. If you have any further "improvements", let me know and we can discuss them.

[derklaro]

Nice work! I've commented some nits on this, but overall 👍

[derklaro]

juuuuust some more :)

[derklaro]

Should be good to go now

[Etrayed]

@dmulloy2 are we g2g?