From 6a8b9b6876ab652bd0a47d5608b77bee518c19e5 Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Wed, 29 Jan 2025 21:02:10 +0100 Subject: [PATCH 01/27] ENH: Add default_zmq_context --- pelita/base_utils.py | 4 ++++ pelita/network.py | 12 +++++------- pelita/team.py | 7 +++---- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/pelita/base_utils.py b/pelita/base_utils.py index d031024b0..22ee1e3d1 100644 --- a/pelita/base_utils.py +++ b/pelita/base_utils.py @@ -1,5 +1,6 @@ from random import Random +import zmq def default_rng(seed=None): """Construct a new RNG from a given seed or return the same RNG. @@ -12,3 +13,6 @@ def default_rng(seed=None): if isinstance(seed, Random): return seed return Random(seed) + +def default_zmq_context(zmq_context=None): + return zmq.Context() diff --git a/pelita/network.py b/pelita/network.py index dc00a616b..0fe0c064f 100644 --- a/pelita/network.py +++ b/pelita/network.py @@ -8,6 +8,8 @@ import zmq +from .base_utils import default_zmq_context + _logger = logging.getLogger(__name__) # 41736 is the word PELI(T)A when read upside down in reverse without glasses @@ -305,10 +307,8 @@ def show_state(self, game_state): class Controller: def __init__(self, address='tcp://127.0.0.1', zmq_context=None): self.address = address - if zmq_context: - self.context = zmq_context - else: - self.context = zmq.Context() + self.context = default_zmq_context(zmq_context) + # We use a ROUTER which we bind. # This means other DEALERs can connect and # each one can take over control. @@ -363,8 +363,6 @@ def recv_start(self, timeout=None): def setup_controller(zmq_context=None): - if not zmq_context: - import zmq - zmq_context = zmq.Context() + zmq_context = default_zmq_context(zmq_context) controller = Controller(zmq_context=zmq_context) return controller diff --git a/pelita/team.py b/pelita/team.py index fa21356e6..610fc4b24 100644 --- a/pelita/team.py +++ b/pelita/team.py @@ -13,6 +13,7 @@ import zmq from . import layout +from .base_utils import default_zmq_context from .exceptions import PlayerDisconnected, PlayerTimeout from .layout import BOT_I2N, layout_as_str, wall_dimensions from .network import (PELITA_PORT, ZMQClientError, ZMQConnection, @@ -308,8 +309,7 @@ class RemoteTeam: the remote clients will be suppressed. """ def __init__(self, team_spec, *, team_name=None, zmq_context=None, idx=None, store_output=False): - if zmq_context is None: - zmq_context = zmq.Context() + zmq_context = default_zmq_context(zmq_context) self._team_spec = team_spec self._team_name = team_name @@ -535,8 +535,7 @@ def make_team(team_spec, team_name=None, zmq_context=None, idx=None, store_outpu elif isinstance(team_spec, str): _logger.info("Making a remote team for %s", team_spec) # set up the zmq connections and build a RemoteTeam - if not zmq_context: - zmq_context = zmq.Context() + zmq_context = default_zmq_context(zmq_context) team_player = RemoteTeam(team_spec=team_spec, zmq_context=zmq_context, idx=idx, store_output=store_output) else: raise TypeError(f"Not possible to create team from {team_spec} (wrong type).") From 81006a4fad44a7eebdd0e3e7a19fabe12c6f5a95 Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Mon, 9 Jun 2025 21:50:02 +0200 Subject: [PATCH 02/27] BF: Do not crash tk when no team name is passed --- pelita/ui/tk_canvas.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pelita/ui/tk_canvas.py b/pelita/ui/tk_canvas.py index ba885533d..7500d5e4e 100644 --- a/pelita/ui/tk_canvas.py +++ b/pelita/ui/tk_canvas.py @@ -459,7 +459,7 @@ def update(self, game_state=None, redraw=False): if winning_team_idx is None: self.draw_end_of_game(None) elif winning_team_idx in (0, 1): - team_name = game_state["team_names"][winning_team_idx] + team_name = game_state["team_names"][winning_team_idx] or "???" self.draw_game_over(team_name) elif winning_team_idx == 2: self.draw_game_draw() @@ -820,8 +820,8 @@ def draw_title(self, game_state): center = self.ui_game_canvas.winfo_width() // 2 - left_name = game_state["team_names"][0] - right_name = game_state["team_names"][1] + left_name = game_state["team_names"][0] or '???' + right_name = game_state["team_names"][1] or '???' left_score = game_state["score"][0] right_score = game_state["score"][1] From 8646bf7e58d08a8aacb5b640dbb1263b11e7afb6 Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Fri, 6 Jun 2025 13:08:33 +0200 Subject: [PATCH 03/27] BF: Keyboard interrupt should not cause a freeze A keyboard interrupt during recv (or sigterm from a parent process) would still cause a send on the socket, leading to a lock. --- pelita/scripts/pelita_player.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pelita/scripts/pelita_player.py b/pelita/scripts/pelita_player.py index da7f37b05..909e2b231 100755 --- a/pelita/scripts/pelita_player.py +++ b/pelita/scripts/pelita_player.py @@ -105,6 +105,11 @@ def player_handle_request(socket, team, team_name_override=False, silent_bots=Fa try: json_message = socket.recv_unicode() + except Exception: + # Exit without sending a value back on the socket + return True + + try: py_obj = json.loads(json_message) msg_id = py_obj["__uuid__"] action = py_obj["__action__"] From 05aec983f83ad55ec88298df0e6cf4fde6747b4a Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Sat, 21 Jun 2025 00:46:36 +0200 Subject: [PATCH 04/27] ENH: Better bot repr --- pelita/team.py | 2 +- test/test_team.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pelita/team.py b/pelita/team.py index 610fc4b24..ad6272edb 100644 --- a/pelita/team.py +++ b/pelita/team.py @@ -762,7 +762,7 @@ def __str__(self): return out.getvalue() def __repr__(self): - return f'' + return f'' # def __init__(self, *, bot_index, position, initial_position, walls, homezone, food, is_noisy, score, random, round, is_blue): diff --git a/test/test_team.py b/test/test_team.py index e3f367d02..70f65ac78 100644 --- a/test/test_team.py +++ b/test/test_team.py @@ -713,9 +713,9 @@ def test_bot_repr(): def asserting_team(bot, state): bot_repr = repr(bot) if bot.is_blue and bot.round == 1: - assert bot_repr == f"" + assert bot_repr == f"" elif not bot.is_blue and bot.round == 1: - assert bot_repr == f"" + assert bot_repr == f"" else: assert False, "Should never be here." From bba5358d6acb44027e59a0ecd2e58690b348977a Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Sat, 21 Jun 2025 00:48:12 +0200 Subject: [PATCH 05/27] RF: New network/subprocess protocol --- contrib/ci_engine.py | 4 +- pelita/exceptions.py | 21 +- pelita/game.py | 351 +++++++++++++++++-------- pelita/network.py | 164 +++++++----- pelita/scripts/pelita_main.py | 2 +- pelita/scripts/pelita_player.py | 92 +++---- pelita/scripts/pelita_server.py | 12 +- pelita/team.py | 437 ++++++++++++++++++++------------ pelita/tournament/__init__.py | 4 + test/test_game.py | 179 ++++++++----- test/test_libpelita.py | 4 +- test/test_network.py | 2 +- test/test_remote_game.py | 13 +- 13 files changed, 814 insertions(+), 471 deletions(-) diff --git a/contrib/ci_engine.py b/contrib/ci_engine.py index 5a2552604..bbbac8e4c 100755 --- a/contrib/ci_engine.py +++ b/contrib/ci_engine.py @@ -55,7 +55,7 @@ from rich.console import Console from rich.table import Table -from pelita.network import ZMQClientError +from pelita.network import RemotePlayerFailure from pelita.scripts.script_utils import start_logging from pelita.tournament import call_pelita, check_team @@ -126,7 +126,7 @@ def load_players(self): _logger.debug('Querying team name for %s.' % pname) team_name = check_team(player['path']) self.dbwrapper.add_team_name(pname, team_name) - except ZMQClientError as e: + except RemotePlayerFailure as e: e_type, e_msg = e.args _logger.debug(f'Could not import {player} at path {path} ({e_type}): {e_msg}') player['error'] = e.args diff --git a/pelita/exceptions.py b/pelita/exceptions.py index 16cac0b67..0295bfea2 100644 --- a/pelita/exceptions.py +++ b/pelita/exceptions.py @@ -1,19 +1,12 @@ -class FatalException(Exception): # TODO rename to FatalGameException etc - pass - -class NonFatalException(Exception): - pass - -class PlayerTimeout(NonFatalException): +class NoFoodWarning(Warning): + """ Warns when a layout has no food during setup. """ pass -class PlayerDisconnected(FatalException): - # unsure, if PlayerDisconnected should be fatal in the sense of that the team loses - # it could simply be a network error for both teams - # and it would be random who will be punished +class GameOverError(Exception): + """ raised from game when match is game over """ pass -class NoFoodWarning(Warning): - """ Warns when a layout has no food during setup. """ - pass +class PelitaBotError(Exception): + """ Raised when allow_exceptions is turned on """ + pass \ No newline at end of file diff --git a/pelita/game.py b/pelita/game.py index 4402a09c0..e8d6fccfb 100644 --- a/pelita/game.py +++ b/pelita/game.py @@ -8,13 +8,14 @@ import time from warnings import warn +import zmq + from . import layout from .base_utils import default_rng -from .exceptions import (FatalException, NoFoodWarning, NonFatalException, - PlayerTimeout) +from .exceptions import NoFoodWarning, PelitaBotError from .gamestate_filters import noiser, relocate_expired_food, update_food_age, in_homezone from .layout import get_legal_positions, initial_positions -from .network import ZMQPublisher, setup_controller +from .network import RemotePlayerFailure, RemotePlayerRecvTimeout, RemotePlayerSendError, ZMQPublisher, setup_controller from .team import make_team from .viewer import (AsciiViewer, ProgressViewer, ReplayWriter, ReplyToViewer, ResultPrinter) @@ -340,6 +341,9 @@ def setup_game(team_specs, *, layout_dict, max_rounds=300, rng=None, food_age=[{}, {}], ### Round/turn information + #: Phase + game_phase='INIT', + #: Current bot, int, None turn=None, @@ -438,6 +442,7 @@ def setup_game(team_specs, *, layout_dict, max_rounds=300, rng=None, controller=viewer_state['controller'] ) + # Wait until the controller tells us that it is ready # We then can send the initial maze # This call *blocks* until the controller replies @@ -460,7 +465,13 @@ def setup_game(team_specs, *, layout_dict, max_rounds=300, rng=None, update_viewers(game_state) # exit remote teams in case we are game over - check_exit_remote_teams(game_state) + if game_state['gameover']: + exit_remote_teams(game_state) + + if game_state['game_phase'] == 'INIT': + send_initial(game_state, allow_exceptions=allow_exceptions) + game_state.update(check_gameover(game_state, detect_final_move=True)) + game_state['game_phase'] = 'RUNNING' return game_state @@ -468,6 +479,8 @@ def setup_game(team_specs, *, layout_dict, max_rounds=300, rng=None, def setup_teams(team_specs, game_state, store_output=False, allow_exceptions=False): """ Creates the teams according to the `teams`. """ + assert game_state['game_phase'] == 'INIT' + # we start with a dummy zmq_context # make_team will generate and return a new zmq_context, # if it is needed for a remote team @@ -476,76 +489,206 @@ def setup_teams(team_specs, game_state, store_output=False, allow_exceptions=Fal teams = [] # First, create all teams # If a team is a RemoteTeam, this will start a subprocess - for idx, team_spec in enumerate(team_specs): - team, zmq_context = make_team(team_spec, idx=idx, zmq_context=zmq_context, store_output=store_output, team_name=game_state['team_names'][idx]) + for team_idx, team_spec in enumerate(team_specs): + team, zmq_context = make_team(team_spec, idx=team_idx, zmq_context=zmq_context, store_output=store_output, team_name=game_state['team_names'][team_idx]) teams.append(team) - # Send the initial state to the teams and await the team name (if the teams are local, the name can be get from the game_state directly - team_names = [] - for idx, team in enumerate(teams): - try: - team_name = team.set_initial(idx, prepare_bot_state(game_state, idx)) - except (FatalException, PlayerTimeout) as e: - # TODO: Not sure if PlayerTimeout should let the other payer win. - # It could simply be a network problem. - if allow_exceptions: - raise + # Await that the teams signal readiness and get the team name + initial_timeout = 4 + start = time.monotonic() + + has_remote_teams = any(hasattr(team, 'zmqconnection') for team in teams) + remote_sockets = {} + + if has_remote_teams: + poll = zmq.Poller() + for team_idx, team in enumerate(teams): + if hasattr(team, 'zmqconnection'): + poll.register(team.zmqconnection.socket, zmq.POLLIN) + remote_sockets[team.zmqconnection.socket] = team_idx + + break_error = False + while remote_sockets and not break_error: + timeout_left = int((initial_timeout - time.monotonic() + start) * 1000) + if timeout_left <= 0: + break + + # socket -> zmq.POLLIN id + evts = dict(poll.poll(timeout_left)) + for socket in evts: + team_idx = remote_sockets[socket] + team = teams[team_idx] + + try: + _state = team.wait_ready(timeout=0) + except (RemotePlayerSendError, RemotePlayerRecvTimeout, RemotePlayerFailure) as e: + + if allow_exceptions: + exit_remote_teams(game_state) + raise + + exception_event = { + 'type': e.__class__.__name__, + 'description': str(e), + 'turn': team_idx, + 'round': None, + } + game_state['fatal_errors'][team_idx].append(exception_event) + + if len(e.args) > 1: + game_print(team_idx, f"{type(e).__name__} ({e.args[0]}): {e.args[1]}") + else: + game_print(team_idx, f"{type(e).__name__}: {e}") + break_error = True + + del remote_sockets[socket] + + # Handle timeouts + if not break_error and remote_sockets: + break_error = True + for socket, team_idx in remote_sockets.items(): exception_event = { - 'type': e.__class__.__name__, - 'description': str(e), - 'turn': idx, + 'type': 'Timeout', + 'description': 'Team did not start (timeout).', + 'turn': team_idx, 'round': None, } - game_state['fatal_errors'][idx].append(exception_event) - if len(e.args) > 1: - game_print(idx, f"{type(e).__name__} ({e.args[0]}): {e.args[1]}") - team_name = f"%%%{e.args[0]}%%%" - else: - game_print(idx, f"{type(e).__name__}: {e}") - team_name = "%%%error%%%" - team_names.append(team_name) + + game_state['fatal_errors'][team_idx].append(exception_event) + game_print(team_idx, f"Team '{teams[team_idx]._team_spec}' did not start (timeout).") + + game_phase = "FAILURE" if break_error else game_state["game_phase"] + + # Send the initial state to the teams + team_names = [team.team_name for team in teams] team_state = { 'teams': teams, - 'team_names': team_names + 'team_names': team_names, + 'game_phase': game_phase } return team_state +def send_initial(game_state, allow_exceptions=False): + assert game_state["game_phase"] == "INIT" + + teams = game_state['teams'] + + for team_idx, team in enumerate(teams): + try: + _res = team.set_initial(team_idx, prepare_bot_state(game_state, team_idx)) + + except RemotePlayerFailure as e: + if allow_exceptions: + exit_remote_teams(game_state) + raise PelitaBotError(e.error_type, e.error_msg) + + exception_event = { + 'type': e.error_type, + 'description': e.error_msg, + 'turn': team_idx, + 'round': None, + } + game_state['fatal_errors'][team_idx].append(exception_event) + game_print(team_idx, f"{e.error_type}: {e.error_msg}") + + except RemotePlayerSendError: + if allow_exceptions: + exit_remote_teams(game_state) + raise PelitaBotError('Send error', 'Remote team unavailable') + + exception_event = { + 'type': 'Send error', + 'description': 'Remote team unavailable', + 'turn': team_idx, + 'round': None, + } + game_state['fatal_errors'][team_idx].append(exception_event) + game_print(team_idx, "Send error: Remote team unavailable") + + except RemotePlayerRecvTimeout: + if allow_exceptions: + exit_remote_teams(game_state) + raise PelitaBotError('timeout', 'Timeout in set initial') + + # Time out in set_initial + exception_event = { + 'type': 'timeout', + 'description': 'Timeout in set initial', + 'turn': team_idx, + 'round': None, + } + game_state['fatal_errors'][team_idx].append(exception_event) + game_print(team_idx, "timeout: Timeout in set initial") + def request_new_position(game_state): - team = game_state['turn'] % 2 - move_fun = game_state['teams'][team] + round = game_state['round'] + turn = game_state['turn'] + team_idx = game_state['turn'] % 2 + _bot_turn = game_state['turn'] // 2 + team = game_state['teams'][team_idx] bot_state = prepare_bot_state(game_state) + try: + start_time = time.monotonic() + bot_reply = team.get_move(bot_state) + + except RemotePlayerFailure as e: + bot_reply = { + 'error': e.error_type, + 'error_msg': e.error_msg + } + + except RemotePlayerSendError: + bot_reply = { + 'error': 'Send error', + 'error_msg': 'Remote team unavailable' + } + + except RemotePlayerRecvTimeout: + # There was a timeout. Execute a random move - start_time = time.monotonic() + legal_positions = get_legal_positions(game_state["walls"], game_state["shape"], + game_state["bots"][game_state["turn"]]) + req_position = game_state['rng'].choice(legal_positions) + game_print(turn, f"Player timeout. Setting a legal position at random: {req_position}") + + bot_reply = { + 'move': req_position, + 'error': 'timeout', + } + timeout_event = { + 'type': 'timeout', + 'description': f"Player timeout. Setting a legal position at random: {req_position}" + } + game_state['errors'][team_idx][(round, turn)] = timeout_event - new_position = move_fun.get_move(bot_state) duration = time.monotonic() - start_time # update the team_time - game_state['team_time'][team] += duration + game_state['team_time'][team_idx] += duration - return new_position + return bot_reply -def prepare_bot_state(game_state, idx=None): +def prepare_bot_state(game_state, team_idx=None): """ Prepares the bot’s game state for the current bot. """ - bot_initialization = game_state.get('turn') is None and idx is not None - bot_finalization = game_state.get('turn') is not None and idx is not None + bot_initialization = game_state.get('turn') is None and team_idx is not None + bot_finalization = game_state.get('turn') is not None and team_idx is not None if bot_initialization: # We assume that we are in get_initial phase - turn = idx + turn = team_idx bot_turn = None seed = game_state['rng'].randint(0, sys.maxsize) elif bot_finalization: # Called for remote players in _exit - turn = idx + turn = team_idx bot_turn = None seed = None else: @@ -622,7 +765,6 @@ def prepare_bot_state(game_state, idx=None): 'shape': game_state['shape'], # only in initial round 'seed': seed # only used in set_initial phase }) - return bot_state @@ -683,7 +825,6 @@ def prepare_viewer_state(game_state): return viewer_state - def play_turn(game_state, allow_exceptions=False): """ Plays the next turn of the game. @@ -712,47 +853,35 @@ def play_turn(game_state, allow_exceptions=False): game_state.update(update_food_age(game_state, team, SHADOW_DISTANCE)) game_state.update(relocate_expired_food(game_state, team, SHADOW_DISTANCE)) - # request a new move from the current team - try: - position_dict = request_new_position(game_state) - if "error" in position_dict: - error_type, error_string = position_dict['error'] - raise FatalException(f"Exception in client ({error_type}): {error_string}") - try: - position = tuple(position_dict['move']) - except TypeError as e: - raise NonFatalException(f"Type error {e}") + position_dict = request_new_position(game_state) + + if "error" in position_dict and not position_dict['error'] == 'timeout': + error_type = position_dict['error'] + error_string = position_dict.get('error_msg', '') - if position_dict.get('say'): - game_state['say'][game_state['turn']] = position_dict['say'] - else: - game_state['say'][game_state['turn']] = "" - except FatalException as e: if allow_exceptions: - raise + exit_remote_teams(game_state) + raise PelitaBotError(error_type, error_string) + # FatalExceptions (such as PlayerDisconnect) should immediately # finish the game exception_event = { - 'type': e.__class__.__name__, - 'description': str(e), + 'type': error_type, + 'description': error_string, 'turn': game_state['turn'], 'round': game_state['round'], } game_state['fatal_errors'][team].append(exception_event) position = None - game_print(turn, f"{type(e).__name__}: {e}") - except NonFatalException as e: - if allow_exceptions: - raise - # NonFatalExceptions (such as Timeouts and ValueErrors in the JSON handling) - # are collected and added to team_errors - exception_event = { - 'type': e.__class__.__name__, - 'description': str(e) - } - game_state['errors'][team][(round, turn)] = exception_event - position = None - game_print(turn, f"{type(e).__name__}: {e}") + game_print(turn, f"{error_type}: {error_string}") + + else: + position = position_dict['move'] + + if position_dict.get('say'): + game_state['say'][game_state['turn']] = position_dict['say'] + else: + game_state['say'][game_state['turn']] = "" # If the returned move looks okay, we add it to the list of requested moves old_position = game_state['bots'][turn] @@ -784,7 +913,8 @@ def play_turn(game_state, allow_exceptions=False): update_viewers(game_state) # exit remote teams in case we are game over - check_exit_remote_teams(game_state) + if game_state['gameover']: + exit_remote_teams(game_state) return game_state @@ -862,7 +992,9 @@ def apply_move(gamestate, bot_position): state of the game after applying current turn """ - # TODO is a timeout counted as an error? + # TODO: gamestate should be immutable + assert gamestate["game_phase"] == "RUNNING" + # define local variables bots = gamestate["bots"] turn = gamestate["turn"] @@ -905,14 +1037,6 @@ def apply_move(gamestate, bot_position): if gamestate['gameover']: return gamestate - # Now check if we must make a random move - if (n_round, turn) in team_errors: - # There was an error for this round and turn - # but the game is not over. - # We execute a random move - bot_position = gamestate['rng'].choice(legal_positions) - game_print(turn, f"Setting a legal position at random: {bot_position}") - # take step bots[turn] = bot_position _logger.info(f"Bot {turn} moves to {bot_position}.") @@ -943,6 +1067,7 @@ def apply_move(gamestate, bot_position): "kills": kills, "bot_was_killed": bot_was_killed, "errors": errors, + "game_phase": "RUNNING", } gamestate.update(gamestate_new) @@ -991,9 +1116,27 @@ def check_gameover(game_state, detect_final_move=False): Returns ------- - dict { 'gameover' , 'whowins' } + dict { 'gameover' , 'whowins', 'game_phase' } Flags if the game is over and who won it """ + if game_state['game_phase'] == 'FAILURE': + return { + 'whowins' : -1, + 'gameover' : True, + 'game_phase': game_state['game_phase'] + } + + if game_state['gameover']: + return { + 'whowins' : game_state['whowins'], + 'gameover' : game_state['gameover'], + 'game_phase': game_state['game_phase'] + } + if game_state['game_phase'] == 'INIT': + num_fatals = [len(f) for f in game_state['fatal_errors']] + if num_fatals[0] > 0 or num_fatals[1] > 0: + # TODO: We must flag failure + return { 'whowins' : -1, 'gameover' : True, 'game_phase': 'FAILURE' } # If any team has a fatal error, this team loses. # If both teams have a fatal error, it’s a draw. @@ -1003,12 +1146,12 @@ def check_gameover(game_state, detect_final_move=False): pass elif num_fatals[0] > 0 and num_fatals[1] > 0: # both teams have fatal errors: it is a draw - return { 'whowins' : 2, 'gameover' : True} + return { 'whowins' : 2, 'gameover' : True, 'game_phase': 'FINISHED' } else: # some one has fatal errors for team in (0, 1): if num_fatals[team] > 0: - return { 'whowins' : 1 - team, 'gameover' : True} + return { 'whowins' : 1 - team, 'gameover' : True, 'game_phase': 'FINISHED' } # If any team has reached error_limit errors, this team loses. # If both teams have reached error_limit errors, it’s a draw. @@ -1021,12 +1164,12 @@ def check_gameover(game_state, detect_final_move=False): pass elif num_errors[0] >= game_state['error_limit'] and num_errors[1] >= game_state['error_limit']: # both teams have reached or exceeded the error limit - return { 'whowins' : 2, 'gameover' : True} + return { 'whowins' : 2, 'gameover' : True, 'game_phase': 'FINISHED' } else: # only one team has reached the error limit for team in (0, 1): if num_errors[team] >= game_state['error_limit']: - return { 'whowins' : 1 - team, 'gameover' : True} + return { 'whowins' : 1 - team, 'gameover' : True, 'game_phase': 'FINISHED' } if detect_final_move: # No team wins/loses because of errors? @@ -1046,27 +1189,27 @@ def check_gameover(game_state, detect_final_move=False): whowins = 1 else: whowins = 2 - return { 'whowins' : whowins, 'gameover' : True} + return { 'whowins' : whowins, 'gameover' : True, 'game_phase': 'FINISHED' } - return { 'whowins' : None, 'gameover' : False} + return { 'whowins' : None, 'gameover' : False, 'game_phase': game_state['game_phase']} -def check_exit_remote_teams(game_state): +def exit_remote_teams(game_state): """ If the we are gameover, we want the remote teams to shut down. """ - if game_state['gameover']: - _logger.info("Gameover. Telling teams to exit.") - for idx, team in enumerate(game_state['teams']): - if len(game_state['fatal_errors'][idx]) > 0: - _logger.info(f"Not sending exit to team {idx} which had a fatal error.") - # We pretend we already send the exit message, otherwise - # the team’s __del__ method will do it once more. - team._sent_exit = True - continue - try: - team_game_state = prepare_bot_state(game_state, idx=idx) - team._exit(team_game_state) - except AttributeError: - pass + _logger.info("Telling teams to exit.") + for idx, team in enumerate(game_state['teams']): + if len(game_state['fatal_errors'][idx]) > 0: + _logger.info(f"Not sending exit to team {idx} which had a fatal error.") + # We pretend we already send the exit message, otherwise + # the team’s __del__ method will do it once more. + team._sent_exit = True + continue + try: + team_game_state = prepare_bot_state(game_state, team_idx=idx) + team._exit(team_game_state) + except AttributeError: + pass + def split_food(width, food): diff --git a/pelita/network.py b/pelita/network.py index 0fe0c064f..0c15edccd 100644 --- a/pelita/network.py +++ b/pelita/network.py @@ -1,4 +1,5 @@ +import enum import json import logging import sys @@ -27,28 +28,33 @@ # Request # {__uuid__, __action__, __data__} +# Status +# {__status__, __data__} + # Reply # {__uuid__, __return__} -# Error +# Error Reply # {__uuid__, __error__, __error_msg__} -class ZMQUnreachablePeer(Exception): +class RemotePlayerSendError(Exception): """ Raised when ZMQ cannot send a message (connection may have been lost). """ -class ZMQReplyTimeout(Exception): +class RemotePlayerRecvTimeout(Exception): """ Is raised when an ZMQ socket does not answer in time. """ - -class ZMQClientError(Exception): +class RemotePlayerFailure(Exception): """ Used to propagate errors from the client. Raised when the zmq connection receives an __error__ message. """ - def __init__(self, message, error_type, *args): - self.message = message + def __init__(self, error_type, error_msg): self.error_type = error_type - super().__init__(message, error_type, *args) + self.error_msg = error_msg + super().__init__(error_type, error_msg) + + def __str__(self): + return f"{self.error_type}: {self.error_msg}" #: The timeout to use during sending @@ -99,8 +105,17 @@ def json_default_handler(o): # we don’t know the type: raise a Type error raise TypeError("Cannot convert %r of type %s to json" % (o, type(o))) +class ConnectionState(enum.Enum): + UNCONNECTED = 1 + AWAIT_ACK = 2 + CONNECTED = 3 + CLOSED = 4 + + # CLOSE_WAIT?? + ERR = 5 -class ZMQConnection: + +class RemotePlayerConnection: """ This class is supposed to ease request–reply connections through a zmq socket. It does so by attaching a uuid to each request. It will only accept a reply if this also includes @@ -127,7 +142,8 @@ class ZMQConnection: pollout : zmq poller Poller for outgoing connections """ - def __init__(self, socket): + + def __init__(self, socket: zmq.Socket): self.socket = socket self.socket.setsockopt(zmq.LINGER, 0) @@ -139,37 +155,49 @@ def __init__(self, socket): self.pollout = zmq.Poller() self.pollout.register(socket, zmq.POLLOUT) - def send(self, action, data, timeout=None): + self.state = ConnectionState.UNCONNECTED + + def _send(self, action, data, msg_id): """ Sends a message or request `action` - and attached data to the socket and returns the - message id that is needed to receive the reply. + and attached data to the socket. """ - if timeout is None: - timeout = DEAD_CONNECTION_TIMEOUT + timeout = DEAD_CONNECTION_TIMEOUT - msg_id = str(uuid.uuid4()) - _logger.debug("---> %r [%s]", action, msg_id) + if msg_id is not None: + message_obj = {"__uuid__": msg_id, "__action__": action, "__data__": data} + _logger.debug("---> %r [%s]", action, msg_id) + else: + message_obj = {"__action__": action, "__data__": data} + _logger.debug("---> %r", action) # Check before sending that the socket can receive socks = dict(self.pollout.poll(timeout * 1000)) - if socks.get(self.socket) == zmq.POLLOUT: + if self.socket in socks and socks[self.socket] == zmq.POLLOUT: # I think we need to set NOBLOCK here, else we may run into a # race condition if a connection was closed between poll and send. # NOBLOCK should raise, so we can catch that - message_obj = {"__uuid__": msg_id, "__action__": action, "__data__": data} json_message = json.dumps(message_obj, cls=SetEncoder) try: self.socket.send_unicode(json_message, flags=zmq.NOBLOCK) except zmq.ZMQError as e: - _logger.info("Could not send message. Assume socket is unavailable. %r", e) - raise ZMQUnreachablePeer() + _logger.info("Could not send message. Socket is unavailable. %r", e) + raise RemotePlayerSendError() else: - raise ZMQUnreachablePeer() + raise RemotePlayerSendError() + return msg_id + + def send_req(self, action, data): + """ Sends a message or request `action` + and attached data to the socket and returns the + message id that is needed to receive the reply. + """ + msg_id = str(uuid.uuid4()) + self._send(action=action, data=data, msg_id=msg_id) return msg_id def _recv(self): - """ Receive the next message on the socket. + """ Receive the next message on the socket. Will wait forever Returns ------- @@ -180,35 +208,68 @@ def _recv(self): ------ ZMQReplyTimeout if the message cannot be parsed from JSON - ZMQClientError + PelitaRemoteError if an error message is returned """ json_message = self.socket.recv_unicode() try: py_obj = json.loads(json_message) except ValueError: - _logger.warning('Received non-json message from self. Triggering a timeout.') - raise ZMQReplyTimeout() + _logger.warning('Received non-json message.') + # TODO This should probably produce a failure + raise RemotePlayerRecvTimeout() - try: + if '__error__' in py_obj: error_type = py_obj['__error__'] error_message = py_obj.get('__error_msg__', '') _logger.warning(f'Received error reply ({error_type}): {error_message}. Closing socket.') self.socket.close() - raise ZMQClientError(error_message, error_type) - except KeyError: - pass - try: - msg_id = py_obj["__uuid__"] - except KeyError: - msg_id = None - _logger.warning('__uuid__ missing in message.') + self.state = ConnectionState.CLOSED + + # Failure in the pelita code on client side + raise RemotePlayerFailure(error_type, error_message) + + if '__uuid__' in py_obj: + msg_id = py_obj['__uuid__'] + msg_return = py_obj.get("__return__") + _logger.debug("<--- %r [%s]", msg_return, msg_id) + + return msg_id, msg_return - msg_return = py_obj.get("__return__") + if '__status__' in py_obj: + msg_ack = py_obj['__status__'] # == 'ok' + msg_data = py_obj.get('__data__') + _logger.debug("<--- %r %r", msg_ack, msg_data) + + self.state = ConnectionState.CONNECTED + + return None, msg_data + + print("NO MATCH", py_obj) + + return None, None + + + def recv_status(self, timeout): + """ Receive the next message on the socket. + + Returns + ------- + status + The message status + + Raises + ------ + ZMQReplyTimeout + if the message cannot be parsed from JSON + PelitaRemoteError + if an error message is returned + """ + status = self.recv_timeout(None, timeout) + + return status - _logger.debug("<--- %r [%s]", msg_return, msg_id) - return msg_id, msg_return def recv_timeout(self, expected_id, timeout): """ Waits `timeout` seconds for a reply with msg_id `expected_id`. @@ -226,15 +287,8 @@ def recv_timeout(self, expected_id, timeout): ZMQConnectionError if an error message is returned """ - # special case for no timeout - # just loop until we receive the correct reply - if timeout is None: - while True: - msg_id, reply = self._recv() - if msg_id == expected_id: - return reply - - # normal timeout handling + if self.state == ConnectionState.CLOSED: + return time_now = time.monotonic() # calculate until when it may take @@ -243,7 +297,7 @@ def recv_timeout(self, expected_id, timeout): # can still be handled timeout_until = time_now + timeout - while time_now < timeout_until: + while time_now <= timeout_until: time_left = timeout_until - time_now socks = dict(self.pollin.poll(time_left * 1000)) # poll needs milliseconds @@ -264,10 +318,10 @@ def recv_timeout(self, expected_id, timeout): # answer did not arrive in time break - raise ZMQReplyTimeout() + raise RemotePlayerRecvTimeout() def __repr__(self): - return "ZMQConnection(%r)" % self.socket + return "RemotePlayerConnection(%r)" % self.socket class ZMQPublisher: """ Sets up a simple Publisher which sends all viewed events @@ -293,6 +347,7 @@ def __init__(self, address, bind=True): def _send(self, action, data): info = {'round': data['round'], 'turn': data['turn']} + # TODO: this should be game_phase if data['gameover']: info['gameover'] = True _logger.debug(f"--#> [{action}] %r", info) @@ -351,15 +406,6 @@ def await_action(self, await_action, timeout=None, accept_exit=True): if action in expected_actions: return action _logger.warning('Unexpected action %r. (Expected: %s) Ignoring.', action, ", ".join(expected_actions)) - continue - - - def recv_start(self, timeout=None): - """ Waits `timeout` seconds for start message. - - Returns `True`, when the message arrives, `False` when an exit - message arrives or a timeout occurs. - """ def setup_controller(zmq_context=None): diff --git a/pelita/scripts/pelita_main.py b/pelita/scripts/pelita_main.py index 2ca4f6713..ba1264c5b 100755 --- a/pelita/scripts/pelita_main.py +++ b/pelita/scripts/pelita_main.py @@ -332,7 +332,7 @@ def main(): try: team_name = check_team(team_spec) print("NAME:", team_name) - except pelita.network.ZMQClientError as e: + except pelita.network.RemotePlayerFailure as e: if e.error_type == 'ModuleNotFoundError': #print(f"{e.message}") pass diff --git a/pelita/scripts/pelita_player.py b/pelita/scripts/pelita_player.py index 909e2b231..5de4bd444 100755 --- a/pelita/scripts/pelita_player.py +++ b/pelita/scripts/pelita_player.py @@ -12,12 +12,11 @@ import zmq from ..network import json_default_handler -from ..team import make_team +from ..team import Team from .script_utils import start_logging _logger = logging.getLogger(__name__) - @contextlib.contextmanager def with_sys_path(dirname): sys.path.insert(0, dirname) @@ -38,8 +37,6 @@ def run_player(team_spec, address, team_name_override=False, silent_bots=False): the address of the remote team socket """ - - address = address.replace('*', 'localhost') # Connect to the given address context = zmq.Context() socket = context.socket(zmq.PAIR) @@ -50,31 +47,25 @@ def run_player(team_spec, address, team_name_override=False, silent_bots=False): try: team = load_team(team_spec) + _logger.info(f"Running player '{team_spec}' ({team.team_name})") + except Exception as e: # We could not load the team. - # Wait for the set_initial message from the server - # and reply with an error. - try: - json_message = socket.recv_unicode() - py_obj = json.loads(json_message) - uuid_ = py_obj["__uuid__"] - _action = py_obj["__action__"] - _data = py_obj["__data__"] - - socket.send_json({ - '__uuid__': uuid_, - '__error__': e.__class__.__name__, - '__error_msg__': f'Could not load {team_spec}: {e}' - }) - except zmq.ZMQError as e: - raise IOError('failed to connect the client to address %s: %s' - % (address, e)) - # TODO: Do not raise here but wait for zmq to return a sensible error message - # We need a way to distinguish between syntax errors in the client - # and general zmq disconnects - raise + # Send an error to the server + + error = { + '__error__': e.__class__.__name__, + '__error_msg__': f'Could not load {team_spec}: {e}', + } + + socket.send_json(error) + # TODO: Exit with a status code? + return False - _logger.info(f"Running player '{team_spec}' ({team.team_name})") + data = { + 'team_name': team.team_name, + } + socket.send_json({'__status__': 'ok', '__data__': data}) while True: cont = player_handle_request(socket, team, team_name_override=team_name_override, silent_bots=silent_bots) @@ -111,7 +102,7 @@ def player_handle_request(socket, team, team_name_override=False, silent_bots=Fa try: py_obj = json.loads(json_message) - msg_id = py_obj["__uuid__"] + msg_id = py_obj.get("__uuid__") # if an uuid is given, we must reply with a value action = py_obj["__action__"] data = py_obj["__data__"] _logger.debug(" %r [%s]", message_obj['__error__'], msg_id) - else: - _logger.debug("o--> %r [%s]", message_obj['__return__'], msg_id) + if msg_id is not None: + # we use our own json_default_handler + # to automatically convert numpy ints to json + json_message = json.dumps(message_obj, default=json_default_handler) + # return the message + socket.send_unicode(json_message) + if '__error__' in message_obj: + _logger.warning("o-!> %r [%s]", message_obj['__error__'], msg_id) + else: + _logger.debug("o--> %r [%s]", message_obj['__return__'], msg_id) def check_team_name(name): @@ -270,13 +260,13 @@ def load_team_from_module(path: str): FileNotFoundError if the parent folder cannot be found """ - path = Path(path) + module_path = Path(path) - if not path.parent.exists(): - raise FileNotFoundError("Folder {} does not exist.".format(path.parent)) + if not module_path.parent.exists(): + raise FileNotFoundError("Folder {} does not exist.".format(module_path.parent)) - dirname = str(path.parent) - modname = path.stem + dirname = str(module_path.parent) + modname = module_path.stem if modname in sys.modules: raise ValueError("A module named ‘{}’ has already been imported.".format(modname)) @@ -290,15 +280,25 @@ def load_team_from_module(path: str): def team_from_module(module): """ Looks for a move function and a team name in `module` and returns a team. + + Raises + ------ + TypeError + move not a function or TEAM_NAME not a string + AttributeError + no move or no TEAM_NAME attribute + """ # look for a new-style team move = module.move name = module.TEAM_NAME + if not callable(move): raise TypeError("move is not a function") if not isinstance(name, str): raise TypeError("TEAM_NAME is not a string") - team, _ = make_team(move, team_name=name) + + team = Team(move, team_name=name) return team diff --git a/pelita/scripts/pelita_server.py b/pelita/scripts/pelita_server.py index 09de10c7f..3db613a1a 100755 --- a/pelita/scripts/pelita_server.py +++ b/pelita/scripts/pelita_server.py @@ -258,14 +258,17 @@ def handle_new_connection(self, dealer_id, message, progress): if msg_obj.get('TEAM') == 'ADD': team_info = load_team_info(team_spec) + if not team_info: + return self.team_infos.append(team_info) - info = zeroconf_register(self.zc, self.advertise, self.port, team.spec, team.server_path, print=progress.console.print) + # TODO broken + info = zeroconf_register(self.zc, self.advertise, self.port, team_info.spec, team_info.server_path, print=progress.console.print) if info: - self.team_serviceinfo_mapping[(team.spec, team.server_path)] = info + self.team_serviceinfo_mapping[(team_info.spec, team_info.server_path)] = info if msg_obj.get('TEAM') == 'REMOVE': # TODO: cannot remove from self.team_infos yet - info = self.team_serviceinfo_mapping[(team.spec, team.server_path)] + info = self.team_serviceinfo_mapping[(team_info.spec, team_info.server_path)] zeroconf_deregister(self.zc, info) elif "SCAN" in msg_obj: @@ -334,7 +337,8 @@ def handle_new_connection(self, dealer_id, message, progress): # Send a reply to the requester (that the process has started) # Otherwise they might already start querying for the team name - self.router_sock.send_multipart([dealer_id, b"OK"]) + ## TODO: Still needed? + # self.router_sock.send_multipart([dealer_id, b"OK"]) else: _logger.info("Unknown incoming DEALER and not a request.") diff --git a/pelita/team.py b/pelita/team.py index ad6272edb..b92d65c1b 100644 --- a/pelita/team.py +++ b/pelita/team.py @@ -7,6 +7,7 @@ from io import StringIO from pathlib import Path from random import Random +import typing from urllib.parse import urlparse import networkx as nx @@ -14,10 +15,8 @@ from . import layout from .base_utils import default_zmq_context -from .exceptions import PlayerDisconnected, PlayerTimeout from .layout import BOT_I2N, layout_as_str, wall_dimensions -from .network import (PELITA_PORT, ZMQClientError, ZMQConnection, - ZMQReplyTimeout, ZMQUnreachablePeer) +from .network import PELITA_PORT, RemotePlayerConnection, RemotePlayerSendError _logger = logging.getLogger(__name__) @@ -143,8 +142,18 @@ class Team: the team’s move function team_name : the name of the team (optional) + + Raises + ------ + TypeError : Move is not a function or team_name is not a string """ - def __init__(self, team_move, *, team_name=""): + def __init__(self, team_move: typing.Callable[[typing.Any, typing.Any], typing.Tuple[int, int]], *, team_name=""): + if not callable(team_move): + raise TypeError("move is not a function") + + if not isinstance(team_name, str): + raise TypeError("TEAM_NAME is not a string") + self._team_move = team_move self.team_name = team_name @@ -168,12 +177,6 @@ def set_initial(self, team_id, game_state): The id of the team game_state : dict The initial game state - - Returns - ------- - Team name : string - The name of the team - """ # Reset the team state self._state.clear() @@ -202,8 +205,7 @@ def set_initial(self, team_id, game_state): # over self._graph = walls_to_graph(self._walls, shape=self._shape).copy(as_view=True) - return self.team_name - + # TODO: get_move could also take the main game state??? def get_move(self, game_state): """ Requests a move from the Player who controls the Bot with id `bot_id`. @@ -249,32 +251,68 @@ def get_move(self, game_state): mybot.track = self._bot_track[idx][:] + move = self.apply_move_fn(self._team_move, team[me._bot_turn], self._state) + if "error" not in move: + move["say"] = me._say + return move + + @staticmethod + def apply_move_fn(move_fn, bot: "Bot", state): try: # request a move from the current bot - move = self._team_move(team[me._bot_turn], self._state) - - # check that the returned value is a position tuple - try: - if len(move) != 2: - raise ValueError(f"Function move did not return a valid position: got {move} instead.") - except TypeError: - # Convert to ValueError - raise ValueError(f"Function move did not return a valid position: got {move} instead.") from None + move = move_fn(bot, state) except Exception as e: # Our client had an exception. We print a traceback and # return the type of the exception to the server. # If this is a remote player, then this will be detected in pelita_player # and pelita_player will close the connection automatically. - traceback.print_exc() + + # Stacktrace is not needed, when we raise the ValueError above! + # from rich.console import Console + # console = Console() + + # if bot.is_blue: + # console.print(f"Team [blue]{bot.team_name}[/] caused an exception:") + # else: + # console.print(f"Team [red]{bot.team_name}[/] caused an exception:") + + # console.print_exception(show_locals=True) #, suppress=["pelita"]) + + try: + import _colorize + colorize = _colorize.can_colorize() + # This probably only works from Python 3.13 onwards + traceback.print_exception(sys.exception(), limit=None, file=None, chain=True, colorize=colorize) + except (ImportError, AttributeError): + traceback.print_exc() + return { - "error": (type(e).__name__, str(e)), + "error": type(e).__name__, + "error_msg": str(e), } - return { - "move": move, - "say": me._say + # check that the returned value is a position tuple + try: + if len(move) == 2: + return { "move": move } + + except TypeError: + pass + + error = { + "error": "ValueError", + "error_msg": f"Function move did not return a valid position: got '{move}' instead." } + from rich.console import Console + console = Console() + console.print(f"[b][red]{error['error']}[/red][/b]: {error['error_msg']}") + + # If move cannot take len, we get a type error; convert it to a ValueError + return error + + + def _exit(self, game_state=None): """ Dummy function. Only needed for `RemoteTeam`. """ pass @@ -282,6 +320,149 @@ def _exit(self, game_state=None): def __repr__(self): return f'Team({self._team_move!r}, {self.team_name!r})' +## TODO: +# Team -> Team + +# Split class RemoteTeam in two: +# One class for handling connections to a server-run Pelita client +# One class that starts its own subprocess and owns it +# +# In the first case, a connection is started with a zmq message +# The game code (setup_teams) is then responsible for awaiting +# the success message +# +# In the second case, the remote client needs to send a success message +# after it has started. The game code (setup_teams) is responsible for awaiting this message +# +# With the Ok message that the process has started, the team name should be included. +# +# The set initial req–rep should be made separately. This ensures that set initial includes the team names +# and the team names do not need to be send during regular move requests. +# +# Rename set_initial -> start_game +# +class SubprocessTeam: + def __init__(self, team_spec, *, zmq_context=None, idx=None, store_output=False): + zmq_context = default_zmq_context(zmq_context) + + self._team_spec = team_spec + self.team_name = None + + #: Default timeout for a request, unless specified in the game_state + self._request_timeout = 3 + + # TODO: State machine -- or add to ZMQConnection? + self._state = "" + + # We bind to a local tcp port with a zmq PAIR socket + # and start a new subprocess of pelita_player.py + # that includes the address of that socket and the + # team_spec as command line arguments. + # The subprocess will then connect to this address + # and load the team. + + socket = zmq_context.socket(zmq.PAIR) + port = socket.bind_to_random_port('tcp://localhost') + self.bound_to_address = f"tcp://localhost:{port}" + if idx == 0: + color='blue' + elif idx == 1: + color='red' + else: + color='' + self.proc, self.stdout_path, self.stderr_path = self._call_pelita_player(team_spec, self.bound_to_address, + color=color, store_output=store_output) + + self.zmqconnection = RemotePlayerConnection(socket) + + def _call_pelita_player(self, team_spec, address, color='', store_output=False): + """ Starts another process with the same Python executable and runs `team_spec` + as a standalone client on URL `addr`. + """ + player = 'pelita.scripts.pelita_player' + external_call = [sys.executable, + '-m', + player, + 'remote-game', + team_spec, + address] + + _logger.debug("Executing: %r", external_call) + if store_output == subprocess.DEVNULL: + return (subprocess.Popen(external_call, stdout=store_output), None, None) + elif store_output: + store_path = Path(store_output) + stdout_path = (store_path / f"{color or team_spec}.out") + stderr_path = (store_path / f"{color or team_spec}.err") + + # We must run in unbuffered mode to enforce flushing of stdout/stderr, + # otherwise we may lose some of what is printed + proc = subprocess.Popen(external_call, stdout=stdout_path.open('w'), stderr=stderr_path.open('w'), + env=dict(os.environ, PYTHONUNBUFFERED='x')) + return (proc, stdout_path, stderr_path) + else: + return (subprocess.Popen(external_call), None, None) + + def wait_ready(self, timeout): + msg = self.zmqconnection.recv_status(timeout) + self.team_name = msg.get('team_name') + + def set_initial(self, team_id, game_state): + + timeout_length = game_state['timeout_length'] + msg_id = self.zmqconnection.send_req("set_initial", {"team_id": team_id, + "game_state": game_state}) + reply = self.zmqconnection.recv_timeout(msg_id, timeout_length) + # TODO check error in reply + return reply + + def get_move(self, game_state): + timeout_length = game_state['timeout_length'] + + msg_id = self.zmqconnection.send_req("get_move", {"game_state": game_state}) + reply = self.zmqconnection.recv_timeout(msg_id, timeout_length) + if "error" in reply: + return reply + # make sure that the move is a tuple + reply["move"] = tuple(reply.get("move")) + # TODO: handle ValueError + return reply + + def _exit(self, game_state=None): + # We only want to exit once. + if getattr(self, '_sent_exit', False): + return + + if game_state: + payload = {'game_state': game_state} + else: + payload = {} + + try: + # TODO: make zmqconnection stateful. set flag when already disconnected + # For now, we simply check the state of the socket so that we do not send + # over an already closed socket. + if self.zmqconnection.socket.closed: + return + + self.zmqconnection.send_req("exit", payload) + self._sent_exit = True + except RemotePlayerSendError: + _logger.info("Remote Player %r is already dead during exit. Ignoring.", self) + + def __del__(self): + try: + self._exit() + if self.proc: + self.proc.terminate() + except AttributeError: + # in case we exit before self.proc or self.zmqconnection have been set + pass + + def __repr__(self): + team_name = f" ({self.team_name})" if self.team_name is not None else "" + return f"SubprocessTeam<{self._team_spec}{team_name} on {self.bound_to_address}>" + class RemoteTeam: """ Start a child process with the given `team_spec` and handle @@ -309,7 +490,8 @@ class RemoteTeam: the remote clients will be suppressed. """ def __init__(self, team_spec, *, team_name=None, zmq_context=None, idx=None, store_output=False): - zmq_context = default_zmq_context(zmq_context) + if zmq_context is None: + zmq_context = zmq.Context() self._team_spec = team_spec self._team_name = team_name @@ -317,153 +499,66 @@ def __init__(self, team_spec, *, team_name=None, zmq_context=None, idx=None, sto #: Default timeout for a request, unless specified in the game_state self._request_timeout = 3 - if team_spec.startswith('pelita://'): - # We connect to a remote player that is listening - # on the given team_spec address. - # We create a new DEALER socket and send a single - # REQUEST message to the remote address. - # The remote player will then create a new instance - # of a player and forward all of our zmq traffic - # to that player. - - # given a url pelita://hostname:port/path we extract hostname and port and - # convert it to tcp://hostname:port that we use for the zmq connection - parsed_url = urlparse(team_spec) - if parsed_url.port: - port = parsed_url.port - else: - port = PELITA_PORT - send_addr = f"tcp://{parsed_url.hostname}:{port}" - address = "tcp://*" - self.bound_to_address = address - - socket = zmq_context.socket(zmq.DEALER) - socket.setsockopt(zmq.LINGER, 0) - socket.connect(send_addr) - _logger.info("Connecting zmq.DEALER to remote player at {}.".format(send_addr)) - - socket.send_json({"REQUEST": team_spec}) - WAIT_TIMEOUT = 5000 - incoming = socket.poll(timeout=WAIT_TIMEOUT) - if incoming == zmq.POLLIN: - _ok = socket.recv() - else: - # Server did not respond - raise PlayerTimeout() - self.proc = None - else: - # We bind to a local tcp port with a zmq PAIR socket - # and start a new subprocess of pelita_player.py - # that includes the address of that socket and the - # team_spec as command line arguments. - # The subprocess will then connect to this address - # and load the team. - - socket = zmq_context.socket(zmq.PAIR) - port = socket.bind_to_random_port('tcp://*') - self.bound_to_address = f"tcp://localhost:{port}" - if idx == 0: - color='blue' - elif idx == 1: - color='red' - else: - color='' - self.proc = self._call_pelita_player(team_spec, self.bound_to_address, - color=color, store_output=store_output) - - self.zmqconnection = ZMQConnection(socket) + # We connect to a remote player that is listening + # on the given team_spec address. + # We create a new DEALER socket and send a single + # REQUEST message to the remote address. + # The remote player will then create a new instance + # of a player and forward all of our zmq traffic + # to that player. - def _call_pelita_player(self, team_spec, address, color='', store_output=False): - """ Starts another process with the same Python executable, - the same start script (pelitagame) and runs `team_spec` - as a standalone client on URL `addr`. - """ - player = 'pelita.scripts.pelita_player' - external_call = [sys.executable, - '-m', - player, - 'remote-game', - team_spec, - address] - - _logger.debug("Executing: %r", external_call) - if store_output == subprocess.DEVNULL: - return (subprocess.Popen(external_call, stdout=store_output), None, None) - elif store_output: - store_path = Path(store_output) - stdout = (store_path / f"{color or team_spec}.out").open('w') - stderr = (store_path / f"{color or team_spec}.err").open('w') - - # We must run in unbuffered mode to enforce flushing of stdout/stderr, - # otherwise we may lose some of what is printed - proc = subprocess.Popen(external_call, stdout=stdout, stderr=stderr, - env=dict(os.environ, PYTHONUNBUFFERED='x')) - return (proc, stdout, stderr) + # given a url pelita://hostname:port/path we extract hostname and port and + # convert it to tcp://hostname:port that we use for the zmq connection + parsed_url = urlparse(team_spec) + if parsed_url.port: + port = parsed_url.port else: - return (subprocess.Popen(external_call), None, None) - - @property - def team_name(self): - if self._team_name is not None: - return self._team_name - - try: - msg_id = self.zmqconnection.send("team_name", {}) - team_name = self.zmqconnection.recv_timeout(msg_id, self._request_timeout) - if team_name: - self._team_name = team_name - return team_name - except ZMQReplyTimeout: - _logger.info("Detected a timeout, returning a string nonetheless.") - return "%error%" - except ZMQUnreachablePeer: - _logger.info("Detected a DeadConnection, returning a string nonetheless.") - return "%error%" + port = PELITA_PORT + send_addr = f"tcp://{parsed_url.hostname}:{port}" + self.bound_to_address = send_addr + + socket = zmq_context.socket(zmq.DEALER) + socket.setsockopt(zmq.LINGER, 0) + socket.connect(send_addr) + _logger.info("Connecting zmq.DEALER to remote player at {}.".format(send_addr)) + + socket.send_json({"REQUEST": team_spec}) + # WAIT_TIMEOUT = 5000 + # incoming = socket.poll(timeout=WAIT_TIMEOUT) + # if incoming == zmq.POLLIN: + # ok = socket.recv() + # else: + # # Server did not respond + # raise PlayerTimeout() + # self.proc = None + + self.zmqconnection = RemotePlayerConnection(socket) + + def wait_ready(self, timeout): + msg = self.zmqconnection.recv_status(timeout) + self.team_name = msg.get('team_name') def set_initial(self, team_id, game_state): + timeout_length = game_state['timeout_length'] - try: - msg_id = self.zmqconnection.send("set_initial", {"team_id": team_id, - "game_state": game_state}) - team_name = self.zmqconnection.recv_timeout(msg_id, timeout_length) - if team_name: - self._team_name = team_name - return team_name - except ZMQReplyTimeout: - # answer did not arrive in time - raise PlayerTimeout() - except ZMQUnreachablePeer: - _logger.info("Could not properly send the message. Maybe just a slow client. Ignoring in set_initial.") - except ZMQClientError as e: - error_message = e.message - error_type = e.error_type - _logger.warning(f"Client connection failed ({error_type}): {error_message}") - raise PlayerDisconnected(*e.args) from None + msg_id = self.zmqconnection.send_req("set_initial", {"team_id": team_id, + "game_state": game_state}) + reply = self.zmqconnection.recv_timeout(msg_id, timeout_length) + # TODO check error in reply + return reply def get_move(self, game_state): timeout_length = game_state['timeout_length'] - try: - msg_id = self.zmqconnection.send("get_move", {"game_state": game_state}) - reply = self.zmqconnection.recv_timeout(msg_id, timeout_length) - # make sure it is a dict - reply = dict(reply) - if "error" in reply: - return reply - # make sure that the move is a tuple - reply["move"] = tuple(reply.get("move")) + + msg_id = self.zmqconnection.send_req("get_move", {"game_state": game_state}) + reply = self.zmqconnection.recv_timeout(msg_id, timeout_length) + if "error" in reply: return reply - except ZMQReplyTimeout: - # answer did not arrive in time - raise PlayerTimeout() - except TypeError: - # if we could not convert into a tuple or dict (e.g. bad reply) - return None - except ZMQUnreachablePeer: - # if the remote connection is closed - raise PlayerDisconnected() - except ZMQClientError: - raise + # make sure that the move is a tuple + reply["move"] = tuple(reply.get("move")) + # TODO: handle ValueError + return reply def _exit(self, game_state=None): # We only want to exit once. @@ -481,10 +576,10 @@ def _exit(self, game_state=None): # over an already closed socket. if self.zmqconnection.socket.closed: return - # TODO: Include final state with exit message - self.zmqconnection.send("exit", payload, timeout=1) + + self.zmqconnection.send_req("exit", payload) self._sent_exit = True - except ZMQUnreachablePeer: + except RemotePlayerSendError: _logger.info("Remote Player %r is already dead during exit. Ignoring.", self) def __del__(self): @@ -533,10 +628,14 @@ def make_team(team_spec, team_name=None, zmq_context=None, idx=None, store_outpu team_name = f'local-team ({team_spec.__name__})' team_player = Team(team_spec, team_name=team_name) elif isinstance(team_spec, str): - _logger.info("Making a remote team for %s", team_spec) # set up the zmq connections and build a RemoteTeam zmq_context = default_zmq_context(zmq_context) - team_player = RemoteTeam(team_spec=team_spec, zmq_context=zmq_context, idx=idx, store_output=store_output) + if team_spec.startswith('pelita://'): + _logger.info("Making a remote team for %s", team_spec) + team_player = RemoteTeam(team_spec=team_spec, zmq_context=zmq_context, idx=idx, store_output=store_output) + else: + _logger.info("Making a subprocess team for %s", team_spec) + team_player = SubprocessTeam(team_spec=team_spec, zmq_context=zmq_context, idx=idx, store_output=store_output) else: raise TypeError(f"Not possible to create team from {team_spec} (wrong type).") diff --git a/pelita/tournament/__init__.py b/pelita/tournament/__init__.py index 740db2161..37f6ef603 100644 --- a/pelita/tournament/__init__.py +++ b/pelita/tournament/__init__.py @@ -31,8 +31,12 @@ def check_team(team_spec): + """ Instantiates a team from a team_spec and returns its name """ team, _zmq_context = make_team(team_spec) + if hasattr(team, 'zmqconnection'): + # TODO: Handle timeout + team.wait_ready(3) return team.team_name diff --git a/test/test_game.py b/test/test_game.py index 9215a50b6..3d3ab6509 100644 --- a/test/test_game.py +++ b/test/test_game.py @@ -9,8 +9,8 @@ import pytest -from pelita import game, maze_generator -from pelita.exceptions import NoFoodWarning +from pelita import game, layout, maze_generator +from pelita.exceptions import NoFoodWarning, PelitaBotError from pelita.game import (apply_move, get_legal_positions, initial_positions, play_turn, run_game, setup_game) from pelita.layout import parse_layout @@ -769,7 +769,8 @@ def test_play_turn_move(): "bot_was_killed": [False]*4, "errors": [[], []], "fatal_errors": [{}, {}], - "rng": Random() + "rng": Random(), + "game_phase": "RUNNING", } legal_positions = get_legal_positions(game_state["walls"], game_state["shape"], game_state["bots"][turn]) game_state_new = apply_move(game_state, legal_positions[0]) @@ -816,8 +817,8 @@ def move(bot, s): assert final_state['gameover'] assert final_state['whowins'] == 1 assert final_state['fatal_errors'][0][0] == { - 'type': 'FatalException', - 'description': 'Exception in client (RuntimeError): We should not be here in this test', + 'type': 'RuntimeError', + 'description': 'We should not be here in this test', 'round': 2, 'turn': 0, } @@ -845,29 +846,27 @@ def test_update_round_counter(): 'round': round0, 'gameover': True,}) - -def test_last_round_check(): - # (max_rounds, current_round, turn): gameover - test_map = { - (1, None, None): False, - (1, 1, 0): False, - (1, 1, 3): True, +@pytest.mark.parametrize( + 'max_rounds, current_round, turn, game_phase, gameover', [ + [1, None, None, 'INIT', False], + [1, 1, 0, 'RUNNING', False], + [1, 1, 3, 'RUNNING', True], + ]) +def test_last_round_check(max_rounds, current_round, turn, game_phase, gameover): + state = { + 'max_rounds': max_rounds, + 'round': current_round, + 'turn': turn, + 'error_limit': 5, + 'fatal_errors': [[],[]], + 'errors': [[],[]], + 'gameover': False, + 'score': [0, 0], + 'food': [{(1,1)}, {(1,1)}], # dummy food + 'game_phase': game_phase } - for test_val, test_res in test_map.items(): - max_rounds, current_round, current_turn = test_val - state = { - 'max_rounds': max_rounds, - 'round': current_round, - 'turn': current_turn, - 'error_limit': 5, - 'fatal_errors': [[],[]], - 'errors': [[],[]], - 'gameover': False, - 'score': [0, 0], - 'food': [{(1,1)}, {(1,1)}] # dummy food - } - res = game.check_gameover(state, detect_final_move=True) - assert res['gameover'] == test_res + res = game.check_gameover(state, detect_final_move=True) + assert res['gameover'] == gameover @pytest.mark.parametrize( @@ -898,6 +897,8 @@ def test_error_finishes_game(team_errors, team_wins): (fatal_0, errors_0), (fatal_1, errors_1) = team_errors # just faking a bunch of errors in our game state state = { + "game_phase": "RUNNING", + "gameover": False, "error_limit": 5, "fatal_errors": [[None] * fatal_0, [None] * fatal_1], "errors": [[None] * errors_0, [None] * errors_1] @@ -979,15 +980,26 @@ def move(b, s): def test_non_existing_file(): - # TODO: Change error message to be more meaningful l = maze_generator.generate_maze() res = run_game(["blah", "nothing"], max_rounds=1, layout_dict=l) - assert res['fatal_errors'][0][0] == { - 'description': '("Could not load blah: No module named \'blah\'", \'ModuleNotFoundError\')', - 'round': None, - 'turn': 0, - 'type': 'PlayerDisconnected' - } + print(res['fatal_errors']) + + # We might only catch only one of the errors + assert len(res['fatal_errors'][0]) > 0 or len(res['fatal_errors'][1]) > 0 + if len(res['fatal_errors'][0]) > 0: + assert res['fatal_errors'][0][0] == { + 'description': "ModuleNotFoundError: Could not load blah: No module named \'blah\'", + 'round': None, + 'turn': 0, + 'type': 'RemotePlayerFailure' + } + if len(res['fatal_errors'][1]) > 0: + assert res['fatal_errors'][1][0] == { + 'description': "ModuleNotFoundError: Could not load nothing: No module named \'nothing\'", + 'round': None, + 'turn': 1, + 'type': 'RemotePlayerFailure' + } # TODO: Get it working again on Windows @pytest.mark.skipif(_mswindows, reason="Test fails on some Python versions.") @@ -1000,34 +1012,55 @@ def test_remote_errors(tmp_path): l = maze_generator.generate_maze() res = run_game([str(syntax_error), str(import_error)], layout_dict=l, max_rounds=20) - # Error messages have changed in Python 3.10. We can only do approximate matching - assert "SyntaxError" in res['fatal_errors'][0][0].pop('description') - assert res['fatal_errors'][0][0] == { - 'round': None, - 'turn': 0, - 'type': 'PlayerDisconnected' - } - # Both teams fail during setup: DRAW - assert res['whowins'] == 2 + print(res['fatal_errors']) + + # We might only catch only one of the errors + assert len(res['fatal_errors'][0]) > 0 or len(res['fatal_errors'][1]) > 0 + if len(res['fatal_errors'][0]) > 0: + # Error messages have changed in Python 3.10. We can only do approximate matching + assert "SyntaxError" in res['fatal_errors'][0][0].pop('description') + assert res['fatal_errors'][0][0] == { + 'round': None, + 'turn': 0, + 'type': 'RemotePlayerFailure' + } + + if len(res['fatal_errors'][1]) > 0: + assert "ModuleNotFoundError" in res['fatal_errors'][1][0].pop('description') + assert res['fatal_errors'][1][0] == { + 'round': None, + 'turn': 1, + 'type': 'RemotePlayerFailure' + } + + # Both teams fail during setup: FAILURE + assert res['game_phase'] == "FAILURE" + assert res['gameover'] is True + assert res['whowins'] == -1 + res = run_game(["0", str(import_error)], layout_dict=l, max_rounds=20) # Error messages have changed in Python 3.10. We can only do approximate matching assert "ModuleNotFoundError" in res['fatal_errors'][1][0].pop('description') assert res['fatal_errors'][1][0] == { 'round': None, 'turn': 1, - 'type': 'PlayerDisconnected' + 'type': 'RemotePlayerFailure' } - assert res['whowins'] == 0 + assert res['game_phase'] == "FAILURE" + assert res['gameover'] is True + assert res['whowins'] == -1 + res = run_game([str(import_error), "1"], layout_dict=l, max_rounds=20) # Error messages have changed in Python 3.10. We can only do approximate matching assert "ModuleNotFoundError" in res['fatal_errors'][0][0].pop('description') assert res['fatal_errors'][0][0] == { 'round': None, 'turn': 0, - 'type': 'PlayerDisconnected' + 'type': 'RemotePlayerFailure' } - assert res['whowins'] == 1 - + assert res['game_phase'] == "FAILURE" + assert res['gameover'] is True + assert res['whowins'] == -1 @pytest.mark.parametrize('team_to_test', [0, 1]) def test_bad_move_function(team_to_test): @@ -1055,30 +1088,35 @@ def test_run_game(move): teams = [stopping, move] return run_game(teams, layout_dict=l, max_rounds=10) + res = test_run_game(stopping) + assert res['gameover'] + assert res['whowins'] == 2 + assert res['fatal_errors'] == [[], []] + other = 1 - team_to_test res = test_run_game(move0) assert res['gameover'] assert res['whowins'] == other - assert res['fatal_errors'][team_to_test][0]['type'] == 'FatalException' - assert res['fatal_errors'][team_to_test][0]['description'] == 'Exception in client (ValueError): Function move did not return a valid position: got None instead.' + assert res['fatal_errors'][team_to_test][0]['type'] == 'ValueError' + assert res['fatal_errors'][team_to_test][0]['description'] == "Function move did not return a valid position: got 'None' instead." res = test_run_game(move1) assert res['gameover'] assert res['whowins'] == other - assert res['fatal_errors'][team_to_test][0]['type'] == 'FatalException' - assert res['fatal_errors'][team_to_test][0]['description'] == 'Exception in client (ValueError): Function move did not return a valid position: got 0 instead.' + assert res['fatal_errors'][team_to_test][0]['type'] == 'ValueError' + assert res['fatal_errors'][team_to_test][0]['description'] == "Function move did not return a valid position: got '0' instead." res = test_run_game(move3) assert res['gameover'] assert res['whowins'] == other - assert res['fatal_errors'][team_to_test][0]['type'] == 'FatalException' - assert res['fatal_errors'][team_to_test][0]['description'] == 'Exception in client (ValueError): Function move did not return a valid position: got (0, 0, 0) instead.' + assert res['fatal_errors'][team_to_test][0]['type'] == 'ValueError' + assert res['fatal_errors'][team_to_test][0]['description'] == "Function move did not return a valid position: got '(0, 0, 0)' instead." res = test_run_game(move4) assert res['gameover'] assert res['whowins'] == other - assert res['fatal_errors'][team_to_test][0]['type'] == 'FatalException' + assert res['fatal_errors'][team_to_test][0]['type'] == 'TypeError' assert "takes 1 positional argument but 2 were given" in res['fatal_errors'][team_to_test][0]['description'] @@ -1219,8 +1257,8 @@ def test_remote_game_closes_players_on_exit(): state = run_game(["0", "1"], layout_dict=l, max_rounds=20, allow_exceptions=True) assert state["gameover"] # Check that both processes have exited - assert state["teams"][0].proc[0].wait(timeout=3) == 0 - assert state["teams"][1].proc[0].wait(timeout=3) == 0 + assert state["teams"][0].proc.wait(timeout=3) == 0 + assert state["teams"][1].proc.wait(timeout=3) == 0 def test_manual_remote_game_closes_players(): @@ -1232,13 +1270,13 @@ def test_manual_remote_game_closes_players(): while not state["gameover"]: # still running # still running - assert state["teams"][0].proc[0].poll() is None - assert state["teams"][1].proc[0].poll() is None + assert state["teams"][0].proc.poll() is None + assert state["teams"][1].proc.poll() is None state = play_turn(state) # Check that both processes have exited - assert state["teams"][0].proc[0].wait(timeout=3) == 0 - assert state["teams"][1].proc[0].wait(timeout=3) == 0 + assert state["teams"][0].proc.wait(timeout=3) == 0 + assert state["teams"][1].proc.wait(timeout=3) == 0 def test_invalid_setup_game_closes_players(): @@ -1248,8 +1286,23 @@ def test_invalid_setup_game_closes_players(): state = setup_game(["0", "1"], layout_dict=l, max_rounds=0, allow_exceptions=True) assert state["gameover"] # Check that both processes have exited - assert state["teams"][0].proc[0].wait(timeout=3) == 0 - assert state["teams"][1].proc[0].wait(timeout=3) == 0 + assert state["teams"][0].proc.wait(timeout=3) == 0 + assert state["teams"][1].proc.wait(timeout=3) == 0 + +def test_raises_and_exits_cleanly(): + l = layout.parse_layout(small_layout) + + path = FIXTURE_DIR / "player_move_division_by_zero" + state = setup_game([str(path), "1"], layout_dict=l, max_rounds=2) + with pytest.raises(PelitaBotError): + while not state["gameover"]: + state = play_turn(state, allow_exceptions=True) + + # This is the state before the exception + assert state["gameover"] is False + # Check that both processes have exited + assert state["teams"][0].proc.wait(timeout=3) == 0 + assert state["teams"][1].proc.wait(timeout=3) == 0 @pytest.mark.parametrize('move_request, expected_prev, expected_req, expected_success', [ diff --git a/test/test_libpelita.py b/test/test_libpelita.py index 4b9a7b92c..63a3698ba 100644 --- a/test/test_libpelita.py +++ b/test/test_libpelita.py @@ -5,7 +5,7 @@ import pytest -from pelita.network import ZMQClientError +from pelita.network import RemotePlayerFailure from pelita.scripts.pelita_tournament import firstNN from pelita.tournament import call_pelita, check_team @@ -71,7 +71,7 @@ def test_check_team_external(): assert check_team("pelita/player/StoppingPlayer") == "Stopping Players" def test_check_team_external_fails(): - with pytest.raises(ZMQClientError): + with pytest.raises(RemotePlayerFailure): check_team("Unknown Module") def test_check_team_internal(): diff --git a/test/test_network.py b/test/test_network.py index a59145d5d..a0ac197b9 100644 --- a/test/test_network.py +++ b/test/test_network.py @@ -68,7 +68,7 @@ def stopping(bot, state): assert sock.recv_json() == { '__uuid__': _uuid, - '__return__': 'test stopping player' + '__return__': None } _uuid = uuid.uuid4().__str__() diff --git a/test/test_remote_game.py b/test/test_remote_game.py index ac470726c..3b638bba0 100644 --- a/test/test_remote_game.py +++ b/test/test_remote_game.py @@ -158,8 +158,8 @@ def test_remote_dumps_with_failure(failing_team): fail_turn = 0 elif failing_team == 1: fail_turn = 1 - assert state['fatal_errors'][failing_team][0] == {'type': 'FatalException', - 'description': 'Exception in client (ZeroDivisionError): division by zero', + assert state['fatal_errors'][failing_team][0] == {'type': 'ZeroDivisionError', + 'description': 'division by zero', 'turn': fail_turn, 'round': 2} assert state['fatal_errors'][1 - failing_team] == [] @@ -231,9 +231,10 @@ def test_remote_move_failures(player_name, is_setup_error, error_type): max_rounds=2, layout_dict=pelita.layout.parse_layout(layout)) - assert state['whowins'] == 1 + assert state['whowins'] == -1 + assert state['game_phase'] == 'FAILURE' - assert state['fatal_errors'][0][0]['type'] == 'PlayerDisconnected' + assert state['fatal_errors'][0][0]['type'] == 'RemotePlayerFailure' assert 'Could not load' in state['fatal_errors'][0][0]['description'] assert error_type in state['fatal_errors'][0][0]['description'] assert state['fatal_errors'][0][0]['turn'] == 0 @@ -247,9 +248,9 @@ def test_remote_move_failures(player_name, is_setup_error, error_type): layout_dict=pelita.layout.parse_layout(layout)) assert state['whowins'] == 1 + assert state['game_phase'] == 'FINISHED' - assert state['fatal_errors'][0][0]['type'] == 'FatalException' - assert f'Exception in client ({error_type})' in state['fatal_errors'][0][0]['description'] + assert state['fatal_errors'][0][0]['type'] == error_type assert state['fatal_errors'][0][0]['turn'] == 0 assert state['fatal_errors'][0][0]['round'] == 1 assert state['fatal_errors'][1] == [] From 037f20f477b18d188038852e5fdece866b0a0253 Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Sat, 21 Jun 2025 01:12:34 +0200 Subject: [PATCH 06/27] BF: Fix remote test --- test/fixtures/remote_timeout_red.py | 2 +- test/test_remote_game.py | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/test/fixtures/remote_timeout_red.py b/test/fixtures/remote_timeout_red.py index 95e6c3502..23f1a6615 100644 --- a/test/fixtures/remote_timeout_red.py +++ b/test/fixtures/remote_timeout_red.py @@ -2,7 +2,7 @@ TEAM_NAME = "500ms timeout" def move(b, s): - if b.round == 1 and b.turn == 1: + if b.round == 2 and b.turn == 1: return (-2, 0) time.sleep(0.5) return b.position \ No newline at end of file diff --git a/test/test_remote_game.py b/test/test_remote_game.py index 3b638bba0..8bb0c250a 100644 --- a/test/test_remote_game.py +++ b/test/test_remote_game.py @@ -60,7 +60,7 @@ def test_remote_run_game(remote_teams): assert state['errors'] == [{}, {}] -@pytest.mark.xfail(reason="TODO: Fails in CI for macOS. Unclear why.") +#@pytest.mark.xfail(reason="TODO: Fails in CI for macOS. Unclear why.") def test_remote_timeout(): # We have a slow player that also generates a bad move # in its second turn. @@ -83,13 +83,15 @@ def test_remote_timeout(): timeout_length=0.4) assert state['whowins'] == 0 - assert state['fatal_errors'] == [[], []] - assert state['errors'] == [{}, - {(1, 1): {'description': '', 'type': 'PlayerTimeout'}, - (1, 3): {'bot_position': (-2, 0), 'reason': 'illegal move'}, - (2, 1): {'description': '', 'type': 'PlayerTimeout'}, - (2, 3): {'description': '', 'type': 'PlayerTimeout'}, - (3, 1): {'description': '', 'type': 'PlayerTimeout'}}] + assert state['fatal_errors'][0] == [] + assert state['fatal_errors'][1][0]['type'] == 'IllegalPosition' + assert state['fatal_errors'][1][0]['turn'] == 3 + assert state['fatal_errors'][1][0]['round'] == 2 + assert state['errors'][0] == {} + assert set(state['errors'][1].keys()) == {(1, 1), (1, 3), (2, 1)} + assert state['errors'][1][(1, 1)]['type'] == 'timeout' + assert state['errors'][1][(1, 3)]['type'] == 'timeout' + assert state['errors'][1][(2, 1)]['type'] == 'timeout' def test_remote_dumps_are_written(): From 7397f86c8593b7cbac9d80fc55f3b28a28290a39 Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Sun, 22 Jun 2025 11:18:07 +0200 Subject: [PATCH 07/27] RF: New error handling mechanism, errors renamed to timeouts Timeouts are handled and counted in request_new_position and create a fatal_error when exceeding the limit. Fatal errors are created with a new function add_fatal_error that also immediately changes the game phase to FINISHED or FAILURE, eliminating a few extra calls to check_gameover. --- pelita/exceptions.py | 6 +- pelita/game.py | 293 ++++++++++++++++++--------------------- pelita/utils.py | 4 +- test/test_game.py | 42 +++--- test/test_players.py | 2 +- test/test_remote_game.py | 22 +-- test/test_team.py | 12 +- 7 files changed, 184 insertions(+), 197 deletions(-) diff --git a/pelita/exceptions.py b/pelita/exceptions.py index 0295bfea2..8c48086a2 100644 --- a/pelita/exceptions.py +++ b/pelita/exceptions.py @@ -9,4 +9,8 @@ class GameOverError(Exception): class PelitaBotError(Exception): """ Raised when allow_exceptions is turned on """ - pass \ No newline at end of file + pass + +class PelitaIllegalGameState(Exception): + """ Raised when there is something wrong with the game state """ + pass diff --git a/pelita/game.py b/pelita/game.py index e8d6fccfb..2f246105b 100644 --- a/pelita/game.py +++ b/pelita/game.py @@ -12,7 +12,7 @@ from . import layout from .base_utils import default_rng -from .exceptions import NoFoodWarning, PelitaBotError +from .exceptions import NoFoodWarning, PelitaBotError, PelitaIllegalGameState from .gamestate_filters import noiser, relocate_expired_food, update_food_age, in_homezone from .layout import get_legal_positions, initial_positions from .network import RemotePlayerFailure, RemotePlayerRecvTimeout, RemotePlayerSendError, ZMQPublisher, setup_controller @@ -366,8 +366,8 @@ def setup_game(team_specs, *, layout_dict, max_rounds=300, rng=None, #: Fatal errors fatal_errors=[[], []], - #: Errors - errors=[{}, {}], + #: Number of timeouts for a team + timeouts=[{}, {}], ### Configuration #: Maximum number of rounds, int @@ -376,6 +376,9 @@ def setup_game(team_specs, *, layout_dict, max_rounds=300, rng=None, #: Time till timeout, int timeout=3, + #: Initial timeout, int + initial_timeout=6, + #: Noise radius, int noise_radius=NOISE_RADIUS, @@ -457,9 +460,8 @@ def setup_game(team_specs, *, layout_dict, max_rounds=300, rng=None, team_state = setup_teams(team_specs, game_state, store_output=store_output, allow_exceptions=allow_exceptions) game_state.update(team_state) - # Check if one of the teams has already generate a fatal error - # or if the game has finished (might happen if we set it up with max_rounds=0). - game_state.update(check_gameover(game_state, detect_final_move=True)) + # Check if the game has finished (might happen if we set it up with max_rounds=0). + game_state.update(check_gameover(game_state)) # Send updated game state with team names to the viewers update_viewers(game_state) @@ -470,7 +472,7 @@ def setup_game(team_specs, *, layout_dict, max_rounds=300, rng=None, if game_state['game_phase'] == 'INIT': send_initial(game_state, allow_exceptions=allow_exceptions) - game_state.update(check_gameover(game_state, detect_final_move=True)) + game_state.update(check_gameover(game_state)) game_state['game_phase'] = 'RUNNING' return game_state @@ -494,7 +496,7 @@ def setup_teams(team_specs, game_state, store_output=False, allow_exceptions=Fal teams.append(team) # Await that the teams signal readiness and get the team name - initial_timeout = 4 + initial_timeout = game_state['initial_timeout'] start = time.monotonic() has_remote_teams = any(hasattr(team, 'zmqconnection') for team in teams) @@ -527,13 +529,7 @@ def setup_teams(team_specs, game_state, store_output=False, allow_exceptions=Fal exit_remote_teams(game_state) raise - exception_event = { - 'type': e.__class__.__name__, - 'description': str(e), - 'turn': team_idx, - 'round': None, - } - game_state['fatal_errors'][team_idx].append(exception_event) + add_fatal_error(game_state, round=None, turn=team_idx, type=e.__class__.__name__, msg=str(e)) if len(e.args) > 1: game_print(team_idx, f"{type(e).__name__} ({e.args[0]}): {e.args[1]}") @@ -547,17 +543,10 @@ def setup_teams(team_specs, game_state, store_output=False, allow_exceptions=Fal if not break_error and remote_sockets: break_error = True for socket, team_idx in remote_sockets.items(): - exception_event = { - 'type': 'Timeout', - 'description': 'Team did not start (timeout).', - 'turn': team_idx, - 'round': None, - } - - game_state['fatal_errors'][team_idx].append(exception_event) + add_fatal_error(game_state, round=None, turn=team_idx, type='Timeout', msg='Team did not start (timeout).') game_print(team_idx, f"Team '{teams[team_idx]._team_spec}' did not start (timeout).") - game_phase = "FAILURE" if break_error else game_state["game_phase"] + # if we encountered an error, the game_phase should have been set to FAILURE # Send the initial state to the teams team_names = [team.team_name for team in teams] @@ -565,7 +554,6 @@ def setup_teams(team_specs, game_state, store_output=False, allow_exceptions=Fal team_state = { 'teams': teams, 'team_names': team_names, - 'game_phase': game_phase } return team_state @@ -583,13 +571,7 @@ def send_initial(game_state, allow_exceptions=False): exit_remote_teams(game_state) raise PelitaBotError(e.error_type, e.error_msg) - exception_event = { - 'type': e.error_type, - 'description': e.error_msg, - 'turn': team_idx, - 'round': None, - } - game_state['fatal_errors'][team_idx].append(exception_event) + add_fatal_error(game_state, round=None, turn=team_idx, type=e.error_type, msg=e.error_msg) game_print(team_idx, f"{e.error_type}: {e.error_msg}") except RemotePlayerSendError: @@ -597,13 +579,7 @@ def send_initial(game_state, allow_exceptions=False): exit_remote_teams(game_state) raise PelitaBotError('Send error', 'Remote team unavailable') - exception_event = { - 'type': 'Send error', - 'description': 'Remote team unavailable', - 'turn': team_idx, - 'round': None, - } - game_state['fatal_errors'][team_idx].append(exception_event) + add_fatal_error(game_state, round=None, turn=team_idx, type='Send error', msg='Remote team unavailable') game_print(team_idx, "Send error: Remote team unavailable") except RemotePlayerRecvTimeout: @@ -611,14 +587,7 @@ def send_initial(game_state, allow_exceptions=False): exit_remote_teams(game_state) raise PelitaBotError('timeout', 'Timeout in set initial') - # Time out in set_initial - exception_event = { - 'type': 'timeout', - 'description': 'Timeout in set initial', - 'turn': team_idx, - 'round': None, - } - game_state['fatal_errors'][team_idx].append(exception_event) + add_fatal_error(game_state, round=None, turn=team_idx, type='timeout', msg='Timeout in set initial') game_print(team_idx, "timeout: Timeout in set initial") @@ -648,22 +617,28 @@ def request_new_position(game_state): } except RemotePlayerRecvTimeout: - # There was a timeout. Execute a random move - - legal_positions = get_legal_positions(game_state["walls"], game_state["shape"], - game_state["bots"][game_state["turn"]]) - req_position = game_state['rng'].choice(legal_positions) - game_print(turn, f"Player timeout. Setting a legal position at random: {req_position}") - - bot_reply = { - 'move': req_position, - 'error': 'timeout', - } - timeout_event = { - 'type': 'timeout', - 'description': f"Player timeout. Setting a legal position at random: {req_position}" - } - game_state['errors'][team_idx][(round, turn)] = timeout_event + if game_state['error_limit'] != 0 and len(game_state['timeouts'][team_idx]) >= game_state['error_limit']: + # We had too many timeouts already. Trigger a fatal_error. + # If error_limit is 0, the game will go on. + bot_reply = { + 'error': 'Timeout error', + 'error_msg': 'Too many timeouts' + } + else: + # There was a timeout. Execute a random move + legal_positions = get_legal_positions(game_state["walls"], game_state["shape"], + game_state["bots"][game_state["turn"]]) + req_position = game_state['rng'].choice(legal_positions) + game_print(turn, f"Player timeout. Setting a legal position at random: {req_position}") + + bot_reply = { + 'move': req_position + } + timeout_event = { + 'type': 'timeout', + 'description': f"Player timeout. Setting a legal position at random: {req_position}" + } + game_state['timeouts'][team_idx][(round, turn)] = timeout_event duration = time.monotonic() - start_time @@ -728,7 +703,7 @@ def prepare_bot_state(game_state, team_idx=None): 'kills': game_state['kills'][own_team::2], 'deaths': game_state['deaths'][own_team::2], 'bot_was_killed': game_state['bot_was_killed'][own_team::2], - 'error_count': len(game_state['errors'][own_team]), + 'error_count': len(game_state['timeouts'][own_team]), 'food': list(game_state['food'][own_team]), 'shaded_food': shaded_food, 'name': game_state['team_names'][own_team], @@ -797,7 +772,7 @@ def prepare_viewer_state(game_state): viewer_state['food_age'] = [item for team_food_age in viewer_state['food_age'] for item in team_food_age.items()] - # game_state["errors"] has a tuple as a dict key + # game_state["timeouts"] has a tuple as a dict key # that cannot be serialized in json. # To fix this problem, we only send the current error # and add another attribute "num_errors" @@ -805,16 +780,16 @@ def prepare_viewer_state(game_state): # the key for the current round, turn round_turn = (game_state["round"], game_state["turn"]) - viewer_state["errors"] = [ + viewer_state["timeouts"] = [ # retrieve the current error or None team_errors.get(round_turn) - for team_errors in game_state["errors"] + for team_errors in game_state["timeouts"] ] # add the number of errors viewer_state["num_errors"] = [ len(team_errors) - for team_errors in game_state["errors"] + for team_errors in game_state["timeouts"] ] # remove unserializable values @@ -855,7 +830,7 @@ def play_turn(game_state, allow_exceptions=False): position_dict = request_new_position(game_state) - if "error" in position_dict and not position_dict['error'] == 'timeout': + if "error" in position_dict: error_type = position_dict['error'] error_string = position_dict.get('error_msg', '') @@ -865,13 +840,7 @@ def play_turn(game_state, allow_exceptions=False): # FatalExceptions (such as PlayerDisconnect) should immediately # finish the game - exception_event = { - 'type': error_type, - 'description': error_string, - 'turn': game_state['turn'], - 'round': game_state['round'], - } - game_state['fatal_errors'][team].append(exception_event) + add_fatal_error(game_state, round=game_state['round'], turn=game_state['turn'], type=error_type, msg=error_string) position = None game_print(turn, f"{error_type}: {error_string}") @@ -891,23 +860,17 @@ def play_turn(game_state, allow_exceptions=False): 'success': False # Success is set to true after apply_move } - # Check if a team has exceeded their maximum number of errors - # (we do not want to apply the move in this case) - # Note: Since we already updated the move counter, we do not check anymore, - # if the game has exceeded its rounds. - game_state.update(check_gameover(game_state)) - if not game_state['gameover']: # ok. we can apply the move for this team # try to execute the move and return the new state game_state = apply_move(game_state, position) # If there was no error, we claim a success in requested_moves - if (round, turn) not in game_state["errors"][team] and not game_state['fatal_errors'][team]: + if (round, turn) not in game_state['timeouts'][team] and not game_state['fatal_errors'][team]: game_state['requested_moves'][turn]['success'] = True # Check again, if we had errors or if this was the last move of the game (final round or food eaten) - game_state.update(check_gameover(game_state, detect_final_move=True)) + game_state.update(check_gameover(game_state)) # Send updated game state with team names to the viewers update_viewers(game_state) @@ -1013,7 +976,7 @@ def apply_move(gamestate, bot_position): bot_was_killed[turn] = False # previous errors - team_errors = gamestate["errors"][team] + team_errors = gamestate["timeouts"][team] # the allowed moves for the current bot legal_positions = get_legal_positions(walls, shape, gamestate["bots"][gamestate["turn"]]) @@ -1024,16 +987,9 @@ def apply_move(gamestate, bot_position): previous_position = gamestate["bots"][gamestate["turn"]] game_print(turn, f"Illegal position. {previous_position}➔{bot_position} not in legal positions:" f" {sorted(legal_positions)}.") - exception_event = { - 'type': 'IllegalPosition', - 'description': f"bot{turn}: {previous_position}➔{bot_position}", - 'turn': turn, - 'round': n_round, - } - gamestate['fatal_errors'][team].append(exception_event) + add_fatal_error(gamestate, round=n_round, turn=turn, type='IllegalPosition', msg=f"bot{turn}: {previous_position}➔{bot_position}") # only execute move if errors not exceeded - gamestate.update(check_gameover(gamestate)) if gamestate['gameover']: return gamestate @@ -1057,7 +1013,7 @@ def apply_move(gamestate, bot_position): # we check if we killed or have been killed and update the gamestate accordingly gamestate.update(apply_bot_kills(gamestate)) - errors = gamestate["errors"] + errors = gamestate["timeouts"] errors[team] = team_errors gamestate_new = { "food": food, @@ -1066,7 +1022,7 @@ def apply_move(gamestate, bot_position): "deaths": deaths, "kills": kills, "bot_was_killed": bot_was_killed, - "errors": errors, + "timeouts": errors, "game_phase": "RUNNING", } @@ -1097,6 +1053,8 @@ def next_round_turn(game_state): if turn is None and round is None: turn = 0 round = 1 + elif turn is None or round is None: + raise PelitaIllegalGameState("Bad configuration for turn and round") else: # if one of turn or round is None bot not both, it is illegal. # TODO: fail with a better error message @@ -1110,8 +1068,52 @@ def next_round_turn(game_state): 'turn': turn, } +def add_fatal_error(game_state, *, round, turn, type, msg): + team_idx = turn % 2 -def check_gameover(game_state, detect_final_move=False): + exception_event = { + 'type': type, + 'description': msg, + 'turn': turn, + 'round': round, + } + game_state['fatal_errors'][team_idx].append(exception_event) + + if game_state['game_phase'] == 'INIT': + num_fatal_errors = [len(f) for f in game_state['fatal_errors']] + if num_fatal_errors[0] > 0 or num_fatal_errors[1] > 0: + game_state.update({ + 'whowins' : -1, + 'gameover' : True, + 'game_phase': 'FAILURE' + }) + + if game_state['game_phase'] == 'RUNNING': + # If any team has a fatal error, this team loses. + # If both teams have a fatal error, it’s a draw. + num_fatal_errors = [len(f) for f in game_state['fatal_errors']] + if num_fatal_errors[0] == 0 and num_fatal_errors[1] == 0: + # no one has any fatal errors + pass + elif num_fatal_errors[0] > 0 and num_fatal_errors[1] > 0: + # both teams have fatal errors: it is a draw + game_state.update({ + 'whowins' : 2, + 'gameover' : True, + 'game_phase': 'FINISHED' + }) + else: + # one team has fatal errors + for team in (0, 1): + if num_fatal_errors[team] > 0: + game_state.update({ + 'whowins' : 1 - team, + 'gameover' : True, + 'game_phase': 'FINISHED' + }) + + +def check_gameover(game_state): """ Checks if this was the final moves or if the errors have exceeded the threshold. Returns @@ -1123,75 +1125,48 @@ def check_gameover(game_state, detect_final_move=False): return { 'whowins' : -1, 'gameover' : True, - 'game_phase': game_state['game_phase'] + 'game_phase': 'FAILURE' } - - if game_state['gameover']: + if game_state['game_phase'] == 'FINISHED': return { 'whowins' : game_state['whowins'], 'gameover' : game_state['gameover'], - 'game_phase': game_state['game_phase'] + 'game_phase': 'FINISHED' } if game_state['game_phase'] == 'INIT': - num_fatals = [len(f) for f in game_state['fatal_errors']] - if num_fatals[0] > 0 or num_fatals[1] > 0: - # TODO: We must flag failure - return { 'whowins' : -1, 'gameover' : True, 'game_phase': 'FAILURE' } - - # If any team has a fatal error, this team loses. - # If both teams have a fatal error, it’s a draw. - num_fatals = [len(f) for f in game_state['fatal_errors']] - if num_fatals[0] == 0 and num_fatals[1] == 0: - # no one has any fatal errors - pass - elif num_fatals[0] > 0 and num_fatals[1] > 0: - # both teams have fatal errors: it is a draw - return { 'whowins' : 2, 'gameover' : True, 'game_phase': 'FINISHED' } - else: - # some one has fatal errors - for team in (0, 1): - if num_fatals[team] > 0: - return { 'whowins' : 1 - team, 'gameover' : True, 'game_phase': 'FINISHED' } - - # If any team has reached error_limit errors, this team loses. - # If both teams have reached error_limit errors, it’s a draw. - # If error_limit is 0, the game will go on without checking. - num_errors = [len(f) for f in game_state['errors']] - if game_state['error_limit'] == 0: - pass - elif num_errors[0] < game_state['error_limit'] and num_errors[1] < game_state['error_limit']: - # no one has reached the error limit - pass - elif num_errors[0] >= game_state['error_limit'] and num_errors[1] >= game_state['error_limit']: - # both teams have reached or exceeded the error limit - return { 'whowins' : 2, 'gameover' : True, 'game_phase': 'FINISHED' } - else: - # only one team has reached the error limit - for team in (0, 1): - if num_errors[team] >= game_state['error_limit']: - return { 'whowins' : 1 - team, 'gameover' : True, 'game_phase': 'FINISHED' } - - if detect_final_move: - # No team wins/loses because of errors? - # Good. Now check if the game finishes because the food is gone - # or because we are in the final turn of the last round. - - # will we overshoot the max rounds with the next step? - next_step = next_round_turn(game_state) - next_round = next_step['round'] - - # count how much food is left for each team - food_left = [len(f) for f in game_state['food']] - if next_round > game_state['max_rounds'] or any(f == 0 for f in food_left): - if game_state['score'][0] > game_state['score'][1]: - whowins = 0 - elif game_state['score'][0] < game_state['score'][1]: - whowins = 1 - else: - whowins = 2 - return { 'whowins' : whowins, 'gameover' : True, 'game_phase': 'FINISHED' } - - return { 'whowins' : None, 'gameover' : False, 'game_phase': game_state['game_phase']} + return { + 'whowins' : None, + 'gameover' : False, + 'game_phase': 'INIT' + } + + # We are in running phase. Check if the food is gone + # or if we are in the final turn of the last round. + + # will we overshoot the max rounds with the next step? + next_step = next_round_turn(game_state) + next_round = next_step['round'] + + # count how much food is left for each team + food_left = [len(f) for f in game_state['food']] + if next_round > game_state['max_rounds'] or any(f == 0 for f in food_left): + if game_state['score'][0] > game_state['score'][1]: + whowins = 0 + elif game_state['score'][0] < game_state['score'][1]: + whowins = 1 + else: + whowins = 2 + return { + 'whowins' : whowins, + 'gameover' : True, + 'game_phase': 'FINISHED' + } + + return { + 'whowins' : None, + 'gameover' : False, + 'game_phase': 'RUNNING' + } def exit_remote_teams(game_state): diff --git a/pelita/utils.py b/pelita/utils.py index b333343ca..3df3ea619 100644 --- a/pelita/utils.py +++ b/pelita/utils.py @@ -117,8 +117,8 @@ def run_background_game(*, blue_move, red_move, layout=None, max_rounds=300, see out['red_bots'] = game_state['bots'][1::2] out['blue_score'] = game_state['score'][0] out['red_score'] = game_state['score'][1] - out['blue_errors'] = game_state['errors'][0] - out['red_errors'] = game_state['errors'][1] + out['blue_errors'] = game_state['timeouts'][0] + out['red_errors'] = game_state['timeouts'][1] out['blue_deaths'] = game_state['deaths'][::2] out['red_deaths'] = game_state['deaths'][1::2] out['blue_kills'] = game_state['kills'][::2] diff --git a/test/test_game.py b/test/test_game.py index 3d3ab6509..0640643e8 100644 --- a/test/test_game.py +++ b/test/test_game.py @@ -11,7 +11,7 @@ from pelita import game, layout, maze_generator from pelita.exceptions import NoFoodWarning, PelitaBotError -from pelita.game import (apply_move, get_legal_positions, initial_positions, +from pelita.game import (add_fatal_error, apply_move, get_legal_positions, initial_positions, play_turn, run_game, setup_game) from pelita.layout import parse_layout from pelita.player import stepping_player, stopping_player @@ -181,6 +181,7 @@ def test_get_legal_positions_random(parsed_l, bot_idx): assert abs((move[0] - bot[0])+(move[1] - bot[1])) <= 1 @pytest.mark.parametrize('turn', (0, 1, 2, 3)) +@pytest.mark.skip("TODO: Timeout handling has moved") def test_play_turn_apply_error(game_state, turn): """check that quits when there are too many errors""" error_dict = { @@ -188,18 +189,18 @@ def test_play_turn_apply_error(game_state, turn): } game_state["turn"] = turn team = turn % 2 - game_state["errors"] = [{(r, t): error_dict for r in (1, 2) for t in (0, 1)}, + game_state["timeouts"] = [{(r, t): error_dict for r in (1, 2) for t in (0, 1)}, {(r, t): error_dict for r in (1, 2) for t in (0, 1)}] # we pretend that two rounds have already been played # so that the error dictionaries are sane game_state["round"] = 3 # add a timeout to the current bot - game_state["errors"][team][(3, turn%2)] = error_dict + game_state["timeouts"][team][(3, turn%2)] = error_dict game_state_new = apply_move(game_state, game_state["bots"][turn]) assert game_state_new["gameover"] - assert len(game_state_new["errors"][team]) == 5 + assert len(game_state_new["timeouts"][team]) == 5 assert game_state_new["whowins"] == int(not team) - assert game_state_new["errors"][team][(3, turn%2)] == error_dict + assert game_state_new["timeouts"][team][(3, turn%2)] == error_dict @pytest.mark.parametrize('turn', (0, 1, 2, 3)) @@ -223,14 +224,15 @@ def test_illegal_position_is_fatal(game_state, turn): def test_play_turn_fatal(game_state, turn): """Checks that game quite after fatal error""" game_state["turn"] = turn + game_state["round"] = 1 + game_state["game_phase"] = "RUNNING" team = turn % 2 - fatal_list = [{}, {}] - fatal_list[team] = {"error":True} - game_state["fatal_errors"] = fatal_list - move = get_legal_positions(game_state["walls"], game_state["shape"], game_state["bots"][turn]) - game_state_new = apply_move(game_state, move[0]) - assert game_state_new["gameover"] - assert game_state_new["whowins"] == int(not team) + add_fatal_error(game_state, round=1, turn=turn, type="some error", msg="") + # move = get_legal_positions(game_state["walls"], game_state["shape"], game_state["bots"][turn]) + # game_state_new = apply_move(game_state, move[0]) + assert game_state["game_phase"] == "FINISHED" + assert game_state["gameover"] + assert game_state["whowins"] == int(not team) @pytest.mark.parametrize('turn', (0, 1, 2, 3)) @@ -344,6 +346,7 @@ def test_multiple_enemies_killing(): for bot in (0, 2): game_state = setup_game([dummy_bot, dummy_bot], layout_dict=parsed_l0) + game_state['round'] = 1 game_state['turn'] = bot # get position of bots x (and y) kill_position = game_state['bots'][1] @@ -358,6 +361,7 @@ def test_multiple_enemies_killing(): for bot in (1, 3): game_state = setup_game([dummy_bot, dummy_bot], layout_dict=parsed_l1) + game_state['round'] = 1 game_state['turn'] = bot # get position of bots 0 (and 2) kill_position = game_state['bots'][0] @@ -390,6 +394,7 @@ def test_suicide(): for bot in (1, 3): game_state = setup_game([dummy_bot, dummy_bot], layout_dict=parsed_l0) + game_state['round'] = 1 game_state['turn'] = bot # get position of bot 2 suicide_position = game_state['bots'][2] @@ -406,6 +411,7 @@ def test_suicide(): for bot in (0, 2): game_state = setup_game([dummy_bot, dummy_bot], layout_dict=parsed_l1) + game_state['round'] = 1 game_state['turn'] = bot # get position of bot 3 suicide_position = game_state['bots'][3] @@ -767,7 +773,7 @@ def test_play_turn_move(): "kills":[0]*4, "deaths": [0]*4, "bot_was_killed": [False]*4, - "errors": [[], []], + "timeouts": [[], []], "fatal_errors": [{}, {}], "rng": Random(), "game_phase": "RUNNING", @@ -788,7 +794,6 @@ def move(bot, s): # in the first round (round #1), # all bots move to the south if bot.round == 1: - # go one step to the right return (bot.position[0], bot.position[1] + 1) else: # There should not be more then one round in this test @@ -859,13 +864,13 @@ def test_last_round_check(max_rounds, current_round, turn, game_phase, gameover) 'turn': turn, 'error_limit': 5, 'fatal_errors': [[],[]], - 'errors': [[],[]], + 'timeouts': [[],[]], 'gameover': False, 'score': [0, 0], 'food': [{(1,1)}, {(1,1)}], # dummy food 'game_phase': game_phase } - res = game.check_gameover(state, detect_final_move=True) + res = game.check_gameover(state) assert res['gameover'] == gameover @@ -889,6 +894,7 @@ def test_last_round_check(max_rounds, current_round, turn, game_phase, gameover) (((0, 5), (1, 0)), 0), ] ) +@pytest.mark.skip("TODO: Timeout handling has moved") def test_error_finishes_game(team_errors, team_wins): # the mapping is as follows: # [(num_fatal_0, num_errors_0), (num_fatal_1, num_errors_1), result_flag] @@ -901,7 +907,9 @@ def test_error_finishes_game(team_errors, team_wins): "gameover": False, "error_limit": 5, "fatal_errors": [[None] * fatal_0, [None] * fatal_1], - "errors": [[None] * errors_0, [None] * errors_1] + "timeouts": [[None] * errors_0, [None] * errors_1], + 'turn': 0, + 'round': 1 } res = game.check_gameover(state) if team_wins is False: diff --git a/test/test_players.py b/test/test_players.py index b8525b943..9b31b1928 100644 --- a/test/test_players.py +++ b/test/test_players.py @@ -305,5 +305,5 @@ def test_players(player): # ensure that all test players ran correctly assert state['fatal_errors'] == [[], []] # our test players should never return invalid moves - assert state['errors'] == [{}, {}] + assert state['timeouts'] == [{}, {}] diff --git a/test/test_remote_game.py b/test/test_remote_game.py index 8bb0c250a..527af7833 100644 --- a/test/test_remote_game.py +++ b/test/test_remote_game.py @@ -43,7 +43,7 @@ def test_remote_call_pelita(remote_teams): assert res['fatal_errors'] == [[], []] # errors for call_pelita only contains the last thrown error, hence None # TODO: should be aligned so that call_pelita and run_game return the same thing - assert res['errors'] == [None, None] + assert res['timeouts'] == [None, None] def test_remote_run_game(remote_teams): @@ -57,7 +57,7 @@ def test_remote_run_game(remote_teams): state = pelita.game.run_game(remote_teams, max_rounds=30, layout_dict=pelita.layout.parse_layout(layout)) assert state['whowins'] == 1 assert state['fatal_errors'] == [[], []] - assert state['errors'] == [{}, {}] + assert state['timeouts'] == [{}, {}] #@pytest.mark.xfail(reason="TODO: Fails in CI for macOS. Unclear why.") @@ -87,11 +87,11 @@ def test_remote_timeout(): assert state['fatal_errors'][1][0]['type'] == 'IllegalPosition' assert state['fatal_errors'][1][0]['turn'] == 3 assert state['fatal_errors'][1][0]['round'] == 2 - assert state['errors'][0] == {} - assert set(state['errors'][1].keys()) == {(1, 1), (1, 3), (2, 1)} - assert state['errors'][1][(1, 1)]['type'] == 'timeout' - assert state['errors'][1][(1, 3)]['type'] == 'timeout' - assert state['errors'][1][(2, 1)]['type'] == 'timeout' + assert state['timeouts'][0] == {} + assert set(state['timeouts'][1].keys()) == {(1, 1), (1, 3), (2, 1)} + assert state['timeouts'][1][(1, 1)]['type'] == 'timeout' + assert state['timeouts'][1][(1, 3)]['type'] == 'timeout' + assert state['timeouts'][1][(2, 1)]['type'] == 'timeout' def test_remote_dumps_are_written(): @@ -116,7 +116,7 @@ def test_remote_dumps_are_written(): assert state['whowins'] == 2 assert state['fatal_errors'] == [[], []] - assert state['errors'] == [{}, {}] + assert state['timeouts'] == [{}, {}] path = Path(out_folder.name) blue_lines = (path / 'blue.out').read_text().split('\n') @@ -165,7 +165,7 @@ def test_remote_dumps_with_failure(failing_team): 'turn': fail_turn, 'round': 2} assert state['fatal_errors'][1 - failing_team] == [] - assert state['errors'] == [{}, {}] + assert state['timeouts'] == [{}, {}] path = Path(out_folder.name) @@ -242,7 +242,7 @@ def test_remote_move_failures(player_name, is_setup_error, error_type): assert state['fatal_errors'][0][0]['turn'] == 0 assert state['fatal_errors'][0][0]['round'] is None assert state['fatal_errors'][1] == [] - assert state['errors'] == [{}, {}] + assert state['timeouts'] == [{}, {}] else: state = pelita.game.run_game([str(failing_player), str(good_player)], @@ -256,4 +256,4 @@ def test_remote_move_failures(player_name, is_setup_error, error_type): assert state['fatal_errors'][0][0]['turn'] == 0 assert state['fatal_errors'][0][0]['round'] == 1 assert state['fatal_errors'][1] == [] - assert state['errors'] == [{}, {}] + assert state['timeouts'] == [{}, {}] diff --git a/test/test_team.py b/test/test_team.py index 70f65ac78..b0e2a2aea 100644 --- a/test/test_team.py +++ b/test/test_team.py @@ -418,7 +418,7 @@ def asserting_team(bot, state): state = run_game([asserting_team, asserting_team], max_rounds=1, layout_dict=parsed) # assertions might have been caught in run_game # check that all is good - assert state['errors'] == [{}, {}] + assert state['timeouts'] == [{}, {}] assert state['fatal_errors'] == [[], []] def test_bot_graph(): @@ -448,7 +448,7 @@ def rough_bot(bot, state): state = run_game([rough_bot, stopping], max_rounds=1, layout_dict=parse_layout(layout)) # assertions might have been caught in run_game # check that all is good - assert state['errors'] == [{}, {}] + assert state['timeouts'] == [{}, {}] assert state['fatal_errors'] == [[], []] def test_bot_graph_is_half_mutable(): @@ -491,7 +491,7 @@ def red(bot, state): state = run_game([blue, red], max_rounds=2, layout_dict=parse_layout(layout)) # assertions might have been caught in run_game # check that all is good - assert state['errors'] == [{}, {}] + assert state['timeouts'] == [{}, {}] assert state['fatal_errors'] == [[], []] assert "".join(observer) == 'BRbrbrbr' @@ -527,12 +527,12 @@ def team_2(bot, state): state = play_turn(state) # check that player did not fail - assert state['errors'] == [{}, {}] + assert state['timeouts'] == [{}, {}] assert state['fatal_errors'] == [[], []] state = play_turn(state) # check that player did not fail - assert state['errors'] == [{}, {}] + assert state['timeouts'] == [{}, {}] assert state['fatal_errors'] == [[], []] @@ -627,7 +627,7 @@ def team_2(bot, state): assert check == old_time # check that player did not fail - assert state['errors'] == [{}, {}] + assert state['timeouts'] == [{}, {}] assert state['fatal_errors'] == [[], []] assert state['team_time'][0] >= 1.0 assert state['team_time'][1] >= 2.0 From c9497287510e4e8d4909a455aef5350a3cebbd90 Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Sun, 22 Jun 2025 13:34:51 +0200 Subject: [PATCH 08/27] BF: Playing with bad rounds or no food should trigger FAILURE --- pelita/game.py | 11 +++++++++++ test/test_game.py | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/pelita/game.py b/pelita/game.py index 2f246105b..59813db34 100644 --- a/pelita/game.py +++ b/pelita/game.py @@ -1134,6 +1134,17 @@ def check_gameover(game_state): 'game_phase': 'FINISHED' } if game_state['game_phase'] == 'INIT': + next_step = next_round_turn(game_state) + next_round = next_step['round'] + + # Fail if there is not enough food or rounds + food_left = [len(f) for f in game_state['food']] + if next_round > game_state['max_rounds'] or any(f == 0 for f in food_left): + return { + 'whowins': -1, + 'gameover': True, + 'game_phase': 'FAILURE' + } return { 'whowins' : None, 'gameover' : False, diff --git a/test/test_game.py b/test/test_game.py index 0640643e8..d2dbadf96 100644 --- a/test/test_game.py +++ b/test/test_game.py @@ -1292,7 +1292,7 @@ def test_invalid_setup_game_closes_players(): # setup a remote demo game with "0" and "1" but bad max rounds state = setup_game(["0", "1"], layout_dict=l, max_rounds=0, allow_exceptions=True) - assert state["gameover"] + assert state["game_phase"] == "FAILURE" # Check that both processes have exited assert state["teams"][0].proc.wait(timeout=3) == 0 assert state["teams"][1].proc.wait(timeout=3) == 0 From 19bf9c192082e94e5acac5732dc2616b8d965a30 Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Sun, 22 Jun 2025 14:13:37 +0200 Subject: [PATCH 09/27] RF: Use game_phase in prepare_bot_state --- pelita/game.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/pelita/game.py b/pelita/game.py index 59813db34..b752c92be 100644 --- a/pelita/game.py +++ b/pelita/game.py @@ -652,24 +652,23 @@ def prepare_bot_state(game_state, team_idx=None): """ Prepares the bot’s game state for the current bot. """ - - bot_initialization = game_state.get('turn') is None and team_idx is not None - bot_finalization = game_state.get('turn') is not None and team_idx is not None - - if bot_initialization: + if game_state['game_phase'] == 'INIT': # We assume that we are in get_initial phase turn = team_idx bot_turn = None seed = game_state['rng'].randint(0, sys.maxsize) - elif bot_finalization: + elif game_state['game_phase'] == 'FINISHED': # Called for remote players in _exit turn = team_idx bot_turn = None seed = None - else: + elif game_state['game_phase'] == 'RUNNING': turn = game_state['turn'] bot_turn = game_state['turn'] // 2 seed = None + else: + _logger.warning("Got bad game_state in prepare_bot_state") + return bot_position = game_state['bots'][turn] own_team = turn % 2 @@ -734,7 +733,7 @@ def prepare_bot_state(game_state, team_idx=None): 'max_rounds': game_state['max_rounds'], } - if bot_initialization: + if game_state['game_phase'] == 'INIT': bot_state.update({ 'walls': game_state['walls'], # only in initial round 'shape': game_state['shape'], # only in initial round From ed66c4a5acff44a48845f28f2671169a7fe6bb9f Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Sun, 22 Jun 2025 22:07:33 +0200 Subject: [PATCH 10/27] =?UTF-8?q?RF:=20Rename=20allow=5Fexceptions=20?= =?UTF-8?q?=E2=86=92=20raise=5Fbot=5Fexceptions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- demo/benchmark_game.py | 2 +- pelita/exceptions.py | 2 +- pelita/game.py | 30 +++++++++++++++--------------- pelita/utils.py | 2 +- test/test_game.py | 8 ++++---- test/test_team.py | 18 +++++++++--------- 6 files changed, 31 insertions(+), 31 deletions(-) diff --git a/demo/benchmark_game.py b/demo/benchmark_game.py index cdc6dedc9..6d4008505 100644 --- a/demo/benchmark_game.py +++ b/demo/benchmark_game.py @@ -34,7 +34,7 @@ layout = parse_layout(LAYOUT) def run(teams, max_rounds): - return run_game(teams, max_rounds=max_rounds, layout_dict=layout, print_result=False, allow_exceptions=True, store_output=subprocess.DEVNULL) + return run_game(teams, max_rounds=max_rounds, layout_dict=layout, print_result=False, raise_bot_exceptions=True, store_output=subprocess.DEVNULL) def parse_args(): parser = argparse.ArgumentParser(description='Benchmark pelita run_game') diff --git a/pelita/exceptions.py b/pelita/exceptions.py index 8c48086a2..8583c5805 100644 --- a/pelita/exceptions.py +++ b/pelita/exceptions.py @@ -8,7 +8,7 @@ class GameOverError(Exception): pass class PelitaBotError(Exception): - """ Raised when allow_exceptions is turned on """ + """ Raised when raise_bot_exceptions is turned on """ pass class PelitaIllegalGameState(Exception): diff --git a/pelita/game.py b/pelita/game.py index b752c92be..89411bca7 100644 --- a/pelita/game.py +++ b/pelita/game.py @@ -110,7 +110,7 @@ def run_game(team_specs, *, layout_dict, max_rounds=300, rng=None, allow_camping=False, error_limit=5, timeout_length=3, viewers=None, store_output=False, team_names=(None, None), team_infos=(None, None), - allow_exceptions=False, print_result=True): + raise_bot_exceptions=False, print_result=True): """ Run a pelita match. Parameters @@ -168,11 +168,11 @@ def run_game(team_specs, *, layout_dict, max_rounds=300, team_info : tuple(team_info_0, team_info_1) a tuple containing additional team info. - allow_exceptions : bool + raise_bot_exceptions : bool when True, allow teams to raise Exceptions. This is especially useful when running local games, where you typically want to see Exceptions do debug them. When running remote games, - allow_exceptions should be False, so that the game can collect + raise_bot_exceptions should be False, so that the game can collect the exceptions and cleanly create a game-over state if needed. @@ -224,7 +224,7 @@ def run_game(team_specs, *, layout_dict, max_rounds=300, break # play the next turn - state = play_turn(state, allow_exceptions=allow_exceptions) + state = play_turn(state, raise_bot_exceptions=raise_bot_exceptions) return state @@ -282,7 +282,7 @@ def setup_game(team_specs, *, layout_dict, max_rounds=300, rng=None, allow_camping=False, error_limit=5, timeout_length=3, viewers=None, store_output=False, team_names=(None, None), team_infos=(None, None), - allow_exceptions=False, print_result=True): + raise_bot_exceptions=False, print_result=True): """ Generates a game state for the given teams and layout with otherwise default values. """ if viewers is None: viewers = [] @@ -457,7 +457,7 @@ def setup_game(team_specs, *, layout_dict, max_rounds=300, rng=None, # to answer to the server. update_viewers(game_state) - team_state = setup_teams(team_specs, game_state, store_output=store_output, allow_exceptions=allow_exceptions) + team_state = setup_teams(team_specs, game_state, store_output=store_output, raise_bot_exceptions=raise_bot_exceptions) game_state.update(team_state) # Check if the game has finished (might happen if we set it up with max_rounds=0). @@ -471,14 +471,14 @@ def setup_game(team_specs, *, layout_dict, max_rounds=300, rng=None, exit_remote_teams(game_state) if game_state['game_phase'] == 'INIT': - send_initial(game_state, allow_exceptions=allow_exceptions) + send_initial(game_state, raise_bot_exceptions=raise_bot_exceptions) game_state.update(check_gameover(game_state)) game_state['game_phase'] = 'RUNNING' return game_state -def setup_teams(team_specs, game_state, store_output=False, allow_exceptions=False): +def setup_teams(team_specs, game_state, store_output=False, raise_bot_exceptions=False): """ Creates the teams according to the `teams`. """ assert game_state['game_phase'] == 'INIT' @@ -525,7 +525,7 @@ def setup_teams(team_specs, game_state, store_output=False, allow_exceptions=Fal _state = team.wait_ready(timeout=0) except (RemotePlayerSendError, RemotePlayerRecvTimeout, RemotePlayerFailure) as e: - if allow_exceptions: + if raise_bot_exceptions: exit_remote_teams(game_state) raise @@ -557,7 +557,7 @@ def setup_teams(team_specs, game_state, store_output=False, allow_exceptions=Fal } return team_state -def send_initial(game_state, allow_exceptions=False): +def send_initial(game_state, raise_bot_exceptions=False): assert game_state["game_phase"] == "INIT" teams = game_state['teams'] @@ -567,7 +567,7 @@ def send_initial(game_state, allow_exceptions=False): _res = team.set_initial(team_idx, prepare_bot_state(game_state, team_idx)) except RemotePlayerFailure as e: - if allow_exceptions: + if raise_bot_exceptions: exit_remote_teams(game_state) raise PelitaBotError(e.error_type, e.error_msg) @@ -575,7 +575,7 @@ def send_initial(game_state, allow_exceptions=False): game_print(team_idx, f"{e.error_type}: {e.error_msg}") except RemotePlayerSendError: - if allow_exceptions: + if raise_bot_exceptions: exit_remote_teams(game_state) raise PelitaBotError('Send error', 'Remote team unavailable') @@ -583,7 +583,7 @@ def send_initial(game_state, allow_exceptions=False): game_print(team_idx, "Send error: Remote team unavailable") except RemotePlayerRecvTimeout: - if allow_exceptions: + if raise_bot_exceptions: exit_remote_teams(game_state) raise PelitaBotError('timeout', 'Timeout in set initial') @@ -799,7 +799,7 @@ def prepare_viewer_state(game_state): return viewer_state -def play_turn(game_state, allow_exceptions=False): +def play_turn(game_state, raise_bot_exceptions=False): """ Plays the next turn of the game. This function increases the round and turn counters, requests a move @@ -833,7 +833,7 @@ def play_turn(game_state, allow_exceptions=False): error_type = position_dict['error'] error_string = position_dict.get('error_msg', '') - if allow_exceptions: + if raise_bot_exceptions: exit_remote_teams(game_state) raise PelitaBotError(error_type, error_string) diff --git a/pelita/utils.py b/pelita/utils.py index 3df3ea619..de12379a5 100644 --- a/pelita/utils.py +++ b/pelita/utils.py @@ -105,7 +105,7 @@ def run_background_game(*, blue_move, red_move, layout=None, max_rounds=300, see game_state = run_game((blue_move, red_move), layout_dict=layout_dict, max_rounds=max_rounds, rng=rng, - team_names=('blue', 'red'), allow_exceptions=True, print_result=False) + team_names=('blue', 'red'), raise_bot_exceptions=True, print_result=False) out = {} out['seed'] = seed out['walls'] = game_state['walls'] diff --git a/test/test_game.py b/test/test_game.py index d2dbadf96..5c836132c 100644 --- a/test/test_game.py +++ b/test/test_game.py @@ -1262,7 +1262,7 @@ def test_remote_game_closes_players_on_exit(): l = maze_generator.generate_maze() # run a remote demo game with "0" and "1" - state = run_game(["0", "1"], layout_dict=l, max_rounds=20, allow_exceptions=True) + state = run_game(["0", "1"], layout_dict=l, max_rounds=20, raise_bot_exceptions=True) assert state["gameover"] # Check that both processes have exited assert state["teams"][0].proc.wait(timeout=3) == 0 @@ -1273,7 +1273,7 @@ def test_manual_remote_game_closes_players(): l = maze_generator.generate_maze() # run a remote demo game with "0" and "1" - state = setup_game(["0", "1"], layout_dict=l, max_rounds=10, allow_exceptions=True) + state = setup_game(["0", "1"], layout_dict=l, max_rounds=10, raise_bot_exceptions=True) assert not state["gameover"] while not state["gameover"]: # still running @@ -1291,7 +1291,7 @@ def test_invalid_setup_game_closes_players(): l = maze_generator.generate_maze() # setup a remote demo game with "0" and "1" but bad max rounds - state = setup_game(["0", "1"], layout_dict=l, max_rounds=0, allow_exceptions=True) + state = setup_game(["0", "1"], layout_dict=l, max_rounds=0, raise_bot_exceptions=True) assert state["game_phase"] == "FAILURE" # Check that both processes have exited assert state["teams"][0].proc.wait(timeout=3) == 0 @@ -1304,7 +1304,7 @@ def test_raises_and_exits_cleanly(): state = setup_game([str(path), "1"], layout_dict=l, max_rounds=2) with pytest.raises(PelitaBotError): while not state["gameover"]: - state = play_turn(state, allow_exceptions=True) + state = play_turn(state, raise_bot_exceptions=True) # This is the state before the exception assert state["gameover"] is False diff --git a/test/test_team.py b/test/test_team.py index b0e2a2aea..49877ca1e 100644 --- a/test/test_team.py +++ b/test/test_team.py @@ -39,7 +39,7 @@ def test_stopping(self): stopping, round_counting ] - state = run_game(team, max_rounds=1, layout_dict=parse_layout(test_layout), allow_exceptions=True) + state = run_game(team, max_rounds=1, layout_dict=parse_layout(test_layout), raise_bot_exceptions=True) assert state['bots'][0] == (1, 1) assert state['bots'][1] == (10, 1) assert round_counting._storage['rounds'] == 1 @@ -49,7 +49,7 @@ def test_stopping(self): stopping, round_counting ] - state = run_game(team, max_rounds=3, layout_dict=parse_layout(test_layout), allow_exceptions=True) + state = run_game(team, max_rounds=3, layout_dict=parse_layout(test_layout), raise_bot_exceptions=True) assert state['bots'][0] == (1, 1) assert state['bots'][1] == (10, 1) assert round_counting._storage['rounds'] == 3 @@ -240,7 +240,7 @@ def move(bot, state): # otherwise return current position return new_pos - state = run_game([move, move], max_rounds=3, layout_dict=parse_layout(layout), allow_exceptions=True) + state = run_game([move, move], max_rounds=3, layout_dict=parse_layout(layout), raise_bot_exceptions=True) # assertions might have been caught in run_game # check that all is good assert state['fatal_errors'] == [[], []] @@ -322,7 +322,7 @@ def move(bot, state): # otherwise return current position return new_pos - state = run_game([move, move], max_rounds=3, layout_dict=parse_layout(layout), allow_exceptions=True) + state = run_game([move, move], max_rounds=3, layout_dict=parse_layout(layout), raise_bot_exceptions=True) # assertions might have been caught in run_game # check that all is good assert state['fatal_errors'] == [[], []] @@ -594,7 +594,7 @@ def team_2(bot, state): return bot.position - state = setup_game([team_1, team_2], layout_dict=parse_layout(test_layout), max_rounds=3, allow_exceptions=True) + state = setup_game([team_1, team_2], layout_dict=parse_layout(test_layout), max_rounds=3, raise_bot_exceptions=True) state = play_turn(state) assert state['team_time'][0] >= 0 @@ -661,7 +661,7 @@ def asserting_team(bot, state): return bot.position state = run_game([asserting_team, asserting_team], max_rounds=1, layout_dict=parsed, - allow_exceptions=True) + raise_bot_exceptions=True) # assertions might have been caught in run_game # check that all is good assert state['fatal_errors'] == [[], []] @@ -690,7 +690,7 @@ def asserting_team(bot, state): return bot.position state = run_game([asserting_team, asserting_team], max_rounds=1, layout_dict=parsed, - allow_exceptions=True) + raise_bot_exceptions=True) # assertions might have been caught in run_game # check that all is good assert state['fatal_errors'] == [[], []] @@ -722,7 +722,7 @@ def asserting_team(bot, state): return bot.position state = run_game([asserting_team, asserting_team], max_rounds=1, layout_dict=parsed, - allow_exceptions=True) + raise_bot_exceptions=True) # assertions might have been caught in run_game # check that all is good assert state['fatal_errors'] == [[], []] @@ -756,7 +756,7 @@ def speaking_team(bot, state): bot.say(say) return bot.position - state = setup_game([speaking_team, speaking_team], max_rounds=1, layout_dict=parsed, allow_exceptions=True) + state = setup_game([speaking_team, speaking_team], max_rounds=1, layout_dict=parsed, raise_bot_exceptions=True) idx = 0 while not state["gameover"]: state = play_turn(state) From 1c523b4bcb3d46d4bd0a3148bf22df735b367479 Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Mon, 23 Jun 2025 00:36:01 +0200 Subject: [PATCH 11/27] ENH: When a UI controller exits in init, we go into failure mode --- pelita/game.py | 11 +++++++---- pelita/scripts/pelita_main.py | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/pelita/game.py b/pelita/game.py index 89411bca7..3aa4111f7 100644 --- a/pelita/game.py +++ b/pelita/game.py @@ -93,7 +93,7 @@ def _run_external_viewer(self, subscribe_sock, controller, geometry, delay, stop p = subprocess.Popen(external_call, preexec_fn=os.setsid) return p -def controller_exit(state, await_action='play_step'): +def controller_await(state, await_action='play_step'): """Wait for the controller to receive a action from a viewer action can be 'exit' (return True), 'play_setup', 'set_initial' (return True) @@ -215,11 +215,11 @@ def run_game(team_specs, *, layout_dict, max_rounds=300, print_result=print_result) # Play the game until it is gameover. - while not state.get('gameover'): + while state['game_phase'] == 'RUNNING': # this is only needed if we have a controller, for example a viewer # this function call *blocks* until the viewer has replied - if controller_exit(state): + if controller_await(state): # if the controller asks us, we'll exit and stop playing break @@ -449,7 +449,10 @@ def setup_game(team_specs, *, layout_dict, max_rounds=300, rng=None, # Wait until the controller tells us that it is ready # We then can send the initial maze # This call *blocks* until the controller replies - if controller_exit(game_state, await_action='set_initial'): + if controller_await(game_state, await_action='set_initial'): + # controller_await has flagged exit + # We should return with an error + game_state['game_phase'] = 'FAILURE' return game_state # Send maze before team creation. diff --git a/pelita/scripts/pelita_main.py b/pelita/scripts/pelita_main.py index ba1264c5b..7097db162 100755 --- a/pelita/scripts/pelita_main.py +++ b/pelita/scripts/pelita_main.py @@ -368,7 +368,7 @@ def main(): if args.replayfile: viewer_state = pelita.game.setup_viewers(viewers) - if pelita.game.controller_exit(viewer_state, await_action='set_initial'): + if pelita.game.controller_await(viewer_state, await_action='set_initial'): sys.exit(0) old_game = Path(args.replayfile).read_text().split("\x04") @@ -382,7 +382,7 @@ def main(): state['food'] = list(map(tuple, state['food'])) for viewer in viewer_state['viewers']: viewer.show_state(state) - if pelita.game.controller_exit(viewer_state): + if pelita.game.controller_await(viewer_state): break sys.exit(0) From 289a2b8ab87fb0a6f2fb99594899be6f600e7e76 Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Mon, 23 Jun 2025 21:42:22 +0200 Subject: [PATCH 12/27] ENH: Rename Errors to Timeouts in Tk --- pelita/ui/tk_canvas.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pelita/ui/tk_canvas.py b/pelita/ui/tk_canvas.py index 7500d5e4e..cbe109a3f 100644 --- a/pelita/ui/tk_canvas.py +++ b/pelita/ui/tk_canvas.py @@ -852,7 +852,7 @@ def status(team_idx): # sum the deaths of both bots in this team deaths = game_state['deaths'][team_idx] + game_state['deaths'][team_idx+2] kills = game_state['kills'][team_idx] + game_state['kills'][team_idx+2] - ret = "Errors: %d, Kills: %d, Deaths: %d, Time: %.2f" % (game_state["num_errors"][team_idx], kills, deaths, game_state["team_time"][team_idx]) + ret = "Timeouts: %d, Kills: %d, Deaths: %d, Time: %.2f" % (game_state["num_errors"][team_idx], kills, deaths, game_state["team_time"][team_idx]) return ret except TypeError: return "" From 5157fb72fd5092e72d3ac4a07888f3704fea9300 Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Mon, 23 Jun 2025 22:18:45 +0200 Subject: [PATCH 13/27] RF: Remove timeout handling from apply_move --- pelita/game.py | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/pelita/game.py b/pelita/game.py index 3aa4111f7..90b956133 100644 --- a/pelita/game.py +++ b/pelita/game.py @@ -871,9 +871,6 @@ def play_turn(game_state, raise_bot_exceptions=False): if (round, turn) not in game_state['timeouts'][team] and not game_state['fatal_errors'][team]: game_state['requested_moves'][turn]['success'] = True - # Check again, if we had errors or if this was the last move of the game (final round or food eaten) - game_state.update(check_gameover(game_state)) - # Send updated game state with team names to the viewers update_viewers(game_state) @@ -977,22 +974,18 @@ def apply_move(gamestate, bot_position): # reset our own bot_was_killed flag bot_was_killed[turn] = False - # previous errors - team_errors = gamestate["timeouts"][team] - # the allowed moves for the current bot legal_positions = get_legal_positions(walls, shape, gamestate["bots"][gamestate["turn"]]) - # unless we have already made an error, check if we made a legal move - if (n_round, turn) not in team_errors: - if bot_position not in legal_positions: - previous_position = gamestate["bots"][gamestate["turn"]] - game_print(turn, f"Illegal position. {previous_position}➔{bot_position} not in legal positions:" - f" {sorted(legal_positions)}.") - add_fatal_error(gamestate, round=n_round, turn=turn, type='IllegalPosition', msg=f"bot{turn}: {previous_position}➔{bot_position}") + # check if we made a legal move + if bot_position not in legal_positions: + previous_position = gamestate["bots"][gamestate["turn"]] + game_print(turn, f"Illegal position. {previous_position}➔{bot_position} not in legal positions:" + f" {sorted(legal_positions)}.") + add_fatal_error(gamestate, round=n_round, turn=turn, type='IllegalPosition', msg=f"bot{turn}: {previous_position}➔{bot_position}") # only execute move if errors not exceeded - if gamestate['gameover']: + if not gamestate['game_phase'] == "RUNNING": return gamestate # take step @@ -1015,8 +1008,6 @@ def apply_move(gamestate, bot_position): # we check if we killed or have been killed and update the gamestate accordingly gamestate.update(apply_bot_kills(gamestate)) - errors = gamestate["timeouts"] - errors[team] = team_errors gamestate_new = { "food": food, "bots": bots, @@ -1024,11 +1015,14 @@ def apply_move(gamestate, bot_position): "deaths": deaths, "kills": kills, "bot_was_killed": bot_was_killed, - "timeouts": errors, "game_phase": "RUNNING", - } + } gamestate.update(gamestate_new) + + # Check if this was the last move of the game (final round or food eaten) + gamestate.update(check_gameover(gamestate)) + return gamestate From c57f72bef37e293e61fb90106a7ca7140e92604d Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Mon, 23 Jun 2025 22:32:40 +0200 Subject: [PATCH 14/27] RF: Use more game_phases --- pelita/game.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/pelita/game.py b/pelita/game.py index 90b956133..36b38a92c 100644 --- a/pelita/game.py +++ b/pelita/game.py @@ -469,14 +469,19 @@ def setup_game(team_specs, *, layout_dict, max_rounds=300, rng=None, # Send updated game state with team names to the viewers update_viewers(game_state) - # exit remote teams in case we are game over - if game_state['gameover']: - exit_remote_teams(game_state) - if game_state['game_phase'] == 'INIT': + # All good. send_initial(game_state, raise_bot_exceptions=raise_bot_exceptions) game_state.update(check_gameover(game_state)) + + # send_initial might have changed our game phase to FAILURE or FINISHED + if game_state['game_phase'] == 'INIT': game_state['game_phase'] = 'RUNNING' + else: + # exit remote teams in case there was a failure or the game has finished + # In this case, we also want to update the viewers + update_viewers(game_state) + exit_remote_teams(game_state) return game_state @@ -811,12 +816,12 @@ def play_turn(game_state, raise_bot_exceptions=False): Raises ------ ValueError - If gamestate['gameover'] is True + If game_state["game_phase"] != "RUNNING": """ # TODO: Return a copy of the game_state # if the game is already over, we return a value error - if game_state['gameover']: + if game_state["game_phase"] != "RUNNING": raise ValueError("Game is already over!") # Now update the round counter @@ -862,7 +867,7 @@ def play_turn(game_state, raise_bot_exceptions=False): 'success': False # Success is set to true after apply_move } - if not game_state['gameover']: + if game_state["game_phase"] == "RUNNING": # ok. we can apply the move for this team # try to execute the move and return the new state game_state = apply_move(game_state, position) @@ -875,7 +880,7 @@ def play_turn(game_state, raise_bot_exceptions=False): update_viewers(game_state) # exit remote teams in case we are game over - if game_state['gameover']: + if game_state["game_phase"] != "RUNNING": exit_remote_teams(game_state) return game_state @@ -1041,6 +1046,8 @@ def next_round_turn(game_state): If gamestate['gameover'] is True """ + # TODO: This should take a whole game_phase + if game_state['gameover']: raise ValueError("Game is already over") turn = game_state['turn'] From f4b557ed14732cda7ead9a907dae143b3b33b735 Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Wed, 2 Jul 2025 18:17:29 +0200 Subject: [PATCH 15/27] BF: Fix timeout counting --- pelita/game.py | 2 +- test/test_game.py | 102 +++++++++++++++++++++++++++------------------- 2 files changed, 62 insertions(+), 42 deletions(-) diff --git a/pelita/game.py b/pelita/game.py index 36b38a92c..7be9ec40f 100644 --- a/pelita/game.py +++ b/pelita/game.py @@ -625,7 +625,7 @@ def request_new_position(game_state): } except RemotePlayerRecvTimeout: - if game_state['error_limit'] != 0 and len(game_state['timeouts'][team_idx]) >= game_state['error_limit']: + if game_state['error_limit'] != 0 and len(game_state['timeouts'][team_idx]) + 1 >= game_state['error_limit']: # We had too many timeouts already. Trigger a fatal_error. # If error_limit is 0, the game will go on. bot_reply = { diff --git a/test/test_game.py b/test/test_game.py index 5c836132c..191d0bf0b 100644 --- a/test/test_game.py +++ b/test/test_game.py @@ -7,6 +7,7 @@ from pathlib import Path from random import Random +from pelita.network import RemotePlayerRecvTimeout import pytest from pelita import game, layout, maze_generator @@ -180,29 +181,6 @@ def test_get_legal_positions_random(parsed_l, bot_idx): assert move not in parsed_l["walls"] assert abs((move[0] - bot[0])+(move[1] - bot[1])) <= 1 -@pytest.mark.parametrize('turn', (0, 1, 2, 3)) -@pytest.mark.skip("TODO: Timeout handling has moved") -def test_play_turn_apply_error(game_state, turn): - """check that quits when there are too many errors""" - error_dict = { - "type": 'PlayerTimeout', - } - game_state["turn"] = turn - team = turn % 2 - game_state["timeouts"] = [{(r, t): error_dict for r in (1, 2) for t in (0, 1)}, - {(r, t): error_dict for r in (1, 2) for t in (0, 1)}] - # we pretend that two rounds have already been played - # so that the error dictionaries are sane - game_state["round"] = 3 - # add a timeout to the current bot - game_state["timeouts"][team][(3, turn%2)] = error_dict - game_state_new = apply_move(game_state, game_state["bots"][turn]) - assert game_state_new["gameover"] - assert len(game_state_new["timeouts"][team]) == 5 - assert game_state_new["whowins"] == int(not team) - assert game_state_new["timeouts"][team][(3, turn%2)] == error_dict - - @pytest.mark.parametrize('turn', (0, 1, 2, 3)) def test_illegal_position_is_fatal(game_state, turn): """check that quits when illegal position""" @@ -885,39 +863,81 @@ def test_last_round_check(max_rounds, current_round, turn, game_phase, gameover) (((0, 4), (0, 4)), False), (((0, 5), (0, 0)), 1), (((0, 0), (0, 5)), 0), - (((0, 5), (0, 5)), 2), + (((0, 5), (0, 5)), 1), # earlier team fails first (((1, 0), (0, 0)), 1), (((0, 0), (1, 0)), 0), - (((1, 0), (1, 0)), 2), - (((1, 1), (1, 0)), 2), + (((1, 0), (1, 0)), 1), # earlier team fails first + (((1, 1), (1, 0)), 1), # earlier team fails first (((1, 0), (0, 5)), 1), (((0, 5), (1, 0)), 0), ] ) -@pytest.mark.skip("TODO: Timeout handling has moved") def test_error_finishes_game(team_errors, team_wins): # the mapping is as follows: # [(num_fatal_0, num_errors_0), (num_fatal_1, num_errors_1), result_flag] - # the result flag: 0/1: team 0/1 wins, 2: draw, False: no winner yet + # the result flag: 0/1: team 0/1 wins, 2: draw, False: draw after 20 rounds + + (fatal_0, timeouts_0), (fatal_1, timeouts_1) = team_errors + + def move0(b, s): + if not s: + s['count'] = 0 + s['count'] += 1 + + if s['count'] <= fatal_0: + return None + + if s['count'] <= timeouts_0: + raise RemotePlayerRecvTimeout + + return b.position + + def move1(b, s): + if not s: + s['count'] = 0 + s['count'] += 1 + + if s['count'] <= fatal_1: + return None + + if s['count'] <= timeouts_1: + raise RemotePlayerRecvTimeout + + return b.position + + l = maze_generator.generate_maze() + state = game.setup_game([move0, move1], max_rounds=20, layout_dict=l) + + # We must patch apply_move_fn so that RemotePlayerRecvTimeout is not caught + # and we can actually test timeouts + def apply_move_fn(move_fn, bot, state): + move = move_fn(bot, state) + if move is None: + return { + 'error': 'Some fatal error', + 'error_msg': 'Some fatal error' + } + return { "move": move } + + state['teams'][0].apply_move_fn = apply_move_fn + state['teams'][1].apply_move_fn = apply_move_fn + + # Play the game until it is gameover. + while state['game_phase'] == 'RUNNING': + # play the next turn + state = play_turn(state) - (fatal_0, errors_0), (fatal_1, errors_1) = team_errors - # just faking a bunch of errors in our game state - state = { - "game_phase": "RUNNING", - "gameover": False, - "error_limit": 5, - "fatal_errors": [[None] * fatal_0, [None] * fatal_1], - "timeouts": [[None] * errors_0, [None] * errors_1], - 'turn': 0, - 'round': 1 - } res = game.check_gameover(state) if team_wins is False: - assert res["whowins"] is None - assert res["gameover"] is False + assert res["whowins"] == 2 + assert res["gameover"] is True + assert state["round"] == 20 + assert len(state['timeouts'][0]) == timeouts_0 + assert len(state['timeouts'][1]) == timeouts_1 else: assert res["whowins"] == team_wins assert res["gameover"] is True + assert state["round"] < 20 @pytest.mark.parametrize('bot_to_move', [0, 1, 2, 3]) From 78375e47f816fb8831a3e203d031f38ee1435c8f Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Thu, 3 Jul 2025 13:09:33 +0200 Subject: [PATCH 16/27] TST: Test that long team names are detected --- test/fixtures/player_bad_team_name.py | 5 +++++ test/test_remote_game.py | 27 +++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 test/fixtures/player_bad_team_name.py diff --git a/test/fixtures/player_bad_team_name.py b/test/fixtures/player_bad_team_name.py new file mode 100644 index 000000000..deda109d8 --- /dev/null +++ b/test/fixtures/player_bad_team_name.py @@ -0,0 +1,5 @@ +# Player with an overly long team name + +TEAM_NAME = "123456789 123456789 123456789 123456789" +def move(b, s): + return b.position diff --git a/test/test_remote_game.py b/test/test_remote_game.py index 527af7833..dd3edfd57 100644 --- a/test/test_remote_game.py +++ b/test/test_remote_game.py @@ -94,6 +94,33 @@ def test_remote_timeout(): assert state['timeouts'][1][(2, 1)]['type'] == 'timeout' +@pytest.mark.parametrize("failing_team", [0, 1]) +def test_bad_team_name(failing_team): + layout = """ + ########## + # b y # + #a .. x# + ########## + """ + + failing_player = FIXTURE_DIR / 'player_bad_team_name.py' + good_player = "0" + + if failing_team == 0: + teams = [str(failing_player), str(good_player)] + elif failing_team == 1: + teams = [str(good_player), str(failing_player)] + + state = pelita.game.run_game(teams, + max_rounds=8, + layout_dict=pelita.layout.parse_layout(layout), + timeout_length=0.4) + + assert state['whowins'] == -1 + assert state['fatal_errors'][failing_team][0]['type'] == "RemotePlayerFailure" + assert "longer than 25" in state['fatal_errors'][failing_team][0]['description'] + + def test_remote_dumps_are_written(): layout = """ ########## From cf17dbee4c3ff947a6ed284eacdefe1d91134c4f Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Thu, 3 Jul 2025 15:27:09 +0200 Subject: [PATCH 17/27] UI: Show failure --- pelita/ui/tk_canvas.py | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/pelita/ui/tk_canvas.py b/pelita/ui/tk_canvas.py index cbe109a3f..114e5565a 100644 --- a/pelita/ui/tk_canvas.py +++ b/pelita/ui/tk_canvas.py @@ -455,15 +455,19 @@ def update(self, game_state=None, redraw=False): for food_pos in eaten_food: del self.food_items[food_pos] - winning_team_idx = game_state.get("whowins") - if winning_team_idx is None: - self.draw_end_of_game(None) - elif winning_team_idx in (0, 1): - team_name = game_state["team_names"][winning_team_idx] or "???" - self.draw_game_over(team_name) - elif winning_team_idx == 2: - self.draw_game_draw() + if game_state.get("game_phase") == "FINISHED": + winning_team_idx = game_state.get("whowins") + if winning_team_idx in (0, 1): + team_name = game_state["team_names"][winning_team_idx] or "???" + self.draw_game_over(team_name) + elif winning_team_idx == 2: + self.draw_game_draw() + + elif game_state.get("game_phase") == "FAILURE": + self.draw_game_failure() + else: + self.draw_end_of_game(None) def draw_universe(self, game_state, redraw): self.draw_overlay(game_state.get('overlays', [])) @@ -960,7 +964,6 @@ def draw_end_of_game(self, display_string): fill="#FFC903", tags="gameover", justify=tkinter.CENTER, anchor=tkinter.CENTER) - def draw_game_over(self, win_name): """ Draw the game over string. """ # shorten the winning name @@ -973,6 +976,10 @@ def draw_game_draw(self): """ Draw the game draw string. """ self.draw_end_of_game("GAME OVER\nDRAW!") + def draw_game_failure(self): + """ Draw the game draw string. """ + self.draw_end_of_game("No Game\nCannot run Pelita") + def clear(self): self.ui_game_canvas.delete(tkinter.ALL) From b42f7104462d74c23a0c54ccb9819db8fb90007a Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Thu, 3 Jul 2025 16:50:21 +0200 Subject: [PATCH 18/27] RF: Cleanup superfluous functions --- pelita/game.py | 10 ++++++---- pelita/network.py | 10 ++-------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/pelita/game.py b/pelita/game.py index 7be9ec40f..360be77fc 100644 --- a/pelita/game.py +++ b/pelita/game.py @@ -15,7 +15,7 @@ from .exceptions import NoFoodWarning, PelitaBotError, PelitaIllegalGameState from .gamestate_filters import noiser, relocate_expired_food, update_food_age, in_homezone from .layout import get_legal_positions, initial_positions -from .network import RemotePlayerFailure, RemotePlayerRecvTimeout, RemotePlayerSendError, ZMQPublisher, setup_controller +from .network import Controller, RemotePlayerFailure, RemotePlayerRecvTimeout, RemotePlayerSendError, ZMQPublisher from .team import make_team from .viewer import (AsciiViewer, ProgressViewer, ReplayWriter, ReplyToViewer, ResultPrinter) @@ -254,12 +254,14 @@ def setup_viewers(viewers, print_result=True): elif viewer == 'write-replay-to': viewer_state['viewers'].append(ReplayWriter(open(viewer_opts, 'w'))) elif viewer == 'publish-to': - zmq_external_publisher = ZMQPublisher(address=viewer_opts, bind=False) + zmq_context = zmq.Context() + zmq_external_publisher = ZMQPublisher(address=viewer_opts, bind=False, zmq_context=zmq_context) viewer_state['viewers'].append(zmq_external_publisher) elif viewer == 'tk': - zmq_publisher = ZMQPublisher(address='tcp://127.0.0.1') + zmq_context = zmq.Context() + zmq_publisher = ZMQPublisher(address='tcp://127.0.0.1', zmq_context=zmq_context) viewer_state['viewers'].append(zmq_publisher) - viewer_state['controller'] = setup_controller() + viewer_state['controller'] = Controller(zmq_context=zmq_context) _proc = TkViewer(address=zmq_publisher.socket_addr, controller=viewer_state['controller'].socket_addr, stop_after=viewer_opts.get('stop_at'), diff --git a/pelita/network.py b/pelita/network.py index 0c15edccd..26950d3ed 100644 --- a/pelita/network.py +++ b/pelita/network.py @@ -334,9 +334,9 @@ class ZMQPublisher: bind : bool Whether we are in bind or connect mode """ - def __init__(self, address, bind=True): + def __init__(self, address, bind=True, zmq_context=None): self.address = address - self.context = zmq.Context() + self.context = default_zmq_context(zmq_context) self.socket = self.context.socket(zmq.PUB) if bind: self.socket_addr = bind_socket(self.socket, self.address, '--publish') @@ -406,9 +406,3 @@ def await_action(self, await_action, timeout=None, accept_exit=True): if action in expected_actions: return action _logger.warning('Unexpected action %r. (Expected: %s) Ignoring.', action, ", ".join(expected_actions)) - - -def setup_controller(zmq_context=None): - zmq_context = default_zmq_context(zmq_context) - controller = Controller(zmq_context=zmq_context) - return controller From 0f7cbccbc5fa2066eca6d5388a22bfd397c5a80f Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Thu, 3 Jul 2025 22:16:10 +0200 Subject: [PATCH 19/27] RF: Raise (if needed) from add_fatal_error --- pelita/game.py | 39 +++++++++++---------------------------- test/test_game.py | 5 +++-- 2 files changed, 14 insertions(+), 30 deletions(-) diff --git a/pelita/game.py b/pelita/game.py index 360be77fc..1d9d52a8a 100644 --- a/pelita/game.py +++ b/pelita/game.py @@ -534,12 +534,7 @@ def setup_teams(team_specs, game_state, store_output=False, raise_bot_exceptions try: _state = team.wait_ready(timeout=0) except (RemotePlayerSendError, RemotePlayerRecvTimeout, RemotePlayerFailure) as e: - - if raise_bot_exceptions: - exit_remote_teams(game_state) - raise - - add_fatal_error(game_state, round=None, turn=team_idx, type=e.__class__.__name__, msg=str(e)) + add_fatal_error(game_state, round=None, turn=team_idx, type=e.__class__.__name__, msg=str(e), raise_bot_exceptions=raise_bot_exceptions) if len(e.args) > 1: game_print(team_idx, f"{type(e).__name__} ({e.args[0]}): {e.args[1]}") @@ -553,7 +548,7 @@ def setup_teams(team_specs, game_state, store_output=False, raise_bot_exceptions if not break_error and remote_sockets: break_error = True for socket, team_idx in remote_sockets.items(): - add_fatal_error(game_state, round=None, turn=team_idx, type='Timeout', msg='Team did not start (timeout).') + add_fatal_error(game_state, round=None, turn=team_idx, type='Timeout', msg='Team did not start (timeout).', raise_bot_exceptions=raise_bot_exceptions) game_print(team_idx, f"Team '{teams[team_idx]._team_spec}' did not start (timeout).") # if we encountered an error, the game_phase should have been set to FAILURE @@ -577,27 +572,15 @@ def send_initial(game_state, raise_bot_exceptions=False): _res = team.set_initial(team_idx, prepare_bot_state(game_state, team_idx)) except RemotePlayerFailure as e: - if raise_bot_exceptions: - exit_remote_teams(game_state) - raise PelitaBotError(e.error_type, e.error_msg) - - add_fatal_error(game_state, round=None, turn=team_idx, type=e.error_type, msg=e.error_msg) + add_fatal_error(game_state, round=None, turn=team_idx, type=e.error_type, msg=e.error_msg, raise_bot_exceptions=raise_bot_exceptions) game_print(team_idx, f"{e.error_type}: {e.error_msg}") except RemotePlayerSendError: - if raise_bot_exceptions: - exit_remote_teams(game_state) - raise PelitaBotError('Send error', 'Remote team unavailable') - - add_fatal_error(game_state, round=None, turn=team_idx, type='Send error', msg='Remote team unavailable') + add_fatal_error(game_state, round=None, turn=team_idx, type='Send error', msg='Remote team unavailable', raise_bot_exceptions=raise_bot_exceptions) game_print(team_idx, "Send error: Remote team unavailable") except RemotePlayerRecvTimeout: - if raise_bot_exceptions: - exit_remote_teams(game_state) - raise PelitaBotError('timeout', 'Timeout in set initial') - - add_fatal_error(game_state, round=None, turn=team_idx, type='timeout', msg='Timeout in set initial') + add_fatal_error(game_state, round=None, turn=team_idx, type='timeout', msg='Timeout in set initial', raise_bot_exceptions=raise_bot_exceptions) game_print(team_idx, "timeout: Timeout in set initial") @@ -843,13 +826,9 @@ def play_turn(game_state, raise_bot_exceptions=False): error_type = position_dict['error'] error_string = position_dict.get('error_msg', '') - if raise_bot_exceptions: - exit_remote_teams(game_state) - raise PelitaBotError(error_type, error_string) - # FatalExceptions (such as PlayerDisconnect) should immediately # finish the game - add_fatal_error(game_state, round=game_state['round'], turn=game_state['turn'], type=error_type, msg=error_string) + add_fatal_error(game_state, round=game_state['round'], turn=game_state['turn'], type=error_type, msg=error_string, raise_bot_exceptions=raise_bot_exceptions) position = None game_print(turn, f"{error_type}: {error_string}") @@ -1073,7 +1052,7 @@ def next_round_turn(game_state): 'turn': turn, } -def add_fatal_error(game_state, *, round, turn, type, msg): +def add_fatal_error(game_state, *, round, turn, type, msg, raise_bot_exceptions=False): team_idx = turn % 2 exception_event = { @@ -1117,6 +1096,10 @@ def add_fatal_error(game_state, *, round, turn, type, msg): 'game_phase': 'FINISHED' }) + if raise_bot_exceptions: + exit_remote_teams(game_state) + raise PelitaBotError(type, msg) + def check_gameover(game_state): """ Checks if this was the final moves or if the errors have exceeded the threshold. diff --git a/test/test_game.py b/test/test_game.py index 191d0bf0b..24bf2121f 100644 --- a/test/test_game.py +++ b/test/test_game.py @@ -1326,8 +1326,9 @@ def test_raises_and_exits_cleanly(): while not state["gameover"]: state = play_turn(state, raise_bot_exceptions=True) - # This is the state before the exception - assert state["gameover"] is False + # Game state is updated before the exception is raised + assert state["gameover"] is True + assert state["fatal_errors"][0][0]["type"] == "ZeroDivisionError" # Check that both processes have exited assert state["teams"][0].proc.wait(timeout=3) == 0 assert state["teams"][1].proc.wait(timeout=3) == 0 From 19410505f527f2e1acc456da8649600497e1e7fb Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Thu, 3 Jul 2025 22:44:24 +0200 Subject: [PATCH 20/27] RF: Print the error before potential exceptions are raised --- pelita/game.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pelita/game.py b/pelita/game.py index 1d9d52a8a..d2ca66b26 100644 --- a/pelita/game.py +++ b/pelita/game.py @@ -534,12 +534,12 @@ def setup_teams(team_specs, game_state, store_output=False, raise_bot_exceptions try: _state = team.wait_ready(timeout=0) except (RemotePlayerSendError, RemotePlayerRecvTimeout, RemotePlayerFailure) as e: - add_fatal_error(game_state, round=None, turn=team_idx, type=e.__class__.__name__, msg=str(e), raise_bot_exceptions=raise_bot_exceptions) - if len(e.args) > 1: game_print(team_idx, f"{type(e).__name__} ({e.args[0]}): {e.args[1]}") else: game_print(team_idx, f"{type(e).__name__}: {e}") + + add_fatal_error(game_state, round=None, turn=team_idx, type=e.__class__.__name__, msg=str(e), raise_bot_exceptions=raise_bot_exceptions) break_error = True del remote_sockets[socket] @@ -548,8 +548,8 @@ def setup_teams(team_specs, game_state, store_output=False, raise_bot_exceptions if not break_error and remote_sockets: break_error = True for socket, team_idx in remote_sockets.items(): - add_fatal_error(game_state, round=None, turn=team_idx, type='Timeout', msg='Team did not start (timeout).', raise_bot_exceptions=raise_bot_exceptions) game_print(team_idx, f"Team '{teams[team_idx]._team_spec}' did not start (timeout).") + add_fatal_error(game_state, round=None, turn=team_idx, type='Timeout', msg='Team did not start (timeout).', raise_bot_exceptions=raise_bot_exceptions) # if we encountered an error, the game_phase should have been set to FAILURE @@ -572,16 +572,16 @@ def send_initial(game_state, raise_bot_exceptions=False): _res = team.set_initial(team_idx, prepare_bot_state(game_state, team_idx)) except RemotePlayerFailure as e: - add_fatal_error(game_state, round=None, turn=team_idx, type=e.error_type, msg=e.error_msg, raise_bot_exceptions=raise_bot_exceptions) game_print(team_idx, f"{e.error_type}: {e.error_msg}") + add_fatal_error(game_state, round=None, turn=team_idx, type=e.error_type, msg=e.error_msg, raise_bot_exceptions=raise_bot_exceptions) except RemotePlayerSendError: - add_fatal_error(game_state, round=None, turn=team_idx, type='Send error', msg='Remote team unavailable', raise_bot_exceptions=raise_bot_exceptions) game_print(team_idx, "Send error: Remote team unavailable") + add_fatal_error(game_state, round=None, turn=team_idx, type='Send error', msg='Remote team unavailable', raise_bot_exceptions=raise_bot_exceptions) except RemotePlayerRecvTimeout: - add_fatal_error(game_state, round=None, turn=team_idx, type='timeout', msg='Timeout in set initial', raise_bot_exceptions=raise_bot_exceptions) game_print(team_idx, "timeout: Timeout in set initial") + add_fatal_error(game_state, round=None, turn=team_idx, type='timeout', msg='Timeout in set initial', raise_bot_exceptions=raise_bot_exceptions) def request_new_position(game_state): @@ -826,11 +826,11 @@ def play_turn(game_state, raise_bot_exceptions=False): error_type = position_dict['error'] error_string = position_dict.get('error_msg', '') - # FatalExceptions (such as PlayerDisconnect) should immediately - # finish the game - add_fatal_error(game_state, round=game_state['round'], turn=game_state['turn'], type=error_type, msg=error_string, raise_bot_exceptions=raise_bot_exceptions) - position = None game_print(turn, f"{error_type}: {error_string}") + add_fatal_error(game_state, round=game_state['round'], turn=game_state['turn'], + type=error_type, msg=error_string, + raise_bot_exceptions=raise_bot_exceptions) + position = None else: position = position_dict['move'] From ac9ad35094bad21d5ff52064d697909211bd5dd0 Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Wed, 30 Jul 2025 12:45:38 +0200 Subject: [PATCH 21/27] TST: Add network protocol test --- test/test_network.py | 172 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 172 insertions(+) diff --git a/test/test_network.py b/test/test_network.py index a0ac197b9..51fc2e77c 100644 --- a/test/test_network.py +++ b/test/test_network.py @@ -1,9 +1,13 @@ + +import concurrent.futures +import queue import sys import uuid import pytest import zmq +from pelita.game import play_turn, setup_game from pelita.network import bind_socket from pelita.scripts.pelita_player import player_handle_request from pelita.team import make_team @@ -128,3 +132,171 @@ def stopping(bot, state): sock.send_json(exit_msg) assert res[0] == "success" + + +@pytest.mark.parametrize("checkpoint", range(11)) +def test_simpleclient_broken(zmq_context, checkpoint): + # This test runs a test game against a (malicious) server client + # (a malicious subprocess client is harder to test) + # Depending on the checkpoint selected, the broken test client will + # run up to a particular point and then send a malicious message. + + # Depending on whether this message occurs in the game setup stage + # or during the game run, this will either set the phase to FAILURE or + # let the good team win. Pelita itself should not break in the process. + + timeout = 3000 + + q1 = queue.Queue() + q2 = queue.Queue() + + def dealer_good(q): + zmq_context = zmq.Context() + sock = zmq_context.socket(zmq.DEALER) + poll = zmq.Poller() + + port = sock.bind_to_random_port('tcp://127.0.0.1') + q.put(port) + + poll.register(sock, zmq.POLLIN) + _available_socks = poll.poll(timeout=timeout) + request = sock.recv_json() + assert request['REQUEST'] + sock.send_json({'__status__': 'ok', '__data__': {'team_name': 'good player'}}) + + _available_socks = poll.poll(timeout=timeout) + set_initial = sock.recv_json(flags=zmq.NOBLOCK) + if set_initial['__action__'] == 'exit': + return + assert set_initial['__action__'] == "set_initial" + sock.send_json({'__uuid__': set_initial['__uuid__'], '__return__': None}) + + for _i in range(8): + _available_socks = poll.poll(timeout=timeout) + game_state = sock.recv_json(flags=zmq.NOBLOCK) + + action = game_state['__action__'] + if action == 'exit': + return + assert set_initial['__action__'] == "set_initial" + + current_pos = game_state['__data__']['game_state']['team']['bot_positions'][game_state['__data__']['game_state']['bot_turn']] + sock.send_json({'__uuid__': game_state['__uuid__'], '__return__': {'move': current_pos}}) + + _available_socks = poll.poll(timeout=timeout) + exit_state = sock.recv_json(flags=zmq.NOBLOCK) + + assert exit_state['__action__'] == 'exit' + + def dealer_bad(q): + zmq_context = zmq.Context() + sock = zmq_context.socket(zmq.DEALER) + poll = zmq.Poller() + + port = sock.bind_to_random_port('tcp://127.0.0.1') + q.put(port) + + poll.register(sock, zmq.POLLIN) + # we set our recv to raise, if there is no message (zmq.NOBLOCK), + # so we do not need to care to check whether something is in the _available_socks + _available_socks = poll.poll(timeout=timeout) + + request = sock.recv_json(flags=zmq.NOBLOCK) + assert request['REQUEST'] + if checkpoint == 1: + sock.send_string("") + return + elif checkpoint == 2: + sock.send_json({'__status__': 'ok'}) + return + else: + sock.send_json({'__status__': 'ok', '__data__': {'team_name': f'bad <{checkpoint}>'}}) + + _available_socks = poll.poll(timeout=timeout) + + set_initial = sock.recv_json(flags=zmq.NOBLOCK) + + if checkpoint == 3: + sock.send_string("") + return + elif checkpoint == 4: + sock.send_json({'__uuid__': 'ok'}) + return + else: + sock.send_json({'__uuid__': set_initial['__uuid__'], '__data__': None}) + + for _i in range(8): + _available_socks = poll.poll(timeout=timeout) + game_state = sock.recv_json(flags=zmq.NOBLOCK) + + action = game_state['__action__'] + if action == 'exit': + return + + current_pos = game_state['__data__']['game_state']['team']['bot_positions'][game_state['__data__']['game_state']['bot_turn']] + if checkpoint == 5: + sock.send_string("No json") + return + elif checkpoint == 6: + # This is an acceptable message that will never match a request + # We can send the correct message afterwards and the match continues + sock.send_json({'__uuid__': "Bad", '__return__': "Nothing"}) + sock.send_json({'__uuid__': game_state['__uuid__'], '__return__': {'move': current_pos}}) + elif checkpoint == 7: + sock.send_json({'__uuid__': game_state['__uuid__'], '__return__': {'move': [0, 0]}}) + return + elif checkpoint == 8: + sock.send_json({'__uuid__': game_state['__uuid__'], '__return__': {'move': "NOTHING"}}) + return + elif checkpoint == 9: + sock.send_json({'__uuid__': game_state['__uuid__'], '__return__': {'move': [0, 0]}}) + return + else: + sock.send_json({'__uuid__': game_state['__uuid__'], '__return__': {'move': current_pos}}) + + with concurrent.futures.ThreadPoolExecutor(max_workers=2) as executor: + players = [] + players.append(executor.submit(dealer_good, q1)) + + if checkpoint == 0: + players.append(executor.submit(dealer_good, q2)) + else: + players.append(executor.submit(dealer_bad, q2)) + + port1 = q1.get() + port2 = q2.get() + + layout = {'walls': ((0, 0), (0, 1), (0, 2), (0, 3), (0, 4), (0, 5), (1, 0), (1, 5), (2, 0), (2, 5), (3, 0), (3, 2), (3, 3), (3, 5), (4, 0), (4, 2), (4, 3), (4, 5), (5, 0), (5, 5), (6, 0), (6, 5), (7, 0), (7, 1), (7, 2), (7, 3), (7, 4), (7, 5)), 'food': [(1, 1), (1, 2), (2, 1), (2, 2), (5, 3), (5, 4), (6, 3), (6, 4)], 'bots': [(1, 3), (6, 2), (1, 4), (6, 1)], 'shape': (8, 6)} + + game_state = setup_game([ + f'pelita://127.0.0.1:{port1}/PLAYER1', + f'pelita://127.0.0.1:{port2}/PLAYER2' + ], + layout_dict=layout, + max_rounds=2, + timeout_length=1, + ) + + # check that the game_state ends in the expected phase + match checkpoint: + case 0|5|6|7|8|9|10: + assert game_state['game_phase'] == 'RUNNING' + case 1|2|3|4: + assert game_state['game_phase'] == 'FAILURE' + + while game_state['game_phase'] == 'RUNNING': + game_state = play_turn(game_state) + + match checkpoint: + case 0|6|10: + assert game_state['game_phase'] == 'FINISHED' + assert game_state['whowins'] == 2 + case 5|7|8|9: + assert game_state['game_phase'] == 'FINISHED' + assert game_state['whowins'] == 0 + case 1|2|3|4: + assert game_state['game_phase'] == 'FAILURE' + + # check that no player had an uncaught exception + for player in concurrent.futures.as_completed(players): + assert player.exception() is None From e7e3dec02f90608968b77e86a859851057fbca11 Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Wed, 30 Jul 2025 15:28:01 +0200 Subject: [PATCH 22/27] ENH: Add 60*60 second timeout to pelita_player --- pelita/scripts/pelita_player.py | 20 +++++++++++++++++--- test/test_network.py | 6 ++++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/pelita/scripts/pelita_player.py b/pelita/scripts/pelita_player.py index 5de4bd444..cf80bf9a9 100755 --- a/pelita/scripts/pelita_player.py +++ b/pelita/scripts/pelita_player.py @@ -17,6 +17,9 @@ _logger = logging.getLogger(__name__) +# Maximum time that a player will wait for a move request +TIMEOUT_SECS = 60 * 60 + @contextlib.contextmanager def with_sys_path(dirname): sys.path.insert(0, dirname) @@ -40,6 +43,9 @@ def run_player(team_spec, address, team_name_override=False, silent_bots=False): # Connect to the given address context = zmq.Context() socket = context.socket(zmq.PAIR) + poller = zmq.Poller() + poller.register(socket, flags=zmq.POLLIN) + try: socket.connect(address) except zmq.ZMQError as e: @@ -68,12 +74,12 @@ def run_player(team_spec, address, team_name_override=False, silent_bots=False): socket.send_json({'__status__': 'ok', '__data__': data}) while True: - cont = player_handle_request(socket, team, team_name_override=team_name_override, silent_bots=silent_bots) + cont = player_handle_request(socket, poller, team, team_name_override=team_name_override, silent_bots=silent_bots) if not cont: return -def player_handle_request(socket, team, team_name_override=False, silent_bots=False): +def player_handle_request(socket, poller, team, team_name_override=False, silent_bots=False): """ Awaits a new request on `socket` and dispatches it to `team`. @@ -95,7 +101,15 @@ def player_handle_request(socket, team, team_name_override=False, silent_bots=Fa # answer from the player. try: - json_message = socket.recv_unicode() + socks = dict(poller.poll(timeout=TIMEOUT_SECS * 1000)) + if socks.get(socket) == zmq.POLLIN: + json_message = socket.recv_unicode() + else: + # TODO: Would be nice to tell Pelita main that we’re exiting + _logger.warning(f"No request in {TIMEOUT_SECS} seconds. Exiting player.") + + # returning False breaks the loop + return False except Exception: # Exit without sending a value back on the socket return True diff --git a/test/test_network.py b/test/test_network.py index 51fc2e77c..0bb68851d 100644 --- a/test/test_network.py +++ b/test/test_network.py @@ -48,6 +48,8 @@ def stopping(bot, state): team, _ = make_team(stopping, team_name='test stopping player') client_sock = zmq_context.socket(zmq.PAIR) client_sock.connect(f'tcp://127.0.0.1:{port}') + poller = zmq.Poller() + poller.register(client_sock, zmq.POLLIN) # Faking some data _uuid = uuid.uuid4().__str__() @@ -68,7 +70,7 @@ def stopping(bot, state): } } sock.send_json(set_initial) - player_handle_request(client_sock, team) + player_handle_request(client_sock, poller, team) assert sock.recv_json() == { '__uuid__': _uuid, @@ -114,7 +116,7 @@ def stopping(bot, state): } } sock.send_json(get_move) - player_handle_request(client_sock, team) + player_handle_request(client_sock, poller, team) assert sock.recv_json() == { '__uuid__': _uuid, From 6925dc0fb96510a0fc55bef8911a694f074718fe Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Wed, 30 Jul 2025 15:43:42 +0200 Subject: [PATCH 23/27] ENH: Reorganise remote Team classes --- pelita/game.py | 19 ++- pelita/network.py | 81 +++++------ pelita/team.py | 249 +++++++++++++--------------------- pelita/tournament/__init__.py | 9 +- 4 files changed, 153 insertions(+), 205 deletions(-) diff --git a/pelita/game.py b/pelita/game.py index d2ca66b26..c5c55ffc1 100644 --- a/pelita/game.py +++ b/pelita/game.py @@ -16,7 +16,7 @@ from .gamestate_filters import noiser, relocate_expired_food, update_food_age, in_homezone from .layout import get_legal_positions, initial_positions from .network import Controller, RemotePlayerFailure, RemotePlayerRecvTimeout, RemotePlayerSendError, ZMQPublisher -from .team import make_team +from .team import RemoteTeam, make_team from .viewer import (AsciiViewer, ProgressViewer, ReplayWriter, ReplyToViewer, ResultPrinter) @@ -509,15 +509,15 @@ def setup_teams(team_specs, game_state, store_output=False, raise_bot_exceptions initial_timeout = game_state['initial_timeout'] start = time.monotonic() - has_remote_teams = any(hasattr(team, 'zmqconnection') for team in teams) + has_remote_teams = any(isinstance(team, RemoteTeam) for team in teams) remote_sockets = {} if has_remote_teams: poll = zmq.Poller() for team_idx, team in enumerate(teams): - if hasattr(team, 'zmqconnection'): - poll.register(team.zmqconnection.socket, zmq.POLLIN) - remote_sockets[team.zmqconnection.socket] = team_idx + if isinstance(team, RemoteTeam): + poll.register(team.conn.socket, zmq.POLLIN) + remote_sockets[team.conn.socket] = team_idx break_error = False while remote_sockets and not break_error: @@ -548,7 +548,7 @@ def setup_teams(team_specs, game_state, store_output=False, raise_bot_exceptions if not break_error and remote_sockets: break_error = True for socket, team_idx in remote_sockets.items(): - game_print(team_idx, f"Team '{teams[team_idx]._team_spec}' did not start (timeout).") + game_print(team_idx, f"Team '{teams[team_idx].team_spec}' did not start (timeout).") add_fatal_error(game_state, round=None, turn=team_idx, type='Timeout', msg='Team did not start (timeout).', raise_bot_exceptions=raise_bot_exceptions) # if we encountered an error, the game_phase should have been set to FAILURE @@ -568,6 +568,10 @@ def send_initial(game_state, raise_bot_exceptions=False): teams = game_state['teams'] for team_idx, team in enumerate(teams): + # NB: Iterating over the teams may set the game_phase to FAILURE + if game_state['game_phase'] == 'FAILURE': + break + try: _res = team.set_initial(team_idx, prepare_bot_state(game_state, team_idx)) @@ -732,6 +736,7 @@ def prepare_bot_state(game_state, team_idx=None): 'shape': game_state['shape'], # only in initial round 'seed': seed # only used in set_initial phase }) + return bot_state @@ -1180,7 +1185,7 @@ def exit_remote_teams(game_state): continue try: team_game_state = prepare_bot_state(game_state, team_idx=idx) - team._exit(team_game_state) + team.send_exit(team_game_state) except AttributeError: pass diff --git a/pelita/network.py b/pelita/network.py index 26950d3ed..6dd28eae7 100644 --- a/pelita/network.py +++ b/pelita/network.py @@ -1,5 +1,4 @@ -import enum import json import logging import sys @@ -17,26 +16,6 @@ # The missing T stands for tcp PELITA_PORT = 41736 -## Pelita network data structures - -# ControlRequest -# {__action__} - -# ViewerUpdate -# {__action__, __data__} - -# Request -# {__uuid__, __action__, __data__} - -# Status -# {__status__, __data__} - -# Reply -# {__uuid__, __return__} - -# Error Reply -# {__uuid__, __error__, __error_msg__} - class RemotePlayerSendError(Exception): """ Raised when ZMQ cannot send a message (connection may have been lost). """ @@ -56,6 +35,13 @@ def __init__(self, error_type, error_msg): def __str__(self): return f"{self.error_type}: {self.error_msg}" +class RemotePlayerProtocolError(RemotePlayerFailure): + def __init__(self): + super().__init__("BadProtocol", "Bad protocol error") + +class BadState(Exception): + pass + #: The timeout to use during sending DEAD_CONNECTION_TIMEOUT = 3.0 @@ -105,15 +91,6 @@ def json_default_handler(o): # we don’t know the type: raise a Type error raise TypeError("Cannot convert %r of type %s to json" % (o, type(o))) -class ConnectionState(enum.Enum): - UNCONNECTED = 1 - AWAIT_ACK = 2 - CONNECTED = 3 - CLOSED = 4 - - # CLOSE_WAIT?? - ERR = 5 - class RemotePlayerConnection: """ This class is supposed to ease request–reply connections @@ -155,7 +132,7 @@ def __init__(self, socket: zmq.Socket): self.pollout = zmq.Poller() self.pollout.register(socket, zmq.POLLOUT) - self.state = ConnectionState.UNCONNECTED + self.state = "WAIT" def _send(self, action, data, msg_id): """ Sends a message or request `action` @@ -196,6 +173,13 @@ def send_req(self, action, data): self._send(action=action, data=data, msg_id=msg_id) return msg_id + def send_exit(self, payload): + if self.state == "EXITING": + return + + self.state = "EXITING" + self.send_req("exit", payload) + def _recv(self): """ Receive the next message on the socket. Will wait forever @@ -215,9 +199,13 @@ def _recv(self): try: py_obj = json.loads(json_message) except ValueError: - _logger.warning('Received non-json message.') - # TODO This should probably produce a failure - raise RemotePlayerRecvTimeout() + _logger.warning('Received non-json message. Closing socket.') + + # TODO: Should we tell the remote end that we are exiting? + self.socket.close() + self.state = "CLOSED" + + raise RemotePlayerProtocolError if '__error__' in py_obj: error_type = py_obj['__error__'] @@ -225,7 +213,7 @@ def _recv(self): _logger.warning(f'Received error reply ({error_type}): {error_message}. Closing socket.') self.socket.close() - self.state = ConnectionState.CLOSED + self.state = "CLOSED" # Failure in the pelita code on client side raise RemotePlayerFailure(error_type, error_message) @@ -242,13 +230,17 @@ def _recv(self): msg_data = py_obj.get('__data__') _logger.debug("<--- %r %r", msg_ack, msg_data) - self.state = ConnectionState.CONNECTED + self.state = "CONNECTED" return None, msg_data - print("NO MATCH", py_obj) + _logger.warning('Received malformed json message. Closing socket.') - return None, None + # TODO: Should we tell the remote end that we are exiting? + self.socket.close() + self.state = "CLOSED" + + raise RemotePlayerProtocolError def recv_status(self, timeout): @@ -265,11 +257,22 @@ def recv_status(self, timeout): if the message cannot be parsed from JSON PelitaRemoteError if an error message is returned + """ + if not self.state == "WAIT": + raise BadState + status = self.recv_timeout(None, timeout) + self.state = "CONNECTED" + return status + def recv_reply(self, expected_id, timeout): + + if not self.state == "CONNECTED": + raise BadState + return self.recv_timeout(expected_id, timeout) def recv_timeout(self, expected_id, timeout): """ Waits `timeout` seconds for a reply with msg_id `expected_id`. @@ -287,7 +290,7 @@ def recv_timeout(self, expected_id, timeout): ZMQConnectionError if an error message is returned """ - if self.state == ConnectionState.CLOSED: + if self.state == "CLOSED": return time_now = time.monotonic() diff --git a/pelita/team.py b/pelita/team.py index b92d65c1b..33615b307 100644 --- a/pelita/team.py +++ b/pelita/team.py @@ -16,7 +16,7 @@ from . import layout from .base_utils import default_zmq_context from .layout import BOT_I2N, layout_as_str, wall_dimensions -from .network import PELITA_PORT, RemotePlayerConnection, RemotePlayerSendError +from .network import PELITA_PORT, RemotePlayerConnection, RemotePlayerRecvTimeout, RemotePlayerSendError _logger = logging.getLogger(__name__) @@ -341,18 +341,88 @@ def __repr__(self): # # Rename set_initial -> start_game # -class SubprocessTeam: - def __init__(self, team_spec, *, zmq_context=None, idx=None, store_output=False): - zmq_context = default_zmq_context(zmq_context) - self._team_spec = team_spec - self.team_name = None +class RemoteTeam: + def __init__(self, team_spec, socket): + self.team_spec = team_spec + self._team_name = None #: Default timeout for a request, unless specified in the game_state - self._request_timeout = 3 + self.request_timeout = 3 + + self.conn = RemotePlayerConnection(socket) + + @property + def team_name(self): + return self._team_name + + def wait_ready(self, timeout): + msg = self.conn.recv_status(timeout) + try: + self._team_name = msg['team_name'] + except TypeError: + raise RemotePlayerRecvTimeout("", "") from None + + def set_initial(self, team_id, game_state): + timeout_length = game_state['timeout_length'] + + msg_id = self.conn.send_req("set_initial", {"team_id": team_id, + "game_state": game_state}) + reply = self.conn.recv_reply(msg_id, timeout_length) + # reply should be None + + return reply + + def get_move(self, game_state): + timeout_length = game_state['timeout_length'] + + msg_id = self.conn.send_req("get_move", {"game_state": game_state}) + reply = self.conn.recv_reply(msg_id, timeout_length) + + if "error" in reply: + return reply + # make sure that the move is a tuple + try: + reply["move"] = tuple(reply.get("move")) + except TypeError as e: + # This should also exit the remote connection + reply = { + "error": type(e).__name__, + "error_msg": str(e), + } + return reply - # TODO: State machine -- or add to ZMQConnection? - self._state = "" + def send_exit(self, game_state=None): + + if game_state: + payload = {'game_state': game_state} + else: + payload = {} + + try: + _logger.info("Sending exit to remote player %r.", self) + self.conn.send_exit(payload) + except RemotePlayerSendError: + _logger.info("Remote Player %r is already dead during exit. Ignoring.", self) + + def _teardown(self): + pass + + def __del__(self): + try: + self.send_exit() + self._teardown() + except AttributeError: + # in case we exit before self.proc or self.zmqconnection have been set + pass + + def __repr__(self): + team_name = f" ({self._team_name})" if self._team_name is not None else "" + return f"RemoteTeam<{self.team_spec}{team_name} on {self.bound_to_address}>" + +class SubprocessTeam(RemoteTeam): + def __init__(self, team_spec, *, zmq_context=None, idx=None, store_output=False): + zmq_context = default_zmq_context(zmq_context) # We bind to a local tcp port with a zmq PAIR socket # and start a new subprocess of pelita_player.py @@ -371,9 +441,9 @@ def __init__(self, team_spec, *, zmq_context=None, idx=None, store_output=False) else: color='' self.proc, self.stdout_path, self.stderr_path = self._call_pelita_player(team_spec, self.bound_to_address, - color=color, store_output=store_output) + color=color, store_output=store_output) - self.zmqconnection = RemotePlayerConnection(socket) + super().__init__(team_spec, socket) def _call_pelita_player(self, team_spec, address, color='', store_output=False): """ Starts another process with the same Python executable and runs `team_spec` @@ -381,11 +451,11 @@ def _call_pelita_player(self, team_spec, address, color='', store_output=False): """ player = 'pelita.scripts.pelita_player' external_call = [sys.executable, - '-m', - player, - 'remote-game', - team_spec, - address] + '-m', + player, + 'remote-game', + team_spec, + address] _logger.debug("Executing: %r", external_call) if store_output == subprocess.DEVNULL: @@ -403,68 +473,12 @@ def _call_pelita_player(self, team_spec, address, color='', store_output=False): else: return (subprocess.Popen(external_call), None, None) - def wait_ready(self, timeout): - msg = self.zmqconnection.recv_status(timeout) - self.team_name = msg.get('team_name') - - def set_initial(self, team_id, game_state): - - timeout_length = game_state['timeout_length'] - msg_id = self.zmqconnection.send_req("set_initial", {"team_id": team_id, - "game_state": game_state}) - reply = self.zmqconnection.recv_timeout(msg_id, timeout_length) - # TODO check error in reply - return reply - - def get_move(self, game_state): - timeout_length = game_state['timeout_length'] - - msg_id = self.zmqconnection.send_req("get_move", {"game_state": game_state}) - reply = self.zmqconnection.recv_timeout(msg_id, timeout_length) - if "error" in reply: - return reply - # make sure that the move is a tuple - reply["move"] = tuple(reply.get("move")) - # TODO: handle ValueError - return reply - - def _exit(self, game_state=None): - # We only want to exit once. - if getattr(self, '_sent_exit', False): - return - - if game_state: - payload = {'game_state': game_state} - else: - payload = {} - - try: - # TODO: make zmqconnection stateful. set flag when already disconnected - # For now, we simply check the state of the socket so that we do not send - # over an already closed socket. - if self.zmqconnection.socket.closed: - return - - self.zmqconnection.send_req("exit", payload) - self._sent_exit = True - except RemotePlayerSendError: - _logger.info("Remote Player %r is already dead during exit. Ignoring.", self) - - def __del__(self): - try: - self._exit() - if self.proc: - self.proc.terminate() - except AttributeError: - # in case we exit before self.proc or self.zmqconnection have been set - pass - - def __repr__(self): - team_name = f" ({self.team_name})" if self.team_name is not None else "" - return f"SubprocessTeam<{self._team_spec}{team_name} on {self.bound_to_address}>" + def _teardown(self): + if self.proc: + self.proc.terminate() -class RemoteTeam: +class RemoteServerTeam(RemoteTeam): """ Start a child process with the given `team_spec` and handle communication with it through a zmq.PAIR connection. @@ -489,16 +503,9 @@ class RemoteTeam: In the special case of store_output==subprocess.DEVNULL, stdout of the remote clients will be suppressed. """ - def __init__(self, team_spec, *, team_name=None, zmq_context=None, idx=None, store_output=False): - if zmq_context is None: - zmq_context = zmq.Context() - - self._team_spec = team_spec - self._team_name = team_name - - #: Default timeout for a request, unless specified in the game_state - self._request_timeout = 3 + def __init__(self, team_spec, *, team_name=None, zmq_context=None): + zmq_context = default_zmq_context(zmq_context) # We connect to a remote player that is listening # on the given team_spec address. @@ -524,76 +531,8 @@ def __init__(self, team_spec, *, team_name=None, zmq_context=None, idx=None, sto _logger.info("Connecting zmq.DEALER to remote player at {}.".format(send_addr)) socket.send_json({"REQUEST": team_spec}) - # WAIT_TIMEOUT = 5000 - # incoming = socket.poll(timeout=WAIT_TIMEOUT) - # if incoming == zmq.POLLIN: - # ok = socket.recv() - # else: - # # Server did not respond - # raise PlayerTimeout() - # self.proc = None - self.zmqconnection = RemotePlayerConnection(socket) - - def wait_ready(self, timeout): - msg = self.zmqconnection.recv_status(timeout) - self.team_name = msg.get('team_name') - - def set_initial(self, team_id, game_state): - - timeout_length = game_state['timeout_length'] - msg_id = self.zmqconnection.send_req("set_initial", {"team_id": team_id, - "game_state": game_state}) - reply = self.zmqconnection.recv_timeout(msg_id, timeout_length) - # TODO check error in reply - return reply - - def get_move(self, game_state): - timeout_length = game_state['timeout_length'] - - msg_id = self.zmqconnection.send_req("get_move", {"game_state": game_state}) - reply = self.zmqconnection.recv_timeout(msg_id, timeout_length) - if "error" in reply: - return reply - # make sure that the move is a tuple - reply["move"] = tuple(reply.get("move")) - # TODO: handle ValueError - return reply - - def _exit(self, game_state=None): - # We only want to exit once. - if getattr(self, '_sent_exit', False): - return - - if game_state: - payload = {'game_state': game_state} - else: - payload = {} - - try: - # TODO: make zmqconnection stateful. set flag when already disconnected - # For now, we simply check the state of the socket so that we do not send - # over an already closed socket. - if self.zmqconnection.socket.closed: - return - - self.zmqconnection.send_req("exit", payload) - self._sent_exit = True - except RemotePlayerSendError: - _logger.info("Remote Player %r is already dead during exit. Ignoring.", self) - - def __del__(self): - try: - self._exit() - if self.proc: - self.proc[0].terminate() - except AttributeError: - # in case we exit before self.proc or self.zmqconnection have been set - pass - - def __repr__(self): - team_name = f" ({self._team_name})" if self._team_name else "" - return f"RemoteTeam<{self._team_spec}{team_name} on {self.bound_to_address}>" + super().__init__(team_spec, socket) def make_team(team_spec, team_name=None, zmq_context=None, idx=None, store_output=False): @@ -632,7 +571,7 @@ def make_team(team_spec, team_name=None, zmq_context=None, idx=None, store_outpu zmq_context = default_zmq_context(zmq_context) if team_spec.startswith('pelita://'): _logger.info("Making a remote team for %s", team_spec) - team_player = RemoteTeam(team_spec=team_spec, zmq_context=zmq_context, idx=idx, store_output=store_output) + team_player = RemoteServerTeam(team_spec=team_spec, zmq_context=zmq_context) else: _logger.info("Making a subprocess team for %s", team_spec) team_player = SubprocessTeam(team_spec=team_spec, zmq_context=zmq_context, idx=idx, store_output=store_output) diff --git a/pelita/tournament/__init__.py b/pelita/tournament/__init__.py index 37f6ef603..5f696b040 100644 --- a/pelita/tournament/__init__.py +++ b/pelita/tournament/__init__.py @@ -15,7 +15,7 @@ import yaml import zmq -from ..team import make_team +from ..team import make_team, RemoteTeam from . import knockout_mode, roundrobin _logger = logging.getLogger(__name__) @@ -34,9 +34,10 @@ def check_team(team_spec): """ Instantiates a team from a team_spec and returns its name """ team, _zmq_context = make_team(team_spec) - if hasattr(team, 'zmqconnection'): - # TODO: Handle timeout - team.wait_ready(3) + match team: + case RemoteTeam(): + team.wait_ready(3) + return team.team_name From 4595c227fe97e5c219347c72c0305ee2f41f21f7 Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Tue, 26 Aug 2025 12:31:57 +0200 Subject: [PATCH 24/27] ENH: Remove xfail --- test/test_remote_game.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/test_remote_game.py b/test/test_remote_game.py index dd3edfd57..2a93b7a2e 100644 --- a/test/test_remote_game.py +++ b/test/test_remote_game.py @@ -60,7 +60,6 @@ def test_remote_run_game(remote_teams): assert state['timeouts'] == [{}, {}] -#@pytest.mark.xfail(reason="TODO: Fails in CI for macOS. Unclear why.") def test_remote_timeout(): # We have a slow player that also generates a bad move # in its second turn. From a26625c90b75a11d3d03111e699f0933ba32d427 Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Tue, 26 Aug 2025 12:44:48 +0200 Subject: [PATCH 25/27] TST: Add test for --no-timeout cli option --- .github/workflows/test_pytest.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/test_pytest.yml b/.github/workflows/test_pytest.yml index 651ce0cc7..a990cbfd2 100644 --- a/.github/workflows/test_pytest.yml +++ b/.github/workflows/test_pytest.yml @@ -55,6 +55,9 @@ jobs: - name: Run Pelita CLI as a script run: | pelita --null --rounds 100 --size small + - name: Run Pelita CLI without timeouts + run: | + pelita --null --rounds 100 --size small --no-timeout - name: Test Pelita template repo run: | # We must clone pelita_template to a location outside of the pelita repo From 0455642eb63194cde78bf708533d7a9550b3e7bd Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Tue, 26 Aug 2025 12:46:38 +0200 Subject: [PATCH 26/27] BF: Fix --no-timeout --- pelita/game.py | 5 ++++- pelita/scripts/pelita_main.py | 2 +- pelita/scripts/pelita_player.py | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pelita/game.py b/pelita/game.py index c5c55ffc1..c578235e8 100644 --- a/pelita/game.py +++ b/pelita/game.py @@ -25,7 +25,6 @@ ### Global constants -# All constants that are currently not redefinable in setup_game #: The points a team gets for killing another bot KILL_POINTS = 5 @@ -42,6 +41,10 @@ #: Food pellet shadow distance SHADOW_DISTANCE = 2 +#: Timeout to be chosen when run with --no-timeout +# (Players are expected to exit after 60 minutes without contact from pelita main) +MAX_TIMEOUT_SECS = 60 * 60 + #: Default maze sizes MSIZE = { 'small' : (16, 8), diff --git a/pelita/scripts/pelita_main.py b/pelita/scripts/pelita_main.py index 7097db162..47cfd8df3 100755 --- a/pelita/scripts/pelita_main.py +++ b/pelita/scripts/pelita_main.py @@ -245,7 +245,7 @@ def long_help(s): timeout_opt = game_settings.add_mutually_exclusive_group() timeout_opt.add_argument('--timeout', type=float, metavar="SEC", dest='timeout_length', help='Time before timeout is triggered (default: 3 seconds).') -timeout_opt.add_argument('--no-timeout', const=None, action='store_const', +timeout_opt.add_argument('--no-timeout', const=pelita.game.MAX_TIMEOUT_SECS, action='store_const', dest='timeout_length', help='Run game without timeouts.') game_settings.add_argument('--error-limit', type=int, default=5, dest='error_limit', help='Error limit. Reaching this limit disqualifies a team (default: 5).') diff --git a/pelita/scripts/pelita_player.py b/pelita/scripts/pelita_player.py index cf80bf9a9..453714636 100755 --- a/pelita/scripts/pelita_player.py +++ b/pelita/scripts/pelita_player.py @@ -18,6 +18,7 @@ _logger = logging.getLogger(__name__) # Maximum time that a player will wait for a move request +# Note that this means that a player will exit when a game has been paused for one hour TIMEOUT_SECS = 60 * 60 @contextlib.contextmanager From d4ce53730233204bb6184c26cb667f1a04418374 Mon Sep 17 00:00:00 2001 From: Rike-Benjamin Schuppner Date: Wed, 27 Aug 2025 17:23:15 +0200 Subject: [PATCH 27/27] BF: Fix default_zmq_context --- pelita/base_utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pelita/base_utils.py b/pelita/base_utils.py index 22ee1e3d1..a0629f0ca 100644 --- a/pelita/base_utils.py +++ b/pelita/base_utils.py @@ -15,4 +15,6 @@ def default_rng(seed=None): return Random(seed) def default_zmq_context(zmq_context=None): - return zmq.Context() + if zmq_context is None: + return zmq.Context() + return zmq_context