Use cached_property to create Futures

[Gatsik]

Resolves #894

Also, fix one flaky test (test_game_matchmaking_start) by fully waiting for the game to start.

I took a look at other flaky tests, but I couldn't reproduce their flakiness: they had been continuing to pass.

The only flaky one which sometimes failed is test_game_matchmaking_close_fa_and_requeue, and I believe this is happening due to @fast_forward not working as wished with loop.run_in_executor futures.

Removing @fast_forward decorator fixes this test's flakiness, but increases runtime (from ~1 to about ~2.5 seconds), so I didn't touch that.

[Askaholic]

Sorry it took me a while to review. I wasn't sure about the approach at first, but after thinking about it for a while, I think I like it! I have just one question about the implementation, and one comment about the test change.

[Askaholic]

Should we be using get_running_loop instead maybe? I'm not entirely sure what all the intricacies are but the asyncio docs for get_event_loop in python 3.10 say this:

Because this function has rather complex behavior (especially when custom event loop policies are in use), using the get_running_loop() function is preferred to get_event_loop() in coroutines and callbacks.

[Askaholic]

I feel like this change wouldn't make any difference. In fact, as far as I can tell from the code the read_until_launched method should actually always wait longer than the wait_launched method. So I'm not sure if this actually fixes the flakiness or if maybe you just got lucky when running the tests. But I could be missing something.

I also want to keep these integration tests using only the protocol messages as much as possible. So if we can use a protocol message to determine that the game launched instead of having to reach into the internal state of a service, that would be much better. That way it can reveal deficiencies in our protocol if there are some use cases that are not possible using only the existing messages.

[Gatsik]

Hey! Yes, you are missing a few things here:

1. Setting player options marks game as dirty
2. read_until_launched method has launched_at as a criteria of game launch, which it receives from server message
3. Sending launched_at is not event-based -- broadcast service reports dirties every (more or less) fixed interval (currently it is 1 second)
4. launched_at is set before updates to the database are made (they happen in on_game_launched method):
   

   server/server/games/game.py

   Lines 727 to 739 in 9d6ba68

   	self.launched_at = time.time()
   	# Freeze currently connected players since we need them for rating when
   	# the game ends.
   	self._players_at_launch = [
   	player for player in self.get_connected_players()
   	if not self._is_observer(player)
   	]
   	self._players_with_unsent_army_stats = list(self._players_at_launch)
   	self.state = GameState.LIVE
   	await self.on_game_launched()
   	await self.validate_game_settings()

5. await statements tell event loop that it can process some other scheduled tasks

fast_forward just exaggerates timing discrepancies -- report_dirties does not wait full second between reports -- therefore reporting happens during database update more often.

But the test can fail even without fast_forward -- just tweak the sleep time here (0.01 and 0.99 worked for me):

server/tests/integration_tests/test_matchmaker.py

Lines 126 to 133 in 9d6ba68

	await send_player_options(
	host,
	(guest_id, "StartSpot", msg["map_position"]),
	(guest_id, "Army", msg["map_position"]),
	(guest_id, "Faction", msg["faction"]),
	(guest_id, "Color", msg["map_position"]),
	)
	await asyncio.sleep(0.5)

(I also don't completely understand the purpose of this sleep, I suppose it 'simulates' launching fa process on the client side). Anyway, some change to broadcast interval can break this test

On the other hand, wait_launched method of ladder_game is event-based -- it always sets its _launch_future after database update (which happens in super class):

server/server/games/ladder_game.py

Lines 46 to 48 in 9d6ba68

	async def launch(self):
	await super().launch()
	self._launch_future.set_result(None)

If you wanted to find deficiencies in protocol -- it is the one.

There's also another thing: at least in integration_tests services are initialized twice -- first time in their corresponding fixtures, and the second time here:

server/tests/integration_tests/conftest.py

Line 161 in 9d6ba68

name: await instance.listen(

because instance.started is False

I split this PR in 2 commits, so if you insist on leaving flaky test as is you can drop the fix