[Client] Address tournament UI review feedback

- Drop the in-game tournament-settings button: Command_TournamentSettingsSelect
  has no server handler (RespContextError), the host-ness decision was evaluated
  once at tab construction, and the button never received a label on first show.
  The dialog can be re-added together with a working Server_Player override.
- Pass PlayerManager::isSpectator() to TournamentWidget instead of relying on
  localPlayerId == -1, which never holds for spectators.
- Report games per match as the total series length (Best of %1) to match the
  DlgTournamentSettings hint and the server's use of the value; the previous
  gamesPerMatch * 2 - 1 mislabeled Bo3 as Best of 5.
- Block signals around read-only tournamentCheckBox setChecked so the disabled
  settings button is not re-enabled, and seed Command_CreateGame through
  mutable_tournament_settings() (field 15 became the settings message).
- return after closing a sub-game that routed to its parent tab.
This commit is contained in:
Lukas Brübach 2026-09-02 11:35:23 +02:00 committed by GitHub
parent c5fd10087a
commit 55099ab82c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 21 additions and 55 deletions

View file

@ -15,6 +15,7 @@
#include <QPushButton> #include <QPushButton>
#include <QRadioButton> #include <QRadioButton>
#include <QSet> #include <QSet>
#include <QSignalBlocker>
#include <QSpinBox> #include <QSpinBox>
#include <libcockatrice/protocol/pb/serverinfo_game.pb.h> #include <libcockatrice/protocol/pb/serverinfo_game.pb.h>
#include <libcockatrice/protocol/pending_command.h> #include <libcockatrice/protocol/pending_command.h>
@ -233,7 +234,10 @@ DlgCreateGame::DlgCreateGame(const ServerInfo_Game &gameInfo, const QMap<int, QS
spectatorsCanTalkCheckBox->setChecked(gameInfo.spectators_can_chat()); spectatorsCanTalkCheckBox->setChecked(gameInfo.spectators_can_chat());
spectatorsSeeEverythingCheckBox->setChecked(gameInfo.spectators_omniscient()); spectatorsSeeEverythingCheckBox->setChecked(gameInfo.spectators_omniscient());
shareDecklistsOnLoadCheckBox->setChecked(gameInfo.share_decklists_on_load()); shareDecklistsOnLoadCheckBox->setChecked(gameInfo.share_decklists_on_load());
tournamentCheckBox->setChecked(gameInfo.is_tournament()); {
const QSignalBlocker blocker(tournamentCheckBox);
tournamentCheckBox->setChecked(gameInfo.is_tournament());
}
QSet<int> types; QSet<int> types;
for (int i = 0; i < gameInfo.game_types_size(); ++i) { for (int i = 0; i < gameInfo.game_types_size(); ++i) {
@ -305,7 +309,7 @@ void DlgCreateGame::actOK()
cmd.set_share_decklists_on_load(shareDecklistsOnLoadCheckBox->isChecked()); cmd.set_share_decklists_on_load(shareDecklistsOnLoadCheckBox->isChecked());
cmd.set_is_tournament(tournamentCheckBox->isChecked()); cmd.set_is_tournament(tournamentCheckBox->isChecked());
if (tournamentCheckBox->isChecked()) { if (tournamentCheckBox->isChecked()) {
cmd.set_games_per_match(tournamentSettings.gamesPerMatch); cmd.mutable_tournament_settings()->set_games_per_match(tournamentSettings.gamesPerMatch);
} }
auto _gameTypes = QString(); auto _gameTypes = QString();

View file

@ -75,6 +75,11 @@ void TournamentWidget::setLocalPlayerId(int playerId)
localPlayerId = playerId; localPlayerId = playerId;
} }
void TournamentWidget::setIsSpectator(bool spectator)
{
isSpectator = spectator;
}
QString TournamentWidget::getPlayerName(const Event_TournamentState &state, int playerId) const QString TournamentWidget::getPlayerName(const Event_TournamentState &state, int playerId) const
{ {
for (int i = 0; i < state.players_size(); ++i) { for (int i = 0; i < state.players_size(); ++i) {
@ -93,15 +98,15 @@ void TournamentWidget::updateTournamentState(const Event_TournamentState &state)
QString statusText; QString statusText;
switch (state.phase()) { switch (state.phase()) {
case Event_TournamentState::PHASE_DECK_BUILDING: case Event_TournamentState::PHASE_DECK_BUILDING:
statusText = gamesPerMatch > 1 ? tr("Tournament - Deck building (Best of %1)").arg(gamesPerMatch * 2 - 1) statusText = gamesPerMatch > 1 ? tr("Tournament - Deck building (Best of %1)").arg(gamesPerMatch)
: tr("Tournament - Deck building"); : tr("Tournament - Deck building");
break; break;
case Event_TournamentState::PHASE_PLAYING: case Event_TournamentState::PHASE_PLAYING:
statusText = gamesPerMatch > 1 ? tr("Tournament - Playing (Best of %1)").arg(gamesPerMatch * 2 - 1) statusText = gamesPerMatch > 1 ? tr("Tournament - Playing (Best of %1)").arg(gamesPerMatch)
: tr("Tournament - Playing"); : tr("Tournament - Playing");
break; break;
case Event_TournamentState::PHASE_FINISHED: case Event_TournamentState::PHASE_FINISHED:
statusText = gamesPerMatch > 1 ? tr("Tournament - Finished (Best of %1)").arg(gamesPerMatch * 2 - 1) statusText = gamesPerMatch > 1 ? tr("Tournament - Finished (Best of %1)").arg(gamesPerMatch)
: tr("Tournament - Finished"); : tr("Tournament - Finished");
break; break;
default: default:
@ -134,7 +139,7 @@ void TournamentWidget::updateOpenMatchButton()
openMatchButton->setEnabled(true); openMatchButton->setEnabled(true);
openMatchButton->setText(tr("Open match game")); openMatchButton->setText(tr("Open match game"));
openMatchButton->setToolTip(tr("Switch to your current match game")); openMatchButton->setToolTip(tr("Switch to your current match game"));
} else if (localPlayerId == -1 && hasAnyLivePairing) { } else if (isSpectator && hasAnyLivePairing) {
// Spectators have no pairing of their own but may watch any running match. // Spectators have no pairing of their own but may watch any running match.
openMatchButton->setEnabled(true); openMatchButton->setEnabled(true);
openMatchButton->setText(tr("Spectate live match")); openMatchButton->setText(tr("Spectate live match"));

View file

@ -27,6 +27,7 @@ public:
void updateTournamentState(const Event_TournamentState &state); void updateTournamentState(const Event_TournamentState &state);
void setLocalPlayerId(int playerId); void setLocalPlayerId(int playerId);
void setIsSpectator(bool spectator);
void retranslateUi(); void retranslateUi();
signals: signals:
@ -47,6 +48,7 @@ private:
QPushButton *openMatchButton; QPushButton *openMatchButton;
int localPlayerId = -1; int localPlayerId = -1;
bool isSpectator = false;
int currentGameId = -1; int currentGameId = -1;
bool hasOwnLivePairing = false; bool hasOwnLivePairing = false;
bool hasAnyLivePairing = false; bool hasAnyLivePairing = false;

View file

@ -949,12 +949,9 @@ bool TabGame::switchToGameTab(int gameId)
void TabGame::closeGame() void TabGame::closeGame()
{ {
int parentId = game->getGameMetaInfo()->parentGameId(); int parentId = game->getGameMetaInfo()->parentGameId();
if (parentId >= 0) { if (parentId >= 0 && switchToGameTab(parentId)) {
if (switchToGameTab(parentId)) { close();
close(); return;
}
// If the parent tab is gone, fall through to the normal leave-game
// flow instead of stranding the user on a dead sub-game tab.
} }
gameMenu->clear(); gameMenu->clear();

View file

@ -1,8 +1,6 @@
#include "tournament_tab_game_extension.h" #include "tournament_tab_game_extension.h"
#include "../../../game/game_event_handler.h" #include "../../../game/game_event_handler.h"
#include "../../../game/player/player_logic.h"
#include "../../widgets/dialogs/dlg_tournament_settings.h"
#include "../../widgets/draft/tournament_widget.h" #include "../../widgets/draft/tournament_widget.h"
#include "tab_game.h" #include "tab_game.h"
#include "tab_supervisor.h" #include "tab_supervisor.h"
@ -12,7 +10,6 @@
#include <QPushButton> #include <QPushButton>
#include <QStackedWidget> #include <QStackedWidget>
#include <QVBoxLayout> #include <QVBoxLayout>
#include <libcockatrice/protocol/pb/command_tournament.pb.h>
#include <libcockatrice/protocol/pb/event_tournament_state.pb.h> #include <libcockatrice/protocol/pb/event_tournament_state.pb.h>
TournamentTabGameExtension::TournamentTabGameExtension(TabGame *parent) : QObject(parent), tabGame(parent) TournamentTabGameExtension::TournamentTabGameExtension(TabGame *parent) : QObject(parent), tabGame(parent)
@ -29,6 +26,7 @@ TournamentTabGameExtension::TournamentTabGameExtension(TabGame *parent) : QObjec
tournamentWidget = new TournamentWidget(tournamentOverviewWidget); tournamentWidget = new TournamentWidget(tournamentOverviewWidget);
tournamentWidget->setLocalPlayerId(parent->getGame()->getPlayerManager()->getLocalPlayerId()); tournamentWidget->setLocalPlayerId(parent->getGame()->getPlayerManager()->getLocalPlayerId());
tournamentWidget->setIsSpectator(parent->getGame()->getPlayerManager()->isSpectator());
overviewLayout->addWidget(tournamentWidget); overviewLayout->addWidget(tournamentWidget);
parent->getMainWidget()->addWidget(tournamentOverviewWidget); parent->getMainWidget()->addWidget(tournamentOverviewWidget);
@ -44,12 +42,6 @@ TournamentTabGameExtension::TournamentTabGameExtension(TabGame *parent) : QObjec
connectSignals(); connectSignals();
} }
bool TournamentTabGameExtension::isLocalPlayerHost() const
{
return tabGame->getGame()->getPlayerManager()->getLocalPlayerId() ==
tabGame->getGame()->getGameState()->getHostId();
}
void TournamentTabGameExtension::connectSignals() void TournamentTabGameExtension::connectSignals()
{ {
auto *handler = tabGame->getGame()->getGameEventHandler(); auto *handler = tabGame->getGame()->getGameEventHandler();
@ -68,11 +60,6 @@ void TournamentTabGameExtension::initializeTournamentMode()
auto *deckLayout = tabGame->getDeckViewContainerLayout(); auto *deckLayout = tabGame->getDeckViewContainerLayout();
int index = 0; int index = 0;
deckLayout->insertWidget(index++, deckViewStatusLabel); deckLayout->insertWidget(index++, deckViewStatusLabel);
if (isLocalPlayerHost()) {
settingsButton = new QPushButton(tabGame->getDeckViewContainerWidget());
connect(settingsButton, &QPushButton::clicked, this, &TournamentTabGameExtension::showTournamentSettingsDialog);
deckLayout->insertWidget(index++, settingsButton);
}
deckLayout->insertWidget(index++, standingsButton); deckLayout->insertWidget(index++, standingsButton);
deckLayout->insertSpacing(index, 4); deckLayout->insertSpacing(index, 4);
} }
@ -81,9 +68,6 @@ void TournamentTabGameExtension::retranslateUi()
{ {
backToGameButton->setText(tr("Back to game view")); backToGameButton->setText(tr("Back to game view"));
standingsButton->setText(tr("Tournament standings")); standingsButton->setText(tr("Tournament standings"));
if (settingsButton) {
settingsButton->setText(tr("Tournament Settings"));
}
tournamentWidget->retranslateUi(); tournamentWidget->retranslateUi();
} }
@ -126,9 +110,6 @@ void TournamentTabGameExtension::updateNavigationButtons(const Event_TournamentS
bool showStandings = bool showStandings =
state.phase() == Event_TournamentState::PHASE_PLAYING || state.phase() == Event_TournamentState::PHASE_FINISHED; state.phase() == Event_TournamentState::PHASE_PLAYING || state.phase() == Event_TournamentState::PHASE_FINISHED;
standingsButton->setVisible(showStandings); standingsButton->setVisible(showStandings);
if (settingsButton) {
settingsButton->setVisible(state.phase() == Event_TournamentState::PHASE_DECK_BUILDING);
}
} }
void TournamentTabGameExtension::onTournamentStateChanged(const Event_TournamentState &state) void TournamentTabGameExtension::onTournamentStateChanged(const Event_TournamentState &state)
@ -168,26 +149,6 @@ void TournamentTabGameExtension::showDeckViewPage()
} }
} }
void TournamentTabGameExtension::showTournamentSettingsDialog()
{
DlgTournamentSettings dlg(tabGame);
if (dlg.exec() != QDialog::Accepted) {
return;
}
DlgTournamentSettingsResult result = dlg.getResult();
PlayerLogic *localPlayer = tabGame->getGame()->getPlayerManager()->getActiveLocalPlayer(-1);
if (!localPlayer) {
TabSupervisor::actShowPopup(tr("You are not an active player in this game."));
return;
}
Command_TournamentSettingsSelect cmd;
cmd.mutable_settings()->set_games_per_match(result.gamesPerMatch);
tabGame->getGame()->getGameEventHandler()->sendGameCommand(cmd, localPlayer->getPlayerInfo()->getId());
}
void TournamentTabGameExtension::openMatchGame(int gameId) void TournamentTabGameExtension::openMatchGame(int gameId)
{ {
if (!tabGame || gameId <= 0) { if (!tabGame || gameId <= 0) {

View file

@ -32,14 +32,12 @@ public:
private slots: private slots:
void onTournamentStateChanged(const Event_TournamentState &state); void onTournamentStateChanged(const Event_TournamentState &state);
void showTournamentSettingsDialog();
void showOverviewPage(); void showOverviewPage();
void showDeckViewPage(); void showDeckViewPage();
void openMatchGame(int gameId); void openMatchGame(int gameId);
private: private:
void connectSignals(); void connectSignals();
[[nodiscard]] bool isLocalPlayerHost() const;
void updateDeckViewStrip(const Event_TournamentState &state); void updateDeckViewStrip(const Event_TournamentState &state);
void updateNavigationButtons(const Event_TournamentState &state); void updateNavigationButtons(const Event_TournamentState &state);
@ -48,7 +46,6 @@ private:
QWidget *tournamentOverviewWidget = nullptr; QWidget *tournamentOverviewWidget = nullptr;
QPushButton *backToGameButton = nullptr; QPushButton *backToGameButton = nullptr;
QPushButton *standingsButton = nullptr; QPushButton *standingsButton = nullptr;
QPushButton *settingsButton = nullptr;
QLabel *deckViewStatusLabel = nullptr; QLabel *deckViewStatusLabel = nullptr;
Event_TournamentState::TournamentPhase lastKnownPhase = Event_TournamentState::PHASE_DECK_BUILDING; Event_TournamentState::TournamentPhase lastKnownPhase = Event_TournamentState::PHASE_DECK_BUILDING;
bool hasLastKnownPhase = false; bool hasLastKnownPhase = false;