[REJECTED] Scarlet-Cogs

Applications
#1

ScarletRaven#0546

General command enhancements and extensions

#2

Just wanted to let you know that your titlecase and capspam cogs looks like they will loop trying to delete the message for every prefix the bot has. Lines 39 and on for titlecase and 38 and on for capspam should probably be unindended once to prevent errors on servers with multiple prefixes.

#3

I have not done a full review of this yet, but a couple issues on the anti chat spam (title case, capspam)

Avoiding triggering this is as simple as

!THIS COG DOESN'T FILTER SO LONG AS THE MESSAGE STARTS WITH THE PREFIX

I’ve brought up a potential upstream solution for this particular problem, but until then, you can achieve more reliable detection of commands with:

if (await self.bot.get_context(message)).valid:
    return  # is prefix + command
#4

Change from above made

#5

Okay, full review here. I only have a couple things worth mentioning, after that, this should be good to go.

Starting with titlecase:

  1. Right now, users can use the titlecase to force the bot to mention @everyone, message is also deleted afterward, so this cog allows ghost pinging everyone.

This can be quickly remedied by using bot.send_filtered(m.channel, stuff_to_send) instead of m.channel.send(stuff_to_send)

Test string to accomplish this with: This@everyone Will Mention Everyone Which Is Bad

  1. There’s an empty settings group command, leaving the impression that there should be settings, but there are not.

  2. This should respect the autoimmune settings.

capspam

  1. Similar mention problem, here’s a string it can be forced with. Due to design choices I’ll bring up in more detail below, send it a few times.

THIS WILL MENTION@everyone WHICH IS BAD

  1. Similar settings (lack of, with a command group indicating the existence of settings) issue

  2. You are using what is essentially a global counter to determine if the bot should chime in with message removal, instead of a per member one. This is not a suitable design for general use.

  3. This should respect the autoimmune settings.

fun:

  1. you are tracking the number of times a command is used, but not using that stored info for anything.

Note: This cog is not substantive. While any potential issues are still going to be pointed out, this cog is literally just some hardcoded meme responses.

spice.

All the issues are in this block.

async def on_command(self, ctx):
    ...
    with open(bundled_data_path(self) / "commandspice.json") as f:
      for k, v in json.load(f).items():
        vchoice = random.choice(v)
        if (k in str(ctx.command)) & (vchoice != ""):
            await ctx.send(vchoice)
  1. loading the json on each command used, instead of storing the data in memory.
  2. you call random.choice for each command in the json data, rather than select the correct entry first.

These are both unneeded performance hits in what is essentially just extra flavor text on certain command usage.

#6

All of the mentioned suggestions have been pushed forward and revised.

Partial exceptions include:

Fun:

  1. Tracks number of times command is used.

While I could improve the tracking of each command, It’s not in-scope to permanently save. Right now it’s a fun counter to say “Neat, that one command has been used 5,392 times”.

#7

Re: fun command tracking: in that case it may be worth noting in the toggle’s help that the tracking is not persistent. (not a required change/fix)

Spice:
I don’t see any changes here, did you forget to push something?

titlecase: changes are :ok_hand:

capspam: changes just made the cog never take any action.

async def on_message(self, m: discord.Message):
    count = 0
    ...    
    if m.author.bot is False and trigger != "[]":
        count += 1
        if count > 2:
        ...    # unreachable code below.

This is an extremely pared down showing of the function here. Since these are the only places you use the variable, and none of the code snipped out causes this to iterate in any way, everything after that conditional can never happen. This was not what I was referring to by removing what was essentially a global counter here. You either need a per member persistent counter, or to ditch the counter entirely.

[Rejected-2] Scarlet-Cogs
#8
#9

Rejecting this after two weeks of inactivity. Feel free to re-apply in 2 weeks if you resolve the remaining issues.

#10