Discord Name:
PhenoM4n4n#0055
My nitro is expiring soon so my discriminator will be scrambled, I’m on both Red servers so finding me shouldn’t be that hard.
GitHub Repository (Must be V3):
Description:
I’m a beginner programmer, and these are my cogs. There may be some code that seems completely stupid or inefficient, and I’ll be happy to review a PR or issue for it. Here’s an overview of my cogs:
altdentifier
- Checks users using the AltDentifier API and allows for automatically taking actions against users that meet a specified Trust Factor
customping
- Provides a ping message that includes the bot, WebSocket, and host latency
disboardreminder
- Allows users to set a reminder to bump on Disboard - this bugs sometimes on bot restarts and I’m working on a fix
plaguegame
- Start a game in which users can infect/heal each other. This is my first cog in which I learned config, so some code may be jank
prefix
- Allows easier management of server prefixes, including adding and removing them
simplecalculator
- Just a basic calc
command
Hi PhenoM4n4n, thank you for your patience while I review your repo.
Commit hash at time of review - da3f4b334aae7d3070789d339924b5c833294757
altdentifier
altdentifier.py
- Line 3: You import asyncio but it is never used. (Optional)
- Lines 101-127: Although it’s unlikely you still need to check for hierarchy on the role being provided here. A moderator might have manage_guild perms in order to set this command and provide a role that they know is higher while also identifying on the third part API as an alt account potentially granting themselves more permissions than normal.
customping
customping.py
- You should avoid using
global
inside of cogs. (Optional) - You are also required to explain in your install message that this cog will replace cores ping command and ideally you let this be optional to replace cores ping with this one not force it to use this one. Ping was put on the bot to have a simple response that always works, making it require embed links is unexpected behavior for what is meant to be a simple response.
embedutils
embed.py
-
Lines 29-30: That is not the point of the data API. If you are storing EUD of any kind (defined by discord TOS for bot creators to include anything retrieved from discords API) it must be handled if you want to comply with the data API specifications. “keep users accountable for inappropriate embeds they make” is not your responsibility or bot owner responsibility if users are creating such things. The intent with the data API is to make it easier for you to remove data when discord has requested it about a specific deleted user. So if you’re going to have this function in here at all you must attempt to delete things stored by a user.
-
Line 286: Redefinition of
remove
you can’t have 2 of the same function name and the latter one will overwrite the previous one. -
Line 410: You’re better off storing the author ID here rather than the mention that way you can more easily retrieve the member object at a later date. (Optional)
linkquoter
linkquoter.py
- Line 45-46: You should add a check for
read_message_history
as well sincefetch_message
will fail without this permission. (Optional) - Line 171-172:
message.channel.permissions_for(message.guild.me).send_messages
is redundant with theawait self.bot.message_eligibile_as_command(message)
check. (Optional)
lock
converters.py
lock.py
- Lines 52, 55, 67, 99, 142, 154, and 190: You should not use operators on boolean values, use
if condition is not True
,if condition is False
,if condition
, orif not condition
instead. (Optional)
plaguegame
plague.py
- This cog should have an opt-in per server. Alternatively it needs to be mentioned in the install message that in order to disable it once it’s setup to unload the cog or reset the log channel and inform users that there’s no way for servers to opt out.
All of the requested changes have been addressed, and some of the optional ones will be addressed at a later date.
Thank you for making those changes. Marking this as approved!