EssentialsX / Essentials

The modern Essentials suite for Spigot and Paper.
https://essentialsx.net
GNU General Public License v3.0
1.95k stars 980 forks source link

Teleport Invulnerability getting triggered when another plugin modifies PlayerMoveEvent #4553

Open repeater64 opened 3 years ago

repeater64 commented 3 years ago

Type of bug

Compatibility issue, Other unexpected behaviour

/ess dump all output

Not relevant

Error log (if applicable)

No response

Bug description

I have a plugin that occasionally makes a player immobile. The way it does this is by listening to PlayerMoveEvent and forcing the TO location to be the same as the FROM location except for yaw and pitch. It uses the following code:

@EventHandler
    public void onPlayerMove(PlayerMoveEvent e) {
        if (conditions) {
                Location to = e.getFrom();
                to.setPitch(e.getTo().getPitch());
                to.setYaw(e.getTo().getYaw());
                e.setTo(to);
        }
    }

Essentials has the option to make players invulnerable for a few seconds after teleporting with the config option teleport-invulnerability.

For some reason this teleport invulnerability period is being triggered when a player tries to move and is stopped by the code above. I know this because I found out that it was Essentials cancelling the EntityDamageEvent using some hacky reflection code that comphenix published, and when I set the invulnerability period to 0 the issue immediately stopped happening.

Clearly the way that Essentials determines when to start the teleport invulnerability period is kinda messed up allowing it to be falsely triggered by a modification of the PlayerMoveEvent.

Steps to reproduce

Ensure that teleport-invulnerability is set to a few seconds in the essentials config Write a plugin with an event listener using the code above Try and move, you will be unable to move because of the PlayerMoveEvent code Right after attempting to move, you will be invulnerable.

Disable teleport-invulnerability and the issue will vanish.

Expected behaviour

Such modification of the PlayerMoveEvent should not trigger teleport invulnerability.

Actual behaviour

Such modification of the PlayerMoveEvent does trigger teleport invulnerability.

mdcfe commented 2 years ago

This sounds suspiciously like a CraftBukkit issue - I'm not sure why overriding movement in PlayerMoveEvent would be considered a teleport. Further investigation is likely required for this issue.

Baylem commented 2 years ago

This is being addressed by Paper. https://github.com/PaperMC/Paper/issues/8144