Skip to content

Spook Name Rate Game#509

Merged
ks129 merged 1 commit into
python-discord:masterfrom
eklipse18:master
Jan 21, 2021
Merged

Spook Name Rate Game#509
ks129 merged 1 commit into
python-discord:masterfrom
eklipse18:master

Conversation

@eklipse18
Copy link
Copy Markdown
Contributor

Relevant Issues

Closes #277

Description

We have added a file called spooknamerate.py which contains relevant code for the game.

Everyday, a random name is generated from a list of first and last names. (The data was downloaded from mockaroo). This name is sent in #seasonalbot-commands and the users need to spookify the name and register it using the command .spooknamerate add the name.

Two hours before the game ends, a poll is sent out, and users cannot add anymore words. The users can add a reaction on each entry on a scale of 1 - 5 (in the form of emojis) and two hours later the results are given out. After this, another name is given out and the game starts again.

Screenshots

It starts off like this:
image

Basic help is provided when you do .spooknamerate
image

This is what you get when you add a word
image

Using the command .spooknamerate list, you can see a list of all the added words
image
Clicking a link takes you that word.

You can delete your entry using .spooknamerate delete:
image

You can see the word name using .spooknamerate word:
image

At the end, the winner is announced:
image

Did you:

  • Join the Python Discord Community?
  • If dependencies have been added or updated, run pipenv lock?
  • Lint your code (pipenv run lint)?
  • Set the PR to allow edits from contributors?

@eklipse18 eklipse18 requested a review from a team as a code owner October 21, 2020 18:35
@eklipse18 eklipse18 requested review from aeros and thomaspet October 21, 2020 18:35
@ghost ghost added the needs 2 approvals label Oct 21, 2020
@ghost
Copy link
Copy Markdown

ghost commented Oct 21, 2020

Thank you for contributing to Python Discord!

Please check out the following documents:

Copy link
Copy Markdown
Contributor

@thomaspet thomaspet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't fully review it at the moment, but most of the things I found now are done multiple places in the code. One comment counts for all the instances.

I'm also not a fan of all the # comments around the code, most of them don't add any value, just noise. For instance
if final: # if it is the final
The code here is self explanatory, and dont need the comment.
While this is a good comment, since I was about to question why you're sleeping there for 1 sec.
await asyncio.sleep(1) # give the drum roll feel

Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
@ghost ghost added status: waiting for author Waiting for author to address a review or respond to a comment needs 2 approvals and removed needs 2 approvals status: waiting for author Waiting for author to address a review or respond to a comment labels Oct 22, 2020
@eklipse18
Copy link
Copy Markdown
Contributor Author

I ran pipenv run lint, but I don't know why I it made changes to all files. So I discarded that and just formatted it using autopep8 and fixed other flake8 complaints, is it fine?

Copy link
Copy Markdown
Contributor

@thomaspet thomaspet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a few more suggestions

Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
@ghost ghost added status: waiting for author Waiting for author to address a review or respond to a comment and removed needs 2 approvals labels Oct 24, 2020
@thomaspet
Copy link
Copy Markdown
Contributor

It's only fine if out linter passes it, as you can see in the tests under, it doens't pass.
running pipenv run lint, should just tell you whats wrong, not change it?

@Akarys42
Copy link
Copy Markdown
Contributor

running pipenv run lint, should just tell you whats wrong, not change it?
Actually some of the hooks will try to fix the errors, in this cases you are missing an empty line at the end of your json file, it seems like.

@Vthechamp22 you should run pipenv run precommit to make sure that your code will get linted before every commit. Also, it seems like your working on your master branch, which isn't advised, but it is fine for this one, just make sure that if you want to work on another feature in the meantime, you need to branch from origin/master, not the local master. Feel free to ask in #dev-contrib for any help, we would be happy to provide you with some advices.

@eklipse18
Copy link
Copy Markdown
Contributor Author

eklipse18 commented Oct 24, 2020

It's only fine if out linter passes it, as you can see in the tests under, it doens't pass.
running pipenv run lint, should just tell you whats wrong, not change it?

@thomaspet Before I run pipenv run lint, git status returns:

On branch master
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   bot/exts/halloween/spooknamerate.py

no changes added to commit (use "git add" and/or "git commit -a")

(This one change is by me)
pipenv run lint returns this and git status returns this.

@eklipse18
Copy link
Copy Markdown
Contributor Author

eklipse18 commented Oct 24, 2020

And, I don't have any flake8 complaints (flake8 . returns nothing)

@eklipse18
Copy link
Copy Markdown
Contributor Author

@Akarys42 pre-commit isn't letting me commit anything, so I have uninstalled it for the time being.

@ghost ghost added needs 2 approvals and removed status: waiting for author Waiting for author to address a review or respond to a comment labels Oct 24, 2020
@eklipse18
Copy link
Copy Markdown
Contributor Author

So anything yet @thomaspet ?

Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
@ghost ghost added status: waiting for author Waiting for author to address a review or respond to a comment and removed needs 2 approvals labels Oct 29, 2020
Comment thread bot/exts/halloween/spooknamerate.py Outdated
@ghost ghost added needs 2 approvals and removed status: waiting for author Waiting for author to address a review or respond to a comment labels Oct 29, 2020
@thomaspet
Copy link
Copy Markdown
Contributor

Have a look at asyncio.Lock for the concurrency issue with the dictionary.

@eklipse18
Copy link
Copy Markdown
Contributor Author

Okay @thomaspet !

@ghost ghost removed the needs 1 approval label Dec 13, 2020
Copy link
Copy Markdown
Contributor

@ks129 ks129 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, some more comments.

Comment thread bot/exts/halloween/spooknamerate.py
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
@ghost ghost added needs 1 approval and removed status: waiting for author Waiting for author to address a review or respond to a comment labels Dec 18, 2020
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Copy link
Copy Markdown
Contributor

@ks129 ks129 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't add reaction to add command invoke message, but bot response instead to avoid cases when user deletes message and then this is impossible to vote for this name, but name still exists in cache.

@eklipse18 eklipse18 requested a review from ks129 January 11, 2021 12:22
@eklipse18
Copy link
Copy Markdown
Contributor Author

eklipse18 commented Jan 11, 2021

I can't understand why the removing reaction part isn't working. It sends the message just fine, but doesn't remove the reaction. @Akarys42 mentioned something about race conditions but I can't find any. Nevermind I got it 😃 😤

Copy link
Copy Markdown
Contributor

@tagptroll1 tagptroll1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly just the use of temporary lists, that could be generators or have it's values calculated on the fly.

Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Comment thread bot/exts/halloween/spooknamerate.py
Comment thread bot/exts/halloween/spooknamerate.py Outdated
Copy link
Copy Markdown
Contributor

@Akarys42 Akarys42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to rename this cog to Spooky Name Rate for two major reasons :

  • "Spook" is an ancient racist term to mock black people
  • "Spooky" makes more sense in this context than "Spook"
    Keep up the good work!

Comment thread bot/exts/halloween/spooknamerate.py
@eklipse18 eklipse18 requested a review from thomaspet January 17, 2021 13:00
Copy link
Copy Markdown
Contributor

@thomaspet thomaspet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as you don't make any more bigger changes, I believe we're getting very close now.

Comment thread bot/exts/halloween/spookynamerate.py Outdated
Comment thread bot/exts/halloween/spookynamerate.py Outdated
Comment thread bot/exts/halloween/spookynamerate.py Outdated
Comment thread bot/exts/halloween/spookynamerate.py Outdated
@eklipse18 eklipse18 requested a review from thomaspet January 20, 2021 08:04
@thomaspet
Copy link
Copy Markdown
Contributor

I'll try to give this a review some time today.

@eklipse18
Copy link
Copy Markdown
Contributor Author

Okay thank you!

Copy link
Copy Markdown
Contributor

@thomaspet thomaspet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say this looks good now.

Copy link
Copy Markdown
Contributor

@ks129 ks129 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ks129 ks129 dismissed stale reviews from Akarys42, tagptroll1, RohanJnr, and Senjan21 January 21, 2021 15:45

Renaming is done.

removed commented code

added the delete command

Changed the name from ratethespook to spooknamerate

renamed file too

rename ratethespook.json to spooknamerate.json

Added more names from mockaroo

added one user one reaction

removed print statements

fixed flake8 comments and prevented the user from
deleting the word while polling

fixed typo, added random messages, made each
entry unique,  fixed one user one reaction,
made it for one day. I think I am done

commented some code

autopep8ed

edited few lines of code

added comments

added some comments

edited code

edited code

edited

fixed syntax error

fixed flake8 complaints

fixed flake8 complaint

fixed small error

added a `word` command and informed user if
they don't have a registered word

fixed a small error where the first and last names
weren't separated

removed unecessary code

changed word to name

remove slash in multiline strings and remove
unecessary comments. Also, lock the background loop
to `OCTOBER` and make emojis into discord
emoij form (:emoij_name:)

fixed an accidental tab

removed another unecessary comment

remove more unecessary comments

remove `.keys()` for dictionaries

removed `len() > 0` for lists and dicts and
changed emojis to '\N{}' form

Fixed code so that return value is that specified
and added fail-safes instead of `if` and `else`s

f-stringify

fixed borderline api abuse and missing space in
defining word

lint code

multiple imports on one line and fix typo

remove unecessary return, shorten var typehint
remove 'Just' in suggestion and add extra line
in json file.

- unecessary comment and replace on with for in
help embed description

shorten emoij_message

remove check in delete command

use defaultdict

sort imports

group imports, add typehinting, remove unnecessary
comments and docstring, remove redundant elses
and returns

sort imports

refactor var wrds, use generator instead of list
use typing.Dict[str, str] instead of dict and use
.items() instead of get()

add a comment

remove reduntant comment

Renamed variable to avoid conflicts

add asyncio locking to prevent Runtime error

add some comments

lock all commands to OCTOBER

enhance looping in checking messages

add `cog_check` instead of limiting each command

remove unused import

remove test code and comments

use fail fast

rename function

Make storage persistent and make sure announce_word
does not go off everytime the bot restarts

fixed typo

make data persist, rename everything that has word
in it to 'name' and make sure announce_name doesn't
start off everytime the bot restarts.

remove testing code which would cause a real mess
if commited. Which I did commit.

use a separate file for the name and first_time
becuase re-dumping such a long file might take time

make var for repetitive paths and change .day to .hour

change scheduling logic

lint code

add cog_unload

fix error in spooknamerate_names.json and fix
the before_loop in spooknamerate.py

revert accidental changes and remove commented code

remove unused code

refactor vars to caps and make emojis_val global

edit docstring and make seasonalbot_commands to
community_bot_commands

make annotation correct and add check when channel is
None for get_channel

Add fullstop

Loop directly over data

Add a proper dash and fix linting

Fix linting

reverted to making EMOJIS_VAL global and fixed capitalization

fix small error

verify it is working and simplify import

remove data files

Use redis caching instead of JSON and rename

remove empty title and description in embed and use
discord's red color

remove var typehint

add Client.month_override for dev

move ping function

rename seasonalbot to sir-lancebot

remove unnecessary newline

fix line formatting

move added_messages to global

Add more info on the caches

remove + alias

improve formatting

use str.format instead of func

fix error

directly used Channels.community_bot_commands

get user from cache instead of actively
fetching the user

move help messages to constant

add more info in err msg

Apply suggestions from review

Co-authored-by: ks129 <45097959+ks129@users.noreply.github.com>

remove unnecessary comments

remove another redundant comment

improve formatting and use better var
names

hard-code a var

Use get or fetch format

Remove redundant commit

Fix in_allowed_month for debugging

remove extra space

make channel name link to channel

simplify uteration

use msg.reactions directly

rename r to reaction and directly use variable

reformat code

use from_dict instead of manually creating the
Embed

Remove commented code

fix channel linking

add some debugging support

add some more info to the debug mode

Directly use getLogger

sort imports

Remove (name) in function doctype

Use SpookNameRate.debug everywhere

Shrink function call to one line and remove extra info in comment

Use fail fast in on_reaction_add

use environment var for debug mode

Set debug val to False by default

Fix some line breaks that formatting with black had made and use fail fast

Use custom environment variable instead of the global bot env var

make bot reply and store info from the bot's reply instead of the user's message

remove an accidental swp file

fix the reaction not getting removed

remove extra brackets

use generators instead of lists

fix logging statement

simplify loop

rename spooknamerate to spookynamerate

Correct docstring

Improve the name announcing code

Ignore reaction of all bots

rearrange or

send "Name deleted" instead of "Message deleted"

Add client prefix
@ks129 ks129 merged commit 7bb5c35 into python-discord:master Jan 21, 2021
@eklipse18 eklipse18 mentioned this pull request Sep 20, 2021
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Spook Name Rate Game

8 participants