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

🍣 New Roles Plugin! #155

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open

Conversation

ArnavK-09
Copy link
Contributor

@ArnavK-09 ArnavK-09 commented Dec 8, 2023

Plugin Roles Todo List!

/role setup

  • cmd init & flashcore init
  • add comments
  • add member check before interactions for plugin
  • fix regexps
  • add variable names
  • check working
  • add roles drop-down in setup msg
  • single role per interaction
  • show modal for emote & description
  • realtime embed review
  • delete role button
  • custom embed modal
  • print setup embed & drop-down
  • cleanup delete db setup instance
  • add color custom
  • role POS check
  • good ux err msgs
  • add readme

/role restrict

  • init
  • admin check
  • autocomplete with portal cmds
  • role opt in cmd Slash
  • code qc
  • testing
  • adminstrator bypass
  • middleware file abort
  • fix pnpm-lock.yaml

Resolves #134

Copy link

vercel bot commented Dec 8, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
robo ❌ Failed (Inspect) Dec 12, 2023 10:17am

@ArnavK-09
Copy link
Contributor Author

But I guess I need to add lock for plugin-roles?

@ArnavK-09
Copy link
Contributor Author

Screenshot_2023-12-10-16-02-53-46_320a9a695de7cdce83ed5281148d6f19.jpg

@Pkmmte
Copy link
Member

Pkmmte commented Dec 12, 2023

But I guess I need to add lock for plugin-roles?

I think it should be fine now. Not sure why my git cached showed it that way.

@Pkmmte
Copy link
Member

Pkmmte commented Dec 12, 2023

@ArnavK-09 Alright, so I just checked it out and found a few things to note:

  • Buttons for "Add", "Edit", and "Delete" are missing. These should show alongside the "Publish" button already present.
Screenshot 2023-12-11 at 10 16 49 PM
  • The "Edit Embed" is a good replacement for the "Description" outlined. I like it! Let's keep it.
Screenshot 2023-12-11 at 10 18 11 PM
  • There should not be selectors in the primary /roles setup present. Currently there are two selectors present. The "Add" and "Delete" buttons are meant to handle this instead. More on this below.
Screenshot 2023-12-11 at 10 18 45 PM
  • We should be able to use even custom server emotes, not just default system emotes. This is very important. Since you brought up the concern about selectors having a 25 choice limit, let's do this instead:

    • Clicking on "Add" should have the bot reply with a non-ephemeral message (due to reaction limits). This message should contain a role selector to choose only one role, along with instructions telling the user to react to the message with an emote to use for the role. This can be done either before or after the role selector is used.
    • The idea is that admins/mods would use this in a private channel, but let's add safety measures anyway in case it's done publicly. Make sure only the person that clicked on "Add" can react or use the role selector. For reactions, if the user is not the same person that triggered it, the reaction should be removed.
    • Once a role is selected and a reaction is added, the bot should apply these to the saved emotes and auto delete the message they just reacted on. If there's no activity for 10 minutes, the message gets removed anyway to prevent spam.
  • The "Delete" button should also create a message similar to "Add". Either selecting a role or reacting with an emote removes that entry and causes the message to self-destruct.

  • If you'd like, you can remove the "Edit" button altogether since the same can be achieved by "Delete" followed by "Add" anyway.

  • "Publish" has a small typo. Currently says "Publush Setup!".

  • When the "Publish" button is pressed, the bot should reply with an ephemeral message containing a channel selector. This allows an admin/moderator to do the entire setup in a private channel while publishing it to a public channel of their choice.

  • Server users should be able to pick a role by adding a reaction, not using a selector. This is an important requirement as it follows an already-established pattern.

Sorry if I wasn't more clear in the GitHub Issue. I look forward to seeing this fully completed. We've been getting more traffic to our blogs lately so I'd love to write an article about this plugin! (& giving you credit, of course)

@ArnavK-09
Copy link
Contributor Author

ArnavK-09 commented Dec 12, 2023

@ArnavK-09 Alright, so I just checked it out and found a few things to note:

  • Buttons for "Add", "Edit", and "Delete" are missing. These should show alongside the "Publish" button already present.
Screenshot 2023-12-11 at 10 16 49 PM
  • The "Edit Embed" is a good replacement for the "Description" outlined. I like it! Let's keep it.
Screenshot 2023-12-11 at 10 18 11 PM
  • There should not be selectors in the primary /roles setup present. Currently there are two selectors present. The "Add" and "Delete" buttons are meant to handle this instead. More on this below.
Screenshot 2023-12-11 at 10 18 45 PM
  • We should be able to use even custom server emotes, not just default system emotes. This is very important. Since you brought up the concern about selectors having a 25 choice limit, let's do this instead:

    • Clicking on "Add" should have the bot reply with a non-ephemeral message (due to reaction limits). This message should contain a role selector to choose only one role, along with instructions telling the user to react to the message with an emote to use for the role. This can be done either before or after the role selector is used.
    • The idea is that admins/mods would use this in a private channel, but let's add safety measures anyway in case it's done publicly. Make sure only the person that clicked on "Add" can react or use the role selector. For reactions, if the user is not the same person that triggered it, the reaction should be removed.
    • Once a role is selected and a reaction is added, the bot should apply these to the saved emotes and auto delete the message they just reacted on. If there's no activity for 10 minutes, the message gets removed anyway to prevent spam.
  • The "Delete" button should also create a message similar to "Add". Either selecting a role or reacting with an emote removes that entry and causes the message to self-destruct.

  • If you'd like, you can remove the "Edit" button altogether since the same can be achieved by "Delete" followed by "Add" anyway.

  • "Publish" has a small typo. Currently says "Publush Setup!".

  • When the "Publish" button is pressed, the bot should reply with an ephemeral message containing a channel selector. This allows an admin/moderator to do the entire setup in a private channel while publishing it to a public channel of their choice.

  • Server users should be able to pick a role by adding a reaction, not using a selector. This is an important requirement as it follows an already-established pattern.

Sorry if I wasn't more clear in the GitHub Issue. I look forward to seeing this fully completed. We've been getting more traffic to our blogs lately so I'd love to write an article about this plugin! (& giving you credit, of course)

1. I added buttons for "add" "edit" & "delete" earlier! But as I discussed on the official discord server, it would be a ux disaster for users! So we decided to use RoleSelectorDropDosn to add roles to the drop-down setup!

2. tysm 👍

@ArnavK-09
Copy link
Contributor Author

"Publish" has a small typo. Currently says "Publush Setup!".

Fixed

@ArnavK-09
Copy link
Contributor Author

Server users should be able to pick a role by adding a reaction, not using a selector. This is an important requirement as it follows an already-established pattern

The role selector one was suggested by the discord community as it is a modern & discord supported option to choose a role!

Isn't it would be a mess if user uses reactions in the era of modern dropdowns & button? + It would lack emotes formality as there is only 26 letters or 9 numbers emotes to choose from?

@ArnavK-09
Copy link
Contributor Author

There should not be selectors in the primary /roles setup present. Currently there are two selectors present. The "Add" and "Delete" buttons are meant to handle this instead. More on this below.

Modal or Message Collector?

  • Modal: there would be a ux decline if user needs to enter role id (as discussed in robo.js discord server)

  • Message Collector: isn't it would be lol moment if during adding role mentioning it, would result in pings? Usually

@ArnavK-09
Copy link
Contributor Author

ArnavK-09 commented Dec 12, 2023

support of custom emotes?

well it is already supported, the emote added by the user in the modal is directory interpreted by .setEmoji() func resulting in every type of emote whether custom, server or tweemoji it would interpret on its own

@ArnavK-09
Copy link
Contributor Author

Idky why but yeh message or reaction collectors are mess own it's own!

@ArnavK-09
Copy link
Contributor Author

When the "Publish" button is pressed, the bot should reply with an ephemeral message containing a channel selector. This allows an admin/moderator to do the entire setup in a private channel while publishing it to a public channel of their choice

Noted, would update soon after we solve add debate

@Pkmmte
Copy link
Member

Pkmmte commented Dec 13, 2023

I added buttons for "add" "edit" & "delete" earlier! But as I discussed on the official discord server, it would be a ux disaster for users! So we decided to use RoleSelectorDropDosn to add roles to the drop-down setup!

I find the current role selectors to be more confusing. They make it seem like it is meant to be used for the end user when it's really not. The earlier implementation was only confusing because it opened up a modal which required the emoji code to be precise, which is not something we can guarantee from anyone to use perfectly.

The role selector one was suggested by the discord community as it is a modern & discord supported option to choose a role! Isn't it would be a mess if user uses reactions in the era of modern dropdowns & button?

Is it? Emote reactions are an already-established pattern and used by extremely popular bots such as MEE6 and YAGPDB:

By following this pattern, users are already familiar with what to do, server owners are more likely to accept bots with it, and overall adoption would be easier which is something we definitely need at this stage. New patterns are best done by established players.

If you feel this strongly about it, how about letting server admins/mods decide when publishing? Via a button toggle or something of the sort.

It would lack emotes formality as there is only 26 letters or 9 numbers emotes to choose from?

I'm not sure what you mean by this. There is a limit of 20 emote reactions to a message, but server limits are much higher than that?

Modal or Message Collector?

Neither. Per my last message, it should be handled by a separate non-ephemeral message the bot replies with after clicking "Add". The emote is chosen by reacting to it, rather than entering its ID, and the role selector currently in "/roles setup" would be present here.

Furthermore, I advise not using the Discord.js "Collector" unless you're willing to set up a revival mechanism. The reason for this is that the collector is stateful and likely to break between bot restarts, requiring you to start a collector again for each and every past instance. The messageReactionAdd event would be a better fit here.

well it is already supported, the emote added by the user in the modal is directory interpreted by .setEmoji() func resulting in every type of emote whether custom, server or tweemoji it would interpret on its own

We shouldn't expect users to know how to enter the correct string in a way that can be interpreted by .setEmoji(). It'd be much easier to simply react to it.

Idky why but yeh message or reaction collectors are mess own it's own!

Agreed. xD

@ArnavK-09
Copy link
Contributor Author

ArnavK-09 commented Dec 13, 2023

Ok so there may be some communication error

  • Are you saying to use reactions during /role setup to choose role emote
+OR
  • To use reactions in Print Message ?? So that members can select a role by reacting to emote?

@ArnavK-09
Copy link
Contributor Author

Ques: is it ok to use RoleSelectorMenu during /role setup to add roles? As it is the only supported way by discord to select role with ease!

Workflow:

  • /print setup
  • click on Add Role Selector Menu to select role
  • Triggers modal asking for role description (if not using reaction roles in print embed)
  • replies ephermaly asking to react with emote on message to choose that emote for role selection by user?

@ArnavK-09
Copy link
Contributor Author

ArnavK-09 commented Dec 13, 2023

Important

How to handle reacted emote on print setup role addition progress embed, that is not available to the client/bot (ie. Another guild emote)?

@Pkmmte
Copy link
Member

Pkmmte commented Dec 14, 2023

Ok so there may be some communication error
Are you saying to use reactions during /role setup to choose role emote
+OR
To use reactions in Print Message ?? So that members can select a role by reacting to emote?

Both, but the later is more important. I'm still willing to debate the former for mods/admins, but users should definitely be able to select a role via emotes.

Ques: is it ok to use RoleSelectorMenu during /role setup to add roles? As it is the only supported way by discord to select role with ease!

Yes, that is what I mentioned the role selector for. The workflow you described is good, minus the ephemeral part. The message need to be non-ephemeral because Discord does not allow reactions on ephemeral messages. Once reacted, the bot should also clean up that message.

How to handle reacted emote on print setup role addition progress embed, that is not available to the client/bot (ie. Another guild emote)?

Discord bots are like Nitro users in this regard. If an emote that the bot does not have access to is used, it should remove it automatically as it is not supported. The bot will not be able to print it if it does not have access to it anyway.

Similarly, if a normal user reacts to the printed message with an emote that is not one of the options, the bot should automatically remove it. I believe MEE6 works similarly here.

@Nazeofel
Copy link
Contributor

Nazeofel commented Nov 1, 2024

Hello ! I would like to get back to you on that !, is the plugin done and working ATM, or did you stop working on it ? thanks so much, if you are still working on it, it can contribute to our hacktoberfest event !

@ArnavK-09
Copy link
Contributor Author

ArnavK-09 commented Nov 2, 2024

Hello ! I would like to get back to you on that !, is the plugin done and working ATM, or did you stop working on it ? thanks so much, if you are still working on it, it can contribute to our hacktoberfest event !

Although this plugin is ready from my side, but since pmktee asked for some modifications, that are still pending
And it's almost 1yr old & forget almost everything
So am in dilemma should I complete requested modifications or not

@Nazeofel
Copy link
Contributor

Nazeofel commented Nov 2, 2024

Only complete it if you feel like it, there is no rush nor any obligations ^^ we just had another user asking us if they could tackle the issue, but since you were already working on it I wanted to make sure it would not overlap yours, so just let me know (you have plenty of time to decide dw)

@ArnavK-09
Copy link
Contributor Author

Only complete it if you feel like it, there is no rush nor any obligations ^^ we just had another user asking us if they could tackle the issue, but since you were already working on it I wanted to make sure it would not overlap yours, so just let me know (you have plenty of time to decide dw)

Sure, i unassigned myself from this issue
Sorry i won't be able to work on it

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.

feat(plugin-roles): new plugin for assigning roles
3 participants