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

[Gallery] Bug fix, Feature Update #228

Merged
merged 3 commits into from
Jan 10, 2025
Merged

Conversation

cool-aid-man
Copy link
Contributor

@cool-aid-man cool-aid-man commented Jan 7, 2025

Hi Kreu,
Noticed this bug today so decided to pr the changes:

  • Bug fix - where the cog was listening to bots messages causing it to fall in a potential loop of 'deleting-sending-deleting'.
    • (For example: In case of a sticky msg in the 'set gallery' channel the cog would keep on deleting the sticked message while the sticky cog would keep on sending a new message)
    • Bots are Ignored now.
  • The galleryset add and galleryset remove commands now accept multiple channel as arguments.
  • Removed colons from : settings embed making it more user-readable.
  • Added a few aliases: showsettings, show, setting (Hope its okay )
  • Bumped the version

Type

  • Bugfix
  • Enhancement
  • New feature
  • Documentation

- Bug fix - where the cog was listening to bots messages causing it to fall in a potential loop of 'deleting-sending-deleting'.
  - (For example: In case of a sticky msg in the 'set gallery' channel the cog would keep on deleting the sticked message while the sticky cog would keep on sending a new message)
  - **Bots are Ignored now**.
- The `galleryset add` and `galleryset remove` commands now accepts **multiple channel** as arguments.
- Removed colons from `:` settings embed making it more user-readable.
- Bumped the version
@cool-aid-man cool-aid-man requested a review from Kreusada as a code owner January 7, 2025 12:28
Copy link
Owner

@Kreusada Kreusada left a comment

Choose a reason for hiding this comment

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

This is great! Thanks very much for making those changes.

One small thing, a bit of a nitpick, could the added VS already added messages be sent together, with a line break or something? Sending two messages at once isn't necessary and may cause unexpected behavior if it interferes with certain listeners with cooldown functionality. I'm referring to:

        if added_channels:
            await ctx.send(f"{', '.join(added_channels)} added to the Gallery channels list.")
        if already_in_list:
            await ctx.send(f"{', '.join(already_in_list)} already in the Gallery channels list.")
        if removed_channels:
            await ctx.send(f"{', '.join(removed_channels)} removed from the Gallery channels list.")
        if not_in_list:
            await ctx.send(f"{', '.join(not_in_list)} not in the Gallery channels list.")

- Added sending response in one message instead of sending multiple.
- `galleryset role` will mention the role as `<@&ID>` instead of role_name.
  - No change in settings embed, it'd still show the roles in **name** format (instead of mention) as mentioning roles require the user-client to be cached, before showing up properly.
- Slight adjustment to the regex:
> Previous: `[!*\(\\),]`
> New:         `[!*\\(\\),]`
 - This portion of the regex matches the literal characters: `!` `*` `(` `)` `,` - where the 1st parenthesis `(` wasn't properly escaped with `\\`
@cool-aid-man
Copy link
Contributor Author

Oh yes! absolutely,
You're right sending two messages at once isn't require atall when it could easily be send in one message.

Done.

  • Added sending response in one message instead of sending multiple.
  • galleryset role will mention the role as <@&ID> instead of role_name.
    • No change in settings embed, it'd still show the roles in name format (instead of mention) as mentioning roles require the user-client to be cached, before showing up properly.
  • Slight adjustment to the regex:

Previous: [!*\(\\),]
New: [!*\\(\\),]

  • This portion of the regex matches the literal special characters: ! * ( ) , where the 1st parenthesis ( wasn't properly escaped with \\

Copy link
Owner

@Kreusada Kreusada left a comment

Choose a reason for hiding this comment

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

LGTM, thanks very much for contributing!

@Kreusada Kreusada merged commit 8906384 into Kreusada:master Jan 10, 2025
1 check failed
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.

2 participants