[Client] Release room-join join markers when a command times out (#7348)

RemoteClient's ping sweep deleted timed-out pending commands without
emitting finished, so TabServer's in-flight room-join markers were never
released: the room stayed silently unjoinable for the rest of the session
even though the connection stayed alive. Answer swept commands with
RespNotConnected instead of dropping them, matching the disconnect
cleanup, so every awaiter gets a deterministic terminal response.

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>

cockatrice/cockatrice.qrc

@@ -62,9 +62,9 @@
         <file>resources/icons/mana/U.svg</file>
         <file>resources/icons/mana/W.svg</file>
 
-        <file>resources/backgrounds/home.png</file>
-        <file>resources/backgrounds/home-dark.png</file>
-        <file>resources/backgrounds/home-light.png</file>
+        <file>resources/backgrounds/home.jpg</file>
+        <file>resources/backgrounds/home-dark.jpg</file>
+        <file>resources/backgrounds/home-light.jpg</file>
         <file>resources/backgrounds/card_triplet.svg</file>
         <file>resources/backgrounds/placeholder_printing_selector.svg</file>
 

cockatrice/src/interface/widgets/dialogs/dlg_report_user.cpp

@@ -15,6 +15,7 @@
 #include <QVBoxLayout>
 #include <libcockatrice/protocol/pb/command_report.pb.h>
 #include <libcockatrice/protocol/pending_command.h>
+#include <libcockatrice/utility/report_categories.h>
 
 DlgReportUser::DlgReportUser(AbstractClient *_client,
                              const QString &_reportedUser,
@@ -56,22 +57,14 @@ DlgReportUser::DlgReportUser(AbstractClient *_client,
     auto *categoryGrid = new QGridLayout(categoryGroup);
 
     categoryBox = new QComboBox;
-    categoryBox->addItem(tr("Cheating / Unsporting behavior"), "cheating");
-    categoryBox->setItemData(categoryBox->count() - 1,
-                             tr("Using external tools, card marked manipulation, or exploiting game bugs"),
-                             Qt::ToolTipRole);
-    categoryBox->addItem(tr("Harassment / Abuse"), "harassment");
-    categoryBox->setItemData(categoryBox->count() - 1, tr("Threatening, bullying, or persistent unwanted contact"),
-                             Qt::ToolTipRole);
-    categoryBox->addItem(tr("Hate speech"), "hate_speech");
-    categoryBox->setItemData(categoryBox->count() - 1,
-                             tr("Discriminatory language targeting race, gender, religion, etc."), Qt::ToolTipRole);
-    categoryBox->addItem(tr("Spam"), "spam");
-    categoryBox->setItemData(categoryBox->count() - 1, tr("Repeated unwanted messages or advertisements"),
-                             Qt::ToolTipRole);
-    categoryBox->addItem(tr("Other"), "other");
-    categoryBox->setItemData(categoryBox->count() - 1, tr("Any behavior not covered by the above categories"),
-                             Qt::ToolTipRole);
+    for (const QString &key : ReportCategories::keys()) {
+        const QString label = categoryLabel(key);
+        if (label.isEmpty()) {
+            continue; // skip keys without a dialog label
+        }
+        categoryBox->addItem(label, key);
+        categoryBox->setItemData(categoryBox->count() - 1, categoryToolTip(key), Qt::ToolTipRole);
+    }
 
     categoryGrid->addWidget(new QLabel(tr("Category:")), 0, 0);
     categoryGrid->addWidget(categoryBox, 0, 1);
@@ -189,3 +182,55 @@ void DlgReportUser::reportResponse(const Response &response)
         QMessageBox::warning(this, tr("Submission Failed"), tr("Failed to submit report. Please try again."));
     }
 }
+
+QString DlgReportUser::categoryLabel(const QString &key)
+{
+    if (key == QLatin1String("cheating")) {
+        return tr("Cheating / Unsporting behavior");
+    }
+    if (key == QLatin1String("bug_abuse")) {
+        return tr("Bug Abuse");
+    }
+    if (key == QLatin1String("harassment")) {
+        return tr("Harassment / Abuse");
+    }
+    if (key == QLatin1String("verbal_abuse")) {
+        return tr("Verbal Abuse");
+    }
+    if (key == QLatin1String("hate_speech")) {
+        return tr("Hate speech");
+    }
+    if (key == QLatin1String("spam")) {
+        return tr("Spam");
+    }
+    if (key == QLatin1String("other")) {
+        return tr("Other");
+    }
+    return QString();
+}
+
+QString DlgReportUser::categoryToolTip(const QString &key)
+{
+    if (key == QLatin1String("cheating")) {
+        return tr("Using external tools, cheat programs, or exploiting game bugs");
+    }
+    if (key == QLatin1String("bug_abuse")) {
+        return tr("Exploiting a bug or glitch to gain an unfair advantage");
+    }
+    if (key == QLatin1String("harassment")) {
+        return tr("Threatening, bullying, or persistent unwanted contact");
+    }
+    if (key == QLatin1String("verbal_abuse")) {
+        return tr("Abusive or offensive language directed at another player");
+    }
+    if (key == QLatin1String("hate_speech")) {
+        return tr("Discriminatory language targeting race, gender, religion, etc.");
+    }
+    if (key == QLatin1String("spam")) {
+        return tr("Repeated unwanted messages or advertisements");
+    }
+    if (key == QLatin1String("other")) {
+        return tr("Any behavior not covered by the above categories");
+    }
+    return QString();
+}

cockatrice/src/interface/widgets/dialogs/dlg_report_user.h

@@ -26,6 +26,9 @@ private slots:
     void reportResponse(const Response &response);
 
 private:
+    static QString categoryLabel(const QString &key);
+    static QString categoryToolTip(const QString &key);
+
     AbstractClient *client;
     QString reportedUser;
     int gameId;

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

@@ -218,9 +218,10 @@ void UserListTWI::setOnline(bool online)
  * 2) Admins, judge/vip/donator status ignored
  * 3) Moderators, judge/vip/donator status ignored
  * 4) Judges
- * 5) VIPs
- * 6) Donators
- * 7) Everyone else
+ * 5) Developers
+ * 6) VIPs
+ * 7) Donators
+ * 8) Everyone else
  * @param other RHS to compare to
  * @return Left is less than the Right
  */
@@ -234,11 +235,10 @@ bool UserListTWI::operator<(const QTreeWidgetItem &other) const
     const auto &lhsUserLevelFlags = UserLevelFlags(data(0, Qt::UserRole).toInt());
     const auto &rhsUserLevelFlags = UserLevelFlags(other.data(0, Qt::UserRole).toInt());
 
-    // Admins, Developers & Mods need no additional comparison checks, just to see if they're an admin, a developer
+    // Admins & Moderators need no additional comparison checks, just to see if they're an admin
     // or a moderator
     static const QList<ServerInfo_User_UserLevelFlag> userLevelWithNoOtherPrefOrder = {
-        ServerInfo_User_UserLevelFlag_IsAdmin, ServerInfo_User_UserLevelFlag_IsDeveloper,
-        ServerInfo_User_UserLevelFlag_IsModerator};
+        ServerInfo_User_UserLevelFlag_IsAdmin, ServerInfo_User_UserLevelFlag_IsModerator};
     for (const auto &userLevelEntry : userLevelWithNoOtherPrefOrder) {
         if (lhsUserLevelFlags.testFlag(userLevelEntry) &&
             lhsUserLevelFlags.testFlag(userLevelEntry) == rhsUserLevelFlags.testFlag(userLevelEntry)) {
@@ -249,10 +249,10 @@ bool UserListTWI::operator<(const QTreeWidgetItem &other) const
         }
     }
 
-    // Judges can be sorted by their additional ranks
-    static const QList<ServerInfo_User_UserLevelFlag> userLevelOrder = {ServerInfo_User_UserLevelFlag_IsJudge,
-                                                                        ServerInfo_User_UserLevelFlag_IsRegistered,
-                                                                        ServerInfo_User_UserLevelFlag_IsUser};
+    // Judges and developers can be sorted by their additional ranks
+    static const QList<ServerInfo_User_UserLevelFlag> userLevelOrder = {
+        ServerInfo_User_UserLevelFlag_IsJudge, ServerInfo_User_UserLevelFlag_IsDeveloper,
+        ServerInfo_User_UserLevelFlag_IsRegistered, ServerInfo_User_UserLevelFlag_IsUser};
     for (const auto &userLevelEntry : userLevelOrder) {
         if (lhsUserLevelFlags.testFlag(userLevelEntry) != rhsUserLevelFlags.testFlag(userLevelEntry)) {
             return lhsUserLevelFlags.testFlag(userLevelEntry) > rhsUserLevelFlags.testFlag(userLevelEntry);

libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp

@@ -559,6 +559,15 @@ void RemoteClient::ping()
         PendingCommand *pend = i.next().value();
         if (pend->tick() > maxTimeout) {
             i.remove();
+            // Answer the command instead of dropping it silently: the connection may still
+            // be alive (other traffic keeps lastDataReceived fresh) while this single
+            // command stalled, and awaiters such as TabServer's room-join dedup can only
+            // release their in-flight markers when `finished` fires. A silent drop wedges
+            // them permanently - e.g. a room whose join timing never surfaces again.
+            Response response;
+            response.set_response_code(Response::RespNotConnected);
+            response.set_cmd_id(pend->getCommandContainer().cmd_id());
+            pend->processResponse(response);
             pend->deleteLater();
         }
     }

libcockatrice_utility/CMakeLists.txt

@@ -6,8 +6,12 @@ set(CMAKE_AUTOUIC ON)
 set(CMAKE_AUTORCC ON)
 
 set(UTILITY_SOURCES
-    libcockatrice/utility/cryptoutil.cpp libcockatrice/utility/expression.cpp libcockatrice/utility/levenshtein.cpp
-    libcockatrice/utility/passwordhasher.cpp libcockatrice/utility/server_rate_limiter.cpp
+    libcockatrice/utility/cryptoutil.cpp
+    libcockatrice/utility/expression.cpp
+    libcockatrice/utility/levenshtein.cpp
+    libcockatrice/utility/passwordhasher.cpp
+    libcockatrice/utility/report_categories.cpp
+    libcockatrice/utility/server_rate_limiter.cpp
     libcockatrice/utility/warning_categories.cpp
 )
 
@@ -20,6 +24,7 @@ set(UTILITY_HEADERS
     libcockatrice/utility/macros.h
     libcockatrice/utility/passwordhasher.h
     libcockatrice/utility/playmat_params.h
+    libcockatrice/utility/report_categories.h
     libcockatrice/utility/string_limits.h
     libcockatrice/utility/dice_limits.h
     libcockatrice/utility/counter_limits.h

libcockatrice_utility/libcockatrice/utility/report_categories.cpp

@@ -0,0 +1,16 @@
+#include "report_categories.h"
+
+namespace ReportCategories
+{
+const QStringList &keys()
+{
+    static const QStringList categories = {"cheating",    "bug_abuse", "harassment", "verbal_abuse",
+                                           "hate_speech", "spam",      "other"};
+    return categories;
+}
+
+bool isValid(const QString &key)
+{
+    return keys().contains(key, Qt::CaseInsensitive);
+}
+} // namespace ReportCategories

libcockatrice_utility/libcockatrice/utility/report_categories.h

@@ -0,0 +1,24 @@
+#ifndef REPORT_CATEGORIES_H
+#define REPORT_CATEGORIES_H
+
+#include <QString>
+#include <QStringList>
+
+/**
+ * Canonical report categories, shared by the client and the server.
+ *
+ * This is the single source of truth for the category keys exchanged over the
+ * wire (Command_Report.category) and stored in the reports table. The server
+ * only accepts keys from this list and the client report dialog only offers
+ * keys from this list, so the two sides cannot drift apart again.
+ */
+namespace ReportCategories
+{
+/** @brief The complete list of valid category keys, in canonical order. */
+const QStringList &keys();
+
+/** @brief Whether @a key is a valid report category (case-insensitive). */
+bool isValid(const QString &key);
+} // namespace ReportCategories
+
+#endif // REPORT_CATEGORIES_H

servatrice/src/serversocketinterface.cpp

@@ -128,6 +128,7 @@
 #include <libcockatrice/protocol/pb/serverinfo_user_alt.pb.h>
 #include <libcockatrice/protocol/pb/serverinfo_user_session.pb.h>
 #include <libcockatrice/utility/passwordhasher.h>
+#include <libcockatrice/utility/report_categories.h>
 #include <libcockatrice/utility/string_limits.h>
 #include <libcockatrice/utility/warning_categories.h>
 #include <server_response_containers.h>
@@ -3775,8 +3776,7 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReport(const Command_Re
         return Response::RespInvalidData;
     }
 
-    static const QStringList validCategories = {"cheating", "bug_abuse", "verbal_abuse", "other"};
-    if (!validCategories.contains(category.toLower())) {
+    if (!ReportCategories::isValid(category)) {
         return Response::RespInvalidData;
     }
 

tests/CMakeLists.txt

@@ -14,6 +14,7 @@ add_test(NAME server_rate_limiter_test COMMAND server_rate_limiter_test)
 add_test(NAME server_developer_role_test COMMAND server_developer_role_test)
 add_test(NAME server_game_join_test COMMAND server_game_join_test)
 add_test(NAME warning_categories_test COMMAND warning_categories_test)
+add_test(NAME report_categories_test COMMAND report_categories_test)
 add_test(NAME lag_monitor_test COMMAND lag_monitor_test)
 add_test(NAME latency_tracker_test COMMAND latency_tracker_test)
 add_test(NAME metrics_registry_test COMMAND metrics_registry_test)
@@ -37,6 +38,7 @@ add_executable(server_rate_limiter_test server_rate_limiter_test.cpp)
 add_executable(server_developer_role_test server_developer_role_test.cpp)
 add_executable(server_game_join_test server_game_join_test.cpp)
 add_executable(warning_categories_test warning_categories_test.cpp)
+add_executable(report_categories_test report_categories_test.cpp)
 add_executable(lag_monitor_test ${CMAKE_SOURCE_DIR}/cockatrice/src/client/lag_monitor.cpp lag_monitor_test.cpp)
 target_include_directories(lag_monitor_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src)
 add_executable(latency_tracker_test latency_tracker_test.cpp)
@@ -91,6 +93,7 @@ if(NOT GTEST_FOUND)
   add_dependencies(server_developer_role_test gtest)
   add_dependencies(server_game_join_test gtest)
   add_dependencies(warning_categories_test gtest)
+  add_dependencies(report_categories_test gtest)
   add_dependencies(lag_monitor_test gtest)
   add_dependencies(latency_tracker_test gtest)
   add_dependencies(metrics_registry_test gtest)
@@ -137,6 +140,9 @@ target_link_libraries(
 target_link_libraries(
   warning_categories_test libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
 )
+target_link_libraries(
+  report_categories_test libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}
+)
 target_link_libraries(lag_monitor_test Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES})
 target_link_libraries(
   latency_tracker_test libcockatrice_network Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}

tests/report_categories_test.cpp

@@ -0,0 +1,54 @@
+#include "gtest/gtest.h"
+#include <QList>
+#include <QString>
+#include <QStringList>
+#include <libcockatrice/utility/report_categories.h>
+
+TEST(ReportCategoriesTest, CanonicalListIsNonEmptyAndStable)
+{
+    const QStringList categories = ReportCategories::keys();
+
+    ASSERT_FALSE(categories.isEmpty());
+    EXPECT_EQ("cheating", categories.at(0));
+    EXPECT_EQ("other", categories.last());
+}
+
+TEST(ReportCategoriesTest, CanonicalListContainsMergedClientAndServerCategories)
+{
+    const QStringList expected = {"cheating",    "bug_abuse", "harassment", "verbal_abuse",
+                                  "hate_speech", "spam",      "other"};
+    EXPECT_EQ(expected, ReportCategories::keys());
+    EXPECT_EQ(7, ReportCategories::keys().size());
+}
+
+TEST(ReportCategoriesTest, AllCanonicalKeysAreValid)
+{
+    for (const QString &category : ReportCategories::keys()) {
+        EXPECT_TRUE(ReportCategories::isValid(category)) << category.toStdString();
+    }
+}
+
+TEST(ReportCategoriesTest, LegacyCategoriesRemainValid)
+{
+    EXPECT_TRUE(ReportCategories::isValid("bug_abuse"));
+    EXPECT_TRUE(ReportCategories::isValid("verbal_abuse"));
+}
+
+TEST(ReportCategoriesTest, ValidationIsCaseInsensitive)
+{
+    EXPECT_TRUE(ReportCategories::isValid("Cheating"));
+    EXPECT_TRUE(ReportCategories::isValid("HATE_SPEECH"));
+}
+
+TEST(ReportCategoriesTest, UnknownAndEmptyCategoriesAreRejected)
+{
+    EXPECT_FALSE(ReportCategories::isValid(QString()));
+    EXPECT_FALSE(ReportCategories::isValid("hate speech"));
+    EXPECT_FALSE(ReportCategories::isValid("griefing"));
+}
+
+int main(int argc, char **argv)
+{
+    ::testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}