disnake icon indicating copy to clipboard operation
disnake copied to clipboard

fix(views): views in interaction responses

Open EQUENOS opened this issue 3 years ago • 6 comments

Summary

Currently views fail to do the job if you send them with interaction responses.

Example:

class TestView(disnake.ui.View):
    def __init__(self):
        super().__init__(timeout=60)

    @disnake.ui.button(label="Hello world", custom_id="test")
    async def on_test(self, button, inter):
        button.label = "Goodbye world"
        button.disabled = True
        await inter.response.edit_message(view=self)
        self.stop()

@bot.slash_command()
async def test(inter):
    await inter.send("...", view=TestView())

Invoke this command twice and then click the buttons under each message.

Expected result: both views respond

Actual result: only one view responds

Checklist

  • [x] If code changes were made, then they have been tested
    • [ ] I have updated the documentation to reflect the changes
    • [x] I have formatted the code properly by running task lint or pre-commit run --all-files
  • [x] This PR fixes an issue
  • [ ] This PR adds something new (e.g. new method or parameters)
  • [ ] This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • [ ] This PR is not a code change (e.g. documentation, README, ...)

EQUENOS avatar Feb 04 '22 16:02 EQUENOS

This unfortunately breaks cases where .response.edit_message is used with a new view, since the new view gets added to the store with the message ID (known, since it's a component interaction), but the button dispatch handler later uses the interaction ID of the slash command interaction that created the initial view.

class TestView(disnake.ui.View):
    def __init__(self, label: str):
        super().__init__(timeout=60)
        self.on_test.label = label

    @disnake.ui.button(label="x", custom_id="test")
    async def on_test(self, button, inter):
        await inter.response.edit_message(view=TestView("2"))
        self.stop()

@bot.slash_command()
async def test(inter):
    await inter.send("...", view=TestView("1"))

shiftinv avatar Feb 04 '22 20:02 shiftinv

Oh, and when using .edit_message(view=self) instead, it results in the view being added a second time to the store, one with the interaction ID in the key (from original .send), and another one with the message ID in the key (from the .edit_message) Though I suppose that was an issue before as well, just with None/message_id keys instead of interaction_id/message_id

shiftinv avatar Feb 04 '22 20:02 shiftinv

This is currently unsolved due to not always having the original interaction ID in the view response, eg component interactions responded to with a new message.

While I don't like the idea, we could solve this by using original_message in those cases and storing the view with that ID.

onerandomusername avatar Apr 28 '22 09:04 onerandomusername

The last attempt to save this PR is to concatenate all custom_ids of view components with the interaction ID before sending the view as an interaction response. While this is a simple solution to the original problem, it can lead to inconveniences in development because it decreases the length limit to 80 and is a breaking change. What do you think?

EQUENOS avatar Aug 27 '22 21:08 EQUENOS

The last attempt to save this PR is to concatenate all custom_ids of view components with the interaction ID before sending the view as an interaction response. While this is a simple solution to the original problem, it can lead to inconveniences in development because it decreases the length limit to 80 and is a breaking change. What do you think?

It'd be a shame to reduce the limit to 80 across the board for a nieche bug case, how about making it an argument? Something like append_interaction_id (think of better naming) so that when people create views that fit the criteria for this bug to occur they can pass the flag and don't encounter this issue, and that also means the regular use case remains the same.

Thoughts?

Skelmis avatar Jul 19 '23 22:07 Skelmis

It's been a while since I last looked into this, but we brainstormed a few ideas some time ago which I believe didn't have any notable limitations. I'll look it up tomorrow; I thought I had linked it here before, turns out I didn't 😅

for future reference, context: https://discord.com/channels/808030843078836254/1131333904683503666

shiftinv avatar Jul 19 '23 22:07 shiftinv