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

Full V13 support: Interactions (buttons and select menus) and reactions! #17

Open
wants to merge 41 commits into
base: master
Choose a base branch
from

Conversation

psibean
Copy link

@psibean psibean commented Jan 18, 2021

I have effectively abandoned this PR and released my own, full v13 compatible version.

Find it here with it's documentation and a working example bot.

Install with

npm install @psibean/discord.js-pagination

This was referenced Jan 18, 2021
@psibean
Copy link
Author

psibean commented Jan 18, 2021

If you do something like this with this code:

    const pageResolver = async (message, pages, emojiList, currentPageIndex, reaction) => {
      let newPage = currentPageIndex;
      switch (reaction.emoji.name) {
        case emojiList[0]:
          newPage = currentPageIndex > 0 ? currentPageIndex - 1 : pages.length - 1;
          break;
        case emojiList[1]:
          newPage = currentPageIndex + 1 < pages.length ? currentPageIndex + 1 : 0;
          break;
        case emojiList[2]:
          await message.delete();
          break;
        default:
          return newPage;
      }
      return newPage;
    };
    const emojiList = ['⏪', '⏩', '🇽']

    return paginationEmbed(message, pages, { emojiList, pageResolver });

If you click the x to delete the message and also click an arrow before the message is deleted, there's a race condition on await reaction.users.remove(user.id); in the collect event with the removal of the users react and the deletion of the message. A better way to handle this would be to queue the collected reactions and then handle them from the queue in the order they are received. I'll see if I can do this as well at a later time.

Edit: This is a discord.js race condition issue.

Copy link

@Alcremie Alcremie left a comment

Choose a reason for hiding this comment

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

Just an initial sweep of the PR, since I got the notification from the issue that one was created.

index.d.ts Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@psibean
Copy link
Author

psibean commented Jan 18, 2021

Thanks for the feedback, completely agree with all points. I’ll get back into it and resolve / reply more specifically once I’m done with work!

@psibean
Copy link
Author

psibean commented Jan 19, 2021

  • Switched all , to ; in typings and made consistent.
  • Fixed spacings and attempted to fix indentation (it wasn't appearing like that on my side).
  • Set defaultCollectorFilter to retain the original logic for increased backwards compatibility.
  • Added a sendMessage option - I always confuse my overlap of what is discord.js and what is akairo.
  • Removed the redundant try / catch and stop and extra deleted check.
  • Switched the deleteOnEnd check on end to use Message#deletable

In my previously presented example,

await reaction.users.remove(user.id);

this line still throws an unknown message error if you hit a delete option then react another before the message is deleted. Since this is happening in the collect event, it's not something you can catch.

I discussed the Message#deleted issue on discord.js discord server here - it's a troublesome race condition, for my use case I'll work around it.

But it does mean there is still potential for errors to be thrown from the reaction removal in a case where collected reactions are sent shortly after a manual message delete is made, but before the message_deleted gateway response has come back for the message to be marked as deleted and fire the messages deleted event.

Alternatively, I could pass the user to the pageResolver and handle the removal there, then people can also provide their custom logic and error handling around that.

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@Alcremie
Copy link

I think the best thing regarding reaction removal would be to just add a Promise#catch noop, e.g.

await reaction.users.remove(user.id).catch(() => {});

Or otherwise allow it to propagate and let the user handle it themselves. I've thought about proposing a throttle for message editing, so that deleting a message via a reaction and then immediately trying to react should not cause an edit to happen from the time the message is deleted to the time the reaction collector catches up and automatically ends, but this doesn't solve the problem of trying to react immediately after the message was deleted through other means (e.g. manually by another user).

@psibean psibean mentioned this pull request Mar 27, 2021
Copy link

@ikrishagarwal ikrishagarwal left a comment

Choose a reason for hiding this comment

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

Awesome Upgrade. Loved it.

@DudeThatsErin
Copy link

This is not working for me.

First, if I include EmojiList in the options on the help.js, page it states "emojiList is not defined" do I put emojies inside brackets there?

Second, this is all of the code: https://sourceb.in/W6WKao4Cvk
I'm getting cannot send an empty message.

@psibean
Copy link
Author

psibean commented Aug 6, 2021

Apologies if formatting here isn't quite right, I'm on mobile.

First, if I include EmojiList in the options on the help.js, page it states "emojiList is not defined" do I put emojies inside brackets there?

This error means that the actual variable emojiList, which you're passing into the function does not exist where you're using it. This is to do with variable scope.

Example:

if (someCondition) {
  const emjiList = [];
// can only access emojiList in this block
  ...
} else {
  // emojiList is not defined here
}
// emojiList is not defined here

Make sure whatever you pass to the paginationEmbed function is actually within scope of the call.

Second, this is all of the code: https://sourceb.in/W6WKao4Cvk

I'm getting cannot send an empty message.

This ones a bit trickier. My initial glance at the code, nothing jumps out. From what I can see, you should have at least 10 valid pages. Does this happen right away, or after navigating? The error indicates that your array has a blank / empty embed, or a null/undefined value

Happy to help over discord!

Also please be aware this version does not yet work with djs v13 which was just released.

If you're using djs b13 this WILL be a problem.

I will update it to v13 in a few hours!

I may also publish this version to npm to make it easier for people to use

@DudeThatsErin
Copy link

Apologies if formatting here isn't quite right, I'm on mobile.

First, if I include EmojiList in the options on the help.js, page it states "emojiList is not defined" do I put emojies inside brackets there?

This error means that the actual variable emojiList, which you're passing into the function does not exist where you're using it. This is to do with variable scope.

Example:

if (someCondition) {
  const emjiList = [];
// can only access emojiList in this block
  ...
} else {
  // emojiList is not defined here
}
// emojiList is not defined here

Make sure whatever you pass to the paginationEmbed function is actually within scope of the call.

Second, this is all of the code: https://sourceb.in/W6WKao4Cvk
I'm getting cannot send an empty message.

This ones a bit trickier. My initial glance at the code, nothing jumps out. From what I can see, you should have at least 10 valid pages. Does this happen right away, or after navigating? The error indicates that your array has a blank / empty embed, or a null/undefined value

Happy to help over discord!

Also please be aware this version does not yet work with djs v13 which was just released.

If you're using djs b13 this WILL be a problem.

I will update it to v13 in a few hours!

I may also publish this version to npm to make it easier for people to use

Ah, yeah, I'm using DJS V13. My discord is DudeThatsErin#8061 whenever you get this working with v13, please shoot me a message. I would definitely appreciate it. Appreciate the fast response as well.

@psibean psibean changed the title Custom footer resolver, custom page resolver, custom sendMessage, custom collectFilter, deleteOnEnd, typings! Massive refactor for configurability / customization, typescript typings, discordjs v13 support! Aug 7, 2021
@psibean
Copy link
Author

psibean commented Aug 7, 2021

Ah, yeah, I'm using DJS V13. My discord is DudeThatsErin#8061 whenever you get this working with v13, please shoot me a message. I would definitely appreciate it. Appreciate the fast response as well.

I have since updated this as per the discordjs Updating from v12 to v13 guide. I have also added you on discord, please let me know if there are any issues with the updates, I'll be happy to continue support and maintenance!

I have also published my version of the package to npm. You can now install and use via:

npm install @psibean/discord.js-pagination
const paginationEmbed = require('@psibean/discord.js-pagination');

Edit: Had not yet pushed the v13 changes into the branch for this PR - they're in now.

For those interested, the changes in v13 that impact this package are:

message.channel.send(embed)
message.edit(embed)

become

message.channel.send({ embeds: [embed] });
message.edit({ embeds: [embed] });

respectively, and for the message#createReactionCollector call, the reaction collector filter callback gets passed in as filter in the options.

On this PR I bumped the version to 2.0.0 as it's no longer backwards compatible.

If you want this version of the code which is still compatible with discordjs v12, use @psibean/discord.js-pagination@1.1.0

@psibean
Copy link
Author

psibean commented Aug 8, 2021

There were some additional issues found, they have now been fixed and this has been tested with discord.js v13 working as of v2.0.8. The paginationEmbed can take either a message or an interaction. (only used to access Message/Interaction#channel#send, unless you otherwise customise sendMessage).

The fixes were also ported to the discord.js v12 version of the package which is now working at v1.1.1

@psibean
Copy link
Author

psibean commented Aug 15, 2021

I'm in the process of updating this to be class based and to support MessageComponents (buttons and select menus).
I currently have support for buttons, select menus and message reactions.
The buttons and select option paginations provide a "custom prefix" so you can avoid the interactions in your client ineractionCreate, e.g.:

if (interaction.customId.startsWith('pagination')) return

Each pagination also uses the received interaction/messages id (from the command initiating the pagination) as a suffix. This way in each paginations collectorFilter you can do:

interaction.customId.endsWith(paginator.customIdSuffix)

This allows all of the event listeners to easily react to only the interactions they need.

WIP: psibean#1

The PR branch contains a WORKING example too, I also plan on fully documenting once it's ready.

@psibean
Copy link
Author

psibean commented Aug 17, 2021

I've abandoned the PR at this point.

Find the new version over here.

Complete support for reaction, button and select menu pagination. Fully customizable and controllable.

@psibean psibean changed the title Massive refactor for configurability / customization, typescript typings, discordjs v13 support! Full V13 support: Interactions (buttons and select menus) and reactions! Aug 17, 2021
humaiyun added a commit to humaiyun/Keplar that referenced this pull request Sep 23, 2021
saanuregh/discord.js-pagination#17

Full V13 support: Interactions (buttons and select menus) and reactions! #17
humaiyun added a commit to humaiyun/Keplar that referenced this pull request Sep 25, 2021
saanuregh/discord.js-pagination#17

Full V13 support: Interactions (buttons and select menus) and reactions! #17
humaiyun added a commit to humaiyun/Keplar that referenced this pull request Sep 25, 2021
saanuregh/discord.js-pagination#17

Full V13 support: Interactions (buttons and select menus) and reactions! #17
humaiyun added a commit to humaiyun/Keplar that referenced this pull request Sep 26, 2021
saanuregh/discord.js-pagination#17

Full V13 support: Interactions (buttons and select menus) and reactions! #17
humaiyun added a commit to humaiyun/Keplar that referenced this pull request Sep 28, 2021
saanuregh/discord.js-pagination#17

Full V13 support: Interactions (buttons and select menus) and reactions! #17
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.

4 participants