Andeeeee / AndyCogs

My redbot cogs.
MIT License
6 stars 2 forks source link

Your giveaway cog is a chance to break big bots #11

Closed ltzmax closed 3 years ago

ltzmax commented 3 years ago

You should take a look at your cogboard review And your cog is a big chance to break a big bot.

I have examples of this to break big bots while it broke and made a lot of heartbeats on my bot which is public and had 190 servers wiht 180k users, while 30 servers using giveaway where breaking the whole bot, so please read though the review and make the changes.

async def is_manager(ctx: commands.Context):
    if not ctx.guild:
        return False
    if ctx.channel.is_news():
        return False
    if (
        ctx.channel.permissions_for(ctx.author).administrator
        or ctx.channel.permissions_for(ctx.author).manage_guild
    ):
        return True
    if await ctx.bot.is_owner(ctx.author):
        return True

    cog = ctx.bot.get_cog("Giveaways")

    role = await cog.config.guild(ctx.guild).manager()

    for r in role:
        if r in [role.id for role in ctx.author.roles]:
            return True

    return False

And this code is actually blocking my help menu. you should change this out with something like @commands.mod_or_permissions(perms) @commands.admin_or_permissions(perms) (perms can be example kick_members=True). but yes actually change this would be better.

This is up to yourself if you want to change or not, but it is really blocking.

Andeeeee commented 3 years ago

ill try to make it more efficient later

Andeeeee commented 3 years ago

Okay, so I'll consider adding a manual check in the command, so it doesn't run when someone uses help, but then this command will pop up regardless if they have permissions or not to run it. I don't really want to do @commands.admin/mod_or_permissions(**perms) because I don't think they should be able to start giveaways just because they have those perms, however I might consider for admin since they pretty much can give them themselves the giveaway role in MOST cases. So anyways is the manual check in the command fine with you?

ltzmax commented 3 years ago

Yes, it should be fine

Andeeeee commented 3 years ago

Changes have been made, lmk if there are errors by making an issue or smthing