שיפור בניית ערים

.github/instructions/BUILD_PLAN.md

@@ -46,8 +46,9 @@
 - [x] error handling משופר (בסיסי)
 
 **הערות:**
-- פונקציית `add_city` ב-Game מכילה באג (משתנים לא מוגדרים)
-- מיפוי פעולות מסחר וערים ב-GameManager עדיין לא מומש מלא
+- ✅ **תיקון באג `add_city`** - תוקן שימוש במשתנים לא מוגדרים (15 דצמבר 2025)
+- ✅ **מימוש בניית ערים מלא** - `_execute_build_city` עכשיו פונקציונלי (15 דצמבר 2025)
+- מיפוי פעולות מסחר עדיין לא מומש מלא
 
 ### משימה 3.2: Validation & Error Handling
 **סטטוס:** 🟡 בתהליך
@@ -60,24 +61,30 @@
 
 ### משימה 3.3: End-to-End Testing
 **סטטוס:** 🟢 בעבודה פעילה!
-**זהו הפוקוס הנוכחי - 10 דצמבר 2025**
+**עדכון אחרון:** 15 דצמבר 2025
 
 - [x] הרצת משחק מלא עם HumanUser
 - [x] בדיקת כל סוגי הפעולות (בנייה, גלגול קוביות)
 - [x] זיהוי ותיקון באגים בזמן אמת
 - [x] וידוא סנכרון בין ה-Visualizations למצב המשחק
 
-**תיקונים שבוצעו היום:**
+**תיקונים שבוצעו:**
 - ✅ תיקון כפתור ה-i בתצוגת הווב (Building Costs modal)
 - ✅ תיקון הצגת שמות שחקנים במקום "Player X" (web + console)
 - ✅ מימוש מלא של קניית קלפי פיתוח (BUY_DEV_CARD)
 - ✅ תיקון 2 באגים ב-game.py ו-player.py (DevCard.VictoryPoint)
 - ✅ תשתית לשימוש בקלפי פיתוח עם הודעות מועילות למשתמש
 - ✅ שיפור תיעוד ה-help עם הסבר מלא על כל קלפי הפיתוח
+- ✅ **בניית ערים - תיקון מלא (15 דצמבר 2025)**:
+  - תיקון באג ב-`human_user.py` - שימוש ב-`board_definition` במקום פונקציה חסרה
+  - תיקון באג ב-`game.py` - משתנים לא מוגדרים בפונקציה `add_city`
+  - מימוש מלא של `_execute_build_city` ב-`game_manager.py`
+  - המרת הודעות שגיאה למספרי נקודות (point 44) במקום קואורדינטות
+  - הצגת הודעות שגיאה בקונסול הראשי של המשתמש
 
 **מה שעובד כרגע:**
 - ✨ משחק מלא עם 3 שחקנים
-- ✨ בנייה: יישובים, כבישים, ערים
+- ✨ בנייה: יישובים, כבישים, **ערים** ✅ (תוקן 15 דצמבר!)
 - ✨ גלגול קוביות וחלוקת משאבים
 - ✨ חוק ה-7: השלכת קלפים, העברת רובר, גניבת קלף
 - ✨ מסחר עם הבנק

game_viz.log

@@ -22,7 +22,7 @@ Current Player: ► a
   Resources: None
   Buildings: Settlements: 0, Cities: 0, Roads: 0
 
-c
+s
   Victory Points: 0
   Resources: None
   Buildings: Settlements: 0, Cities: 0, Roads: 0
@@ -53,7 +53,7 @@ Current Player: ► a
   Resources: None
   Buildings: Settlements: 0, Cities: 0, Roads: 0
 
-c
+s
   Victory Points: 0
   Resources: None
   Buildings: Settlements: 0, Cities: 0, Roads: 0
@@ -86,7 +86,7 @@ Current Player: ► b
   Resources: None
   Buildings: Settlements: 1, Cities: 0, Roads: 0
 
-c
+s
   Victory Points: 0
   Resources: None
   Buildings: Settlements: 0, Cities: 0, Roads: 0
@@ -117,7 +117,7 @@ Current Player: ► b
   Resources: None
   Buildings: Settlements: 1, Cities: 0, Roads: 1
 
-c
+s
   Victory Points: 0
   Resources: None
   Buildings: Settlements: 0, Cities: 0, Roads: 0
@@ -127,15 +127,15 @@ Current Player: ► b
 Board Tiles: 19 tiles configured
 
 
->>> Turn 2: c's turn
-✓ c built a settlement
+>>> Turn 2: s's turn
+✓ s built a settlement
 
 ==================================================
                     GAME STATE                    
 ==================================================
 
 Turn: 2
-Current Player: ► c
+Current Player: ► s
 
 PLAYERS
 -------
@@ -150,7 +150,7 @@ Current Player: ► c
   Resources: None
   Buildings: Settlements: 1, Cities: 0, Roads: 1
 
-► c
+► s
   Victory Points: 1
   Resources: None
   Buildings: Settlements: 1, Cities: 0, Roads: 0
@@ -159,14 +159,14 @@ Current Player: ► c
 -----
 Board Tiles: 19 tiles configured
 
-✓ c built a road
+✓ s built a road
 
 ==================================================
                     GAME STATE                    
 ==================================================
 
 Turn: 2
-Current Player: ► c
+Current Player: ► s
 
 PLAYERS
 -------
@@ -181,7 +181,7 @@ Current Player: ► c
   Resources: None
   Buildings: Settlements: 1, Cities: 0, Roads: 1
 
-► c
+► s
   Victory Points: 1
   Resources: None
   Buildings: Settlements: 1, Cities: 0, Roads: 1
@@ -191,18 +191,18 @@ Current Player: ► c
 Board Tiles: 19 tiles configured
 
 
->>> Turn 3: c's turn
+>>> Turn 3: s's turn
 
 📦 Resources distributed:
-  c: Ore, Wheat, Wheat
-✓ c built a settlement
+  s: Ore, Wheat, Wheat
+✓ s built a settlement
 
 ==================================================
                     GAME STATE                    
 ==================================================
 
 Turn: 3
-Current Player: ► c
+Current Player: ► s
 
 PLAYERS
 -------
@@ -217,7 +217,7 @@ Current Player: ► c
   Resources: None
   Buildings: Settlements: 1, Cities: 0, Roads: 1
 
-► c
+► s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 1
@@ -226,14 +226,14 @@ Current Player: ► c
 -----
 Board Tiles: 19 tiles configured
 
-✓ c built a road
+✓ s built a road
 
 ==================================================
                     GAME STATE                    
 ==================================================
 
 Turn: 3
-Current Player: ► c
+Current Player: ► s
 
 PLAYERS
 -------
@@ -248,7 +248,7 @@ Current Player: ► c
   Resources: None
   Buildings: Settlements: 1, Cities: 0, Roads: 1
 
-► c
+► s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -284,7 +284,7 @@ Current Player: ► b
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 1
 
-c
+s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -315,7 +315,7 @@ Current Player: ► b
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
 
-c
+s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -351,7 +351,7 @@ Current Player: ► a
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
 
-c
+s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -382,7 +382,7 @@ Current Player: ► a
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
 
-c
+s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -421,7 +421,7 @@ Current Player: ► a
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
 
-c
+s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -452,7 +452,7 @@ Current Player: ► a
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
 
-c
+s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -487,7 +487,7 @@ Current Player: ► b
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
 
-c
+s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -518,7 +518,7 @@ Current Player: ► b
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
 
-c
+s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -549,7 +549,7 @@ Current Player: ► b
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
 
-c
+s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -559,21 +559,21 @@ Current Player: ► b
 Board Tiles: 19 tiles configured
 
 
->>> Turn 8: c's turn
+>>> Turn 8: s's turn
 
-🎲 c rolled: 3 + 5 = 8
+🎲 s rolled: 3 + 5 = 8
 
 📦 Resources distributed:
   Player 1: Sheep
   Player 2: Wheat
-✓ c rolled the dice
+✓ s rolled the dice
 
 ==================================================
                     GAME STATE                    
 ==================================================
 
 Turn: 8
-Current Player: ► c
+Current Player: ► s
 
 PLAYERS
 -------
@@ -588,7 +588,7 @@ Current Player: ► c
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
 
-► c
+► s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -597,14 +597,14 @@ Current Player: ► c
 -----
 Board Tiles: 19 tiles configured
 
-✓ c ended their turn
+✓ s ended their turn
 
 ==================================================
                     GAME STATE                    
 ==================================================
 
 Turn: 8
-Current Player: ► c
+Current Player: ► s
 
 PLAYERS
 -------
@@ -619,7 +619,7 @@ Current Player: ► c
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
 
-► c
+► s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -654,7 +654,7 @@ Current Player: ► a
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
 
-c
+s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -685,7 +685,7 @@ Current Player: ► a
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
 
-c
+s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -716,7 +716,7 @@ Current Player: ► a
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
 
-c
+s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -754,7 +754,7 @@ Current Player: ► b
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
 
-c
+s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -786,7 +786,7 @@ Current Player: ► b
   Dev Cards: 1
   Buildings: Settlements: 2, Cities: 0, Roads: 2
 
-c
+s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -817,7 +817,7 @@ Current Player: ► b
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 4
 
-c
+s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -848,7 +848,7 @@ Current Player: ► b
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 4
 
-c
+s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -858,20 +858,20 @@ Current Player: ► b
 Board Tiles: 19 tiles configured
 
 
->>> Turn 11: c's turn
+>>> Turn 11: s's turn
 
-🎲 c rolled: 5 + 4 = 9
+🎲 s rolled: 5 + 4 = 9
 
 📦 Resources distributed:
   Player 3: Wheat, Wheat
-✓ c rolled the dice
+✓ s rolled the dice
 
 ==================================================
                     GAME STATE                    
 ==================================================
 
 Turn: 11
-Current Player: ► c
+Current Player: ► s
 
 PLAYERS
 -------
@@ -886,7 +886,7 @@ Current Player: ► c
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 4
 
-► c
+► s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -895,14 +895,14 @@ Current Player: ► c
 -----
 Board Tiles: 19 tiles configured
 
-✓ c proposed a trade
+✓ s proposed a trade
 
 ==================================================
                     GAME STATE                    
 ==================================================
 
 Turn: 11
-Current Player: ► c
+Current Player: ► s
 
 PLAYERS
 -------
@@ -917,7 +917,7 @@ Current Player: ► c
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 4
 
-► c
+► s
   Victory Points: 2
   Resources: None
   Buildings: Settlements: 2, Cities: 0, Roads: 2
@@ -926,3 +926,99 @@ Current Player: ► c
 -----
 Board Tiles: 19 tiles configured
 
+✗ s built a city
+  Error: You don't own the settlement at point 40
+
+==================================================
+                    GAME STATE                    
+==================================================
+
+Turn: 11
+Current Player: ► s
+
+PLAYERS
+-------
+
+a
+  Victory Points: 2
+  Resources: None
+  Buildings: Settlements: 2, Cities: 0, Roads: 2
+
+b
+  Victory Points: 2
+  Resources: None
+  Buildings: Settlements: 2, Cities: 0, Roads: 4
+
+► s
+  Victory Points: 2
+  Resources: None
+  Buildings: Settlements: 2, Cities: 0, Roads: 2
+
+BOARD
+-----
+Board Tiles: 19 tiles configured
+
+✓ s built a city
+
+==================================================
+                    GAME STATE                    
+==================================================
+
+Turn: 11
+Current Player: ► s
+
+PLAYERS
+-------
+
+a
+  Victory Points: 2
+  Resources: None
+  Buildings: Settlements: 2, Cities: 0, Roads: 2
+
+b
+  Victory Points: 2
+  Resources: None
+  Buildings: Settlements: 2, Cities: 0, Roads: 4
+
+► s
+  Victory Points: 3
+  Resources: None
+  Buildings: Settlements: 1, Cities: 1, Roads: 2
+
+BOARD
+-----
+Board Tiles: 19 tiles configured
+
+✓ s ended their turn
+
+==================================================
+                    GAME STATE                    
+==================================================
+
+Turn: 11
+Current Player: ► s
+
+PLAYERS
+-------
+
+a
+  Victory Points: 2
+  Resources: None
+  Buildings: Settlements: 2, Cities: 0, Roads: 2
+
+b
+  Victory Points: 2
+  Resources: None
+  Buildings: Settlements: 2, Cities: 0, Roads: 4
+
+► s
+  Victory Points: 3
+  Resources: None
+  Buildings: Settlements: 1, Cities: 1, Roads: 2
+
+BOARD
+-----
+Board Tiles: 19 tiles configured
+
+
+>>> Turn 12: a's turn

pycatan/game.py

@@ -197,11 +197,13 @@ class Game:
 
     # changes a settlement on the board for a city
     def add_city(self, point, player):
-        status = self.board.upgrade_settlement(player, r, i)
+        # Upgrade settlement to city using the point object
+        status = self.board.upgrade_settlement(player, point)
 
         if status == Statuses.ALL_GOOD:
             # checks if the player won
             if self.players[player].get_VP() >= 10:
+                self.has_ended = True
                 self.winner = player
 
         return status

pycatan/game_manager.py

@@ -383,21 +383,54 @@ class GameManager:
         
         coords = action.parameters['point_coords']
         
-        # Get the point object from board
+        # Get the point object from board and convert coords to point ID
         try:
             point = self.game.board.points[coords[0]][coords[1]]
+            # Try to get the point ID for better user messages
+            from pycatan.board_definition import board_definition
+            point_id = board_definition.game_coords_to_point_id(coords[0], coords[1])
+            location_str = f"point {point_id}" if point_id else f"coordinates {coords}"
         except (IndexError, TypeError):
             return ActionResult.failure_result(
                 f"Invalid coordinates: {coords}",
                 "INVALID_COORDS"
             )
         
-        # Call the actual Game method (add_city doesn't exist, need to implement via settlement upgrade)
-        # For now, return not implemented
-        return ActionResult.failure_result(
-            "City building not yet implemented in Game class",
-            "NOT_IMPLEMENTED"
-        )
+        # Upgrade the settlement to a city
+        status = self.game.add_city(point, action.player_id)
+        
+        # Convert status to ActionResult
+        if status == Statuses.ALL_GOOD:
+            player = self.game.players[action.player_id]
+            return ActionResult.success_result(
+                f"City built at {location_str}! Victory Points: {player.get_VP()}",
+                "CITY_BUILT"
+            )
+        elif status == Statuses.ERR_NOT_EXIST:
+            return ActionResult.failure_result(
+                f"No settlement found at {location_str} to upgrade",
+                "NO_SETTLEMENT"
+            )
+        elif status == Statuses.ERR_BAD_OWNER:
+            return ActionResult.failure_result(
+                f"You don't own the settlement at {location_str}",
+                "NOT_YOUR_SETTLEMENT"
+            )
+        elif status == Statuses.ERR_UPGRADE_CITY:
+            return ActionResult.failure_result(
+                f"Already a city at {location_str}",
+                "ALREADY_CITY"
+            )
+        elif status == Statuses.ERR_CARDS:
+            return ActionResult.failure_result(
+                "Not enough resources. Need: 3 Ore, 2 Wheat",
+                "NOT_ENOUGH_RESOURCES"
+            )
+        else:
+            return ActionResult.failure_result(
+                f"Failed to build city: {status.name}",
+                status.name
+            )
 
     def _execute_build_road(self, action: Action) -> ActionResult:
         """Execute road building action."""

pycatan/game_moves_3Players.txt

@@ -31,3 +31,6 @@ end
 roll
 t player a nothing for ore 2
 y
+c 40
+c 44
+end

pycatan/human_user.py

@@ -278,10 +278,10 @@ class HumanUser(User):
             try:
                 point = int(parts[1])
                 
-                # Convert point to coordinates
-                coords = point_to_coords(point)
+                # Convert point to coordinates using BoardDefinition
+                coords = board_definition.point_id_to_game_coords(point)
                 if coords is None:
-                    raise UserInputError(f"Invalid point number: {point}. Valid points: 1-{len(get_all_points())}")
+                    raise UserInputError(f"Invalid point number: {point}. Valid points: 1-{len(board_definition.get_all_point_ids())}")
                 
                 row, index = coords
                 
@@ -889,9 +889,12 @@ class HumanUser(User):
     
     def notify_action(self, action: Action, success: bool, message: str = "") -> None:
         """Notify the user about an action result."""
-        # Don't print here - the console visualization already displays this
-        # This method is kept for compatibility but doesn't produce output
-        pass
+        # Display success/error messages to the user in their input console
+        if message:
+            if success:
+                print(f"    ✓ {message}")
+            else:
+                print(f"    ✗ {message}")
     
     def notify_game_event(self, event_type: str, message: str, 
                          affected_players: Optional[List[int]] = None) -> None:

test_city_building.py

@@ -0,0 +1,52 @@
+"""
+Test script for city building functionality.
+"""
+from pycatan import Game
+from pycatan.card import ResCard
+
+# Create game with 3 players
+game = Game(num_of_players=3)
+board = game.board
+
+print("Testing City Building...")
+print("=" * 60)
+
+# Setup: Add a settlement for player 0
+point = board.points[0][0]
+print(f"\n1. Adding settlement at point [0][0] for player 0...")
+status = game.add_settlement(player=0, point=point, is_starting=True)
+print(f"   Result: {status}")
+
+# Give player resources for city (3 Ore, 2 Wheat)
+print(f"\n2. Giving player 0 resources for city (3 Ore, 2 Wheat)...")
+needed_cards = [
+    ResCard.Ore,
+    ResCard.Ore,
+    ResCard.Ore,
+    ResCard.Wheat,
+    ResCard.Wheat
+]
+game.players[0].add_cards(needed_cards)
+print(f"   Player 0 cards: {[str(c) for c in game.players[0].cards]}")
+
+# Try to build a city
+print(f"\n3. Upgrading settlement to city at point [0][0]...")
+status = game.add_city(point=point, player=0)
+print(f"   Result: {status}")
+
+# Check victory points
+print(f"\n4. Checking player 0 victory points...")
+vp = game.players[0].get_VP()
+print(f"   Victory Points: {vp}")
+
+# Check the building type
+print(f"\n5. Checking building at point [0][0]...")
+building = point.building
+if building:
+    print(f"   Building type: {building.type}")
+    print(f"   Building owner: {building.owner}")
+else:
+    print(f"   No building found!")
+
+print("\n" + "=" * 60)
+print("Test complete!")