Deterministic Discards

.gitignore

@@ -20,6 +20,8 @@ __pycache__/
 build/
 develop-eggs/
 dist/
+.dist/
+db-data/
 downloads/
 eggs/
 .eggs/
@@ -137,4 +139,3 @@ dmypy.json
 # Pyre type checker
 .pyre/
 
-/db-data

catanatron/catanatron/apply_action.py

@@ -93,8 +93,8 @@ def apply_action(
         action_record = apply_buy_development_card(state, action, action_record)
     elif action.action_type == ActionType.ROLL:
         action_record = apply_roll(state, action, action_record)
-    elif action.action_type == ActionType.DISCARD:
-        action_record = apply_discard(state, action, action_record)
+    elif action.action_type == ActionType.DISCARD_RESOURCE:
+        action_record = apply_discard(state, action)
     elif action.action_type == ActionType.MOVE_ROBBER:
         action_record = apply_move_robber(state, action, action_record)
     elif action.action_type == ActionType.PLAY_KNIGHT_CARD:
@@ -266,17 +266,25 @@ def apply_roll(state: State, action: Action, action_record=None):
     action = Action(action.color, action.action_type, dices)
 
     if number == 7:
-        discarders = [
-            player_num_resource_cards(state, color) > state.discard_limit
-            for color in state.colors
-        ]
-        should_enter_discarding_sequence = any(discarders)
+        discard_counts = []
+        first_discarding_player_index = None
+
+        for i, color in enumerate(state.colors):
+            num_cards = player_num_resource_cards(state, color)
+            discard_count = num_cards // 2 if num_cards > state.discard_limit else 0
+            discard_counts.append(discard_count)
+
+            if discard_count > 0 and first_discarding_player_index is None:
+                first_discarding_player_index = i
+
+        state.discard_counts = discard_counts
 
-        if should_enter_discarding_sequence:
-            state.current_player_index = discarders.index(True)
+        if first_discarding_player_index is not None:
+            state.current_player_index = first_discarding_player_index
             state.current_prompt = ActionPrompt.DISCARD
             state.is_discarding = True
         else:
+            state.discard_counts = [0] * len(state.colors)
             # state.current_player_index stays the same
             state.current_prompt = ActionPrompt.MOVE_ROBBER
             state.is_moving_knight = True
@@ -295,33 +303,40 @@ def apply_roll(state: State, action: Action, action_record=None):
     return ActionRecord(action=action, result=dices)
 
 
-def apply_discard(state: State, action: Action, action_record=None):
-    hand = player_deck_to_array(state, action.color)
-    num_to_discard = len(hand) // 2
-    if action_record is None:
-        # TODO: Forcefully discard randomly so that decision tree doesnt explode in possibilities.
-        discarded = random.sample(hand, k=num_to_discard)
-    else:
-        discarded = action_record.result  # for replay functionality
-    to_discard = freqdeck_from_listdeck(discarded)
+def apply_discard(state: State, action: Action):
+    discarded = action.value
+    player_index = state.color_to_index[action.color]
+    remaining = state.discard_counts[player_index]
+    assert remaining > 0, "Trying to discard when not required"
 
+    to_discard = freqdeck_from_listdeck([discarded])
     player_freqdeck_subtract(state, action.color, to_discard)
     state.resource_freqdeck = freqdeck_add(state.resource_freqdeck, to_discard)
+    state.discard_counts[player_index] -= 1
     action = Action(action.color, action.action_type, discarded)
 
-    # Advance turn
-    discarders_left = [
-        player_num_resource_cards(state, color) > 7 for color in state.colors
-    ][state.current_player_index + 1 :]
-    if any(discarders_left):
-        to_skip = discarders_left.index(True)
-        state.current_player_index = state.current_player_index + 1 + to_skip
+    if state.discard_counts[player_index] > 0:
+        # state.current_player_index stays the same
         # state.current_prompt stays the same
+        pass
     else:
-        state.current_player_index = state.current_turn_index
-        state.current_prompt = ActionPrompt.MOVE_ROBBER
-        state.is_discarding = False
-        state.is_moving_knight = True
+        next_discarder_index = next(
+            (
+                i
+                for i in range(state.current_player_index + 1, len(state.colors))
+                if state.discard_counts[i] > 0
+            ),
+            None,
+        )
+        if next_discarder_index is not None:
+            state.current_player_index = next_discarder_index
+            # state.current_prompt stays the same
+        else:
+            state.current_player_index = state.current_turn_index
+            state.current_prompt = ActionPrompt.MOVE_ROBBER
+            state.is_discarding = False
+            state.is_moving_knight = True
+            state.discard_counts = [0] * len(state.colors)
 
     return ActionRecord(action=action, result=discarded)
 

catanatron/catanatron/features.py

@@ -486,7 +486,7 @@ def game_features(game: Game, p0_color: Color):
     features = {
         "BANK_DEV_CARDS": len(game.state.development_listdeck),
         "IS_MOVING_ROBBER": ActionType.MOVE_ROBBER in possibilities,
-        "IS_DISCARDING": ActionType.DISCARD in possibilities,
+        "IS_DISCARDING": ActionType.DISCARD_RESOURCE in possibilities,
     }
     for resource in RESOURCES:
         features[f"BANK_{resource}"] = freqdeck_count(

catanatron/catanatron/gym/envs/action_space.py

@@ -20,7 +20,7 @@ def get_action_array(
     actions_array = sorted(
         [
             (ActionType.ROLL, None),
-            (ActionType.DISCARD, None),
+            *[(ActionType.DISCARD_RESOURCE, resource) for resource in RESOURCES],
             *[
                 (ActionType.BUILD_ROAD, tuple(sorted(edge)))
                 for edge in get_edges(catan_map.land_nodes)
@@ -69,7 +69,7 @@ def get_action_array(
             ],
             (ActionType.END_TURN, None),
         ],
-        key=lambda x: str(x),
+        key=lambda action: str(action),
     )
     return actions_array
 

catanatron/catanatron/gym/envs/catanatron_env.py

@@ -329,9 +329,8 @@ def _is_done(self) -> bool:
      - Float
 
    * - IS_DISCARDING
-     - Whether current player must discard. For now, there is only 1 
-       discarding action (at random), since otherwise action space
-       would explode in size.
+     - Whether current player must discard. Discarding is represented
+       as one action per resource type currently held.
      - 1
      - Boolean
    * - IS_MOVING_ROBBER

catanatron/catanatron/json.py

@@ -97,6 +97,9 @@ def default(self, obj):
                 "robber_coordinate": obj.state.board.robber_coordinate,
                 "current_color": obj.state.current_color(),
                 "current_prompt": obj.state.current_prompt,
+                "current_discard_count": obj.state.discard_counts[
+                    obj.state.current_player_index
+                ],
                 "current_playable_actions": obj.playable_actions,
                 "longest_roads_by_player": longest_roads_by_player(obj.state),
                 "winning_color": obj.winning_color(),

catanatron/catanatron/models/actions.py

@@ -88,7 +88,7 @@ def generate_playable_actions(state: State) -> List[Action]:
             actions.extend(maritime_trade_possibilities(state, color))
         return actions
     elif action_prompt == ActionPrompt.DISCARD:
-        return discard_possibilities(color)
+        return discard_possibilities(state, color)
     elif action_prompt == ActionPrompt.DECIDE_TRADE:
         actions = [Action(color, ActionType.REJECT_TRADE, state.current_trade)]
 
@@ -278,24 +278,15 @@ def initial_road_possibilities(state, color) -> List[Action]:
     return [Action(color, ActionType.BUILD_ROAD, edge) for edge in buildable_edges]
 
 
-def discard_possibilities(color) -> List[Action]:
-    return [Action(color, ActionType.DISCARD, None)]
-    # TODO: Be robust to high dimensionality of DISCARD
-    # hand = player.resource_deck.to_array()
-    # num_cards = player.resource_deck.num_cards()
-    # num_to_discard = num_cards // 2
-
-    # num_possibilities = ncr(num_cards, num_to_discard)
-    # if num_possibilities > 100:  # if too many, just take first N
-    #     return [Action(player, ActionType.DISCARD, hand[:num_to_discard])]
-
-    # to_discard = itertools.combinations(hand, num_to_discard)
-    # return list(
-    #     map(
-    #         lambda combination: Action(player, ActionType.DISCARD, combination),
-    #         to_discard,
-    #     )
-    # )
+def discard_possibilities(state: State, color) -> List[Action]:
+    if state.discard_counts[state.color_to_index[color]] <= 0:
+        return []
+
+    return [
+        Action(color, ActionType.DISCARD_RESOURCE, resource)
+        for resource in RESOURCES
+        if player_num_resource_cards(state, color, resource) > 0
+    ]
 
 
 def ncr(n, r):

catanatron/catanatron/models/enums.py

@@ -75,8 +75,7 @@ class ActionType(Enum):
     ROLL = "ROLL"  # value is None
     MOVE_ROBBER = "MOVE_ROBBER"  # value is (coordinate, Color|None).
 
-    # TODO: None for now to avoid complexity, but should be Resource[].
-    DISCARD = "DISCARD"  # value is None
+    DISCARD_RESOURCE = "DISCARD_RESOURCE"  # value is Resource
 
     # Building/Buying
     BUILD_ROAD = "BUILD_ROAD"  # value is edge_id
@@ -132,7 +131,7 @@ def __repr__(self):
 
 The "result" field is polymorphic depending on the action_type.
 - ROLL: result is (int, int) 2 dice rolled
-- DISCARD: result is List[Resource] discarded
+- DISCARD_RESOURCE: result is Resource discarded in this action
 - MOVE_ROBBER: result is card stolen (Resource|None)
 - BUY_DEVELOPMENT_CARD: result is card
 - ...for the rest, result is None since they are deterministic actions

catanatron/catanatron/players/tree_search_utils.py

@@ -31,7 +31,7 @@
         ActionType.PLAY_YEAR_OF_PLENTY,
         ActionType.PLAY_ROAD_BUILDING,
         ActionType.MARITIME_TRADE,
-        ActionType.DISCARD,  # for simplicity... ok if reality is slightly different
+        ActionType.DISCARD_RESOURCE,  # for simplicity... ok if reality is slightly different
         ActionType.PLAY_MONOPOLY,  # for simplicity... we assume good card-counting and bank is visible...
     ]
 )

catanatron/catanatron/state.py

@@ -1,7 +1,7 @@
-import random
 import pickle
+import random
 from collections import defaultdict
-from typing import Any, List, Sequence, Tuple, Dict
+from typing import Any, Dict, List, Sequence, Tuple
 
 from catanatron.models.map import BASE_MAP_TEMPLATE, CatanMap, NumberPlacement
 from catanatron.models.board import Board
@@ -76,6 +76,8 @@ class State:
         current_prompt (ActionPrompt): DEPRECATED. Not needed; use is_initial_build_phase,
             is_moving_knight, etc... instead.
         is_discarding (bool): If current player needs to discard.
+        discard_counts (List[int]): Color-index aligned number of cards each player
+            must discard in the current discard sequence.
         is_moving_knight (bool): If current player needs to move robber.
         is_road_building (bool): If current player needs to build free roads per Road
             Building dev card.
@@ -131,6 +133,7 @@ def __init__(
             self.current_prompt = ActionPrompt.BUILD_INITIAL_SETTLEMENT
             self.is_initial_build_phase = True
             self.is_discarding = False
+            self.discard_counts: List[int] = [0] * len(self.colors)
             self.is_moving_knight = False
             self.is_road_building = False
             self.free_roads_available = 0
@@ -182,6 +185,7 @@ def copy(self):
         state_copy.current_prompt = self.current_prompt
         state_copy.is_initial_build_phase = self.is_initial_build_phase
         state_copy.is_discarding = self.is_discarding
+        state_copy.discard_counts = self.discard_counts.copy()
         state_copy.is_moving_knight = self.is_moving_knight
         state_copy.is_road_building = self.is_road_building
         state_copy.free_roads_available = self.free_roads_available

catanatron/catanatron/web/models.py

@@ -91,5 +91,4 @@ def get_game_state(game_id, state_index=None) -> Game | None:
         if result is None:
             abort(404)
     db.session.commit()
-    game = pickle.loads(result.pickle_data)  # type: ignore
-    return game
+    return pickle.loads(result.pickle_data)  # type: ignore

tests/models/test_actions.py

@@ -1,5 +1,6 @@
 from catanatron.state import State
 from catanatron.models.actions import (
+    discard_possibilities,
     generate_playable_actions,
     monopoly_possibilities,
     year_of_plenty_possibilities,
@@ -10,6 +11,7 @@
     maritime_trade_possibilities,
 )
 from catanatron.models.enums import (
+    Action,
     BRICK,
     ORE,
     RESOURCES,
@@ -55,6 +57,56 @@ def test_monopoly_possible_actions():
     assert len(monopoly_possibilities(Color.RED)) == len(RESOURCES)
 
 
+def test_discard_possibilities_are_per_resource():
+    player = SimplePlayer(Color.RED)
+    state = State([player])
+    state.discard_counts[0] = 2
+
+    player_deck_replenish(state, player.color, WHEAT, 2)
+    player_deck_replenish(state, player.color, BRICK, 1)
+
+    assert discard_possibilities(state, player.color) == [
+        Action(player.color, ActionType.DISCARD_RESOURCE, BRICK),
+        Action(player.color, ActionType.DISCARD_RESOURCE, WHEAT),
+    ]
+
+
+def test_discard_possibilities_empty_when_player_does_not_need_to_discard():
+    player = SimplePlayer(Color.RED)
+    state = State([player])
+
+    player_deck_replenish(state, player.color, WHEAT, 2)
+    player_deck_replenish(state, player.color, BRICK, 1)
+
+    assert discard_possibilities(state, player.color) == []
+
+
+def test_discard_possibilities_do_not_repeat_same_resource():
+    player = SimplePlayer(Color.RED)
+    state = State([player])
+    state.discard_counts[0] = 3
+
+    player_deck_replenish(state, player.color, WHEAT, 3)
+
+    assert discard_possibilities(state, player.color) == [
+        Action(player.color, ActionType.DISCARD_RESOURCE, WHEAT),
+    ]
+
+
+def test_discard_possibilities_include_each_resource_in_resource_order():
+    player = SimplePlayer(Color.RED)
+    state = State([player])
+    state.discard_counts[0] = len(RESOURCES)
+
+    for resource in RESOURCES:
+        player_deck_replenish(state, player.color, resource)
+
+    assert discard_possibilities(state, player.color) == [
+        Action(player.color, ActionType.DISCARD_RESOURCE, resource)
+        for resource in RESOURCES
+    ]
+
+
 def test_road_possible_actions():
     player = SimplePlayer(Color.RED)
     state = State([player])
@@ -175,13 +227,6 @@ def test_initial_placement_possibilities():
     assert len(settlement_possibilities(state, Color.RED, True)) == 54
 
 
-# TODO: Forcing random selection to ease dimensionality.
-# def test_discard_possibilities():
-#     player = SimplePlayer(Color.RED)
-#     player_deck_replenish(state, player.color, Resource.WHEAT)
-#     assert len(discard_possibilities(player)) == 70
-
-
 def test_4to1_maritime_trade_possibilities():
     player = SimplePlayer(Color.RED)
     state = State([player])

tests/test_accumulators.py

@@ -44,7 +44,7 @@ def after(self, game):
     discard_actions = [
         (i, ar.action)
         for i, ar in enumerate(game.state.action_records)
-        if ar.action.action_type == ActionType.DISCARD
+        if ar.action.action_type == ActionType.DISCARD_RESOURCE
     ]
     for index, action in discard_actions:
         game_snapshot = accumulator.games[index]