Discord Name: Bobloy
GitHub Repository (Must be V3): https://github.com/bobloy/Fox-V3
Description: Assortment of tools and games for any server.
Discord Name: Bobloy
GitHub Repository (Must be V3): https://github.com/bobloy/Fox-V3
Description: Assortment of tools and games for any server.
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 “ for A-M, 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)
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.
All required changes should be done now.
Most suggested changes have been done as well.
Please see previously linked PR #37 for details.
Looks great, thanks for getting to everything including the suggestions/not so critical stuff. You should see an approved tag here soon.