[APPROVED] PCXCogs

Applications
#1

Discord Name:
PhasecoreX#0635

GitHub Repository (Must be V3):

Description:
My cogs focus on automation, where the bot will automatically respond or perform actions when needed, even if commands are not used. For instance, we have BanCheck, which checks global ban lists when a new user joins a server. There’s also DecodeBinary, which will automatically convert binary strings it detects in chats into human readable text. Finally there’s RemindMe, which is a port and enhancement of the Twentysix26 cog of the same name.

I developed DecodeBinary and a few other non-listed ones originally for v2 and for personal use. I’ve spent a bit of time converting and figuring out the code style of the new v3 bot. So far I’m loving it! Much better than v2, especially with how much less boilerplate code is now needed (config files especially).

#2
#3

Hey, thank you for being patient while we get to your application.

Commit hash at time of review 858c965bf83323185ad8a81e2b5d969fb260342b.

BanCheck -

  • info.json
    Author is now a list of strings, reference.
  • bancheck.py
    Line 46-48 are unneccesary since you already have the channel object stored.(Optional)
            channel = self.bot.get_channel(
        await self.config.guild(ctx.message.guild).channel()
        )

DecodeBinary -

  • info.json
    Author is now a list of strings.
  • decodebinary.py
    It’s possible for someone to post 01000000 01100101 01110110 01100101 01110010 01111001 01101111 01101110 01100101 in chat and have the bot mention everyone it might be worth checking the authors permissions for mentioning everyone or using bot.send_filtered to avoid mass mentions.

You may consider changing your regex pattern with [01]{8} to only find matching 8 length sets of binary or [01]{8,} to ensure at least 8 digits of 1 and 0 are present. This would help reduce some later checks in the code and is entirely optional with the current implimentation. (Optional)

Dice -

  • info.json
    Author is now a list of strings.
  • dice.py
    There should be further warnings for the bow owner to not set a maximum amount of sides or rolls too high as it can crash the bot. You have checks in place to prevent users from doing unsafe things but allow the bot owner to bypass those checks.

RemindMe -

  • info.json
    Author is now a list of strings.

Wikipedia -

  • info.json
    Author is now a list of strings.
#4

All files have been updated:

  1. All the info.json files have been updated to have the author be a list of strings.
  2. Removed that extra code from bancheck.py.
  3. Never knew about bot.send_filtered, pretty cool. Updated decodebinary.py to use it.
  4. Originally the regex was that way so that it could support binary with spaces in it. I have updated it so that there’s less code checking if it’s valid binary.
  5. I did some testing, and it turns out that you can set the max sides to a huge number and it’s still instant when processing. Setting max rolls to anything over one million however seemed to slow stuff down. I have added warnings for this, and with large enough numbers, an extra in-your-face warning. I’ve also reduced memory consumption if there’s over 750 rolls performed at once.

These are all ready to be re-checked. Thanks for checking these out!

#5

Hey thanks for making these changes.

Couple things I noticed in Dice you have the warning as an embed which will error out entirely if a user tries to set the dice higher than 1000000 in a channel where the bot doesn’t have embed permissions. This can be a good place to use await ctx.maybe_send_embed("Your warning message."). It won’t look as nice as your embed but may help prevent confusion.

I was half expecting it to not let me add more than 1000000 dice roles after looking at the code and was surprised to see it still set after the warning was displayed this could be a good place to return or provide another confirmation message using MessagePredicate like:

from redbot.core.utils.predicates import MessagePredicate
import asyncio
pred = MessagePredicate.yes_or_no(ctx)
await ctx.send("Are you **sure** you want to set the maximum rolls to {maximum}(yes/no)? This could let anyone crash the bot for rolling more than 1 million dice".format(maximum=maximum))
try:
    await ctx.bot.wait_for("message", check=pred, timeout=30)
except asyncio.TimeoutError:
    return
if pred.result:
    await self.config.max_dice_rolls.set(maximum)
else:
    return

Given the warnings, [p]diceset is owner only, and the other changes I’m marking this as approved!

#6
#7