-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(CommandInteraction): add CommandInteractionOptionResolver #6107
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suppose you could do [required=false]
but I only thought this after I made all the review comments and im lazy
This may also close #5880 |
Can you make the name parameter in getSubCommand optional? A command has multiple subcommands but only one can be ran at once so it doesn’t make much sense to specify the name of the command since you’re not supposed to know what name that’s gonna be. |
That's why async function run(interaction: CommandInteraction) {
const subcommand1 = interaction.options.getSubCommand("sub1");
const subcommand2 = interaction.options.getSubCommand("sub2");
if (subcommand1) {
const subarg1 = subcommand1.getInteger("subarg1");
// etc.
} else if (subcommand2) {
const subarg2 = subcommand2.getUser("subarg2");
// etc.
}
} |
I don't think this captures what Rodry meant, nor does it really address #5880 if I still don't immediately know which subcommand was run. Why would I want to try to get both (or more) subcommands - you know it has to be one of them. I just want to get whichever one it is. const subcommand = interaction.options.getSubCommand();
if (subcommand.name === "sub1") {
const subarg = subcommand.getInteger("integer");
// etc.
} else if (subcommand.name === "sub2") {
const subarg = subcommand.getUser("user");
// etc.
} |
Co-authored-by: Sugden <28943913+NotSugden@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Collection.get() returns undefined when it doesn't find the specified key so I think this behavior should stay the same
I don't agree with changing the interface to return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, following the convo on the discord server let's make group and subcommand properties instead of methods
Please describe the changes this PR makes and why it should be merged:
This PR creates a new structure,
CommandInteractionOptionResolver
, which acts as a resolver forCommandInteraction
options. This replaces the existingCollection<string, CommandInteractionOption>
onCommandInteraction#options
with the new class.The new class will allow for easier usage and validation of command options. It also will serve as an additional check against users having mismatches between their option types and their execution code. This also allows for TypeScript users to access the properties in a cleaner and type-safe way without custom parsers or casting.
Example:
Status and versioning classification: