search

python-discord / sir-lancebot

A Discord bot started as a community project for Hacktoberfest 2018, later evolved to an introductory project for aspiring new developers starting out with open source development.
MIT License
241 stars 239 forks source link

Trashcan delete reaction does not work in DMs #1450

Open hedyhli opened 8 months ago

hedyhli commented 8 months ago

Steps to Reproduce

  1. Run a help command in DM
  2. Click on the trash emoji

If we have the emoji there, users would expect it to delete the help embed, however it does not do anything currently.

Known Impacted Platforms

It's probably the same, but I only tested on iOS for now.

Possible Solutions

If it's not possible or infeasible to actually implement this, there shouldn't be a trashcan reaction there at all. It can be misleading.

Currently I'm using .bm del to delete those help embeds instead.

Would you like to implement a fix?

Note: For high-priority or critical bugs, fixes may be implemented by staff.

Anyone can implement the bug fix

shtlrs commented 8 months ago

To help track this down, when the MESSAGE_REACTION_ADD event is fired from the gateway, discord.py calls the parse_message_reaction_add

The reaction_add event is dispatched only when the user variable is not None in this line user = raw.member or self._get_reaction_user(message.channel, raw.user_id)

Since this is a DMChannel, the user is fetched from the _users cache, but the user doesn't exist there for reasons that I don't know yet.

One solution to this is to use the on_raw_reaction_add listener, or to use buttons instead of emojis. We've previously discussed the usage of buttons before and we didn't quite come to the conclusion, so maybe the best approach is to either

  1. Find out why the _users cache doesn't contain the user who initiated the reaction
  2. Use the raw reaction event if the question to the previous question isn't answered.
ChrisLovering commented 8 months ago

The second note in the docs mentions this behaviour with the event, so option 1 doesn't seem worth persuing

shtlrs commented 8 months ago

I have completely missed that :/ Yeah well, i guess we have our answer