aly/servatrice
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Enable buttons for current game when receiving server response (#4737)

Browse source 
* Enable buttons for current game when receiving server response

Previously, upon joining a game, we were unconditionally re-enabling the
"Join" button in the lobby, even if it was not enabled in the first
place, causing #4698. This could also lead to issues where if the user
selects a different game after joining (which they can do in case of
e.g. network connectivity issues), the "Join as spectator" button could
get incorrectly disabled.

This fixes #4698 by re-enabling the buttons based on the state of the
currently selected game at the time the response is received. This also
avoids inconsistencies if a different game has been selected in between
joining and receiving a response from the server.

* Typo: enable gameButton in enableButtons

The "create game" button was incorrectly being disabled in enableButtons
whereas (as the name indicates) it should have been enabled

* Remove misleading comment about race conditions
This commit is contained in:
Basile Clement 2023-02-06 12:49:45 +00:00 committed by GitHub
parent 44d1ab348b
commit 00c9efe541
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 44 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

55
cockatrice/src/gameselector.cpp
View file

@ -198,9 +198,19 @@ void GameSelector::actCreate()
void GameSelector::checkResponse(const Response &response)
{
if (createButton)
createButton->setEnabled(true);
joinButton->setEnabled(true);
// NB: We re-enable buttons for the currently selected game, which may not
// be the same game as the one for which we are processing a response.
// This could lead to situations where we join a game, select a different
// game, join the new game, then receive the response for the original
// join, which would re-activate the buttons for the second game too soon.
// However, that is better than doing things the other ways, because then
// the response to the first game could lock us out of join/join as
// spectator for the second game!
//
// Ideally we should have a way to figure out if the current game is the
// same as the one we are getting a response for, but it's probably not
// worth the trouble.
enableButtons();
switch (response.response_code()) {
case Response::RespNotInRoom:
@ -229,9 +239,6 @@ void GameSelector::checkResponse(const Response &response)
break;
default:;
}
if (response.response_code() != Response::RespSpectatorsNotAllowed)
spectateButton->setEnabled(true);
}
void GameSelector::actJoin()
@ -270,12 +277,39 @@ void GameSelector::actJoin()
connect(pend, SIGNAL(finished(Response, CommandContainer, QVariant)), this, SLOT(checkResponse(Response)));
r->sendRoomCommand(pend);
disableButtons();
}
void GameSelector::disableButtons()
{
if (createButton)
createButton->setEnabled(false);
joinButton->setEnabled(false);
spectateButton->setEnabled(false);
}
void GameSelector::enableButtons()
{
if (createButton)
createButton->setEnabled(true);
// Enable buttons for the currently selected game
enableButtonsForIndex(gameListView->currentIndex());
}
void GameSelector::enableButtonsForIndex(const QModelIndex &current)
{
if (!current.isValid())
return;
const ServerInfo_Game &game = gameListModel->getGame(current.data(Qt::UserRole).toInt());
bool overrideRestrictions = !tabSupervisor->getAdminLocked();
spectateButton->setEnabled(game.spectators_allowed() || overrideRestrictions);
joinButton->setEnabled(game.player_count() < game.max_players() || overrideRestrictions);
}
void GameSelector::retranslateUi()
{
filterButton->setText(tr("&Filter games"));
@ -296,14 +330,7 @@ void GameSelector::processGameInfo(const ServerInfo_Game &info)
void GameSelector::actSelectedGameChanged(const QModelIndex &current, const QModelIndex & /* previous */)
{
if (!current.isValid())
return;
const ServerInfo_Game &game = gameListModel->getGame(current.data(Qt::UserRole).toInt());
bool overrideRestrictions = !tabSupervisor->getAdminLocked();
spectateButton->setEnabled(game.spectators_allowed() || overrideRestrictions);
joinButton->setEnabled(game.player_count() < game.max_players() || overrideRestrictions);
enableButtonsForIndex(current);
}
void GameSelector::updateTitle()

3
cockatrice/src/gameselector.h
View file

@ -49,6 +49,9 @@ private:
GameTypeMap gameTypeMap;
void updateTitle();
void disableButtons();
void enableButtons();
void enableButtonsForIndex(const QModelIndex &current);
public:
GameSelector(AbstractClient *_client,