From 8d30ac54f0434906d3bf2c3c36fc1c4d0fc9306b Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Mon, 7 Sep 2026 01:57:35 +0200 Subject: [PATCH] [Game] Prevent spectator duplication when replaying joined events (#7248) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Game] Prevent spectator duplication when replaying joined events The spectator branch of eventJoin emitted spectatorJoined unconditionally even when the spectator was already present (e.g. replayed during a rewind). Guard it like the player branch and eventGameStateChanged, and make PlayerListWidget::addPlayer idempotent as defense in depth. * In resetChatAndPhase() (the rewound() handler), also clear all spectators from both PlayerManager and PlayerListWidget before the replay rebuilds from event 0. The forward replay then re-adds exactly the spectators whose join events fall within the new time range via eventGameStateChanged/eventJoin. --------- Co-authored-by: Lukas Brübach --- cockatrice/src/game/game_event_handler.cpp | 5 +++-- cockatrice/src/game/player/player_manager.cpp | 8 ++++++++ cockatrice/src/game/player/player_manager.h | 3 +++ .../game_graphics/player/player_list_widget.cpp | 16 ++++++++++++++++ .../game_graphics/player/player_list_widget.h | 1 + .../src/interface/widgets/tabs/tab_game.cpp | 4 ++++ 6 files changed, 35 insertions(+), 2 deletions(-) diff --git a/cockatrice/src/game/game_event_handler.cpp b/cockatrice/src/game/game_event_handler.cpp index bc68d4d7c..f146cdbb4 100644 --- a/cockatrice/src/game/game_event_handler.cpp +++ b/cockatrice/src/game/game_event_handler.cpp @@ -430,12 +430,13 @@ void GameEventHandler::eventJoin(const Event_Join &event, int /*eventPlayerId*/, QString playerName = QString::fromStdString(playerInfo.user_info().name()); emit addPlayerToAutoCompleteList(playerName); - if (game->getPlayerManager()->getPlayers().contains(playerId)) { + PlayerManager *playerManager = game->getPlayerManager(); + if (playerManager->getPlayers().contains(playerId) || playerManager->getSpectators().contains(playerId)) { return; } if (playerInfo.spectator()) { - game->getPlayerManager()->addSpectator(playerId, playerInfo); + playerManager->addSpectator(playerId, playerInfo); emit logJoinSpectator(playerName); emit spectatorJoined(playerInfo); } else { diff --git a/cockatrice/src/game/player/player_manager.cpp b/cockatrice/src/game/player/player_manager.cpp index 6772d3ff1..8486efbeb 100644 --- a/cockatrice/src/game/player/player_manager.cpp +++ b/cockatrice/src/game/player/player_manager.cpp @@ -75,6 +75,14 @@ PlayerLogic *PlayerManager::getPlayer(int playerId) const return player; } +void PlayerManager::clearSpectators() +{ + const QList spectatorIds = spectators.keys(); + for (int spectatorId : spectatorIds) { + removeSpectator(spectatorId); + } +} + void PlayerManager::onPlayerConceded(int playerId, bool conceded) { // Everything else cares about this diff --git a/cockatrice/src/game/player/player_manager.h b/cockatrice/src/game/player/player_manager.h index 2f8b87af8..504e65396 100644 --- a/cockatrice/src/game/player/player_manager.h +++ b/cockatrice/src/game/player/player_manager.h @@ -100,6 +100,9 @@ public: emit spectatorRemoved(spectatorId, spectatorInfo); } + /** @brief Remove all spectators, emitting the removal signal for each. */ + void clearSpectators(); + [[nodiscard]] AbstractGame *getGame() const { return game; diff --git a/cockatrice/src/game_graphics/player/player_list_widget.cpp b/cockatrice/src/game_graphics/player/player_list_widget.cpp index 4268e1019..13a077af8 100644 --- a/cockatrice/src/game_graphics/player/player_list_widget.cpp +++ b/cockatrice/src/game_graphics/player/player_list_widget.cpp @@ -92,6 +92,11 @@ void PlayerListWidget::retranslateUi() void PlayerListWidget::addPlayer(const ServerInfo_PlayerProperties &player) { + if (players.contains(player.player_id())) { + updatePlayerProperties(player); + return; + } + QTreeWidgetItem *newPlayer = new PlayerListTWI; players.insert(player.player_id(), newPlayer); updatePlayerProperties(player); @@ -176,6 +181,17 @@ void PlayerListWidget::removePlayer(int playerId) delete takeTopLevelItem(indexOfTopLevelItem(player)); } +void PlayerListWidget::clearSpectators() +{ + const QList playerIds = players.keys(); + for (int playerId : playerIds) { + QTreeWidgetItem *player = players.value(playerId, 0); + if (player && !player->data(1, Qt::UserRole).toBool()) { + removePlayer(playerId); + } + } +} + void PlayerListWidget::setActivePlayer(int playerId) { QMapIterator i(players); diff --git a/cockatrice/src/game_graphics/player/player_list_widget.h b/cockatrice/src/game_graphics/player/player_list_widget.h index a53cfa989..f2f0be5fd 100644 --- a/cockatrice/src/game_graphics/player/player_list_widget.h +++ b/cockatrice/src/game_graphics/player/player_list_widget.h @@ -66,6 +66,7 @@ public slots: void addPlayer(const ServerInfo_PlayerProperties &player); void removePlayer(int playerId); void updatePlayerProperties(const ServerInfo_PlayerProperties &prop, int playerId = -1); + void clearSpectators(); }; #endif diff --git a/cockatrice/src/interface/widgets/tabs/tab_game.cpp b/cockatrice/src/interface/widgets/tabs/tab_game.cpp index 196ea4526..035ab1004 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_game.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_game.cpp @@ -266,6 +266,10 @@ void TabGame::resetChatAndPhase() // reset phase markers game->getGameState()->setCurrentPhase(-1); + + // reset spectator state so the replay can rebuild it from the start + game->getPlayerManager()->clearSpectators(); + playerListWidget->clearSpectators(); } void TabGame::emitUserEvent()