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 slowmode command #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

RytoEX
Copy link
Member

@RytoEX RytoEX commented Sep 7, 2021

Description

Add the ability to edit a channel's Slowmode delay.

Note: This requires the bot to have the "Manage Channels" permission on the server.

This uses the Discord Slowmode setting for a channel. This setting is enforced per-user per-channel according to how the Dyno documentation describes Discord's Slowmode:

Discord Slowmode activates the native Discord slowmode, which is similar to User Slowmode but it works through the Discord app to prevent messages violating the slowmode from being sent, whereas Dyno slowmode (user & channel) will delete any messages that violate the slowmode. This has a max time of 21600 seconds (6 hours).

References:

Thoughts for the future:

  • Add a help item for this command?
  • Slash command?
  • Edit Slowmode for a specified channel instead of current channel?

Motivation and Context

Give us another tool to use to quickly address mass flooding/spam without relying only on Dyno Premium (or Dyno itself) or having to hand out the Manage Channel permission to bot admins or whoever else should have access to this feature.

How Has This Been Tested?

Tested on a test server with a locally hosted build of the bot.

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@derrod
Copy link
Member

derrod commented Sep 8, 2021

Two things:

  1. Could you also add the command the help text?
  2. You could set a default value of "0" for slowmode, causing inoking of the command without a specified time to disable slowmode

Edit: I haven't written commit guidelines for this repo, but the commit title format here generally is package.name: Message (ignoring public for cogs) so for this it would be cogs.admin: Add slowmode command.

@RytoEX RytoEX force-pushed the add-slow-mode branch 2 times, most recently from 2262eaa to a07176b Compare September 8, 2021 13:31
@RytoEX
Copy link
Member Author

RytoEX commented Sep 8, 2021

Two things:

1. Could you also add the command the help text?

2. You could set a default value of "0" for slowmode, causing inoking of the command without a specified time to disable slowmode

Edit: I haven't written commit guidelines for this repo, but the commit title format here generally is package.name: Message (ignoring public for cogs) so for this it would be cogs.admin: Add slowmode command.

Done.

Do we need any sanity checks on the seconds parameter? Like clamping the seconds at the max possible value (21600) or making sure the parameter is actually an int? I forget if Python will handle the latter automatically.

@RytoEX
Copy link
Member Author

RytoEX commented Sep 8, 2021

Well, it seems we should definitely either clamp seconds at 21600 or warn if the value is invalid. Invalid values cause exceptions. Preference?

@derrod
Copy link
Member

derrod commented Sep 8, 2021

As a note (because flake8 is going to complain about this), there should be a space between the assignment operator in the parameters, e.g. int = 0.

@RytoEX
Copy link
Member Author

RytoEX commented Sep 8, 2021

As a note (because flake8 is going to complain about this), there should be a space between the assignment operator in the parameters, e.g. int = 0.

I'll rebase shortly. Was working on the clamping/error-handling code to prevent exceptions. Thoughts on that, by the way?

@derrod
Copy link
Member

derrod commented Sep 8, 2021

Not many, ideally just respond with an error if anything fails.

@RytoEX
Copy link
Member Author

RytoEX commented Sep 8, 2021

Not many, ideally just respond with an error if anything fails.

I can either clamp:

# Clamp to the min value of 0 seconds (disabled, no delay)
if seconds < 0:
    seconds = 0

# Clamp to the max value of 21600 seconds (6 hours)
if seconds > 21600:
    seconds = 21600

Or just return error messages. I'm fine with either. I think I prefer clamping, because if a channel is being flooded, you might not notice the error message.

@RytoEX RytoEX force-pushed the add-slow-mode branch 2 times, most recently from cf05b04 to 9ddda16 Compare September 8, 2021 14:40
@derrod
Copy link
Member

derrod commented Sep 8, 2021

Oh yeah, also use ' for strings rather than " to stay in line with the rest of the code.

@RytoEX
Copy link
Member Author

RytoEX commented Sep 8, 2021

Oh yeah, also use ' for strings rather than " to stay in line with the rest of the code.

Done.

@RytoEX
Copy link
Member Author

RytoEX commented Sep 8, 2021

In its current form, this clamps negative int parameters to 0, and clamps anything higher than 21600 to 21600.

If a user specifies a non-integer to the command, it will result in this exception:

discord.ext.commands.errors.BadArgument: Converting to "int" failed for parameter "seconds".

This appears like we would need to handle it in main.py in on_command_error. Do we want to ignore the exception there, print a warning message, or handle it some other way?

@derrod
Copy link
Member

derrod commented Sep 8, 2021

Silent ignore is fine,

Add the ability to edit a channel's slowmode delay.
@RytoEX
Copy link
Member Author

RytoEX commented Sep 8, 2021

Silent ignore is fine,

Done. This should address all current feedback.

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