[WIP] Players can choose a name and they can replay a party #15

JulienMattiussi · 2018-06-08T09:40:41Z

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

Kmaschta · 2018-06-08T09:44:06Z

src/game/game.py

+        ui = UIRender()
+
+        ui.clear_terminal()
+        Players.player1_name = ui.prompt_player_name(1, Players.player1_name)


Are you mutating static class attributes on the fly ? :o

fzaninotto · 2018-06-08T10:01:46Z

src/game/ui.py

+
+    def prompt_restart(self):
+        try:
+            return input("Let's play another party ? (o/n) ") == 'o'


s/party/game/

jdemangeon · 2018-06-08T10:11:21Z

src/game/tools.py

@@ -3,6 +3,20 @@
 EMPTY_POSITION = '.'


+class Players:


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.

It was also destined to store players scores (other story)

In general, all this informations are stored in only one object (a "game" state) to ensure data integrity and synchronicity.

jdemangeon · 2018-06-08T10:12:08Z

src/game/ui.py

+    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:


It's still possible to add an empty name with space

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

It's a bonus ;)

jdemangeon · 2018-06-08T10:12:28Z

src/game/ui.py

+
+    def prompt_restart(self):
+        try:
+            return input("Let's play again ? (o/n) ") == 'o'


jdemangeon · 2018-06-08T10:15:09Z

src/game/ui.py

-    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):


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

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

jdemangeon · 2018-06-08T10:15:58Z

src/game/ui.py

@@ -22,24 +22,41 @@ def prompt_piece_selection(self, game_state):
                game_state.message = "You must choose a number available in the list"
            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)


It's strange to not integrate the players into the game_state, why have you choice this architecture ?

State represent the state of the current round game.
Players remains between rounds

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

jdemangeon · 2018-06-08T12:03:53Z

src/game/state.py

@@ -66,13 +67,13 @@ def check_position_availability(self, x, y):
    def switch_player(self):
        self.game_turn.player_one_active = not self.game_turn.player_one_active

-    def check_winner(self):
+    def check_winner(self, players):


"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.

Players can choose name and they can replay a party

e3f3a22

Kmaschta reviewed Jun 8, 2018

View reviewed changes

Graphic adjustment

cc6cd9c

fzaninotto reviewed Jun 8, 2018

View reviewed changes

src/game/ui.py Outdated

def prompt_restart(self):

try:

return input("Let's play another party ? (o/n) ") == 'o'

Copy link

Member

fzaninotto Jun 8, 2018

Choose a reason for hiding this comment

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

s/party/game/

JulienMattiussi added 4 commits June 8, 2018 12:02

Review for players name

bb3ecff

Review for ending message

bd31689

Review for player names creation

1cd6496

Fix for winner name

c7ec3e3

jdemangeon suggested changes Jun 8, 2018

View reviewed changes

JulienMattiussi added 3 commits June 8, 2018 12:17

Display winner score

158b658

Module refctoring for plyers

f1df813

fix tests and lint

dc53721

jdemangeon reviewed Jun 8, 2018

View reviewed changes

JulienMattiussi added 2 commits June 14, 2018 11:29

Add singleiplayer mode (basic for now)

3686bc0

fix docker

8354051

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[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 #15

JulienMattiussi commented Jun 8, 2018 •

edited

Loading

Kmaschta Jun 8, 2018

fzaninotto Jun 8, 2018

jdemangeon Jun 8, 2018

JulienMattiussi Jun 8, 2018

jdemangeon Jun 8, 2018

jdemangeon Jun 8, 2018

JulienMattiussi Jun 8, 2018

jdemangeon Jun 8, 2018

jdemangeon Jun 8, 2018

jdemangeon Jun 8, 2018

JulienMattiussi Jun 8, 2018 •

edited

Loading

jdemangeon Jun 8, 2018

JulienMattiussi Jun 8, 2018

jdemangeon Jun 8, 2018

jdemangeon Jun 8, 2018

[WIP] Players can choose a name and they can replay a party #15

Are you sure you want to change the base?

[WIP] Players can choose a name and they can replay a party #15

Conversation

JulienMattiussi commented Jun 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JulienMattiussi Jun 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JulienMattiussi commented Jun 8, 2018 •

edited

Loading

JulienMattiussi Jun 8, 2018 •

edited

Loading