[APPROVED] Seina-Cogs

inthedark.org · June 12, 2023, 2:41pm

GitHub Repository (Must be V3): GitHub - japandotorg/Seina-Cogs: My public cogs for red.

Description: A collection of cogs that was made by inthedark.org with some cogs being refactored that were originally made by other cog creators.

Flame442 · August 27, 2023, 6:08pm

Flame442 · September 13, 2023, 8:14pm

Thank you for your patience.

Commit hash: 2f3f92a0a16840259246075f36615a7cab40905c
Red v3.5.4
Consider only bullets prefixed with to be required.

General

Your code is distributed under two licenses, however neither license file states what scope they apply to. This makes it unclear what terms the code is being released under. Additionally, code files contain bundled licenses.
You aren’t including the __red_end_user_data_statement__ value in some of your __init__.py files, so [p]mydata 3rdparty does not properly identify the end user data statements for your cogs.
red_delete_data_for_user does not need to have a return value, it should simply be return if the cog does not delete data.

Animals

Running commands (besides the creds commands) in a channel where the bot does not have embed_links permission raises the following error:

Traceback (most recent call last):
  File "datapath\cogs\CogManager\cogs\animals\core.py", line 148, in _cat
    await ctx.reply(
discord.errors.Forbidden: 403 Forbidden (error code: 50013): Missing Permissions

Running [p]cat or [p]dog without setting the API key raises the following error:

Traceback (most recent call last):
  File "datapath\cogs\CogManager\cogs\animals\core.py", line 197, in _dog
    image, name, details = await DogAPI(ctx, self.session).image(breed)  # type: ignore
  File "datapath\cogs\CogManager\cogs\animals\api.py", line 279, in image
    async with self.session.get(url=url, headers={"x-api-key": token}) as response:
TypeError: Cannot serialize non-str key None

The majority of the command functions are identical, with the only difference being whether a fact is fetched and what type of image should be fetched. It might make sense to abstract this logic to a helper function, and reduce the command bodies to a simple function call to avoid repeating code.

AntiLinks

Setting [p]antilinks channel to a channel where the bot does not have embed links permission makes it send 403 Forbidden (error code: 50013): Missing Permissions when a link is sent. This should probably be an embedless version of the same message.
If the bot is missing delete_messages permissions in a channel where a link is sent, the [p]antilinks channel is sent 403 Forbidden (error code: 50013): Missing Permissions. This should probably be a more clear warning about which watched channel is missing permissions to delete.

list commands raise the following error when used in a channel where the bot does not have embed_links permissions:

Traceback (most recent call last):
  File "datapath\cogs\CogManager\cogs\antilinks\core.py", line 239, in _user_list
    await SimpleMenu(pages).start(ctx)
discord.errors.Forbidden: 403 Forbidden (error code: 50013): Missing Permissions

You should use a raw string (r"content") for your regex pattern to avoid python complaints about invalid escape sequences.
[p]antilinks watch add only accepts discord.TextChannels, which excludes the ability to prevent links in (text in) voice channels, threads, and other strange but valid channel types.
- Forum and thread channels are explicitly excluded from on_message, but voice channels are not.

BattleRoyale

Running [p]battleroyale auto raises the following error if players is greater than the number of members in the server:

Traceback (most recent call last):
  File "datapath\cogs\CogManager\cogs\battleroyale\core.py", line 389, in auto
    users: List[discord.Member] = random.sample(list(ctx.guild.members), players - 1)
ValueError: Sample larger than population or is negative

Running [p]battleroyale (+ others) in a channel where the bot does not have embed_links permissions raises the following error:

Traceback (most recent call last):
  File "datapath\cogs\CogManager\cogs\battleroyale\core.py", line 352, in battleroyale
    join_view._message = await ctx.send(embed=embed, view=join_view)
discord.errors.Forbidden: 403 Forbidden (error code: 50013): Missing Permissions

The minimum number of players is 10 for [p]battleroyale auto, but is 3 for [p]battleroyale.

Running [p]battleroyale with no backgrounds in the background folder raises the following error:

Traceback (most recent call last):
  File "datapath\cogs\CogManager\cogs\battleroyale\core.py", line 403, in auto
    await game.start(ctx, players=player, original_message=message)
  File "datapath\cogs\CogManager\cogs\battleroyale\game.py", line 113, in start
    image: discord.File = await self.cog.generate_image(
  File "datapath\cogs\CogManager\cogs\battleroyale\core.py", line 138, in generate_image
    background = random.choice(backgrounds)
IndexError: Cannot choose from an empty sequence

You should not be modifying any files outside of cog_data_path with your cog. bundled_data_path (ie, the data folder) should be considered read only, since it is directly affected by downloader/git, and will be wiped if the cog is uninstalled.
- You also should decide if you want the bot owner to need to go to the folder manually to modify images (currently, viewing what is already there requires this), or if it should be able to be managed entirely from within discord (currently, you can add/remove with commands, but can’t know what is there already without going to the folder directly).
There are some typos in the strings of constants.py, a {winne} that may cause an error trying to interpolate, and inflicated, becaome, leathal.
Rather than using session.get for Asset objects (like profile pictures), consider using Asset.save().

Including a file that is not a Pillow-recognized image in the backgrounds folder raises the following error:

Traceback (most recent call last):
  File "datapath\cogs\CogManager\cogs\battleroyale\core.py", line 403, in auto
    await game.start(ctx, players=player, original_message=message)
  File "datapath\cogs\CogManager\cogs\battleroyale\game.py", line 113, in start
    image: discord.File = await self.cog.generate_image(
  File "datapath\cogs\CogManager\cogs\battleroyale\core.py", line 141, in generate_image
    img = Image.open(BytesIO(background_bytes))
PIL.UnidentifiedImageError: cannot identify image file <_io.BytesIO object at 0x00000272B20C2DB0>

The cog doesn’t state the requirements for background images. The cog expects them to be a certain size/aspect ratio to look “proper”, and this isn’t stated anywhere.
The code for resetting the emoji will never run, because the emoji argument has no default.

Chemistry

Using regex to find Average, Monoisotopic, Nominal, and Mean mass by blindly assuming there will be exactly 4 mass:s in the output breaks when an element like Na is missing one of these keys, but still is a valid formula.
- Consider either outputting the already formatted output of molmass.analyze, or instead using the various attributes of molmass.Formula to achieve the same result.
- Using a newer version of the library than the currently locked version might make this easier.

Running [p]molarmass in a channel where the bot does not have embed_links permissions raises the following error:

Traceback (most recent call last):
  File "datapath\cogs\CogManager\cogs\chemistry\core.py", line 62, in _molar_mass
    await ctx.send(embed=embed)
discord.errors.Forbidden: 403 Forbidden (error code: 50013): Missing Permissions

CodeForces
N/A

ConversationGames

Commands with embeds raise an error when used in a channel that the bot does not have embed_links in:

Traceback (most recent call last):
  File "datapath\cogs\CogManager\cogs\conversationgames\core.py", line 218, in _dare
    _out = await ctx.send(
discord.errors.Forbidden: 403 Forbidden (error code: 50013): Missing Permissions

Consider reading the current locale with redbot.core.i18n.get_locale, and pre-selecting one of the translations based on it.
Depending on how nsfw the R rated prompts can get, you may need to only allow them to be shown while in an age restriction marked channel.
I have no idea where your cache is getting stored, or if it is even working, on windows. ~/ isn’t a valid path on windows, and it isn’t appearing in any of the places I would expect it to. I highly suggest swapping this to store in cog_data_path instead.
- I also don’t know why you have a cache that appears to expire in 3 seconds, when the commands can only be used once per 3 seconds.
It seems like the end_user_data_statement from another cog was used for this one. It doesn’t appear to store any user data.
It appears that targeting a command on another member doesn’t allow that member to use the translations drop down. If the intention is for one player to be requesting the targeted player take that action, it would make sense that the targeted player could translate the message to be able to read it.

FirstMessage

Cog fails to load on python 3.8, likely due to newer type syntax being used that is not supported on that version. Either remove the new type syntax, or add min_python_version to your info.json.
```
  File "datapath\cogs\CogManager\cogs\firstmessage\core.py", line 86, in FirstMessage
    async def firstmessage(
TypeError: unsupported operand type(s) for |: 'type' and 'type'
```

MassUnban

Your cog says that entering the banner’s name will unban all the people they banned. It would be more specific to say that entering their ID (also listed in the ban message example) would do so, as it would account for username changes.
It would be nice if the cog listed the number (and maybe names?) of members it intends on unbanning in the warning message. It also might be nice to still have a warning when a reason is provided.

PersonalChannels

Why is it possible to clear the category, when the default value isn’t used to create channels outside of a category?

[p]mychan friends list raises the following error if the bot is missing embed_links permissions:

Traceback (most recent call last):
  File "datapath\cogs\CogManager\cogs\personalchannels\core.py", line 460, in _friends_list
    await SimpleMenu(pages).start(ctx)
discord.errors.Forbidden: 403 Forbidden (error code: 50013): Missing Permissions

The rate limit for editing a channel’s topic is much higher than the command’s cooldown. A channel’s topic can only be edited by a bot about once every 10 minutes.
Deleting a channel has no message/reaction output, so besides the channel disappearing it isn’t clear that something happened. If the command is used outside of the personal channel itself, it might make sense to add some extra feedback.
The name blacklist matches <user input> IN <blacklist word>. Generally blacklisted words are going to be a subset of the words in the user input, so it probably should be <blacklist word> IN <user input> the few places this setting is used.
The position command compares against ctx.author, not user, so it incorrectly affects the author’s channel.
The position converter limits to the total number of text channeles in the guild, however:
- It is possible to want to position before/after another type of channel, like voice
- I believe the position would be relative to other things within its category, not globally.

RainbowRole

Discord prefers bots who only make API calls in response to a direct user input. There is some leeway for things that are operationally important or infrequent (ie: reminder cogs), but a general rule of thumb is that tasks shouldn’t make API calls, events/commands should. One particular case that discord has been pretty firm about is rainbow roles, explicitly calling them out as being against ToS. This is because they “spam” (at whatever frequency) the role edit endpoint, for no operational benefit besides a visual one. They do so even when no one would be looking at the role, since there is no user input involved. Since this cog is against discord ToS, it must be removed from the repo.
- The cog also does not properly function due to a config typo, which is arguably a good thing since the cog would not have worked to break ToS anyways.

SeinaTools

[p]screenshot with an invalid url raises an error:

Traceback (most recent call last):
  File "datapath\cogs\CogManager\cogs\seinatools\core.py", line 354, in _screenshot
    await page.goto(url)
playwright._impl._api_types.Error: Protocol error (Page.navigate): Cannot navigate to invalid URL

[p]screenshot raises an error if used in a channel where the bot does not have attach_files permissions:

Traceback (most recent call last):
  File "datapath\cogs\CogManager\cogs\seinatools\core.py", line 366, in _screenshot
    await ctx.send(file=file)
discord.errors.Forbidden: 403 Forbidden (error code: 50013): Missing Permissions

[p]spy raises an error if used in a channel without embed_links or add_reactions permissions:

Traceback (most recent call last):
  File "datapath\cogs\CogManager\cogs\seinatools\core.py", line 261, in _spy
    await menu(ctx, embed_list_member, DEFAULT_CONTROLS, timeout=60)
discord.errors.Forbidden: 403 Forbidden (error code: 50013): Missing Permissions

[p]botstat raises an error if used in a channel without embed_links if [p]botstat embed is set:

  File "datapath\cogs\CogManager\cogs\seinatools\core.py", line 311, in _botstat
    return await ctx.send(

[p]spotify seems to intend to exit early if the API key is not set, however it is missing a return in that case.

Shazam
N/A

StatusRole
N/A

inthedark.org · September 24, 2023, 1:59pm

Hello, I have made the necessary changes, with a good percentage of the optional ones, there are some optionals left but i’ll look onto them later when I have a little more time.

Flame442 · September 24, 2023, 10:34pm

You’re all set, I’m marking this as approved now. Cog Creator perks will be rolled out over the next 24 hours.

A few additional optional notes are listed below:

Commit hash: 6f1f7fe792969ca62ed6d62d52ba4bc748488e17
Red v3.5.5

General

You still need to define red_delete_data_for_user with a body that is just return if you want [p]mydata forgetme to properly mark your cog as handling the deletion request. The default superclass implementation is to raise a version of NotImplementedError, which signals to that command that you did not attempt to delete the user’s data. Overriding the method to do nothing signals that you did in fact consider data deletion, even if nothing actually needed to be deleted.

BattleRoyale

To prevent wasting resources in cases where there are invalid backgrounds, rather than while True looping over the backgrounds and random.choiceing until a valid one is found, consider random.shuffleing the list one time then looping over the list in the now shuffled order. This will ensure the background is still random, but will also prevent a background from being “tested” more than once unnecessarily.
It would make more sense for [p]battleroyale auto to filter out bots first then sample from that filtered list, rather than filtering out bots after sampling the fixed number of players, to make the requested number of players more accurate.

open only implicitly creates files, not directories. [p]setbattleroyale currently errors because it does not create the subdirectory backgrounds within cog_data_path:

Traceback (most recent call last):
  File "datapath\cogs\CogManager\cogs\battleroyale\core.py", line 257, in _add_background
    with open(filepath, "wb") as f:
FileNotFoundError: [Errno 2] No such file or directory: 'datapath\cogs\BattleRoyale\backgrounds\image.png'

FirstMessage

It might make sense to check the invoker’s permissions in the requested channel to see if they can actually read messages in that channel (or in the case of DM messages, if the user is requesting themselves). Currently, the only information given is who sent the message and when, but it is still more information than would be possible to get otherwise.
The bot says it is unable to read message history in cases where there are no messages in a chat.

SeinaTools

[p]screenshot with an invalid url sends a blank white image, since there is no return when the exception is handled.

inthedark.org · September 25, 2023, 3:28pm

Thankyou for being this quick approving my repo! hopefully i can finish the optionals within this week.