Support for Bukkit#createCommandSender

[learliet]

CommandAPI version

9.1.0

Minecraft version

1.20.1

Are you shading the CommandAPI?

Yes

What I did

I encountered an issue when trying to use Bukkit#createCommandSender in combination with Bukkit#dispatchCommand.

val myCommandSender = Bukkit.createCommandSender { }
Bukkit.dispatchCommand(myCommandSender, "<anycommandapicommand>")

What actually happened

Executing commands from custom senders generated by Bukkit#createCommandSender results in a NullPointerException (see below) due to incorrect handling by the CommandAPI.

What should have happened

The CommandAPI should support the special CommandSender returned by Bukkit#createCommandSender.

Server logs and CommandAPI config

Cannot invoke "[...].dev.jorel.commandapi.commandsenders.AbstractCommandSender.getSource()" 
because "this.val$sender" is null

Other

Given this method, the CommandAPI assumes that CommandSender will only be the types provided by Spigot. However, on Paper, Bukkit#createCommandSender returns io.papermc.paper.commands.FeedbackForwardingSender, which extends CommandSender.

[JorelAli]

Is this just going to be a case of adding support for io.papermc.paper.commands.FeedbackForwardingSender, or should the CommandAPI support generic CommandSender implementations?

[learliet]

In my personal case, adding support for io.papermc.paper.commands.FeedbackForwardingSender would be just fine. It would be greatly appreciated to implement this support first, regardless of whether generic CommandSender implementations are added later just to get support for custom command senders in place as quickly as possible.

As for generic CommandSender support, I’m not sure whether it is necessary or even appropriate. Then again, being able to register custom CommandSender classes seems like a useful niche feature at first glance.

[willkroboth]

I don't think supporting generic CommandSender implementations is necessary. While implementing the CommandSender class is not supported by Spigot (SPIGOT-7023), you can ignore Spigot and create your own command senders anyway. In my experience, custom command senders do seem to work within the Bukkit CommandMap system. However, the bridge from Bukkit to Brigadier is incompatible with custom command senders. This is related to #477.

VanillaCommandWrapper#getListener is responsible for turning the Bukkit CommandSender into a Vanilla CommandListenerWrapper. This is the code that does that:

public static CommandListenerWrapper getListener(CommandSender sender) {
    if (sender instanceof Entity) {
        if (sender instanceof CommandMinecart) {
            return ((EntityMinecartCommandBlock) ((CraftMinecartCommand) sender).getHandle()).getCommandBlock().createCommandSourceStack();
        }

        return ((CraftEntity) sender).getHandle().createCommandSourceStack();
    }
    if (sender instanceof BlockCommandSender) {
        return ((CraftBlockCommandSender) sender).getWrapper();
    }
    if (sender instanceof RemoteConsoleCommandSender) {
        return ((CraftRemoteConsoleCommandSender) sender).getListener().createCommandSourceStack();
    }
    if (sender instanceof ConsoleCommandSender) {
        return ((CraftServer) sender.getServer()).getServer().createCommandSourceStack();
    }
    if (sender instanceof ProxiedCommandSender) {
        return ((ProxiedNativeCommandSender) sender).getHandle();
    }

    throw new IllegalArgumentException("Cannot make " + sender + " a vanilla command listener");
}

This method only succeeds if the CommandSender is a block, console, entity, or proxy. These checks are actually much stricter, as the sender is usually cast to a CraftBukkit class, throwing a ClassCastException if the sender simply implements the Bukkit-API interfaces. You can still create custom command senders if you work with these restrictions, but I don't recommend that. Regardless, in order for a CommandSender to run Vanilla commands, it has to be one of these specific classes. So, it should be safe for the CommandAPI to assume the CommandSender objects passed into CommandAPIBukkit#wrapCommandSender will only be those that are already being checked for.

The issue here only occurs because Paper added special logic to make their FeedbackForwardingSender work with VanillaCommandWrapper#getListener:

diff --git a/src/main/java/org/bukkit/craftbukkit/command/VanillaCommandWrapper.java b/src/main/java/org/bukkit/craftbukkit/command/VanillaCommandWrapper.java
index 45e85f252acd72800956cc2a120564256d2de797..c3742b6c4abea173b38307048091fc56bd5051d7 100644
--- a/src/main/java/org/bukkit/craftbukkit/command/VanillaCommandWrapper.java
+++ b/src/main/java/org/bukkit/craftbukkit/command/VanillaCommandWrapper.java
@@ -83,6 +83,11 @@ public final class VanillaCommandWrapper extends BukkitCommand {
         if (sender instanceof ProxiedCommandSender) {
             return ((ProxiedNativeCommandSender) sender).getHandle();
         }
+        // Paper start
+        if (sender instanceof io.papermc.paper.commands.FeedbackForwardingSender feedback) {
+            return feedback.asVanilla();
+        }
+        // Paper end
 
         throw new IllegalArgumentException("Cannot make " + sender + " a vanilla command listener");
     }

So yeah, this should just be a case of adding support for FeedbackForwardingSender when running on Paper. Since the CommandAPI generates Vanilla commands, any other custom CommandSender, while technically possible, can't run CommandAPI commands anyway unless they implement specific classes we handle already.

[JorelAli]

@willkroboth Thanks for the insight. Let's go ahead with adding support for FeedbackForwardingSender when running on Paper. Let's see if we can push this out for our next release (9.2.0).

[JorelAli]

Implemented for 9.2.0's release. I haven't been able to write an automatic test for this, however was able to verify it works with a manual test:

• Compile a plugin with these commands:

  new CommandAPICommand("sayhi")
      .executes((sender, args) -> {
          Bukkit.broadcastMessage("hi");
      })
      .register();
  
  new CommandAPICommand("mycmd")
      .executes((sender, args) -> {
          CommandSender myCommandSender = Bukkit.createCommandSender(a -> {});
          Bukkit.dispatchCommand(myCommandSender, "sayhi");
      })
      .register();

• Run sayhi from the console (verify this prints hi in the console)

• Run mycmd from the console (verify that this now no longer crashes)

[JorelAli]

Implemented in 9.2.0.