Discord Name: @sravan_krishna
GitHub Repository (Must be V3): GitHub - sravan1946/sravan-cogs: Some random cogs made by me for my bot made using Red-DiscordBot
Description:
The repo contains 8 cogs which includes a few games and other useful utilities.
Thank you for your patience.
Commit hash: b524fd6112abe540b1fdc3c81dbe78c444198251
Red v3.5.5
Consider only bullets prefixed with 
to be required.
General
while loops over the value of a variable, that you exit out of both by breaking and by setting the value of that variable to a falsey value. Both of these actions accomplish mostly the same thing, with the only difference being that break instantly breaks out, and setting a variable requires the current execution of the loop to finish before it will exit. Depending on how you want these loops to exit, you may want to refector to only use one of these options.from redbot.core import checks is deprecated, you can use @commands... instead of @checks....Acromania
[p]acromania has the word “play” in an in-line code block? It seems out of place, and somewhat implies that it is a command name (or similar), when it does not appear to be. It’s currently unclear how to enter a value to [p]acromania set guessingtime and votingtime. The docstrings should be updated with an explanation and/or examples of what to enter.[p]acromania set manager. [p]acromania start does not properly handle missing embed_links permissions:Traceback (most recent call last):
File "datapath\cogs\CogManager\cogs\acromania\acromania.py", line 142, in start
await ctx.send(embed=startem)
discord.errors.Forbidden: 403 Forbidden (error code: 50013): Missing Permissions
[p]acromania start does not properly handle missing add_reactions permissions:Traceback (most recent call last):
File "datapath\cogs\CogManager\cogs\acromania\acromania.py", line 217, in start
await vote.add_reaction("✅")
discord.errors.Forbidden: 403 Forbidden (error code: 50013): Missing Permissions
[p]acromania set manager, as the m.append and m.remove calls happen outside of the context manager where the config value can be modified. As a result, the state is not actually changed.Aki
N/A
Altdentifier
[p]altcheck does not properly handle missing embed_links permissions:Traceback (most recent call last):
File "datapath\cogs\CogManager\cogs\altdentifier\altdentifier.py", line 117, in altcheck
await ctx.send(embed=e)
discord.errors.Forbidden: 403 Forbidden (error code: 50013): Missing Permissions
[p]altset settings and the on_member_join listener both also have this issue. [p]altset settings does not properly handle guilds without a custom icon:Traceback (most recent call last):
File "datapath\cogs\CogManager\cogs\altdentifier\altdentifier.py", line 146, in settings
e.set_author(name=ctx.guild, icon_url=ctx.guild.icon.url)
AttributeError: 'NoneType' object has no attribute 'url'
[p]altset settings stores description as a list, .appends a single item to it, then .joins. It seems like this was intended to be a loop over multiple possible channels or settings, however that’s not how it’s currently being used.[p]altset channel checks that the bot has send_messages permissions in the channel twice. This probably was intended to check if the bot has embed_links permissions instead. Even if that is checked here, you still should check the bot still has that permission in the listener itself to cleanly handle errors.[p]altset whitelist the same user multiple times, which will require them to be unwhitelisted that many times to actually be removed from the whitelist. You should probably check if the user is already on the whitelist before adding them again.member_has_default_avatar appears to never be used. The cog should not advertise itself as using the Altdentifier API if it is not actually using it. It should be clearer that it calculates whether a user is an alt purely using the account age.Dps
[p]dps staffrole add the same role multiple times, which will require them to be removeded that many times to actually be removed.
role == config happens on L401, instead of role in config.[p]dps message to a very long string breaks [p]dps settings:Traceback (most recent call last):
File "datapath\cogs\CogManager\cogs\dps\dps.py", line 510, in settings
await ctx.send(embeds=embeds)
discord.errors.HTTPException: 400 Bad Request (error code: 50035): Invalid Form Body
In embeds.0.fields.6.value: Must be 1024 or fewer in length.
[p]dps addroles roles don’t appear in [p]dps settings.action command, and to instead allow users to either leave these role buckets empty to not use them, or fill them to potentially combine them. You would also need to add toggles for ban/kick individually.per and amount settings?await self.bot.get_embed_color(ctx), consider await ctx.embed_color().allowed_in_channel typehints a Context object, yet gets a Message. Setting [p]dps scope to anything other than guild and sending any message in a channel that is not inside of a category causes the following error in console:Traceback (most recent call last):
File "datapath\cogs\CogManager\cogs\dps\dps.py", line 612, in on_message
if not await self.allowed_in_channel(message):
File "datapath\cogs\CogManager\cogs\dps\dps.py", line 593, in allowed_in_channel
if channel.category.id in categories:
AttributeError: 'NoneType' object has no attribute 'id'
check_ping function. L636 loops over all mentions within a message. L675 breaks out of this loop at the end of processing a mention. Since there are no continues in the loop to bypass this break and loop again, this means that exactly 1 mention will be processed per message, with the rest ignored. Pinging a non staff member first, followed by a member of the staff role, bypasses this cog’s expected logic.
return is used if a ping matches the author’s own ID, rather than just continueing to check the next mention.Member objects, L638 looping over their ids, and L639 re-fetching the Member object again.1 ping requires 2 pings to actually trigger the action, since the L644 cache miss handling does not actually check if the ping count requirement is met.ForceMention
mention_everyone permission. They cannot mention a role that allows anyone to mention it, unlike normal users. The current logic to briefly allow the role to be mentionable was for older discord behavior, and no longer has the desired effect.
mention_everyone permissions could not override the mentionable setting, however that is no longer an issue. This cog already requires that permission to be able to use it, so it isn’t really giving any extra functionality.allowed_mentions) which may not be covered by existing say cogs. If that’s the goal, there’s no need for the role.edit calls, and the branding of the cog should probably be updated.Gtn
5-5) fails claiming the first is higher than the second. [p]gtn does not properly handle missing embed_links permissions:Traceback (most recent call last):
File "datapath\cogs\CogManager\cogs\gtn\Guessthenumber.py", line 109, in gtn
starting_message = await ctx.message.reply(embed=startem)
discord.errors.Forbidden: 403 Forbidden (error code: 50013): Missing Permissions
check function does not appear to be used.isdigit is not sufficient to check if int will raise a ValueError. Many number-like symbols pass isdigit but cannot be inted, like ²:Traceback (most recent call last):
File "datapath\cogs\CogManager\cogs\gtn\Guessthenumber.py", line 91, in gtn
_range = await self.get_values(ctx, user)
File "datapath\BOT\cogs\CogManager\cogs\gtn\Guessthenumber.py", line 65, in get_values
if int(_range[0]) < int(_range[1]):
ValueError: invalid literal for int() with base 10: '²'
get_values function nests ifs inside of each other and each fail state makes the bot send the same Could not start the gtn event message. It would be easier to read if you exited early in the failure cases, and had the caller send the overall failure message if it detects a failure for any reason.HidePing
It is currently possible to make this cog error from a message over 2000 characters without it being caught by the safeguard:Traceback (most recent call last):
File "datapath\cogs\CogManager\cogs\hideping\hideping.py", line 42, in hideping
await ctx.send(msg)
discord.errors.HTTPException: 400 Bad Request (error code: 50035): Invalid Form Body
In content: Must be 2000 or fewer in length.
||ZWSPACE would be more readable if the control character was entered with a unicode escape sequence and the repeats were instead string multiplication, ie:msg = f"{message} {'||\u200d' * 598} <@!{member.id}>"
Perform
[p]performapi / [p]performstats error if the bot does not have embed_links permissions:Traceback (most recent call last):
File "datapath\cogs\CogManager\cogs\perform\perform.py", line 1030, in performapi
await ctx.send(embed=embed)
discord.errors.Forbidden: 403 Forbidden (error code: 50013): Missing Permissions
content defined, but the embed still does not appear.Poll
[p]poll expects a | between the question and first option. A custom usage= or a usage example would help. [p]poll / [p]quickpoll error if the bot does not have embed_links permissions:Traceback (most recent call last):
File "datapath\cogs\CogManager\cogs\poll\poll.py", line 85, in poll
e = await ctx.send(embed=embed)
discord.errors.Forbidden: 403 Forbidden (error code: 50013): Missing Permissions
[p]poll / [p]quickpoll error if the bot does not have add_reactions permissions:Traceback (most recent call last):
File "datapath\cogs\CogManager\cogs\poll\poll.py", line 87, in poll
await e.add_reaction(EMOJIS[i])
discord.errors.Forbidden: 403 Forbidden (error code: 50013): Missing Permissions
[p]poll / [p]quickpoll error if the question is longer than 256 characters (the limit for embed titles):Traceback (most recent call last):
File "datapath\cogs\CogManager\cogs\poll\poll.py", line 53, in quickpoll
e = await ctx.send(embed=embed)
discord.errors.HTTPException: 400 Bad Request (error code: 50035): Invalid Form Body
In embeds.0.title: Must be 256 or fewer in length.
ziping with a list of length 11. Since zip uses the smaller size between the two inputs, it will always be scaled to have no more than 10 choices.Prefix
N/A
Session
[p]session type’s fail message claims 1 is a valid option instead of 2.r"") on L149 for the regex pattern, to avoid console errors warning about the \d. It also avoids accidentally using a python escape when you mean to use a regex escape or literal.self.commands dict, creating a string of sorted and newline-separated counts, then using regex to parse and sum those counts into the final sum s. s = sum(map(lambda a: a["count"], commands.values())) should give the same result in far fewer steps.
command name -> count map for anything besides determining the final count, so you could also consider replacing the map with a simple int count to further reduce the complexity.Timeout
[p]timeout and [p]untimeout raise an error if the bot does not have moderate_members permissions:Traceback (most recent call last):
File "datapath\cogs\CogManager\cogs\timeout\timeout.py", line 166, in timeout
await self.timeout_user(ctx, member_or_role, time, reason)
File "datapath\cogs\CogManager\cogs\timeout\timeout.py", line 66, in timeout_user
await member.timeout(time, reason=reason)
discord.errors.Forbidden: 403 Forbidden (error code: 50013): Missing Permissions
Hey Flame, Thanks for the review.
I have made all the required changes and have fixed most of the general points too.
The per and amount values were fetched multiple times during execution so i believed it would be better idea to keep cache instead of making config calls
This cog provides utility commands for other cogs like tags which cant mention roles by itself. The role is edited and made mentionable only if the bot didnt have the mention_everyone permissions.
Do let me know if there are any more changes required or if i made any mistakes while fixing the issues
Alright, everything looks all set now. Marking this as approved!
A few remaining notes I have are listed below.
Commit hash: c3c90b7bf829f197b75908d519bea4f87d5bcd5c
Red v3.5.5
Altdentifier
channel.permissions_for(guild.me).embed_links instead of guild.me.guild_permissions.embed_links to avoid missing channel overrides. The error is caught by the Forbidden check, but that also clears the channel from config so it has slightly different behavior.[p]altcheck command to error on L254 after about a minute. The error is due to raise APIError being used without constructing the APIError object, causing a TypeError due to missing __init__ args.Traceback (most recent call last):
File "datapath\cogs\CogManager\cogs\altdentifier\altdentifier.py", line 113, in altcheck
trust = await self.alt_request(member)
File "datapath\cogs\CogManager\cogs\altdentifier\altdentifier.py", line 254, in alt_request
raise APIError
TypeError: __init__() missing 2 required positional arguments: 'response' and 'message'
Poll
[p]poll / [p]quickpoll error if the question is between 252 and 256 characters, as you check the length before doing processing on the string (bolding it).