[APPROVED] Fox-V3 Application

#1

Discord Name: Bobloy

GitHub Repository (Must be V3): https://github.com/bobloy/Fox-V3

Description: Assortment of tools and games for any server.

#2

Hi Bobloy, this may look like a pretty long list but it’s mostly info.json changes.

announcedaily

Required:
Rename info…json to info.json so the cog will be properly hidden
If you are intending to hide this cog, don’t worry about any of the below until you want to work on it, once the info…json is renamed:
No need for an empty requirements field in the info.json

Suggestions:
For your tags, you may want to put “daily”, “announce”, etc
Typo on line 143 in “images”
Line 195 could be “Announcement time…” instead of plural
You can use while True: if you’d like on line 232 instead of the cog instance

audiotrivia

Required:
No need for an empty requirements field in the info.json

Suggestions:
You may want to add “audio” to your tags in the info.json

audiotrivia.py:

Suggestions:
Lines 20 and 21 should be changed to reflect the cog
When starting a trivia session, check for audioset notify being on (same idea as statuses - catch at beginning with the notify toggle in audio’s config)
You can remove line 113 (storing the guild ID in the audio player). The same needs to be done in audio, it was something from a long time ago that doesn’t need to be there any more. The channel storing is for the notify messages, though, and you’d want to leave that in there in case someone does still keep notify on even through the prompt not to.

ccrole

Suggestions:
Remove extra slash on “Is this a targeted command?(yes//no)”
Custom command verification that the command completes does not fire until another command is used. For example, I add a ccrole of “testing2”. It will report that the command has been added when the configuration is complete, but will not say “Custom Command testing2 successfully added” until another additional command is used afterwards.

chatter

Required:
Possibly add warning about chatter taking a really long time to respond if there’s a large database, I don’t remember if chatter cogs use up a bunch of memory when it’s at that point - also leaving this sort of vague as I don’t have a large database to test this with. Good that you disclosed that it can take a while on the _get_conversation function, but it’s not visible to the user afaik.

coglint

Required:
Rename info..json to info.json so that the cog is properly hidden if this is what you’d like on this cog.

FYI:
Got an error while running the lint command, so I’d assume this is a cog you want hidden and that you’re still working on.

dad

Required:
Rename info..json to info.json so that the cog is properly hidden if this is what you’d like on this cog.

FYI:
On message “I’m” detection didn’t seem to work for me, for the “Hi {} I’m dad” response

exclusiverole

Required:
No need for an empty requirements field in the info.json

Suggestion:
Add a list command to exclusiverole to display all current roles that are exclusive

flag

Required:
Rename info..json to info.json so that the cog is properly hidden if this is what you’d like on this cog. Also will fix install message not appearing/short description not being shown.

FYI:
Got an error while running the some commands, so I’d assume this is a cog you want hidden and that you’re still working on.

forcemention

Required:
Rename info..json to info.json - will fix install message not appearing/short description not being shown.

hangman

Required:
Your decorators use pass_context=True, which is not needed in v3 unless you are not passing context on purpose

Suggestions:
[p]hangset face with a non-standard/custom emoji throws a traceback/IndexError
[p]hangset toggleemoji removes reactions to play the game as expected, but the “:arrow_up_small: for A-M, :arrow_down_small: for N-Z” text makes me think it will respond with those emojis added. May want to replace that with something like “React with your letter choice” when this toggle is on

infochannel

Required:
No need for an empty requirements field in the info.json

leaver

Required:
No need for an empty requirements field in the info.json

Suggestions:
[p]leaverset channel has no help string
member.nick can be None in the leave message (line 41), looks a little odd on a leave msg (e.g. “aikaterna#1393(None) has left the server!”)

lovecalculator

Looks good!

lseen

Required:
Rename info..json to info.json - will fix install message not appearing/short description not being shown.

planttycoon

Suggestion:
May want to move your data files (plants, badges, defaults etc) out of the cog. You could store those values in Config if you wanted, or use an external data file if these items won’t be changed. Putting them in Config also gives you the option later of adding customizable values for items.

qrinvite

Required:
Rename info..json to info.json - will fix install message not appearing/short description not being shown.

Suggestion:
Line 17 should have help instead of a placeholder

reactrestrict

Cog is hidden appropriately

FYI:
The line break on line 209 makes the help look incomplete for reactrestrict add when using the [p]reactrestrict group command.
There’s an on_raw_reaction_add() error when adding an emoji to a message

recyclingplant

Required if intended to be listed in the repo and not hidden:
Got an error that a json file (Red/cogs/RecyclingPlant/bundled_data/junk.json) was missing on load.

rpsls

FYI:
“RPSLP” on info.json’s install message instead of “RPSLS”

sayurl

Required:
Rename info..json to info.json to properly hide cog, will also fix install message not appearing/short description not being shown.

Suggestions:
“[p]load forcemention” in line 12 of your info.json should be “[p]load sayurl”
Catch invalid urls being passed

scp

Required:
ctx.embed_requested() needs to be awaited at multiple points… lines 31, 97, 121. Traceback can be seen on the scp and scparc commands at least.
No need for an empty requirements field in the info.json

stealemoji

Required:
Rename info..json to info.json to properly hide cog, will also fix install message not appearing/short description not being shown.
No requirements field needed.

Suggested:
Maybe add something that can alert when the emoji bank server is full or an overflow, multiple guild possibility

timerole

Required:
No need for an empty requirements field in the info.json

Suggestion:
“Configure with [p]timerole" in the install message on the info.json doesn’t have markdown for the command, i.e. “Configure with [p]timerole

tts

Required:
Rename info..json to info.json to properly hide cog, will also fix install message not appearing/short description not being shown.

unicode

Looks good!

werewolf

Should be required since this is a visible, valid cog, but it may be that you just more work on it or it’s in progress:

[p]wwset category errors out on a non-int for the id param
[p]wwset channel and wwset logchannel don’t check for valid perms to speak in said channel
[p]buildgame does not have help
[p]ww group command sends help twice (line 297)

#3

I have put most of the requested changes into PR #37

However, I have some questions about the comments.

I did not experience this behavior when testing. The last step is selecting the text that will be sent when using the custom command, so it may have been waiting for you to choose the custom text.

I am having difficulty fixing the PlantTycoon and RecyclingPlant, bundled_data does not seem to be being added correctly. I will continue to try to fix these and update when it’s done.

#4

All required changes should be done now.
Most suggested changes have been done as well.

Please see previously linked PR #37 for details.

#5

Looks great, thanks for getting to everything including the suggestions/not so critical stuff. You should see an approved tag here soon.

closed #6
unlisted #7
listed #8
assigned aikaterna #9