From 1b449fc91ad45203c551f7d9087973bed8d63d7c Mon Sep 17 00:00:00 2001 From: Askaholic Date: Sun, 21 Nov 2021 12:54:01 -0900 Subject: [PATCH 1/7] Refactor game options dictionary --- server/gameconnection.py | 24 +---------- server/games/coop.py | 2 +- server/games/game.py | 53 +++++++++++++++++++------ server/ladder_service/ladder_service.py | 2 +- tests/unit_tests/test_game.py | 6 +-- tests/unit_tests/test_gameconnection.py | 12 +++--- tests/unit_tests/test_ladder_service.py | 4 +- 7 files changed, 56 insertions(+), 47 deletions(-) diff --git a/server/gameconnection.py b/server/gameconnection.py index e447e9668..9c7784563 100644 --- a/server/gameconnection.py +++ b/server/gameconnection.py @@ -21,8 +21,7 @@ GameConnectionState, GameError, GameState, - ValidityState, - Victory + ValidityState ) from .games.typedefs import FA from .player_service import PlayerService @@ -228,26 +227,7 @@ async def handle_game_option(self, key: str, value: Any): if not self.is_host(): return - if key == "Victory": - self.game.gameOptions["Victory"] = Victory.__members__.get( - value.upper(), None - ) - else: - self.game.gameOptions[key] = value - - if key == "Slots": - self.game.max_players = int(value) - elif key == "ScenarioFile": - raw = repr(value) - self.game.map_scenario_path = \ - raw.replace("\\", "/").replace("//", "/").replace("'", "") - self.game.map_file_path = "maps/{}.zip".format( - self.game.map_scenario_path.split("/")[2].lower() - ) - elif key == "Title": - with contextlib.suppress(ValueError): - self.game.name = value - + self.game.set_game_option(key, value) self._mark_dirty() async def handle_game_mods(self, mode: Any, args: list[Any]): diff --git a/server/games/coop.py b/server/games/coop.py index e556de87a..b6a4b27e9 100644 --- a/server/games/coop.py +++ b/server/games/coop.py @@ -12,7 +12,7 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.validity = ValidityState.COOP_NOT_RANKED - self.gameOptions.update({ + self.game_options.update({ "Victory": Victory.SANDBOX, "TeamSpawn": "fixed", "RevealedCivilians": "No", diff --git a/server/games/game.py b/server/games/game.py index a489005e6..ad59f8c5d 100644 --- a/server/games/game.py +++ b/server/games/game.py @@ -1,4 +1,5 @@ import asyncio +import contextlib import json import logging import time @@ -87,7 +88,6 @@ def __init__( ) self.id = id_ self.visibility = VisibilityState.PUBLIC - self.max_players = max_players self.host = host self.name = name self.map_id = None @@ -107,7 +107,8 @@ def __init__( self._connections = {} self._configured_player_ids: set[int] = set() self.enforce_rating = False - self.gameOptions = { + self.game_options = { + "Slots": max_players, "FogOfWar": "explored", "GameSpeed": "normal", "Victory": Victory.DEMORALIZATION, @@ -155,6 +156,10 @@ def set_name_unchecked(self, value: str): max_len = game_stats.c.gameName.type.length self._name = value[:max_len] + @property + def max_players(self) -> Optional[int]: + return self.get_game_option("Slots") + @property def armies(self) -> frozenset[int]: return frozenset( @@ -327,7 +332,7 @@ def _process_army_stats_for_player(self, player): try: if ( len(self._army_stats_list) == 0 - or self.gameOptions["CheatsEnabled"] != "false" + or self.game_options["CheatsEnabled"] != "false" ): return @@ -565,12 +570,29 @@ def get_basic_info(self) -> BasicGameInfo: self.get_team_sets(), ) - def set_player_option(self, player_id: int, key: str, value: Any): - """ - Set game-associative options for given player, by id - """ - self._configured_player_ids.add(player_id) - self._player_options[player_id][key] = value + def get_game_option(self, key: str, default: Any = None) -> Optional[Any]: + return self.game_options.get(key, default) + + def set_game_option(self, key: str, value: Any): + # Type transformations + if key == "Victory": + value = Victory.__members__.get(value.upper()) + elif key == "Slots": + value = int(value) + + self.game_options[key] = value + + # Additional attributes + if key == "ScenarioFile": + raw = repr(value) + self.map_scenario_path = \ + raw.replace("\\", "/").replace("//", "/").replace("'", "") + self.game.map_file_path = "maps/{}.zip".format( + self.map_scenario_path.split("/")[2].lower() + ) + elif key == "Title": + with contextlib.suppress(ValueError): + self.name = value def get_player_option(self, player_id: int, key: str) -> Optional[Any]: """ @@ -578,6 +600,13 @@ def get_player_option(self, player_id: int, key: str) -> Optional[Any]: """ return self._player_options[player_id].get(key) + def set_player_option(self, player_id: int, key: str, value: Any): + """ + Set game-associative options for given player, by id + """ + self._configured_player_ids.add(player_id) + self._player_options[player_id][key] = value + def set_ai_option(self, name, key, value): """ Set game-associative options for given AI, by name @@ -662,11 +691,11 @@ async def validate_game_mode_settings(self): async def _validate_game_options( self, valid_options: dict[str, tuple[Any, ValidityState]] ) -> bool: - for key, value in self.gameOptions.items(): + for key, value in self.game_options.items(): if key in valid_options: (valid_value, validity_state) = valid_options[key] - if valid_value != self.gameOptions[key]: await self.mark_invalid(validity_state) + if valid_value != self.game_options[key]: return False return True @@ -733,7 +762,7 @@ async def update_game_stats(self): # In some cases, games can be invalidated while running: we check for those cases when # the game ends and update this record as appropriate. - game_type = str(self.gameOptions.get("Victory").value) + game_type = str(self.get_game_option("Victory").value) async with self._db.acquire() as conn: await conn.execute( diff --git a/server/ladder_service/ladder_service.py b/server/ladder_service/ladder_service.py index e7046493d..ba6a680fe 100644 --- a/server/ladder_service/ladder_service.py +++ b/server/ladder_service/ladder_service.py @@ -559,7 +559,7 @@ def get_displayed_rating(player: Player) -> float: game_options = queue.get_game_options() if game_options: - game.gameOptions.update(game_options) + game.game_options.update(game_options) mapname = re.match("maps/(.+).zip", map_path).group(1) # FIXME: Database filenames contain the maps/ prefix and .zip suffix. diff --git a/tests/unit_tests/test_game.py b/tests/unit_tests/test_game.py index 14ff3f24d..efdd84487 100644 --- a/tests/unit_tests/test_game.py +++ b/tests/unit_tests/test_game.py @@ -141,11 +141,11 @@ async def check_game_settings( game: Game, settings: list[tuple[str, Any, ValidityState]] ): for key, value, expected in settings: - old = game.gameOptions.get(key) - game.gameOptions[key] = value + old = game.game_options.get(key) + game.game_options[key] = value await game.validate_game_settings() assert game.validity is expected - game.gameOptions[key] = old + game.game_options[key] = old async def test_add_result_unknown(game, game_add_players): diff --git a/tests/unit_tests/test_gameconnection.py b/tests/unit_tests/test_gameconnection.py index 0780cdb5b..06c9ac728 100644 --- a/tests/unit_tests/test_gameconnection.py +++ b/tests/unit_tests/test_gameconnection.py @@ -406,11 +406,11 @@ async def test_handle_action_GameOption( game: Game, game_connection: GameConnection ): - game.gameOptions = {"AIReplacement": "Off"} + game.game_options = {"AIReplacement": "Off"} await game_connection.handle_action("GameOption", ["Victory", "sandbox"]) - assert game.gameOptions["Victory"] == Victory.SANDBOX + assert game.game_options["Victory"] == Victory.SANDBOX await game_connection.handle_action("GameOption", ["AIReplacement", "On"]) - assert game.gameOptions["AIReplacement"] == "On" + assert game.game_options["AIReplacement"] == "On" await game_connection.handle_action("GameOption", ["Slots", "7"]) assert game.max_players == 7 # I don't know what these paths actually look like @@ -419,7 +419,7 @@ async def test_handle_action_GameOption( await game_connection.handle_action("GameOption", ["Title", "All welcome"]) assert game.name == "All welcome" await game_connection.handle_action("GameOption", ["ArbitraryKey", "ArbitraryValue"]) - assert game.gameOptions["ArbitraryKey"] == "ArbitraryValue" + assert game.game_options["ArbitraryKey"] == "ArbitraryValue" async def test_handle_action_GameOption_not_host( @@ -428,9 +428,9 @@ async def test_handle_action_GameOption_not_host( players ): game_connection.player = players.joining - game.gameOptions = {"Victory": "asdf"} + game.game_options = {"Victory": "asdf"} await game_connection.handle_action("GameOption", ["Victory", "sandbox"]) - assert game.gameOptions == {"Victory": "asdf"} + assert game.game_options == {"Victory": "asdf"} async def test_json_stats( diff --git a/tests/unit_tests/test_ladder_service.py b/tests/unit_tests/test_ladder_service.py index b62b063a3..d27c3f74d 100644 --- a/tests/unit_tests/test_ladder_service.py +++ b/tests/unit_tests/test_ladder_service.py @@ -190,8 +190,8 @@ async def test_start_game_with_game_options( assert game.rating_type == queue.rating_type assert game.max_players == 2 - assert game.gameOptions["Share"] == "ShareUntilDeath" - assert game.gameOptions["UnitCap"] == 500 + assert game.game_options["Share"] == "ShareUntilDeath" + assert game.game_options["UnitCap"] == 500 LadderGame.wait_launched.assert_called_once() From f3ac27d020c87c5c45167ca8a8db418249b362a9 Mon Sep 17 00:00:00 2001 From: Askaholic Date: Sun, 3 Apr 2022 20:38:44 -0800 Subject: [PATCH 2/7] Refactor validity and include it in game_info message --- Pipfile | 1 + Pipfile.lock | 11 +- server/game_service.py | 31 ++- server/gameconnection.py | 4 +- server/games/coop.py | 27 ++- server/games/custom_game.py | 25 ++- server/games/game.py | 242 ++++++++++-------------- server/games/typedefs.py | 7 +- server/games/validator.py | 105 ++++++++++ server/ladder_service/ladder_service.py | 7 +- server/types.py | 9 + tests/conftest.py | 2 +- tests/unit_tests/test_custom_game.py | 8 +- tests/unit_tests/test_game.py | 141 ++++++++------ tests/unit_tests/test_gameconnection.py | 27 +-- tests/unit_tests/test_laddergame.py | 4 +- 16 files changed, 390 insertions(+), 261 deletions(-) create mode 100644 server/games/validator.py diff --git a/Pipfile b/Pipfile index 5104a3fc5..74d879ec7 100644 --- a/Pipfile +++ b/Pipfile @@ -15,6 +15,7 @@ aio_pika = "~=8.2" aiocron = "*" aiohttp = "*" aiomysql = {git = "https://github.com/aio-libs/aiomysql"} +cachetools = "*" docopt = "*" humanize = ">=2.6.0" maxminddb = "*" diff --git a/Pipfile.lock b/Pipfile.lock index 33dcdb526..615d77df8 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "4d529b570c113ec473c190611dff3214e3c53b6e91cb90fd53d49249c2638849" + "sha256": "fbf667c3fa0630610daae2298439d383883041c04a430f2c17a66fc02e641701" }, "pipfile-spec": 6, "requires": { @@ -152,6 +152,15 @@ "markers": "python_version >= '3.7'", "version": "==23.1.0" }, + "cachetools": { + "hashes": [ + "sha256:086ee420196f7b2ab9ca2db2520aca326318b68fe5ba8bc4d49cca91add450f2", + "sha256:861f35a13a451f94e301ce2bec7cac63e881232ccce7ed67fab9b5df4d3beaa1" + ], + "index": "pypi", + "markers": "python_version >= '3.7'", + "version": "==5.3.2" + }, "cffi": { "hashes": [ "sha256:0c9ef6ff37e974b73c25eecc13952c55bceed9112be2d9d938ded8e856138bcc", diff --git a/server/game_service.py b/server/game_service.py index 1e44bfa42..64751d2a3 100644 --- a/server/game_service.py +++ b/server/game_service.py @@ -7,6 +7,7 @@ from typing import Optional, Union, ValuesView import aiocron +from cachetools import LRUCache from sqlalchemy import select from server.config import config @@ -30,6 +31,7 @@ from .message_queue_service import MessageQueueService from .players import Player from .rating_service import RatingService +from .types import MapInfo @with_logger @@ -57,12 +59,15 @@ def __init__( self._allow_new_games = False self._drain_event = None - # Populated below in really_update_static_ish_data. + # Populated below in update_data. self.featured_mods = dict() # A set of mod ids that are allowed in ranked games self.ranked_mods: set[str] = set() + # A cache of map_version info needed by Game + self.map_info_cache = LRUCache(maxsize=128) + # The set of active games self._games: dict[int, Game] = dict() @@ -120,6 +125,30 @@ async def update_data(self): # Turn resultset into a list of uids self.ranked_mods = {row.uid for row in result} + async def get_map_info(self, filename: str) -> Optional[MapInfo]: + filename = filename.lower() + map_info = self.map_info_cache.get(filename) + if map_info is not None: + return map_info + + async with self._db.acquire() as conn: + result = await conn.execute( + "SELECT id, filename, ranked FROM map_version " + "WHERE lower(filename) = lower(:filename)", + filename=filename + ) + row = result.fetchone() + if not row: + return None + + map_info = MapInfo( + id=row.id, + filename=row.filename, + ranked=row.ranked + ) + self.map_info_cache[filename] = map_info + return map_info + def mark_dirty(self, obj: Union[Game, MatchmakerQueue]): if isinstance(obj, Game): self._dirty_games.add(obj) diff --git a/server/gameconnection.py b/server/gameconnection.py index 9c7784563..412f4ffbd 100644 --- a/server/gameconnection.py +++ b/server/gameconnection.py @@ -312,7 +312,9 @@ async def handle_operation_complete( ) return - if self.game.validity != ValidityState.COOP_NOT_RANKED: + validity = self.game.get_validity() + if validity is not ValidityState.COOP_NOT_RANKED: + self._logger.info("Game was not valid: %s", validity) return secondary, delta = secondary, str(delta) diff --git a/server/games/coop.py b/server/games/coop.py index b6a4b27e9..12716ee8a 100644 --- a/server/games/coop.py +++ b/server/games/coop.py @@ -1,3 +1,5 @@ +from server.games.validator import COMMON_RULES, GameOptionRule, Validator + from .game import Game from .typedefs import FA, GameType, InitMode, ValidityState, Victory @@ -6,12 +8,21 @@ class CoopGame(Game): """Class for coop game""" init_mode = InitMode.NORMAL_LOBBY game_type = GameType.COOP + default_validity = ValidityState.COOP_NOT_RANKED + validator = Validator([ + *COMMON_RULES, + GameOptionRule("Victory", Victory.SANDBOX, ValidityState.WRONG_VICTORY_CONDITION), + GameOptionRule("TeamSpawn", "fixed", ValidityState.SPAWN_NOT_FIXED), + GameOptionRule("RevealedCivilians", FA.DISABLED, ValidityState.CIVILIANS_REVEALED), + GameOptionRule("Difficulty", 3, ValidityState.WRONG_DIFFICULTY), + GameOptionRule("Expansion", FA.ENABLED, ValidityState.EXPANSION_DISABLED), + ]) def __init__(self, *args, **kwargs): kwargs["game_mode"] = "coop" super().__init__(*args, **kwargs) - self.validity = ValidityState.COOP_NOT_RANKED + self.is_coop = True self.game_options.update({ "Victory": Victory.SANDBOX, "TeamSpawn": "fixed", @@ -21,20 +32,6 @@ def __init__(self, *args, **kwargs): }) self.leaderboard_saved = False - async def validate_game_mode_settings(self): - """ - Checks which only apply to the coop mode - """ - - valid_options = { - "Victory": (Victory.SANDBOX, ValidityState.WRONG_VICTORY_CONDITION), - "TeamSpawn": ("fixed", ValidityState.SPAWN_NOT_FIXED), - "RevealedCivilians": (FA.DISABLED, ValidityState.CIVILIANS_REVEALED), - "Difficulty": (3, ValidityState.WRONG_DIFFICULTY), - "Expansion": (FA.ENABLED, ValidityState.EXPANSION_DISABLED), - } - await self._validate_game_options(valid_options) - async def process_game_results(self): """ When a coop game ends, we don't expect there to be any game results. diff --git a/server/games/custom_game.py b/server/games/custom_game.py index c71523bc4..588451e0c 100644 --- a/server/games/custom_game.py +++ b/server/games/custom_game.py @@ -1,26 +1,37 @@ import time +from typing import Optional from server.decorators import with_logger +from server.games.validator import COMMON_RULES, NON_COOP_RULES, Validator from server.rating import RatingType from .game import Game from .typedefs import GameType, InitMode, ValidityState +def minimum_length_rule(game: Game) -> Optional[ValidityState]: + if game.launched_at is None: + return + + limit = len(game.players) * 60 + if not game.enforce_rating and time.time() - game.launched_at < limit: + return ValidityState.TOO_SHORT + + @with_logger class CustomGame(Game): init_mode = InitMode.NORMAL_LOBBY game_type = GameType.CUSTOM + validator = Validator([ + *COMMON_RULES, + *NON_COOP_RULES, + minimum_length_rule + ]) - def __init__(self, id_, *args, **kwargs): + def __init__(self, *args, **kwargs): new_kwargs = { "rating_type": RatingType.GLOBAL, "setup_timeout": 30 } new_kwargs.update(kwargs) - super().__init__(id_, *args, **new_kwargs) - - async def _run_pre_rate_validity_checks(self): - limit = len(self.players) * 60 - if not self.enforce_rating and time.time() - self.launched_at < limit: - await self.mark_invalid(ValidityState.TOO_SHORT) + super().__init__(*args, **new_kwargs) diff --git a/server/games/game.py b/server/games/game.py index ad59f8c5d..a90d0b777 100644 --- a/server/games/game.py +++ b/server/games/game.py @@ -2,6 +2,7 @@ import contextlib import json import logging +import re import time from collections import defaultdict from typing import Any, Iterable, Optional @@ -26,12 +27,13 @@ GameResultReports, resolve_game ) +from server.games.validator import COMMON_RULES, NON_COOP_RULES, Validator from server.rating import InclusiveRange, RatingType from server.timing import datetime_now +from server.types import MapInfo from ..players import Player, PlayerState from .typedefs import ( - FA, BasicGameInfo, EndedGameInfo, FeaturedModType, @@ -44,6 +46,8 @@ VisibilityState ) +MAP_FILE_PATTERN = re.compile("maps/(.+).zip") + class GameError(Exception): pass @@ -55,6 +59,11 @@ class Game: """ init_mode = InitMode.NORMAL_LOBBY game_type = GameType.CUSTOM + default_validity = ValidityState.VALID + validator = Validator([ + *COMMON_RULES, + *NON_COOP_RULES + ]) def __init__( self, @@ -81,7 +90,7 @@ def __init__( self.game_service = game_service self._player_options: dict[int, dict[str, Any]] = defaultdict(dict) self.hosted_at = None - self.launched_at = None + self.launched_at: Optional[float] = None self.finished = False self._logger = logging.getLogger( f"{self.__class__.__qualname__}.{id_}" @@ -90,14 +99,15 @@ def __init__( self.visibility = VisibilityState.PUBLIC self.host = host self.name = name - self.map_id = None + self.is_coop = False + self.map_info = None + self.map_id: Optional[int] = None + self.map_ranked = True self.map_file_path = f"maps/{map_}.zip" - self.map_scenario_path = None self.password = None self._players_at_launch: list[Player] = [] self.AIs = {} self.desyncs = 0 - self.validity = ValidityState.VALID self.game_mode = game_mode self.rating_type = rating_type or RatingType.GLOBAL self.displayed_rating_range = displayed_rating_range or InclusiveRange() @@ -121,6 +131,8 @@ def __init__( "Unranked": "No" } self.mods = {} + self._override_validity: Optional[ValidityState] = None + self._persisted_validity: Optional[ValidityState] = None self._hosted_future = asyncio.Future() self._finish_lock = asyncio.Lock() @@ -156,6 +168,21 @@ def set_name_unchecked(self, value: str): max_len = game_stats.c.gameName.type.length self._name = value[:max_len] + @property + def map_folder_name(self) -> str: + """ + Map folder name + """ + m = re.match(MAP_FILE_PATTERN, self.map_file_path) + if m: + return m.group(1) + else: + return "scmp_009" + + @property + def is_map_generated(self) -> bool: + return "neroxis_map_generator" in self.map_file_path + @property def max_players(self) -> Optional[int]: return self.get_game_option("Slots") @@ -246,6 +273,12 @@ def is_even(self) -> bool: team_sizes = set(len(team) for team in teams) return len(team_sizes) == 1 + def get_validity(self) -> ValidityState: + if self._override_validity is not None: + return self._override_validity + + return self.validator.get_one(self) or self.default_validity + def get_team_sets(self) -> list[set[Player]]: """ Returns a list of teams represented as sets of players. @@ -273,6 +306,19 @@ def set_hosted(self): self._hosted_future.set_result(None) self.hosted_at = datetime_now() + async def update_map_info(self): + self.set_map_info( + await self.game_service.get_map_info(self.map_file_path) + ) + + def set_map_info(self, map_info: Optional[MapInfo]): + if map_info: + self.map_id = map_info.id + self.map_ranked = map_info.ranked + else: + self.map_id = None + self.map_ranked = False + async def add_result( self, reporter: int, @@ -430,12 +476,26 @@ async def on_game_finish(self): elif self.state is GameState.LIVE: self._logger.info("Game finished normally") - if self.desyncs > 20: - await self.mark_invalid(ValidityState.TOO_MANY_DESYNCS) - return - + # Needed by some validity checks + self.state = GameState.ENDED await self.process_game_results() + validity = self.get_validity() + if validity is not self._persisted_validity: + assert validity is not self.default_validity + + self._logger.info("Updating validity to: %s", validity) + async with self._db.acquire() as conn: + await conn.execute( + game_stats.update().where( + game_stats.c.id == self.id + ).values( + validity=validity.value + ) + ) + self._persisted_validity = validity + return + self._process_pending_army_stats() except Exception: # pragma: no cover self._logger.exception("Error during game end") @@ -444,12 +504,8 @@ async def on_game_finish(self): self.game_service.mark_dirty(self) - async def _run_pre_rate_validity_checks(self): - pass - async def process_game_results(self): if not self._results: - await self.mark_invalid(ValidityState.UNKNOWN_RESULT) return await self.persist_results() @@ -461,8 +517,6 @@ async def resolve_game_results(self) -> EndedGameInfo: if self.state not in (GameState.LIVE, GameState.ENDED): raise GameError("Cannot rate game that has not been launched.") - await self._run_pre_rate_validity_checks() - basic_info = self.get_basic_info() team_army_results = [ @@ -472,7 +526,8 @@ async def resolve_game_results(self) -> EndedGameInfo: team_outcomes = [GameOutcome.UNKNOWN for _ in basic_info.teams] - if self.validity is ValidityState.VALID: + validity = self.get_validity() + if validity is ValidityState.VALID: team_player_partial_outcomes = [ {self.get_player_outcome(player) for player in team} for team in basic_info.teams @@ -485,7 +540,7 @@ async def resolve_game_results(self) -> EndedGameInfo: or resolve_game(team_player_partial_outcomes) ) except GameResolutionError: - await self.mark_invalid(ValidityState.UNKNOWN_RESULT) + self._override_validity = ValidityState.UNKNOWN_RESULT try: commander_kills = { @@ -497,7 +552,7 @@ async def resolve_game_results(self) -> EndedGameInfo: return EndedGameInfo.from_basic( basic_info, - self.validity, + validity, team_outcomes, commander_kills, team_army_results, @@ -584,12 +639,19 @@ def set_game_option(self, key: str, value: Any): # Additional attributes if key == "ScenarioFile": + # TODO: What is the point of this transformation? raw = repr(value) - self.map_scenario_path = \ + scenario_path = \ raw.replace("\\", "/").replace("//", "/").replace("'", "") - self.game.map_file_path = "maps/{}.zip".format( - self.map_scenario_path.split("/")[2].lower() - ) + with contextlib.suppress(IndexError): + self.map_file_path = "maps/{}.zip".format( + scenario_path.split("/")[2].lower() + ) + map_info = self.game_service.map_info_cache.get(self.map_file_path) + if map_info is not None: + self.set_map_info(map_info) + else: + asyncio.create_task(self.update_map_info()) elif key == "Title": with contextlib.suppress(ValueError): self.name = value @@ -636,69 +698,6 @@ def clear_slot(self, slot_index): for item in to_remove: del self.AIs[item] - async def validate_game_settings(self): - """ - Mark the game invalid if it has non-compliant options - """ - - # Only allow ranked mods - for mod_id in self.mods.keys(): - if mod_id not in self.game_service.ranked_mods: - await self.mark_invalid(ValidityState.BAD_MOD) - return - - if self.has_ai: - await self.mark_invalid(ValidityState.HAS_AI_PLAYERS) - return - if self.is_multi_team: - await self.mark_invalid(ValidityState.MULTI_TEAM) - return - if self.is_ffa: - await self.mark_invalid(ValidityState.FFA_NOT_RANKED) - return - valid_options = { - "AIReplacement": (FA.DISABLED, ValidityState.HAS_AI_PLAYERS), - "FogOfWar": ("explored", ValidityState.NO_FOG_OF_WAR), - "CheatsEnabled": (FA.DISABLED, ValidityState.CHEATS_ENABLED), - "PrebuiltUnits": (FA.DISABLED, ValidityState.PREBUILT_ENABLED), - "NoRushOption": (FA.DISABLED, ValidityState.NORUSH_ENABLED), - "RestrictedCategories": (0, ValidityState.BAD_UNIT_RESTRICTIONS), - "TeamLock": ("locked", ValidityState.UNLOCKED_TEAMS), - "Unranked": (FA.DISABLED, ValidityState.HOST_SET_UNRANKED) - } - if await self._validate_game_options(valid_options) is False: - return - - await self.validate_game_mode_settings() - - async def validate_game_mode_settings(self): - """ - A subset of checks that need to be overridden in coop games. - """ - if None in self.teams or not self.is_even: - await self.mark_invalid(ValidityState.UNEVEN_TEAMS_NOT_RANKED) - return - - if len(self.players) < 2: - await self.mark_invalid(ValidityState.SINGLE_PLAYER) - return - - valid_options = { - "Victory": (Victory.DEMORALIZATION, ValidityState.WRONG_VICTORY_CONDITION) - } - await self._validate_game_options(valid_options) - - async def _validate_game_options( - self, valid_options: dict[str, tuple[Any, ValidityState]] - ) -> bool: - for key, value in self.game_options.items(): - if key in valid_options: - (valid_value, validity_state) = valid_options[key] - await self.mark_invalid(validity_state) - if valid_value != self.game_options[key]: - return False - return True - async def launch(self): """ Mark the game as live. @@ -718,44 +717,23 @@ async def launch(self): self.state = GameState.LIVE await self.on_game_launched() - await self.validate_game_settings() self._logger.info("Game launched") async def on_game_launched(self): for player in self.players: player.state = PlayerState.PLAYING + await self.update_map_info() await self.update_game_stats() await self.update_game_player_stats() async def update_game_stats(self): """ - Runs at game-start to populate the game_stats table (games that start are ones we actually - care about recording stats for, after all). + Runs at game-start to populate the game_stats table (games that start + are ones we actually care about recording stats for, after all). """ assert self.host is not None - async with self._db.acquire() as conn: - # Determine if the map is blacklisted, and invalidate the game for ranking purposes if - # so, and grab the map id at the same time. - result = await conn.execute( - "SELECT id, ranked FROM map_version " - "WHERE lower(filename) = lower(:filename)", - filename=self.map_file_path - ) - row = result.fetchone() - - is_generated = (self.map_file_path and "neroxis_map_generator" in self.map_file_path) - - if row: - self.map_id = row.id - - if ( - self.validity is ValidityState.VALID - and ((row and not row.ranked) or (not row and not is_generated)) - ): - await self.mark_invalid(ValidityState.BAD_MAP) - modId = self.game_service.featured_mods[self.game_mode].id # Write out the game_stats record. @@ -765,6 +743,10 @@ async def update_game_stats(self): game_type = str(self.get_game_option("Victory").value) async with self._db.acquire() as conn: + validity = self.get_validity() + if validity is not self.default_validity: + self._logger.info("Game is invalid at launch: %s", validity) + await conn.execute( game_stats.insert().values( id=self.id, @@ -773,9 +755,10 @@ async def update_game_stats(self): host=self.host.id, mapId=self.map_id, gameName=self.name, - validity=self.validity.value, + validity=validity.value, ) ) + self._persisted_validity = validity if self.matchmaker_queue_id is not None: await conn.execute( @@ -833,28 +816,6 @@ async def update_game_player_stats(self): ) raise - async def mark_invalid(self, new_validity_state: ValidityState): - self._logger.info( - "Marked as invalid because: %s", repr(new_validity_state) - ) - self.validity = new_validity_state - - # If we haven't started yet, the invalidity will be persisted to the database when we start. - # Otherwise, we have to do a special update query to write this information out. - if self.state is not GameState.LIVE: - return - - # Currently, we can only end up here if a game desynced or was a custom game that terminated - # too quickly. - async with self._db.acquire() as conn: - await conn.execute( - game_stats.update().where( - game_stats.c.id == self.id - ).values( - validity=new_validity_state.value - ) - ) - def get_army_score(self, army): return self._results.score(army) @@ -920,6 +881,10 @@ def to_dict(self): "state": client_state, "game_type": self.game_type.value, "featured_mod": self.game_mode, + "validity": [ + validity.name.lower() + for validity in self.validator.get_all(self) + ] or [self.default_validity.name.lower()], "sim_mods": self.mods, "mapname": self.map_folder_name, "map_file_path": self.map_file_path, @@ -951,19 +916,6 @@ def to_dict(self): } } - @property - def map_folder_name(self) -> str: - """ - Map folder name - """ - try: - return str(self.map_scenario_path.split("/")[2]).lower() - except (IndexError, AttributeError): - if self.map_file_path: - return self.map_file_path[5:-4].lower() - else: - return "scmp_009" - def __eq__(self, other): if not isinstance(other, Game): return False diff --git a/server/games/typedefs.py b/server/games/typedefs.py index 8d8142f7d..44f6c309e 100644 --- a/server/games/typedefs.py +++ b/server/games/typedefs.py @@ -41,9 +41,10 @@ class VisibilityState(Enum): FRIENDS = "friends" -# Identifiers must be kept in sync with the contents of the invalid_game_reasons table. -# New reasons added should have a description added to that table. Identifiers should never be -# reused, and values should never be deleted from invalid_game_reasons. +# Identifiers must be kept in sync with the contents of the invalid_game_reasons +# table. New reasons added should have a description added to that table. +# Identifiers should never be reused, and values should never be deleted from +# invalid_game_reasons. @unique class ValidityState(Enum): VALID = 0 diff --git a/server/games/validator.py b/server/games/validator.py new file mode 100644 index 000000000..2e31084d7 --- /dev/null +++ b/server/games/validator.py @@ -0,0 +1,105 @@ +import time +from typing import Any, Callable, Optional, Sequence + +from server.games.typedefs import FA, GameState, ValidityState, Victory + +ValidationRule = Callable[["Game"], Optional[ValidityState]] + + +class Validator: + def __init__(self, rules: Sequence[ValidationRule]): + self.rules = rules + + def get_one(self, game: "Game") -> Optional[ValidityState]: + for rule in self.rules: + validity = rule(game) + if validity is not None: + return validity + + def get_all(self, game: "Game") -> list[ValidityState]: + return [ + validity + for rule in self.rules + if (validity := rule(game)) is not None + ] + + +class GameOptionRule: + def __init__(self, key: str, value: Any, validity: ValidityState): + self.key = key + self.value = value + self.validity = validity + + def __call__(self, game: "Game") -> Optional[ValidityState]: + if game.game_options[self.key] != self.value: + return self.validity + + +class PropertyRule: + def __init__(self, name: str, value: Any, validity: ValidityState): + self.name = name + self.value = value + self.validity = validity + + def __call__(self, game: "Game") -> Optional["ValidityState"]: + if getattr(game, self.name) != self.value: + return self.validity + + +def not_desynced_rule(game: "Game") -> Optional[ValidityState]: + if game.desyncs > 20: + return ValidityState.TOO_MANY_DESYNCS + + +def has_results_rule(game: "Game") -> Optional[ValidityState]: + if game.state is GameState.ENDED and not game._results: + return ValidityState.UNKNOWN_RESULT + + +def ranked_mods_rule(game: "Game") -> Optional[ValidityState]: + for mod_id in game.mods.keys(): + if mod_id not in game.game_service.ranked_mods: + return ValidityState.BAD_MOD + + +def ranked_map_rule(game: "Game") -> Optional[ValidityState]: + if game.map_id is not None and not game.map_ranked: + return ValidityState.BAD_MAP + if game.map_id is None and not game.is_map_generated and not game.is_coop: + return ValidityState.BAD_MAP + + +def even_teams_rule(game: "Game") -> Optional[ValidityState]: + if None in game.teams or not game.is_even: + return ValidityState.UNEVEN_TEAMS_NOT_RANKED + + +def multi_player_rule(game: "Game") -> Optional[ValidityState]: + if len(game.players) < 2: + return ValidityState.SINGLE_PLAYER + + +# Rules that apply for all games +COMMON_RULES = ( + not_desynced_rule, + ranked_mods_rule, + ranked_map_rule, + PropertyRule("has_ai", False, ValidityState.HAS_AI_PLAYERS), + PropertyRule("is_multi_team", False, ValidityState.MULTI_TEAM), + PropertyRule("is_ffa", False, ValidityState.FFA_NOT_RANKED), + GameOptionRule("AIReplacement", FA.DISABLED, ValidityState.HAS_AI_PLAYERS), + GameOptionRule("FogOfWar", "explored", ValidityState.NO_FOG_OF_WAR), + GameOptionRule("CheatsEnabled", FA.DISABLED, ValidityState.CHEATS_ENABLED), + GameOptionRule("PrebuiltUnits", FA.DISABLED, ValidityState.PREBUILT_ENABLED), + GameOptionRule("NoRushOption", FA.DISABLED, ValidityState.NORUSH_ENABLED), + GameOptionRule("RestrictedCategories", 0, ValidityState.BAD_UNIT_RESTRICTIONS), + GameOptionRule("TeamLock", "locked", ValidityState.UNLOCKED_TEAMS), + GameOptionRule("Unranked", FA.DISABLED, ValidityState.HOST_SET_UNRANKED) +) +# Rules that apply for everything but coop +NON_COOP_RULES = ( + has_results_rule, + even_teams_rule, + multi_player_rule, + GameOptionRule("Victory", Victory.DEMORALIZATION, ValidityState.WRONG_VICTORY_CONDITION) +) diff --git a/server/ladder_service/ladder_service.py b/server/ladder_service/ladder_service.py index ba6a680fe..8cd50a166 100644 --- a/server/ladder_service/ladder_service.py +++ b/server/ladder_service/ladder_service.py @@ -4,7 +4,6 @@ import asyncio import json import random -import re import statistics from collections import defaultdict from typing import Awaitable, Callable, Optional @@ -561,15 +560,11 @@ def get_displayed_rating(player: Player) -> float: if game_options: game.game_options.update(game_options) - mapname = re.match("maps/(.+).zip", map_path).group(1) - # FIXME: Database filenames contain the maps/ prefix and .zip suffix. - # Really in the future, just send a better description - self._logger.debug("Starting ladder game: %s", game) def make_game_options(player: Player) -> GameLaunchOptions: return GameLaunchOptions( - mapname=mapname, + mapname=game.map_folder_name, expected_players=len(all_players), game_options=game_options, team=game.get_player_option(player.id, "Team"), diff --git a/server/types.py b/server/types.py index 2f4e1f243..e4abfe19e 100644 --- a/server/types.py +++ b/server/types.py @@ -30,6 +30,14 @@ class GameLaunchOptions(NamedTuple): game_options: Optional[dict[str, Any]] = None +class MapInfo(NamedTuple): + # map_version.id + id: int + filename: str + ranked: bool + + +# The below classes are used for choosing maps from a map pool class MapPoolMap(Protocol): id: int weight: int @@ -38,6 +46,7 @@ def get_map(self) -> "Map": ... class Map(NamedTuple): + # map_version.id id: int name: str path: str diff --git a/tests/conftest.py b/tests/conftest.py index 8d550740e..6a33e2313 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -226,7 +226,7 @@ async def ugame(database, players): async def coop_game(database, players): global COOP_GAME_UID game = make_game(database, COOP_GAME_UID, players, game_type=CoopGame) - game.validity = ValidityState.COOP_NOT_RANKED + game.get_validity.return_value = ValidityState.COOP_NOT_RANKED game.leaderboard_saved = False COOP_GAME_UID += 1 return game diff --git a/tests/unit_tests/test_custom_game.py b/tests/unit_tests/test_custom_game.py index f8e88183a..1148dd54b 100644 --- a/tests/unit_tests/test_custom_game.py +++ b/tests/unit_tests/test_custom_game.py @@ -28,7 +28,7 @@ async def test_rate_game_early_abort_no_enforce( custom_game.launched_at = time.time() - 60 # seconds await custom_game.on_game_finish() - assert custom_game.validity == ValidityState.TOO_SHORT + assert custom_game.get_validity() == ValidityState.TOO_SHORT async def test_rate_game_early_abort_with_enforce( @@ -48,7 +48,7 @@ async def test_rate_game_early_abort_with_enforce( custom_game.launched_at = time.time() - 60 # seconds await custom_game.on_game_finish() - assert custom_game.validity == ValidityState.VALID + assert custom_game.get_validity() == ValidityState.VALID async def test_rate_game_late_abort_no_enforce( @@ -67,7 +67,7 @@ async def test_rate_game_late_abort_no_enforce( custom_game.launched_at = time.time() - 600 # seconds await custom_game.on_game_finish() - assert custom_game.validity == ValidityState.VALID + assert custom_game.get_validity() == ValidityState.VALID async def test_global_rating_higher_after_custom_game_win( @@ -88,5 +88,5 @@ async def test_global_rating_higher_after_custom_game_win( # await game being rated await rating_service._join_rating_queue() - assert game.validity is ValidityState.VALID + assert game.get_validity() is ValidityState.VALID assert players[0].ratings[RatingType.GLOBAL][0] > old_mean diff --git a/tests/unit_tests/test_game.py b/tests/unit_tests/test_game.py index efdd84487..5f56605b0 100644 --- a/tests/unit_tests/test_game.py +++ b/tests/unit_tests/test_game.py @@ -32,7 +32,16 @@ @pytest.fixture async def game(database, game_service, game_stats_service): - return Game(42, database, game_service, game_stats_service, rating_type=RatingType.GLOBAL) + game = Game( + 42, + database, + game_service, + game_stats_service, + rating_type=RatingType.GLOBAL + ) + await game.update_map_info() + + return game @pytest.fixture @@ -67,7 +76,7 @@ async def test_instance_logging(database, game_stats_service): logger.debug.assert_called_with("%s created", game) -async def test_validate_game_settings(game: Game, game_add_players): +async def test_get_validity(game: Game, game_add_players): settings = [ ("Victory", Victory.SANDBOX, ValidityState.WRONG_VICTORY_CONDITION), ("FogOfWar", "none", ValidityState.NO_FOG_OF_WAR), @@ -96,18 +105,15 @@ async def test_validate_game_settings(game: Game, game_add_players): await check_game_settings(game, settings) - game.validity = ValidityState.VALID - await game.validate_game_settings() - assert game.validity is ValidityState.VALID + assert game.get_validity() is ValidityState.VALID for player in game.players: game.set_player_option(player.id, "Team", 2) - await game.validate_game_settings() - assert game.validity is ValidityState.UNEVEN_TEAMS_NOT_RANKED + assert game.get_validity() is ValidityState.UNEVEN_TEAMS_NOT_RANKED -async def test_validate_game_settings_coop(coop_game: Game): +async def test_get_validity_coop(coop_game: Game): settings = [ ( "Victory", Victory.DEMORALIZATION, @@ -121,9 +127,7 @@ async def test_validate_game_settings_coop(coop_game: Game): await check_game_settings(coop_game, settings) - coop_game.validity = ValidityState.VALID - await coop_game.validate_game_settings() - assert coop_game.validity is ValidityState.VALID + assert coop_game.get_validity() is ValidityState.COOP_NOT_RANKED async def test_missing_teams_marked_invalid(game: Game, game_add_players): @@ -132,9 +136,7 @@ async def test_missing_teams_marked_invalid(game: Game, game_add_players): game_add_players(game, player_id, team=2) del game._player_options[player_id]["Team"] - await game.validate_game_settings() - - assert game.validity is ValidityState.UNEVEN_TEAMS_NOT_RANKED + assert game.get_validity() is ValidityState.UNEVEN_TEAMS_NOT_RANKED async def check_game_settings( @@ -143,8 +145,7 @@ async def check_game_settings( for key, value, expected in settings: old = game.game_options.get(key) game.game_options[key] = value - await game.validate_game_settings() - assert game.validity is expected + assert game.get_validity() is expected game.game_options[key] = old @@ -163,7 +164,7 @@ async def test_ffa_not_rated(game, game_add_players): await game.add_result(0, 1, "victory", 5) game.launched_at = time.time() - 60 * 20 # seconds await game.on_game_finish() - assert game.validity == ValidityState.FFA_NOT_RANKED + assert game.get_validity() is ValidityState.FFA_NOT_RANKED async def test_generated_map_is_rated(game, game_add_players): @@ -174,7 +175,7 @@ async def test_generated_map_is_rated(game, game_add_players): await game.add_result(0, 1, "victory", 5) game.launched_at = time.time() - 60 * 20 # seconds await game.on_game_finish() - assert game.validity == ValidityState.VALID + assert game.get_validity() is ValidityState.VALID async def test_unranked_generated_map_not_rated(game, game_add_players): @@ -185,7 +186,7 @@ async def test_unranked_generated_map_not_rated(game, game_add_players): await game.add_result(0, 1, "victory", 5) game.launched_at = time.time() - 60 * 20 # seconds await game.on_game_finish() - assert game.validity == ValidityState.BAD_MAP + assert game.get_validity() is ValidityState.BAD_MAP async def test_two_player_ffa_is_rated(game, game_add_players): @@ -195,7 +196,7 @@ async def test_two_player_ffa_is_rated(game, game_add_players): await game.add_result(0, 1, "victory", 5) game.launched_at = time.time() - 60 * 20 # seconds await game.on_game_finish() - assert game.validity == ValidityState.VALID + assert game.get_validity() is ValidityState.VALID async def test_multi_team_not_rated(game, game_add_players): @@ -207,7 +208,7 @@ async def test_multi_team_not_rated(game, game_add_players): await game.add_result(0, 1, "victory", 5) game.launched_at = time.time() - 60 * 20 # seconds await game.on_game_finish() - assert game.validity == ValidityState.MULTI_TEAM + assert game.get_validity() is ValidityState.MULTI_TEAM async def test_has_ai_players_not_rated(game, game_add_players): @@ -229,7 +230,7 @@ async def test_has_ai_players_not_rated(game, game_add_players): await game.add_result(0, 1, "victory", 5) game.launched_at = time.time() - 60 * 20 # seconds await game.on_game_finish() - assert game.validity == ValidityState.HAS_AI_PLAYERS + assert game.get_validity() is ValidityState.HAS_AI_PLAYERS async def test_uneven_teams_not_rated(game, game_add_players): @@ -240,7 +241,7 @@ async def test_uneven_teams_not_rated(game, game_add_players): await game.add_result(0, 1, "victory", 5) game.launched_at = time.time() - 60 * 20 # seconds await game.on_game_finish() - assert game.validity == ValidityState.UNEVEN_TEAMS_NOT_RANKED + assert game.get_validity() is ValidityState.UNEVEN_TEAMS_NOT_RANKED async def test_single_team_not_rated(game, game_add_players): @@ -252,7 +253,7 @@ async def test_single_team_not_rated(game, game_add_players): for i in range(n_players): await game.add_result(0, i + 1, "victory", 5) await game.on_game_finish() - assert game.validity is ValidityState.UNEVEN_TEAMS_NOT_RANKED + assert game.get_validity() is ValidityState.UNEVEN_TEAMS_NOT_RANKED async def test_game_visible_to_host(game: Game, players): @@ -324,6 +325,32 @@ async def test_invalid_get_player_option_key(game: Game, players): assert game.get_player_option(players.hosting.id, -1) is None +def test_set_game_option(game: Game): + game.game_options = {"AIReplacement": "Off"} + game.set_game_option("Victory", "sandbox") + assert game.game_options["Victory"] == Victory.SANDBOX + game.set_game_option("AIReplacement", "On") + assert game.game_options["AIReplacement"] == "On" + game.set_game_option("Slots", "7") + assert game.max_players == 7 + game.set_game_option("Title", "All welcome") + assert game.name == "All welcome" + game.set_game_option("ArbitraryKey", "ArbitraryValue") + assert game.game_options["ArbitraryKey"] == "ArbitraryValue" + + +async def test_set_game_option_scenario_file(game: Game): + # Valid example from a replay + game.set_game_option("ScenarioFile", "/maps/scmp_009/scmp_009_scenario.lua") + assert game.map_file_path == "maps/scmp_009.zip" + + # Examples that document behavior but might not make sense or be necessary + game.set_game_option("ScenarioFile", "C:\\Maps\\Some_Map") + assert game.map_file_path == "maps/some_map.zip" + game.set_game_option("ScenarioFile", "/maps/'some map'/scenario.lua") + assert game.map_file_path == "maps/some map.zip" + + async def test_add_game_connection(game: Game, players, mock_game_connection): game.state = GameState.LOBBY mock_game_connection.player = players.hosting @@ -509,7 +536,7 @@ async def test_game_ends_in_mutually_agreed_draw(game: Game, game_add_players): await game.add_result(players[1].id, 1, "mutual_draw", 0) await game.on_game_finish() - assert game.validity is ValidityState.VALID + assert game.get_validity() is ValidityState.VALID async def test_game_not_ends_in_unilatery_agreed_draw( @@ -525,7 +552,7 @@ async def test_game_not_ends_in_unilatery_agreed_draw( await game.add_result(players.joining.id, 1, "victory", 10) await game.on_game_finish() - assert game.validity is not ValidityState.MUTUAL_DRAW + assert game.get_validity() is not ValidityState.MUTUAL_DRAW async def test_game_is_invalid_due_to_desyncs(game: Game, players): @@ -537,7 +564,7 @@ async def test_game_is_invalid_due_to_desyncs(game: Game, players): game.desyncs = 30 await game.on_game_finish() - assert game.validity is ValidityState.TOO_MANY_DESYNCS + assert game.get_validity() is ValidityState.TOO_MANY_DESYNCS async def test_game_get_player_outcome_ignores_unknown_results( @@ -560,7 +587,7 @@ async def test_on_game_end_single_player_gives_unknown_result(game): await game.on_game_finish() assert game.state is GameState.ENDED - assert game.validity is ValidityState.UNKNOWN_RESULT + assert game.get_validity() is ValidityState.UNKNOWN_RESULT async def test_on_game_end_two_players_is_valid( @@ -577,7 +604,7 @@ async def test_on_game_end_two_players_is_valid( await game.on_game_finish() assert game.state is GameState.ENDED - assert game.validity is ValidityState.VALID + assert game.get_validity() is ValidityState.VALID async def test_name_sanitization(game, players): @@ -597,39 +624,41 @@ async def test_to_dict(game, player_factory): players = [ (player_factory(f"{i}", player_id=i, global_rating=rating), result, team) for i, (rating, result, team) in enumerate([ - (Rating(1500, 250), 0, 1), - (Rating(1700, 120), 0, 1), - (Rating(1200, 72), 0, 2), - (Rating(1200, 72), 0, 2), - ], 1)] + (Rating(1500, 250), 0, 2), + (Rating(1700, 120), 0, 2), + (Rating(1200, 72), 0, 3), + (Rating(1200, 72), 0, 3), + ], 1) + ] add_connected_players(game, [player for player, _, _ in players]) for player, _, team in players: game.set_player_option(player.id, "Team", team) - game.set_player_option(player.id, "Army", player.id - 1) + game.set_player_option(player.id, "Army", player.id) game.host = players[0][0] await game.launch() data = game.to_dict() expected = { "command": "game_info", - "visibility": game.visibility.value, - "password_protected": game.password is not None, - "uid": game.id, - "title": game.name, + "visibility": "public", + "password_protected": False, + "uid": 42, + "title": "None", "game_type": "custom", "state": "playing", - "featured_mod": game.game_mode, - "sim_mods": game.mods, - "mapname": game.map_folder_name, - "map_file_path": game.map_file_path, - "host": game.host.login, - "num_players": len(game.players), - "max_players": game.max_players, + "featured_mod": "faf", + "validity": ["valid"], + "sim_mods": {}, + "mapname": "SCMP_007", + "map_file_path": "maps/SCMP_007.zip", + "host": "1", + "num_players": 4, + "max_players": 12, "hosted_at": None, "launched_at": game.launched_at, - "rating_type": game.rating_type, - "rating_min": game.displayed_rating_range.lo, - "rating_max": game.displayed_rating_range.hi, - "enforce_rating_range": game.enforce_rating_range, + "rating_type": "global", + "rating_min": None, + "rating_max": None, + "enforce_rating_range": False, "teams_ids": [ { "team_id": team, @@ -683,7 +712,7 @@ async def test_persist_results_not_called_with_no_results( assert len(game.players) == 4 assert len(game._results) == 0 - assert game.validity is ValidityState.UNKNOWN_RESULT + assert game.get_validity() is ValidityState.UNKNOWN_RESULT game.persist_results.assert_not_called() @@ -711,8 +740,10 @@ async def test_persist_results_called_for_unranked(game, game_add_players): game.state = GameState.LOBBY game_add_players(game, 2) await game.launch() - game.validity = ValidityState.BAD_UNIT_RESTRICTIONS + game.game_options["RestrictedCategories"] = 1 + assert game.get_validity() is ValidityState.BAD_UNIT_RESTRICTIONS assert len(game.players) == 2 + await game.add_result(0, 1, "victory", 5) await game.on_game_finish() @@ -824,7 +855,7 @@ async def test_partial_stats_not_affecting_rating_persistence( # await game being rated await rating_service._join_rating_queue() - assert game.validity is ValidityState.VALID + assert game.get_validity() is ValidityState.VALID assert players[0].ratings[RatingType.GLOBAL][0] > old_mean @@ -1043,8 +1074,10 @@ async def test_army_results_present_for_invalid_games(game: Game, game_add_playe game.state = GameState.LOBBY players = (*game_add_players(game, 2, 2), *game_add_players(game, 2, 3)) + game.game_options["CheatsEnabled"] = "On" + assert game.get_validity() is ValidityState.CHEATS_ENABLED + await game.launch() - game.validity = ValidityState.CHEATS_ENABLED for i, player in enumerate(players): for _ in players: diff --git a/tests/unit_tests/test_gameconnection.py b/tests/unit_tests/test_gameconnection.py index 06c9ac728..63e468f96 100644 --- a/tests/unit_tests/test_gameconnection.py +++ b/tests/unit_tests/test_gameconnection.py @@ -402,35 +402,20 @@ async def test_cannot_parse_game_results( assert "Invalid result" in caplog.messages[0] -async def test_handle_action_GameOption( - game: Game, - game_connection: GameConnection -): - game.game_options = {"AIReplacement": "Off"} +# TODO: Convert this to test for game.set_game_option +async def test_handle_action_GameOption(game, game_connection: GameConnection): await game_connection.handle_action("GameOption", ["Victory", "sandbox"]) - assert game.game_options["Victory"] == Victory.SANDBOX - await game_connection.handle_action("GameOption", ["AIReplacement", "On"]) - assert game.game_options["AIReplacement"] == "On" - await game_connection.handle_action("GameOption", ["Slots", "7"]) - assert game.max_players == 7 - # I don't know what these paths actually look like - await game_connection.handle_action("GameOption", ["ScenarioFile", "C:\\Maps\\Some_Map"]) - assert game.map_file_path == "maps/some_map.zip" - await game_connection.handle_action("GameOption", ["Title", "All welcome"]) - assert game.name == "All welcome" - await game_connection.handle_action("GameOption", ["ArbitraryKey", "ArbitraryValue"]) - assert game.game_options["ArbitraryKey"] == "ArbitraryValue" + game.set_game_option.assert_called_once_with("Victory", "sandbox") async def test_handle_action_GameOption_not_host( - game: Game, + game, game_connection: GameConnection, players ): game_connection.player = players.joining - game.game_options = {"Victory": "asdf"} await game_connection.handle_action("GameOption", ["Victory", "sandbox"]) - assert game.game_options == {"Victory": "asdf"} + game.set_game_option.assert_not_called() async def test_json_stats( @@ -596,7 +581,7 @@ async def test_handle_action_OperationComplete_invalid( coop_game: CoopGame, game_connection: GameConnection, database ): coop_game.map_file_path = "maps/prothyon16.v0005.zip" - coop_game.validity = ValidityState.OTHER_UNRANK + coop_game.get_validity.return_value = ValidityState.OTHER_UNRANK game_connection.game = coop_game time_taken = "09:08:07.654321" diff --git a/tests/unit_tests/test_laddergame.py b/tests/unit_tests/test_laddergame.py index 424060dc8..36755e155 100644 --- a/tests/unit_tests/test_laddergame.py +++ b/tests/unit_tests/test_laddergame.py @@ -128,7 +128,7 @@ async def test_rate_game(laddergame: LadderGame, database, game_add_players): await laddergame.game_service._rating_service._join_rating_queue() - assert laddergame.validity is ValidityState.VALID + assert laddergame.get_validity() is ValidityState.VALID assert players[0].ratings[RatingType.LADDER_1V1][0] > player_1_old_mean assert players[1].ratings[RatingType.LADDER_1V1][0] < player_2_old_mean @@ -179,7 +179,7 @@ async def test_persist_rating_victory(laddergame: LadderGame, database, await laddergame.game_service._rating_service._join_rating_queue() - assert laddergame.validity is ValidityState.VALID + assert laddergame.get_validity() is ValidityState.VALID async with database.acquire() as conn: result_after = list(await conn.execute(str(compiled))) From 80ceb2c1bd5253cb74856728594fa99bded6f33c Mon Sep 17 00:00:00 2001 From: Askaholic Date: Sun, 3 Apr 2022 20:41:25 -0800 Subject: [PATCH 3/7] Add test and refactor mark_dirty --- server/game_service.py | 3 +- server/gameconnection.py | 6 +-- server/games/game.py | 7 ++- server/ladder_service/ladder_service.py | 2 +- server/lobbyconnection.py | 7 +-- tests/integration_tests/test_game.py | 51 ++++++++++++++++++++++ tests/integration_tests/test_matchmaker.py | 16 +++---- tests/integration_tests/test_server.py | 9 ++-- tests/unit_tests/test_games_service.py | 14 +++--- tests/unit_tests/test_lobbyconnection.py | 1 - 10 files changed, 81 insertions(+), 35 deletions(-) diff --git a/server/game_service.py b/server/game_service.py index 64751d2a3..9dac52c97 100644 --- a/server/game_service.py +++ b/server/game_service.py @@ -172,7 +172,7 @@ def create_uid(self) -> int: return self.game_id_counter - def create_game( + async def create_game( self, game_mode: str, game_class: type[Game] = CustomGame, @@ -204,6 +204,7 @@ def create_game( } game_args.update(kwargs) game = game_class(**game_args) + game.set_map_info(await self.get_map_info(game.map_file_path)) self._games[game_id] = game diff --git a/server/gameconnection.py b/server/gameconnection.py index 412f4ffbd..aeabed35c 100644 --- a/server/gameconnection.py +++ b/server/gameconnection.py @@ -228,7 +228,7 @@ async def handle_game_option(self, key: str, value: Any): return self.game.set_game_option(key, value) - self._mark_dirty() + # Game object handles marking as dirty async def handle_game_mods(self, mode: Any, args: list[Any]): if not self.is_host(): @@ -453,10 +453,6 @@ async def handle_game_state(self, state: str): return elif state == "Lobby": - # TODO: Do we still need to schedule with `ensure_future`? - # - # We do not yield from the task, since we - # need to keep processing other commands while it runs await self._handle_lobby_state() elif state == "Launching": diff --git a/server/games/game.py b/server/games/game.py index a90d0b777..3797a66b4 100644 --- a/server/games/game.py +++ b/server/games/game.py @@ -318,6 +318,7 @@ def set_map_info(self, map_info: Optional[MapInfo]): else: self.map_id = None self.map_ranked = False + self.mark_dirty() async def add_result( self, @@ -502,7 +503,7 @@ async def on_game_finish(self): finally: self.state = GameState.ENDED - self.game_service.mark_dirty(self) + self.mark_dirty() async def process_game_results(self): if not self._results: @@ -655,6 +656,7 @@ def set_game_option(self, key: str, value: Any): elif key == "Title": with contextlib.suppress(ValueError): self.name = value + self.mark_dirty() def get_player_option(self, player_id: int, key: str) -> Optional[Any]: """ @@ -839,6 +841,9 @@ def report_army_stats(self, stats_json): self._army_stats_list = json.loads(stats_json)["stats"] self._process_pending_army_stats() + def mark_dirty(self): + self.game_service.mark_dirty(self) + def is_visible_to_player(self, player: Player) -> bool: """ Determine if a player should see this game in their games list. diff --git a/server/ladder_service/ladder_service.py b/server/ladder_service/ladder_service.py index 8cd50a166..cd1cf7559 100644 --- a/server/ladder_service/ladder_service.py +++ b/server/ladder_service/ladder_service.py @@ -519,7 +519,7 @@ def get_displayed_rating(player: Player) -> float: raise RuntimeError(f"No map pool available for rating {rating}!") _, _, map_path, _ = pool.choose_map(played_map_ids) - game = self.game_service.create_game( + game = await self.game_service.create_game( game_class=LadderGame, game_mode=queue.featured_mod, host=host, diff --git a/server/lobbyconnection.py b/server/lobbyconnection.py index 245ed204e..82370b648 100644 --- a/server/lobbyconnection.py +++ b/server/lobbyconnection.py @@ -964,7 +964,7 @@ async def command_game_host(self, message): game_class = CoopGame if game_mode == FeaturedModType.COOP else CustomGame - game = self.game_service.create_game( + game = await self.game_service.create_game( visibility=visibility, game_mode=game_mode, game_class=game_class, @@ -1048,11 +1048,6 @@ def _prepare_launch_game( "args": ["/numgames", self.player.game_count[game.rating_type]], "uid": game.id, "mod": game.game_mode, - # Following parameters may not be used by the client yet. They are - # needed for setting up auto-lobby style matches such as ladder, gw, - # and team machmaking where the server decides what these game - # options are. Currently, options for ladder are hardcoded into the - # client. "name": game.name, # DEPRICATED: init_mode can be inferred from game_type "init_mode": game.init_mode.value, diff --git a/tests/integration_tests/test_game.py b/tests/integration_tests/test_game.py index 8ca9f39e7..b9c72fc71 100644 --- a/tests/integration_tests/test_game.py +++ b/tests/integration_tests/test_game.py @@ -264,6 +264,57 @@ async def send_player_options(proto, *options): }) +@fast_forward(20) +async def test_game_validity_states(lobby_server): + host_id, _, host_proto = await connect_and_sign_in( + ("test", "test_password"), lobby_server + ) + guest_id, _, guest_proto = await connect_and_sign_in( + ("Rhiza", "puff_the_magic_dragon"), lobby_server + ) + # Set up the game + game_id = await host_game(host_proto) + msg = await read_until_command(host_proto, "game_info", uid=game_id, timeout=5) + assert msg["validity"] == ["single_player"] + + # The host configures themselves, causing them to show up as connected + await send_player_options(host_proto, [host_id, "Team", 1]) + msg = await read_until_command(host_proto, "game_info", uid=game_id, timeout=5) + assert msg["validity"] == ["uneven_teams_not_ranked", "single_player"] + + # Change the map to an unranked map + await host_proto.send_message({ + "target": "game", + "command": "GameOption", + "args": [ + "ScenarioFile", + "/maps/neroxis_map_generator_sneaky_map/sneaky_map_scenario.lua" + ] + }) + msg = await read_until_command(host_proto, "game_info", uid=game_id, timeout=5) + assert msg["validity"] == [ + "bad_map", + "uneven_teams_not_ranked", + "single_player" + ] + + # Another player joins + await join_game(guest_proto, game_id) + await send_player_options(host_proto, [guest_id, "Team", 1]) + msg = await read_until_command(host_proto, "game_info", uid=game_id, timeout=5) + assert msg["validity"] == ["bad_map"] + + # Change the map to a ranked map + await host_proto.send_message({ + "target": "game", + "command": "GameOption", + "args": ["ScenarioFile", "/maps/scmp_001/scmp_001_scenario.lua"] + }) + + msg = await read_until_command(host_proto, "game_info", uid=game_id, timeout=5) + assert msg["validity"] == ["valid"] + + @fast_forward(60) async def test_game_info_messages(lobby_server): host_id, _, host_proto = await connect_and_sign_in( diff --git a/tests/integration_tests/test_matchmaker.py b/tests/integration_tests/test_matchmaker.py index 30ba1cbd9..8017edb2e 100644 --- a/tests/integration_tests/test_matchmaker.py +++ b/tests/integration_tests/test_matchmaker.py @@ -100,7 +100,7 @@ async def test_game_launch_message_game_options(lobby_server, tmp_user): @pytest.mark.flaky -@fast_forward(15) +@fast_forward(60) async def test_game_matchmaking_start(lobby_server, database): host_id, host, guest_id, guest = await queue_players_for_matchmaking(lobby_server) @@ -120,8 +120,6 @@ async def test_game_matchmaking_start(lobby_server, database): await read_until_command(guest, "game_launch") await open_fa(guest) - await read_until_command(host, "game_info") - await read_until_command(guest, "game_info") await send_player_options( host, (guest_id, "StartSpot", msg["map_position"]), @@ -165,7 +163,7 @@ async def test_game_matchmaking_start(lobby_server, database): assert row.technical_name == "ladder1v1" -@fast_forward(15) +@fast_forward(60) async def test_game_matchmaking_start_while_matched(lobby_server): _, proto1, _, _ = await queue_players_for_matchmaking(lobby_server) @@ -278,7 +276,7 @@ async def test_game_matchmaking_timeout_guest(lobby_server, game_service): await read_until_command(proto2, "search_info", state="start", timeout=5) -@fast_forward(15) +@fast_forward(60) async def test_game_matchmaking_cancel(lobby_server): _, proto = await queue_player_for_matchmaking( ("ladder1", "ladder1"), @@ -422,7 +420,7 @@ async def test_anti_map_repetition(lobby_server): ) -@fast_forward(10) +@fast_forward(60) async def test_matchmaker_info_message(lobby_server, mocker): mocker.patch("server.matchmaker.pop_timer.time", return_value=1_562_000_000) mocker.patch( @@ -453,7 +451,7 @@ async def test_matchmaker_info_message(lobby_server, mocker): assert queue["boundary_75s"] == [] -@fast_forward(10) +@fast_forward(60) async def test_command_matchmaker_info(lobby_server, mocker): mocker.patch("server.matchmaker.pop_timer.time", return_value=1_562_000_000) mocker.patch( @@ -487,7 +485,7 @@ async def test_command_matchmaker_info(lobby_server, mocker): assert queue["boundary_75s"] == [] -@fast_forward(10) +@fast_forward(60) async def test_matchmaker_info_message_on_cancel(lobby_server): _, _, proto = await connect_and_sign_in( ("ladder1", "ladder1"), @@ -532,7 +530,7 @@ async def read_update_msg(): assert len(queue_message["boundary_80s"]) == 0 -@fast_forward(10) +@fast_forward(60) async def test_search_info_messages(lobby_server): _, _, proto = await connect_and_sign_in( ("ladder1", "ladder1"), diff --git a/tests/integration_tests/test_server.py b/tests/integration_tests/test_server.py index 6c8f7ba01..50d5c04ff 100644 --- a/tests/integration_tests/test_server.py +++ b/tests/integration_tests/test_server.py @@ -565,10 +565,11 @@ async def test_game_info_broadcast_to_players_in_lobby(lobby_server): game_id = msg["uid"] await join_game(proto2, game_id) - - await read_until_command(proto1, "game_info", teams={"1": ["friends"]}) - await read_until_command(proto2, "game_info", teams={"1": ["friends"]}) - await send_player_options(proto1, [test_id, "Army", 1], [test_id, "Team", 1]) + await send_player_options( + proto1, + [test_id, "Army", 1], + [test_id, "Team", 1] + ) await read_until_command(proto1, "game_info", teams={"1": ["friends", "test"]}) await read_until_command(proto2, "game_info", teams={"1": ["friends", "test"]}) diff --git a/tests/unit_tests/test_games_service.py b/tests/unit_tests/test_games_service.py index 55fd74674..e92ff818b 100644 --- a/tests/unit_tests/test_games_service.py +++ b/tests/unit_tests/test_games_service.py @@ -35,14 +35,14 @@ async def test_graceful_shutdown(game_service): await game_service.graceful_shutdown() with pytest.raises(DisabledError): - game_service.create_game( + await game_service.create_game( game_mode="faf", ) @fast_forward(2) async def test_drain_games(game_service): - game = game_service.create_game( + game = await game_service.create_game( game_mode="faf", name="TestGame" ) @@ -61,7 +61,7 @@ async def test_drain_games(game_service): async def test_create_game(players, game_service): players.hosting.state = PlayerState.IDLE - game = game_service.create_game( + game = await game_service.create_game( visibility=VisibilityState.PUBLIC, game_mode="faf", host=players.hosting, @@ -78,7 +78,7 @@ async def test_create_game(players, game_service): async def test_all_games(players, game_service): - game = game_service.create_game( + game = await game_service.create_game( visibility=VisibilityState.PUBLIC, game_mode="faf", host=players.hosting, @@ -92,7 +92,7 @@ async def test_all_games(players, game_service): async def test_create_game_ladder1v1(players, game_service): - game = game_service.create_game( + game = await game_service.create_game( game_mode="ladder1v1", game_class=LadderGame, host=players.hosting, @@ -105,7 +105,7 @@ async def test_create_game_ladder1v1(players, game_service): async def test_create_game_other_gamemode(players, game_service): - game = game_service.create_game( + game = await game_service.create_game( visibility=VisibilityState.PUBLIC, game_mode="labwars", host=players.hosting, @@ -120,7 +120,7 @@ async def test_create_game_other_gamemode(players, game_service): async def test_close_lobby_games(players, game_service): - game = game_service.create_game( + game = await game_service.create_game( visibility=VisibilityState.PUBLIC, game_mode="faf", host=players.hosting, diff --git a/tests/unit_tests/test_lobbyconnection.py b/tests/unit_tests/test_lobbyconnection.py index 3714dd75b..69648835c 100644 --- a/tests/unit_tests/test_lobbyconnection.py +++ b/tests/unit_tests/test_lobbyconnection.py @@ -457,7 +457,6 @@ async def test_command_game_host_calls_host_game_invalid_title( lobbyconnection, mock_games, test_game_info_invalid ): lobbyconnection.send = mock.AsyncMock() - mock_games.create_game = mock.Mock() await lobbyconnection.on_message_received({ "command": "game_host", **test_game_info_invalid From 4caeb69f890f214604180519e454f53b4aeae88f Mon Sep 17 00:00:00 2001 From: Askaholic Date: Sat, 9 Apr 2022 15:49:32 -0800 Subject: [PATCH 4/7] Move game option logic back to GameConnection --- server/gameconnection.py | 26 ++++++++++++++++++--- server/games/game.py | 30 ------------------------- tests/unit_tests/test_game.py | 26 --------------------- tests/unit_tests/test_gameconnection.py | 29 ++++++++++++++++++++++-- 4 files changed, 50 insertions(+), 61 deletions(-) diff --git a/server/gameconnection.py b/server/gameconnection.py index aeabed35c..3e6a5c3b3 100644 --- a/server/gameconnection.py +++ b/server/gameconnection.py @@ -23,7 +23,7 @@ GameState, ValidityState ) -from .games.typedefs import FA +from .games.typedefs import FA, Victory from .player_service import PlayerService from .players import Player, PlayerState from .protocol import DisconnectedError, GpgNetServerProtocol, Protocol @@ -227,8 +227,28 @@ async def handle_game_option(self, key: str, value: Any): if not self.is_host(): return - self.game.set_game_option(key, value) - # Game object handles marking as dirty + # Type transformations + if key == "Victory": + value = Victory.__members__.get(value.upper()) + elif key == "Slots": + value = int(value) + + self.game.game_options[key] = value + + # Additional attributes + if key == "ScenarioFile": + # TODO: What is the point of this transformation? + raw = repr(value) + scenario_path = \ + raw.replace("\\", "/").replace("//", "/").replace("'", "") + with contextlib.suppress(IndexError): + map_name = scenario_path.split("/")[2].lower() + self.game.map_file_path = f"maps/{map_name}.zip" + await self.game.update_map_info() + elif key == "Title": + with contextlib.suppress(ValueError): + self.game.name = value + self._mark_dirty() async def handle_game_mods(self, mode: Any, args: list[Any]): if not self.is_host(): diff --git a/server/games/game.py b/server/games/game.py index 3797a66b4..0139f054a 100644 --- a/server/games/game.py +++ b/server/games/game.py @@ -1,5 +1,4 @@ import asyncio -import contextlib import json import logging import re @@ -629,35 +628,6 @@ def get_basic_info(self) -> BasicGameInfo: def get_game_option(self, key: str, default: Any = None) -> Optional[Any]: return self.game_options.get(key, default) - def set_game_option(self, key: str, value: Any): - # Type transformations - if key == "Victory": - value = Victory.__members__.get(value.upper()) - elif key == "Slots": - value = int(value) - - self.game_options[key] = value - - # Additional attributes - if key == "ScenarioFile": - # TODO: What is the point of this transformation? - raw = repr(value) - scenario_path = \ - raw.replace("\\", "/").replace("//", "/").replace("'", "") - with contextlib.suppress(IndexError): - self.map_file_path = "maps/{}.zip".format( - scenario_path.split("/")[2].lower() - ) - map_info = self.game_service.map_info_cache.get(self.map_file_path) - if map_info is not None: - self.set_map_info(map_info) - else: - asyncio.create_task(self.update_map_info()) - elif key == "Title": - with contextlib.suppress(ValueError): - self.name = value - self.mark_dirty() - def get_player_option(self, player_id: int, key: str) -> Optional[Any]: """ Retrieve game-associative options for given player, by their uid diff --git a/tests/unit_tests/test_game.py b/tests/unit_tests/test_game.py index 5f56605b0..1ad114298 100644 --- a/tests/unit_tests/test_game.py +++ b/tests/unit_tests/test_game.py @@ -325,32 +325,6 @@ async def test_invalid_get_player_option_key(game: Game, players): assert game.get_player_option(players.hosting.id, -1) is None -def test_set_game_option(game: Game): - game.game_options = {"AIReplacement": "Off"} - game.set_game_option("Victory", "sandbox") - assert game.game_options["Victory"] == Victory.SANDBOX - game.set_game_option("AIReplacement", "On") - assert game.game_options["AIReplacement"] == "On" - game.set_game_option("Slots", "7") - assert game.max_players == 7 - game.set_game_option("Title", "All welcome") - assert game.name == "All welcome" - game.set_game_option("ArbitraryKey", "ArbitraryValue") - assert game.game_options["ArbitraryKey"] == "ArbitraryValue" - - -async def test_set_game_option_scenario_file(game: Game): - # Valid example from a replay - game.set_game_option("ScenarioFile", "/maps/scmp_009/scmp_009_scenario.lua") - assert game.map_file_path == "maps/scmp_009.zip" - - # Examples that document behavior but might not make sense or be necessary - game.set_game_option("ScenarioFile", "C:\\Maps\\Some_Map") - assert game.map_file_path == "maps/some_map.zip" - game.set_game_option("ScenarioFile", "/maps/'some map'/scenario.lua") - assert game.map_file_path == "maps/some map.zip" - - async def test_add_game_connection(game: Game, players, mock_game_connection): game.state = GameState.LOBBY mock_game_connection.player = players.hosting diff --git a/tests/unit_tests/test_gameconnection.py b/tests/unit_tests/test_gameconnection.py index 63e468f96..57f2d3cea 100644 --- a/tests/unit_tests/test_gameconnection.py +++ b/tests/unit_tests/test_gameconnection.py @@ -404,8 +404,32 @@ async def test_cannot_parse_game_results( # TODO: Convert this to test for game.set_game_option async def test_handle_action_GameOption(game, game_connection: GameConnection): + game.game_options = {"AIReplacement": "Off"} await game_connection.handle_action("GameOption", ["Victory", "sandbox"]) - game.set_game_option.assert_called_once_with("Victory", "sandbox") + assert game.game_options["Victory"] == Victory.SANDBOX + await game_connection.handle_action("GameOption", ["AIReplacement", "On"]) + assert game.game_options["AIReplacement"] == "On" + await game_connection.handle_action("GameOption", ["Slots", "7"]) + assert game.game_options["Slots"] == 7 + await game_connection.handle_action("GameOption", ["Title", "All welcome"]) + assert game.name == "All welcome" + await game_connection.handle_action("GameOption", ["ArbitraryKey", "ArbitraryValue"]) + assert game.game_options["ArbitraryKey"] == "ArbitraryValue" + + +async def test_handle_action_GameOption_ScenarioFile( + game, + game_connection: GameConnection +): + # Valid example from a replay + game.set_game_option("ScenarioFile", "/maps/scmp_009/scmp_009_scenario.lua") + assert game.map_file_path == "maps/scmp_009.zip" + + # Examples that document behavior but might not make sense or be necessary + game.set_game_option("ScenarioFile", "C:\\Maps\\Some_Map") + assert game.map_file_path == "maps/some_map.zip" + game.set_game_option("ScenarioFile", "/maps/'some map'/scenario.lua") + assert game.map_file_path == "maps/some map.zip" async def test_handle_action_GameOption_not_host( @@ -414,8 +438,9 @@ async def test_handle_action_GameOption_not_host( players ): game_connection.player = players.joining + game.game_options = {"Victory": "asdf"} await game_connection.handle_action("GameOption", ["Victory", "sandbox"]) - game.set_game_option.assert_not_called() + assert game.game_options == {"Victory": "asdf"} async def test_json_stats( From 039e54756385acbb0ffa7233c59c086975597903 Mon Sep 17 00:00:00 2001 From: Askaholic Date: Sat, 9 Apr 2022 15:51:51 -0800 Subject: [PATCH 5/7] Add initialize function to Game for async initialization --- server/game_service.py | 2 +- server/games/game.py | 11 +++++-- tests/unit_tests/test_game.py | 21 ++++++++++--- tests/unit_tests/test_game_rating.py | 33 +++++++++++++++++---- tests/unit_tests/test_game_stats_service.py | 9 +++++- tests/unit_tests/test_gameconnection.py | 11 +++++-- tests/unit_tests/test_laddergame.py | 4 ++- 7 files changed, 74 insertions(+), 17 deletions(-) diff --git a/server/game_service.py b/server/game_service.py index 9dac52c97..bc97bff53 100644 --- a/server/game_service.py +++ b/server/game_service.py @@ -204,7 +204,7 @@ async def create_game( } game_args.update(kwargs) game = game_class(**game_args) - game.set_map_info(await self.get_map_info(game.map_file_path)) + await game.initialize() self._games[game_id] = game diff --git a/server/games/game.py b/server/games/game.py index 0139f054a..199f05c29 100644 --- a/server/games/game.py +++ b/server/games/game.py @@ -113,6 +113,7 @@ def __init__( self.enforce_rating_range = enforce_rating_range self.matchmaker_queue_id = matchmaker_queue_id self.state = GameState.INITIALIZING + self.setup_timeout = setup_timeout self._connections = {} self._configured_player_ids: set[int] = set() self.enforce_rating = False @@ -132,11 +133,16 @@ def __init__( self.mods = {} self._override_validity: Optional[ValidityState] = None self._persisted_validity: Optional[ValidityState] = None - self._hosted_future = asyncio.Future() + self._hosted_future: Optional[asyncio.Future] = None self._finish_lock = asyncio.Lock() self._logger.debug("%s created", self) - asyncio.get_event_loop().create_task(self.timeout_game(setup_timeout)) + + async def initialize(self): + # So we can perform async operations at game creation time + self._hosted_future = asyncio.Future() + asyncio.create_task(self.timeout_game(self.setup_timeout)) + await self.update_map_info() async def timeout_game(self, timeout: int = 60): await asyncio.sleep(timeout) @@ -302,6 +308,7 @@ def get_team_sets(self) -> list[set[Player]]: return list(teams.values()) + ffa_players def set_hosted(self): + assert self._hosted_future is not None self._hosted_future.set_result(None) self.hosted_at = datetime_now() diff --git a/tests/unit_tests/test_game.py b/tests/unit_tests/test_game.py index 1ad114298..98b6e74e5 100644 --- a/tests/unit_tests/test_game.py +++ b/tests/unit_tests/test_game.py @@ -39,19 +39,32 @@ async def game(database, game_service, game_stats_service): game_stats_service, rating_type=RatingType.GLOBAL ) - await game.update_map_info() - + await game.initialize() return game @pytest.fixture async def coop_game(database, game_service, game_stats_service): - return CoopGame(42, database, game_service, game_stats_service) + game = CoopGame( + 42, + database, + game_service, + game_stats_service + ) + await game.initialize() + return game @pytest.fixture async def custom_game(database, game_service, game_stats_service): - return CustomGame(42, database, game_service, game_stats_service) + game = CustomGame( + 42, + database, + game_service, + game_stats_service + ) + await game.initialize() + return game async def game_player_scores(database, game): diff --git a/tests/unit_tests/test_game_rating.py b/tests/unit_tests/test_game_rating.py index 5ef4f8d65..d80280654 100644 --- a/tests/unit_tests/test_game_rating.py +++ b/tests/unit_tests/test_game_rating.py @@ -121,19 +121,40 @@ def get_published_results_by_player_id(mock_service): @pytest.fixture async def game(event_loop, database, game_service, game_stats_service): - return Game( - 42, database, game_service, game_stats_service, rating_type=RatingType.GLOBAL + game = Game( + 42, + database, + game_service, + game_stats_service, + rating_type=RatingType.GLOBAL ) + await game.initialize() + return game @pytest.fixture -async def custom_game(event_loop, database, game_service, game_stats_service): - return CustomGame(42, database, game_service, game_stats_service) +async def custom_game(database, game_service, game_stats_service): + game = CustomGame( + 42, + database, + game_service, + game_stats_service + ) + await game.initialize() + return game @pytest.fixture -async def ladder_game(event_loop, database, game_service, game_stats_service): - return LadderGame(42, database, game_service, game_stats_service, rating_type=RatingType.LADDER_1V1) +async def ladder_game(database, game_service, game_stats_service): + game = LadderGame( + 42, + database, + game_service, + game_stats_service, + rating_type=RatingType.LADDER_1V1 + ) + await game.initialize() + return game def add_players_with_rating(player_factory, game, ratings, teams): diff --git a/tests/unit_tests/test_game_stats_service.py b/tests/unit_tests/test_game_stats_service.py index caf6ce153..d1772ed7d 100644 --- a/tests/unit_tests/test_game_stats_service.py +++ b/tests/unit_tests/test_game_stats_service.py @@ -4,6 +4,7 @@ import pytest from server.factions import Faction +from server.game_service import GameService from server.games import Game from server.games.game_results import ( ArmyReportedOutcome, @@ -41,10 +42,16 @@ def player(player_factory): @pytest.fixture() async def game(database, game_stats_service, player): - game = Game(1, database, mock.Mock(), game_stats_service) + game = Game( + 1, + database, + mock.create_autospec(GameService), + game_stats_service + ) game._player_options[player.id] = {"Army": 1} game._results = GameResultReports(1) game._results.add(GameResultReport(1, 1, ArmyReportedOutcome.VICTORY, 0)) + await game.initialize() return game diff --git a/tests/unit_tests/test_gameconnection.py b/tests/unit_tests/test_gameconnection.py index 57f2d3cea..6a67dd4e1 100644 --- a/tests/unit_tests/test_gameconnection.py +++ b/tests/unit_tests/test_gameconnection.py @@ -23,8 +23,15 @@ @pytest.fixture -async def real_game(event_loop, database, game_service, game_stats_service): - return Game(42, database, game_service, game_stats_service) +async def real_game(database, game_service, game_stats_service): + game = Game( + 42, + database, + game_service, + game_stats_service + ) + await game.initialize() + return game def assert_message_sent(game_connection: GameConnection, command, args): diff --git a/tests/unit_tests/test_laddergame.py b/tests/unit_tests/test_laddergame.py index 36755e155..e7c27fcb5 100644 --- a/tests/unit_tests/test_laddergame.py +++ b/tests/unit_tests/test_laddergame.py @@ -13,13 +13,15 @@ @pytest.fixture() async def laddergame(database, game_service, game_stats_service): - return LadderGame( + game = LadderGame( id_=465312, database=database, game_service=game_service, game_stats_service=game_stats_service, rating_type=RatingType.LADDER_1V1 ) + await game.initialize() + return game async def test_handle_game_closed_manually(laddergame, players): From 0e20b6cbfabe2cf638cd9c8e2f360cb1e0af27bf Mon Sep 17 00:00:00 2001 From: Askaholic Date: Sat, 9 Apr 2022 15:52:36 -0800 Subject: [PATCH 6/7] Other minor code cleanup --- server/games/game.py | 2 +- tests/unit_tests/test_game.py | 8 ++++---- tests/unit_tests/test_game_stats_service.py | 6 ++++-- tests/unit_tests/test_gameconnection.py | 12 +++++++++--- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/server/games/game.py b/server/games/game.py index 199f05c29..26a49553c 100644 --- a/server/games/game.py +++ b/server/games/game.py @@ -905,7 +905,7 @@ def __eq__(self, other): return self.id == other.id def __hash__(self): - return self.id.__hash__() + return hash(self.id) def __str__(self) -> str: return ( diff --git a/tests/unit_tests/test_game.py b/tests/unit_tests/test_game.py index 98b6e74e5..753d80192 100644 --- a/tests/unit_tests/test_game.py +++ b/tests/unit_tests/test_game.py @@ -76,12 +76,12 @@ async def game_player_scores(database, game): return set(tuple(f) for f in result.fetchall()) -async def test_initialization(game: Game): +def test_initialization(game: Game): assert game.state is GameState.INITIALIZING assert game.enforce_rating is False -async def test_instance_logging(database, game_stats_service): +def test_instance_logging(database, game_stats_service): logger = logging.getLogger(f"{Game.__qualname__}.5") logger.debug = mock.Mock() mock_parent = mock.Mock() @@ -785,12 +785,12 @@ async def test_get_army_score_conflicting_results_tied(game, game_add_players): assert game.get_army_score(1) == 123 -async def test_equality(game): +def test_equality(game): assert game == game assert game != Game(5, mock.Mock(), mock.Mock(), mock.Mock()) -async def test_hashing(game): +def test_hashing(game): assert { game: 1, Game(game.id, mock.Mock(), mock.Mock(), mock.Mock()): 1 diff --git a/tests/unit_tests/test_game_stats_service.py b/tests/unit_tests/test_game_stats_service.py index d1772ed7d..862e5639d 100644 --- a/tests/unit_tests/test_game_stats_service.py +++ b/tests/unit_tests/test_game_stats_service.py @@ -31,8 +31,10 @@ def achievement_service(): @pytest.fixture() -def game_stats_service(event_service, achievement_service): - return GameStatsService(event_service, achievement_service) +async def game_stats_service(event_service, achievement_service): + service = GameStatsService(event_service, achievement_service) + await service.initialize() + return service @pytest.fixture() diff --git a/tests/unit_tests/test_gameconnection.py b/tests/unit_tests/test_gameconnection.py index 6a67dd4e1..eab981ee0 100644 --- a/tests/unit_tests/test_gameconnection.py +++ b/tests/unit_tests/test_gameconnection.py @@ -429,13 +429,19 @@ async def test_handle_action_GameOption_ScenarioFile( game_connection: GameConnection ): # Valid example from a replay - game.set_game_option("ScenarioFile", "/maps/scmp_009/scmp_009_scenario.lua") + await game_connection.handle_action( + "GameOption", ["ScenarioFile", "/maps/scmp_009/scmp_009_scenario.lua"] + ) assert game.map_file_path == "maps/scmp_009.zip" # Examples that document behavior but might not make sense or be necessary - game.set_game_option("ScenarioFile", "C:\\Maps\\Some_Map") + await game_connection.handle_action( + "GameOption", ["ScenarioFile", "C:\\Maps\\Some_Map"] + ) assert game.map_file_path == "maps/some_map.zip" - game.set_game_option("ScenarioFile", "/maps/'some map'/scenario.lua") + await game_connection.handle_action( + "GameOption", ["ScenarioFile", "/maps/'some map'/scenario.lua"] + ) assert game.map_file_path == "maps/some map.zip" From 36907fa848c9f0f0f269901e1ec8dfa56c8110b0 Mon Sep 17 00:00:00 2001 From: Askaholic Date: Sat, 9 Apr 2022 16:38:19 -0800 Subject: [PATCH 7/7] Make sure game_info messages are sent when GameOptions change --- server/gameconnection.py | 3 +- tests/integration_tests/test_game.py | 67 ++++++++++++++++++++++------ 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/server/gameconnection.py b/server/gameconnection.py index 3e6a5c3b3..57ad01073 100644 --- a/server/gameconnection.py +++ b/server/gameconnection.py @@ -248,7 +248,8 @@ async def handle_game_option(self, key: str, value: Any): elif key == "Title": with contextlib.suppress(ValueError): self.game.name = value - self._mark_dirty() + + self._mark_dirty() async def handle_game_mods(self, mode: Any, args: list[Any]): if not self.is_host(): diff --git a/tests/integration_tests/test_game.py b/tests/integration_tests/test_game.py index b9c72fc71..0469590bb 100644 --- a/tests/integration_tests/test_game.py +++ b/tests/integration_tests/test_game.py @@ -264,6 +264,52 @@ async def send_player_options(proto, *options): }) +async def send_game_options(proto, *options): + for option in options: + await proto.send_message({ + "target": "game", + "command": "GameOption", + "args": list(option) + }) + + +@fast_forward(20) +async def test_game_info_message_updates(lobby_server): + _, _, proto = await connect_and_sign_in( + ("test", "test_password"), lobby_server + ) + game_id = await host_game(proto) + await read_until_command(proto, "game_info", uid=game_id, timeout=5) + + await send_game_options(proto, ["Title", "Test GameOptions"]) + msg = await read_until_command(proto, "game_info", uid=game_id, timeout=5) + assert msg["title"] == "Test GameOptions" + + await send_game_options(proto, ["Slots", "7"]) + msg = await read_until_command(proto, "game_info", uid=game_id, timeout=5) + assert msg["max_players"] == 7 + + await send_game_options(proto, ["ScenarioFile", "/maps/foobar/scenario.lua"]) + msg = await read_until_command(proto, "game_info", uid=game_id, timeout=5) + assert msg["mapname"] == "foobar" + assert msg["map_file_path"] == "maps/foobar.zip" + + await send_game_options(proto, ["RestrictedCategories", "1"]) + msg = await read_until_command(proto, "game_info", uid=game_id, timeout=5) + assert "bad_unit_restrictions" in msg["validity"] + + await send_game_options(proto, ["Unranked", "Yes"]) + msg = await read_until_command(proto, "game_info", uid=game_id, timeout=5) + assert "host_set_unranked" in msg["validity"] + + await send_game_options(proto, ["Victory", "eradication"]) + msg = await read_until_command(proto, "game_info", uid=game_id, timeout=5) + assert "wrong_victory_condition" in msg["validity"] + + # TODO: Some options don't need to trigger game info updates. Pruning these + # could help with performance and reduce network bandwidth + + @fast_forward(20) async def test_game_validity_states(lobby_server): host_id, _, host_proto = await connect_and_sign_in( @@ -283,14 +329,10 @@ async def test_game_validity_states(lobby_server): assert msg["validity"] == ["uneven_teams_not_ranked", "single_player"] # Change the map to an unranked map - await host_proto.send_message({ - "target": "game", - "command": "GameOption", - "args": [ - "ScenarioFile", - "/maps/neroxis_map_generator_sneaky_map/sneaky_map_scenario.lua" - ] - }) + await send_game_options(host_proto, [ + "ScenarioFile", + "/maps/neroxis_map_generator_sneaky_map/sneaky_map_scenario.lua" + ]) msg = await read_until_command(host_proto, "game_info", uid=game_id, timeout=5) assert msg["validity"] == [ "bad_map", @@ -305,11 +347,10 @@ async def test_game_validity_states(lobby_server): assert msg["validity"] == ["bad_map"] # Change the map to a ranked map - await host_proto.send_message({ - "target": "game", - "command": "GameOption", - "args": ["ScenarioFile", "/maps/scmp_001/scmp_001_scenario.lua"] - }) + await send_game_options(host_proto, [ + "ScenarioFile", + "/maps/scmp_001/scmp_001_scenario.lua" + ]) msg = await read_until_command(host_proto, "game_info", uid=game_id, timeout=5) assert msg["validity"] == ["valid"]