History (e.g. //undo) sometimes stops working completely when previous worlds are unloaded
Schuwi opened this issue ยท 2 comments
Server Implementation
Paper
Server Version
1.18.2
Describe the bug
When using a plugin to manage multiple worlds (e.g. MultiVerse or BuildSystem) which unloads worlds when not in use, sporadically //undo
stops working.
The circumstances to evoke this bug might seem a bit convoluted (see steps to reproduce) but the bug has occured multiple times in practice.
To Reproduce
- Setup a PaperMC 1.18.2 Server with FAWE and MultiVerse
- Create a new world e.g. "test" with
/mv create test NORMAL -t FLAT
(generation options are not relevant) - Go into the newly generated world (
/mv tp test
) - Do any worldedit operation that is saved in the history (e.g.
//pos1
,//pos2
,//set stone
) - Go back to the default world and unload the test world (
/mv tp world
,/mv unload test
) - The problem now is that FAWE caches the world object with a WeakReference before trying to get the world by name via the Bukkit API so we need to:
6.1. Either force garbage collection a few times for example with VisualVM, clutter the heap by flying around, loading chunks, wait a few minutes and pray to your favorite god(s)
6.2. Or replace this line withWorld tmp = null;
for testing - Do another worldedit operation that impacts history (an exception should be thrown in the console but the player won't notice anything is wrong yet)
- Try to
//undo
this last operation. This will fail with an exception (java.lang.NullPointerException: The world was unloaded and the reference is unavailable
) and the changes won't be undone.
Expected behaviour
No exceptions should be thrown and //undo should undo the latest changes.
Screenshots / Videos
No response
Error log (if applicable)
https://gist.github.com/Schuwi/072b8abd3b4861d21c89165963835c97
Fawe Debugpaste
https://athion.net/ISPaster/paste/view/a26d3e931462486c87a96737b4ade9f5
Fawe Version
FastAsyncWorldEdit version 2.1.1-SNAPSHOT-134;465c81d
Checklist
- I have included a Fawe debugpaste.
- I am using the newest build from https://ci.athion.net/job/FastAsyncWorldEdit/ and the issue still persists.
Anything else?
No response
This currently works as intended, however, is technically a bug. Ultimately the decision needs to be made if we want to attempt to load an unloaded world, or if we want to skip history for this world (could be an issue for "lost" history in that world in the "future"), before implementing much of a fix
Correct me if I'm wrong but as I understand the issue the world would neither have to be loaded nor would history be lost. I presume the history is recorded the moment that an operation is executed. Following the stack trace the only issue seems to be that FAWE wants to save a negativeHistoryIndex
(don't ask me what it's for but it does not depend on the world being accessible) into a file that has the world name in the file name.
So the fix could be as simple as caching the world name or file name somewhere (the world name is actually already cached in BukkitWorld
and returning worldNameRef
in getName()
[and getId
for good measure I suppose] would fix the problem).
Another fix would be to call loadSessionHistoryFromDisk
when the player switches worlds (PlayerChangedWorldEvent
in bukkit). Then there should be no issues with deciding between loading world or lost history now or in the future (in case the current implementation of history saving changes) because history is already taken care of while the world is definitely still loaded (or maybe PlayerTeleportEvent
would guarantee that, not sure).