Skip to content

feat: add quoi-feur game with react #31

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

Closed
wants to merge 9 commits into from
Closed

Conversation

luca-montaigut
Copy link
Contributor

@luca-montaigut luca-montaigut commented Aug 2, 2023

Feature quoi-feur :

  • slash command /quoi-feur :
    • add => add game to current channel
    • remove => remove game from current channel

Le jeu :

  • detection of "quoi" at the end of the sentence

  • response in emoji (in 'react' for non-boomers), with a random draw:

    -> 19 on 20 : react 🇫 🇪 🇺 🇷

    -> 1 on 20 : react 🇨 🇴 🇺 🇧 🇪 🇭 🔇 and mute the author for 5 minutes

@luca-montaigut luca-montaigut force-pushed the quoi-feur branch 2 times, most recently from acce502 to 1d6dc49 Compare August 2, 2023 23:56
Copy link
Member

@neolectron neolectron left a comment

Choose a reason for hiding this comment

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

I like the idea, however we should be careful with the state of each features before merging.

@potb potb mentioned this pull request Aug 12, 2023
@luca-montaigut luca-montaigut force-pushed the quoi-feur branch 4 times, most recently from 250fb5e to 8e8836c Compare September 1, 2023 12:22
Co-authored-by: Peïo Thibault <[email protected]>
@luca-montaigut luca-montaigut force-pushed the quoi-feur branch 2 times, most recently from 08ae209 to d394298 Compare September 3, 2023 10:12
Copy link
Member

@neolectron neolectron left a comment

Choose a reason for hiding this comment

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

Good changes! Made me notice some of the limitation we have with the core as it is now.

Don't forget that any feature in addition of being stateless should work for any number of instance the bot is invited in.

@@ -1,6 +1,15 @@
const socialNetworksUrlRegex = new RegExp(
'^(https?://)?(www.)?(facebook.com|fb.me|twitter.com|vxtwitter.com|instagram.com|linkedin.com|youtube.com|youtu.be|pinterest.com|snapchat.com|tiktok.com)/[a-zA-Z0-9.-/?=&#_]+$',
);
const punctuationRegex = new RegExp(/[.,!?]/g);
const emojiRegex = new RegExp(/(\p{Extended_Pictographic}|\p{Emoji_Component})/gu);
const quoiDetectorRegex = new RegExp(/\b\s*[q][u][o][i]\s*$/i);
Copy link
Member

Choose a reason for hiding this comment

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

While punctuationRegex and emojiRegex can make sense as global helpers, I don't think there's a lot of use-cases for other modules to detect sentences that ends with quoi.

Can you move this inside your module helper please ?


export const removePunctuation = (text: string) => text.replaceAll(punctuationRegex, '');
export const removeEmoji = (text: string) => text.replaceAll(emojiRegex, '');
export const endWithQuoi = (text: string) =>
Copy link
Member

Choose a reason for hiding this comment

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

same as above - should be in module helpers.

)
.toJSON(),
handler: {
add: async (interaction) => {
Copy link
Member

Choose a reason for hiding this comment

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

You don't need to wrap this inside an arrow-function, you can pass directly addQuoiFeurToChannel to add.

Whenever a command throw/crash, it already prints out to the console as an error, so the .catch is unnecessary.

Same with remove below.

},
],
eventHandlers: {
ready: async (client) => {
Copy link
Member

Choose a reason for hiding this comment

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

Same as above, you should pass directly deleteRoleMutedByBot here.
(See other review to understand the signature change)

);
};

export const deleteRoleMutedByBot = async (guild: Guild | null): Promise<void> => {
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't depend on a specific guild but rather run for any guild the bot is in.

So I recommend to remove the role for every guild the bot is in.
(otherwise if you invite the same bot instance to another server it won't work)

I think you can change the signature of this and take a Client as input instead, so you can map it to the ready event directly.

(or even code it inline if you want)

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