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

Fix some code style issues #837

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion license.rtf
Original file line number Diff line number Diff line change
Expand Up @@ -676,4 +676,4 @@ Public License instead of this License. But first, please read\par
<http://www.gnu.org/philosophy/why-not-lgpl.html>.\par
\par
}
1 change: 1 addition & 0 deletions server/api/api_accessor.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class SessionManager:
"""
Garantor for API access
"""

def __init__(self):
self.session = None # Instance of session

Expand Down
9 changes: 8 additions & 1 deletion server/broadcast_service.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
from __future__ import annotations

from typing import TYPE_CHECKING

from aio_pika import DeliveryMode

from .config import config
Expand All @@ -9,6 +13,9 @@
from .player_service import PlayerService
from .timing import LazyIntervalTimer

if TYPE_CHECKING:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think the if check is needed here. At least flake8 doesn't generate unused import warnings here. Hopefully its the same on codacy

Copy link
Contributor Author

@Gatsik Gatsik Sep 23, 2021

Choose a reason for hiding this comment

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

The check is needed because of import cycle. If you remove import, then flake8 will report the undefined name error.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Huh, then how is it possible for the type checker to import it?

Copy link
Contributor Author

@Gatsik Gatsik Sep 23, 2021

Choose a reason for hiding this comment

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

Type checkers assume this constant (TYPE_CHECKING) to be True. (mypy docs)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yea I know, so wouldn’t the type checker still experience an import cycle?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, type checker wouldn't experience an import cycle

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

@Gatsik Gatsik Sep 23, 2021

Choose a reason for hiding this comment

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

from server import ServerInstance


@with_logger
class BroadcastService(Service):
Expand All @@ -18,7 +25,7 @@ class BroadcastService(Service):

def __init__(
self,
server: "ServerInstance",
server: ServerInstance,
message_queue_service: MessageQueueService,
game_service: GameService,
player_service: PlayerService,
Expand Down
3 changes: 3 additions & 0 deletions server/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class ClientError(Exception):
If recoverable is False, it is expected that the connection be terminated
immediately.
"""

def __init__(self, message, recoverable=True, *args, **kwargs):
super().__init__(*args, **kwargs)
self.message = message
Expand All @@ -24,6 +25,7 @@ class BanError(Exception):
"""
Signals that an operation could not be completed because the user is banned.
"""

def __init__(self, ban_expiry, ban_reason, *args, **kwargs):
super().__init__(*args, **kwargs)
self.ban_expiry = ban_expiry
Expand All @@ -50,6 +52,7 @@ class AuthenticationError(Exception):
"""
The operation failed to authenticate.
"""

def __init__(self, message, method, *args, **kwargs):
super().__init__(*args, **kwargs)
self.message = message
Expand Down
1 change: 1 addition & 0 deletions server/game_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class GameService(Service):
"""
Utility class for maintaining lifecycle of games
"""

def __init__(
self,
database: FAFDatabase,
Expand Down
2 changes: 1 addition & 1 deletion server/gameconnection.py
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ async def handle_game_state(self, state: str):
await self.on_connection_lost()
self._mark_dirty()

async def handle_game_ended(self, *args: List[Any]):
async def handle_game_ended(self, *args: List[Any]):
"""
Signals that the simulation has ended.
"""
Expand Down
2 changes: 0 additions & 2 deletions server/games/coop.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import asyncio
Copy link
Contributor Author

Choose a reason for hiding this comment

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

and this


from .game import Game
from .typedefs import FA, GameType, InitMode, ValidityState, Victory

Expand Down
1 change: 0 additions & 1 deletion server/games/custom_game.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import asyncio
Copy link
Contributor Author

Choose a reason for hiding this comment

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

and why this was not marked as unused import before?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually I am seeing an unused import warning for this using flake8. It might have been in there for a long time though, like since before we added codacy?

import time

from server.decorators import with_logger
Expand Down
26 changes: 21 additions & 5 deletions server/games/game.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,21 @@
from __future__ import annotations
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh does this add forward references? That's pretty nice, although I'm hoping to migrate to python 3.9 soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this will become the default behavior only in python 3.10


import asyncio
import json
import logging
import time
from collections import defaultdict
from typing import Any, Dict, FrozenSet, Iterable, List, Optional, Set, Tuple
from typing import (
TYPE_CHECKING,
Any,
Dict,
FrozenSet,
Iterable,
List,
Optional,
Set,
Tuple
)

import pymysql
from sqlalchemy import and_, bindparam
Expand Down Expand Up @@ -43,6 +55,10 @@
VisibilityState
)

if TYPE_CHECKING:
from server import GameConnection, GameService, GameStatsService
from server.db import FAFDatabase


class GameError(Exception):
pass
Expand All @@ -58,9 +74,9 @@ class Game():
def __init__(
self,
id_: int,
database: "FAFDatabase",
game_service: "GameService",
game_stats_service: "GameStatsService",
database: FAFDatabase,
game_service: GameService,
game_stats_service: GameStatsService,
host: Optional[Player] = None,
name: str = "None",
map_: str = "SCMP_007",
Expand Down Expand Up @@ -192,7 +208,7 @@ def _is_observer(self, player: Player) -> bool:
return army is None or army < 0

@property
def connections(self) -> Iterable["GameConnection"]:
def connections(self) -> Iterable[GameConnection]:
return self._connections.values()

@property
Expand Down
1 change: 1 addition & 0 deletions server/ice_servers/nts.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class TwilioNTS:

Creates new twilio NTS tokens
"""

def __init__(self, sid=None, token=None):
if sid is None:
sid = config.TWILIO_ACCOUNT_SID # pragma: no cover
Expand Down
9 changes: 7 additions & 2 deletions server/ladder_service.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
"""
Manages interactions between players and matchmakers
"""
from __future__ import annotations

import asyncio
import json
import random
import re
from collections import defaultdict
from typing import Dict, List, Optional, Set, Tuple
from typing import TYPE_CHECKING, Dict, List, Optional, Set, Tuple

import aiocron
from sqlalchemy import and_, func, select, text, true
Expand Down Expand Up @@ -37,13 +38,17 @@
from .players import Player, PlayerState
from .types import GameLaunchOptions, Map, NeroxisGeneratedMap

if TYPE_CHECKING:
from .lobbyconnection import LobbyConnection


@with_logger
class LadderService(Service):
"""
Service responsible for managing the 1v1 ladder. Does matchmaking, updates
statistics, and launches the games.
"""

def __init__(
self,
database: FAFDatabase,
Expand Down Expand Up @@ -516,7 +521,7 @@ async def get_game_history(
])
return result

def on_connection_lost(self, conn: "LobbyConnection") -> None:
def on_connection_lost(self, conn: LobbyConnection) -> None:
if not conn.player:
return

Expand Down
5 changes: 3 additions & 2 deletions server/lobbyconnection.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
CoopGame,
CustomGame,
FeaturedModType,
Game,
GameConnectionState,
GameState,
InitMode,
Expand Down Expand Up @@ -1065,7 +1066,7 @@ async def command_modvault(self, message):
downloads=downloads, date=int(date.timestamp()), uid=uid, name=name, version=version, author=author,
ui=ui)
await self.send(out)
except:
except Exception:
self._logger.error(f"Error handling table_mod row (uid: {uid})", exc_info=True)

elif type == "like":
Expand All @@ -1091,7 +1092,7 @@ async def command_modvault(self, message):
canLike = False
else:
likers.append(self.player.id)
except:
except Exception:
likers = []

# TODO: Avoid sending all the mod info in the world just because we liked it?
Expand Down
2 changes: 1 addition & 1 deletion server/matchmaker/algorithm/bucket_teams.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def find(
teams, searches_without_team = self._find_teams(searches, team_size)

matchmaker1v1 = StableMarriageMatchmaker()
matches, unmatched_searches = matchmaker1v1.find(teams, 1)
matches, unmatched_searches = matchmaker1v1.find(teams, 1)

unmatched_searches.extend(searches_without_team)
return matches, unmatched_searches
Expand Down
2 changes: 2 additions & 0 deletions server/matchmaker/algorithm/stable_marriage.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

WeightedGraph = Dict[Search, List[Tuple[Search, float]]]


class StableMarriage(MatchmakingPolicy1v1):
def find(self, ranks: WeightedGraph) -> Dict[Search, Search]:
"""Perform the stable matching algorithm until a maximal stable matching
Expand Down Expand Up @@ -78,6 +79,7 @@ class StableMarriageMatchmaker(Matchmaker):
Runs stable marriage to produce a list of matches
and afterwards adds random matchups for previously unmatched new players.
"""

def find(
self, searches: Iterable[Search], team_size: int
) -> Tuple[List[Match], List[Search]]:
Expand Down
1 change: 1 addition & 0 deletions server/matchmaker/algorithm/team_matchmaker.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class TeamMatchMaker(Matchmaker):
8. pick the first game from the game list and remove all other games that contain the same players
9. repeat 8. until the list is empty
"""

def find(self, searches: Iterable[Search], team_size: int) -> Tuple[List[Match], List[Search]]:
if not searches:
return [], []
Expand Down
18 changes: 16 additions & 2 deletions server/matchmaker/matchmaker_queue.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,20 @@
from __future__ import annotations

import asyncio
import time
from collections import OrderedDict
from concurrent.futures import CancelledError
from datetime import datetime, timezone
from typing import Any, Callable, Dict, Iterable, List, Optional, Tuple
from typing import (
TYPE_CHECKING,
Any,
Callable,
Dict,
Iterable,
List,
Optional,
Tuple
)

import server.metrics as metrics

Expand All @@ -15,6 +26,9 @@
from .pop_timer import PopTimer
from .search import Match, Search

if TYPE_CHECKING:
from server import GameService

MatchFoundCallback = Callable[[Search, Search, "MatchmakerQueue"], Any]


Expand Down Expand Up @@ -42,7 +56,7 @@ def __exit__(self, exc_type, exc_value, traceback):
class MatchmakerQueue:
def __init__(
self,
game_service: "GameService",
game_service: GameService,
on_match_found: MatchFoundCallback,
name: str,
queue_id: int,
Expand Down
10 changes: 8 additions & 2 deletions server/matchmaker/pop_timer.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
from __future__ import annotations

import asyncio
from collections import deque
from time import time
from typing import Deque
from typing import TYPE_CHECKING, Deque

import server.metrics as metrics

from ..config import config
from ..decorators import with_logger

if TYPE_CHECKING:
from .matchmaker_queue import MatchmakerQueue


@with_logger
class PopTimer(object):
Expand All @@ -25,7 +30,8 @@ class PopTimer(object):
The player queue rate is based on a moving average over the last few pops.
The exact size can be set in config.
"""
def __init__(self, queue: "MatchmakerQueue"):

def __init__(self, queue: MatchmakerQueue):
self.queue = queue
# Set up deque's for calculating a moving average
self.last_queue_amounts: Deque[int] = deque(maxlen=config.QUEUE_POP_TIME_MOVING_AVG_SIZE)
Expand Down
1 change: 1 addition & 0 deletions server/message_queue_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class MessageQueueService(Service):
Service handling connection to the message queue
and providing an interface to publish messages.
"""

def __init__(self) -> None:
self._logger.debug("Message queue service created.")
self._connection = None
Expand Down
8 changes: 6 additions & 2 deletions server/party_service.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
"""
Manages interactions between players and parties
"""
from __future__ import annotations

from typing import Dict, List, Set
from typing import TYPE_CHECKING, Dict, List, Set

from .core import Service
from .decorators import with_logger
Expand All @@ -13,6 +14,9 @@
from .team_matchmaker.player_party import PlayerParty
from .timing import at_interval

if TYPE_CHECKING:
from .lobbyconnection import LobbyConnection


@with_logger
class PartyService(Service):
Expand Down Expand Up @@ -186,7 +190,7 @@ def remove_party(self, party):
# TODO: Send a special "disbanded" command?
self.write_broadcast_party(party, members=members)

def on_connection_lost(self, conn: "LobbyConnection") -> None:
def on_connection_lost(self, conn: LobbyConnection) -> None:
if not conn.player or conn.player not in self.player_parties:
return

Expand Down
8 changes: 6 additions & 2 deletions server/player_service.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
"""
Manages connected and authenticated players
"""
from __future__ import annotations

import asyncio
from typing import Optional, Set, ValuesView
from typing import TYPE_CHECKING, Optional, Set, ValuesView

import aiocron
from sqlalchemy import and_, select
Expand Down Expand Up @@ -32,6 +33,9 @@
user_group_assignment
)

if TYPE_CHECKING:
from .lobbyconnection import LobbyConnection


@with_logger
class PlayerService(Service):
Expand Down Expand Up @@ -279,7 +283,7 @@ async def shutdown(self):
"Could not send shutdown message to %s: %s", player, ex
)

def on_connection_lost(self, conn: "LobbyConnection") -> None:
def on_connection_lost(self, conn: LobbyConnection) -> None:
if not conn.player:
return

Expand Down
Loading