AuthMeReloaded
AuthMeReloaded copied to clipboard
Folia support
Folia doesn't allow to use the Bukkit scheduler, it has four different scheduler types, tasks must be scheduled on the correct ones.
I created two implementations of BukkitService, since Bukkit/Spigot/Paper API and Folia API are mutually incompatible.
I marked the legacy scheduling methods as @Deprecated to make the tests compile, but they must be removed.
I made fallback implementations for all the new Folia scheduling methods in the legacy Bukkit/Spigot/Paper BukkitService.
I still haven't implemented the unit tests, I don't have time to re-implement most of them.
Related issue: https://github.com/AuthMe/AuthMeReloaded/issues/2702
teleportAsync is introduced by Paper. By adding the Folia API, it hides the unavailability of such methods from like Spigot. Maven multi modules could fix that, but could be overkill for this project. However, then requires such manual checks by the programmer and not be automatic by the compiler.
The API is commented with "// Paper start", "// Paper end" around every new API method, and the Spigot API javadocs are easily navigable, probably it's easier to manually check than overengineering everything.
teleportAsync is introduced by Paper. By adding the Folia API, it hides the unavailability of such methods from like Spigot. Maven multi modules could fix that, but could be overkill for this project. However, then requires such manual checks by the programmer and not be automatic by the compiler.
The API is commented with "// Paper start", "// Paper end" around every new API method, and the Spigot API javadocs are easily navigable, probably it's easier to manually check than overengineering everything.
Not everybody is looking at the source code. You are more likely look at the auto-complete suggestions of your favorite IDE. With the Maven modules, at least it could be verified by the compiler.
Existing thread-safety assumptions Similar what about existing thread-safe methods. AFAIK Player.sendMessage was declared as thread-safe. Do we now require that this method should accessed on the EntityScheduler? Taking our MessageTask should then no longer run asynchronous, but on this scheduler.
Messages can be executed outside of any entity scheduler, since they do not interact with the world
Furthermore, it could be useful to add verification methods before we access anything player API (implemeneted as a Service?). So that we don't miss anything by accident. Something similar like Spigot already has for some API methods.
sendMessage(...) { // thread-safe API access if (currentThread == EntityScheduler || currentThread == player.getRegionThread() }It should be noted, that we still need to verify with the Spigot platform for potential deadlocks.
Folia has already implemented this checks for us, so we could simply add that methods to the BukkitService
I made the verification methods available. By the way, keep in mind that they are not so useful, because Folia already throws exceptions if you call any method outside of its scheduler.
Also, the old .scheduleSyncTaskFromOptionallyAsyncTask(...), replaced by their new counterparts, can overcome this limitation as you did in the past.
Messages can be executed outside of any entity scheduler, since they do not interact with the world
Do you know if there is some kind of documentation about what is safe to be executed asynchronous? Spigot is such a huge project, it is hard to check if not a specific method accesses an internal component that isn't thread-safe. For example: The conversation and metadata (Player.setMetadata()) API. Either this has locking mechanism or it can only run on the entity scheduler.
I made the verification methods available. By the way, keep in mind that they are not so useful, because Folia already throws exceptions if you call any method outside of its scheduler. Also, the old .scheduleSyncTaskFromOptionallyAsyncTask(...), replaced by their new counterparts, can overcome this limitation as you did in the past.
So something like this already exists? That would be perfect, because such thing is better suited to be in Folia anyway.
PlayerService {
public void setHealth(...) {
if (!currentThread == entityScheduler?/region thread?)
throw new IllegalStateException("Wrong thread");
}
}
The checks you implemented could be unnecessary if Folia is aware of which thread is currently itself is running on. In other words your checks (if globaltick then run.run()) could already be in Folia directly.
I'm sorry for all these questions, but I'm not that familiar with the project. Maybe it's also because Folia is still new and many things are planned.