-
Notifications
You must be signed in to change notification settings - Fork 302
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
Defer World modification to when the worlds are not being ticked on Paper #2802
base: main
Are you sure you want to change the base?
Conversation
This PR no longer depends on PaperMC/Paper#8316 being accepted. Instead, the private boolean safeToAddOrRemoveWorld(){
Server server = Bukkit.getServer();
Logging.finest("Using reflection to test for Paper build after PR #7653");
try {
// basically doing ((CraftServer) Bukkit.getServer()).getServer().isIteratingOverLevels;
Method getConsole = server.getClass().getMethod("getServer");
Object console = getConsole.invoke(server);
Field isTickingWorlds = console.getClass().getField("isIteratingOverLevels");
boolean isTicking = isTickingWorlds.getBoolean(console);
Logging.finest("Paper fix active");
return !isTicking;
} catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) {
Logging.finest("%sUnexpected exception: %s", ChatColor.RED, e.getMessage());
Logging.finest("Assuming Paper fix is inactive");
// If the Paper fix actually is active it should become obvious when Paper complains
// about a world being loaded/unloaded while being ticked
// If that happens, this method needs to be fixed
return true;
} catch (NoSuchFieldException ignored) {
// Expected to fail when field isIteratingOverLevels doesn't exist
// Therefore, Paper fixes aren't active, so it is always considered safe to proceed
Logging.finest("Paper fix inactive");
return true;
}
} Since we're no longer waiting on Paper, this PR is ready to be reviewed and merged. |
Paper has aligned with Spigot and currently allows worlds to be loaded/unloaded while being ticked. While this technically makes this PR unnecessary, they have asked to consider deferring world load/unload to when the worlds are not being ticked, and this PR still does that. |
src/main/java/com/onarandombox/MultiverseCore/utils/WorldManager.java
Outdated
Show resolved
Hide resolved
Addresses Multiverse#2560 Commands that load or unload worlds trigger an IllegalStateException if they are run via a command block while the worlds are being ticked. Using a BukkitRunnable, the operation can be delayed until the next tick at a time when the worlds are not being ticked. MVWorldManager#addOrRemoveWorldSafely performs this logic, either running the operation now or delaying it. 8 commands were modified to use MVWorldManager#addOrRemoveWorldSafely, which I think are all the relevant commands. Unfortunately I haven't found a way to tell when the worlds are being ticked on a Spigot server. There probably won't be any way to do this until [SPIGOT-7089](https://hub.spigotmc.org/jira/browse/SPIGOT-7089) resolves
…nload worlds Note: this code was designed under the assumption that PaperMC/Paper#8316 will be accepted, so it won't work unless the commit from that PR is included in the Paper build.
(PaperMC/Paper#8316 was accepted, so this works now) Also some more logging messages
PaperMC/Paper/issues/8300 was resolved as: "plugins have to deal with this difference between Spigot and Paper," so this PR is dealing with the fact that Spigot allows worlds to be loaded/unloaded while the worlds are being ticked, while Paper will throw an IllegalStateException. This PR would resolve #2560.
First, this PR adds the method
MVWorldManager#addOrRemoveWorldSafely
. The implementation for this method inMVWorldManager
looks like this:If it is safe to add or remove a world when this method is called, it will run the requested worldModification immediately. If it is not safe (because we're on a Paper server and the worlds are being ticked), it will schedule the worldModification to happen later using a BukkitRunnable. All the commands (8 by my count) that may load or unload worlds were changed to use this method to make sure their task always happens when it is safe.
To tell if it is currently safe to add or remove a world, this method is used:
If the method can't find Paper fixes, it will always return true because Spigot always considers it safe to load or unload a world. If Paper is present, the method checks the value of
Bukkit.isTickingWorlds()
to know when it is safe.This PR is currently a draft because I am waiting for PaperMC/Paper#8316 to be accepted, which will add the method
Bukkit.isTickingWorlds()
to the Paper API. I'm submitting this pull request now for feedback on the way I solved the issue.These changes were tested on Spigot 1.19.2 and Paper 1.19.2 (with the changes added by PaperMC/Paper#8316), and no issues were noticed.