Jannyboy11/InvSee-plus-plus

Target player's world switches to overworld when they log off in newly generated chunks in Nether.

Jannyboy11 opened this issue · 4 comments

According to discord user ltrhrd:

If a player logs off in nether in newly generated chunks and you look through their inventory or/and echest, their world seems to switch to overworld. (link)

I am having trouble reproducing this issue locally..
Asked the reporter to help me investigate further.

+1! Just encountered this issue with two players on Purpur 1.20.4. Currently investigating further if it also affects other dimensions, not just the nether

@Jannyboy11 here's what we've found:

  1. Chunks don't need to be newly generated.
  2. From what we can see using our custom /whois command, the world change happens when the InvSee menu closes, regardless of whether any changes were made to the inventory or not.
  3. This also affects other dimensions and custom worlds (even custom worlds with overworld dimension type), looks like it's not the dimension that is changed to overworld, the world info is lost completely and I assume the server just selects the first world (like Bukkit.getWorlds()[0]).
  4. The gamemode info is lost too, there might be more info that gets lost, but we didn't notice anything else.
  5. No errors or warnings are logged when this happens.

Also this was verified on Purpur 1.20.6 ^
Contact me on Discord @metabrix if you have any questions

There is indeed some code that creates a new player entity where its location is in world server.getWorlds().get(0). This value should however be overwritten by the call to Player#loadData().

The current logic is as follows:

  1. create new blank Player object, p.
  2. load the existing save data onto it (from existing save files): p.loadData()
  3. load the inventory contents from the spectator inventory view onto it: MainSpectatorInventory/EnderSpectatorInventory#setContents
  4. save the player object p back to disk: p.saveData()

It seems that somehow, the p.loadData() implementation failed to load the correct data (either it does nothing, or it loads the wrong data).
One other possible explanation is the original player data from p.loadData() is outdated when the p.saveData() is called. I don't see how this could happen because all of this should happen single-threaded.

Perhaps the algorithm can be made more robust using some optimistic locking or CAS operations. I am open to other suggestions too.
You can find the code here: https://github.com/Jannyboy11/InvSee-plus-plus/blob/d880d7a49f6dd4e07c62c5b084743081d3d24edb/InvSee%2B%2B_Platforms/Impl_1_21_R1/src/main/java/com/janboerman/invsee/spigot/impl_1_21_R1/InvseeImpl.java#L232C1-L259C6

Edit: I should mention that this saving logic is called every time a spectator inventory closes.