obsproject / obs-bot

Source code for the obsproject Discord bot
https://discord.gg/obsproject
GNU General Public License v3.0
38 stars 18 forks source link

Add slowmode command #8

Open RytoEX opened 3 years ago

RytoEX commented 3 years ago

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:

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

Checklist:

derrod commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

derrod commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

RytoEX commented 3 years ago

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.

derrod commented 3 years ago

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

RytoEX commented 3 years ago

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

Done.

RytoEX commented 3 years ago

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 commented 3 years ago

Silent ignore is fine,

RytoEX commented 3 years ago

Silent ignore is fine,

Done. This should address all current feedback.