Multivit4min / Sinusbot-Command

Simple Command Library for Sinusbot! Create commands, arguments and documentation for the command
https://multivit4min.github.io/Sinusbot-Command/
MIT License
7 stars 24 forks source link

Feature Request? ClientArgument should accpet client names #5

Closed kaerns closed 5 years ago

kaerns commented 5 years ago

Hi,

i think ClientArgument should also take the client name beside the uid and convert both to an client object for the exec.

random code: .addArgument(commandlib.createArgument("client").setName("target"))

greetings essem

PS: I'm sorry if you don't take feature requests or this is the wrong place for it.

Multivit4min commented 5 years ago

I thought about this aswell, however I designed this library to use as minimal sinusbot functions as possible for performance reasons.

You could emulate this behaviour:

createCommand("foo")
  .addArgument(
    createGroupedArgument("or")
      .setName("client")
      .argument(createArgument("client").setName("uid"))
      .argument(createArgument("string").setName("name").min(1))
  )
  .exec((invoker, args) => {
    let client = null
    if (args.client.name) {
      client = backend.getClientByName(args.client.name)
    } else {
      client = backend.getClientByUID(args.client.uid)
    }
    //no online client found
    if (!client) return 
  })

However i did not test the above code

PS: I'm sorry if you don't take feature requests or this is the wrong place for it. this is the right place for it ;)

kaerns commented 5 years ago

Got some time for testing and it dosn't seem to work properly. If i set the client argument first it processes the uuid but gives me invalid usage with a string:

        .addArgument(
            commandlib.createGroupedArgument("or")
            .setName("target")
            .argument(commandlib.createArgument("client").setName("uid"))
            .argument(commandlib.createArgument("string").setName("name").min(1))
        )

The other way around it takes both but treats both as string, so the client argument is ignored:

        .addArgument(
            commandlib.createGroupedArgument("or")
            .setName("target")
            .argument(commandlib.createArgument("string").setName("name").min(1))
            .argument(commandlib.createArgument("client").setName("uid"))
        )

seems like i'll have to do it the manual way. Also wouldn't string give problems with names with a space? or is there some way to input (in chat) a string with spaces which will count as one argument?

For the moment i'll use this workaround which works pretty good (except for names with spaces):

        .addArgument(commandlib.createArgument("string").setName("target"))
        .exec((client, args, reply, raw) => {
            let target = backend.getClientByUID(args.target);
            if(!target) {
                target = backend.getClientByName(args.target);
            }

            if(!target) return reply("Could not find Client: "+args.target);
        });
Multivit4min commented 5 years ago

I tried to reproduce your first example with following code:

const command = require("command")

command.createCommand("test")
  .addArgument(
    command.createGroupedArgument("or")
      .setName("target")
      .argument(command.createArgument("client").setName("uid"))
      .argument(command.createArgument("string").setName("name").min(1))
  )
  .exec((invoker, args, reply) => {
    console.log(args)
    reply(JSON.stringify(args))
  })

which seems to work for me: image

with command.js version 1.2.3 the Grouped "or" argument always priorizes the first added argument, if it fails it checks the next one

kaerns commented 5 years ago

Ok i found the issue. The command.js i got with the sinusbot update was on version 1.2.1 which seems to have that bug. Updated to 1.2.3 and your example instantly worked

Multivit4min commented 5 years ago

Alright, the feature with the nickname will not be implemented since its a bit problematic parsing the arguments when a client himself has spaces in his name