diff --git a/cockatrice/src/interface/widgets/tabs/tab_server.cpp b/cockatrice/src/interface/widgets/tabs/tab_server.cpp index 13a77e957..fca32094c 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_server.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_server.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -185,25 +186,37 @@ void TabServer::processServerMessageEvent(const Event_ServerMessage &event) void TabServer::joinRoom(int id, bool setCurrent) { TabRoom *room = tabSupervisor->getRoomTabs().value(id); - if (!room) { - Command_JoinRoom cmd; - cmd.set_room_id(id); - - PendingCommand *pend = client->prepareSessionCommand(cmd); - pend->setExtraData(setCurrent); - connect(pend, &PendingCommand::finished, this, - [this, id](const Response &r, const CommandContainer &c, const QVariant &v) { - joinRoomFinished(r, c, v, id); - }); - - client->sendCommand(pend); - + if (room) { + if (setCurrent) { + tabSupervisor->setCurrentWidget((QWidget *)room); + } return; } - if (setCurrent) { - tabSupervisor->setCurrentWidget((QWidget *)room); + auto pendingIt = pendingRoomJoins.find(id); + if (pendingIt != pendingRoomJoins.end()) { + // A join for this room is already in flight: the room tab opens when its response + // arrives. Fold the new request into the pending one so that, for example, clicking + // a room the selector is auto-joining does not send a second Command_JoinRoom - the + // server would reject that duplicate with RespContextError. + if (setCurrent) { + pendingIt.value() = true; + } + return; } + + pendingRoomJoins.insert(id, setCurrent); + + Command_JoinRoom cmd; + cmd.set_room_id(id); + + PendingCommand *pend = client->prepareSessionCommand(cmd); + pend->setExtraData(setCurrent); + connect( + pend, &PendingCommand::finished, this, + [this, id](const Response &r, const CommandContainer &c, const QVariant &v) { joinRoomFinished(r, c, v, id); }); + + client->sendCommand(pend); } void TabServer::joinRoomFinished(const Response &r, @@ -211,34 +224,72 @@ void TabServer::joinRoomFinished(const Response &r, const QVariant &extraData, int roomId) { + const bool setCurrent = pendingRoomJoins.value(roomId, extraData.toBool()); + pendingRoomJoins.remove(roomId); + const bool healedJoin = healedRoomJoins.contains(roomId); + healedRoomJoins.remove(roomId); + switch (r.response_code()) { case Response::RespOk: break; case Response::RespNameNotFound: - QMessageBox::critical(this, tr("Error"), - tr("Failed to join the server room: it doesn't exist on the server.")); + if (setCurrent) { + QMessageBox::critical(this, tr("Error"), + tr("Failed to join the server room: it doesn't exist on the server.")); + } emit roomJoinFailed(roomId); return; case Response::RespContextError: - QMessageBox::critical( - this, tr("Error"), - tr("The server thinks you are in the server room but your client is unable to display it. " - "Try restarting your client.")); - emit roomJoinFailed(roomId); + if (healedJoin) { + // The rejoin below was already answered and the server still rejects the join, so + // the stale-membership heal cannot help: surface the error. The guard was already + // released above so a later user-initiated join may try a fresh heal. + if (setCurrent) { + QMessageBox::critical( + this, tr("Error"), + tr("The server thinks you are in the server room but your client is unable to display it. " + "Try restarting your client.")); + } + emit roomJoinFailed(roomId); + return; + } + // The server already had us registered in the room even though no tab was open, + // usually because two join attempts for the same room overlapped. Leaving and + // rejoining makes the server reply with a fresh RespOk so the tab is displayed + // without requiring a client restart. The guard above covers exactly the rejoin that + // leaveAndRejoinRoom triggers, so a server that keeps replying with RespContextError + // gets one heal attempt per join instead of an endless recursion. + healedRoomJoins.insert(roomId); + leaveAndRejoinRoom(roomId, setCurrent); return; case Response::RespUserLevelTooLow: - QMessageBox::critical(this, tr("Error"), - tr("You do not have the required permission to join this server room.")); + if (setCurrent) { + QMessageBox::critical(this, tr("Error"), + tr("You do not have the required permission to join this server room.")); + } emit roomJoinFailed(roomId); return; default: - QMessageBox::critical( - this, tr("Error"), - tr("Failed to join the server room due to an unknown error: %1.").arg(r.response_code())); + if (setCurrent) { + QMessageBox::critical( + this, tr("Error"), + tr("Failed to join the server room due to an unknown error: %1.").arg(r.response_code())); + } emit roomJoinFailed(roomId); return; } const Response_JoinRoom &resp = r.GetExtension(Response_JoinRoom::ext); - emit roomJoined(resp.room_info(), extraData.toBool()); + emit roomJoined(resp.room_info(), setCurrent); +} + +void TabServer::leaveAndRejoinRoom(int roomId, bool setCurrent) +{ + // Clear the stale room membership server-side. The leave is sent before the rejoin below, + // so the server no longer considers us a member by the time the join arrives. The leave + // response is intentionally not awaited: commands are processed in send order on the + // connection, and a failed leave (RespNotInRoom) only means the membership was already gone. + client->sendCommand(client->prepareRoomCommand(Command_LeaveRoom(), roomId)); + + joinRoom(roomId, setCurrent); } diff --git a/cockatrice/src/interface/widgets/tabs/tab_server.h b/cockatrice/src/interface/widgets/tabs/tab_server.h index c10b7945b..121ff814d 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_server.h +++ b/cockatrice/src/interface/widgets/tabs/tab_server.h @@ -10,6 +10,8 @@ #include "tab.h" #include +#include +#include #include #include @@ -58,10 +60,17 @@ private slots: int roomId); private: + void leaveAndRejoinRoom(int roomId, bool setCurrent); + AbstractClient *client; RoomSelector *roomSelector; QTextBrowser *serverInfoBox; bool shouldEmitUpdate = false; + /** Room ids with a join command in flight, mapped to whether the tab should be focused once it opens. */ + QHash pendingRoomJoins; + /** Room ids for which a stale-membership heal (leave + rejoin) is currently in flight. Released as soon as the + * rejoin has been answered, so a heal is attempted at most once per join. */ + QSet healedRoomJoins; public: TabServer(TabSupervisor *_tabSupervisor, AbstractClient *_client);