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

CommandReloader - fix reload issue on paper #6619

Merged

Conversation

ShaneBeee
Copy link
Contributor

Description

This PR aims to update the reloader code for modern PaperMC servers.
Paper has removed the CB Relocation revision number for the packages.
As it stands right now Skript fails to load command and/or allow for reloading on Paper 1.20.5.


Target Minecraft Versions: 1.20.5+
Requirements: none
Related Issues: none

@Moderocky Moderocky added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Apr 28, 2024
@sovdeeth
Copy link
Member

I just want to confirm this doesn't cause any issues with older versions, like Spigot 1.13? I tested it and it seems to work, but I'm not 100% sure how this change affects things.

@ShaneBeee
Copy link
Contributor Author

I just want to confirm this doesn't cause any issues with older versions, like Spigot 1.13? I tested it and it seems to work, but I'm not 100% sure how this change affects things.

The class is the same on all versions it should work.
Im not really sure why bensku chose to write it that way. I will test on 1.13 and confirm shortly.

@Moderocky
Copy link
Member

I just want to confirm this doesn't cause any issues with older versions, like Spigot 1.13? I tested it and it seems to work, but I'm not 100% sure how this change affects things.

From what I can tell it used to look for the method explicitly from the CraftServer class (by getting the package name of the CraftServer class and then... finding the CraftServer class..?) and now it just uses the class of the server instance so it would only break servers running a different Bukkit server implementation that just happened to be in the same package, which seems a bit incredible.

@ShaneBeee
Copy link
Contributor Author

Update:

  • Tested on Paper 1.13.2, script/commands reload as expected 👍🏻

Copy link
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

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

sounds good!

@sovdeeth sovdeeth added the patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version. label Apr 28, 2024
@APickledWalrus APickledWalrus merged commit a9d47fe into SkriptLang:dev/patch Apr 30, 2024
4 checks passed
@ShaneBeee ShaneBeee deleted the fix/command-load-on-paper branch May 8, 2024 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants