Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework CommandService#ExecuteAsync logic and allow easier extensibility. #1699

Closed
3 tasks done
alikindsys opened this issue Nov 27, 2020 · 0 comments · Fixed by #1700
Closed
3 tasks done

Rework CommandService#ExecuteAsync logic and allow easier extensibility. #1699

alikindsys opened this issue Nov 27, 2020 · 0 comments · Fixed by #1700
Labels
Needs investigation Needs to be looked at by a maintainer

Comments

@alikindsys
Copy link
Contributor

alikindsys commented Nov 27, 2020

In the current state, ExecuteAsync does two functions. Command Execution and Command Validation (Valid command check/ predicate check/ parameter check/ override check, etc.).
My idea is to split those into two separate functions, one responsible for validating the commands and another for execution.
For compatibility and no major breaks, i decided to keep the current public api unchanged with only the addition of a ValidateAndGetBestMatch function with the following signature.

public async Task<(IResult, Optional<CommandMatch>)> ValidateAndGetBestMatch(SearchResult matches,
ICommandContext context,
IServiceProvider provider,
MultiMatchHandling multiMatchHandling = MultiMatchHandling.Exception)

This function is to be called with Search(String)'s output making ExecuteAsync's logic exclusive for execution,
Search's exclusive for searching and Validate... exclusive for validation and command matching.

An intended side-effect is the extensibility this would allow if accepted. An external CommandService wrapper could leverage Discord.Net's Validate... in order to get the command that best fits without executing it, as the only current way of getting the command that was called is via an event handler, but there is no clean way for getting a command that will be called.

class Wrapper {
    //Task CheckForWhatever(string)
    var search = _commandService.Search(message.Content.Substring(argPos));
    var (status, command) = await ValidateAndGetBestMatch(search); //Other params ommited for sake of clarity
    if (!status.IsSucess) return;
    command.Value.ExecuteAsync(command.Command, context, status); 
}

Status

@MrCakeSlayer MrCakeSlayer added the Needs investigation Needs to be looked at by a maintainer label Nov 23, 2021
@quinchs quinchs linked a pull request Nov 26, 2021 that will close this issue
@quinchs quinchs closed this as completed Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs investigation Needs to be looked at by a maintainer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants