jeirmeister/Cockatrice
1
0
Fork 0
mirror of https://github.com/Cockatrice/Cockatrice.git synced 2026-11-13 05:20:00 -08:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

[Server] Keep player ids stable across seat shuffles

Browse source
This commit is contained in:
Lukas Brübach 2026-10-11 14:58:26 +02:00
parent cc9847c33b
commit 00c0b2c96b
3 changed files with 129 additions and 74 deletions
Download patch file Download diff file Expand all files Collapse all files

125
libcockatrice_network/libcockatrice/network/server/remote/game/server_game.cpp
View file

@ -308,7 +308,28 @@ void Server_Game::createGameStateChangedEvent(Event_GameStateChanged *event,
event->set_game_started(false);
}
for (Server_AbstractParticipant *participant : participants.values()) {
// Emit the player list in seat order so every client (including old ones
// that ignore Event_PlayerOrderChanged) sees the current seating. Player
// ids are stable; only this ordering changes on shuffle/reorder.
QList<Server_AbstractParticipant *> ordered;
ordered.reserve(participants.size());
for (int id : seatOrder) {
auto *participant = participants.value(id, nullptr);
if (participant && !participant->isSpectator() && !ordered.contains(participant)) {
ordered.append(participant);
}
}
for (auto *participant : participants.values()) {
if (!participant->isSpectator() && !ordered.contains(participant)) {
ordered.append(participant);
}
}
for (auto *participant : participants.values()) {
if (participant->isSpectator()) {
ordered.append(participant);
}
}
for (Server_AbstractParticipant *participant : ordered) {
participant->getInfo(event->add_player_list(), recipient, omniscient, withUserInfo);
}
@ -448,14 +469,24 @@ void Server_Game::shufflePlayerSeats()
{
QMutexLocker locker(&gameMutex);
// Split participants into players and spectators. Spectators keep their seats.
// Collect players in current seat order (fall back to id order for seats
// predating seatOrder). Spectators keep their seats.
QList<Server_AbstractParticipant *> players;
for (int id : seatOrder) {
auto *participant = participants.value(id, nullptr);
if (participant && !participant->isSpectator()) {
players.append(participant);
}
}
for (auto *participant : participants.values()) {
if (!participant->isSpectator() && !players.contains(participant)) {
players.append(participant);
}
}
QList<Server_AbstractParticipant *> spectators;
for (auto *participant : participants.values()) {
if (participant->isSpectator()) {
spectators.append(participant);
} else {
players.append(participant);
}
}
@ -473,14 +504,24 @@ void Server_Game::reorderPlayerSeats(const QList<QString> &orderedNames)
{
QMutexLocker locker(&gameMutex);
// Split participants into players and spectators. Spectators keep their seats.
// Collect players in current seat order (fall back to id order for seats
// predating seatOrder). Spectators keep their seats.
QList<Server_AbstractParticipant *> players;
for (int id : seatOrder) {
auto *participant = participants.value(id, nullptr);
if (participant && !participant->isSpectator()) {
players.append(participant);
}
}
for (auto *participant : participants.values()) {
if (!participant->isSpectator() && !players.contains(participant)) {
players.append(participant);
}
}
QList<Server_AbstractParticipant *> spectators;
for (auto *participant : participants.values()) {
if (participant->isSpectator()) {
spectators.append(participant);
} else {
players.append(participant);
}
}
@ -510,36 +551,17 @@ void Server_Game::reseatPlayers(const QList<Server_AbstractParticipant *> &order
{
QMutexLocker locker(&gameMutex);
// Reassign fresh, unique player ids following the given order.
QMap<int, Server_AbstractParticipant *> newParticipants;
int newHostId = -1;
// Keep player ids stable and only reorder the seating/turn order. Changing
// ids would break old clients (stale localPlayerId from Event_GameJoined
// duplicates their own seat as an opponent), invalidate the home-server
// per-user game/seat mapping used for ISL command routing, and orphan the
// host id. All of those maps stay valid because ids never change here.
// The current host therefore keeps its id and needs no re-key tracking.
seatOrder.clear();
for (auto *participant : orderedParticipants) {
const int newPlayerId = nextPlayerId++;
const int oldPlayerId = participant->getPlayerId();
participant->setPlayerId(newPlayerId);
newParticipants.insert(newPlayerId, participant);
// Keep the user interface's per-user game/seat mapping in sync so that
// incoming game commands are routed to the participant under its new id.
if (participant->getUserInterface()) {
participant->getUserInterface()->playerAddedToGame(gameId, room->getId(), newPlayerId);
if (!participant->isSpectator()) {
seatOrder.append(participant->getPlayerId());
}
if (!participant->isSpectator() && (participant->getUserInfo()->user_level() & ServerInfo_User::IsRegistered)) {
const QString playerName = QString::fromStdString(participant->getUserInfo()->name());
room->getServer()->removePersistentPlayer(playerName, room->getId(), gameId, oldPlayerId);
room->getServer()->addPersistentPlayer(playerName, room->getId(), gameId, newPlayerId);
}
if (participant->getUserInfo()->name() == creatorInfo->name()) {
newHostId = newPlayerId;
}
}
participants = newParticipants;
if (newHostId != -1) {
hostId = newHostId;
sendGameEventContainer(prepareGameEvent(Event_GameHostChanged(), hostId));
}
// Broadcast the new seating order so that every client can display it.
@ -683,6 +705,7 @@ void Server_Game::addPlayer(Server_AbstractUserInterface *userInterface,
allSpectatorsEver.insert(playerName);
} else {
allPlayersEver.insert(playerName);
seatOrder.append(newParticipant->getPlayerId());
// if the original creator of the game joins, give them host status back
//! \todo Transferring host to spectators has side effects.
@ -712,6 +735,7 @@ void Server_Game::removeParticipant(Server_AbstractParticipant *participant, Eve
room->getServer()->removePersistentPlayer(QString::fromStdString(participant->getUserInfo()->name()), room->getId(),
gameId, participant->getPlayerId());
participants.remove(participant->getPlayerId());
seatOrder.removeAll(participant->getPlayerId());
bool spectator = participant->isSpectator();
GameEventStorage ges;
@ -731,10 +755,20 @@ void Server_Game::removeParticipant(Server_AbstractParticipant *participant, Eve
participant->prepareDestroy();
if (playerHost) {
// Pass host to the next player in seat order so the game always keeps
// a host while players remain.
int newHostId = -1;
for (auto *otherPlayer : getPlayers().values()) {
newHostId = otherPlayer->getPlayerId();
break;
for (int id : seatOrder) {
if (participants.contains(id)) {
newHostId = id;
break;
}
}
if (newHostId == -1) {
for (auto *otherPlayer : getPlayers().values()) {
newHostId = otherPlayer->getPlayerId();
break;
}
}
if (newHostId != -1) {
hostId = newHostId;
@ -891,7 +925,20 @@ void Server_Game::nextTurn()
}
auto players = getPlayers();
const QList<int> keys = players.keys();
// Advance in seat order, not id order, so shuffles/reorders change the
// turn sequence while ids stay stable.
QList<int> keys;
keys.reserve(players.size());
for (int id : seatOrder) {
if (players.contains(id)) {
keys.append(id);
}
}
for (int id : players.keys()) {
if (!keys.contains(id)) {
keys.append(id);
}
}
int listPos = -1;
if (activePlayer != -1) {
listPos = keys.indexOf(activePlayer);

9
libcockatrice_network/libcockatrice/network/server/remote/game/server_game.h
View file

@ -74,6 +74,11 @@ private:
int hostId;
ServerInfo_User *creatorInfo;
QMap<int, Server_AbstractParticipant *> participants;
// Seating/turn order as an ordered list of player ids. Player ids themselves
// are stable for the lifetime of the participant; shuffling or reordering
// only permutes this list so old clients, ISL routes and persistent
// references (all keyed by player id) keep working.
QList<int> seatOrder;
QSet<QString> allPlayersEver, allSpectatorsEver;
bool gameStarted;
bool gameClosed;
@ -151,6 +156,10 @@ public:
{
return participants;
}
QList<int> getSeatOrder() const
{
return seatOrder;
}
int getGameId() const
{
return gameId;

69
tests/movecard_tests/player_order_test.cpp
View file

@ -1,4 +1,5 @@
#include "game/game_config.h"
#include "game/role_info.h"
#include "game/server_abstract_participant.h"
#include "game/server_game.h"
#include "server_abstractuserinterface.h"
@ -70,19 +71,18 @@ namespace
QStringList participantNamesInOrder(const Server_Game &game)
{
QStringList names;
for (const auto *participant : game.getParticipants()) {
names.append(QString::fromStdString(participant->getUserInfo()->name()));
for (int id : game.getSeatOrder()) {
const auto *participant = game.getParticipants().value(id, nullptr);
if (participant) {
names.append(QString::fromStdString(participant->getUserInfo()->name()));
}
}
return names;
}
QList<int> participantIdsInOrder(const Server_Game &game)
{
QList<int> ids;
for (int id : game.getParticipants().keys()) {
ids.append(id);
}
return ids;
return game.getSeatOrder();
}
} // namespace
@ -129,9 +129,9 @@ TEST_F(PlayerOrderTest, JoinOrderIsSeatOrder)
Server_Game game(config, &room);
ResponseContainer rc(0);
game.addPlayer(&uiA, rc, false, false);
game.addPlayer(&uiB, rc, false, false);
game.addPlayer(&uiC, rc, false, false);
game.addPlayer(&uiA, rc, RoleInfo{}, false);
game.addPlayer(&uiB, rc, RoleInfo{}, false);
game.addPlayer(&uiC, rc, RoleInfo{}, false);
EXPECT_EQ(participantNamesInOrder(game), QStringList({"A", "B", "C"}));
EXPECT_EQ(participantIdsInOrder(game), QList<int>({0, 1, 2}));
@ -164,27 +164,25 @@ TEST_F(PlayerOrderTest, ReorderPlayerSeats)
Server_Game game(config, &room);
ResponseContainer rc(0);
game.addPlayer(&uiA, rc, false, false);
game.addPlayer(&uiB, rc, false, false);
game.addPlayer(&uiC, rc, false, false);
game.addPlayer(&uiA, rc, RoleInfo{}, false);
game.addPlayer(&uiB, rc, RoleInfo{}, false);
game.addPlayer(&uiC, rc, RoleInfo{}, false);
game.reorderPlayerSeats({"C", "A", "B"});
const QList<int> ids = participantIdsInOrder(game);
// player ids are stable; only the seat order changes
EXPECT_EQ(participantNamesInOrder(game), QStringList({"C", "A", "B"}));
EXPECT_EQ(ids.size(), 3);
EXPECT_GE(ids.at(0), 3); // re-seated players get fresh ids
EXPECT_LT(ids.at(0), ids.at(1));
EXPECT_LT(ids.at(1), ids.at(2));
EXPECT_EQ(participantIdsInOrder(game), QList<int>({2, 0, 1}));
EXPECT_EQ(game.getParticipants().size(), 3);
// the host seat follows the creator
EXPECT_EQ(game.getHostId(), ids.at(1));
// the host keeps its seat: creator A is still in the game, so host stays 0
EXPECT_EQ(game.getHostId(), 0);
// every user interface's game list is updated to the new player id
// no re-keying means every mapping keyed by player id stays valid
const int gameId = game.getGameId();
EXPECT_EQ(uiA.getGames().value(gameId).second, ids.at(1));
EXPECT_EQ(uiB.getGames().value(gameId).second, ids.at(2));
EXPECT_EQ(uiC.getGames().value(gameId).second, ids.at(0));
EXPECT_EQ(uiA.getGames().value(gameId).second, 0);
EXPECT_EQ(uiB.getGames().value(gameId).second, 1);
EXPECT_EQ(uiC.getGames().value(gameId).second, 2);
}
TEST_F(PlayerOrderTest, ReorderPlayerSeatsKeepsPlayerCount)
@ -212,16 +210,17 @@ TEST_F(PlayerOrderTest, ReorderPlayerSeatsKeepsPlayerCount)
Server_Game game(config, &room);
ResponseContainer rc(0);
game.addPlayer(&uiA, rc, false, false);
game.addPlayer(&uiB, rc, false, false);
game.addPlayer(&uiC, rc, false, false);
game.addPlayer(&uiA, rc, RoleInfo{}, false);
game.addPlayer(&uiB, rc, RoleInfo{}, false);
game.addPlayer(&uiC, rc, RoleInfo{}, false);
// listing only a subset appends the remaining players in their current order
game.reorderPlayerSeats({"C"});
EXPECT_EQ(participantNamesInOrder(game), QStringList({"C", "A", "B"}));
EXPECT_EQ(participantIdsInOrder(game), QList<int>({2, 0, 1}));
EXPECT_EQ(game.getPlayerCount(), 3);
EXPECT_EQ(game.getHostId(), participantIdsInOrder(game).at(1));
EXPECT_EQ(game.getHostId(), 0);
}
TEST_F(PlayerOrderTest, ShufflePlayerSeatsIsAPermutation)
@ -249,9 +248,9 @@ TEST_F(PlayerOrderTest, ShufflePlayerSeatsIsAPermutation)
Server_Game game(config, &room);
ResponseContainer rc(0);
game.addPlayer(&uiA, rc, false, false);
game.addPlayer(&uiB, rc, false, false);
game.addPlayer(&uiC, rc, false, false);
game.addPlayer(&uiA, rc, RoleInfo{}, false);
game.addPlayer(&uiB, rc, RoleInfo{}, false);
game.addPlayer(&uiC, rc, RoleInfo{}, false);
// fisher-yates on [A, B, C]: rand(0, 2) -> 1 (take B), rand(0, 1) -> 0 (take A), rand(0, 0) -> 0 (take C)
auto *scriptedRng = new ScriptedRng;
@ -261,9 +260,9 @@ TEST_F(PlayerOrderTest, ShufflePlayerSeatsIsAPermutation)
game.shufflePlayerSeats();
EXPECT_EQ(participantNamesInOrder(game), QStringList({"B", "A", "C"}));
const QList<int> ids = participantIdsInOrder(game);
EXPECT_EQ(ids.size(), 3);
QSet<int> idSet(ids.cbegin(), ids.cend());
EXPECT_EQ(participantIdsInOrder(game), QList<int>({1, 0, 2}));
QSet<int> idSet(participantIdsInOrder(game).cbegin(), participantIdsInOrder(game).cend());
EXPECT_EQ(idSet.size(), 3); // ids are unique
EXPECT_EQ(game.getHostId(), ids.at(1));
// ids are stable, so the host (creator A, id 0) is unchanged
EXPECT_EQ(game.getHostId(), 0);
}