From d400af5bc02e49143ee961fd00d6f371a3ed19e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sat, 15 Aug 2026 21:47:49 +0200 Subject: [PATCH] [UserList] Use an enum for the list sections The section identifiers were stringly-typed: eleven hardcoded QStringLiteral comparisons scattered through user_list_widget.cpp, and the display path (sectionTitle) maps every id through tr() anyway, so the raw strings were never shown. A typo compiled fine and silently broke a section. - enum class Section { Buddy, Online, Ignore } replaces the section strings across the sectioned-list API (setSectioned, getSectionIds, setSectionExpanded, the sectionExpanded signal and all membership helpers), giving compile-time checks at every call site. - sectionTitle becomes a switch over the enum and the dead raw-string fallback is gone. - The expanded-section state persists the same stable keys via the panel widget boundary, so existing settings files survive unchanged. - The divider reverse lookup in handleSectionExpansion no longer relies on an empty-string sentinel from QMap::key; it scans the three dividers and bails when the item is not one of them. Took 12 minutes # Commit time for manual adjustment: # Took 2 minutes --- .../server/user/user_list_panel_widget.cpp | 34 ++++- .../server/user/user_list_panel_widget.h | 5 +- .../widgets/server/user/user_list_widget.cpp | 139 +++++++++--------- .../widgets/server/user/user_list_widget.h | 43 +++--- 4 files changed, 126 insertions(+), 95 deletions(-) diff --git a/cockatrice/src/interface/widgets/server/user/user_list_panel_widget.cpp b/cockatrice/src/interface/widgets/server/user/user_list_panel_widget.cpp index 3fa5591dc..937058024 100644 --- a/cockatrice/src/interface/widgets/server/user/user_list_panel_widget.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_list_panel_widget.cpp @@ -9,6 +9,24 @@ #include #include +namespace +{ +// The persisted section keys are the serialization contract with the user's +// settings file, so the values must stay stable across versions. +QString sectionKey(UserListWidget::Section section) +{ + switch (section) { + case UserListWidget::Section::Buddy: + return QStringLiteral("buddy"); + case UserListWidget::Section::Online: + return QStringLiteral("online"); + case UserListWidget::Section::Ignore: + return QStringLiteral("ignore"); + } + return {}; +} +} // namespace + UserListPanelWidget::UserListPanelWidget(TabSupervisor *_tabSupervisor, AbstractClient *_client, QWidget *parent) : QWidget(parent) { @@ -21,7 +39,8 @@ UserListPanelWidget::UserListPanelWidget(TabSupervisor *_tabSupervisor, Abstract mainLayout->addWidget(searchBar); userList = new UserListWidget(_tabSupervisor, _client, UserListWidget::RoomList, this); - userList->setSectioned({QStringLiteral("buddy"), QStringLiteral("online"), QStringLiteral("ignore")}); + userList->setSectioned( + {UserListWidget::Section::Buddy, UserListWidget::Section::Online, UserListWidget::Section::Ignore}); mainLayout->addWidget(userList, 1); connect(searchBar, &QLineEdit::textChanged, userList, &UserListWidget::setFilterText); @@ -31,8 +50,8 @@ UserListPanelWidget::UserListPanelWidget(TabSupervisor *_tabSupervisor, Abstract // Restore the persisted expansion state, then apply it to the tree. const QStringList expandedSections = SettingsCache::instance().userInterface().getUserListExpandedSections(); - for (const QString §ionId : userList->getSectionIds()) { - userList->setSectionExpanded(sectionId, expandedSections.contains(sectionId)); + for (const UserListWidget::Section section : userList->getSectionIds()) { + userList->setSectionExpanded(section, expandedSections.contains(sectionKey(section))); } retranslateUi(); @@ -43,15 +62,16 @@ void UserListPanelWidget::bind(UserListManager *manager) userList->bind(manager); } -void UserListPanelWidget::persistExpandedSections(const QString §ionId, bool expanded) +void UserListPanelWidget::persistExpandedSections(UserListWidget::Section section, bool expanded) { + const QString key = sectionKey(section); QStringList expandedSections = SettingsCache::instance().userInterface().getUserListExpandedSections(); if (expanded) { - if (!expandedSections.contains(sectionId)) { - expandedSections.append(sectionId); + if (!expandedSections.contains(key)) { + expandedSections.append(key); } } else { - expandedSections.removeAll(sectionId); + expandedSections.removeAll(key); } SettingsCache::instance().userInterface().setUserListExpandedSections(expandedSections); } diff --git a/cockatrice/src/interface/widgets/server/user/user_list_panel_widget.h b/cockatrice/src/interface/widgets/server/user/user_list_panel_widget.h index 23eb1f9e6..7ae14dfcf 100644 --- a/cockatrice/src/interface/widgets/server/user/user_list_panel_widget.h +++ b/cockatrice/src/interface/widgets/server/user/user_list_panel_widget.h @@ -6,13 +6,14 @@ #ifndef COCKATRICE_USER_LIST_PANEL_WIDGET_H #define COCKATRICE_USER_LIST_PANEL_WIDGET_H +#include "user_list_widget.h" + #include class AbstractClient; class QLineEdit; class TabSupervisor; class UserListManager; -class UserListWidget; /** * A unified user list: a search bar above a single tree whose section headers @@ -33,7 +34,7 @@ signals: void openMessageDialog(const QString &userName, bool focus); private: - void persistExpandedSections(const QString §ionId, bool expanded); + void persistExpandedSections(UserListWidget::Section section, bool expanded); QLineEdit *searchBar = nullptr; UserListWidget *userList = nullptr; diff --git a/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp b/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp index a0d7c8a4c..7a82b0c76 100644 --- a/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp @@ -805,13 +805,13 @@ void UserListWidget::bind(UserListManager *mgr) connect(manager, &UserListManager::userLeftOnline, this, [this](const QString &name) { handleOnlineChangeLeft(name); }); connect(manager, &UserListManager::addedToBuddyList, this, - [this](const ServerInfo_User &user) { handleListAdd(QStringLiteral("buddy"), user); }); + [this](const ServerInfo_User &user) { handleListAdd(Section::Buddy, user); }); connect(manager, &UserListManager::removedFromBuddyList, this, - [this](const QString &name) { handleListRemove(QStringLiteral("buddy"), name); }); + [this](const QString &name) { handleListRemove(Section::Buddy, name); }); connect(manager, &UserListManager::addedToIgnoreList, this, - [this](const ServerInfo_User &user) { handleListAdd(QStringLiteral("ignore"), user); }); + [this](const ServerInfo_User &user) { handleListAdd(Section::Ignore, user); }); connect(manager, &UserListManager::removedFromIgnoreList, this, - [this](const QString &name) { handleListRemove(QStringLiteral("ignore"), name); }); + [this](const QString &name) { handleListRemove(Section::Ignore, name); }); } // ── Popup button refresh ────────────────────────────────────────────────── @@ -1201,8 +1201,8 @@ void UserListWidget::requestAvatarsForVisibleItems() { if (sectioned) { // Top level items are dividers, user rows hang below them. - for (const QString §ionId : sectionIds) { - QTreeWidgetItem *divider = sectionItems.value(sectionId); + for (const Section section : sectionIds) { + QTreeWidgetItem *divider = sectionItems.value(section); if (!divider) { continue; } @@ -1247,15 +1247,15 @@ void UserListWidget::rebuild() beginBulkLoad(); const auto &onlineUsers = manager->getAllUsersList(); for (auto it = onlineUsers.cbegin(); it != onlineUsers.cend(); ++it) { - processUserInfo(QStringLiteral("online"), it.value(), true); + processUserInfo(Section::Online, it.value(), true); } const auto &buddyUsers = manager->getBuddyList(); for (auto it = buddyUsers.cbegin(); it != buddyUsers.cend(); ++it) { - processUserInfo(QStringLiteral("buddy"), it.value(), manager->getOnlineUser(it.key()) != nullptr); + processUserInfo(Section::Buddy, it.value(), manager->getOnlineUser(it.key()) != nullptr); } const auto &ignoreUsers = manager->getIgnoreList(); for (auto it = ignoreUsers.cbegin(); it != ignoreUsers.cend(); ++it) { - processUserInfo(QStringLiteral("ignore"), it.value(), manager->getOnlineUser(it.key()) != nullptr); + processUserInfo(Section::Ignore, it.value(), manager->getOnlineUser(it.key()) != nullptr); } endBulkLoad(); applyFilter(); @@ -1334,9 +1334,9 @@ void UserListWidget::processUserInfo(const ServerInfo_User &user, bool online) } } -void UserListWidget::processUserInfo(const QString §ionId, const ServerInfo_User &user, bool online) +void UserListWidget::processUserInfo(Section section, const ServerInfo_User &user, bool online) { - ensureSectionMembership(sectionId, user, online); + ensureSectionMembership(section, user, online); if (!bulkLoading) { sortItems(); applyFilter(); @@ -1349,9 +1349,9 @@ bool UserListWidget::deleteUser(const QString &userName) if (sectioned) { // The user may own several rows (one per section). Drop them all. bool removed = false; - const QStringList sections = sectionUsers.keys(); // snapshot: maps mutate - for (const QString §ionId : sections) { - removed = dropSectionMembership(sectionId, userName) || removed; + const QList
sections = sectionUsers.keys(); // snapshot: maps mutate + for (const Section section : sections) { + removed = dropSectionMembership(section, userName) || removed; } if (removed && !bulkLoading) { sortItems(); @@ -1430,8 +1430,8 @@ void UserListWidget::updateCount() if (sectioned) { // The dividers carry the section titles setTitle(QString()); - for (const QString §ionId : sectionIds) { - updateSectionDivider(sectionId); + for (const Section section : sectionIds) { + updateSectionDivider(section); } return; } @@ -1467,8 +1467,8 @@ void UserListWidget::applyFilter() if (sectioned) { const bool searching = !filterText.isEmpty(); const QString lower = filterText.toLower(); - for (const QString §ionId : sectionIds) { - QTreeWidgetItem *divider = sectionItems.value(sectionId); + for (const Section section : sectionIds) { + QTreeWidgetItem *divider = sectionItems.value(section); if (!divider) { continue; } @@ -1490,9 +1490,9 @@ void UserListWidget::applyFilter() setExpandedProgrammatically(divider, visible > 0); } else { divider->setHidden(false); - setExpandedProgrammatically(divider, expandedSections.contains(sectionId)); + setExpandedProgrammatically(divider, expandedSections.contains(section)); } - updateSectionDivider(sectionId); + updateSectionDivider(section); } requestAvatarsForVisibleItems(); userTree->viewport()->update(); @@ -1552,7 +1552,7 @@ void UserListWidget::sortItems() // Sectioned mode -void UserListWidget::setSectioned(const QStringList &ids) +void UserListWidget::setSectioned(const QList
&ids) { if (sectioned || ids.isEmpty()) { return; @@ -1561,8 +1561,8 @@ void UserListWidget::setSectioned(const QStringList &ids) sectioned = true; sectionIds = ids; expandedSections.clear(); - for (const QString §ionId : sectionIds) { - expandedSections.insert(sectionId); // everything starts expanded + for (const Section section : sectionIds) { + expandedSections.insert(section); // everything starts expanded } // The single tree owns scrolling and the dividers carry the section titles, @@ -1587,16 +1587,16 @@ void UserListWidget::createSectionItems() { sectionItems.clear(); QSignalBlocker blocker(userTree); // no expansion signals while building - for (const QString §ionId : sectionIds) { - QTreeWidgetItem *divider = createSectionItem(sectionId); - sectionItems.insert(sectionId, divider); - divider->setExpanded(expandedSections.contains(sectionId)); + for (const Section section : sectionIds) { + QTreeWidgetItem *divider = createSectionItem(section); + sectionItems.insert(section, divider); + divider->setExpanded(expandedSections.contains(section)); } } -QTreeWidgetItem *UserListWidget::createSectionItem(const QString §ionId) +QTreeWidgetItem *UserListWidget::createSectionItem(Section section) { - Q_UNUSED(sectionId); + Q_UNUSED(section); auto *divider = new QTreeWidgetItem(SectionItemType); // Selectable so keyboard navigation (Up/Down) can land on the dividers. // They act as collapsible section headers once they have focus. @@ -1618,23 +1618,22 @@ QTreeWidgetItem *UserListWidget::createSectionItem(const QString §ionId) return divider; } -QString UserListWidget::sectionTitle(const QString §ionId) const +QString UserListWidget::sectionTitle(Section section) const { - if (sectionId == QLatin1String("buddy")) { - return tr("Buddies"); + switch (section) { + case Section::Buddy: + return tr("Buddies"); + case Section::Online: + return tr("Online"); + case Section::Ignore: + return tr("Ignored"); } - if (sectionId == QLatin1String("online")) { - return tr("Online"); - } - if (sectionId == QLatin1String("ignore")) { - return tr("Ignored"); - } - return sectionId; + return {}; } -void UserListWidget::updateSectionDivider(const QString §ionId) +void UserListWidget::updateSectionDivider(Section section) { - QTreeWidgetItem *divider = sectionItems.value(sectionId); + QTreeWidgetItem *divider = sectionItems.value(section); if (!divider) { return; } @@ -1647,7 +1646,7 @@ void UserListWidget::updateSectionDivider(const QString §ionId) // The tree draws no branches (rows are flush), so the divider carries its // own collapse arrow glyph. const QString arrow = divider->isExpanded() ? QStringLiteral("\u25BE") : QStringLiteral("\u25B8"); - divider->setText(0, tr("%1 %2 (%3)").arg(arrow, sectionTitle(sectionId)).arg(visible)); + divider->setText(0, tr("%1 %2 (%3)").arg(arrow, sectionTitle(section)).arg(visible)); } void UserListWidget::handleSectionExpansion(QTreeWidgetItem *item, bool expanded) @@ -1655,17 +1654,23 @@ void UserListWidget::handleSectionExpansion(QTreeWidgetItem *item, bool expanded if (!sectioned || item->type() != SectionItemType) { return; } - const QString sectionId = sectionItems.key(item); - if (sectionId.isEmpty()) { + // Reverse lookup. Only three dividers exist, so a linear scan over the + // section map is cheaper than caching the section on each divider. + auto dividerIt = sectionItems.constBegin(); + while (dividerIt != sectionItems.constEnd() && dividerIt.value() != item) { + ++dividerIt; + } + if (dividerIt == sectionItems.constEnd()) { return; } + const Section section = dividerIt.key(); if (expanded) { - expandedSections.insert(sectionId); + expandedSections.insert(section); } else { - expandedSections.remove(sectionId); + expandedSections.remove(section); } - updateSectionDivider(sectionId); // the arrow glyph follows the state - emit sectionExpanded(sectionId, expanded); + updateSectionDivider(section); // the arrow glyph follows the state + emit sectionExpanded(section, expanded); } void UserListWidget::setExpandedProgrammatically(QTreeWidgetItem *item, bool expanded) @@ -1674,23 +1679,23 @@ void UserListWidget::setExpandedProgrammatically(QTreeWidgetItem *item, bool exp item->setExpanded(expanded); } -void UserListWidget::setSectionExpanded(const QString §ionId, bool expanded) +void UserListWidget::setSectionExpanded(Section section, bool expanded) { if (!sectioned) { return; } if (expanded) { - expandedSections.insert(sectionId); + expandedSections.insert(section); } else { - expandedSections.remove(sectionId); + expandedSections.remove(section); } - QTreeWidgetItem *divider = sectionItems.value(sectionId); + QTreeWidgetItem *divider = sectionItems.value(section); if (!divider) { return; } QSignalBlocker blocker(userTree); divider->setExpanded(expanded); - updateSectionDivider(sectionId); // the arrow glyph follows the state + updateSectionDivider(section); // the arrow glyph follows the state userTree->viewport()->update(); } @@ -1699,12 +1704,12 @@ void UserListWidget::handleOnlineChange(const ServerInfo_User &user) // A user came online: they get a row in the "Online" section, plus (if // applicable) a row in the buddy/ignore sections, which flip to online. const QString name = QString::fromStdString(user.name()); - ensureSectionMembership(QStringLiteral("online"), user, true); + ensureSectionMembership(Section::Online, user, true); if (manager->isUserBuddy(name)) { - ensureSectionMembership(QStringLiteral("buddy"), user, true); + ensureSectionMembership(Section::Buddy, user, true); } if (manager->isUserIgnored(name)) { - ensureSectionMembership(QStringLiteral("ignore"), user, true); + ensureSectionMembership(Section::Ignore, user, true); } finishSectionedMutation(); } @@ -1714,7 +1719,7 @@ void UserListWidget::handleOnlineChangeLeft(const QString &userName) // The user is no longer online: their "Online" row disappears. Buddies and // ignored users keep their own section's row, marked offline. A plain user // has no rows left. - const bool dropped = dropSectionMembership(QStringLiteral("online"), userName); + const bool dropped = dropSectionMembership(Section::Online, userName); const bool kept = manager->isUserBuddy(userName) || manager->isUserIgnored(userName); if (kept) { setUserOnline(userName, false); @@ -1724,40 +1729,40 @@ void UserListWidget::handleOnlineChangeLeft(const QString &userName) } } -void UserListWidget::handleListAdd(const QString §ionId, const ServerInfo_User &user) +void UserListWidget::handleListAdd(Section section, const ServerInfo_User &user) { const QString name = QString::fromStdString(user.name()); const bool online = manager->getOnlineUser(name) != nullptr; - ensureSectionMembership(sectionId, user, online); + ensureSectionMembership(section, user, online); if (online) { // The user belongs to the "Online" section as well. Make sure the row // exists even if the join event raced ahead of the list mutation. - ensureSectionMembership(QStringLiteral("online"), user, true); + ensureSectionMembership(Section::Online, user, true); } finishSectionedMutation(); } -void UserListWidget::handleListRemove(const QString §ionId, const QString &userName) +void UserListWidget::handleListRemove(Section section, const QString &userName) { // Only the row of the removed section disappears: an online user keeps // their "Online" row, and other list memberships keep theirs. - if (dropSectionMembership(sectionId, userName)) { + if (dropSectionMembership(section, userName)) { finishSectionedMutation(); } } -UserListTWI *UserListWidget::ensureSectionMembership(const QString §ionId, const ServerInfo_User &user, bool online) +UserListTWI *UserListWidget::ensureSectionMembership(Section section, const ServerInfo_User &user, bool online) { const QString userName = QString::fromStdString(user.name()); updateCardArtParams(user, userName); - QTreeWidgetItem *divider = sectionItems.value(sectionId); + QTreeWidgetItem *divider = sectionItems.value(section); if (!divider) { return nullptr; } - QMap §ionMap = sectionUsers[sectionId]; + QMap §ionMap = sectionUsers[section]; UserListTWI *item = sectionMap.value(userName); if (!item) { item = new UserListTWI(user); @@ -1781,9 +1786,9 @@ UserListTWI *UserListWidget::ensureSectionMembership(const QString §ionId, c return item; } -bool UserListWidget::dropSectionMembership(const QString §ionId, const QString &userName) +bool UserListWidget::dropSectionMembership(Section section, const QString &userName) { - QMap §ionMap = sectionUsers[sectionId]; + QMap §ionMap = sectionUsers[section]; UserListTWI *item = sectionMap.take(userName); if (!item) { return false; diff --git a/cockatrice/src/interface/widgets/server/user/user_list_widget.h b/cockatrice/src/interface/widgets/server/user/user_list_widget.h index d7aeb3762..298a5f8d8 100644 --- a/cockatrice/src/interface/widgets/server/user/user_list_widget.h +++ b/cockatrice/src/interface/widgets/server/user/user_list_widget.h @@ -19,7 +19,6 @@ #include #include #include -#include #include #include #include @@ -150,6 +149,12 @@ public: BuddyList, IgnoreList }; + enum class Section + { + Buddy, + Online, + Ignore + }; private: UserListManager *manager = nullptr; @@ -181,31 +186,31 @@ private: // Sectioned mode (single tree with inline dividers) bool sectioned = false; - QStringList sectionIds; - QMap sectionItems; + QList
sectionIds; + QMap sectionItems; // One row per (section, user): a user that is online AND a buddy appears in // both the "Online" and the "Buddies" sections, so the same user can own // several rows, each hanging off its section's divider. - QMap> sectionUsers; - QSet expandedSections; + QMap> sectionUsers; + QSet
expandedSections; void createSectionItems(); - QTreeWidgetItem *createSectionItem(const QString §ionId); - [[nodiscard]] QString sectionTitle(const QString §ionId) const; - void updateSectionDivider(const QString §ionId); + QTreeWidgetItem *createSectionItem(Section section); + [[nodiscard]] QString sectionTitle(Section section) const; + void updateSectionDivider(Section section); void handleSectionExpansion(QTreeWidgetItem *item, bool expanded); void setExpandedProgrammatically(QTreeWidgetItem *item, bool expanded); void handleOnlineChange(const ServerInfo_User &user); void handleOnlineChangeLeft(const QString &userName); - void handleListAdd(const QString §ionId, const ServerInfo_User &user); - void handleListRemove(const QString §ionId, const QString &userName); - /** Creates or updates the row for @p user in @p sectionId. */ - UserListTWI *ensureSectionMembership(const QString §ionId, const ServerInfo_User &user, bool online); - /** Removes and deletes the row for @p userName in @p sectionId. */ - bool dropSectionMembership(const QString §ionId, const QString &userName); + void handleListAdd(Section section, const ServerInfo_User &user); + void handleListRemove(Section section, const QString &userName); + /** Creates or updates the row for @p user in @p section. */ + UserListTWI *ensureSectionMembership(Section section, const ServerInfo_User &user, bool online); + /** Removes and deletes the row for @p userName in @p section. */ + bool dropSectionMembership(Section section, const QString &userName); /** Sorts, refilters and repaints after a sectioned mode mutation. */ void finishSectionedMutation(); void updateCardArtParams(const ServerInfo_User &user, const QString &userName); - void processUserInfo(const QString §ionId, const ServerInfo_User &user, bool online); + void processUserInfo(Section section, const ServerInfo_User &user, bool online); QMap users; TabSupervisor *tabSupervisor; @@ -231,7 +236,7 @@ signals: void addIgnore(const QString &userName); void removeIgnore(const QString &userName); void joinGameRequested(int gameId, int roomId, bool asSpectator); - void sectionExpanded(const QString §ionId, bool expanded); + void sectionExpanded(Section section, bool expanded); public: UserListWidget(TabSupervisor *_tabSupervisor, @@ -251,9 +256,9 @@ public: void setUserOnline(const QString &userName, bool online); void setFilterText(const QString &text); void setShowTitle(bool showTitle); - void setSectioned(const QStringList &ids); - void setSectionExpanded(const QString §ionId, bool expanded); - [[nodiscard]] const QStringList &getSectionIds() const + void setSectioned(const QList
&ids); + void setSectionExpanded(Section section, bool expanded); + [[nodiscard]] const QList
&getSectionIds() const { return sectionIds; }