Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove references to equilibrium #744 #764

Merged

Conversation

IanNaughton
Copy link
Contributor

@IanNaughton IanNaughton commented Apr 6, 2021

Remove references to equilibrium

I was able do three things as part of this change set.

  • Remove Equilibrium from the FeaturedModType enum and all references
  • Migrate game class detection out of the game_service class
  • Update game_service tests to pass in a game class since the method signature technically changed

Please let me know if you have any questions or feedback for improvement!

Closes #744

…ass detection up the call stack out of game_service.py
@IanNaughton IanNaughton closed this Apr 6, 2021
@IanNaughton IanNaughton reopened this Apr 6, 2021
@codecov
Copy link

codecov bot commented Apr 6, 2021

Codecov Report

Merging #764 (1ae15b0) into develop (c255301) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
server/game_service.py 99.01% <ø> (-0.02%) ⬇️
server/games/typedefs.py 98.07% <ø> (-0.02%) ⬇️
server/lobbyconnection.py 93.97% <100.00%> (+0.01%) ⬆️

@IanNaughton IanNaughton marked this pull request as ready for review April 6, 2021 02:14
Copy link
Collaborator

@Askaholic Askaholic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, looks pretty close!

Comment on lines 906 to 911
game_class = {
FeaturedModType.LADDER_1V1: LadderGame,
FeaturedModType.COOP: CoopGame,
FeaturedModType.FAF: CustomGame,
FeaturedModType.FAFBETA: CustomGame
}.get(game_mode, Game)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only need to check for coop here, and default to CustomGame.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use an if statement

@@ -15,6 +15,7 @@ async def test_create_game(players, game_service):
game = game_service.create_game(
visibility=VisibilityState.PUBLIC,
game_mode="faf",
game_class=CustomGame,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could make this the default argument.

@@ -154,15 +151,6 @@ def create_game(
"matchmaker_queue_id": matchmaker_queue_id,
}
game_args.update(kwargs)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would definitely either make game_class a required argument, or give it a sensible default. Defaulting to None just doesn't make any sense anymore.

Copy link
Collaborator

@Askaholic Askaholic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@Askaholic Askaholic merged commit aa535a9 into FAForever:develop Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove references to equilibrium
2 participants