Refactored search to remove redundant parameter 'player' (#377)

* Refactored search to remove redundant parameter 'player'

* Fixed wait_for timeout parameter

server/lobbyconnection.py

@@ -469,7 +469,6 @@ async def check_user_login(self, cursor, login, password):
 
         return player_id, real_username, steamid
 
-
     def check_version(self, message):
         versionDB, updateFile = self.player_service.client_version_info
         update_msg = dict(command="update",
@@ -575,7 +574,6 @@ async def command_hello(self, message):
                 if not conforms_policy:
                     return
 
-
             # Update the user's IRC registration (why the fuck is this here?!)
             m = hashlib.md5()
             m.update(password.encode())
@@ -799,13 +797,13 @@ def command_game_matchmaking(self, message):
                 if self.search:
                     self.search.cancel()
                 assert self.player is not None
-                self.search = Search(self.player)
                 self.player.faction = message['faction']
+                self.search = Search(self.player)
 
                 self.game_service.ladder_service.inform_player(self.player)
 
                 self._logger.info("%s is searching for ladder: %s", self.player, self.search)
-                asyncio.ensure_future(self.player_service.ladder_queue.search(self.player, search=self.search))
+                asyncio.ensure_future(self.player_service.ladder_queue.search(self.search))
 
     def command_coop_list(self, message):
         """ Request for coop map list"""

server/matchmaker/matchmaker_queue.py

@@ -62,7 +62,7 @@ def to_dict(self):
     def __repr__(self):
         return repr(self.queue)
 
-    async def search(self, player, start_time=None, search=None):
+    async def search(self, search: Search):
         """
         Search for a match.
 
@@ -72,7 +72,9 @@ async def search(self, player, start_time=None, search=None):
 
         :param player: Player to search for a matchup for
         """
-        search = search or Search(player, start_time)
+        assert search is not None
+
+        player = search.player
         with server.stats.timer('matchmaker.search'):
             try:
                 self._logger.debug("Searching for matchup for %s", player)
@@ -82,7 +84,8 @@ async def search(self, player, start_time=None, search=None):
 
                     quality = search.quality_with(opponent_search)
                     threshold = search.match_threshold
-                    self._logger.debug("Game quality between %s and %s: %f (threshold: %f)", player, opponent, quality, threshold)
+                    self._logger.debug("Game quality between %s and %s: %f (threshold: %f)",
+                                       player, opponent, quality, threshold)
                     if quality >= threshold:
                         if self.match(search, opponent_search):
                             return

tests/unit_tests/test_matchmaker_queue.py

@@ -12,6 +12,7 @@
 def matchmaker_queue(player_service, game_service):
     return MatchmakerQueue('test_queue', player_service, Mock())
 
+
 @pytest.fixture
 def matchmaker_players():
     return Player('Dostya',   id=1, ladder_rating=(2300, 64), numGames=(config.NEWBIE_MIN_GAMES+1)), \
@@ -21,6 +22,7 @@ def matchmaker_players():
            Player('Rhiza',    id=5, ladder_rating=(1200, 175), numGames=(config.NEWBIE_MIN_GAMES+1)), \
            Player('Newbie',   id=6, ladder_rating=(1200, 175), numGames=(config.NEWBIE_MIN_GAMES-1))
 
+
 @pytest.fixture
 def matchmaker_players_all_match():
     return Player('Dostya',   id=1, ladder_rating=(1500, 50), numGames=(config.NEWBIE_MIN_GAMES+1)), \
@@ -29,38 +31,45 @@ def matchmaker_players_all_match():
            Player('QAI',      id=4, ladder_rating=(1500, 50), numGames=(config.NEWBIE_MIN_GAMES+1)), \
            Player('Rhiza',    id=5, ladder_rating=(1500, 50), numGames=(config.NEWBIE_MIN_GAMES+1))
 
+
 def test_newbie_min_games(mocker, loop, matchmaker_players):
     p1, _, _, _, _, p6 = matchmaker_players
     s1, s6 = Search(p1), Search(p6)
     assert s1.rating == p1.ladder_rating and s6.rating != p6.ladder_rating
 
+
 def test_search_threshold(mocker, loop, matchmaker_players):
     s = Search(matchmaker_players[0])
     assert s.match_threshold <= 1
     assert s.match_threshold >= 0
 
+
 def test_search_quality_equivalence(mocker, loop, matchmaker_players):
     p1, _, _, p4, _, _ = matchmaker_players
     s1, s4 = Search(p1), Search(p4)
     assert s1.quality_with(s4) == s4.quality_with(s1)
 
+
 def test_search_quality(mocker, loop, matchmaker_players):
     p1, _, p3, _, p5, p6 = matchmaker_players
     s1, s3, s5, s6 = Search(p1), Search(p3), Search(p5), Search(p6)
     assert s3.quality_with(s5) > 0.7 and s1.quality_with(s6) < 0.2
 
+
 @asyncio.coroutine
 def test_search_match(mocker, loop, matchmaker_players):
     p1, _, _, p4, _, _ = matchmaker_players
     s1, s4 = Search(p1), Search(p4)
     assert s1.matches_with(s4)
 
+
 @asyncio.coroutine
 def test_search_no_match(mocker, loop, matchmaker_players):
     p1, p2, _, _, _, _ = matchmaker_players
     s1, s2 = Search(p1), Search(p2)
     assert not s1.matches_with(s2)
 
+
 @asyncio.coroutine
 def test_search_await(mocker, loop, matchmaker_players):
     p1, p2, _, _, _, _ = matchmaker_players
@@ -71,25 +80,27 @@ def test_search_await(mocker, loop, matchmaker_players):
     yield from asyncio.wait_for(await_coro, 1)
     assert await_coro.done()
 
+
 @asyncio.coroutine
 def test_queue_race(mocker, player_service, matchmaker_queue):
     p1, p2, p3 = Player('Dostya', id=1, ladder_rating=(2300, 150), numGames=(config.NEWBIE_MIN_GAMES+1)), \
                  Player('Brackman', id=2, ladder_rating=(2200, 150), numGames=(config.NEWBIE_MIN_GAMES+1)), \
                  Player('Zoidberg', id=3, ladder_rating=(2300, 125), numGames=(config.NEWBIE_MIN_GAMES+1))
 
-    player_service.players = {p1.id: p1, p2.id:p2, p3.id:p3}
+    player_service.players = {p1.id: p1, p2.id: p2, p3.id: p3}
 
     matchmaker_queue.game_service.ladder_service.start_game = CoroMock()
 
     try:
-        yield from asyncio.gather(asyncio.wait_for(matchmaker_queue.search(p1), 0.1),
-                                  asyncio.wait_for(matchmaker_queue.search(p2), 0.1),
-                                  asyncio.wait_for(matchmaker_queue.search(p3), 0.1))
+        yield from asyncio.gather(asyncio.wait_for(matchmaker_queue.search(Search(p1)), 0.1),
+                                  asyncio.wait_for(matchmaker_queue.search(Search(p2)), 0.1),
+                                  asyncio.wait_for(matchmaker_queue.search(Search(p3)), 0.1))
     except (TimeoutError, CancelledError):
         pass
 
     assert len(matchmaker_queue) == 0
 
+
 @asyncio.coroutine
 def test_queue_cancel(mocker, player_service, matchmaker_queue, matchmaker_players):
     # Turn list of players into map from ids to players.
@@ -99,32 +110,32 @@ def test_queue_cancel(mocker, player_service, matchmaker_queue, matchmaker_playe
     matchmaker_queue.push(s1)
     s1.cancel()
     try:
-        yield from asyncio.wait_for(matchmaker_queue.search(s2.player, search=s2), 0.01)
+        yield from asyncio.wait_for(matchmaker_queue.search(s2), 0.01)
     except (TimeoutError, CancelledError):
         pass
 
     assert not s1.is_matched
     assert not s2.is_matched
 
+
 async def test_queue_mid_cancel(mocker, player_service, matchmaker_queue, matchmaker_players_all_match):
     # Turn list of players into map from ids to players.
     player_service.players = dict(map(lambda x: (x.id, x), list(matchmaker_players_all_match)))
 
     matchmaker_queue.game_service.ladder_service.start_game = CoroMock()
 
-    s1, s2, s3 = Search(matchmaker_players_all_match[1]),\
-                 Search(matchmaker_players_all_match[2]),\
-                 Search(matchmaker_players_all_match[3])
-    asyncio.ensure_future(matchmaker_queue.search(s1.player, search=s1))
-    asyncio.ensure_future(matchmaker_queue.search(s2.player, search=s2))
+    (s1, s2, s3) = (Search(matchmaker_players_all_match[1]),
+                    Search(matchmaker_players_all_match[2]),
+                    Search(matchmaker_players_all_match[3]))
+    asyncio.ensure_future(matchmaker_queue.search(s1))
+    asyncio.ensure_future(matchmaker_queue.search(s2))
     s1.cancel()
     try:
-        await asyncio.wait_for(matchmaker_queue.search(s3.player, search=s3), 0.1)
+        await asyncio.wait_for(matchmaker_queue.search(s3), 0.1)
     except CancelledError:
         pass
 
     assert not s1.is_matched
     assert s2.is_matched
     assert s3.is_matched
     assert len(matchmaker_queue) == 0
-