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

Add induction command #13

Merged
merged 4 commits into from
Sep 5, 2022
Merged

Add induction command #13

merged 4 commits into from
Sep 5, 2022

Conversation

sapphyree
Copy link
Collaborator

As per @rainestormee's suggestion, adds an /induct command to automatically give a user the Guest role whilst sending a message in general to highlight the fact they've joined and to draw their attention to the new channels.

Copy link
Member

@MattyTheHacker MattyTheHacker left a comment

Choose a reason for hiding this comment

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

Personally I think it's better to have this done by automatically checking reacts but this works too

@sapphyree
Copy link
Collaborator Author

Personally I think it's better to have this done by automatically checking reacts but this works too

The problem with automatically checking reacts is that you're spending processing time constantly checking all of the messages in introductions. In addition, you're checking every user that reacts (because there's no way to just filter) and that takes up a lot of time checking the cache (or, in some cases, performaing an async fetch and re-checking the cache). This way is more annoying, yes, but it's easier to not have to worry about checking every message in the channel.

@MattyTheHacker
Copy link
Member

Very good points, can't argue with that

Copy link
Contributor

@LMBishop LMBishop left a comment

Choose a reason for hiding this comment

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

The only concern I have is that the welcome message could get spammy, especially during welcome week.

Also, not really that important, but something to consider is a logging library for less 'fragile?' (for lack of a better term) logs.

src/commands/makeMember.js Outdated Show resolved Hide resolved
src/events/interactionCreate.js Outdated Show resolved Hide resolved
src/commands/induct.js Outdated Show resolved Hide resolved
@LMBishop
Copy link
Contributor

LMBishop commented Sep 3, 2022

Oooo I've just had an idea, I know you've already implemented it like this, buuut I've seen an 'Apps' thing when you right click messages on Discord and I think it would be pretty cool to have one of those also trigger this

image

@MattyTheHacker
Copy link
Member

Oooo I've just had an idea, I know you've already implemented it like this, buuut I've seen an 'Apps' thing when you right click messages on Discord and I think it would be pretty cool to have one of those also trigger this

Seconded, that would be pretty cool

@MattyTheHacker
Copy link
Member

MattyTheHacker commented Sep 4, 2022

The only concern I have is that the welcome message could get spammy, especially during welcome week.

Could potentially add an option to the command where the user can specify whether they want it to send a message in general or not? Partially because I think it should DM them as well in all cases also to remind them to change their nickname and to grab roles etc...

https://github.com/CSSUoB/TeX-Bot/issues/37

#8

@sapphyree
Copy link
Collaborator Author

The only concern I have is that the welcome message could get spammy, especially during welcome week.

Could potentially add an option to the command where the user can specify whether they want it to send a message in general or not? Partially because I think it should DM them as well in all cases also to remind them to change their nickname and to grab roles etc...

https://github.com/CSSUoB/TeX-Bot/issues/37

#8

The DMing functionality is OoS for this PR and this command right now, though I was thinking of adding a silent Boolean option yes.

@JTWWilson
Copy link
Contributor

Re: @LMBishop's concerns about spamming, I think the best approach would be to enable the functionality and see how annoying it is. Anything else is conjecture and I don't think that enabling it for a short amount of time is much of an issue.

@sapphyree
Copy link
Collaborator Author

Re: @LMBishop's suggestion with the logging library - I agree, in the future I'd like to move to a better solution to logging events, but for right now I think it's probably fine? Feel free to open an issue and/or implement it yourself <3

@LMBishop
Copy link
Contributor

LMBishop commented Sep 5, 2022

Oh yeah of course, this PR is fine as is (aside from the typos). I'll raise the other issues separately when I get the chance

@sapphyree sapphyree merged commit 238b589 into main Sep 5, 2022
@MattyTheHacker MattyTheHacker deleted the add-induction branch September 5, 2022 13:32
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

4 participants