EssentialsX

EssentialsX

2M Downloads

force-disable-teleport-safety Doesn't "disable teleport safety checks"

ProbablySomeone opened this issue ยท 3 comments

commented

Information

Version:

Server version: 1.7.10-R0.1-SNAPSHOT 1.7.10-1614.57 (MC: 1.7.10)
EssX version: 2.15.0.59
PermsEx version: 1.22.10
Vault version: 1.4.1-b436

The server is modded with Thermos by the way, although that's really besides the point.

EssentialsX config:

This doesn't really have an affect, but here are the important parts anyway...

# If the teleport destination is unsafe, should players be teleported to the nearest safe location?
# If this is set to true, Essentials will attempt to teleport players close to the intended destination.
# If this is set to false, attempted teleports to unsafe locations will be cancelled with a warning.
teleport-safety: true

# This forcefully disables teleport safety checks without a warning if attempting to teleport to unsafe locations.
# teleport-safety and this option need to be set to true to force teleportation to dangerous locations.
force-disable-teleport-safety: true

Details

Description
About 90% of the time any player in survival runs a teleport command handled by essentials, essentials will throw an exception unless the player is in Creative. This is because there is no spectator mode in 1.7.10, and line 227 of com/earth2me/essentials/utils/LocationUtil.java fails to short circuit when players are not in creative, causing GameMode.SPECTATOR to throw a NoSuchFieldException. This may not matter very much if support for 1.7.10 is not a priority for this plugin.

The real issue here is that setting force-disable-teleport-safety to true in the configuration should disable this. The configuration states that force-disable-teleport-safety:

forcefully disables teleport safety checks without a warni...

However, in the code...

        if (LocationUtil.isBlockUnsafeForUser(teleportee, loc.getWorld(), loc.getBlockX(), loc.getBlockY(), loc.getBlockZ())) {
            if (ess.getSettings().isTeleportSafetyEnabled()) {
                if (ess.getSettings().isForceDisableTeleportSafety()) {
                    teleportee.getBase().teleport(loc, cause);
                } else {
                    teleportee.getBase().teleport(LocationUtil.getSafeDestination(ess, teleportee, loc), cause);
                }
            } else {
                throw new Exception(tl("unsafeTeleportDestination", loc.getWorld().getName(), loc.getBlockX(), loc.getBlockY(), loc.getBlockZ()));
            }
        } else ...

teleport safety checks are made regardless of whether or not checking is "disabled" via the config, at the beginning of the very outer if block (LocationUtil.isBlockUnsafeForUser(...)).

I'd rewrite the code to match the config better, but I really don't know exactly what the two teleportation safety config options are supposed to do. The comments in the config aren't clear enough for me to convert to code. :(

Steps to reproduce
In my modded server set up, this issue occurs when teleporting in survival almost all the time. Essentials says "Teleporting..." in gold then the error comes up in the console and nothing else happens on the player's side.

To reproduce the error of teleport safety being checked even when the configuration "disables" it, just execute a teleportation in a way that an exception is thrown in by the checking algorithm (i.e. in a way that LocationUtil.isBlockUnsafeForUser(...) throws an exception).

Expected behavior
The player actually gets teleported.

The gist, (as in the important part), of the stacktrace is here below. I can upload a github gist of the full trace, if it's needed for whatever reason...

java.lang.NoSuchFieldError: SPECTATOR
        at com.earth2me.essentials.utils.LocationUtil.isBlockUnsafeForUser(LocationUtil.java:227) ~[EssentialsX-2.15.0.59.jar:?]
        at com.earth2me.essentials.Teleport.now(Teleport.java:122) ~[EssentialsX-2.15.0.59.jar:?]
        at com.earth2me.essentials.Teleport.teleport(Teleport.java:198) ~[EssentialsX-2.15.0.59.jar:?]
        at com.earth2me.essentials.Teleport.teleport(Teleport.java:162) ~[EssentialsX-2.15.0.59.jar:?]
        at com.earth2me.essentials.commands.Commandtpaccept.run(Commandtpaccept.java:62) ~[EssentialsX-2.15.0.59.jar:?]
commented
Server version: 1.7.10-R0.1-SNAPSHOT 1.7.10-1614.57 (MC: 1.7.10)

[...] This may not matter very much if support for 1.7.10 is not a priority for this plugin.

We don't guarantee or provide and support for 1.7; the README highlights clearly which versions of Minecraft we support:

  • EssentialsX supports Minecraft versions 1.8.8, 1.9.4, 1.10.2, 1.11.2 and 1.12.2. Support for 1.13.2 is coming soon.

This forcefully disables teleport safety checks without a warning [...]

The teleport safety checks are intended to run no matter what.

teleport-safety controls whether or not the user is teleported to what EssentialsX deems a "safe location" if the location they were originally targeting is deemed "unsafe".

If force-disable-teleport-safety is set to true, the teleport safety checks still take place, but the teleport relocation is ignored (ie they go to the unsafe destination). I believe this is the intended behaviour.

commented

@md678685 I am well aware of the lack of support for versions below 1.8.8. The problem was about the fact that the check takes place even though its outcome should be entirely ignored when force-disable-teleport-safety, which you discussed. If you think the current behavior is correct, then who am I to argue? However, I'd like to note that if force-disable-teleport-safety ignores the outcome of the safety check, then running the safety check with force-disable-teleport-safety set to true should be pointless, unless there is an intent to simply crash execution if the check fails. Also, the documentation on those teleportation configuration items were a bit confusing. Your comment to this thread cleared things up quite a lot.

You should consider changing intended behavior of those configuration items to better match their documentation, or possibly vice versa.

commented

We will not be introducing breaking changes to the functionality of the config option.

if (LocationUtil.isBlockUnsafeForUser(teleportee, loc.getWorld(), loc.getBlockX(), loc.getBlockY(), loc.getBlockZ())) {
if (ess.getSettings().isTeleportSafetyEnabled()) {
if (ess.getSettings().isForceDisableTeleportSafety()) {
teleportee.getBase().teleport(loc, cause);
} else {
teleportee.getBase().teleport(LocationUtil.getSafeDestination(ess, teleportee, loc), cause);
}
} else {
throw new Exception(tl("unsafeTeleportDestination", loc.getWorld().getName(), loc.getBlockX(), loc.getBlockY(), loc.getBlockZ()));
}

If teleport-safety is set to false, EssentialsX will refuse to teleport players to unsafe locations and warn the player. However, if force-disable-teleport-safety: true prevented the check from taking place and teleport-safety is set to false, this warning will not happen. Changing this will complicate existing setups and cause undesired effects for server administrators who have configured their server in a specific way.