-
Notifications
You must be signed in to change notification settings - Fork 35
feat: Added Folia Support #21
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: master
Are you sure you want to change the base?
Conversation
LoJoSho
left a comment
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'm just confused at the direction of this PR, please review and modify based on some of the feedback I've left.
| import org.jetbrains.annotations.ApiStatus; | ||
|
|
||
| public final class HibiscusCommonsPlugin extends HibiscusPlugin { | ||
| public final class HibiscusCommonsPlugin extends HibiscusPlugin |
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.
Do not change formatting of code.
| if (onPaper) { | ||
|
|
||
| if (onFolia) { | ||
| getLogger().info("Detected Folia by Cássio Martim! Enabling Folia support..."); |
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.
Why can't it be a generic message? The plugin doesn't go in saying "Detected Paper by LoJoSho!"
| getLogger().info("Detected Folia by Cássio Martim! Enabling Folia support..."); | ||
|
|
||
| foliaTask = new FoliaTaskService(this); | ||
| } else if (onPaper) { |
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.
Why is this an else if statement? Folia runs on Paper, which means we can use the methods provided by Paper?
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.
Ok, so after looking at some NMSPackets code, it seems your uisng onPaper has a way to differentiate between regular and folia servers? That's not the intended use case, it's for Spigot vs Paper supported forks.
|
|
||
| public HibiscusCommonsPlugin() { | ||
| @Getter | ||
| private static FoliaTaskService foliaTask; |
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'm confused why the Folia Task is only for Folia? It would be incredibly annoying for every single task in every single plugin that extrends Hibiscus Commons to be like:
if (onFolia) foliaTask.run
else bukkitTask.run
No description provided.