andersfylling / disgord

Go module for interacting with the documented Discord's bot interface; Gateway, REST requests and voice
BSD 3-Clause "New" or "Revised" License
502 stars 71 forks source link

Add message components #399

Closed Acnologla closed 3 years ago

Acnologla commented 3 years ago

Description

Basically "InteractionApplicationCommandCallbackData" is a message with fewer fields, so it doesn't make sense to be a separate type.

Type of change

Please delete options that are not relevant.

Benchmarks

If this PR requires benchmarks (say it is an very dependent component or takes a lot of resources/use, use pprof if you need to) then the benchmarks are provided before and after such that we can make logical decisions. Note that if you add a benchmark and find your solution to run slower, the code might still be valuable so your results are welcomed anyways! If no benchmarks are needed, feel free to delete til paragraph.

Checklist:

andersfylling commented 3 years ago

If this is changed to Message, the message struct must be updated to avoid sending too much information via "omitempty". While another struct seems redundant, it's actually a little nice, cause lib users know exactly what arguments they can give and we don't have to care about how the Message type is displayed when used for this endpoint.

Acnologla commented 3 years ago

The problem is that not all accepted fields are listed in https://discord.com/developers/docs/interactions/slash-commands#interaction-response-interactionapplicationcommandcallbackdata. I've done some tests and you can send fields like "Components", and it works perfectly.

andersfylling commented 3 years ago

Does the added Components affect the product?

On Tue, 1 Jun 2021, 21:22 Acnologla, @.***> wrote:

The problem is that not all accepted fields are listed in https://discord.com/developers/docs/interactions/slash-commands#interaction-response-interactionapplicationcommandcallbackdata. I've done some tests and you can send fields like "Components", and it works perfectly.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/andersfylling/disgord/pull/399#issuecomment-852385431, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB346VDC32MIH6MYEQ4T3L3TQUXQBANCNFSM45Z4MWUQ .

Acnologla commented 3 years ago

yes

andersfylling commented 3 years ago

Alright, ill re-open this. I think we can add omitempty to every message field. As the message type is kinda read only.

On Wed, 2 Jun 2021, 00:24 Acnologla, @.***> wrote:

yes

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/andersfylling/disgord/pull/399#issuecomment-852521358, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB346VG2R45ZZWHVHLEMFSLTQVMYRANCNFSM45Z4MWUQ .

Acnologla commented 3 years ago

maybe MessageCreateParams instead of Message?

andersfylling commented 3 years ago

I think we should make a issue or PR at https://github.com/discord/discord-api-docs/pulls

Then just stick to whatever the docs says. Discord employees have consistently shown and stated that whatever is not in the docs, should not be used as it might be removed or changed at any time. That way we at least have some stability.

Acnologla commented 3 years ago

I think we should make a issue or PR at https://github.com/discord/discord-api-docs/pulls

Then just stick to whatever the docs says. Discord employees have consistently shown and stated that whatever is not in the docs, should not be used as it might be removed or changed at any time. That way we at least have some stability.

So today they added "Components" as a field to https://discord.com/developers/docs/interactions/slash-commands#interaction-response-interactionapplicationcommandcallbackdata. Apparently they are adding fields gradually. So what do you think? we can put "createMessageParams" for future updates or, for now, we can just add the Components field to the "interacInteractionApplicationCommandCommandbackData".

andersfylling commented 3 years ago

Yeah, I think reflecting the docs and gradually adding changes is nice. We keep the same data type, which means no one has to re-write code, only append fields that want on updates.

andersfylling commented 3 years ago

I think this is still using CreateMessageParams

Acnologla commented 3 years ago

I think this is still using CreateMessageParams

oops

andersfylling commented 3 years ago

awesome!