serenity icon indicating copy to clipboard operation
serenity copied to clipboard

Flatten the `Interaction` enum into a struct

Open nickelc opened this issue 3 years ago • 1 comments

The Interaction enum is changed to a struct and a InteractionData enum for the different data.

This brings the model closer to Discord's model, direct access to the common fields and removes the duplicated interaction response methods.

I have created this PR as a draft because the complex collector module is in a broken state and is based on #2031.

nickelc avatar Jul 06 '22 14:07 nickelc

Collector could be made functional again by instead of using an enum, use a generic, eg: Interaction<MessageComponent> and have MessageComponent store the actual data

GnomedDev avatar Jul 06 '22 15:07 GnomedDev

I just attempted to re-implement this PR to be able to close this outdated one.

I don't think this is a good idea. CommandData is now wrapped behind an Option and must be unwrap()ed in user code, which is an unnecessary unwrap for ApplicationCommand, MesageComponent and ModalSubmit interactions. Also, each of these three have very different fields in their CommandData. Also, the message field is specific to MessageComponent, so it also means redundant unwrapping in the MessageComponent, and a redundant field in all other cases.

Edit: when I wrote that comment, I misunderstood the PR. I thought it had a flat CommandData struct that contained all fields of all possible interaction types. This is not the case - it uses an enum, just like my #2229. Just wanted to get this right because the above is party unfair criticism

kangalio avatar Oct 11 '22 17:10 kangalio

Clean diff: b4b8bd446d25a687d08e9d348cf744cf6c718346

kangalio avatar Oct 26 '22 14:10 kangalio