Issue/#675 Matchmaking requests from RabbitMQ

[Askaholic]

I changed a few things from the issue. Mostly I'm using name keys instead of ids for everything except players because that's what the ladder service expects in client messages already.

Probably need to refactor the rating_type on Game objects to allow unrated games by setting rating_type to None.

Closes #675

[codecov]

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (ea2b7d3) 94.61% compared to head (6677516) 96.55%.

❗ Current head 6677516 differs from pull request most recent head d3ed7da. Consider uploading reports for the commit d3ed7da to get more accurate results

Additional details and impacted files

Files	Coverage Δ
server/game_service.py	99.02% <ø> (+0.55%)	⬆️
server/gameconnection.py	95.01% <100.00%> (+3.75%)	⬆️
server/games/game.py	95.75% <94.73%> (+2.77%)	⬆️
server/message_queue_service.py	96.15% <87.50%> (-1.75%)	⬇️
server/ladder_service.py	96.02% <91.66%> (ø)

... and 62 files with indirect coverage changes

[eoinnoble]

Much of this is beyond my understanding, but what I do understand seems correct. I think we need to get more of the branches covered, and I have some very minor stylistic suggestions.

[eoinnoble]

Maybe keep the queue_name logic in one block and pull these out of the request closer to where they're being used.

[eoinnoble]

Certainly not a big deal but feels like we can probably reduce some of this looping?

Suggested change

	player_ids = [participant["player_id"] for participant in participants]
	missing_players = [
	id for id in player_ids if self.player_service[id] is None
	]
	if missing_players:
	raise Exception(
	"players_not_found",
	*[{"player_id": id} for id in missing_players]
	)
	all_players = [
	self.player_service[player_id] for player_id in player_ids
	]
	non_idle_players = [
	player for player in all_players
	if player.state != PlayerState.IDLE
	]
	all_players = []
	missing_players = []
	non_idle_players = []
	for participant in participants:
	player_id = participant["player_id"]
	player = self.player_service[player_id]
	if self.player_service[player_id]:
	all_players.append(player)
	if player.state != PlayerState.IDLE:
	non_idle_players.append(player)
	else:
	missing_players.append(player_id)
	if missing_players:
	raise Exception(
	"players_not_found",
	*[{"player_id": id} for id in missing_players]
	)

[eoinnoble]

Did you want to unpack this list like you did when there were missing_players?

[eoinnoble]

It would be easier on my poor brain if these two lines were closer in the code to where we're grabbing initial values.

[eoinnoble]

Do we need to check anywhere that len(all_players) >= 2?

[Brutus5000]

Recap of additionally required gw features as discussed in chat:

• map name instead of map id (for map generator maps)
• access to game settings such as
  • full share settings
  • maybe unit restrictions
  • game args (I want to override player names)
• usage of gw as exclusive queue (if you join it you can't join another queue or game)

[Askaholic]

This branch now supports:

• map name instead of map id (for map generator maps)
• access to game settings such as
  • full share settings
  • maybe unit restrictions
    not supported by the game
  • game args (I want to override player names)
    not supported by the game
• usage of gw as exclusive queue (if you join it you can't join another queue or game)

The autolobby system already supports setting arbitrary game options, but only for scalar values. Unit restrictions are represented as a nested value under the RestrictedCategories game option key. The lobby server is able to pass this on to the client, but I'm not sure how the client will translate that to a command line argument to the game, and the game doesn't have code to parse a dictionary or list value there anyway.

The player name is not part of the game options, so it would not be able to be set in this way. Supporting that would require starting on the lua side to figure out how that value should be passed in. I would also think this is not a blocker for an initial GW implementation.

Since these rabbitmq requests come from an external service which must be treated as a black box by the lobby server, there is no way for the lobby server to know if a player is in queue for GW or not, so we can't build any 'exclusive queue' logic around it.