[SPIGOT-7023] Custom inheritors of CommandSender should be able to run Vanilla commands Created: 18/May/22 Updated: 25/Dec/24 Resolved: 18/May/22 |
|
Status: | Resolved |
Project: | Spigot |
Component/s: | None |
Affects Version/s: | None |
Fix Version/s: | None |
Type: | New Feature | Priority: | Minor |
Reporter: | Will Kroboth | Assignee: | Unassigned |
Resolution: | Invalid | Votes: | 0 |
Labels: | Bukkit, Craftbukkit |
Version: | This server is running Paper version git-Paper-283 (MC: 1.18.2) (Implementing API version 1.18.2-R0.1-SNAPSHOT) (Git: f8e8d6c) |
Guidelines Read: | Yes |
Description |
Currently, when a Vanilla command is attempted to be run using a custom object inheriting from CommandSender, an IllegalArgumentException is thrown since the CommandSender cannot be made into a vanilla command listener. This is intended behavior and stems from the getListener() method of the VanillaCommandWrapper class. public static CommandListenerWrapper getListener(CommandSender sender) { if (sender instanceof Player) { return ((CraftPlayer) sender).getHandle().createCommandSourceStack(); } if (sender instanceof BlockCommandSender) { return ((CraftBlockCommandSender) sender).getWrapper(); } if (sender instanceof CommandMinecart) { return ((EntityMinecartCommandBlock) ((CraftMinecartCommand) sender).getHandle()).getCommandBlock().createCommandSourceStack(); } if (sender instanceof RemoteConsoleCommandSender) { return ((DedicatedServer) MinecraftServer.getServer()).rconConsoleSource.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"); } I suggest that developers should have the ability to easily make their custom CommandSenders run Vanilla commands. To make this possible, a new method should be added to the CommandSender interface, like so: public interface CommandSender ...{ default CommandListenerWrapper getListener(){ throw new IllegalArgumentException("Cannot make " + this + " a vanilla command listener"); } } the getListener() method in VanillaCommandWrapper should be changed to: public static CommandListenerWrapper getListener(CommandSender sender) { sender.getListener(); } and the functionality previously held in the if-block in the old version of getListener should be moved to the corresponding concrete classes, for example, CraftPlayer: public class CraftPlayer ...{ public CommandListenerWrapper getListener(){ return this.getHandle().createCommandSourceStack(); } } Then, developers can allow their own custom CommandSenders to run Vanilla commands by implementing CommandSender#getListener(). If they do not, the default implementation in CommandSender will cause everything to act exactly the same as before and just throw an IllegalArgumentException. This code change also avoids incurring a confusing CastClassException if an object, for example, implements Player but not CraftPlayer or ConsoleCommandSender but not CraftServer, and is passed into getListener(). The custom Player that is not a CraftPlayer can either implement getListener() or throw a much more understandable IllegalArgumentException. This suggestion is related to |
Comments |
Comment by md_5 [ 19/May/22 ] |
When I say NMS I mean anything in the API. Those classes are not in the API. As for this request, it is clearly outside scope per the previous link: "Implementing interfaces. The Bukkit API is designed to only be implemented by server software. Unless a class/interface is obviously designed for extension (eg BukkitRunnable), or explicitly marked as such, it should not be implemented by plugins. Although this can sometimes work, it is not guaranteed to do so and resulting bugs will be disregarded." |
Comment by Will Kroboth [ 19/May/22 ] |
Are these NMS classes? The classes I've mentioned don't seem to belong to the net.minecraft.server namespace (org.bukkit.craftbukkit.v1_18_R2.command.VanillaCommandWrapper, org.bukkit.command.CommandSender, org.bukkit.craftbukkit.v1_18_R2.entity.CraftPlayer, etc.), except maybe CommandListenerWrapper (net.minecraft.commands.CommandListenerWrapper), which I don't think needs to change. If belonging to the net.minecraft.server namespace does not make a class NMS and therefore off-limits, what dose? What/where are the guidelines for what feature requests are valid? I didn't anything about not being able to extend the Bukkit API in the contributing guidelines here or here or in the bug report guidelines here or here. In fact, the post here also mentions some API changes that were made around 1.14. Is there a document somewhere I missed that I could check first to avoid making invalid feature suggestions in the future? |
Comment by md_5 [ 18/May/22 ] |
The Bukkit API is not designed to be extended: https://hub.spigotmc.org/javadocs/spigot/index.html?overview-summary.html and furthermore you are talking about NMS classes which aren't even API |