I received a request to give this another look (via discord DM). I’m moving this to a definitive, but not indefinite “No”. There is not enough up to current standards to put this in the approved cog repository list at this time. At such time when that has changed, you may reapply.
I’ll note a handful of things below which were noticed as I was going through what has changed since being added (as well as noting a few things which hadn’t)
Let’s start with the previous things left unaddressed:
Twenty and simon both still work, and both are accompanied by warnings about potential ratelimits, but neither were changed to address the issue of multiple games for what are ostensibly to be used by multiple people (otherwise, why discord, instead of a dedicated app?
This is not what I meant with needing to handle that. While it technically does in most cases, you’re done this in an incredibly inefficient manner that still doesn’t address that the site you are using for this does not work with arbitrary sites, but a specific list of services.
This is a good case for working with
urllib or a regex, then confirming the service inquired about is included in what are tracked on
Okay, with that said, here’s what I noticed in what is new:
Help for this is incredibly unhelpful:
missing docstrings, shadowing the python builtin
set in the settings command.
Ambiguous user facing variables in command signatures.
Attempts to match things which cannot exist in names:
(you’re attempting to match
Method of matching is poor for the intent, and is potentially paired with automated kick or ban.
This cog with just set to “auto” paired with an action of “ban” would ban people with any of the following names:
“.com bubble burster”, “Free Kim .com”, “polka.organizer”
None of which match the intent (a url in the name).
It’s also matching a fixed set of strings by iterative comparison to
member.name.lower() and does not cache the
.lower() per iteration.
This is again, another good case for regex use.
every time you look at an action type, you are using
.lower() and a string comparison, instead of storing it lowered to begin with.
You should use the built-in handling for boolean types in the commands extension for true/false handling for consistency with other commands in core red.
This is a modification to someone else’s cog. You acknowledged that, and I don’t have a problem with that, but the modifications here are not done well enough to hold up to scrutiny.
- If invoked with mentioning the bot, there will be no feedback due to a misuse of
- This only matches the current prefix, and only if the current prefix isn’t modified by clean_content. (see #1)
- This doesn’t accurately match commands invoked with an alias.
This entire block can be replaced with:
async for msg in channel.history(limit=number, after=datetime_object):
message_context = await self.bot.get_context(msg)
And it would be both more accurate, and faster.