Type of bug

Other unexpected behaviour

/ess dump all output

https://essentialsx.net/dump.html?id=1756ddc95189431880f92de1ce418b0b

Error log (if applicable)

No response

Bug description

A warp to the Nether with Y > 128 takes the player to the wrong coordinate if he's in Survival mode.

Steps to reproduce

Define a warp in the Nether to 3000, 243, 3000
Doing /warp in Survival mode takes you to 3000, 128, 3000
Doing /warp in Creative mode takes you to 3000, 243, 3000

Expected behaviour

Should always warp to 3000, 243, 3000

Actual behaviour

Doing /warp in Survival mode takes you to 3000, 128, 3000
Doing /warp in Creative mode takes you to 3000, 243, 3000

Discovered the same thing applies to /tppos 3000 243 3000

This behavior is different from 1.17.1

Seems to be related to force-disable-teleport-safety; needs to be set to true. Did this start applying to warps recently?

Yes that is the intended behavior of teleportation safety.

To my knowledge, this was fixed here: 3f0cec6 (#4640)

Do the builds here fix the issue?

From that commit:

// Allow spawning at the top of the world, but not above the nether roof

I guess the question becomes why is Nether Y > 128 considered unsafe? Whatever the coordinates are, shouldn't that location actually be examined for safety and the teleport be allowed or not as appropriate?

I think there was an assumption made that people generally don't want players to teleport above y=128 which seemed reasonable but it is definitely a breaking change and technically those locations are all valid and safe.

Might be worth reverting that assumption and adding a configuration option specifically for the nether roof.

I like the teleport safety feature. e.g., if someone's /home turns into a lava pool, he won't die, not to mention the destinations of /tpr. But I don't see why specific coordinates are deemed unsafe. No matter the coordinates, can't you examine the actual location, even above or below the world height?

I guess there are other use cases, such as normal players being allowed to use /tppos, but the server owner doesn't want them above the Nether roof. In any event, if you could allow teleports above Y 128 somehow, it would be appreciated.

I dunno. Making a breaking change that requires people to force-disable-teleport-safety, which then allows players to accidentally /tpr into lava seems like a bad idea.

Is it even possible to accidentally tp above the Nether roof? Anyone who ends up there wanted to go there on purpose. So why stop them? In contrast, it's doubtful anyone wants to purposely tpr into lava.

Vanilla has never considered y > 128 in the Nether to be a safe teleport destination in the past, with chorus fruits capping safe teleports at y=127 since Minecraft 1.9 and Nether portals being capped for years before that, and EssentialsX teleporting players above the nether roof is a long-standing known issue.

3f0cec6 (#4640) changes safe teleports to use the world's logical height, which is a dimension type parameter that represents the worldgen height and can be changed via datapacks as of 1.16. Vanilla uses this when finding safe teleportation locations, and most servers either don't allow building above the Nether roof or use datapack-driven custom worldgen that overrides the logical height, so this change both fixes unintentional roof teleports and aligns EssentialsX's behaviour more closely with vanilla.

There are two options I can see EssentialsX taking:

• Keep using the logical height:
  • This aligns with how Mojang already finds safe teleport locations on 1.16+
  • We currently only support this on 1.17.1, and would need to add a new NMS provider to use this correctly on 1.16.5
  • We could supply datapacks to modify the logical height for servers who desperately want to allow safe teleports above the Nether roof
• Add a config option that lets server owners override the world height on a per-world or per-dimension type basis:
  • We would need to either make the safe teleport code incredibly ugly, or completely refactor how we do safe teleports to supply settings to it
  • This is already configurable through datapacks on 1.16+ and so only serves a purpose on 1.15.x and below, which is outside of our target version range

I would much prefer we use the vanilla logical height than add special cases for non-standard setups, as this avoids confusion and allows us to properly support datapack-driven worlds.