Skip to content

Support 1.20.6 #271

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Sep 9, 2024
Merged

Support 1.20.6 #271

merged 9 commits into from
Sep 9, 2024

Conversation

@EpicPlayerA10
Copy link
Contributor

@EpicPlayerA10 EpicPlayerA10 commented May 29, 2024 •
edited
Loading

Changes

  • Added required adapters for 1.20.6 due to internals breaking changes
  • Restructured a bit PlayerHeadAdapters
  • Added paper-specific adapters too. This change will make dough future-proof on paper.
  • Added exclusions in pom.xml in module dough-protection because dough couldn't be compiled. This is because one of dependencies provided very old version of bukkit which broke compilation
  • In main pom.xml changed from spigot to paper to be able to use paper features in adapters

@EpicPlayerA10 EpicPlayerA10 mentioned this pull request May 30, 2024
Closed
4 tasks
@EpicPlayerA10
Copy link
Contributor Author

ItemUtils.damageItem needs an update as well, the unbreaking enchantment enum name is renamed.

We don't need to do this as from what i see dough is not updating minecraft version in the pom.xml:

dough/pom.xml

Lines 287 to 292 in baf2d79

<dependency>
<groupId>org.spigotmc</groupId>
<artifactId>spigot-api</artifactId>
<version>1.18.1-R0.1-SNAPSHOT</version>
<scope>provided</scope>
</dependency>

And spigot already auto renames these fields for plugins that are built for older minecraft versions: https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/src/main/java/org/bukkit/craftbukkit/legacy/FieldRename.java . So no error will be thrown.

@mcchampions
Copy link

This pr conflicts with #219.

@md5sha256
Copy link
Contributor

ItemUtils.damageItem needs an update as well, the unbreaking enchantment enum name is renamed.

We don't need to do this as from what i see dough is not updating minecraft version in the pom.xml:

dough/pom.xml

Lines 287 to 292 in baf2d79

<dependency>
<groupId>org.spigotmc</groupId>
<artifactId>spigot-api</artifactId>
<version>1.18.1-R0.1-SNAPSHOT</version>
<scope>provided</scope>
</dependency>

And spigot already auto renames these fields for plugins that are built for older minecraft versions: https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/src/main/java/org/bukkit/craftbukkit/legacy/FieldRename.java . So no error will be thrown.

This does need to be updated as otherwise dough cannot compile if the consumer is compiling against 1.20.5+. The suggested fix is to refactor it to use the bukkit Registry.ENCHANTMENT api and grab it via the minecraft namespaced key.

@EpicPlayerA10
Copy link
Contributor Author

Done

mcchampions
mcchampions suggested changes Aug 12, 2024
View reviewed changes
dough-skins/src/main/java/io/github/bakedlibs/dough/skins/nms/PlayerHeadAdapterPaper.java Outdated
Property property = profile.getProperties().get("textures").iterator().next();

PlayerProfile paperPlayerProfile = Bukkit.createProfile(profile.getId(), profile.getName());
paperPlayerProfile.setProperty(new ProfileProperty(property.name(), property.value(), property.signature()));
Copy link

@mcchampions mcchampions Aug 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Java started using records in Java 16, while Authlib didn't use records until much later. As of the last version of Paper 1.18.2 that depended on Authlib, Property was still a class. Therefore, you might need to use reflection to call the getter method of an older version of Property.

ybw0014
ybw0014 suggested changes Sep 8, 2024
View reviewed changes
Copy link
Contributor

@ybw0014 ybw0014 left a comment

Choose a reason for hiding this comment

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

Based on the result server testing by boomer:

@WalshyDev
Put paper adapter behind 1.19.4 gate
66870dd 
The adapter is using adventure which was not bundled into Paper until 1.19.4
so to support it, we can only use this version or higher.
@WalshyDev WalshyDev mentioned this pull request Sep 9, 2024
Merged
10 tasks
@WalshyDev WalshyDev merged commit f8ff251 into Slimefun:main Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@WalshyDev WalshyDev WalshyDev approved these changes

+4 more reviewers

@ybw0014 ybw0014 ybw0014 requested changes

@mcchampions mcchampions mcchampions requested changes

@DexterSK DexterSK DexterSK approved these changes

@IvanMishin1 IvanMishin1 IvanMishin1 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@EpicPlayerA10 @mcchampions @md5sha256 @DexterSK @ybw0014 @WalshyDev @IvanMishin1