Fix user card games for offline users and stale requests (#7738)

cockatrice/src/interface/widgets/server/user/user_games_model.cpp

@@ -0,0 +1,149 @@
+#include "user_games_model.h"
+
+#include <QVariant>
+#include <libcockatrice/network/client/abstract/abstract_client.h>
+#include <libcockatrice/protocol/pb/response.pb.h>
+#include <libcockatrice/protocol/pb/response_get_games_of_user.pb.h>
+#include <libcockatrice/protocol/pb/session_commands.pb.h>
+#include <libcockatrice/protocol/pending_command.h>
+#include <utility>
+
+UserGamesModel::UserGamesModel(AbstractClient *_client, int requestTimeoutMs, QObject *parent)
+    : QAbstractListModel(parent), client(_client)
+{
+    timeout.setSingleShot(true);
+    timeout.setInterval(requestTimeoutMs);
+    connect(&timeout, &QTimer::timeout, this, [this] {
+        invalidateRequest();
+        setState(State::Error);
+    });
+    connected = client->getStatus() == StatusLoggedIn;
+    connect(client, &AbstractClient::statusChanged, this, [this](ClientStatus status) {
+        // AbstractClient::setStatus emits this signal while holding the (non-recursive)
+        // status mutex, which getStatus() would try to lock again. Track the emitted
+        // value here, and read this cache from canRefresh() for the same reason.
+        connected = status == StatusLoggedIn;
+        if (!connected) {
+            invalidateRequest();
+            replaceGames({});
+            setState(online ? State::Disconnected : State::Offline);
+        } else if (online) {
+            refresh();
+        }
+    });
+    connect(client, &QObject::destroyed, this, [this] {
+        client = nullptr;
+        connected = false;
+        invalidateRequest();
+        replaceGames({});
+        setState(online ? State::Disconnected : State::Offline);
+    });
+}
+
+int UserGamesModel::rowCount(const QModelIndex &parent) const
+{
+    return parent.isValid() ? 0 : games.size();
+}
+
+QVariant UserGamesModel::data(const QModelIndex &index, int role) const
+{
+    if (!index.isValid() || index.model() != this || index.column() != 0 || index.row() < 0 ||
+        index.row() >= games.size() || role != GameRole) {
+        return {};
+    }
+    return QVariant::fromValue(games.at(index.row()));
+}
+
+void UserGamesModel::setUser(const QString &name, bool isOnline)
+{
+    userName = name;
+    online = isOnline;
+    refresh();
+}
+
+UserGamesModel::State UserGamesModel::getState() const
+{
+    return state;
+}
+
+bool UserGamesModel::canRefresh() const
+{
+    return online && !userName.isEmpty() && client && connected && state != State::Loading;
+}
+
+void UserGamesModel::invalidateRequest()
+{
+    ++requestGeneration;
+    // The client owns pending commands. Detach our interest without deleting
+    // an object that may already have moved to the client's thread.
+    disconnect(responseConnection);
+    responseConnection = {};
+    timeout.stop();
+}
+
+void UserGamesModel::replaceGames(QList<ServerInfo_Game> replacement)
+{
+    beginResetModel();
+    games = std::move(replacement);
+    endResetModel();
+}
+
+void UserGamesModel::setState(State newState)
+{
+    state = newState;
+    emit stateChanged();
+}
+
+void UserGamesModel::refresh()
+{
+    invalidateRequest();
+    const quint64 request = requestGeneration;
+    replaceGames({});
+    if (!online || userName.isEmpty()) {
+        setState(State::Offline);
+        return;
+    }
+    if (!client || !connected) {
+        setState(State::Disconnected);
+        return;
+    }
+
+    setState(State::Loading);
+    // State listeners may select another user or disconnect the client.
+    if (request != requestGeneration) {
+        return;
+    }
+
+    Command_GetGamesOfUser command;
+    command.set_user_name(userName.toStdString());
+    PendingCommand *pending = client->prepareSessionCommand(command);
+    responseConnection = connect(pending, &PendingCommand::finished, this,
+                                 [this, request](const Response &response) { finishRequest(response, request); });
+    timeout.start();
+    client->sendCommand(pending);
+}
+
+void UserGamesModel::finishRequest(const Response &response, quint64 request)
+{
+    if (request != requestGeneration) {
+        return;
+    }
+    invalidateRequest();
+    const quint64 completion = requestGeneration;
+    if (response.response_code() != Response::RespOk || !response.HasExtension(Response_GetGamesOfUser::ext)) {
+        setState(State::Error);
+        return;
+    }
+
+    const auto &result = response.GetExtension(Response_GetGamesOfUser::ext);
+    QList<ServerInfo_Game> replacement;
+    replacement.reserve(result.game_list_size());
+    for (const auto &game : result.game_list()) {
+        replacement.append(game);
+    }
+    replaceGames(std::move(replacement));
+    if (completion != requestGeneration) {
+        return;
+    }
+    setState(games.isEmpty() ? State::Empty : State::Ready);
+}

cockatrice/src/interface/widgets/server/user/user_games_model.h

@@ -0,0 +1,63 @@
+#ifndef COCKATRICE_USER_GAMES_MODEL_H
+#define COCKATRICE_USER_GAMES_MODEL_H
+
+#include <QAbstractListModel>
+#include <QList>
+#include <QPointer>
+#include <QString>
+#include <QTimer>
+#include <libcockatrice/protocol/pb/serverinfo_game.pb.h>
+
+class AbstractClient;
+class Response;
+
+/** Current user's games and the lifetime of the request that supplies them. */
+class UserGamesModel : public QAbstractListModel
+{
+    Q_OBJECT
+
+public:
+    enum class State
+    {
+        Offline,
+        Disconnected,
+        Loading,
+        Ready,
+        Empty,
+        Error
+    };
+    static constexpr int DefaultRequestTimeoutMs = 15000;
+    static constexpr int GameRole = Qt::UserRole + 10;
+
+    explicit UserGamesModel(AbstractClient *client,
+                            int requestTimeoutMs = DefaultRequestTimeoutMs,
+                            QObject *parent = nullptr);
+    int rowCount(const QModelIndex &parent = {}) const override;
+    QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override;
+
+    void setUser(const QString &name, bool isOnline);
+    void refresh();
+    State getState() const;
+    bool canRefresh() const;
+
+signals:
+    void stateChanged();
+
+private:
+    void invalidateRequest();
+    void replaceGames(QList<ServerInfo_Game> replacement);
+    void finishRequest(const Response &response, quint64 request);
+    void setState(State newState);
+
+    QPointer<AbstractClient> client;
+    QString userName;
+    bool online = false;
+    bool connected = false;
+    State state = State::Offline;
+    QList<ServerInfo_Game> games;
+    quint64 requestGeneration = 0;
+    QMetaObject::Connection responseConnection;
+    QTimer timeout;
+};
+
+#endif // COCKATRICE_USER_GAMES_MODEL_H

cockatrice/src/interface/widgets/server/user/user_info_popup.cpp

@@ -6,10 +6,9 @@
 #include "../../interface/widgets/tabs/tab_supervisor.h"
 #include "libcockatrice/card/database/card_database_querier.h"
 #include "libcockatrice/card/printing/exact_card.h"
-#include "libcockatrice/protocol/pb/response.pb.h"
 #include "libcockatrice/protocol/pb/serverinfo_game.pb.h"
 #include "libcockatrice/protocol/pb/serverinfo_user.pb.h"
-#include "libcockatrice/protocol/pb/session_commands.pb.h"
+#include "user_games_model.h"
 #include "user_level.h"
 #include "user_list_painter.h"
 
@@ -34,6 +33,7 @@
 #include <QPainter>
 #include <QPainterPath>
 #include <QPen>
+#include <QPersistentModelIndex>
 #include <QPoint>
 #include <QPushButton>
 #include <QRect>
@@ -41,8 +41,6 @@
 #include <QSharedPointer>
 #include <QSize>
 #include <QSizePolicy>
-#include <QStandardItem>
-#include <QStandardItemModel>
 #include <QStringLiteral>
 #include <QStyle>
 #include <QStyleOptionViewItem>
@@ -52,8 +50,6 @@
 #include <QtGlobal>
 #include <libcockatrice/card/database/card_database_manager.h>
 #include <libcockatrice/network/client/abstract/abstract_client.h>
-#include <libcockatrice/protocol/pb/response_get_games_of_user.pb.h>
-#include <libcockatrice/protocol/pending_command.h>
 #include <libcockatrice/utility/card_ref.h>
 #include <libcockatrice/utility/qt_utils.h>
 #include <qnumeric.h>
@@ -427,6 +423,7 @@ void UserInfoPopup::buildUi()
     refreshBtn = new QPushButton(QStringLiteral("↻"), this);
     refreshBtn->setFixedSize(20, 20);
     refreshBtn->setFlat(true);
+    refreshBtn->setToolTip(tr("Refresh games"));
     connect(refreshBtn, &QPushButton::clicked, this, &UserInfoPopup::refreshGames);
     gh->addWidget(refreshBtn);
     root->addLayout(gh);
@@ -437,7 +434,8 @@ void UserInfoPopup::buildUi()
     root->addWidget(gamesStatus);
 
     // Games list
-    gamesModel = new QStandardItemModel(this);
+    gamesModel = new UserGamesModel(client, UserGamesModel::DefaultRequestTimeoutMs, this);
+    connect(gamesModel, &UserGamesModel::stateChanged, this, &UserInfoPopup::updateGamesDisplay);
     gamesView = new QListView(this);
     gamesView->setModel(gamesModel);
     gamesView->setItemDelegate(new PopupGameDelegate(gamesView));
@@ -645,13 +643,21 @@ void UserInfoPopup::rebuildActionButtons(const ServerInfo_User &userInfo, bool o
 
 void UserInfoPopup::updateActionButtons(const ServerInfo_User &userInfo, bool online, bool isBuddy, bool isIgnored)
 {
-    rebuildActionButtons(userInfo, online, isBuddy, isIgnored);
-    adjustSize();
+    currentUserInfo = userInfo;
+    currentBuddy = isBuddy;
+    currentIgnored = isIgnored;
+    if (currentOnline != online) {
+        setOnline(online);
+    } else {
+        refreshHeader();
+        rebuildActionButtons(userInfo, online, isBuddy, isIgnored);
+        adjustSize();
+    }
 }
 
 void UserInfoPopup::onGamesContextMenu(const QPoint &pos)
 {
-    const QModelIndex idx = gamesView->indexAt(pos);
+    const QPersistentModelIndex idx = gamesView->indexAt(pos);
     if (!idx.isValid()) {
         return;
     }
@@ -679,7 +685,7 @@ void UserInfoPopup::onGamesContextMenu(const QPoint &pos)
     }
 
     const QAction *chosen = menu.exec(gamesView->viewport()->mapToGlobal(pos));
-    if (!chosen) {
+    if (!chosen || !idx.isValid()) {
         return;
     }
 
@@ -716,6 +722,8 @@ void UserInfoPopup::showForUser(const QString &userName,
     currentUser = userName;
     currentUserInfo = userInfo;
     currentOnline = online;
+    currentBuddy = isBuddy;
+    currentIgnored = isIgnored;
 
     // Header
     refreshHeader();
@@ -723,88 +731,71 @@ void UserInfoPopup::showForUser(const QString &userName,
     // Actions
     rebuildActionButtons(userInfo, online, isBuddy, isIgnored);
 
-    // Games list reset
-    gamesModel->clear();
-    gamesView->hide();
-    gamesStatus->setText(tr("Loading games…"));
-    gamesStatus->show();
-
     // Close button, top right corner, above everything
     closeBtn->move(PopupWidth - closeBtn->width() - 6, 6);
     closeBtn->raise();
 
+    gamesModel->setUser(currentUser, currentOnline);
     adjustSize();
-    fetchGames();
 }
 
 // ── Games fetch ───────────────────────────────────────────────────────────────
 
 void UserInfoPopup::fetchGames()
 {
-    if (!client || currentUser.isEmpty()) {
-        return;
-    }
-
-    Command_GetGamesOfUser cmd;
-    cmd.set_user_name(currentUser.toStdString());
-
-    const QString snapshot = currentUser;
-    PendingCommand *pend = client->prepareSessionCommand(cmd);
-    connect(pend, &PendingCommand::finished, this,
-            [this, snapshot](const Response &r) { onGamesReceived(r, snapshot); });
-    client->sendCommand(pend);
+    gamesModel->refresh();
 }
 
-void UserInfoPopup::onGamesReceived(const Response &r, const QString &forUser)
+void UserInfoPopup::setOnline(bool online)
 {
-    if (forUser != currentUser) {
-        return; // stale response, different user showing now
-    }
-
-    gamesModel->clear();
-
-    if (r.response_code() != Response::RespOk) {
-        gamesStatus->setText(tr("Could not load games."));
-        gamesStatus->show();
-        gamesView->hide();
+    if (currentOnline == online) {
         return;
     }
+    currentOnline = online;
+    refreshHeader();
+    rebuildActionButtons(currentUserInfo, online, currentBuddy, currentIgnored);
+    gamesModel->setUser(currentUser, online);
+}
 
-    const auto &resp = r.GetExtension(Response_GetGamesOfUser::ext);
-    if (resp.game_list_size() == 0) {
-        gamesStatus->setText(tr("No active games."));
-        gamesStatus->show();
-        gamesView->hide();
-        return;
+void UserInfoPopup::updateGamesDisplay()
+{
+    refreshBtn->setEnabled(gamesModel->canRefresh());
+    const auto state = gamesModel->getState();
+    const bool ready = state == UserGamesModel::State::Ready;
+    gamesView->setVisible(ready);
+    gamesStatus->setVisible(!ready);
+
+    switch (state) {
+        case UserGamesModel::State::Offline:
+            gamesStatus->setText(tr("User is offline."));
+            break;
+        case UserGamesModel::State::Disconnected:
+            gamesStatus->setText(tr("Not connected to the server."));
+            break;
+        case UserGamesModel::State::Loading:
+            gamesStatus->setText(tr("Loading games…"));
+            break;
+        case UserGamesModel::State::Empty:
+            gamesStatus->setText(tr("No active games."));
+            break;
+        case UserGamesModel::State::Error:
+            gamesStatus->setText(tr("Could not load games."));
+            break;
+        case UserGamesModel::State::Ready: {
+            // Fit exactly to the number of visible rows, scroll when more than 5.
+            constexpr int rowH = 38; // must match PopupGameDelegate::sizeHint
+            constexpr int maxRows = 5;
+            const int count = gamesModel->rowCount();
+            gamesView->setFixedHeight(qMin(count, maxRows) * rowH + 2);
+            gamesView->setVerticalScrollBarPolicy(count > maxRows ? Qt::ScrollBarAlwaysOn : Qt::ScrollBarAlwaysOff);
+            break;
+        }
     }
-
-    for (int i = 0; i < resp.game_list_size(); ++i) {
-        auto *item = new QStandardItem;
-        item->setData(QVariant::fromValue(resp.game_list(i)), PopupRoles::GameData);
-        item->setEditable(false);
-        gamesModel->appendRow(item);
-    }
-
-    gamesStatus->hide();
-    gamesView->show();
-
-    // Fit exactly to the number of visible rows, scroll when more than 5
-    constexpr int rowH = 38; // must match PopupGameDelegate::sizeHint
-    constexpr int maxRows = 5;
-    const int count = gamesModel->rowCount();
-    const int visible = qMin(count, maxRows);
-    gamesView->setFixedHeight(visible * rowH + 2);
-    gamesView->setVerticalScrollBarPolicy(count > maxRows ? Qt::ScrollBarAlwaysOn : Qt::ScrollBarAlwaysOff);
-
     adjustSize();
 }
 
 void UserInfoPopup::refreshGames()
 {
-    gamesModel->clear();
-    gamesView->hide();
-    gamesStatus->setText(tr("Loading games…"));
-    gamesStatus->show();
     fetchGames();
 }
 

cockatrice/src/interface/widgets/server/user/user_info_popup.h

@@ -1,6 +1,7 @@
 #ifndef COCKATRICE_USER_INFO_POPUP_H
 #define COCKATRICE_USER_INFO_POPUP_H
 
+#include "user_games_model.h"
 #include "user_list_painter.h"
 
 #include <QColor>
@@ -21,15 +22,13 @@ class QPushButton;
 class TabSupervisor;
 class QListView;
 class QPoint;
-class QStandardItemModel;
-class Response;
 template <class Key, class T> class QMap;
 
 // ── Roles ─────────────────────────────────────────────────────────────────────
 
 namespace PopupRoles
 {
-constexpr int GameData = Qt::UserRole + 10;
+constexpr int GameData = UserGamesModel::GameRole;
 }
 
 // Popup theme
@@ -143,6 +142,7 @@ public:
     void
     showForUser(const QString &userName, const ServerInfo_User &userInfo, bool online, bool isBuddy, bool isIgnored);
     void fetchGames();
+    void setOnline(bool online);
 
     [[nodiscard]] QString getCurrentUser() const
     {
@@ -199,7 +199,7 @@ protected:
 
 private slots:
     void refreshGames();
-    void onGamesReceived(const Response &r, const QString &forUser);
+    void updateGamesDisplay();
     void onGamesContextMenu(const QPoint &pos);
 
 private:
@@ -218,6 +218,8 @@ private:
     QString currentUser;
     ServerInfo_User currentUserInfo;
     bool currentOnline = false;
+    bool currentBuddy = false;
+    bool currentIgnored = false;
     std::function<bool(const QString &userName)> gameInviteAvailable;
 
     UserInfoHeaderWidget *header;
@@ -225,7 +227,7 @@ private:
     QLabel *gamesLabel;
     QFrame *separator;
     QListView *gamesView;
-    QStandardItemModel *gamesModel;
+    UserGamesModel *gamesModel;
     QLabel *gamesStatus;
     QPushButton *closeBtn;
     QPushButton *refreshBtn;

cockatrice/src/interface/widgets/server/user/user_list_widget.cpp

@@ -617,7 +617,14 @@ void UserListWidget::bind(UserListManager *mgr)
     connect(manager, &UserListManager::addedToIgnoreList, this, refreshCurrentPopup);
     connect(manager, &UserListManager::removedFromIgnoreList, this, refreshIfPopupOpen);
     connect(manager, &UserListManager::userJoinedOnline, this, refreshCurrentPopup);
-    connect(manager, &UserListManager::userLeftOnline, this, refreshIfPopupOpen);
+    connect(manager, &UserListManager::userLeftOnline, this, [this, refreshIfPopupOpen](const QString &name) {
+        // The online row may already have been removed; the pinned popup must
+        // still invalidate its games request and display the offline state.
+        if (userInfoPopup && userInfoPopup->getCurrentUser() == name) {
+            userInfoPopup->setOnline(false);
+        }
+        refreshIfPopupOpen(name);
+    });
 
     rebuild();
 }