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

Feature/#1032 working veto system #1033

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from
11 changes: 7 additions & 4 deletions server/db/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,13 @@

matchmaker_queue_map_pool = Table(
"matchmaker_queue_map_pool", metadata,
Column("matchmaker_queue_id", Integer, ForeignKey("matchmaker_queue.id"), nullable=False),
Column("map_pool_id", Integer, ForeignKey("map_pool.id"), nullable=False),
Column("min_rating", Integer),
Column("max_rating", Integer),
Column("matchmaker_queue_id", Integer, ForeignKey("matchmaker_queue.id"), nullable=False),
Column("map_pool_id", Integer, ForeignKey("map_pool.id"), nullable=False),
Column("min_rating", Integer),
Column("max_rating", Integer),
Column("veto_tokens_per_player", Integer, nullable=False),
Column("max_tokens_per_map", Integer, nullable=False),
Column("minimum_maps_after_veto", Float, nullable=False),
)

teamkills = Table(
Expand Down
199 changes: 190 additions & 9 deletions server/ladder_service/ladder_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import re
import statistics
from collections import defaultdict
from typing import Awaitable, Callable, Optional
from typing import Awaitable, Callable, Optional, Iterable

import aiocron
import humanize
Expand Down Expand Up @@ -39,13 +39,15 @@
from server.ladder_service.violation_service import ViolationService
from server.matchmaker import (
MapPool,
MatchmakerQueueMapPool,
MatchmakerQueue,
OnMatchedCallback,
Search
)
from server.metrics import MatchLaunch
from server.player_service import PlayerService
from server.players import Player, PlayerState
from server.types import GameLaunchOptions, Map, NeroxisGeneratedMap
from server.types import GameLaunchOptions, Map, NeroxisGeneratedMap, MatchmakerQueueMapPoolVetoData


@with_logger
Expand All @@ -59,12 +61,15 @@
self,
database: FAFDatabase,
game_service: GameService,
player_service: PlayerService,
violation_service: ViolationService,
):
self._db = database
self._informed_players: set[Player] = set()
self.game_service = game_service
self.player_service = player_service
self.queues = {}
self.pools_veto_data: list[MatchmakerQueueMapPoolVetoData] = []
self.violation_service = violation_service

self._searches: dict[Player, dict[str, Search]] = defaultdict(dict)
Expand Down Expand Up @@ -100,7 +105,7 @@
queue.team_size = info["team_size"]
queue.rating_peak = await self.fetch_rating_peak(info["rating_type"])
queue.map_pools.clear()
for map_pool_id, min_rating, max_rating in info["map_pools"]:
for map_pool_id, min_rating, max_rating, veto_tokens_per_player, max_tokens_per_map, minimum_maps_after_veto in info["map_pools"]:
map_pool_name, map_list = map_pool_maps[map_pool_id]
if not map_list:
self._logger.warning(
Expand All @@ -110,21 +115,33 @@
name
)
queue.add_map_pool(
MapPool(map_pool_id, map_pool_name, map_list),
min_rating,
max_rating
MatchmakerQueueMapPool(
MapPool(map_pool_id, map_pool_name, map_list),
min_rating,
max_rating,
veto_tokens_per_player,
max_tokens_per_map,
minimum_maps_after_veto
)
)
# Remove queues that don't exist anymore
for queue_name in list(self.queues.keys()):
if queue_name not in db_queues:
self.queues[queue_name].shutdown()
del self.queues[queue_name]
prev_pools_veto_data = self.pools_veto_data
new_pools_veto_data = self.get_pools_veto_data()
if (new_pools_veto_data != prev_pools_veto_data):
self.pools_veto_data = new_pools_veto_data
for player in self.player_service.all_players:
await player.update_vetoes(self.pools_veto_data)

async def fetch_map_pools(self, conn) -> dict[int, tuple[str, list[Map]]]:
result = await conn.execute(
select(
map_pool.c.id,
map_pool.c.name,
map_pool_map_version.c.id.label("map_pool_map_version_id"),
map_pool_map_version.c.weight,
map_pool_map_version.c.map_params,
map_version.c.id.label("map_id"),
Expand All @@ -150,6 +167,7 @@
map_list.append(
Map(
id=row.map_id,
map_pool_map_version_id=row.map_pool_map_version_id,
folder_name=folder_name,
ranked=row.ranked,
weight=row.weight,
Expand All @@ -161,7 +179,7 @@
map_type = params["type"]
if map_type == "neroxis":
map_list.append(
NeroxisGeneratedMap.of(params, row.weight)
NeroxisGeneratedMap.of(params, row.weight, row.map_pool_map_version_id)
)
else:
self._logger.warning(
Expand Down Expand Up @@ -191,6 +209,9 @@
matchmaker_queue_map_pool.c.map_pool_id,
matchmaker_queue_map_pool.c.min_rating,
matchmaker_queue_map_pool.c.max_rating,
matchmaker_queue_map_pool.c.veto_tokens_per_player,
matchmaker_queue_map_pool.c.max_tokens_per_map,
matchmaker_queue_map_pool.c.minimum_maps_after_veto,
game_featuredMods.c.gamemod,
leaderboard.c.technical_name.label("rating_type")
)
Expand Down Expand Up @@ -219,7 +240,10 @@
info["map_pools"].append((
row.map_pool_id,
row.min_rating,
row.max_rating
row.max_rating,
row.veto_tokens_per_player,
row.max_tokens_per_map,
row.minimum_maps_after_veto
))
except Exception:
self._logger.warning(
Expand Down Expand Up @@ -523,7 +547,23 @@
pool = queue.get_map_pool_for_rating(rating)
if not pool:
raise RuntimeError(f"No map pool available for rating {rating}!")
game_map = pool.choose_map(played_map_ids)

pool, *_, max_tokens_per_map, minimum_maps_after_veto = queue.map_pools[pool.id]

vetoes_map = defaultdict(int)

for m in pool.maps.values():
for player in all_players:
vetoes_map[m.map_pool_map_version_id] += player.vetoes.get(m.map_pool_map_version_id, 0)

if (max_tokens_per_map == 0):
max_tokens_per_map = self.calculate_dynamic_tokens_per_map(minimum_maps_after_veto, vetoes_map.values())
# this should never happen actually so i am not sure do we need this here or not
if (max_tokens_per_map == 0):
self._logger.error("calculate_dynamic_tokens_per_map received impossible vetoes setup, all vetoes cancelled for a match")
vetoes_map = {}
max_tokens_per_map = 1
game_map = pool.choose_map(played_map_ids, vetoes_map, max_tokens_per_map)

game = self.game_service.create_game(
game_class=LadderGame,
Expand Down Expand Up @@ -673,6 +713,147 @@
if player not in connected_players
])

"""

Check warning on line 716 in server/ladder_service/ladder_service.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

server/ladder_service/ladder_service.py#L716

String statement has no effect
function finds minimal max_tokens_per_map > 0 for given M (minimal_maps_after_veto) > 0 and [iterable] of veto tokens applied for each map in the bracket
max_tokens_per_map - is the amount of veto_tokens required to fully ban a map
minimal_maps_after_veto - minimal sum of map weights, anything lower is forbidden (for map diversity purpuses)

lets rename max_tokens_per_map to T for simplicity
then weight of map with V tokens applied is max((T - V) / T, 0)

example: lets say we have A maps with 0 tokens applied, B Maps with 1 token, C Maps with 2 tokens
the inequality to be true:
A * max((T - 0) / T, 0) + B * max((T - 1) / T, 0) + C * max((T - 2) / T, 0) >= M

max((T - 0) / T, 0) is always 1, so:
B * max((T - 1) / T, 0) + C * max((T - 2) / T, 0) >= M - A

due to max() function, it splits to 3:
1) for 0 < T <= 1: 0 >= M - A which is the same as A >= M
2) for 1 < T <= 2: B * (T - 1) / T >= M - A
3) for T > 2: B * (T - 1) / T + C * (T - 2) / T >= M - A

since we are looking for minimal T > 0, we should just check out cases from min T to max T

in case 1): range T <= 1
here we trying to find answer using only 0-tokens maps
- if A >= M, then any value 0 < T <= 1 is valid answer, but to avoid division by zero errors, we return 1
a bit contradicts with "minimal" in function definition, but since veto tokens applied is always an integer, result of 1 will give the same veto system result as 0.0001
- if A < M, there is no answer in this case

[1] for every next case: M > A is true (otherwise we would return answer in case 1)
in case 2): range 1 < T <= 2
here we trying to find answer using maps with 0-1 tokens
B * (T - 1) / T >= M - A
B * (T - 1) >= (M - A) * T
BT - B - MT + AT >= 0
(B - M + A)T >= B
T >= B / (B + A - M)
note 1: B + A is just amount of maps to which we try to shrink bracket (only maps with 0 and 1 tokens applied)
so we can say that denominator is "map balance", amount of map to which we try to shrink the bracket minus the minimal amount it should be,
so if map balance is negative, the answer should be obviously discarded
in case if map balance is 0 answer also should be discarded because B > 0 and division by zero happens
note 2: since M is guaranteed to be > A due to [1], then A - M is negative, then B + A - M < B, so B / (B + A - M) > 1
thus, since we're looking for MINIMAL answer, we should just replace > with = :
T = B / (B + A - M)
its always > 1 so we can forget lower border issue in 1 < T <= 2 above
just calculating T, checking if its <= 2, if true - return T, if false - case 3

[2] for every next case: B / (B + A - M) > 2 which is the same as B > 2B + 2A - 2M
in case 3): range T > 2
here we trying to find answer using maps with 0-2 tokens
B * (T - 1) / T + C * (T - 2) / T >= M - A
BT - B + CT - 2C >= (M - A) * T
(B + C + A - M)T >= B + 2C
T >= (B + 2C) / (B + C + A - M)
note 1: now we can see pattern in nominator: its just sum of tokens applied to all maps to wich we currently try to shrink the bracket
its basically 0A + 1B in case 2, and 0A + 1B + 2C in case 3, you can return to A B C definitions above to be sure
note 2: denominator is the same as in case 2, map balance. Again, A - M is negative, so B + C + A - M < B + C
let prove that there is no need to worry about lower border, again:
lets assume that (B + 2C) / (B + C + A - M) < 2
then B + 2C < 2B + 2C + 2A - 2M
then B < 2B + 2A - 2M
which is contradicts with [2], so again, we can just replace >= with = :
T = (B + 2C) / (B + C + A - M)
just calulating it and return the value


case X)
if we had some number D of maps with X tokens, and in case 3 we received result more than X (and discarded it due to exceeding upper border), then we would say that
[3] (B + 2C) / (B + C + A - M) > X which is the same as B + 2C > XB + XC + XA - XM
and our X equation would have lower border of X ofcourse
T = (B + 2C + XD) / (D + B + C + A - M)
now lets prove that T > X:
lets assume that T < X
(B + 2C + XD) / (D + B + C + A - M) < X
B + 2C + XD < XD + XB + XC + XA - XM
B + 2C < XB + XC + XA - XM
which is contradicts with [3]
so, we just proved that we should not worry about lower border in any case
and just solve equations and not inequality for each case except case 1

notices:
1) in every case except case 1, nominator is just sum of tokens applied to all maps in equation
and denominator is map balance
2) for case 1 we always have tokens_sum equal to 0

conclusion: whole solution process is
1) sorting tokens applied in ascending order
cycle:
2) including next bunch of maps (with the same tokens applied value) to our group
3) checking if tokens_sum == 0
then its case 1, and return 1 if maps_balance > 0
4) otherwise
solving equation for current group
and checking the result vs upper border
and upper border is equal to the amount of tokens applied to the map next to last map in our group, or infinity if there is no such one
Copy link
Contributor

@Gatsik Gatsik Jan 3, 2025

Choose a reason for hiding this comment

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

Maybe the explanation could be shorter

Lets denote max_tokens_per_map = $d$ and len(tokens) = $n$. You are trying to achieve
$\sum_{i=1}^{n}(1 - \frac{a_{i}}{d}) = M$, which means $d = \frac{\sum_{i=1}^{n}a_{i}}{n - M}$

If some $a_{i} &gt; d$ then $1 - \frac{a_{i}}{d} &lt; 0$, which results in contributing nothing due to $weight = \max(0, 1 - \frac{a_{i}}{d})$ . Will excluding those ${a_{i}}$ from $tokens$ decrease $d$?

Given sorted array, excluding the last element ${a_{n}}$ from $tokens$ gives new $d_{1} = \frac{\sum_{i=1}^{n-1}a_{i}}{n - M - 1}$.
This excluding won't help if $d_{1} &gt; d$

Lets denote $\sum_{i=1}^{n-1}a_{i} = S$, then $d &lt; d_{1}$ is achieved when $\frac{S + a_{n}}{n - M} &lt; \frac{S}{n - M - 1}$, which means $a_{n} &lt; \frac{S + a_{n}}{n - M}$

But $S + a_{n} = \sum_{i=1}^{n}a_{i}$, therefore $a_{n} &lt; \frac{\sum_{i=1}^{n}a_{i}}{n - M}$$a_{n} &lt; d$

This means, that we can stop trying to decrease $d$ when $\max(tokens) &lt; d$

And you can describte the algorithm as easy as:

  • Find $d = \frac{\sum_{i=1}^{n}a_{i}}{n - M}$, if $d &gt; \max(tokens)$ then $d$ is found. otherwise exclude all $a_{i} \geq d$. Repeat while $n &gt; M$

(And of course handle special case when $d = 0$, which means $d$ can be anything and you've chosen it to be $1$)

"""
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe put docstring inside the function definition?

def calculate_dynamic_tokens_per_map(self, M: float, tokens: Iterable[int]) -> float:
sorted_tokens = sorted(tokens)
# adding infinity as last upper border
sorted_tokens.append(float("inf"))
# t is the maximum amount of tokens applied to any single map in current group
t = 0
tokens_sum = 0
for index, tokens in enumerate(sorted_tokens):
# if at [index] our current group is ended
if tokens > t:
maps_balance = index - M
# if our group is only 0-tokened maps
if tokens_sum == 0 and maps_balance >= 0:
return 1
if maps_balance > 0:
# solving the equation
candidate = tokens_sum / maps_balance
# checking it vs upper border
if candidate <= tokens:
return candidate
t = tokens
tokens_sum += tokens

# return 0 never happens for correct tokens - M pairs
return 0

def get_pools_veto_data(self) -> list[MatchmakerQueueMapPoolVetoData]:
result = []
for queue in self.queues.values():
for pool, *_, veto_tokens_per_player, max_tokens_per_map, minimum_maps_after_veto in queue.map_pools.values():
if max_tokens_per_map == 0 and minimum_maps_after_veto >= len(pool.maps.values()) \
or max_tokens_per_map != 0 and queue.team_size * 2 * veto_tokens_per_player / max_tokens_per_map > len(pool.maps.values()) - minimum_maps_after_veto:
veto_tokens_per_player = 0
max_tokens_per_map = 1
minimum_maps_after_veto = 1
self._logger.error(f"Wrong vetoes setup detected for pool {pool.id} in queue {queue.id}")
result.append(
MatchmakerQueueMapPoolVetoData(
Copy link
Contributor

Choose a reason for hiding this comment

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

I still don't really understand why you need MatchmakerQueueMapPoolVetoData. Why don't you just initialize MatchmakerQueueMapPool with correct vetoes setup and use it?

Copy link
Author

@K-ETFreeman K-ETFreeman Jan 4, 2025

Choose a reason for hiding this comment

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

atleast to avoid having any possible issues with queue.map_pools.clear() in update_data
theoretically user can change vetoes mid-queues-update, then server will delete all user's vetoes and he will need to apply all of them again which is very bad

Copy link
Author

Choose a reason for hiding this comment

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

Askaholic said "Technically we do run the matchmaking algorithm in a separate thread because it is the one piece of the code that has the potential to block the rest of the server if there are a lot of players in queue."

So i prefer to have robust solution here

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 that statement is relevant to vetos because the 'chose_map' stage happens after the matchmaking algorithm has finished. But you should probably set it up in such a way that vetos are locked in the moment a match is found like we do with factions in the PlayerParty.on_matched function. I think keeping the behavior for choosing vetos and factions consistent makes the most sense.

Copy link
Author

@K-ETFreeman K-ETFreeman Jan 5, 2025

Choose a reason for hiding this comment

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

I don't think that statement is relevant to vetos because the 'chose_map' stage happens after the matchmaking algorithm has finished. But you should probably set it up in such a way that vetos are locked in the moment a match is found like we do with factions in the PlayerParty.on_matched function. I think keeping the behavior for choosing vetos and factions consistent makes the most sense.

its not about chose_map, it about command_set_player_vetoes in lobbyconection.py
it uses matchmaker map pool data and this is async function, and i am not 100% sure how this works here

also whats the point of locking vetoes when match found? what can possibly go wrong if we dont do that?

map_pool_map_version_ids=[map.map_pool_map_version_id for map in pool.maps.values()],
veto_tokens_per_player=veto_tokens_per_player,
max_tokens_per_map=max_tokens_per_map,
minimum_maps_after_veto=minimum_maps_after_veto
)
)
return result

async def get_game_history(
self,
players: list[Player],
Expand Down
4 changes: 4 additions & 0 deletions server/lobbyconnection.py
Original file line number Diff line number Diff line change
Expand Up @@ -1315,6 +1315,10 @@ async def command_set_party_factions(self, message):

self.party_service.set_factions(self.player, list(factions))

async def command_set_player_vetoes(self, message):
converted = {v["map_pool_map_version_id"]: v["veto_tokens_applied"] for v in message["vetoes"]}
await self.player.update_vetoes(self.ladder_service.pools_veto_data, converted)

async def send_warning(self, message: str, fatal: bool = False):
"""
Display a warning message to the client
Expand Down
3 changes: 2 additions & 1 deletion server/matchmaker/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@
Used for keeping track of queues of players wanting to play specific kinds of
games, currently just used for 1v1 ``ladder``.
"""
from .map_pool import MapPool
from .map_pool import MapPool, MatchmakerQueueMapPool
from .matchmaker_queue import MatchmakerQueue
from .pop_timer import PopTimer
from .search import CombinedSearch, OnMatchedCallback, Search

__all__ = (
"CombinedSearch",
"MapPool",
"MatchmakerQueueMapPool",
"MatchmakerQueue",
"OnMatchedCallback",
"PopTimer",
Expand Down
28 changes: 20 additions & 8 deletions server/matchmaker/map_pool.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import random
from collections import Counter
from typing import Iterable
from typing import Iterable, NamedTuple

from ..decorators import with_logger
from ..types import Map, MapPoolMap
Expand All @@ -21,11 +21,11 @@
def set_maps(self, maps: Iterable[MapPoolMap]) -> None:
self.maps = {map_.id: map_ for map_ in maps}

def choose_map(self, played_map_ids: Iterable[int] = ()) -> Map:
def choose_map(self, played_map_ids: Iterable[int] = (), vetoes_map={}, max_tokens_per_map=1) -> Map:

Check warning on line 24 in server/matchmaker/map_pool.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

server/matchmaker/map_pool.py#L24

Dangerous default value {} as argument
"""
Select a random map who's id does not appear in `played_map_ids`. If
all map ids appear in the list, then pick one that appears the least
amount of times.
Select a random map using veto system weights.
The maps which are least played from played_map_ids
and not vetoed by any player are getting x2 weight multiplier.
"""
if not self.maps:
self._logger.critical(
Expand All @@ -50,10 +50,22 @@
least_common = least_common[:i]
break

weights = [self.maps[id_].weight for id_, _ in least_common]
least_common_ids = {id_ for id_, _ in least_common}

Check failure on line 53 in server/matchmaker/map_pool.py

View workflow job for this annotation

GitHub Actions / flake8

local variable 'least_common_ids' is assigned to but never used

Check warning on line 53 in server/matchmaker/map_pool.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

server/matchmaker/map_pool.py#L53

Unused variable 'least_common_ids'

Check notice on line 53 in server/matchmaker/map_pool.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

server/matchmaker/map_pool.py#L53

local variable 'least_common_ids' is assigned to but never used (F841)

map_id = random.choices(least_common, weights=weights, k=1)[0][0]
return self.maps[map_id].get_map()
# Anti-repetition is temporary disabled
# map_list = list((map.map_pool_map_version_id, map, 2 if (map.id in least_common_ids) and (vetoes_map.get(map.map_pool_map_version_id, 0) == 0) else 1) for map in self.maps.values())
map_list = list((map.map_pool_map_version_id, map, 1) for map in self.maps.values())
weights = [max(0, (1 - vetoes_map.get(id, 0) / max_tokens_per_map) * map.weight * least_common_multiplier) for id, map, least_common_multiplier in map_list]
return random.choices(map_list, weights=weights, k=1)[0][1]

Check warning on line 59 in server/matchmaker/map_pool.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

server/matchmaker/map_pool.py#L59

Standard pseudo-random generators are not suitable for security/cryptographic purposes.

def __repr__(self) -> str:
return f"MapPool({self.id}, {self.name}, {list(self.maps.values())})"


class MatchmakerQueueMapPool(NamedTuple):
map_pool: MapPool
min_rating: int | None
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use Optional[int], because looks like server hasn't adopted this syntax yet

max_rating: int | None
veto_tokens_per_player: int
max_tokens_per_map: int
minimum_maps_after_veto: float
Loading
Loading