Hi @Neuro_Assassin, I’ve been assigned to take over review of your application.
Commit hash: a916bbfdef96045f6af4be988639d929ceae5f34
Red v3.1.1
Consider anything prefaced with “You might want” to be optional
Color
-You do not have a class doc string, so [p]help Color does not have a cog description.
-You might want to limit the number of decimals in your floats to make the numbers more readable.
-[p]color msgshort should either be disabled by default or there should be a warning in the install message that it starts off enabled.
-What [p]color msgshort does and how to trigger it needs to be explained better in the help text.
-If [p]color msgshort is enabled, using [p]color hex <hex value prefaced with #> will cause both the invoked command and [p]color msgshort to respond at the same time.
-If the bot does not have permission to send embeds, all of the commands will only send a square of the color. You need to handle that in a better way.
-The __author__ var does not have your current discrim (4227 vs 4779).
-The cog uses pillow, however it is not in your requirements in your info.json.
-As far as I can tell, rgb2hex is not a valid import from colour.
Commandchart
-You do not have a class doc string, so [p]help CommandChart does not have a cog description.
-The __author__ var does not have your current discrim (4227 vs 4779).
-You might want to use typing.Optional to allow users to see a certain number of messages w/o needing to specify the channel.
-The command errors if the bot does not have permission to send embeds:
"<datapath>\cogs\CogManager\cogs\commandchart\commandchart.py", line 105, in commandchart
em = await ctx.send(embed=e)
discord.errors.Forbidden: FORBIDDEN (status code: 403): Missing Permissions
Cooldown
-You do not have a class doc string, so [p]help Cooldown does not have a cog description.
-If a cooldown is removed from a cog that had a cooldown not set from this cog, it is not kept on a bot restart.
-You should explain that adding a cooldown will overwrite the current cooldown (if one exists).
-A negative value can be passed to the rate value of [p]cooldown add.
-You might want to handle a command with a value being passed to [p]cooldown add. Right now, running [p]cooldown add 1 1m global play foo locks play to 1 per 1 min, ignoring foo.
-You have an arg parser in converters.py, but it does not look like it is ever used.
-The group command does not have any checks, so the command shows up in [p]help for all users.
Deleter
-You do not have a class doc string, so [p]help Deleter does not have a cog description.
-The help text for [p]deleter channel is a bit confusing and does not have good grammar. I don’t understand what this command does just by looking at the help text. The command sets up a channel to have auto-deleting messages, however the text only talks about “a message”.
-The help text for [p]deleter remove has bad grammar, specifically the phrase “Helpful for when announcements that you want to stay in the channel”.
-You might want to specify that the wait in [p]deleter channel is in seconds.
-You might want to specify that the messages in [p]deleter remove should be a message id.
-A good percentage of messages seem to get through the deletion. It could be a race condition in the on_message, or something with rate limits.
-You should consider using async with when you are managing the self.conf.channel(channel).messages dict.
-Why does [p]deleter wipe require admin? Someone could already disable the deleter with the base perms, so why does this specific command have elevated perms?
-The background task errors out and stops working if a channel is deleted that has a message set to be deleted:
"<datapath>\cogs\CogManager\cogs\deleter\deleter.py", line 33, in background_task
m = await c.fetch_message(int(message))
AttributeError: 'NoneType' object has no attribute 'fetch_message'
Editor
-You do not have a class doc string, so [p]help Editor does not have a cog description.
-Passing a voice channel ID causes the command to error:
"<datapath>\cogs\CogManager\cogs\editor\editor.py", line 41, in editmessage
editmessage = await editchannel.fetch_message(editid)
AttributeError: 'VoiceChannel' object has no attribute 'fetch_message'
-Passing a category channel ID causes the command to error:
"<datapath>\cogs\CogManager\cogs\editor\editor.py", line 41, in editmessage
editmessage = await editchannel.fetch_message(editid)
AttributeError: 'CategoryChannel' object has no attribute 'fetch_message'
-Attempting to edit a message that is not by the bot AND specifying content (instead of a message to copy from) causes the command to error:
"<datapath>\cogs\CogManager\cogs\editor\editor.py", line 65, in editmessage
await editmessage.edit(content=content, embed=None)
discord.errors.Forbidden: FORBIDDEN (status code: 403): Cannot edit a message authored by another user
-Attempting to edit a message in a channel the bot cannot access causes the command to error:
"<datapath>\cogs\CogManager\cogs\editor\editor.py", line 41, in editmessage
editmessage = await editchannel.fetch_message(editid)
discord.errors.Forbidden: FORBIDDEN (status code: 403): Missing Access
Evolution
-You should warn the user that the cog requires the bank to be global sometime before the cog load.
-[p]evolution backyard 1 throws an error because you defined the paramater name as menu, overriding the menu var provided by red:
"<datapath>\cogs\CogManager\cogs\evolution\evolution.py", line 260, in backyard
await menu(ctx, embed_list, DEFAULT_CONTROLS)
TypeError: 'bool' object is not callable
-[p]evolution backyard errors if the bot does not have permission to send embeds:
"<datapath>\cogs\CogManager\cogs\evolution\evolution.py", line 272, in backyard
await ctx.send(embed=embed)
discord.errors.Forbidden: FORBIDDEN (status code: 403): Missing Permissions
-[p]evolution shop errors if the bot does not have permission to send embeds:
"<datapath>\cogs\CogManager\cogs\evolution\evolution.py", line 240, in shop
await menu(ctx, embed_list, controls)
discord.errors.Forbidden: FORBIDDEN (status code: 403): Missing Permissions
-L318, you have a hardcoded animal name of “chicken”.
ListPermissions
-You do not have a class doc string, so [p]help ListPermissions does not have a cog description.
-You might want to allow role IDs to be used instead of just names.
-There are better ways to handle the places you sent two messages “in case the x name is really long”, such as pagify.
Maintenance
-You do not have a class doc string, so [p]help Maintenance does not have a cog description.
-You might want [p]maintenance deleteafter to require a value to be passed, setting it to 0 when no value is passed is a little confusing.
-[p]maintenance settings says that messages will be deleted after 0 seconds instead of saying that they will not be deleted.
-Negative values can be passed to [p]maintenance deleteafter
-An incredibly long message value in [p]maintenance message causes [p]maintenance settings to error:
"<datapath>\cogs\CogManager\cogs\maintenance\maintenance.py", line 126, in settings
return await ctx.send(sending)
discord.errors.HTTPException: BAD REQUEST (status code: 400): Invalid Form Body
In content: Must be 2000 or fewer in length.
-If any users are whitelisted, [p]maintenance settings errors:
"<datapath>\cogs\CogManager\cogs\maintenance\maintenance.py", line 134, in settings
user_profile = await self.bot.get_user_info(user)
AttributeError: 'Red' object has no attribute 'get_user_info'
-You might want to make it more clear that [p]maintenance whitelist only affects the current maintenance.
-[p]maintenance whitelist allows you to “add” users to the whitelist even though users should not be able to be added to the whitelist unless the bot is in maintenance since it only affects the current maintenance.
-You might want the error messages of [p]maintenance off and [p]maintenance on to be the same. Right now, only [p]maintenance on tells the user about [p]maintenance off.
Minesweeper
-You do not have a class doc string, so [p]help Minesweeper does not have a cog description.
-The __author__ var does not have your current discrim (4227 vs 4779).
-The help text of the two commands have bad grammar, specifically the phrase “with allowing you to”.
-The full help text of spoilerms is longer than 256 characters, so it is cut off. (Note: v3.1.2 removes the help text limit, but you still should probably do something about the size of this help text)
-You might want to say how many bombs are actually in a game, especially when it is randomly picked so user will know how many there are to know when they’ve “won”.
-You might want to silently ignore bad inputs in minesweeper so that the user can still chat while playing.
-An error is printed for every invalid space-separated guess in minesweeper, which can cause spam or hit rate limits.
-The board is edited for every space-separated guess in minesweeper, which can cause spam or hit rate limits.
-You need to put your warnings under the install_msg of the info.json.
Simon
-The __author__ var does not have your current discrim (4227 vs 4779).
-You have a group command with only 1 subcommand in a cog that hasn’t been updated for 2 months. I doubt you are going to add more subcommands, so why a group command?
-There are a lot of reasons why I don’t think this game is able to work in discord:
—The user has a text box to type the sequence as it appears.
—The rate limits make some of the “flashes” last for over 5 seconds.
—If the same number “flashes” twice in a row, the blank spot in between “flashes” can be incredibly short, making it hard to tell that it “flashed” again at all.
-In a normal game of simon, the sequence “builds” on top of the beginning sequence. In your game, each sequence seems completely random.
-Bad grammar in L53 (“due to no response for”).
-You might want to print the score if someone reacts with the X emoji.
-You only wait for reactions when waiting for the green check, X only works in between rounds.
SQL
-You do not have a class doc string, so [p]help Sql does not have a cog description.
-The __author__ var does not have your current discrim (4227 vs 4779).
-The group command [p]sql settings does not have any checks, so the command shows up in [p]help for all users.
-L1212, you do not have a catch for asyncio.TimeoutError
I didn’t review this cog in full for a few reasons:
1] I do not know SQL and do not feel confident reviewing it without understanding the basics of what it is doing and what it can do.
2] I’m not even sure what the use case is supposed to be, can you please explain what someone (or you) can use this cog for?
SW
-You do not have a class doc string, so [p]help SW does not have a cog description.
-The all commands do not seem to be sorted in any particular way, making them a bit more confusing.
-You might want to replace the “Hold on” messages with ctx.typing().
-You might want to add searching by name instead of just the id system.
-You redefine the built in next a couple of times.
-The commands all error if the bot does not have permission to send embeds:
"<datapath>\cogs\CogManager\cogs\sw\sw.py", line 187, in film
await menu(ctx, embeds, DEFAULT_CONTROLS)
discord.errors.Forbidden: FORBIDDEN (status code: 403): Missing Permissions
Switcher
-You do not have a class doc string, so [p]help Switcher does not have a cog description.
-You might want to restrict this cog to DMs with @commands.dm_only() and add a warning in the install message that the cog is DM only.
-You might not want the [p]switcher list code block to have python syntax highlighting.
Targeter
-You do not have a class doc string, so [p]help Targeter does not have a cog description.
-You might want to have alternate outputs, such as different file formates or an IDs only option.
-You might want to allow certain params to be used without values. For example, --not-nick should be able to used with no params to see all members w/o a nick. Right now you have to pass a fake “nick” for it to test against for that to work.
-The date parser seems to be a bit out of whack. I ran [p]target --joined-after 2017 50 01 and it saw that as a valid date, returning everyone in my server.
-Quote parsing is broken.
—You cannot search for users with a name/nick that has a quote in it.
—You cannot have a one-word argument surrounded in quotes.
—" x" is valid, however "x " is not.
—I’m not particularly sure why the embed colors go 0: Red, 1-499: Green, 500-999: Orange, 1000+: Red.
-Commands can be used in DMs, however trying to target anyone raises an error:
"<datapath>\cogs\CogManager\cogs\targeter\targeter.py", line 317, in lookup
matched = ctx.guild.members
AttributeError: 'NoneType' object has no attribute 'members'
-The command errors if the bot does not have permission to send embeds:
"<datapath>\cogs\CogManager\cogs\targeter\targeter.py", line 644, in target
await ctx.send(embed=embed)
discord.errors.Forbidden: FORBIDDEN (status code: 403): Missing Permissions
Twenty
-The __author__ var does not have your current discrim (4227 vs 4779).
-You have a group command with only 1 subcommand in a cog that hasn’t been updated for 3 months. I doubt you are going to add more subcommands, so why a group command?
-You are unable to make a move in the same direction as your last move, even if that move is valid because of the last spawned number.
-If there are two valid sets in a row or column, only one is combined (2 2 2 2 becomes X 2 2 4).
-You might want to keep track of the player’s score (like in the actual game).
-The loss message is “On no…” instead of “Oh no…”.
-After the game is lost, the player still has to react one more reaction (that wasn’t the one they did last) in order for the game to end since the logic for that is after the wait_for.
-L31: start is defined yet never used.
UpdateChecker
-You do not have a class doc string, so [p]help UpdateChecker does not have a cog description.
-The warnings in your install message are wrong, the command is not [p]update all.
-You might want to make [p]cogupdater task a hidden command.
-You might want to revert what black did to your config identifier.
-Right now, if the channel does not exist the bot will message the owner that the channel does not exist, however it does not send the update message to the owner.
-The task will error if the bot does not have permissions to send embeds but embed mode is on. You might want to add a catch when sending the embed.
-The group command does not have any checks, so the command shows up in [p]help for all users.
-Trying to run [p]cogupdater all without downloader loaded causes and error:
"<datapath>\cogs\CogManager\cogs\updatechecker\updatechecker.py", line 204, in _update_all
await ctx.invoke(cog._cog_update)
AttributeError: 'NoneType' object has no attribute '_cog_update'