Fixed synchronization issues

- Added __enter__, __exit__, and .current_state() to game object
- set_orders throws an exception is the server phase is not the same as the client phase
- Returning only waiting dummy powers to bot

4 files changed, 71 insertions, 16 deletions

diff --git a/diplomacy/daide/request_managers.py b/diplomacy/daide/request_managers.py
index 9e37407..4d29189 100644
--- a/diplomacy/daide/request_managers.py
+++ b/diplomacy/daide/request_managers.py
@@ -278,6 +278,7 @@ def on_submit_orders_request(server, request, connection_handler, game):
         return [responses.REJ(bytes(request))]
 
     request.token = token
+    request.phase = game.get_current_phase()
 
     power = game.get_power(power_name)
     initial_power_adjusts = power.adjust[:]
diff --git a/diplomacy/engine/game.py b/diplomacy/engine/game.py
index 542bb27..fdd079c 100644
--- a/diplomacy/engine/game.py
+++ b/diplomacy/engine/game.py
@@ -21,6 +21,7 @@
 # pylint: disable=too-many-lines
 import base64
 import os
+import logging
 import sys
 import time
 import random
@@ -41,6 +42,7 @@ from diplomacy.utils.game_phase_data import GamePhaseData, MESSAGES_TYPE
 
 # Constants
 UNDETERMINED, POWER, UNIT, LOCATION, COAST, ORDER, MOVE_SEP, OTHER = 0, 1, 2, 3, 4, 5, 6, 7
+LOGGER = logging.getLogger(__name__)
 
 class Game(Jsonable):
     """
@@ -67,6 +69,10 @@ class Game(Jsonable):
                 e.g. { 'A PAR': 'MAR' }
         - error - Contains a list of errors that the game generated
                 e.g. ['NO MASTER SPECIFIED']
+        - fixed_state - used when game is a context of a with-block.
+            Store values that define the game state when entered in with-statement.
+            Compared to actual fixed state to detect any changes in methods where changes are not allowed.
+            Reset to None when exited from with-statement.
         - game_id: String that contains the current game's ID
                 e.g. '123456'
         - lost - Contains a dictionary of centers that have been lost during the term
@@ -167,7 +173,7 @@ class Game(Jsonable):
                  'convoy_paths_dest', 'zobrist_hash', 'renderer', 'game_id', 'map_name', 'role', 'rules',
                  'message_history', 'state_history', 'result_history', 'status', 'timestamp_created', 'n_controls',
                  'deadline', 'registration_password', 'observer_level', 'controlled_powers', '_phase_wrapper_type',
-                 'phase_abbr', '_unit_owner_cache', 'daide_port']
+                 'phase_abbr', '_unit_owner_cache', 'daide_port', 'fixed_state']
     zobrist_tables = {}
     rule_cache = ()
     model = {
@@ -235,6 +241,7 @@ class Game(Jsonable):
         self.observer_level = None
         self.controlled_powers = None
         self.daide_port = None
+        self.fixed_state = None
 
         # Caches
         self._unit_owner_cache = None               # {(unit, coast_required): owner}
@@ -399,6 +406,43 @@ class Game(Jsonable):
     # Application/network methods (mainly used for connected games).
     # ==============================================================
 
+    def current_state(self):
+        """ Returns the game object. To be used with the following syntax:
+            ```
+                with game.current_state():
+                    orders = players.get_orders(game, power_name)
+                    game.set_orders(power_name, orders)
+            ```
+        """
+        return self
+
+    def __enter__(self):
+        """ Enter into game context. Initialize fixed state.
+            Raise an exception if fixed state is already initialized to a different state, to prevent using the game
+            into multiple contexts at same time.
+        """
+        current_state = (self.get_current_phase(), self.get_hash())
+        if self.fixed_state and self.fixed_state != current_state:
+            raise RuntimeError('Game already used in a different context.')
+        self.fixed_state = current_state
+        return self
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        """ Exit from game context. Reset fixed state to None. """
+        self.fixed_state = None
+
+    def is_fixed_state_unchanged(self, log_error=True):
+        """ Check if actual state matches saved fixed state, if game is used as context of a with-block.
+            :param log_error: Boolean that indicates to log an error if state has changed
+            :return: boolean that indicates if the state has changed.
+        """
+        current_state = (self.get_current_phase(), self.get_hash())
+        if self.fixed_state and current_state != self.fixed_state:
+            if log_error:
+                LOGGER.error('State has changed from: %s to %s', self.fixed_state, current_state)
+            return False
+        return True
+
     def is_player_game(self):
         """ Return True if this game is a player game. """
         return self.has_power(self.role)
@@ -490,7 +534,9 @@ class Game(Jsonable):
                 # power must not have yet orders
                 and not self.get_orders(power_name)
                 # power must have orderable locations
-                and self.get_orderable_locations(power_name)]
+                and self.get_orderable_locations(power_name)
+                # power must be waiting
+                and self.get_power(power_name).wait]
 
     def get_controllers(self):
         """ Return a dictionary mapping each power name to its current controller name."""
@@ -1080,6 +1126,8 @@ class Game(Jsonable):
                 A LON H, F IRI - MAO, A IRI - MAO VIA, A WAL S F LON, A WAL S F MAO - IRI, F NWG C A NWY - EDI
                 A IRO R MAO, A IRO D, A LON B, F LIV B
         """
+        if not self.is_fixed_state_unchanged(log_error=bool(orders)):
+            return
         power_name = power_name.upper()
 
         if not self.has_power(power_name):
@@ -1112,6 +1160,8 @@ class Game(Jsonable):
             :param power_name: name of power to set wait flag.
             :param wait: wait flag (boolean).
         """
+        if not self.is_fixed_state_unchanged(log_error=False):
+            return
         power_name = power_name.upper()
 
         if not self.has_power(power_name):
@@ -1148,6 +1198,8 @@ class Game(Jsonable):
                                 all powers if None.
             :return: Nothing
         """
+        if not self.is_fixed_state_unchanged():
+            return
         if power_name is not None:
             power = self.get_power(power_name.upper())
             power.clear_orders()
diff --git a/diplomacy/server/request_managers.py b/diplomacy/server/request_managers.py
index 59c0e88..ff93977 100644
--- a/diplomacy/server/request_managers.py
+++ b/diplomacy/server/request_managers.py
@@ -71,6 +71,9 @@ def on_clear_orders(server, request, connection_handler):
     """
     level = verify_request(server, request, connection_handler, observer_role=False)
     assert_game_not_finished(level.game)
+    if not request.phase or request.phase != level.game.current_short_phase:
+        raise exceptions.ResponseException(
+            'Invalid order phase, received %s, server phase is %s' % (request.phase, level.game.current_short_phase))
     level.game.clear_orders(level.power_name)
     Notifier(server, ignore_addresses=[request.address_in_game]).notify_cleared_orders(level.game, level.power_name)
 
@@ -988,6 +991,9 @@ def on_set_orders(server, request, connection_handler):
     """
     level = verify_request(server, request, connection_handler, observer_role=False, require_power=True)
     assert_game_not_finished(level.game)
+    if not request.phase or request.phase != level.game.current_short_phase:
+        raise exceptions.ResponseException(
+            'Invalid order phase, received %s, server phase is %s' % (request.phase, level.game.current_short_phase))
     power = level.game.get_power(level.power_name)
     previous_wait = power.wait
     power.clear_orders()
diff --git a/diplomacy/server/server.py b/diplomacy/server/server.py
index 120e893..06648c1 100644
--- a/diplomacy/server/server.py
+++ b/diplomacy/server/server.py
@@ -53,7 +53,7 @@ import os
 from random import randint
 import socket
 import signal
-from typing import Dict, Set
+from typing import Dict, Set, List
 
 import tornado
 import tornado.web
@@ -236,8 +236,8 @@ class Server():
         # Each game also stores tokens connected (player tokens, observer tokens, omniscient tokens).
         self.games = {}  # type: Dict[str, ServerGame]
 
-        # Dictionary mapping game IDs to dummy power names.
-        self.games_with_dummy_powers = {}  # type: dict{str, set}
+        # Dictionary mapping game ID to list of power names.
+        self.games_with_dummy_powers = {}  # type: Dict[str, List[str]]
 
         # Dictionary mapping a game ID present in games_with_dummy_powers, to
         # a couple of associated bot token and time when bot token was associated to this game ID.
@@ -532,7 +532,7 @@ class Server():
             :param server_game: server game to check
             :type server_game: ServerGame
         """
-        updated = False
+        dummy_power_names = []
         if server_game.is_game_active or server_game.is_game_paused:
             dummy_power_names = server_game.get_dummy_unordered_power_names()
             if dummy_power_names:
@@ -542,22 +542,17 @@ class Server():
                 # then we also update bot time in registry of dummy powers associated to bot tokens.
                 bot_token, _ = self.dispatched_dummy_powers.get(server_game.game_id, (None, None))
                 self.dispatched_dummy_powers[server_game.game_id] = (bot_token, common.timestamp_microseconds())
-                updated = True
-        if not updated:
-            # Registry not updated for this game, meaning that there is no
-            # dummy powers waiting for orders or 'no wait' for this game.
+        if not dummy_power_names:
+            # No waiting dummy powers for this game, or game is not playable (canceled, completed, or forming).
             self.games_with_dummy_powers.pop(server_game.game_id, None)
-            # We remove game from registry of dummy powers associated to bot tokens only if game is terminated.
-            # Otherwise, game will remain associated to a previous bot token, until bot failed to order powers.
-            if server_game.is_game_completed or server_game.is_game_canceled:
-                self.dispatched_dummy_powers.pop(server_game.game_id, None)
+            self.dispatched_dummy_powers.pop(server_game.game_id, None)
 
     def get_dummy_waiting_power_names(self, buffer_size, bot_token):
         """ Return names of dummy powers waiting for orders for current loaded games.
             This query is allowed only for bot tokens.
             :param buffer_size: maximum number of powers queried.
             :param bot_token: bot token
-            :return: a dictionary mapping game IDs to lists of power names.
+            :return: a dictionary mapping each game ID to a list of power names.
         """
         if self.users.get_name(bot_token) != constants.PRIVATE_BOT_USERNAME:
             raise exceptions.ResponseException('Invalid bot token %s' % bot_token)
@@ -568,7 +563,8 @@ class Server():
             if registered_token is not None:
                 time_elapsed_seconds = (common.timestamp_microseconds() - registered_time) / 1000000
                 if time_elapsed_seconds > constants.PRIVATE_BOT_TIMEOUT_SECONDS or registered_token == bot_token:
-                    # This game still has dummy powers but time allocated to previous bot token is over.
+                    # This game still has dummy powers but, either time allocated to previous bot token is over,
+                    # or bot dedicated to this game is asking for current dummy powers of this game.
                     # Forget previous bot token.
                     registered_token = None
             if registered_token is None: