Skip to content

Conversation

codemasher
Copy link
Contributor

As per the conversation in #1413.

@codemasher codemasher force-pushed the register-command-handlers branch from 773b633 to 59ea77d Compare October 6, 2025 02:20
@codemasher codemasher force-pushed the register-command-handlers branch from 59ea77d to 9a78cb4 Compare October 6, 2025 02:22
@codemasher
Copy link
Contributor Author

codemasher commented Oct 6, 2025

@valzargaming re this comment:
I see why this might be a breaking change for some, but I think the main issue is that there is no clear behaviour to this method (not to mention the lack of documentation), so people might be using it in unexpected ways. I guess there's a wider (definitely breaking) overhaul necessary, also in order to properly accommodate subcommand groups.

Right now, there seems an issue with registering a command with subcommands in one go, like so:

$discord->listenCommand(['command', 'subcommand1', 'subcommand2', /* ... */], execute(...), autocomplete(...));

Registering a single command and adding subcommands via the method on the RegisteredCommand instance works as expected:

$registeredCommand = $discord->listenCommand('command', execute(...), autocomplete(...));

foreach(['subcommand1', 'subcommand2', /* ... */] as $subcommand){
    $registeredCommand->addSubCommand($subcommand, execute(...), autocomplete(...));
}

Then apparently there's this case too, as mentioned in the other issue:

$discord->listenCommand(['command', 'subcommand1'], execute(...), autocomplete(...));
$discord->listenCommand(['command', 'subcommand2'], execute(...), autocomplete(...));
/* ... */

Further I can see it being used somehow like this:

$discord->listenCommand(['command', ['subcommandgroup1', 'subcommand1'], ['subcommandgroup2', 'subcommand1'], /* ... */], execute(...), autocomplete(...));

All in all I think that's too much complexity and error prone behaviour to handle (I don't even know yet where and how subcommand groups are handled). The method should offer stricter parameters, and/or additional methods that properly handle a single task in a concise way should be added.

Discord::listenCommand(
    string $name, 
    ?string $subcommandGroup = null, 
    array $subcommands = [], 
    ?callable $callback = null, 
    ?callable $autocomplete_callback = null.
):RegisteredCommand

This way it should be possible to call the method repeatedly without throwing and handle the internals without confusion:

$discord->listenCommand(name: 'command', callback: execute(...), autocomplete_callback: autocomplete(...));
$discord->listenCommand('command', 'subcommandgroup1', ['subcommand1', /* ... */], execute(...), autocomplete(...));
$discord->listenCommand('command', 'subcommandgroup2', ['subcommand1', /* ... */], execute(...), autocomplete(...));

@valzargaming
Copy link
Member

@Log1x, whenever you get the chance, what are your thoughts on this? I know Laracord abstracts a lot of this logic, and I think changing things as they are would break the existing framework.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants