aadilvarsh : Added Tyrant ask (previously lemon say 🍋) #13
Conversation
lemonsaurus
left a comment
lemonsaurus
left a comment
There was a problem hiding this comment.
Okay, so I have to request some changes here, especially not duplicating the constants.
In addition to addressing the comments in this review, I want us to have a conversation in the server about what to name this function. I don't like "lemon say". I am not the bot, and I don't want the bot to speak on my behalf. What you're building here is basically a Magic 8-ball cog, which is fine, but let's name it something along those lines then.
I also think it works just as well if it's the Tyrant who says these things. So, instead of "8-ball says" or "Lemon says:", maybe something like "The Tyrant commands: Absolutely not!".
Like, we're making a command here that let's you ask the Tyrant a yes or no question.
|
Made changes raised in the reviews! |
advrxh
changed the title
lemonsaurus
left a comment
lemonsaurus
left a comment
There was a problem hiding this comment.
Getting closer. Nice work so far.
When you address these changes, please update the original PR description to match the new functionality, since the original PR body text is now out of date and a bit misleading.
tyrant/cogs/magic_8_ball.py
Outdated
| log = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class Magic8Ball(Cog): |
There was a problem hiding this comment.
This should be updated since it is now a "say command".
There was a problem hiding this comment.
@Bluenix2 Lemon suggested to use the name Magic8ball...
There was a problem hiding this comment.
it is a magic 8-ball. it's just one that has a specific "evil tyrant" flavor.
..but this classname may make it harder to find this code for people who go looking for it. We can probably name it something like AskTyrant and ask_tyrant.py.
There was a problem hiding this comment.
Are we going forward with this renaming?
There was a problem hiding this comment.
it is a magic 8-ball. it's just one that has a specific "evil tyrant" flavor.
..but this classname may make it harder to find this code for people who go looking for it. We can probably name it something like AskTyrant and ask_tyrant.py.
Sure! AskTyrant Will be fine, I was off town for few days thats what took long to respond, fixing it ASAP.
There was a problem hiding this comment.
still waiting for this change.
lemonsaurus
left a comment
lemonsaurus
left a comment
There was a problem hiding this comment.
Just a few more changes left before this is ready to merge.
tyrant/cogs/magic_8_ball.py
Outdated
| log = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class Magic8Ball(Cog): |
There was a problem hiding this comment.
still waiting for this change.
tyrant/cogs/magic_8_ball.py
Outdated
| self.positive_replies = list(constants.POSITIVE_REPLIES) | ||
| self.uncertain_replies = list(constants.UNCERTAIN_REPLIES) | ||
|
|
||
| # filtering responses |
There was a problem hiding this comment.
| # filtering responses | |
| # Filter out certain responses |
tyrant/cogs/ask_tyrant.py
Outdated
|
|
||
| from tyrant import constants | ||
|
|
||
| # unsuitable responses to be removed from self.RESPONSES |
There was a problem hiding this comment.
| # unsuitable responses to be removed from self.RESPONSES | |
| # Unsuitable responses we need to remove from self.RESPONSES |
I would probably type this in the present-tense but future-tense works as well.
tyrant/cogs/ask_tyrant.py
Outdated
| self.positive_replies = list(constants.POSITIVE_REPLIES) | ||
| self.uncertain_replies = list(constants.UNCERTAIN_REPLIES) | ||
|
|
||
| # filtering responses |
There was a problem hiding this comment.
| # filtering responses | |
| # Filter out unsuitable responses for use here |
lemonsaurus
left a comment
lemonsaurus
left a comment
There was a problem hiding this comment.
I think this looks good now. Let's merge! 🐱
3 participants
A fun and simple command to answer to a yes/no question with a random response!
eg.
Created 1 file
tyrant/cogs/lemon_say.pyfor the command CogUpdated 1 file
tyrant/__main__.pyto load the Cog