FAForever / server

The servercode for the Forged Alliance Forever lobby
http://www.faforever.com
GNU General Public License v3.0
66 stars 61 forks source link

Cancel matches if someone closes the game #882

Closed BlackYps closed 2 years ago

BlackYps commented 2 years ago

873

I's not unlikely that the cancellation doesn't happen at the ideal place

codecov[bot] commented 2 years ago

Codecov Report

Merging #882 (75bf15f) into develop (2f39b97) will increase coverage by 0.02%. The diff coverage is 100.00%.

:exclamation: Current head 75bf15f differs from pull request most recent head e6f1fe4. Consider uploading reports for the commit e6f1fe4 to get more accurate results Impacted Files Coverage Δ
server/gameconnection.py 95.07% <100.00%> (-0.04%) :arrow_down:
server/games/game.py 95.38% <100.00%> (-0.05%) :arrow_down:
server/games/ladder_game.py 97.82% <100.00%> (+1.67%) :arrow_up:
server/ladder_service.py 98.38% <100.00%> (+0.01%) :arrow_up:
server/timing/timer.py 79.71% <0.00%> (+1.44%) :arrow_up:
BlackYps commented 2 years ago

test_game_matchmaking_close_fa_and_requeue seems to be flaky

BlackYps commented 2 years ago

Do you want to finish your temp ban logic first before we test or should we test that separately?

Askaholic commented 2 years ago

No let’s do them separately. It might still be a while until I get around to finishing mine.

BlackYps commented 2 years ago

Do you know why this small change in the last commit makes the two unit tests fail? I have no idea why check_sim_end() doesn't get called properly. It is literally the first line in the function I call.

Askaholic commented 2 years ago

It’s just because the game object is a Mock, and you changed the name of the function that is being called. You just need to update the tests as well to check for a call to the new function.

BlackYps commented 2 years ago

Oh I see, so the function calls of the game object are just mocks. Yeah, that makes sense.

BlackYps commented 2 years ago

Looking at the code I just noticed the TODO about the match_cancelled message again. Sheikah said that the client supports it now. So we could change it.

Askaholic commented 2 years ago

Yea, it's merged on the v2-develop branch already.