Feature: Add Folia Support #243
Conversation
|
I'm not sure if just replacing all the schedulers would work, as there isn't a main thread anymore. Some calls depend on or must be done on the main thread, so those ones might be a bit more difficult to get right on Folia. |
There still is the concept of a "Global" scheduler. For other things that would modify inventories, entities, etc. those would need to be done at a specific location and I tried to account for that in the PR. I recognize that there is heavy testing overhead with this PR and would be happy to assist however I can. Many servers are moving to Folia and many plugins are adding support. Would love to help out but if not it's totally up to you. Cheers. |
|
Yes, I would love to support Folia, and support Folia for all my remaining plugins as well. The main thing standing in the way of merging this PR is just the testing part as I don't have that much time to research and test Folia just yet. But thank you for your work and rest assured that it will definitely be done at some point. |
|
I understand Folia might not be a priority and this change would require testing - however, many large servers are going to be supporting Folia and Paper has been spending increasing time on it. Hopefully this can at least be a starting point - I am happy to help anyway I can. |
| int result = 1; | ||
| result = prime * result + ((receiver == null) ? 0 : receiver.hashCode()); | ||
| result = prime * result + ((sender == null) ? 0 : sender.hashCode()); | ||
| result = prime * result + taskid; |
There was a problem hiding this comment.
Perhaps replace this with the hashcode of the task instead of entirely removing it from hashCode()?
There was a problem hiding this comment.
It looks like you've since added some sort of timestamp here. Still needed?
| if (!InteractiveChat.ecoSetLoreOnMainThread || plugin.getScheduler().isGlobalTickThread()) { | ||
| return CompletableFuture.completedFuture(setEcoLore0(itemStack.clone(), player)); | ||
| } else { | ||
| return Bukkit.getScheduler().callSyncMethod(InteractiveChat.plugin, () -> setEcoLore0(itemStack.clone(), player)); |
There was a problem hiding this comment.
I was hoping perhaps FoliaLib can add build-in support for returning CompletableFuture<T> for result instead of just CompletableFuture<Void> for completion. It would save a lot of boiler plate code from users like us. :)
| InteractiveChat.placeholderCooldownManager.reloadPlaceholders(); | ||
| PlayerUtils.resetAllPermissionCache(); | ||
| Bukkit.getScheduler().runTaskAsynchronously(InteractiveChat.plugin, () -> InteractiveChat.playerDataManager.reload()); | ||
| InteractiveChat.plugin.getScheduler().runAsync((task) -> InteractiveChat.playerDataManager.reload()); |
There was a problem hiding this comment.
For consistency, I think the rest of my lambdas doesn't use () to surround a single argument.
So task -> instead of (task) ->
It would be nice if you can refractor it too, if not I guess I can do it afterwards.
| if (sender instanceof Player) { | ||
| String message = String.join(" ", Arrays.copyOfRange(args, 1, args.length)); | ||
| Bukkit.getScheduler().runTask(InteractiveChat.plugin, () -> { | ||
| InteractiveChat.plugin.getScheduler().runNextTick((task) -> { |
There was a problem hiding this comment.
I obviously haven't done any research yet, but are you sure that player.chat(message); is called on the global scheduler instead of the player region scheduler? (Just a question, not saying you are wrong)
There was a problem hiding this comment.
This one I'm actually not sure of, good question. Any advice on how to test?
|
Actually, with #256, I plan to merge that one first and then perhaps get folia support to work after those changes. Therefore it might be best to not make the changes I suggested in this PR first. |
|
Hi, I'm glad this is getting some attention again. I've been using this on my server for months with no issues! I'll see if I can address your feedback soon. |
fix: clear folialib 0 tick repeatable warnings
Looks like you've merged #256! I've pulled your changes in and have addressed some of your comments. |
|
Also a note that I have a PR open for the DiscordSRV as well LOOHP/InteractiveChat-DiscordSRV-Addon#69 |
2 participants
This change introduces a library called FoliaLib that allows this plugin to run on both Folia and Spigot/Paper. This library includes methods to replace scheduling calls.
Most of the changes in this PR are agnostic inline scheduler call replacements. However, for uses of BukkitRunnable a more intensive refactor was required as there is not an equivalent in Folia/FoliaLib.