Now allow an array to be set for $numberOfArguments

[NanoSector]

On the built-in commands this isn't applicable. But consider this: My bot has an !issue command. If !issue is passed without parameters, it will return the last 5 issues posted on GitHub. If !issue has 1 parameter, it will return info about a specific issue.

Now, before this patch, $numberOfArguments was required to be set to -1. This allows multiple arguments to be passed, while it will only process one.

With this patch, I can simply set it to array(0, 1) and it'll take either 0 or 1 argument(s), and errors out on anything else. -1 can still be passed in the array and it behaves differently after this patch. Before: -1 would include 0 arguments as correct. After: -1 will now work if amount of arguments > 0 This is done to avoid possible issues that could arise from passing no arguments to a command that won't work without arguments. To allow both 0 and any amount of arguments, pass array(0, -1).

This patch also includes a safeguard that will block commands that do not have the $numberOfArguments set from running, again to prevent issues which can arise by an incorrect amount of arguments.

Do note that I left the "For help, use !help [command name]" in here. If you want to remove it, go ahead. Signed-off-by: Yoshi2889 rick.2889@gmail.com

[JackBlower]

Is it still possible to emulate the original ability to pass any amount of arguments?

It's possible a command works with no arguments but also with 1 or more.

[NanoSector]

JackBlower, for both no and 1 or more arguments, pass array(0, -1). Read my wall of text to find out why I did this. :) ETA: if you want 1+ arguments, just pass -1 like before. But for allowing 0 arguments, you now have to explicitly set it.

[JackBlower]

I read the wall of text and I now I see that somehow I missed the last sentence. =)

To allow both 0 and any amount of arguments, pass array(0, -1).

Good to see that it's still possible to have both.

[JackBlower]

Oh I'm on the wrong account, that's why I'm not a collaborator on here.

[ElvenSpellmaker]

I can see what's being done here and I agree a change is needed.

It'd be nice if you could update all of the exisiting Commands to ensure they have an argument value set (and that it's the right one), else this code will break them out of the box. =)

Also, seeing as it breaks backwards compatibility it'd be nice to get a comment from another collaborator.

[NanoSector]

I made sure of that @ElvenSpellmaker, check my commit; only the Quit and Restart commands were missing it as far as I can see. :)

[ElvenSpellmaker]

Sorry I'm obviously completely loopy today...

[NanoSector]

No problem, we all have those days I guess ;)

Also, responding to your comment on breaking backwards compatibility; in general this patch should not break any commands; commands can still pass regular ints like before in much the same way.

The only exception to this would be a command which relies on -1 also accepting 0 arguments as correct. This patch will require those commands to have their -1 changed into array(0, -1). This might be worth mentioning indeed. On the other hand this patch also provides a bug fix for commands that simply relied on -1 allowing multiple arguments, and wouldn't work with 0 arguments. Authors can now drop their custom argument count check and rely on the bot doing the actual checks (and have the bot handle errors in such case, allowing for consistency between commands).

[NanoSector]

By the way, if this gets through, I can publish a fix/improvement for #43 using this new functionality. :)

[TimTims]

I like it a lot, I'll merge it in a few minutes. I'm also going to open an IRC channel again seeing as the popularity of the bot is rising substantially. So I'll add that to the readme when I'm all done.

[ElvenSpellmaker]

eec0e820c4fa6ba5090335cca0f838c20f11fc adds something to the README to explain the new change.