Essentials icon indicating copy to clipboard operation
Essentials copied to clipboard

VanishStatusChangeEvent is passed invalid parameters

Open ghost opened this issue 6 years ago • 3 comments

Information

Full output of /ess version:

[14:31:50] [Server thread/INFO]: Server version: 1.14.2-R0.1-SNAPSHOT git-Spigot-df0eb25-f2757f9 (MC: 1.14.2)
[14:31:50] [Server thread/INFO]: EssentialsX version: 2.16.1.125
[14:31:50] [Server thread/INFO]: PermissionsEx version: 1.23.4
[14:31:50] [Server thread/INFO]: Vault version: 1.7.1-b91
[14:31:50] [Server thread/INFO]: You are running unsupported plugins!
[14:31:50] [Server thread/INFO]: You are running an unsupported server version!

Details

Description
I've made custom vanishing command (which just sends 'player left' message on vanish), and I've encountered the following problem: when issuing /v <player> in console, VanishStatusChangeEvent#getAffected is null, while VanishStatusChangeEvent#getController is the <player>.

Steps to reproduce
Here's the code I use in my command which is using EssentialsX API:

@EventHandler
    public void onPlayerVanishStatusChange(VanishStatusChangeEvent event) {
        boolean vanished = event.getValue();
        Player player = event.getAffected().getBase();

        PlayerFakeLogonEvent playerFakeLogonEvent;
        if (vanished) {
            playerFakeLogonEvent = new PlayerFakeLeaveEvent(player);
        } else {
            playerFakeLogonEvent = new PlayerFakeJoinEvent(player);
        }

        // PlayerFakeLogonEvent is my custom event for showing join/quit messages upon vanish/unvanish
        Bukkit.getPluginManager().callEvent(playerFakeLogonEvent);
    }

Expected behavior /v <player> triggered in the game (not in console) makes VanishStatusChangeEvent#getAffected return target player, and makes VanishStatusChangeEvent#getController return command issuer.

ghost avatar Jun 16 '19 10:06 ghost

Confirmed - arguments are passed in the wrong order here: https://github.com/EssentialsX/Essentials/blob/9b77c0802b53b5ed61a7169c245cd88ca09a63bc/Essentials/src/com/earth2me/essentials/commands/Commandvanish.java#L34

When the correct order is: https://github.com/EssentialsX/Essentials/blob/9b77c0802b53b5ed61a7169c245cd88ca09a63bc/Essentials/src/net/ess3/api/events/VanishStatusChangeEvent.java#L9

~~Feel free to submit a PR if you'd like.~~

This will be fixed during a major update, as it would break existing implementations that listen for this event.

triagonal avatar Jun 16 '19 11:06 triagonal

In my opinion it would still be good to add an event with the correct signature, and deprecate the old one. That way it can slowly be phased out, and by the time a big milestone is reached most plugins are hopefully using/updated to use the updated event.

pop4959 avatar Oct 27 '19 01:10 pop4959