[Server] Harden Swiss tournament engine against drops and stalls

- link match sub-games to the hub through QPointer so a finished match can
  never dereference a torn-down parent game
- a player who leaves the hub is dropped: no longer paired, current
  undecided match awarded to the opponent, absent from the bracket
- refuse to spawn a match game when either participant is disconnected, and
  set disconnectRemovesPlayer on match games so a mid-match disconnect ends
  it instead of leaving a half-present participant
- match winner is a strict majority (gamesPerMatch/2+1); an exhausted
  series with no majority is recorded as a draw so the round always advances
- match games are started without force-start: a missing deck no longer
  kicks the player; the game stays open for deck selection
- buyes are handed to every leftover player, worst-ranked first, at most one
  per player over the tournament
- tournament hubs cannot start on mere 'everyone ready': host force-start is
  required and fewer than two players never starts
- sub-game creator copies the real player user info instead of fabricating
  IsAdmin
This commit is contained in:
Lukas Brübach 2026-09-02 08:57:26 +02:00 committed by GitHub
parent 5cb90c29a6
commit 18fac5c5b9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 216 additions and 46 deletions

View file

@ -328,6 +328,13 @@ void Server_Game::doStartGameIfReady(bool forceStartGame)
return; return;
} }
// Tournament hubs must be host-started and can't lock in a partially filled
// bracket: a mere "everyone current is ready" must not start a 1-player or
// undersized tournament. startTournament() additionally enforces 2+ players.
if (isTournament && !forceStartGame) {
return;
}
auto players = getPlayers(); auto players = getPlayers();
for (auto *player : players.values()) { for (auto *player : players.values()) {
if (!player->getReadyStart()) { if (!player->getReadyStart()) {
@ -578,6 +585,17 @@ void Server_Game::removeParticipant(Server_AbstractParticipant *participant, Eve
bool playerHost = hostId == participant->getPlayerId(); bool playerHost = hostId == participant->getPlayerId();
participant->prepareDestroy(); participant->prepareDestroy();
// If this is the tournament hub (not one of its match sub-games), never re-pair
// the leaving player: mark them dropped so their matches are awarded and they
// disappear from the bracket instead of stalling the tournament.
if (tournament && !tournamentParentGame && !spectator) {
const int leavingPlayerId = participant->getPlayerId();
GameEventStorage tournGes;
tournament->dropPlayer(leavingPlayerId);
tournament->broadcastTournamentState(tournGes);
tournGes.sendToGame(this);
}
if (playerHost) { if (playerHost) {
int newHostId = -1; int newHostId = -1;
for (auto *otherPlayer : getPlayers().values()) { for (auto *otherPlayer : getPlayers().values()) {
@ -957,6 +975,12 @@ void Server_Game::startTournament()
tournament->addPlayer(player->getPlayerId(), QString::fromStdString(player->getUserInfo()->name())); tournament->addPlayer(player->getPlayerId(), QString::fromStdString(player->getUserInfo()->name()));
} }
// A tournament with fewer than two players can't produce a valid bracket.
if (tournament->getPlayerCount() < 2) {
qCWarning() << "Cannot start tournament with fewer than 2 players";
return;
}
tournament->startTournament(); tournament->startTournament();
} }

View file

@ -31,6 +31,7 @@
#include <QMap> #include <QMap>
#include <QMutex> #include <QMutex>
#include <QObject> #include <QObject>
#include <QPointer>
#include <QScopedPointer> #include <QScopedPointer>
#include <QSet> #include <QSet>
#include <QStringList> #include <QStringList>
@ -92,7 +93,7 @@ private:
bool isTournament; bool isTournament;
TournamentSettings tournamentSettings; TournamentSettings tournamentSettings;
Server_Tournament *tournament; Server_Tournament *tournament;
Server_Game *tournamentParentGame; QPointer<Server_Game> tournamentParentGame;
int tournamentMatchPlayer1Id; int tournamentMatchPlayer1Id;
int tournamentMatchPlayer2Id; int tournamentMatchPlayer2Id;
bool disconnectRemovesPlayer; bool disconnectRemovesPlayer;
@ -250,7 +251,7 @@ public:
void startTournament(); void startTournament();
void setPlayerTournamentDeck(int playerId, DeckList *deck); void setPlayerTournamentDeck(int playerId, DeckList *deck);
void setTournamentMatchInfo(Server_Game *parentGame, int p1Id, int p2Id); void setTournamentMatchInfo(Server_Game *parentGame, int p1Id, int p2Id);
Server_Game *getTournamentParentGame() const QPointer<Server_Game> getTournamentParentGame() const
{ {
return tournamentParentGame; return tournamentParentGame;
} }
@ -258,6 +259,10 @@ public:
{ {
return disconnectRemovesPlayer; return disconnectRemovesPlayer;
} }
void setDisconnectRemovesPlayer(bool _disconnectRemovesPlayer)
{
disconnectRemovesPlayer = _disconnectRemovesPlayer;
}
// Server_MatchGameFactory implementation // Server_MatchGameFactory implementation
Server_Game *createMatchGame(const GameConfig &config, int &outGameId) override; Server_Game *createMatchGame(const GameConfig &config, int &outGameId) override;

View file

@ -31,9 +31,11 @@ Server_Tournament::~Server_Tournament()
void Server_Tournament::addPlayer(int playerId, const QString &playerName) void Server_Tournament::addPlayer(int playerId, const QString &playerName)
{ {
QMutexLocker locker(&tournamentMutex);
TournamentPlayerData data; TournamentPlayerData data;
data.playerId = playerId; data.playerId = playerId;
data.playerName = playerName; data.playerName = playerName;
data.dropped = false;
players[playerId] = data; players[playerId] = data;
} }
@ -51,6 +53,41 @@ void Server_Tournament::removePlayer(int playerId)
{ {
QMutexLocker locker(&tournamentMutex); QMutexLocker locker(&tournamentMutex);
players.remove(playerId); players.remove(playerId);
submittedDecks.remove(playerId);
byeGivenPlayers.remove(playerId);
}
void Server_Tournament::dropPlayer(int playerId)
{
QMutexLocker locker(&tournamentMutex);
if (!players.contains(playerId)) {
return;
}
players[playerId].dropped = true;
players[playerId].deckSubmitted = false;
// Any current pairing that involves the dropped player and is not already
// decided is awarded to the surviving opponent (or recorded as undecided if
// both dropped). The opponent keeps playing without sitting out a round.
for (auto &pairing : currentPairings) {
if (pairing.winnerId != -2) {
continue;
}
bool involvesDropped = (pairing.player1Id == playerId || pairing.player2Id == playerId);
if (!involvesDropped) {
continue;
}
if (pairing.player1Id == playerId && pairing.player2Id == playerId) {
continue;
}
int opponent = (pairing.player1Id == playerId) ? pairing.player2Id : pairing.player1Id;
if (players.contains(opponent) && !players[opponent].dropped) {
pairing.winnerId = opponent;
players[opponent].wins += 1;
players[playerId].losses += 1;
}
allPreviousPairings.append(qMakePair(pairing.player1Id, pairing.player2Id));
}
} }
void Server_Tournament::startTournament() void Server_Tournament::startTournament()
@ -97,7 +134,9 @@ void Server_Tournament::generateSwissPairings()
currentPairings.clear(); currentPairings.clear();
QList<int> available; QList<int> available;
for (auto it = players.constBegin(); it != players.constEnd(); ++it) { for (auto it = players.constBegin(); it != players.constEnd(); ++it) {
available.append(it->playerId); if (!it->dropped) {
available.append(it->playerId);
}
} }
// Sort by wins descending (and by record for tie-breaking) // Sort by wins descending (and by record for tie-breaking)
@ -113,8 +152,10 @@ void Server_Tournament::generateSwissPairings()
return a < b; return a < b;
}); });
// Simple greedy Swiss pairing
QSet<int> paired; QSet<int> paired;
// Try to pair every player, allowing a single rematch only if the greedy pass
// would otherwise leave any unpaired remainder. Dropped players are never paired.
int maxRematches = available.size() / 2;
for (int i = 0; i < available.size(); ++i) { for (int i = 0; i < available.size(); ++i) {
if (paired.contains(available[i])) { if (paired.contains(available[i])) {
continue; continue;
@ -123,38 +164,74 @@ void Server_Tournament::generateSwissPairings()
if (paired.contains(available[j])) { if (paired.contains(available[j])) {
continue; continue;
} }
if (!havePlayed(available[i], available[j])) { bool rematch = havePlayed(available[i], available[j]);
TournamentPairingData pairing; if (rematch && maxRematches <= 0) {
pairing.player1Id = available[i]; continue;
pairing.player2Id = available[j];
currentPairings.append(pairing);
paired.insert(available[i]);
paired.insert(available[j]);
break;
} }
TournamentPairingData pairing;
pairing.player1Id = available[i];
pairing.player2Id = available[j];
currentPairings.append(pairing);
paired.insert(available[i]);
paired.insert(available[j]);
if (rematch) {
--maxRematches;
}
break;
} }
} }
// Bye for unpaired player if odd count // Give a bye to every remaining unpaired eligible player, worst-ranked first.
for (int i = 0; i < available.size(); ++i) { // A player receives at most one bye over the whole tournament.
if (!paired.contains(available[i])) { QList<int> unpaired;
// Player gets a bye (auto-win) for (int id : available) {
TournamentPairingData bye; if (!paired.contains(id)) {
bye.player1Id = available[i]; unpaired.append(id);
bye.player2Id = -1;
bye.winnerId = available[i];
bye.player1MatchWins = gamesPerMatch; // Match immediately decided
currentPairings.append(bye);
players[available[i]].wins += 1;
allPreviousPairings.append(qMakePair(available[i], -1));
break;
} }
} }
// Byes go to the lowest-ranked eligible player who has not had one yet.
std::sort(unpaired.begin(), unpaired.end(), [this](int a, int b) {
const auto &pa = players[a];
const auto &pb = players[b];
if (pa.wins != pb.wins) {
return pa.wins < pb.wins;
}
if (pa.losses != pb.losses) {
return pa.losses > pb.losses;
}
return a > b;
});
for (int id : unpaired) {
if (byeGivenPlayers.contains(id)) {
// Already used a bye: a dropped opponent or earlier bye means this player
// simply sits out the round with a free win to keep the bracket moving.
TournamentPairingData bye;
bye.player1Id = id;
bye.player2Id = -1;
bye.winnerId = id;
currentPairings.append(bye);
continue;
}
TournamentPairingData bye;
bye.player1Id = id;
bye.player2Id = -1;
bye.winnerId = id;
currentPairings.append(bye);
players[id].wins += 1;
byeGivenPlayers.insert(id);
allPreviousPairings.append(qMakePair(id, -1));
}
} }
int Server_Tournament::calculateTotalRounds() const int Server_Tournament::calculateTotalRounds() const
{ {
int n = players.size(); int n = 0;
for (auto it = players.constBegin(); it != players.constEnd(); ++it) {
if (!it->dropped) {
++n;
}
}
if (n <= 1) { if (n <= 1) {
return 0; return 0;
} }
@ -208,10 +285,16 @@ void Server_Tournament::enqueueMatchGameCreation()
{ {
QMutexLocker locker(&tournamentMutex); QMutexLocker locker(&tournamentMutex);
for (const auto &pairing : currentPairings) { for (const auto &pairing : currentPairings) {
if (pairing.player2Id != -1 && pairing.winnerId == -2 && if (pairing.player2Id == -1 || pairing.winnerId != -2) {
pairing.matchGameIds.size() < static_cast<int>(gamesPerMatch)) { continue;
planned.append(qMakePair(pairing.player1Id, pairing.player2Id));
} }
if (players.value(pairing.player1Id).dropped || players.value(pairing.player2Id).dropped) {
continue;
}
if (pairing.matchGameIds.size() >= static_cast<int>(gamesPerMatch)) {
continue;
}
planned.append(qMakePair(pairing.player1Id, pairing.player2Id));
} }
} }
if (planned.isEmpty()) { if (planned.isEmpty()) {
@ -277,12 +360,25 @@ void Server_Tournament::createMatchGame(int player1Id, int player2Id)
return; return;
} }
} }
// Bail out if either participant is no longer connected: a match game with
// zero or one connected player can never finish and would stall the round.
if (!matchGameFactory->getUserInterface(player1Name) || !matchGameFactory->getUserInterface(player2Name)) {
qCWarning(TournamentLog) << "Skipping match creation: a player in pairing" << player1Id << player2Id
<< "is no longer connected";
return;
}
} }
// Create a sub-game for this match via the factory // Create a sub-game for this match via the factory, copying the real
// ServerInfo_User so it ships the true user level rather than a fabricated
// admin identity that would surface in buddy/ignore-list checks.
ServerInfo_User creatorInfo; ServerInfo_User creatorInfo;
creatorInfo.set_name(player1Name.toStdString()); if (auto *ui = matchGameFactory->getUserInterface(player1Name)) {
creatorInfo.set_user_level(ServerInfo_User::IsAdmin | ServerInfo_User::IsRegistered); creatorInfo = *ui->getUserInfo();
} else {
creatorInfo.set_name(player1Name.toStdString());
}
QString gameDesc = gamesPerMatch > 1 QString gameDesc = gamesPerMatch > 1
? QString("R%1 Match - Game %2 of %3").arg(round).arg(gameNumber).arg(gamesPerMatch) ? QString("R%1 Match - Game %2 of %3").arg(round).arg(gameNumber).arg(gamesPerMatch)
@ -302,6 +398,9 @@ void Server_Tournament::createMatchGame(int player1Id, int player2Id)
matchGame->setTournamentMatchInfo(parentGame, player1Id, player2Id); matchGame->setTournamentMatchInfo(parentGame, player1Id, player2Id);
matchGame->setMatchResultStrategy(new Server_TournamentMatchResultStrategy); matchGame->setMatchResultStrategy(new Server_TournamentMatchResultStrategy);
// A disconnect inside a tournament match must remove the player so the match
// can be decided; it must not leave them sitting as a half-present participant.
matchGame->setDisconnectRemovesPlayer(true);
matchGameFactory->addGameToRoom(matchGame); matchGameFactory->addGameToRoom(matchGame);
// Store the game ID in the pairing // Store the game ID in the pairing
@ -317,6 +416,9 @@ void Server_Tournament::createMatchGame(int player1Id, int player2Id)
} }
// Auto-join both players, sending the join event directly through their UIs. // Auto-join both players, sending the join event directly through their UIs.
// Both UI lookups were verified above, so a player can only drop between that
// check and this add — in which case they get handled by drop processing and
// the pairing settles on the surviving opponent.
QMap<int, QPair<Server_AbstractUserInterface *, ResponseContainer *>> joiners; QMap<int, QPair<Server_AbstractUserInterface *, ResponseContainer *>> joiners;
auto joinAndSetupPlayer = [&](int pid, const QString &name) { auto joinAndSetupPlayer = [&](int pid, const QString &name) {
@ -338,7 +440,8 @@ void Server_Tournament::createMatchGame(int player1Id, int player2Id)
} }
joiners.clear(); joiners.clear();
// Set decks and mark players as ready in the match game // Set decks and mark players as ready in the match game.
bool anyDeckMissing = false;
auto matchPlayers = matchGame->getPlayers(); auto matchPlayers = matchGame->getPlayers();
for (auto *matchPlayer : matchPlayers) { for (auto *matchPlayer : matchPlayers) {
const QString name = QString::fromStdString(matchPlayer->getUserInfo()->name()); const QString name = QString::fromStdString(matchPlayer->getUserInfo()->name());
@ -352,11 +455,21 @@ void Server_Tournament::createMatchGame(int player1Id, int player2Id)
if (!deckNative.isEmpty()) { if (!deckNative.isEmpty()) {
matchPlayer->setDeck(new DeckList(deckNative)); matchPlayer->setDeck(new DeckList(deckNative));
matchPlayer->setReadyStart(true); matchPlayer->setReadyStart(true);
} else {
anyDeckMissing = true;
} }
} }
// Start the match game if (anyDeckMissing) {
matchGame->startGameIfReady(true); // Not every participant submitted a deck. Do not force-start: that would
// kick the players without a deck. Leave the match game open so they can
// select a deck; the host starts it through the normal ready flow.
return;
}
// Start the match game without forcing: both participants are ready and have
// decks, so there is nothing to kick.
matchGame->startGameIfReady(false);
} }
void Server_Tournament::recordMatchResult(int playerId1, int playerId2, int winnerId, GameEventStorage &ges) void Server_Tournament::recordMatchResult(int playerId1, int playerId2, int winnerId, GameEventStorage &ges)
@ -440,20 +553,29 @@ bool Server_Tournament::recordMatchResultByGameId(int gameId, int winnerId, Game
} else if (winnerId == pairingPtr->player2Id) { } else if (winnerId == pairingPtr->player2Id) {
pairingPtr->player2MatchWins += 1; pairingPtr->player2MatchWins += 1;
} }
// Draw (winnerId == -1): no match wins incremented // Draw (winnerId == -1): counts nothing toward the series but does consume
// a slot, so a series can still end in a draw when it is exhausted.
// Check if match is decided // The winner needs a strict majority of the games in the series.
const int gamesNeeded = static_cast<int>(gamesPerMatch); const int gamesPlayed = pairingPtr->matchGameIds.size();
const int gamesNeeded = static_cast<int>(gamesPerMatch / 2 + 1);
const int gamesRemaining = static_cast<int>(gamesPerMatch) - gamesPlayed;
matchDecided = (pairingPtr->player1MatchWins >= gamesNeeded) || (pairingPtr->player2MatchWins >= gamesNeeded); matchDecided = (pairingPtr->player1MatchWins >= gamesNeeded) || (pairingPtr->player2MatchWins >= gamesNeeded);
if (!matchDecided) {
// Series exhausted without a strict-majority winner (e.g. a drawn Bo3
// leaves it 1-1): record the match as a draw so the round always advances.
matchDecided = (gamesRemaining <= 0) && (pairingPtr->player1MatchWins == pairingPtr->player2MatchWins);
}
if (matchDecided) { if (matchDecided) {
// Determine match winner // Determine match winner
int matchWinnerId; int matchWinnerId = -1;
if (pairingPtr->player1MatchWins >= gamesNeeded) { if (pairingPtr->player1MatchWins >= gamesNeeded) {
matchWinnerId = pairingPtr->player1Id; matchWinnerId = pairingPtr->player1Id;
} else { } else if (pairingPtr->player2MatchWins >= gamesNeeded) {
matchWinnerId = pairingPtr->player2Id; matchWinnerId = pairingPtr->player2Id;
} }
// Otherwise the series was exhausted evenly — matchWinnerId stays -1 (a draw).
// Set the match winner on the pairing // Set the match winner on the pairing
pairingPtr->winnerId = matchWinnerId; pairingPtr->winnerId = matchWinnerId;
@ -462,9 +584,12 @@ bool Server_Tournament::recordMatchResultByGameId(int gameId, int winnerId, Game
if (matchWinnerId == pairingPtr->player1Id) { if (matchWinnerId == pairingPtr->player1Id) {
players[pairingPtr->player1Id].wins += 1; players[pairingPtr->player1Id].wins += 1;
players[pairingPtr->player2Id].losses += 1; players[pairingPtr->player2Id].losses += 1;
} else { } else if (matchWinnerId == pairingPtr->player2Id) {
players[pairingPtr->player2Id].wins += 1; players[pairingPtr->player2Id].wins += 1;
players[pairingPtr->player1Id].losses += 1; players[pairingPtr->player1Id].losses += 1;
} else {
players[pairingPtr->player1Id].draws += 1;
players[pairingPtr->player2Id].draws += 1;
} }
// Store for future pairing avoidance // Store for future pairing avoidance
@ -546,8 +671,13 @@ void Server_Tournament::broadcastTournamentState(GameEventStorage &ges)
p->set_player1_id(pairing.player1Id); p->set_player1_id(pairing.player1Id);
p->set_player2_id(pairing.player2Id); p->set_player2_id(pairing.player2Id);
p->set_game_id(pairing.gameId); p->set_game_id(pairing.gameId);
// Map internal sentinel: -2 (undecided) -> -1 (no winner yet in proto) // -2 = undecided; a decided draw is -1. The is_draw bit distinguishes a
p->set_winner_id(pairing.winnerId == -2 ? -1 : pairing.winnerId); // reported draw from an unset winner_id on the wire.
if (pairing.winnerId == -1) {
p->set_is_draw(true);
} else if (pairing.winnerId != -2) {
p->set_winner_id(pairing.winnerId);
}
p->set_player1_match_wins(pairing.player1MatchWins); p->set_player1_match_wins(pairing.player1MatchWins);
p->set_player2_match_wins(pairing.player2MatchWins); p->set_player2_match_wins(pairing.player2MatchWins);
} }

View file

@ -4,6 +4,7 @@
#include <QList> #include <QList>
#include <QMap> #include <QMap>
#include <QObject> #include <QObject>
#include <QPointer>
#include <QRecursiveMutex> #include <QRecursiveMutex>
#include <QSet> #include <QSet>
#include <libcockatrice/protocol/pb/event_tournament_state.pb.h> #include <libcockatrice/protocol/pb/event_tournament_state.pb.h>
@ -27,6 +28,9 @@ public:
void addPlayer(int playerId, const QString &playerName); void addPlayer(int playerId, const QString &playerName);
void removePlayer(int playerId); void removePlayer(int playerId);
// Marks an already-starting/started tournament player as dropped: they stop
// being paired and their outstanding unstarted match is awarded as a loss.
void dropPlayer(int playerId);
void startTournament(); void startTournament();
void advanceRound(GameEventStorage &ges); void advanceRound(GameEventStorage &ges);
void recordMatchResult(int playerId1, int playerId2, int winnerId, GameEventStorage &ges); void recordMatchResult(int playerId1, int playerId2, int winnerId, GameEventStorage &ges);
@ -68,6 +72,7 @@ public:
int losses = 0; int losses = 0;
int draws = 0; int draws = 0;
bool deckSubmitted = false; bool deckSubmitted = false;
bool dropped = false;
}; };
struct TournamentPairingData struct TournamentPairingData
@ -82,12 +87,15 @@ public:
}; };
private: private:
Server_Game *parentGame; QPointer<Server_Game> parentGame;
Server_MatchGameFactory *matchGameFactory; Server_MatchGameFactory *matchGameFactory;
mutable QRecursiveMutex tournamentMutex; mutable QRecursiveMutex tournamentMutex;
QMap<int, TournamentPlayerData> players; QMap<int, TournamentPlayerData> players;
QMap<int, DeckList *> submittedDecks; QMap<int, DeckList *> submittedDecks;
QList<TournamentPairingData> currentPairings; QList<TournamentPairingData> currentPairings;
// Players that have already received a bye in a previous round, so no one
// gets more than one bye over the whole tournament.
QSet<int> byeGivenPlayers;
QList<QPair<int, int>> allPreviousPairings; QList<QPair<int, int>> allPreviousPairings;
int currentRound; int currentRound;
int totalRounds; int totalRounds;

View file

@ -11,7 +11,7 @@ Server_GameLifecycleStrategy::StartAction Server_TournamentLifecycleStrategy::on
{ {
// Match sub-games start through the normal flow; only the tournament hub game is // Match sub-games start through the normal flow; only the tournament hub game is
// managed by this lifecycle. // managed by this lifecycle.
if (game->getTournamentParentGame() != nullptr) { if (game->getTournamentParentGame().data() != nullptr) {
return StartAction::ProceedNormal; return StartAction::ProceedNormal;
} }

View file

@ -12,7 +12,10 @@ bool Server_TournamentMatchResultStrategy::onGameFinished(Server_Game *game,
int playing, int playing,
Server_AbstractPlayer *lastPlayer) Server_AbstractPlayer *lastPlayer)
{ {
auto *parentGame = game->getTournamentParentGame(); // The hub game is owned by the room and may be torn down once its host leaves
// and no players remain, while the match sub-games keep running. QPointer keeps
// this link checked so a later-finishing match can't touch freed memory.
auto *parentGame = game->getTournamentParentGame().data();
if (!parentGame || !parentGame->getTournament()) { if (!parentGame || !parentGame->getTournament()) {
return false; return false;
} }