Issue/#624 fix map anti repetition (#758)

* Fix getting map version id

* Set async test deadlines to None

* Fix indent

* Add integration test for anti map repetition

* Refactor variables in choose_map

* Set default limit to 2

* Fix typing

server/config.py

@@ -84,7 +84,7 @@ def __init__(self):
         self.GEO_IP_DATABASE_MAX_AGE_DAYS = 22
 
         self.LADDER_1V1_OUTCOME_OVERRIDE = True
-        self.LADDER_ANTI_REPETITION_LIMIT = 3
+        self.LADDER_ANTI_REPETITION_LIMIT = 2
         self.LADDER_SEARCH_EXPANSION_MAX = 0.25
         self.LADDER_SEARCH_EXPANSION_STEP = 0.05
         # The maximum amount of time in seconds) to wait between pops.

server/db/models.py

@@ -141,7 +141,7 @@
     Column("gameType",  Enum(Victory),  nullable=False),
     Column("gameMod",   Integer,        ForeignKey("game_featuredMods.id"), nullable=False),
     Column("host",      Integer,        nullable=False),
-    Column("mapId",     Integer),
+    Column("mapId",     Integer,        ForeignKey("map_version.id")),
     Column("gameName",  String(128),    nullable=False),
     Column("validity",  Integer,        nullable=False),
 )

server/ladder_service.py

@@ -108,7 +108,7 @@ async def fetch_map_pools(self, conn) -> Dict[int, Tuple[str, List[Map]]]:
                 map_pool.c.name,
                 map_pool_map_version.c.weight,
                 map_pool_map_version.c.map_params,
-                map_version.c.map_id,
+                map_version.c.id.label("map_id"),
                 map_version.c.filename,
                 t_map.c.display_name
             ]).select_from(
@@ -137,8 +137,8 @@ async def fetch_map_pools(self, conn) -> Dict[int, Tuple[str, List[Map]]]:
                     else:
                         self._logger.warning(
                             "Unsupported map type %s in pool %s",
-                             map_type,
-                             row.id
+                            map_type,
+                            row.id
                         )
 
                 except Exception:

server/matchmaker/map_pool.py

@@ -19,7 +19,7 @@ def __init__(
         self.maps = None
         self.set_maps(maps)
 
-    def set_maps(self, maps: Iterable[Map]) -> None:
+    def set_maps(self, maps: Iterable[Union[Map, NeroxisGeneratedMap]]) -> None:
         self.maps = {map_.id: map_ for map_ in maps}
 
     def choose_map(self, played_map_ids: Iterable[int] = ()) -> Map:
@@ -40,9 +40,9 @@ def choose_map(self, played_map_ids: Iterable[int] = ()) -> Map:
 
         least_common = counter.most_common()[::-1]
         least_count = 1
-        for map_count in least_common:
-            if isinstance(self.maps[map_count[0]], Map):
-                least_count = map_count[1]
+        for id_, count in least_common:
+            if isinstance(self.maps[id_], Map):
+                least_count = count
                 break
 
         # Trim off the maps with higher play counts
@@ -55,5 +55,5 @@ def choose_map(self, played_map_ids: Iterable[int] = ()) -> Map:
 
         return self.maps[random.choices(least_common, weights=weights, k=1)[0][0]].get_map()
 
-    def __repr__(self):
+    def __repr__(self) -> str:
         return f"MapPool({self.id}, {self.name}, {list(self.maps.values())})"

tests/integration_tests/test_matchmaker.py

@@ -300,6 +300,54 @@ async def test_game_matchmaking_close_fa_and_requeue(lobby_server):
     await read_until_command(proto1, "match_found", timeout=5)
 
 
+@fast_forward(200)
+async def test_anti_map_repetition(lobby_server):
+    proto1, proto2 = await queue_players_for_matchmaking(lobby_server)
+
+    # Play one game so that it exists in the players history
+    msg1, _ = await asyncio.gather(
+        client_response(proto1),
+        client_response(proto2)
+    )
+    mapname = msg1["mapname"]
+
+    for proto in (proto1, proto2):
+        await proto.send_message({
+            "command": "GameState",
+            "target": "game",
+            "args": ["Launching"]
+        })
+
+    for proto in (proto1, proto2):
+        for result in (
+            [1, "draw 0"],
+            [2, "draw 0"],
+        ):
+            await proto.send_message({
+                "command": "GameResult",
+                "target": "game",
+                "args": result
+            })
+
+    for proto in (proto1, proto2):
+        await proto.send_message({
+            "command": "GameEnded",
+            "target": "game",
+            "args": []
+        })
+
+    # Now match a whole bunch of times and make sure we never get the map that
+    # was played. We don't actually play the game out here, so the players
+    # game history should remain unchanged.
+    for _ in range(20):
+        await proto1.close()
+        await proto2.close()
+
+        proto1, proto2 = await queue_players_for_matchmaking(lobby_server)
+        msg = await read_until_command(proto1, "game_launch")
+        assert msg["mapname"] != mapname
+
+
 @fast_forward(10)
 async def test_matchmaker_info_message(lobby_server, mocker):
     mocker.patch("server.matchmaker.pop_timer.time", return_value=1_562_000_000)

tests/unit_tests/test_ladder_service.py

@@ -54,12 +54,16 @@ async def test_load_from_database(ladder_service, queue_factory):
         assert queue.name == "ladder1v1"
         assert len(queue.map_pools) == 3
         assert list(queue.map_pools[1][0].maps.values()) == [
-            Map(id=15, name="SCMP_015", path="maps/scmp_015.v0003.zip"),
+            Map(id=15, name="SCMP_015", path="maps/scmp_015.zip"),
+            Map(id=16, name="SCMP_015", path="maps/scmp_015.v0002.zip"),
+            Map(id=17, name="SCMP_015", path="maps/scmp_015.v0003.zip"),
         ]
         assert list(queue.map_pools[2][0].maps.values()) == [
             Map(id=11, name="SCMP_011", path="maps/scmp_011.zip"),
             Map(id=14, name="SCMP_014", path="maps/scmp_014.zip"),
-            Map(id=15, name="SCMP_015", path="maps/scmp_015.v0003.zip"),
+            Map(id=15, name="SCMP_015", path="maps/scmp_015.zip"),
+            Map(id=16, name="SCMP_015", path="maps/scmp_015.v0002.zip"),
+            Map(id=17, name="SCMP_015", path="maps/scmp_015.v0003.zip"),
         ]
         assert list(queue.map_pools[3][0].maps.values()) == [
             Map(id=1, name="SCMP_001", path="maps/scmp_001.zip"),
@@ -112,7 +116,7 @@ async def test_load_from_database_new_data(ladder_service, database):
     player1=st_players("p1", player_id=1, lobby_connection_spec="mock"),
     player2=st_players("p2", player_id=2, lobby_connection_spec="mock")
 )
-@settings(deadline=300)
+@settings(deadline=None)
 @autocontext("ladder_and_game_service_context", "monkeypatch_context")
 async def test_start_game_1v1(
     ladder_and_game_service,
@@ -172,7 +176,7 @@ async def test_start_game_timeout(
     player3=st_players("p3", player_id=3, lobby_connection_spec="mock"),
     player4=st_players("p4", player_id=4, lobby_connection_spec="mock")
 )
-@settings(deadline=300)
+@settings(deadline=None)
 @autocontext("ladder_and_game_service_context", "monkeypatch_context")
 async def test_start_game_with_teams(
     ladder_and_game_service,