[APPROVED] FlameCogs

Discord Name:
Flame#2941
GitHub Repository (Must be V3):


Description:
A mixture of board games and random things packed together
1 Like

Just wanted to make a note that I provided Flame with a small list of issues I found on the repo/cogs that need some attention - haven’t reviewed the repo in full yet but at least he could get started on some changes. Mostly just user interaction modifications/catching a couple errors.

Hello again Flame, have some new things for you to take a look at.

The “say I” command from the bot when starting a game of battleship and monopoly listens to everything and not just “I”

Hangmanset wordlist has too long of a docstring to show up all the way in help (gets cut off on the last line)

Hangman and Monopoly both use load_bundled_data which is now depreciated. This is a rather new change that not a lot of people have made yet, see https://github.com/Cog-Creators/Red-DiscordBot/pull/2342 for more info. I’m mentioning it as more of a suggestion to look at because otherwise it prints a message in the console that that function is depreciated and you may hear from users about it.

Hangmanset wordlist list does not return filenames of files present in the \cogs\CogManager\cogs\hangman\data folder and instead says “You do not have any wordlists.” as it’s actually looking in \cogs\Hangman\ for the files instead.

My comments in earlier revisions where I suggested using box() from the chat_formatting utils for Red instead of wrapping strings with “```” was just for code clarity’s sake. Wasn’t suggesting for you to write a function to do so as it’s already in the utils.

Re your response from the first round of edits I asked you to make where you said: “On hangman with doEdit set to True, sometimes (like once or twice per word) the bot will “delete” the message but it will stay there for only the user who is playing. . . " That seems to be a client issue as sometimes messages will still show on a client but really won’t be visible to others as they don’t exist any more and really were deleted. You could add a very small wait time before deletion, that may fix it for the slower clients. Haven’t seen that issue too much on different cogs/commands though.

After loading a Monopoly save and letting it time out, it printed out the Save info for that game in a code block. Was this intended? I actually can’t find where this is being called from. https://i.imgur.com/RaXaPXy.png



For visibility’s sake, I’m attaching the points I made to Flame and his responses in earlier conversations as they were done off-site and not posted here. He has addressed everything below already.

Battleship

-Coords can be placed outside of the boundaries when guessing (ie Z18)
*Fixed - Legacy support for any height/with game before I had the code on red that I never bothered checking
*(FYI, the game only looks at the first 2 characters not the whole message, so something like Z18 is equivalent to Z1 as far as the code cares)
-You can use the battleship command while there’s a game running and it will dump the current game
*Fixed - Added self.runningin, a list of channels a game is running in, and a check if the channel is in that list on !battleship
-There should be a way to end a game with a command
*Would require either a minor rewrite of the code to do it like trivia (using a class), or a game command (instead of a cord type stop to stop), I don’t want to do it the lazy way and I don’t have time right now to do it the right way, so added to Planned Changes.
-May want to pass on invalid input instead of saying so, people like to chat in channels while playing sometimes
*Fixed - After setup “invalid input” removed, just continues in the loop like it would have anyways. Already shot there message remains

Deepfry

-Looks good
* :slight_smile:

Gamevoice

-May want to add an unset command or make it more clear how to unset it - had to look at the specific help for gv set to figure it out.
*Fixed - Added [p]gv reset
-Was playing a game with RPC on and gv set returned: Lobby will now only allow people playing <discord.activity.Activity object at 0x7f6e7ee7b950> to join
*Might be an issue with how discord handles RPC? I only have a vague understanding from google. It uses str(ctx.message.author.activity) to print the activity it was set to (and to make a role for that activity), ctx.message.author.activity.name might fix it (can you test something like [p]eval return author.activity while running it) because it the api does not list str() as a valid operation for activities.

Hangman
*See note 2

-May want to pass on invalid input instead of saying so, people like to chat in channels while playing sometimes
*Fixed - Expected a different issue, past behavior has it take the first character of every message and parse based on that which would not return an invalid input, rather it would attempt the first letter of your message as a guess. Either way, hangman now only parses messages of len == 1, and only returns a message on success or already guessed letter. This also has the effect of causing the game message to rise if there is a lot of conversation and doEdit == True.
-Error on hangmanset, Windows 7 computer:

Exception in command 'hangmanset'
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/discord/ext/commands/core.py", line 61, in wrapped
    ret = await coro(*args, **kwargs)
  File "/root/.local/share/Red-DiscordBot/cogs/CogManager/cogs/hangman/hangman.py", line 151, in hangmanset
    await ctx.send('The wordlist is set to `'+v[::-1].split('\\')[0][::-1][:-4]+'`.')
TypeError: 'PosixPath' object is not subscriptable

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/discord/ext/commands/bot.py", line 898, in invoke
    await ctx.command.invoke(ctx)
  File "/usr/local/lib/python3.6/dist-packages/discord/ext/commands/core.py", line 614, in invoke
    await injected(*ctx.args, **ctx.kwargs)
  File "/usr/local/lib/python3.6/dist-packages/discord/ext/commands/core.py", line 70, in wrapped
    raise CommandInvokeError(e) from e
discord.ext.commands.errors.CommandInvokeError: Command raised an exception: TypeError: 'PosixPath' object is not subscriptable
*See note 1, v = self.config.guild(ctx.guild).fp(), would changing the line to use str(v) instead fix that?

Monopoly
-May want to pass on invalid input instead of saying so, people like to chat in channels while playing sometimes
*Fixed, only in some places (not in a y/n, for example)
-Saved a game but couldn’t load it again with [p]monopoly savename - the bot replied with “You have no save files.”
*See note 1, save files are a text file with the name you saved it as in your data folder for monopoly, no idea why it didn’t save but I plan to switch to using config anyway for save “files” so unless it is an obvious issue I’m not going to worry about it. What do you see in \cogs\Monopoly? You should see bundled_data, temp.png, and any save files you made as .txt. Could be an issue related to hangman not wanting to display save files, as they are handled in the same way.
-await ctx.send('```'+hold.strip()+'```') - use box (ln 209)
*Do you mean use a command to add the “```” instead of making it myself? Don’t see the point if that’s the case.

***1:
I have only tested this running on a Windows 7 machine and I have very little understanding of how OS works, so I just tried a few things until I got something that I wanted. There are probably a lot of bugs related to how I did things with files that I need to work out to some degree. Possible cause, my bot runs on 3.7.1, you tested on 3.6.6, could os have some difference in those two versions? Again I don’t know enough about os to know.
***2:
On hangman with doEdit set to True, sometimes (like once or twice per word) the bot will “delete” the message but it will stay there for only the user who is playing. The message then can’t be deleted (as it doesn’t exist) until the user reloads discord. Can you reproduce and how do I fix it? Is that just a problem with discord API? My only guess for fixing it is to add a short delay before deleting it (because I think the cause is the user’s discord not knowing if it sent successfully until after the message was already deleted), but then the user has to wait for that delay to continue playing.

Gamevoice

Line 42: roleid = await ctx.message.guild.create_role(name=str(ctx.message.author.activity.name))

will error out if the bot doesn’t have perms to create roles.

Member update also needs catching if the bot loses it’s perms:

[30/12/2018 16:41] ERROR events on_error 188: Exception in member_update
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/discord/client.py", line 225, in _run_event
    await coro(*args, **kwargs)
  File "/Users/Erin/Red/cogs/CogManager/cogs/gamevoice/gamevoice.py", line 128, in update
    await afterMem.remove_roles(afterMem.guild.get_role(rolelist[x]))
  File "/usr/local/lib/python3.6/site-packages/discord/member.py", line 570, in remove_roles
    await req(guild_id, user_id, role.id, reason=reason)
  File "/usr/local/lib/python3.6/site-packages/discord/http.py", line 209, in request
    raise Forbidden(r, data)
discord.errors.Forbidden: FORBIDDEN (status code: 403): Missing Access

[30/12/2018 16:43] ERROR events on_command_error 208: Exception in command 'gamevoice deleterole'
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/discord/ext/commands/core.py", line 61, in wrapped
    ret = await coro(*args, **kwargs)
  File "/Users/Erin/Red/cogs/CogManager/cogs/gamevoice/gamevoice.py", line 119, in gamevoice_deleterole
    await role.delete()
  File "/usr/local/lib/python3.6/site-packages/discord/role.py", line 283, in delete
    await self._state.http.delete_role(self.guild.id, self.id, reason=reason)
  File "/usr/local/lib/python3.6/site-packages/discord/http.py", line 209, in request
    raise Forbidden(r, data)
discord.errors.Forbidden: FORBIDDEN (status code: 403): Missing Permissions

[30/12/2018 16:47] ERROR events on_command_error 208: Exception in command 'gamevoice reset'
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/discord/ext/commands/core.py", line 61, in wrapped
    ret = await coro(*args, **kwargs)
  File "/Users/Erin/Red/cogs/CogManager/cogs/gamevoice/gamevoice.py", line 65, in gamevoice_reset
    await ctx.message.author.voice.channel.set_permissions(everyone, connect=True, speak=True)
  File "/usr/local/lib/python3.6/site-packages/discord/abc.py", line 593, in set_permissions
    await http.edit_channel_permissions(self.id, target.id, allow.value, deny.value, perm_type, reason=reason)
  File "/usr/local/lib/python3.6/site-packages/discord/http.py", line 209, in request
    raise Forbidden(r, data)
discord.errors.Forbidden: FORBIDDEN (status code: 403): Missing Permissions

In battleship it seems you can still set shots outside of bounds (used J20 as a test)

Monopoly:

Cannot find a way to exit out of mortgage list when there are no entries - maybe use “cancel” in a message to return to the board
Also same thing for manage houses, can’t escape the list if there’s nothing to choose

1 Like

All concerns are at least addressed below and any changes can be found on the dev branch of my repo.

Although this was technically intended behavior, I can see how that could be confusing. Fixed.

When I was first creating the cog (which was probably in v2) I had to load bundled data. I am hesitant to touch this due to strange behavior in later comments.

Repeating what I said in discord, the default wordlist is copied from \cogs\CogManager\cogs\hangman\data to \cogs\Hangman, and other wordlists are expected to be put in \cogs\Hangman. I don’t understand what isn’t showing up for you, however it works fine (at least as intended) for me.

This is a debate I’ve been having with myself on what to do in a timeout for monopoly. The time investment into a long monopoly game shouldn’t be lost, but there isn’t a good way to handle this that I can see. The options I came up with are:

  1. Print the save info and let the user handle it, and give better instructions on how to recover a save
    • Only the bot owner can recover a game, doesn’t scale well
  2. Save lost games to a static named file (autosave)
    • Larger servers that might use the cog more often could have overwritten games
  3. Save lost games to a dynamic named file (autosave+time)
    • Lots of different save files taking up space

Right now that serves as a last resort if somebody has an important game and really wants to save it, I can tell them how to recover that save manually. At some point I will probably add a more user friendly version.

Hey there. Did some poking around in Monopoly this morning. I see you’ve already removed the depreciated line from your init.py for bundled data, which is good - but the other file path issues I was speaking to are still present. It looks like you’re using cog_data_path for items when you should be using bundled_data_path, and also since you write on Windows I saw that you’re using \\ for path dividers in some places. If you change those cog_data_path instances to bundled_data_path and replace your \\ with / it looks like things are saving in the correct places now, and I’m able to save and load save files. I’m going to DM you an example as well in case this isn’t clear. I also tested this change I’m describing on Windows and Mac, since you were concerned about OS paths and you only have Windows available for your testing.

Howdy Aika,
As we discussed, I did not make the bundled_data_path change as it is described as a directory you should “NEVER write to” in the Red API. I fixed the \\ to / bug for Monopoly, Hangman, and Deepfry in the dev branch of the repo.

I tested your changes on your dev branch and don’t have any more fixes for you or other comments. Thanks for being responsive on needed changes and for being helpful to others in the cog and main servers.

The changes have been pulled from my dev branch. Thank you for your time!