Deterministic discards

catanatron/catanatron/apply_action.py

@@ -93,7 +93,7 @@ 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:
+    elif action.action_type == ActionType.DISCARD_RESOURCE:
         action_record = apply_discard(state, action, action_record)
     elif action.action_type == ActionType.MOVE_ROBBER:
         action_record = apply_move_robber(state, action, action_record)
@@ -266,17 +266,26 @@ 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
+        state.discard_counts = {
+            color: (
+                player_num_resource_cards(state, color) // 2
+                if player_num_resource_cards(state, color) > state.discard_limit
+                else 0
+            )
             for color in state.colors
-        ]
-        should_enter_discarding_sequence = any(discarders)
+        }
+        should_enter_discarding_sequence = any(state.discard_counts.values())
 
         if should_enter_discarding_sequence:
-            state.current_player_index = discarders.index(True)
+            state.current_player_index = next(
+                i
+                for i, color in enumerate(state.colors)
+                if state.discard_counts[color] > 0
+            )
             state.current_prompt = ActionPrompt.DISCARD
             state.is_discarding = True
         else:
+            state.discard_counts = {color: 0 for color in state.colors}
             # state.current_player_index stays the same
             state.current_prompt = ActionPrompt.MOVE_ROBBER
             state.is_moving_knight = True
@@ -295,33 +304,53 @@ 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):
+def normalize_discarded_cards(state: State, action: Action, action_record=None):
+    if action.value is not None:
+        if isinstance(action.value, (list, tuple)):
+            return list(action.value)
+        return [action.value]
+
+    if action_record is not None and action_record.result is not None:
+        if isinstance(action_record.result, (list, tuple)):
+            return list(action_record.result)
+        return [action_record.result]
+
     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
+    return [random.choice(hand)]
+
+
+def apply_discard(state: State, action: Action, action_record=None):
+    discarded = normalize_discarded_cards(state, action, action_record)
+    remaining = state.discard_counts[action.color]
+    if len(discarded) > remaining:
+        raise ValueError("Trying to discard more cards than 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)
-    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
+    state.discard_counts[action.color] = remaining - len(discarded)
+    action_value = discarded[0] if len(discarded) == 1 else tuple(discarded)
+    action = Action(action.color, action.action_type, action_value)
+
+    if state.discard_counts[action.color] > 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
+        discarders_left = [state.discard_counts[color] > 0 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
+            # 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 = {color: 0 for color in 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

@@ -17,10 +17,15 @@ def get_action_array(
 
     # We sort the actions to ensure a consistent ordering and reproducibility
     # without sorting, we couldn't get gym usages to be reproducible
+    def action_sort_key(action):
+        # Preserve historical DISCARD ordering so the rename does not reshuffle
+        # integer action ids for gym consumers.
+        return str(action).replace("DISCARD_RESOURCE", "DISCARD")
+
     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 +74,7 @@ def get_action_array(
             ],
             (ActionType.END_TURN, None),
         ],
-        key=lambda x: str(x),
+        key=action_sort_key,
     )
     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

@@ -30,6 +30,15 @@ def action_from_json(data) -> Action:
         if len(resources) not in [1, 2]:
             raise ValueError("Year of Plenty action must have 1 or 2 resources")
         action = Action(color, action_type, resources)
+    elif action_type == ActionType.DISCARD_RESOURCE:
+        value = data[2]
+        if isinstance(value, list):
+            if len(value) != 1:
+                raise ValueError(
+                    "Discard action must have 1 resource when encoded as a list"
+                )
+            value = value[0]
+        action = Action(color, action_type, value)
     elif action_type == ActionType.MOVE_ROBBER:
         coordinate, victim = data[2]
         coordinate = tuple(coordinate)

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[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 List[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 (Dict[Color, int]): Remaining 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,8 @@ def __init__(
             self.current_prompt = ActionPrompt.BUILD_INITIAL_SETTLEMENT
             self.is_initial_build_phase = True
             self.is_discarding = False
+            self.discard_counts: Dict[Color, int] = {color: 0 for color in self.colors}
+            self.discard_counts: Dict[Color, int] = {color: 0 for color in self.colors}
             self.is_moving_knight = False
             self.is_road_building = False
             self.free_roads_available = 0
@@ -182,6 +186,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,20 @@ 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[player.color] = 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_road_possible_actions():
     player = SimplePlayer(Color.RED)
     state = State([player])
@@ -175,13 +191,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]