CommandAPI icon indicating copy to clipboard operation
CommandAPI copied to clipboard

Players cannot run Entity executors

Open willkroboth opened this issue 1 year ago • 1 comments

CommandAPI version

9.4.2

Minecraft version

1.19.4

Are you shading the CommandAPI?

Yes

What I did

I registered the following command with CommandAPI 9.4.2 and 8.8.0 (whose latest supported version is 1.19.4):

new CommandAPICommand("test")
        .executesEntity((entity, args) -> {
            Bukkit.getServer().broadcastMessage(entity + " ran the command");
        })
        .register();

What actually happened

When using 9.4.2 (plugin jar: 9.4.2.zip), running /test as a player will result in the message This command has no implementations for craftplayer:

9 4 2

What should have happened

When using 8.8.0 (plugin jar: 8.8.0.zip), running /test as a player runs the code defined in executesEntity:

8 8 0

Server logs and CommandAPI config

No response

Other

I believe this change was introduced in 9.0.0 due to #378. That changed this code in CommandAPIExecutor like so:

8.8.0 (focus line 104): https://github.com/JorelAli/CommandAPI/blob/aff3dc62f045fa0377bff311fab2271f066d5fee/commandapi-core/src/main/java/dev/jorel/commandapi/CommandAPIExecutor.java#L98-L120

9.0.0 (focus line 95): https://github.com/JorelAli/CommandAPI/blob/d3f17da0f67ed0de99f7e79f3e85a1e2ce220632/commandapi-core/src/main/java/dev/jorel/commandapi/CommandAPIExecutor.java#L89-L111

The relevant change here is sender instanceof Entity && matches(executors, ExecutorType.ENTITY) to info.senderWrapper() instanceof AbstractEntity && matches(executors, ExecutorType.ENTITY). In 8.8.0, a Player is an instance of Entity, while in 9.0.0, an AbstractPlayer is not an instance of AbstractEntity. Hence, the player is allowed to use the executesEntity executor in 8.8.0, but not 9.0.0 or later.


I noticed this 'issue' when I refactored the executor system in commit https://github.com/JorelAli/CommandAPI/commit/62d5ea51c9caaa48cf39e8331d1d68942e7e0937 on the dev/command-build-rewrite branch. I unknowingly reintroduced the logic from 8.8.0 where the executor is selected using the platform-specific senders.

dev/command-build-rewrite (focus line 32)

https://github.com/JorelAli/CommandAPI/blob/62d5ea51c9caaa48cf39e8331d1d68942e7e0937/commandapi-platforms/commandapi-bukkit/commandapi-bukkit-core/src/main/java/dev/jorel/commandapi/executors/BukkitTypedExecutor.java#L23-L52

Hence, if you execute the same test command using this branch (plugin jar: command-build-rewrite.zip), you get the 8.8.0 behavior where the code in executesEntity is run.

I think the 8.8.0 behavior is correct. It is still possible to run different code if the sender is a player using executesPlayer.

new CommandAPICommand("test")
        .executesPlayer((player, args) -> {
            // Run if player
        })
        .executesEntity((entity, args) -> {
            // Otherwise, run if entity
        })
        .register();

If the 8.8.0 behavior is correct, then this issue is already fixed on dev/command-build-rewrite, so I think we can just wait for that to be merged. If the 9.4.2 behavior is correct, then I'll make sure to take that into account on dev/command-build-rewrite.

willkroboth avatar May 31 '24 01:05 willkroboth

I'd say the 8.8.0 behaviour is correct. What we could've done for 9.0.0 is to take the inheritance into account for the wrapper classes but I'd say waiting for your command build rewrite is fine.

DerEchtePilz avatar May 31 '24 09:05 DerEchtePilz