API breakage: Player#loadData() doesn't load player's world info

[metabrixkt]

Expected behavior

org.bukkit.entity.Player#loadData() loads the current location, including world info, like it does on the upstream.

Observed/Actual behavior

org.bukkit.entity.Player#loadData() loads the current location without world info, unlike it does on the upstream.

Steps/models to reproduce

Player#loadData() results in Entity#load(CompoundTag) call on that player's ServerPlayer instance, which loads the world info on Paper until 1.20.2 build 272 and on Spigot.

Paper moves the world info loading from Entity#load(CompoundTag) to PlayerList#placeNewPlayer(Connection, ServerPlayer, CommonListenerCookie) since 1.20.2 build 272: https://github.com/PaperMC/Paper/blob/d348cb88a9fe8d19e46102c8b9febe18f746d46b/patches/server/0343-Move-player-to-spawn-point-if-spawn-in-unloaded-worl.patch (the patch was initially included in 1.15.2 build 202 but did not contain the breaking change initially), so Entity#load(CompoundTag) and, therefore, Player#loadData() in the API no longer load information about the player's world, even though the javadoc states that it should (since API Location includes World) and it does on the upstream.

Plugin and Datapack List

No datapacks. Investigated and identified the issue by comparing relevant Paper and Spigot code. Initially discovered as Jannyboy11/InvSee-plus-plus#105

Paper version

> version
[01:20:05 INFO]: Checking version, please wait...
[01:20:05 INFO]: This server is running Paper version 1.21.1-131-ver/1.21.1@84281ce (2024-10-31T17:43:44Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are running the latest version

Based on the relevant patch, this should still be an issue on Paper 1.21.3 build 11 (latest 1.21.3 build at the time of writing)

Other

No response

[electronicboy]

This generally looks like the documentation fell behind mojangs tweaks and only worked due to CB specific hacks; I'm not really sure if it's viable to properly restore this, I guess maybe we could try to rehack the behavior into the load method, but, this is all around bleh

[metabrixkt]

the documentation fell behind mojangs tweaks

@electronicboy, worlds don't have UUIDs on vanilla and are identified by dimension keys, worlds UUIDs are introduced by Spigot (or Bukkit? doesn't matter). So, since Player#loadData() loads world info on Spigot, it should behave the same way on Paper.

I mean, Spigot has indeed brought a lot of weird internal changes to the game, but I don't think this is one of them, at least in the context of Player#loadData(). The API behavior shouldn't be changed in this way, because, well, that's an API, and Spigot plugins using it don't expect forks to change it.

[electronicboy]

Except this is not purely API behavior, This is mojang code which is being called into, in which spigot introduced several bugs by the change here, we are not reverting that change, this will need to be solved in a different matter, however, I'm not sure if there is enough will to deal with that

[Jikoo]

Related to #9928 (comment)
Tl;dr: Paper removed some Craftbukkit mimicry of vanilla spawn logic that was a source of inconsistencies for years.

Personally, Player#loadData is wildly unreliable anyway as it doesn't load a bunch of player data. I haven't looked at what InvSee++ does (and I won't, either, sorry - don't want to be in a situation where I might copy an idea by mistake), but I would suspect that they have to have some NMS internals. Adding a Paper check and parsing the world myself was my solution.