-
-
Notifications
You must be signed in to change notification settings - Fork 36
Bukkit - Update MinecraftComponentSerializer for 1.21.6/7 #212
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
base: main
Are you sure you want to change the base?
Conversation
Works on Paper 1.21.6 but not fully on Spigot. I'll push a fix when I can. |
Spigot works properly now. The |
Cached JsonOps.INSTANCE via reflection to avoid repeated class and field lookups. PR should be ready for pull now. |
@bloodmc Checkstyle is failing, probably want the build to pass before merging 😅 |
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.
Need to create a registry-aware JsonOps similar to https://github.com/KyoriPowered/adventure-platform-mod/blob/mc/1.21/mod-shared/src/main/java/net/kyori/adventure/platform/modcommon/impl/NonWrappingComponentSerializer.java and https://github.com/PaperMC/Paper/blob/0cadaefc094c1d25eb19332cfebc02f9b5885c4a/paper-server/src/main/java/io/papermc/paper/adventure/PaperAdventure.java#L143
This comment was marked as spam.
This comment was marked as spam.
I've tested this PR in mcMMO and it is working with Paper 1.21.7 builds, hover components and action bar messages are now being sent correctly. |
Waiting for this on our end as well; currently testing the PR with some custom builds and going well. :) |
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.
I've (smoke-)tested this PR alongside Adventure 4.23.0 on the currently "most popular" versions of Spigot and Paper (latest builds of 1.8.8, 1.12.2, 1.16.5, 1.18.2, 1.19.4, 1.20.6 and 1.21.7), and didn't notice any issues. My testing included whether MinecraftComponentSerializer#isSupported() evaluates to true, and that (de)serialization works by checking that the following snippet produces working components:
Component adventureComponent = text("Hello, Adventure!", RED, BOLD);
Object nmsComponent = MinecraftComponentSerializer.get().serialize(adventureComponent);
Component deserializedComponent = MinecraftComponentSerializer.get().deserialize(nmsComponent);
I also threw this into production with Paper 1.21.7 and Citizens (CitizensDev/Citizens2@1049ca6), and didn't notice any issues.
Just for reference for those of you following this PR:
We appreciate your patience 💪 |
This PR adds support for Minecraft 1.21.6 by integrating the new
ComponentSerialization.CODEC
for component serialization, while preserving compatibility with older versions.