Opening an offline player's inventory will reset the player's world when using Paper

[KioProject123]

Steps/models to reproduce 1、Player A goes to the Nether 2、Player A leaves the server 3、Player B uses the command /openinv A (Requires op permissions) 4、Player B closes the open GUI 5、Player A enters the server, will spawn in the overworld instead of the Nether

I also reported this issue to paper: https://github.com/PaperMC/Paper/issues/9928

[Jikoo]

Looks like Paper removed loading world from loading entity data, so I guess we get to load that separately now. Since Paper now always has a null world, https://github.com/Jikoo/OpenInv/blob/faf1d387e9fca963c4b36cb71b8ee9db2600719b/internal/v1_20_R2/src/main/java/com/lishid/openinv/internal/v1_20_R2/PlayerDataManager.java#L139-L142 always resets them to spawn.

[Jikoo]

I've pushed c9a2ae44e2c5dd6947467023fca87581561a4f16, please let me know if the resulting build fixes the issue for you. A jar can be obtained by downloading and unzipping the "dist" artifact when https://github.com/Jikoo/OpenInv/actions/runs/6832530271 completes.

[KioProject123]

I've pushed c9a2ae4, please let me know if the resulting build fixes the issue for you. A jar can be obtained by downloading and unzipping the "dist" artifact when https://github.com/Jikoo/OpenInv/actions/runs/6832530271 completes.

It looks like this issue still exists

[KioProject123]

After testing, this issue will also change these NBTs of players:

bukkit.lastPlayed (Also happens with Spigot)
Paper.LastLogin (reset to 0)
Paper.LastSeen

Although it won't cause obvious problems, it will mess up some statistics plugins.

[Jikoo]

After testing, this issue will also change these NBTs of players:

bukkit.lastPlayed (Also happens with Spigot)
Paper.LastLogin (reset to 0)
Paper.LastSeen

Although it won't cause obvious problems, it will mess up some statistics plugins.

That data is read and written already.

Read is here: https://github.com/Jikoo/OpenInv/blob/faf1d387e9fca963c4b36cb71b8ee9db2600719b/internal/v1_20_R2/src/main/java/com/lishid/openinv/internal/v1_20_R2/PlayerDataManager.java#L137 https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/nms-patches/net/minecraft/server/level/EntityPlayer.patch#142 https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java#1595-1614

Write is here: https://github.com/Jikoo/OpenInv/blob/faf1d387e9fca963c4b36cb71b8ee9db2600719b/internal/v1_20_R2/src/main/java/com/lishid/openinv/internal/v1_20_R2/OpenPlayer.java#L52 https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java#1616-1631

If you have an instance of this data being lost, please let me know exact server and OpenInv versions so I can replicate.

[KioProject123]

After testing, this issue will also change these NBTs of players:

bukkit.lastPlayed (Also happens with Spigot)
Paper.LastLogin (reset to 0)
Paper.LastSeen

Although it won't cause obvious problems, it will mess up some statistics plugins.

That data is read and written already.

Read is here:

https://github.com/Jikoo/OpenInv/blob/faf1d387e9fca963c4b36cb71b8ee9db2600719b/internal/v1_20_R2/src/main/java/com/lishid/openinv/internal/v1_20_R2/PlayerDataManager.java#L137

https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/nms-patches/net/minecraft/server/level/EntityPlayer.patch#142 https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java#1595-1614 Write is here:

https://github.com/Jikoo/OpenInv/blob/faf1d387e9fca963c4b36cb71b8ee9db2600719b/internal/v1_20_R2/src/main/java/com/lishid/openinv/internal/v1_20_R2/OpenPlayer.java#L52

https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java#1616-1631 If you have an instance of this data being lost, please let me know exact server and OpenInv versions so I can replicate.

I'm using Paper1.20.2 and OpenInv4.4.0

The data loss may be related to the way they are saved, they save the current timestamp. Maybe you need to record the original timestamp when reading the data and re-overwrite it after saving.

https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java#1629 https://github.com/PaperMC/Paper/blob/4675152f4908431e0f944a7bf9fa3b2181a2dfd6/patches/server/0290-Add-APIs-to-replace-OfflinePlayer-getLastPlayed.patch#L159-L160

In addition, I also tried this file you provided https://github.com/Jikoo/OpenInv/actions/runs/6832530271, the issue of the player world being reset to the overworld still exists

[Jikoo]

The data loss may be related to the way they are saved, they save the current timestamp. Maybe you need to record the original timestamp when reading the data and re-overwrite it after saving.

https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java#1629 https://github.com/PaperMC/Paper/blob/4675152f4908431e0f944a7bf9fa3b2181a2dfd6/patches/server/0290-Add-APIs-to-replace-OfflinePlayer-getLastPlayed.patch#L159-L160

In addition, I also tried this file you provided https://github.com/Jikoo/OpenInv/actions/runs/6832530271, the issue of the player world being reset to the overworld still exists

That would only result in the last played timestamp being updated to the current time, not a reset to 0. You're right that adding it to the special handling with vehicles would be good. Either way, thanks, got a starting point now.

[Jikoo]

Okay, I'm not able to replicate the issue you're having with Bukkit/Paper special data, those exist as expected. They do update on save, which should be rectified, but that's a separate issue. They are properly loaded and saved, just improperly updated.

[Jikoo]

And found time to look into why my fix wasn't working - turns out that of course it wouldn't, because Paper moved the Spigot world parsing into the PlayerList in the same commit. Basically just have to do a full world parse ourselves if it's null, which is exactly what I was trying to avoid for pretty much the same reason Paper is trying to move the parsing to a more maintainable location.

[Jikoo]

Pushed f9c7ed72f2329c9cad0c5d7b15016e5007af32d1 which will resolve this. Available here, including update removal for session timestamps: https://github.com/Jikoo/OpenInv/actions/runs/6839202210 Will probably backport as necessary and merge tomorrow. May end up dropping the commit for undoing session updates to deal with separately because I want to verify that the potential issue with not re-mounting vehicles exists in previous versions.