-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
"Deal or No Deal" Game #203
base: master
Are you sure you want to change the base?
Conversation
fixed user being able to pick boxes multiple times
I'll get to reviewing the code right now, but I first want to say that this is an awesome idea, @AleksAce ! |
} | ||
public IIntervalAction GetIntervalAction(DealNoDealGameState gameState) | ||
{ | ||
if (gameState == DealNoDealGameState.CHOSING_STARTING_BOXES) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't require a change, but I'd recommend using a switch
statement here, since this is an enum. It's possible that the C# compiler is smart enough to optimize this to a switch for you, but I'm not sure if it does for this code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change made. aeab4d3 (I hope this is how you link commits 😄 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works!
private DealNoDealGame _dealNoDealGame; | ||
|
||
|
||
public ChoseStartingBoxesAction(DealNoDealGame dealNoDealGame, IChatClient chatClient, IClock clock) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can IClock clock = new SystemClock()
here giving it a default value, so you don't have to specify it everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, that was sloppy from my side 👍
Love these new game ideas coming in. This bot is really something special. Congrats on your first pull request! |
Wow, thanks guys 😄 , I'm glad i can contribute to something, since you've all given me a lot of knowledge. 👍 I'm having a pretty busy week with college, so I'll make sure to watch the VOD's if you review it live. Thanks devchatter community! |
On the costs and payouts: We should make sure the average payout is somewhere equal to the cost to play. It wouldn't be a gamble if someone could start a game and accept the first deal of lets say 500 coins right away 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize that this PR hasn't been touched in a while.
It looks good overall! I left some comments for potential improvements.
{ | ||
_chatClient.SendMessage("Picking a random Box. User did not respond"); | ||
_dealNoDealGame.PickRandomBox(_dealNoDealGame.MainPlayer.DisplayName); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indenting seems to be off here
_dealNoDealGame = dealNoDealGame; | ||
_chatClient = chatClient; | ||
HelpText = | ||
"Use \"!dnd\" to start a game. Use \"!dnd pick x\" to pick/guess a box. Use \"!dnd accept\" or \"!dnd decline\" to accept or decline offers ."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is quite long, split up into two lines?
|
||
namespace DevChatter.Bot.Core.Games.DealNoDeal | ||
{ | ||
public class DNDCommand : BaseCommand, IGameCommand |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This breaks the C# convention for capitalization, see https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/capitalization-conventions#capitalization-rules-for-identifiers
Maybe even call it DealOrNoDealCommand
?
{ | ||
if (string.IsNullOrWhiteSpace(argumentOne)) | ||
{ | ||
// attempting to start a game |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we lose anything by removing this comment?
public const UserRole ROLE_REQUIRED_TO_START = UserRole.Everyone; | ||
public const int SECONDS_TO_CHOOSE_BOXES = 30; | ||
public const int TOKENS_TO_PLAY = 25; | ||
public const int MIN_PLAYABLE_BOXES = 5; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All public members should be pascal cased according to https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/capitalization-conventions#capitalization-rules-for-identifiers. Still a taste thing though.
}; | ||
|
||
public List<Box> StartingBoxes; | ||
public List<Box> BoxesWithOwners; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to initialize these to empty list to avoid NullReferenceException
?
} | ||
else | ||
{ | ||
//Restart timer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is confusing, what does the code below have to do with restarting the timer?
private int GetADeal() | ||
{ | ||
//average | ||
int result = BoxesWithOwners.Sum(b => b.TokenValue) / BoxesWithOwners.Count; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe name average
and remove the comment?
{ | ||
string allPrices = "Prices left: "; | ||
|
||
//Printe all the prices left in order |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo => "print" instead of "printe"
Game Description:
The main game revolves around the opening of a set of numbered boxes, each of which contains a different prize (tokens). The contents (i.e., the values) of all of the cases are known at the start of the game, but the specific location of any prize is unknown.
-Only a unique box number can be picked.
-A user can hold 1 box per game. (5 players - 5 boxes)
-If no one picks, the game is guaranteed to have at least 5 playable boxes.
-The "box picker" gets an immediate whisper from the bot about the approximate token value that his box holds.
-There's a 30% chance that the bot will lie to you about the value you hold"
-if the player is AFK, a random box is picked after 30 seconds.
-the other players might think they have the correct value that the bot whispered, but can be wrong. The main player might not trust them.
game commands;
!dnd - starts a game
!dnd pick x - picks a box number
!dnd accept or !dnd decline - accept/decline deal
Summary: You pay 25 tokens, you might win 100 (or you can set this higher) or you can win 1 token.
It's a gamble in the end. Good luck reading my code. I tried to use as many interfaces of yours as possible.
you can read more on https://en.wikipedia.org/wiki/Deal_or_No_Deal
It's a variation of a casino "deal no deal game" made for the chat.
Feel free to modify the configuration of the game.
There might be a lot of bugs, but I wanted you to review it and fix a lot of the stuff.
It can be really fun if there are a lot of people playing it, and some people would be saying "I have a HIGH VALUE, Pick me last" etc, but you shouldn't trust anyone in the chat 😄, or the bot might have lied to them .
I Hope you can review it on stream, so I can learn a lot more!
Thank you 😸
Your loyal viewer,
aceflameseer
P.S. this is my REAL first pull request ever. ❤️