Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(InteractionCollector): reworked to be more generic #5999

Merged
merged 10 commits into from Jul 3, 2021

Conversation

monbrey
Copy link
Member

@monbrey monbrey commented Jul 1, 2021

Accidentally pushed a delete to remote on the old PR #5936 so.... yeaaaaaah.

Another half-baked idea that comes out of late night conversations and early morning coding.

This refactors the MessageComponentInteractionCollector into a generic InteractionCollector that can be used to collect any sort of interaction (yes, even Slash Commands).

It also removes the need for it to be bound to any particular message or channel, though it still can be should you want to do that. These are still separate options to the filter as it allows the Collector to also listen to the relevant DELETE events and cancel itself accordingly. It will use these based on the most specific option first, so Message > Channel > Guild.

Draft for now - I'd still like reviews and opinions from other people. I intend to add some checking to the constructor to account for options that don't make sense together, e.g. setting a componentType option if interactionType is set for slash commands.

Status and versioning classification

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating
  • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)

@iCrawl iCrawl added this to the Version 13 milestone Jul 1, 2021
src/structures/InteractionCollector.js Outdated Show resolved Hide resolved
src/structures/InteractionCollector.js Outdated Show resolved Hide resolved
src/structures/Message.js Outdated Show resolved Hide resolved
src/structures/interfaces/TextBasedChannel.js Outdated Show resolved Hide resolved
Copy link
Member

@vladfrangu vladfrangu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than kyra's comments, LGTM

@kyranet kyranet requested a review from iCrawl July 2, 2021 22:49
Copy link
Member

@SpaceEEC SpaceEEC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM

src/structures/InteractionCollector.js Outdated Show resolved Hide resolved
@iCrawl
Copy link
Member

iCrawl commented Jul 3, 2021

This needs a rebase.

@iCrawl iCrawl merged commit 374c779 into discordjs:master Jul 3, 2021
@monbrey monbrey deleted the collector-rework branch July 3, 2021 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants