Conversation
Three of a Kind and Four of a Kind should return the sum of ALL dice, not just the matching dice multiplied by their count. Example: - Before: Three 3s = 9 points (3 * 3) - After: Three 3s + two other dice = sum of all 5 dice This aligns with official Yahtzee rules.
Changed from comparing with NumberOfDice (5) to MaxRolls (3). Players should be limited to 3 rolls per turn, not 5.
Added DisplayGameOver method to IOHandler interface and ConsoleIOHandler. The game over screen now displays: - Final scoreboard - Winner announcement with trophy emoji - Handles tie situations Resolves TODO in client.go:178
Added 35-point bonus when Upper Section (Ones through Sixes) totals 63 or more points, following official Yahtzee rules. The CalculateTotalScore function now: - Calculates Upper Section total separately - Applies bonus if threshold is met - Adds Lower Section scores
Added Error message type and ErrorMessage field to messages package. Server now sends error messages to clients when they exceed the maximum number of rerolls. Changes: - Added messages.Error type and ErrorMessage field - Server sends error when reroll limit exceeded - Client displays error messages to user Resolves TODO in server/gameplay_controller.go:112
- Fixed player.Conn to player.Connection in gameplay_controller.go - Added DisplayGameOver method to MockIOHandler to satisfy IOHandler interface
Corrected method call to use the proper Connection interface method. All tests now pass successfully.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes several bugs in the Yahtzee game server logic and implements missing features to align with official Yahtzee rules.
Key Changes:
- Fixed score calculation for Three/Four of a Kind to return the sum of all dice instead of just the matching dice
- Corrected reroll limit from
NumberOfDice(5) toMaxRolls(3) - Implemented upper section bonus (35 points when upper section total ≥ 63 points)
- Added winner display feature with tie handling and error message system
Reviewed Changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| server/gameplay_controller.go | Updated reroll validation to use MaxRolls constant and implemented error message sending to client when reroll limit is exceeded |
| messages/messages.go | Added Error message type and ErrorMessage field to support server-to-client error communication |
| game/score.go | Fixed Three/Four of a Kind to return sum of all dice; implemented upper section bonus calculation in total score |
| client/mock_iohandler.go | Added DisplayGameOver method to mock for testing winner display functionality |
| client/iohandler.go | Added DisplayGameOver interface method for game over display |
| client/console_io_handler.go | Implemented DisplayGameOver with winner determination, tie handling, and colorful UI output |
| client/client.go | Added error message handler and updated game over handler to display winners using the new DisplayGameOver method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.