kordlib/kord

Responding to interaction with public message update implicitly clears components, embeds & files

Ampflower opened this issue · 1 comments

Description

Within UpdateMessageInteractionResponseCreateBuilder, there's non-nullable fields named components, embeds and files, each populated with an empty list.

override var files: MutableList<NamedFile> = mutableListOf()
override var content: String? = null
override var tts: Boolean? = null
override var embeds: MutableList<EmbedBuilder> = mutableListOf()
override var allowedMentions: AllowedMentionsBuilder? = null
override var components: MutableList<MessageComponentBuilder> = mutableListOf()

This empty list makes it impossible to persist components, embeds and files without having to redefine them.

This is also inconsistent with the regular MessageBehavior.edit {} API, and inconsistent with JDA, both of which can persist components, embeds and files without having to redefine them anywhere in the code.

Steps to reproduce

In this case, it happened with buttons.

kord.on<ButtonInteractionCreateEvent> {
  interaction.updatePublicMessage { content = Math.random().toString() }
}

Expected behaviour

With the given example above, clicking the button would only change the content of the message to a random double, leaving the button and other input intact.

Actual behaviour

The message gets edited to a random double, while also removing the input.

Workarounds

  • Tell the API you're deferring an edit, and edit using the original message behaviour and its API.
  • Manually adding all components that were originally on the message.

Forcefully setting the lists to null (which would've normally cleared this bug entirely) will bug-check due to Optional having an explicit non-nullable API.

Extra

This may affect other aspects of code.
Side effect of fixing this will bring a breaking API change to applications expecting the current behaviour, which is clearing on interaction edit or fixing the list itself.

Thanks for the detailed report!

I've also come accross this before but didn't get to fixing it yet.

I think it's fine to break applications depending on the current behavior if we have a warning about this in the release that delivers this fix.