TShock icon indicating copy to clipboard operation
TShock copied to clipboard

RFC/Discussion: New commands

Open kevzhao2 opened this issue 4 years ago • 2 comments

Commands are a major pain in the ass to implement. It's a pain to have to parse each argument manually. It's a pain to have to check the parameter count each time. It's a pain to have to use a utility method to lookup item or player names.

They should thus be as easy to implement as possible, even if this makes the command parsing significantly more complex. I propose the following (all naming is TBD):

// Plugin uses this to define the command:

[Command("i"), Command("item")]
private void ItemCommandHandler(
    ICommandSender sender, ItemType itemType,
    int stackSize = 1, ItemPrefix prefix = ItemPrefix.None) {
}

// Plugin uses this to register the command:

var itemCommand = _commandService.RegisterCommand(ItemCommandHandler);

// Plugin uses this to unregister the command:

_commandService.UnregisterCommand(itemCommand);

ICommandSender

ICommandSender is an interface which will look like the following:

public interface ICommandSender {
    string Name { get; }
    ILogger Log { get; }

    void SendMessage(string message);
    void SendMessage(string message, Color color);
    void SendObject(object obj);
}

The whole point is to allow multiple types of senders to exist: a PlayerCommandSender for players who send the command, a ConsoleCommandSender for commands entered through the command line, a RestCommandSender for commands sent via the REST API, a SignCommandSender for commands sent from a sign, etc. For special player-related behavior, you could check if sender is PlayerCommandSender player.

The Log property will allow a command handler to log various information to the sender. This will be extremely useful for plugin developers debugging their commands.

Overall, this should allow our command infrastructure to be more extensible than ever before.

Argument Parsing

Arguments will be parsed according to the argument structure of the method annotated with CommandAttribute. Optional arguments can be left out and they will take on their default values. Named arguments may or may not be supported. params arguments will be parsed repeatedly until failure to parse.

All basic types (int, string, etc.) should be parsed properly by default. For custom types, there will be some sort of mechanism to define parsers. This may be done via an attribute on the argument; I haven't fully decided yet. One could, for example, define an NpcType parser:

public class NpcTypeParser : ICommandArgParser<NpcType> {
    public NpcType Parse(string str) {
         return ...
    }
}

and then create the following command:

private void SpawnMobHandler(
    ICommandSender sender, [CommandArgParser(typeof(NpcTypeParser))] npcType) {
}

and the NpcTypeParser would be constructed and parse the string as appropriate.

One final feature that needs to be included is to allow a fail-safe string capture of the entire input. This could be done via an CommandInputCaptureAttribute, for example.

Command Service

The command service will expose the following hooks:

  • RegisteringCommand, which can be handled.
  • RegisteredCommand
  • ExecutingCommand, which can be handled.
  • ExecutedCommand

These hooks can be added to set up things such as permissions, command rate-limiting, etc. Additionally, I want to explore easy command registration that might scan an object for all methods which have CommandAttribute and register them as necessary.

Hypothetical Q&A

How would I define aliases for commands?

Use the CommandAttribute multiple times on your method.

Are command names unique?

~~No. There can be multiple commands with the same name. This allows you to provide two different branches of argument parsing! However, care should be taken to avoid two commands from actually executing.~~

Yes, they will be unique. It is probably not worth the time implementing multi-branched parsing. Let me know if there are any real use cases of this.

How would permissions work?

Via a hook handler on RegisteringCommand, which will handle the event if the user is deemed to not have the permissions necessary. Note that this enables advanced permissions to take place, and even temporary permissions without a temporary group hack.

A separate PermissionAttribute would need to be placed on the commands.

What about subcommands?

~~Subcommands will differ wildly in parsing rules. Therefore, spaces should be allowed in the command name. You should be able to register /region add and /region del simultaneously.~~

Subcommands will involve passing multiple arguments into the CommandAttribute constructor.

How exactly is the string parsing going to work?

As precisely as possible. Backslashes will be used as escape characters. Assuming you have a command which takes multiple string arguments:

Input Output
/command test1 test2 ["test1", "test2"]
/command a\ b c d ["a b", "c", "d"]
/command "a b" c d ["a b", "c", "d"]
/command \" ["\""]
/command \\ ["\\"]
/command \ ERROR
/command " ERROR

Questions and comments are appreciated!

kevzhao2 avatar Sep 12 '19 01:09 kevzhao2

Updated command name uniqueness requirement. Let me know if branched parsing is actually worth the implementation effort of doing so.

kevzhao2 avatar Sep 12 '19 07:09 kevzhao2

I figure namespacing & some consistent way of determining "who takes main command slot" makes sense. For instance, /restart could have two contenders for it: /tshock:restart and /someplugin:restart. Ideally the other plugin overrides tShock (tShock could, somehow, specify if they want the command to be overridden by anyone else, or not?)
This is useful because some servers will inevitably want to override some stock commands with customized stuff, and I don't know if that's easily doable with the current system (in that in CURRENT tshock you need to find the other command object and unregister it, and with the PROPOSED system you either can't register a duplicate, or you [seemingly] can't unregister the colliding command)

bartico6 avatar Sep 13 '19 09:09 bartico6