[WIP] Players can choose a name and they can replay a party#15
[WIP] Players can choose a name and they can replay a party#15JulienMattiussi wants to merge 11 commits intomasterfrom
Conversation
| ui = UIRender() | ||
|
|
||
| ui.clear_terminal() | ||
| Players.player1_name = ui.prompt_player_name(1, Players.player1_name) |
There was a problem hiding this comment.
Are you mutating static class attributes on the fly ? :o
|
|
||
| def prompt_restart(self): | ||
| try: | ||
| return input("Let's play another party ? (o/n) ") == 'o' |
| EMPTY_POSITION = '.' | ||
|
|
||
|
|
||
| class Players: |
There was a problem hiding this comment.
It's very strange to have a "Players" class in "tools.py". Moreover, it seems that you use Players class as a property container, i'm not sure that it's the best way to achieve this.
There was a problem hiding this comment.
It was also destined to store players scores (other story)
There was a problem hiding this comment.
In general, all this informations are stored in only one object (a "game" state) to ensure data integrity and synchronicity.
| def prompt_player_name(self, player_id, default_name): | ||
| try: | ||
| name = input("Player " + str(player_id) + ", what is your name (" + default_name + " if empty) ? ") | ||
| if len(name) > 0: |
There was a problem hiding this comment.
It's still possible to add an empty name with space
There was a problem hiding this comment.
That's right. I don't see some real reason to forbid "spaces names" if the player has entered it, but I 'll manage for the game visibility
|
|
||
| def prompt_restart(self): | ||
| try: | ||
| return input("Let's play again ? (o/n) ") == 'o' |
| def players_to_string(self, game_turn): | ||
| player_1 = self.selected_player_to_string('Player 1', game_turn.player_one_active) | ||
| player_2 = self.selected_player_to_string('Player 2', not game_turn.player_one_active) | ||
| def players_to_string(self, game_turn, players): |
There was a problem hiding this comment.
In the case you've create a "Player" class, it would be possible to implement a "special" method (str) to achieve this task (cast an object to a string representation). This is the Java equivalent of toString().
More informations here: https://openclassrooms.com/courses/apprenez-a-programmer-en-python/les-methodes-speciales-1
There was a problem hiding this comment.
I thinked about it, but I prefer to keep all render methods in the UI class
In this case : Changing UI doesn't impact other games classes
| except ValueError: | ||
| game_state.message = "You have to type number between 1 and " + str(PIECES_NUMBER) | ||
| self.display_game(game_state) | ||
| self.display_game(game_state, players) |
There was a problem hiding this comment.
It's strange to not integrate the players into the game_state, why have you choice this architecture ?
There was a problem hiding this comment.
State represent the state of the current round game.
Players remains between rounds
There was a problem hiding this comment.
You can still integrate player into the game state even if they don't change. This way, you manipulate one state only and it's easier to reason about and test
| self.game_turn.player_one_active = not self.game_turn.player_one_active | ||
|
|
||
| def check_winner(self): | ||
| def check_winner(self, players): |
There was a problem hiding this comment.
"check_winner" is not a good name for this function. Moreover, 1 function = 1 task. Here, the function is responsible of "message" change and return a boolean
I would create a function to get the current winner (or nul if no one) and another to assign the message from the returned value instead.
https://trello.com/c/BsVVCYmg
https://trello.com/c/TbJAqfdC
https://trello.com/c/33ocNyd9
Name is asked at start
Replay is asked at end of party
Score is displayed
Writing tests in progress