From a1a8a59f4a7a5d27cf3f7252beadbbac334d3378 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Mon, 2 Nov 2026 08:35:58 +0100 Subject: [PATCH 01/15] [VisualDatabase] Draw the card grid with a model/view (#7701) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [VisualDatabase] Draw the card grid with a model/view instead of a widget per card The visual card database built one CardInfoPictureWithTextOverlayWidget per card, 100 per page, and tore them down on every search, resize and scroll page. It also looked each card up again by name from the model row, kept a shadow QList, and carried paging state plus a 5 s timer that only existed to defer the first page. Replace that with a real view on a flattened model: - CardDatabaseDisplayModel gains CardInfoRole and cardInfoAt(), so a view can get the CardInfoPtr behind a proxy row without a name round trip. - New CardDatabaseVisualDisplayModel (libcockatrice_models) flattens the filtered and sorted proxy into one entry per displayed card picture: the preferred printing normally, every printing of the selected sets while set filters are active. Proxy changes rebuild it on a 30 ms coalescing timer. - New CardDatabaseVisualView (QListView IconMode) paints only visible rows through the new CardPictureDelegate, keeps the old interaction contract (left/right/middle press -> cardClicked, hover -> cardHovered, Ctrl+wheel resize, no selection, no focus stealing, scroll to top on a new result) and repaints when CardPictureLoader reports an async image. - CardSizeWidget::enableCtrlScrollResize() also watches scroll-area viewports, which is what makes Ctrl+wheel work over a QListView. - VisualDatabaseDisplayWidget drops the paging, the shadow list, the dead members and the 5 s timer. Behaviour-preserving: no gameplay change, no new user-facing strings. * [VisualDatabase] Fix the card grid wheel step QListView hands the delegate's item height to the scroll bar as its single step under ScrollPerPixel, so QAbstractSlider moved wheelScrollLines() whole card rows per notch. Handle the wheel in the view instead: one line is an eighth of the grid cell, so a notch moves about a third of a card row at any card size and still follows the OS line count. --------- Co-authored-by: Lukas Brübach --- cockatrice/CMakeLists.txt | 2 + cockatrice/src/filters/filter_tree_model.cpp | 72 +++--- cockatrice/src/filters/filter_tree_model.h | 14 ++ .../card_picture_loader.cpp | 1 + .../card_picture_loader/card_picture_loader.h | 8 + .../cards/card_info_picture_widget.cpp | 19 +- .../widgets/cards/card_info_picture_widget.h | 11 + .../widgets/cards/card_picture_delegate.cpp | 83 +++++++ .../widgets/cards/card_picture_delegate.h | 45 ++++ .../widgets/cards/card_size_widget.cpp | 30 ++- .../widgets/cards/card_size_widget.h | 9 +- .../card_database_visual_view.cpp | 203 +++++++++++++++++ .../card_database_visual_view.h | 67 ++++++ .../visual_database_display_widget.cpp | 215 +++--------------- .../visual_database_display_widget.h | 49 ++-- .../libcockatrice/filters/filter_card.h | 2 +- .../models/database/CMakeLists.txt | 2 + .../database/card_database_display_model.cpp | 22 +- .../database/card_database_display_model.h | 24 ++ .../card_database_visual_display_model.cpp | 123 ++++++++++ .../card_database_visual_display_model.h | 90 ++++++++ tests/carddatabase/CMakeLists.txt | 15 ++ .../visual_display_model_test.cpp | 188 +++++++++++++++ vcpkg | 2 +- 24 files changed, 1019 insertions(+), 277 deletions(-) create mode 100644 cockatrice/src/interface/widgets/cards/card_picture_delegate.cpp create mode 100644 cockatrice/src/interface/widgets/cards/card_picture_delegate.h create mode 100644 cockatrice/src/interface/widgets/visual_database_display/card_database_visual_view.cpp create mode 100644 cockatrice/src/interface/widgets/visual_database_display/card_database_visual_view.h create mode 100644 libcockatrice_models/libcockatrice/models/database/card_database_visual_display_model.cpp create mode 100644 libcockatrice_models/libcockatrice/models/database/card_database_visual_display_model.h create mode 100644 tests/carddatabase/visual_display_model_test.cpp diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index ce7ad5712..642d998a0 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -171,6 +171,7 @@ set(cockatrice_SOURCES src/interface/widgets/cards/card_info_picture_widget.cpp src/interface/widgets/cards/card_info_picture_with_text_overlay_widget.cpp src/interface/widgets/cards/card_info_text_widget.cpp + src/interface/widgets/cards/card_picture_delegate.cpp src/interface/widgets/cards/card_size_widget.cpp src/interface/widgets/cards/deck_card_zone_display_widget.cpp src/interface/widgets/cards/deck_preview_card_picture_widget.cpp @@ -290,6 +291,7 @@ set(cockatrice_SOURCES src/interface/widgets/utility/sequence_edit.cpp src/interface/widgets/utility/visibility_change_listener.cpp src/interface/widgets/utility/visibility_change_listener.h + src/interface/widgets/visual_database_display/card_database_visual_view.cpp src/interface/widgets/visual_database_display/visual_database_display_color_filter_widget.cpp src/interface/widgets/visual_database_display/visual_database_display_filter_save_load_widget.cpp src/interface/widgets/visual_database_display/visual_database_display_filter_toolbar_widget.cpp diff --git a/cockatrice/src/filters/filter_tree_model.cpp b/cockatrice/src/filters/filter_tree_model.cpp index b3bf307f1..9b55ce2fd 100644 --- a/cockatrice/src/filters/filter_tree_model.cpp +++ b/cockatrice/src/filters/filter_tree_model.cpp @@ -11,6 +11,27 @@ class QObject; +namespace +{ +/** + * @brief Visits every filter item below @p node, depth first. + */ +template void forEachFilter(const FilterTreeNode *node, const Callback &callback) +{ + if (node == nullptr) { + return; + } + + if (const auto *filterItem = dynamic_cast(node)) { + callback(filterItem); + } + + for (int i = 0; i < node->childCount(); ++i) { + forEachFilter(node->nodeAt(i), callback); + } +} +} // namespace + FilterTreeModel::FilterTreeModel(QObject *parent) : QAbstractItemModel(parent) { fTree = new FilterTree; @@ -110,46 +131,31 @@ void FilterTreeModel::clearFiltersOfType(CardFilter::Attr filterType) QList FilterTreeModel::getFiltersOfType(CardFilter::Attr filterType) const { QList filters; - if (!fTree) { - return filters; - } - - std::function traverse; - traverse = [&](const FilterTreeNode *node) { - if (const auto *filterItem = dynamic_cast(node)) { - if (filterItem->attr() == filterType) { - QString text = filterItem->text(); - filters.append(new CardFilter(text, filterItem->type(), filterItem->attr())); - } + forEachFilter(fTree, [&](const FilterItem *filterItem) { + if (filterItem->attr() == filterType) { + filters.append(new CardFilter(filterItem->text(), filterItem->type(), filterItem->attr())); } - for (int i = 0; i < node->childCount(); ++i) { - traverse(node->nodeAt(i)); - } - }; - - traverse(fTree); + }); return filters; } +QStringList FilterTreeModel::getFilterTermsOfType(CardFilter::Attr filterType) const +{ + QStringList terms; + forEachFilter(fTree, [&](const FilterItem *filterItem) { + if (filterItem->attr() == filterType) { + terms.append(filterItem->text()); + } + }); + return terms; +} + QList FilterTreeModel::allFilters() const { QList filters; - if (!fTree) { - return filters; - } - - std::function traverse; - traverse = [&](const FilterTreeNode *node) { - if (const auto *filterItem = dynamic_cast(node)) { - QString text = filterItem->text(); - filters.append(new CardFilter(text, filterItem->type(), filterItem->attr())); - } - for (int i = 0; i < node->childCount(); ++i) { - traverse(node->nodeAt(i)); - } - }; - - traverse(fTree); + forEachFilter(fTree, [&](const FilterItem *filterItem) { + filters.append(new CardFilter(filterItem->text(), filterItem->type(), filterItem->attr())); + }); return filters; } diff --git a/cockatrice/src/filters/filter_tree_model.h b/cockatrice/src/filters/filter_tree_model.h index 826fcc941..05f34f508 100644 --- a/cockatrice/src/filters/filter_tree_model.h +++ b/cockatrice/src/filters/filter_tree_model.h @@ -28,7 +28,14 @@ public slots: void addFilter(const CardFilter *f); void removeFilter(const CardFilter *f); void clearFiltersOfType(CardFilter::Attr filterType); + /** + * @brief Copies every active filter of one attribute into new CardFilter objects. + * + * The caller owns the returned filters. Prefer getFilterTermsOfType() when only + * the terms are needed. + */ [[nodiscard]] QList getFiltersOfType(CardFilter::Attr filterType) const; + /// @brief Copies every active filter into new CardFilter objects. The caller owns them. [[nodiscard]] QList allFilters() const; private slots: @@ -48,6 +55,13 @@ public: { return fTree; } + + /** + * @brief The texts of every active filter of one attribute, without allocating filters. + * + * Prefer this over getFiltersOfType() when only the filter terms are needed. + */ + [[nodiscard]] QStringList getFilterTermsOfType(CardFilter::Attr filterType) const; [[nodiscard]] int rowCount(const QModelIndex &parent = QModelIndex()) const override; [[nodiscard]] int columnCount(const QModelIndex & /*parent*/ = QModelIndex()) const override; [[nodiscard]] QVariant data(const QModelIndex &index, int role) const override; diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader.cpp index 4f77c7096..2d35538d2 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader.cpp @@ -238,6 +238,7 @@ void CardPictureLoader::imageLoaded(const ExactCard &card, const QImage &image) }); card.emitPixmapUpdated(); + emit getInstance().pixmapLoaded(card); } void CardPictureLoader::deleteAllLocalOverrides(const ExactCard &card) diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader.h b/cockatrice/src/interface/card_picture_loader/card_picture_loader.h index c6d91570b..0c73e5d59 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader.h +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader.h @@ -146,6 +146,14 @@ signals: /** @brief Emitted after the worker has finished clearing the network and redirect caches. */ void networkCacheCleared(); + /** + * @brief Emitted once an image has been loaded and put into the pixmap cache. + * + * Views that paint cards from the loader without owning a widget per card can + * listen to this to repaint the rows whose picture just became available. + */ + void pixmapLoaded(const ExactCard &card); + private slots: /** * @brief Triggered when the user changes the picture download settings. diff --git a/cockatrice/src/interface/widgets/cards/card_info_picture_widget.cpp b/cockatrice/src/interface/widgets/cards/card_info_picture_widget.cpp index 0d94e7209..8f02ebcde 100644 --- a/cockatrice/src/interface/widgets/cards/card_info_picture_widget.cpp +++ b/cockatrice/src/interface/widgets/cards/card_info_picture_widget.cpp @@ -43,11 +43,6 @@ #include #include -static constexpr qreal MTG_CARD_ASPECT_RATIO = 1.396; -// static constexpr qreal YUGIOH_CARD_ASPECT_RATIO = 1.457; -static constexpr qreal ASPECT_RATIO = MTG_CARD_ASPECT_RATIO; - -static constexpr int BASE_WIDTH = 200; static constexpr int BASE_HEIGHT = 200; static constexpr int ENLARGED_PIXMAP_OFFSET = 10; @@ -162,12 +157,11 @@ void CardInfoPictureWidget::resizeEvent(QResizeEvent *event) */ void CardInfoPictureWidget::setScaleFactor(const int scale) { - const int newWidth = BASE_WIDTH * scale / 100; - const int newHeight = static_cast(newWidth * ASPECT_RATIO); + const QSize newSize = sizeForScaleFactor(scale); scaleFactor = scale; - setFixedSize(newWidth, newHeight); + setFixedSize(newSize); updatePixmap(); emit cardScaleFactorChanged(scale); @@ -266,8 +260,13 @@ void CardInfoPictureWidget::paintEvent(QPaintEvent *event) */ QSize CardInfoPictureWidget::sizeHint() const { - return {static_cast(BASE_WIDTH * scaleFactor / 100.0), - static_cast(BASE_WIDTH * scaleFactor / 100.0 * ASPECT_RATIO)}; + return sizeForScaleFactor(scaleFactor); +} + +QSize CardInfoPictureWidget::sizeForScaleFactor(int scaleFactor) +{ + const int width = BASE_WIDTH * scaleFactor / 100; + return {width, static_cast(width * ASPECT_RATIO)}; } /** diff --git a/cockatrice/src/interface/widgets/cards/card_info_picture_widget.h b/cockatrice/src/interface/widgets/cards/card_info_picture_widget.h index 8d2d63736..0eb731d92 100644 --- a/cockatrice/src/interface/widgets/cards/card_info_picture_widget.h +++ b/cockatrice/src/interface/widgets/cards/card_info_picture_widget.h @@ -28,6 +28,11 @@ class CardInfoPictureWidget : public QWidget Q_OBJECT public: + /// Width of a card picture at 100 % scale, in pixels. + static constexpr int BASE_WIDTH = 200; + /// Height / width of a card picture. + static constexpr qreal ASPECT_RATIO = 1.396; + explicit CardInfoPictureWidget(QWidget *parent = nullptr, bool hoverToZoomEnabled = false, bool raiseOnEnter = false); @@ -37,6 +42,12 @@ public: } [[nodiscard]] QSize sizeHint() const override; + /** + * @brief The size a card picture has at @p scaleFactor percent. + * @param scaleFactor Percentage of BASE_WIDTH, as set by the card size slider. + */ + [[nodiscard]] static QSize sizeForScaleFactor(int scaleFactor); + public slots: void setCard(const ExactCard &card); void setScaleFactor(int scale); // New slot for scaling diff --git a/cockatrice/src/interface/widgets/cards/card_picture_delegate.cpp b/cockatrice/src/interface/widgets/cards/card_picture_delegate.cpp new file mode 100644 index 000000000..781c767ef --- /dev/null +++ b/cockatrice/src/interface/widgets/cards/card_picture_delegate.cpp @@ -0,0 +1,83 @@ +#include "card_picture_delegate.h" + +#include "../../../client/settings/cache_settings.h" +#include "../../card_picture_loader/card_picture_loader.h" +#include "card_art_utils.h" +#include "card_database_visual_display_model.h" +#include "libcockatrice/card/printing/exact_card.h" +#include "libcockatrice/settings/cards_display_settings.h" + +#include +#include +#include +#include +#include +#include + +CardPictureDelegate::CardPictureDelegate(CardDatabaseVisualDisplayModel *_model, QObject *parent) + : QStyledItemDelegate(parent), model(_model) +{ +} + +void CardPictureDelegate::setCardSize(const QSize &size) +{ + if (cardSize == size) { + return; + } + + cardSize = size; +} + +void CardPictureDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const +{ + if (model == nullptr) { + return; + } + + const ExactCard card = model->cardAt(index.row()); + if (!card) { + return; + } + + // The grid cell is the card plus SPACING; give the picture the rest. + const int inset = SPACING / 2; + const QRect cardArea = option.rect.adjusted(inset, inset, -inset, -inset); + + QPixmap pixmap; + CardPictureLoader::getCardBackLoadingInProgressPixmap(pixmap, cardArea.size()); + CardPictureLoader::getPixmap(pixmap, card, cardArea.size()); + + if (pixmap.isNull()) { + return; + } + + if (SettingsCache::instance().cardsDisplay().getAutoRotateSidewaysLayoutCards()) { + pixmap = CardArtUtils::rotateSidewaysLayoutArt(pixmap, card); + } + + // Fit the picture into the cell and center it, like CardInfoPictureWidget does. + const QSize pictureSize = pixmap.deviceIndependentSize().toSize().scaled(cardArea.size(), Qt::KeepAspectRatio); + const QRect pictureRect(cardArea.center().x() - pictureSize.width() / 2, + cardArea.center().y() - pictureSize.height() / 2, pictureSize.width(), + pictureSize.height()); + + const qreal radius = SettingsCache::instance().cardsDisplay().getRoundCardCorners() + ? 0.05 * static_cast(pictureRect.width()) + : 0.0; + + painter->save(); + painter->setRenderHint(QPainter::Antialiasing); + painter->setRenderHint(QPainter::SmoothPixmapTransform); + QPainterPath shape; + shape.addRoundedRect(pictureRect, radius, radius); + painter->setClipPath(shape); + // Source rect is in device pixels, target in logical ones: drawPixmap scales + // between them without the device pixel ratio being applied twice. + painter->drawPixmap(pictureRect, pixmap, QRectF(pixmap.rect())); + painter->restore(); +} + +QSize CardPictureDelegate::sizeHint(const QStyleOptionViewItem & /*option*/, const QModelIndex & /*index*/) const +{ + return cardSize + QSize(SPACING, SPACING); +} diff --git a/cockatrice/src/interface/widgets/cards/card_picture_delegate.h b/cockatrice/src/interface/widgets/cards/card_picture_delegate.h new file mode 100644 index 000000000..31e673624 --- /dev/null +++ b/cockatrice/src/interface/widgets/cards/card_picture_delegate.h @@ -0,0 +1,45 @@ +/** + * @file card_picture_delegate.h + * @ingroup CardWidgets + * @brief Paints a card picture for a visual card grid item. + */ + +#ifndef CARD_PICTURE_DELEGATE_H +#define CARD_PICTURE_DELEGATE_H + +#include +#include +#include + +class CardDatabaseVisualDisplayModel; + +/** + * @brief Draws the card of a CardDatabaseVisualDisplayModel row as a picture. + * + * The delegate only paints rows the view asks for, so a grid of the whole card + * database stays cheap: no widget per card and no pixmap loaded for items that + * are off screen. Card images come from CardPictureLoader, which caches them and + * loads missing ones asynchronously. + */ +class CardPictureDelegate : public QStyledItemDelegate +{ + Q_OBJECT + +public: + /// Gap left around a card picture inside its grid cell, in pixels. + static constexpr int SPACING = 4; + + explicit CardPictureDelegate(CardDatabaseVisualDisplayModel *_model, QObject *parent = nullptr); + + /// The picture size inside a grid cell. The cell is this size plus SPACING. + void setCardSize(const QSize &size); + + void paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const override; + [[nodiscard]] QSize sizeHint(const QStyleOptionViewItem &option, const QModelIndex &index) const override; + +private: + CardDatabaseVisualDisplayModel *model; + QSize cardSize; +}; + +#endif // CARD_PICTURE_DELEGATE_H diff --git a/cockatrice/src/interface/widgets/cards/card_size_widget.cpp b/cockatrice/src/interface/widgets/cards/card_size_widget.cpp index ab1840591..ac1ab9cb1 100644 --- a/cockatrice/src/interface/widgets/cards/card_size_widget.cpp +++ b/cockatrice/src/interface/widgets/cards/card_size_widget.cpp @@ -2,6 +2,7 @@ #include "../general/layout_containers/flow_widget.h" +#include #include #include #include @@ -33,8 +34,8 @@ CardSizeWidget::CardSizeWidget(QWidget *parent, FlowWidget *_flowWidget, int def cardSizeLabel->setSizePolicy(QSizePolicy::Minimum, QSizePolicy::Minimum); cardSizeSlider = new QSlider(Qt::Horizontal, this); - cardSizeSlider->setRange(50, 250); ///< Slider range for card size adjustment. - cardSizeSlider->setValue(defaultValue); ///< Initial slider value. + cardSizeSlider->setRange(MIN_CARD_SIZE, MAX_CARD_SIZE); ///< Slider range for card size adjustment. + cardSizeSlider->setValue(defaultValue); ///< Initial slider value. cardSizeLayout->addWidget(cardSizeLabel); cardSizeLayout->addWidget(cardSizeSlider); @@ -73,13 +74,26 @@ QSlider *CardSizeWidget::getSlider() const void CardSizeWidget::enableCtrlScrollResize(QWidget *host) { - host->installEventFilter(this); - - const auto scrollAreas = host->findChildren(); - for (QScrollArea *scrollArea : scrollAreas) { - if (QWidget *content = scrollArea->widget()) { - content->installEventFilter(this); + const auto watchScrollArea = [this](QAbstractScrollArea *scrollArea) { + if (scrollArea == nullptr) { + return; } + if (QWidget *viewport = scrollArea->viewport()) { + viewport->installEventFilter(this); + } + if (auto *contentScrollArea = qobject_cast(scrollArea)) { + if (QWidget *content = contentScrollArea->widget()) { + content->installEventFilter(this); + } + } + }; + + host->installEventFilter(this); + watchScrollArea(qobject_cast(host)); + + const auto scrollAreas = host->findChildren(); + for (QAbstractScrollArea *scrollArea : scrollAreas) { + watchScrollArea(scrollArea); } } diff --git a/cockatrice/src/interface/widgets/cards/card_size_widget.h b/cockatrice/src/interface/widgets/cards/card_size_widget.h index 7de97a5e9..fced90d33 100644 --- a/cockatrice/src/interface/widgets/cards/card_size_widget.h +++ b/cockatrice/src/interface/widgets/cards/card_size_widget.h @@ -24,6 +24,10 @@ class CardSizeWidget : public QWidget Q_OBJECT public: + /// Range of the card size slider, as a percentage of the base card width. + static constexpr int MIN_CARD_SIZE = 50; + static constexpr int MAX_CARD_SIZE = 250; + explicit CardSizeWidget(QWidget *parent, FlowWidget *flowWidget = nullptr, int defaultValue = 100); [[nodiscard]] QSlider *getSlider() const; @@ -31,8 +35,9 @@ public: * @brief Resizes the cards when the user Ctrl + scrolls over @p host or any of its descendants. * * Installs this widget as an event filter on the given host widget. If the host contains a - * scroll area the filter is also installed on the scroll area's content widget, so the resize - * intercepts the wheel event before the scroll area would scroll the view. + * scroll area the filter is also installed on the scroll area's content widget, and on the + * viewport of every scroll area base (QListView, QTableView, ...), so the resize intercepts + * the wheel event before the scroll area would scroll the view. */ void enableCtrlScrollResize(QWidget *host); diff --git a/cockatrice/src/interface/widgets/visual_database_display/card_database_visual_view.cpp b/cockatrice/src/interface/widgets/visual_database_display/card_database_visual_view.cpp new file mode 100644 index 000000000..7b0d526f9 --- /dev/null +++ b/cockatrice/src/interface/widgets/visual_database_display/card_database_visual_view.cpp @@ -0,0 +1,203 @@ +#include "card_database_visual_view.h" + +#include "../../card_picture_loader/card_picture_loader.h" +#include "../cards/card_info_picture_widget.h" +#include "../cards/card_picture_delegate.h" +#include "../cards/card_size_widget.h" +#include "card_database_visual_display_model.h" +#include "libcockatrice/card/printing/exact_card.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +CardDatabaseVisualView::CardDatabaseVisualView(CardDatabaseVisualDisplayModel *_model, QWidget *parent) + : QListView(parent), model(_model) +{ + auto *cardDelegate = new CardPictureDelegate(model, this); + setItemDelegate(cardDelegate); + setModel(model); + + setViewMode(QListView::IconMode); + setFlow(QListView::LeftToRight); + setWrapping(true); + setResizeMode(QListView::Adjust); + setMovement(QListView::Static); + setUniformItemSizes(true); + setLayoutMode(QListView::SinglePass); + setVerticalScrollMode(QAbstractItemView::ScrollPerPixel); + setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); + setFrameShape(QFrame::NoFrame); + setWordWrap(false); + + // QListView hands the delegate's item height to the scroll bar as its single + // step under ScrollPerPixel, so one wheel notch would jump wheelScrollLines() + // whole card rows. Give the bar a text line instead; the wheel handler below + // scales that line to the grid. A positive step also stops Qt from taking the + // item height back on the next layout. + verticalScrollBar()->setSingleStep(qMax(1, fontMetrics().height())); + + // The grid is a read-only surface: clicks are actions on a card, not a + // selection, and they must not steal focus from the search field. + setSelectionMode(QAbstractItemView::NoSelection); + setEditTriggers(QAbstractItemView::NoEditTriggers); + setFocusPolicy(Qt::NoFocus); + + // Hover drives the card preview. + setMouseTracking(true); + viewport()->setMouseTracking(true); + + connect(&CardPictureLoader::getInstance(), &CardPictureLoader::pixmapLoaded, this, + [this] { viewport()->update(); }); + + // A new search result starts at the top, like the widget-per-card grid did. + connect(model, &QAbstractItemModel::modelReset, this, [this] { + verticalScrollBar()->setValue(0); + // The rows under a stationary cursor changed with the reset: re-resolve the + // hover so the preview follows the grid, as enterEvent did per card widget. + hoveredRow = -1; + updateHoveredRow(viewport()->mapFromGlobal(QCursor::pos())); + }); + + setCardSizeFactor(cardSizeFactor); +} + +void CardDatabaseVisualView::setCardSizeFactor(int factor) +{ + cardSizeFactor = qBound(CardSizeWidget::MIN_CARD_SIZE, factor, CardSizeWidget::MAX_CARD_SIZE); + + const QSize cardSize = CardInfoPictureWidget::sizeForScaleFactor(cardSizeFactor); + if (auto *cardDelegate = qobject_cast(itemDelegate())) { + cardDelegate->setCardSize(cardSize); + } + // The delegate's sizeHint does not emit sizeHintChanged, so the grid size is what + // makes a resize take effect. Keep the two in step. + setGridSize(cardSize + QSize(CardPictureDelegate::SPACING, CardPictureDelegate::SPACING)); +} + +void CardDatabaseVisualView::mousePressEvent(QMouseEvent *event) +{ + const QModelIndex index = indexAt(event->pos()); + if (index.isValid() && model != nullptr) { + const ExactCard card = model->cardAt(index.row()); + if (card) { + emit cardClicked(event, card); + event->accept(); + return; + } + } + + QListView::mousePressEvent(event); +} + +void CardDatabaseVisualView::mouseMoveEvent(QMouseEvent *event) +{ + QListView::mouseMoveEvent(event); + updateHoveredRow(event->pos()); +} + +bool CardDatabaseVisualView::viewportEvent(QEvent *event) +{ + // A per-card widget reported its hover from enterEvent, which also fires when the + // grid changes under a stationary cursor. A QListView only reports moves, so + // re-resolve the hover on enter and clear it on leave. + if (event->type() == QEvent::Enter) { + updateHoveredRow(viewport()->mapFromGlobal(QCursor::pos())); + } else if (event->type() == QEvent::Leave) { + hoveredRow = -1; + } + + return QListView::viewportEvent(event); +} + +void CardDatabaseVisualView::wheelEvent(QWheelEvent *event) +{ + // Ctrl+wheel is the card-size gesture and is filtered out before it reaches + // the view; any other modifier keeps QAbstractSlider's own page scrolling. + if (event->modifiers() != Qt::NoModifier) { + QListView::wheelEvent(event); + return; + } + + QScrollBar *bar = verticalScrollBar(); + if (!bar->isVisible()) { + event->ignore(); + return; + } + + // Touchpads and high-resolution wheels already report pixels: follow them. + if (!event->pixelDelta().isNull()) { + bar->setValue(bar->value() - event->pixelDelta().y()); + event->accept(); + return; + } + + if (event->angleDelta().y() == 0) { + QListView::wheelEvent(event); + return; + } + + // Accumulate wheel deltas smaller than a notch, like QAbstractSlider does, so + // a fine-grained wheel still moves. + wheelRemainder += static_cast(event->angleDelta().y()) / 120.0; + const int notches = static_cast(wheelRemainder); + if (notches == 0) { + event->accept(); + return; + } + wheelRemainder -= notches; + + const int pixels = -notches * wheelStep(); + bar->setValue(qBound(bar->minimum(), bar->value() + pixels, bar->maximum())); + event->accept(); +} + +int CardDatabaseVisualView::wheelStep() const +{ + // A card row is far too coarse for one wheel line, and a text line is too + // fine, so a line is an eighth of the grid cell. A notch then moves about a + // third of a card row at any card size, and the OS line count still applies. + const int lineHeight = qMax(1, gridSize().height() / 8); + return qMax(1, QApplication::wheelScrollLines()) * lineHeight; +} + +void CardDatabaseVisualView::updateHoveredRow(const QPoint &position) +{ + // indexAt() also answers for points outside the viewport, so a cursor parked + // below the grid (on the size slider, say) would resolve to the row just under + // the visible area. Only a point inside the viewport can hover a card. + if (model == nullptr || !viewport()->rect().contains(position)) { + return; + } + + const QModelIndex index = indexAt(position); + const int row = index.isValid() ? index.row() : -1; + if (row == hoveredRow) { + return; + } + + hoveredRow = row; + if (row >= 0) { + const ExactCard card = model->cardAt(row); + if (card) { + emit cardHovered(card); + } + } +} + +void CardDatabaseVisualView::leaveEvent(QEvent *event) +{ + hoveredRow = -1; + QListView::leaveEvent(event); +} diff --git a/cockatrice/src/interface/widgets/visual_database_display/card_database_visual_view.h b/cockatrice/src/interface/widgets/visual_database_display/card_database_visual_view.h new file mode 100644 index 000000000..5f004484d --- /dev/null +++ b/cockatrice/src/interface/widgets/visual_database_display/card_database_visual_view.h @@ -0,0 +1,67 @@ +/** + * @file card_database_visual_view.h + * @ingroup VisualCardDatabaseWidgets + * @brief Icon-mode view of the card database, one card picture per item. + */ + +#ifndef CARD_DATABASE_VISUAL_VIEW_H +#define CARD_DATABASE_VISUAL_VIEW_H + +#include +#include +#include + +class CardDatabaseVisualDisplayModel; +class ExactCard; +class QEvent; +class QMouseEvent; +class QPoint; +class QWheelEvent; + +/** + * @brief A wrapping grid of card pictures. + * + * The view virtualizes painting: it only asks the delegate for the items that are + * on screen, so the whole card database can sit in the model without a widget (or + * pixmap) per card. Clicks and hover are reported with the exact card under the + * cursor, which is what the deck editor needs to add, decrement or preview a card. + */ +class CardDatabaseVisualView : public QListView +{ + Q_OBJECT + +public: + explicit CardDatabaseVisualView(CardDatabaseVisualDisplayModel *_model, QWidget *parent = nullptr); + + /** + * @brief Sets the card size as a percentage of the base card width. + * @param factor Percentage in the range of the card size slider. + */ + void setCardSizeFactor(int factor); + +signals: + /** @brief A card was pressed. The event is valid for the duration of the emission. */ + void cardClicked(QMouseEvent *event, const ExactCard &card); + /** @brief The cursor moved onto a different card. */ + void cardHovered(const ExactCard &card); + +protected: + void mousePressEvent(QMouseEvent *event) override; + void mouseMoveEvent(QMouseEvent *event) override; + void leaveEvent(QEvent *event) override; + bool viewportEvent(QEvent *event) override; + void wheelEvent(QWheelEvent *event) override; + +private: + void updateHoveredRow(const QPoint &position); + /// Pixels one wheel notch moves, scaled to the grid and the OS line count. + [[nodiscard]] int wheelStep() const; + + CardDatabaseVisualDisplayModel *model; + /// Fraction of a line left over from a wheel event smaller than one notch. + qreal wheelRemainder = 0.0; + int cardSizeFactor = 100; + int hoveredRow = -1; +}; + +#endif // CARD_DATABASE_VISUAL_VIEW_H diff --git a/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.cpp b/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.cpp index 30e0af66b..6037418ad 100644 --- a/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.cpp +++ b/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.cpp @@ -5,40 +5,29 @@ #include "../../../filters/syntax_help.h" #include "../../key_signals.h" #include "../../pixel_map_generator.h" -#include "../cards/card_info_picture_widget.h" -#include "../cards/card_info_picture_with_text_overlay_widget.h" #include "../cards/card_size_widget.h" #include "../deck_editor/card_database_view.h" #include "../general/layout_containers/flow_widget.h" #include "../utility/custom_line_edit.h" #include "card_database_display_model.h" #include "card_database_model.h" +#include "card_database_visual_display_model.h" +#include "card_database_visual_view.h" #include "deck_list_model.h" #include "libcockatrice/card/card_localization.h" #include "libcockatrice/card/database/card_database_loader.h" #include "libcockatrice/card/database/card_database_querier.h" #include "libcockatrice/card/printing/exact_card.h" -#include "libcockatrice/card/printing/printing_info.h" #include "libcockatrice/filters/filter_card.h" #include "libcockatrice/filters/filter_tree.h" #include "visual_database_display_color_filter_widget.h" #include "visual_database_display_filter_toolbar_widget.h" -#include #include -#include #include #include #include -#include -#include -#include -#include -#include #include -#include -#include -#include #include #include #include @@ -48,14 +37,11 @@ #include #include #include -#include -#include #include #include #include #include #include -#include class QMouseEvent; @@ -64,11 +50,6 @@ VisualDatabaseDisplayWidget::VisualDatabaseDisplayWidget(QWidget *parent, DeckListModel *deckListModel) : QWidget(parent) { - debounceTimer = new QTimer(this); - debounceTimer->setSingleShot(true); // Ensure it only fires once after the timeout - - connect(debounceTimer, &QTimer::timeout, this, &VisualDatabaseDisplayWidget::onSearchModelChanged); - // Create display model databaseDisplayModel = new CardDatabaseDisplayModel(this); databaseDisplayModel->setObjectName("databaseDisplayModel"); @@ -86,23 +67,12 @@ VisualDatabaseDisplayWidget::VisualDatabaseDisplayWidget(QWidget *parent, connect(&SettingsCache::instance().cardsDisplay(), &CardsDisplaySettings::cardSearchLanguageChanged, this, applyCardSearchLanguage); - cards = new QList; - connect(databaseDisplayModel, &CardDatabaseDisplayModel::modelDirty, this, - &VisualDatabaseDisplayWidget::modelDirty); - // Set up main layout and widgets setMinimumSize(0, 0); setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding); mainLayout = QtUtils::makeLayout(this, 0, 1); setLayout(mainLayout); - flowWidget = new FlowWidget(this, Qt::Horizontal, Qt::ScrollBarAlwaysOff, Qt::ScrollBarPolicy::ScrollBarAsNeeded); - cardSizeWidget = new CardSizeWidget(this, flowWidget, - SettingsCache::instance().cardsDisplay().getVisualDatabaseDisplayCardSize()); - connect(cardSizeWidget, &CardSizeWidget::cardSizeSettingUpdated, &SettingsCache::instance().cardsDisplay(), - &CardsDisplaySettings::setVisualDatabaseDisplayCardSize); - cardSizeWidget->enableCtrlScrollResize(flowWidget); - searchContainer = new FlowWidget(this, Qt::Horizontal, Qt::ScrollBarAlwaysOff, Qt::ScrollBarAlwaysOff); searchEdit = new SearchLineEdit(); @@ -159,6 +129,25 @@ VisualDatabaseDisplayWidget::VisualDatabaseDisplayWidget(QWidget *parent, connect(databaseView, &CardDatabaseView::relatedCardClicked, this, &VisualDatabaseDisplayWidget::onRelatedCardClicked); + // The visual grid is a real view on a flattened model: one row per displayed + // card picture, painted on demand. + visualDisplayModel = new CardDatabaseVisualDisplayModel(databaseDisplayModel, CardDatabaseManager::query(), this); + + cardView = new CardDatabaseVisualView(visualDisplayModel, this); + cardView->setObjectName("visualCardView"); + connect(cardView, &CardDatabaseVisualView::cardClicked, this, + &VisualDatabaseDisplayWidget::cardClickedDatabaseDisplay); + connect(cardView, &CardDatabaseVisualView::cardHovered, this, + &VisualDatabaseDisplayWidget::cardHoveredDatabaseDisplay); + + const int cardSize = SettingsCache::instance().cardsDisplay().getVisualDatabaseDisplayCardSize(); + cardSizeWidget = new CardSizeWidget(this, nullptr, cardSize); + connect(cardSizeWidget, &CardSizeWidget::cardSizeSettingUpdated, &SettingsCache::instance().cardsDisplay(), + &CardsDisplaySettings::setVisualDatabaseDisplayCardSize); + connect(cardSizeWidget->getSlider(), &QSlider::valueChanged, cardView, &CardDatabaseVisualView::setCardSizeFactor); + cardView->setCardSizeFactor(cardSize); + cardSizeWidget->enableCtrlScrollResize(cardView); + colorFilterWidget = new VisualDatabaseDisplayColorFilterWidget(this, filterModel); filterContainer = new VisualDatabaseDisplayFilterToolbarWidget(this, deckListModel); @@ -190,8 +179,6 @@ VisualDatabaseDisplayWidget::VisualDatabaseDisplayWidget(QWidget *parent, databaseLoadIndicator->setVisible(false); } - QScrollBar *scrollBar = flowWidget->scrollArea->verticalScrollBar(); - connect(scrollBar, &QScrollBar::valueChanged, [this](int /*value*/) { loadCurrentPage(); }); retranslateUi(); } @@ -210,7 +197,7 @@ void VisualDatabaseDisplayWidget::initialize() mainLayout->addWidget(databaseView); - mainLayout->addWidget(flowWidget); + mainLayout->addWidget(cardView); mainLayout->addWidget(cardSizeWidget); @@ -241,8 +228,6 @@ void VisualDatabaseDisplayWidget::initializeFilters() databaseDisplayModel->setFilterTree(filterModel->filterTree()); - QTimer::singleShot(5000, this, [this] { loadCurrentPage(); }); - retranslateUi(); }); } @@ -278,71 +263,36 @@ void VisualDatabaseDisplayWidget::updateDisplayModeButton() displayModeButton->setText(displayModeButton->isChecked() ? tr("Table") : tr("Visual")); } -void VisualDatabaseDisplayWidget::resizeEvent(QResizeEvent *event) -{ - QWidget::resizeEvent(event); - loadCurrentPage(); -} - void VisualDatabaseDisplayWidget::onDisplayModeChanged(bool checked) { updateDisplayModeButton(); if (checked) { // Table mode - flowWidget->setVisible(false); + cardView->setVisible(false); cardSizeWidget->setVisible(false); databaseView->setItemDelegate(new QStyledItemDelegate(databaseView)); databaseView->setVisible(true); } else { // Visual mode - flowWidget->setVisible(true); + cardView->setVisible(true); cardSizeWidget->setVisible(true); databaseView->setVisible(false); - populateCards(); } } -void VisualDatabaseDisplayWidget::onClick(QMouseEvent *event, const ExactCard &card) -{ - emit cardClickedDatabaseDisplay(event, card); -} - -void VisualDatabaseDisplayWidget::onHover(const ExactCard &hoveredCard) -{ - emit cardHoveredDatabaseDisplay(hoveredCard); -} - -void VisualDatabaseDisplayWidget::addCardToDisplay(const ExactCard &cardToAdd) -{ - cards->append(cardToAdd); - auto *display = new CardInfoPictureWithTextOverlayWidget(flowWidget, false); - display->setScaleFactor(cardSizeWidget->getSlider()->value()); - display->setCard(cardToAdd); - flowWidget->addWidget(display); - connect(display, &CardInfoPictureWithTextOverlayWidget::cardClicked, this, &VisualDatabaseDisplayWidget::onClick); - connect(display, &CardInfoPictureWithTextOverlayWidget::hoveredOnCard, this, &VisualDatabaseDisplayWidget::onHover); - connect(cardSizeWidget->getSlider(), &QSlider::valueChanged, display, &CardInfoPictureWidget::setScaleFactor); -} - bool VisualDatabaseDisplayWidget::isVisualDisplayMode() const { return !displayModeButton->isChecked(); } +QStringList VisualDatabaseDisplayWidget::collectSetFilterTerms() const +{ + return filterModel->getFilterTermsOfType(CardFilter::AttrSet); +} + void VisualDatabaseDisplayWidget::onSearchModelChanged() { - if (flowWidget->isVisible()) { - // Clear the current page and prepare for new data - flowWidget->clearLayout(); // Clear existing cards - cards->clear(); // Clear the card list - // Reset scrollbar position to the top after loading new cards - QScrollBar *scrollBar = flowWidget->scrollArea->verticalScrollBar(); - scrollBar->setValue(0); // Reset scrollbar to top - - currentPage = 0; - loadCurrentPage(); - qCDebug(VisualDatabaseDisplayLog) << "Search model changed"; - } + visualDisplayModel->setSetFilterTerms(collectSetFilterTerms()); } void VisualDatabaseDisplayWidget::onSelectedCardChanged(const QString &cardName) @@ -368,108 +318,3 @@ void VisualDatabaseDisplayWidget::onRelatedCardClicked(const QString &relatedCar ExactCard exactCard = CardDatabaseManager::query()->guessCard({relatedCard}); emit cardInfoRequested(exactCard); } - -bool VisualDatabaseDisplayWidget::nearEndOfPage() const -{ - if (!flowWidget->isVisible()) { - return false; - } - - QScrollBar *scrollBar = flowWidget->scrollArea->verticalScrollBar(); - if (scrollBar->value() + scrollBar->pageStep() * 2 < scrollBar->maximum()) { - return false; // there is at least two pages of space to scroll remaining - } - - return true; -} - -void VisualDatabaseDisplayWidget::loadCurrentPage() -{ - // Ensure only the initial page is loaded - if (currentPage == 0) { - if (!initialLoadScheduled) { - initialLoadScheduled = true; - qCDebug(VisualDatabaseDisplayLog) << "Loading the first page"; - // Defer the first page so the tab switch stays responsive. The card - // grid builds one event loop turn later. This also applies to - // search-driven reloads, which reset currentPage back to 0. - QTimer::singleShot(0, this, [this] { - initialLoadScheduled = false; - populateCards(); - }); - } - } else if (nearEndOfPage()) { - // If not the first page, just load the next page and append to the flow widget - loadNextPage(); - } -} - -void VisualDatabaseDisplayWidget::populateCards() -{ - cards->clear(); - loadNextPage(); -} - -void VisualDatabaseDisplayWidget::loadNextPage() -{ - // Calculate the start and end indices for the next page - int rowCount = databaseDisplayModel->rowCount(); - int start = currentPage * cardsPerPage; - int end = qMin(start + cardsPerPage, rowCount); - - // Load the next page of cards and add them to the flow widget - flowWidget->setUpdatesEnabled(false); - loadPage(start, end); - flowWidget->setUpdatesEnabled(true); -} - -void VisualDatabaseDisplayWidget::loadPage(int start, int end) -{ - QList setFilters = filterModel->getFiltersOfType(CardFilter::AttrSet); - for (int row = start; row < end; ++row) { - qCDebug(VisualDatabaseDisplayLog) << "Adding " << row; - QModelIndex index = databaseDisplayModel->index(row, CardDatabaseModel::NameColumn); - QVariant name = databaseDisplayModel->data(index, Qt::DisplayRole); - qCDebug(VisualDatabaseDisplayLog) << name.toString(); - - if (CardInfoPtr info = CardDatabaseManager::query()->getCardInfo(name.toString())) { - if (!setFilters.empty()) { - const auto &setMap = info->getSets(); - for (const CardFilter *setFilter : setFilters) { - const auto it = setMap.constFind(setFilter->term()); - if (it != setMap.constEnd()) { - for (const PrintingInfo &printing : it.value()) { - addCardToDisplay(ExactCard(info, printing)); - } - } - } - } else { - addCardToDisplay(CardDatabaseManager::query()->getPreferredCard(info)); - } - } else { - qCDebug(VisualDatabaseDisplayLog) << "Card not found in database!"; - } - } - currentPage++; -} - -void VisualDatabaseDisplayWidget::modelDirty() const -{ - debounceTimer->start(debounceTime); -} - -void VisualDatabaseDisplayWidget::sortCardList(const QStringList &properties, - Qt::SortOrder order = Qt::AscendingOrder) const -{ - CardInfoComparator comparator(properties, order); - std::sort(cards->begin(), cards->end(), [comparator](const ExactCard &a, const ExactCard &b) { - return comparator(a.getCardPtr(), b.getCardPtr()); - }); -} - -void VisualDatabaseDisplayWidget::databaseDataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight) -{ - (void)topLeft; - (void)bottomRight; - qCDebug(VisualDatabaseDisplayLog) << "Database Data changed"; -} diff --git a/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.h b/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.h index 15adca214..6148670a3 100644 --- a/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.h +++ b/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.h @@ -9,8 +9,6 @@ #include "libcockatrice/card/card_info.h" -#include -#include #include #include #include @@ -21,26 +19,21 @@ class CardDatabaseDisplayModel; class CardDatabaseModel; class CardDatabaseView; +class CardDatabaseVisualDisplayModel; +class CardDatabaseVisualView; class CardSizeWidget; -class DeckList; class DeckListModel; class ExactCard; class FilterTreeModel; class FlowWidget; -class OverlapControlWidget; class QLabel; -class QModelIndex; +class QMouseEvent; class QPushButton; -class QScrollArea; -class QTimer; class QToolButton; class QVBoxLayout; class SearchLineEdit; class VisualDatabaseDisplayColorFilterWidget; class VisualDatabaseDisplayFilterToolbarWidget; -template class QList; - -inline Q_LOGGING_CATEGORY(VisualDatabaseDisplayLog, "visual_database_display"); class VisualDatabaseDisplayWidget : public QWidget { @@ -52,14 +45,6 @@ public: DeckListModel *deckListModel = nullptr); void retranslateUi(); - void adjustCardsPerPage(); - void populateCards(); - void loadPage(int start, int end); - void loadNextPage(); - void loadCurrentPage(); - void sortCardList(const QStringList &properties, Qt::SortOrder order) const; - void setDeckList(const DeckList &new_deck_list_model); - /** * @brief Sets the callback used to create a custom zone from the add-to-zone menu. * The callback returns the name of the created zone, or an empty string if creation was cancelled. @@ -87,6 +72,13 @@ public: bool isVisualDisplayMode() const; public slots: + /** + * @brief Pushes the active set filters to the visual grid. + * + * The grid's rows follow the display model's filter and sort signals on their + * own; this only forwards the set filters, which decide which printings the + * grid shows. + */ void onSearchModelChanged(); signals: @@ -101,11 +93,6 @@ signals: protected slots: void initialize(); - void onClick(QMouseEvent *event, const ExactCard &card); - void onHover(const ExactCard &hoveredCard); - void addCardToDisplay(const ExactCard &cardToAdd); - void databaseDataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight); - void modelDirty() const; void onDisplayModeChanged(bool checked); void onSelectedCardChanged(const QString &cardName); @@ -125,31 +112,21 @@ private: QToolButton *clearFilterWidget; VisualDatabaseDisplayFilterToolbarWidget *filterContainer; CardDatabaseDisplayModel *databaseDisplayModel; + CardDatabaseVisualDisplayModel *visualDisplayModel; CardDatabaseView *databaseView; + CardDatabaseVisualView *cardView; std::function newZoneCreator; - QList *cards; QVBoxLayout *mainLayout; - QScrollArea *scrollArea; - FlowWidget *flowWidget; - QWidget *overlapCategories; - QVBoxLayout *overlapCategoriesLayout; - OverlapControlWidget *overlapControlWidget; CardSizeWidget *cardSizeWidget; - QTimer *debounceTimer; - int debounceTime = 300; // in Ms - int currentPage = 0; // Current page index - int cardsPerPage = 100; // Number of cards per page bool filtersInitialized = false; - bool initialLoadScheduled = false; void initializeFilters(); void highlightAllSearchEdit(); void updateDisplayModeButton(); - bool nearEndOfPage() const; + [[nodiscard]] QStringList collectSetFilterTerms() const; protected: - void resizeEvent(QResizeEvent *event) override; void showEvent(QShowEvent *event) override; }; diff --git a/libcockatrice_filters/libcockatrice/filters/filter_card.h b/libcockatrice_filters/libcockatrice/filters/filter_card.h index e6a37ee40..dbeffd18f 100644 --- a/libcockatrice_filters/libcockatrice/filters/filter_card.h +++ b/libcockatrice_filters/libcockatrice/filters/filter_card.h @@ -54,7 +54,7 @@ private: enum Attr a; public: - CardFilter(QString &term, Type type, Attr attr) : trm(term), t(type), a(attr) + CardFilter(const QString &term, Type type, Attr attr) : trm(term), t(type), a(attr) { } diff --git a/libcockatrice_models/libcockatrice/models/database/CMakeLists.txt b/libcockatrice_models/libcockatrice/models/database/CMakeLists.txt index 5cf3caa52..22a04cd32 100644 --- a/libcockatrice_models/libcockatrice/models/database/CMakeLists.txt +++ b/libcockatrice_models/libcockatrice/models/database/CMakeLists.txt @@ -1,6 +1,7 @@ set(HEADERS card_database_model.h card_database_display_model.h + card_database_visual_display_model.h card/card_completer_proxy_model.h card/card_search_model.h card_set/card_sets_model.h @@ -13,6 +14,7 @@ add_library( ${HEADERS} card_database_model.cpp card_database_display_model.cpp + card_database_visual_display_model.cpp card/card_completer_proxy_model.cpp card/card_search_model.cpp card_set/card_sets_model.cpp diff --git a/libcockatrice_models/libcockatrice/models/database/card_database_display_model.cpp b/libcockatrice_models/libcockatrice/models/database/card_database_display_model.cpp index 6ed9d8827..9ec0afd9c 100644 --- a/libcockatrice_models/libcockatrice/models/database/card_database_display_model.cpp +++ b/libcockatrice_models/libcockatrice/models/database/card_database_display_model.cpp @@ -21,7 +21,7 @@ class QObject; CardDatabaseDisplayModel::CardDatabaseDisplayModel(QObject *parent) - : QSortFilterProxyModel(parent), isToken(ShowAll), filterString(nullptr), + : QSortFilterProxyModel(parent), isToken(ShowAll), filterString(nullptr), cardModel(nullptr), collator(NaturalSort::makeNaturalSortCollator()) { filterTree = nullptr; @@ -37,6 +37,7 @@ CardDatabaseDisplayModel::CardDatabaseDisplayModel(QObject *parent) void CardDatabaseDisplayModel::setSourceModel(QAbstractItemModel *model) { + cardModel = qobject_cast(model); QSortFilterProxyModel::setSourceModel(model); connect(model, &QAbstractItemModel::rowsInserted, this, [this]() { dirty(); }); @@ -46,6 +47,25 @@ void CardDatabaseDisplayModel::setSourceModel(QAbstractItemModel *model) connect(model, &QAbstractItemModel::modelReset, this, [this]() { dirty(); }); } +QVariant CardDatabaseDisplayModel::data(const QModelIndex &index, int role) const +{ + if (role == CardInfoRole) { + return QVariant::fromValue(cardInfoAt(index)); + } + + return QSortFilterProxyModel::data(index, role); +} + +CardInfoPtr CardDatabaseDisplayModel::cardInfoAt(const QModelIndex &index) const +{ + if (!index.isValid() || cardModel == nullptr) { + return {}; + } + + const QModelIndex sourceIndex = mapToSource(index); + return sourceIndex.isValid() ? cardModel->getCard(sourceIndex.row()) : CardInfoPtr(); +} + QMap CardDatabaseDisplayModel::characterTranslation = {{L'“', L'\"'}, {L'”', L'\"'}, {L'‘', L'\''}, diff --git a/libcockatrice_models/libcockatrice/models/database/card_database_display_model.h b/libcockatrice_models/libcockatrice/models/database/card_database_display_model.h index be074bba5..3a9bbc6ab 100644 --- a/libcockatrice_models/libcockatrice/models/database/card_database_display_model.h +++ b/libcockatrice_models/libcockatrice/models/database/card_database_display_model.h @@ -21,6 +21,7 @@ #include #include +class CardDatabaseModel; class FilterTree; class QObject; template class QMap; @@ -36,12 +37,25 @@ public: ShowAll }; + enum Role + { + /** + * @brief The CardInfoPtr behind a proxy row. + * + * Saves views from looking the card up again through its name, and keeps + * them from depending on the source model's column layout. + */ + CardInfoRole = Qt::UserRole + 1 + }; + private: FilterBool isToken; QString cardName, cardText; QSet cardNameSet, cardTypes, cardColors; FilterTree *filterTree; FilterString *filterString; + /// The source model, kept typed so CardInfoRole does not cast per row. + CardDatabaseModel *cardModel; QTimer dirtyTimer; CardSearchLanguage searchLanguage; QString searchText; @@ -84,6 +98,16 @@ public: dirtyTimer.start(20); } void clearFilterAll(); + [[nodiscard]] QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override; + + /** + * @brief Returns the card behind a proxy row, without the QVariant wrapper of CardInfoRole. + * + * The visual grid calls this for every row on each rebuild; CardInfoRole stays the + * generic way in for consumers that only see the model through data(). + */ + [[nodiscard]] CardInfoPtr cardInfoAt(const QModelIndex &index) const; + signals: void modelDirty(); diff --git a/libcockatrice_models/libcockatrice/models/database/card_database_visual_display_model.cpp b/libcockatrice_models/libcockatrice/models/database/card_database_visual_display_model.cpp new file mode 100644 index 000000000..f6a3a925c --- /dev/null +++ b/libcockatrice_models/libcockatrice/models/database/card_database_visual_display_model.cpp @@ -0,0 +1,123 @@ +#include "card_database_visual_display_model.h" + +#include "card_database_display_model.h" +#include "libcockatrice/card/database/card_database_querier.h" + +#include +#include +#include +#include + +/// Coalesces the bursts of proxy signals a filter, sort or database load produces. +static constexpr int REBUILD_COALESCE_MS = 30; + +CardDatabaseVisualDisplayModel::CardDatabaseVisualDisplayModel(CardDatabaseDisplayModel *_displayModel, + CardDatabaseQuerier *_querier, + QObject *parent) + : QAbstractListModel(parent), displayModel(_displayModel), querier(_querier) +{ + rebuildTimer.setSingleShot(true); + rebuildTimer.setInterval(REBUILD_COALESCE_MS); + connect(&rebuildTimer, &QTimer::timeout, this, &CardDatabaseVisualDisplayModel::rebuild); + + if (displayModel == nullptr) { + return; + } + + connect(displayModel, &QAbstractItemModel::modelReset, this, &CardDatabaseVisualDisplayModel::scheduleRebuild); + connect(displayModel, &QAbstractItemModel::layoutChanged, this, &CardDatabaseVisualDisplayModel::scheduleRebuild); + connect(displayModel, &QAbstractItemModel::rowsInserted, this, &CardDatabaseVisualDisplayModel::scheduleRebuild); + connect(displayModel, &QAbstractItemModel::rowsRemoved, this, &CardDatabaseVisualDisplayModel::scheduleRebuild); + connect(displayModel, &QAbstractItemModel::dataChanged, this, &CardDatabaseVisualDisplayModel::scheduleRebuild); +} + +void CardDatabaseVisualDisplayModel::setSetFilterTerms(const QStringList &terms) +{ + if (setFilterTerms == terms) { + return; + } + + setFilterTerms = terms; + refresh(); +} + +void CardDatabaseVisualDisplayModel::refresh() +{ + rebuildTimer.stop(); + rebuild(); +} + +void CardDatabaseVisualDisplayModel::scheduleRebuild() +{ + rebuildTimer.start(); +} + +void CardDatabaseVisualDisplayModel::rebuild() +{ + beginResetModel(); + entries.clear(); + + if (displayModel != nullptr) { + const int proxyRows = displayModel->rowCount(); + entries.reserve(proxyRows); + for (int row = 0; row < proxyRows; ++row) { + const CardInfoPtr info = displayModel->cardInfoAt(displayModel->index(row, 0)); + if (info) { + appendPrintingsFor(info); + } + } + } + + endResetModel(); +} + +void CardDatabaseVisualDisplayModel::appendPrintingsFor(const CardInfoPtr &info) +{ + if (setFilterTerms.isEmpty()) { + entries.append({info, PrintingInfo()}); + return; + } + + const auto &printingsBySet = info->getSets(); + for (const QString &term : setFilterTerms) { + const auto printings = printingsBySet.constFind(term); + if (printings == printingsBySet.constEnd()) { + continue; + } + for (const PrintingInfo &printing : printings.value()) { + entries.append({info, printing}); + } + } +} + +int CardDatabaseVisualDisplayModel::rowCount(const QModelIndex &parent) const +{ + return parent.isValid() ? 0 : static_cast(entries.size()); +} + +QVariant CardDatabaseVisualDisplayModel::data(const QModelIndex &index, int role) const +{ + if (!index.isValid() || index.row() >= entries.size()) { + return {}; + } + + if (role == Qt::DisplayRole || role == Qt::AccessibleTextRole) { + return cardAt(index.row()).getName(); + } + + return {}; +} + +ExactCard CardDatabaseVisualDisplayModel::cardAt(int row) const +{ + if (row < 0 || row >= entries.size()) { + return {}; + } + + const Entry &entry = entries.at(row); + if (entry.printing.isEmpty() && setFilterTerms.isEmpty()) { + return querier != nullptr ? querier->getPreferredCard(entry.info) : ExactCard(entry.info); + } + + return ExactCard(entry.info, entry.printing); +} diff --git a/libcockatrice_models/libcockatrice/models/database/card_database_visual_display_model.h b/libcockatrice_models/libcockatrice/models/database/card_database_visual_display_model.h new file mode 100644 index 000000000..1303b6464 --- /dev/null +++ b/libcockatrice_models/libcockatrice/models/database/card_database_visual_display_model.h @@ -0,0 +1,90 @@ +/** + * @file card_database_visual_display_model.h + * @ingroup CardDatabaseModels + * @brief A list model of the exact cards a visual card grid paints. + * + * The filtered and sorted CardDatabaseDisplayModel holds one row per card name. + * A visual grid shows one picture per *printing* instead, so this model flattens + * the proxy rows into ExactCards: the preferred printing per card normally, and + * every printing of the selected sets while set filters are active. + */ + +#ifndef COCKATRICE_CARD_DATABASE_VISUAL_DISPLAY_MODEL_H +#define COCKATRICE_CARD_DATABASE_VISUAL_DISPLAY_MODEL_H + +#include "libcockatrice/card/card_info.h" +#include "libcockatrice/card/printing/exact_card.h" +#include "libcockatrice/card/printing/printing_info.h" + +#include +#include +#include +#include +#include +#include +#include + +class CardDatabaseDisplayModel; +class CardDatabaseQuerier; + +class CardDatabaseVisualDisplayModel : public QAbstractListModel +{ + Q_OBJECT + +public: + /** + * @brief Flattens _displayModel into the cards a visual grid displays. + * @param _displayModel The filtered and sorted proxy model of the card database. + * @param _querier Resolves the preferred printing of a card. + * @param parent Parent object. + */ + explicit CardDatabaseVisualDisplayModel(CardDatabaseDisplayModel *_displayModel, + CardDatabaseQuerier *_querier, + QObject *parent = nullptr); + + /** + * @brief Sets the set shorthand names of the active set filters. + * + * With no terms every card is shown with its preferred printing. With terms + * every printing belonging to one of the named sets is shown instead, in term + * order. Rebuilds immediately when the terms change. + */ + void setSetFilterTerms(const QStringList &terms); + + /** + * @brief Rebuilds the row list from the proxy model right away. + * + * Proxy model changes normally rebuild on a short coalescing timer; call this + * when the caller needs the new rows before returning. + */ + void refresh(); + + [[nodiscard]] int rowCount(const QModelIndex &parent = QModelIndex()) const override; + [[nodiscard]] QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override; + + /** + * @brief Returns the card painted at @p row, with its preferred printing resolved. + * @param row Row in this model, not in the proxy model. + */ + [[nodiscard]] ExactCard cardAt(int row) const; + +private: + struct Entry + { + CardInfoPtr info; + /// Empty when the entry follows the preferred printing rather than a set filter. + PrintingInfo printing; + }; + + void scheduleRebuild(); + void rebuild(); + void appendPrintingsFor(const CardInfoPtr &info); + + QPointer displayModel; + CardDatabaseQuerier *querier; + QStringList setFilterTerms; + QList entries; + QTimer rebuildTimer; +}; + +#endif // COCKATRICE_CARD_DATABASE_VISUAL_DISPLAY_MODEL_H diff --git a/tests/carddatabase/CMakeLists.txt b/tests/carddatabase/CMakeLists.txt index 22171a9e8..7d83640a1 100644 --- a/tests/carddatabase/CMakeLists.txt +++ b/tests/carddatabase/CMakeLists.txt @@ -35,6 +35,21 @@ if(WITH_ORACLE OR WITH_CLIENT) LIBS libcockatrice_filters ) + + # ------------------------ + # Visual Display Model Test + # ------------------------ + cockatrice_add_test( + NAME + visual_display_model_test + SOURCES + ${VERSION_STRING_CPP} + visual_display_model_test.cpp + mocks.cpp + LIBS + libcockatrice_card + libcockatrice_models + ) endif() # ------------------------ diff --git a/tests/carddatabase/visual_display_model_test.cpp b/tests/carddatabase/visual_display_model_test.cpp new file mode 100644 index 000000000..5fa554bb3 --- /dev/null +++ b/tests/carddatabase/visual_display_model_test.cpp @@ -0,0 +1,188 @@ +/** + * @file visual_display_model_test.cpp + * @brief Tests for the flattened model behind the visual card database grid. + */ + +#include "card_database_display_model.h" +#include "card_database_model.h" +#include "card_database_visual_display_model.h" +#include "libcockatrice/card/card_info.h" +#include "libcockatrice/card/database/card_database.h" +#include "libcockatrice/card/database/card_database_querier.h" +#include "libcockatrice/card/printing/exact_card.h" +#include "libcockatrice/card/printing/printing_info.h" +#include "libcockatrice/card/set/card_set.h" +#include "libcockatrice/interfaces/noop_card_preference_provider.h" +#include "libcockatrice/interfaces/noop_card_set_priority_controller.h" +#include "test_card_database_path_provider.h" + +#include "gtest/gtest.h" +#include +#include +#include +#include +#include +#include +#include +#include + +namespace +{ +// GTest's main() creates no QCoreApplication, which QTimer needs for its event loop. +const QCoreApplication &application() +{ + static int argc = 1; + static char executable[] = "visual_display_model_test"; + static char *argv[] = {executable, nullptr}; + static const QCoreApplication app(argc, argv); + return app; +} + +/// Lets the rebuild timer fire, as it would in the running client. +void waitForRebuild() +{ + QEventLoop loop; + QTimer::singleShot(100, &loop, &QEventLoop::quit); + loop.exec(); +} + +class CardDatabaseVisualDisplayModelTest : public ::testing::Test +{ +protected: + void SetUp() override + { + (void)application(); + + db = std::make_unique(nullptr, new NoopCardPreferenceProvider(), + new TestCardDatabasePathProvider(), new NoopCardSetPriorityController()); + db->loadCardDatabases(); + + cardModel = std::make_unique(db.get(), false); + displayModel = std::make_unique(); + displayModel->setSourceModel(cardModel.get()); + visualModel = std::make_unique(displayModel.get(), db->query()); + visualModel->refresh(); + } + + /** + * @brief Builds a card with @p printingCount printings in each named set. + * @param printingCountPerSet Set short name to the number of printings in that set. + */ + [[nodiscard]] CardInfoPtr makeCard(const QString &name, const QMap &printingCountPerSet) + { + SetToPrintingsMap setsInfo; + for (auto it = printingCountPerSet.constBegin(); it != printingCountPerSet.constEnd(); ++it) { + CardSetPtr set = CardSet::newInstance(&setController, it.key(), it.key() + " test set", "expansion", + QDate(), CardSet::PriorityPrimary); + for (int printing = 0; printing < it.value(); ++printing) { + QHash properties; + properties["uuid"] = QString("%1-%2-%3").arg(it.key(), name).arg(printing); + setsInfo[it.key()].append(PrintingInfo(set, LazyPropertiesHash(properties))); + } + } + + CardInfo::UiAttributes attributes = {.tableRow = 1}; + return CardInfo::newInstance(name, "", false, {}, {}, {}, setsInfo, attributes); + } + + NoopCardSetPriorityController setController; + std::unique_ptr db; + std::unique_ptr cardModel; + std::unique_ptr displayModel; + std::unique_ptr visualModel; +}; + +TEST_F(CardDatabaseVisualDisplayModelTest, ExposesTheCardInfoBehindAProxyRow) +{ + ASSERT_GT(displayModel->rowCount(), 0); + + const QVariant info = displayModel->data(displayModel->index(0, CardDatabaseModel::NameColumn), + CardDatabaseDisplayModel::CardInfoRole); + ASSERT_TRUE(info.isValid()); + EXPECT_FALSE(info.value().isNull()); +} + +TEST_F(CardDatabaseVisualDisplayModelTest, FlattensOneRowPerProxyCardWithoutSetFilters) +{ + ASSERT_GT(displayModel->rowCount(), 0); + EXPECT_EQ(displayModel->rowCount(), visualModel->rowCount()); + + for (int row = 0; row < visualModel->rowCount(); ++row) { + const ExactCard card = visualModel->cardAt(row); + ASSERT_TRUE(card) << "row " << row; + EXPECT_EQ( + displayModel->data(displayModel->index(row, CardDatabaseModel::NameColumn), Qt::DisplayRole).toString(), + card.getName()) + << "row " << row; + } +} + +TEST_F(CardDatabaseVisualDisplayModelTest, ShowsOneRowPerCardWhenNoSetIsFiltered) +{ + const int rowsBefore = visualModel->rowCount(); + + db->addCard(makeCard("Multi", {{"TST", 2}, {"OTH", 1}})); + visualModel->refresh(); + + EXPECT_EQ(rowsBefore + 1, visualModel->rowCount()); + EXPECT_EQ("Multi", visualModel->cardAt(visualModel->rowCount() - 1).getName()); +} + +TEST_F(CardDatabaseVisualDisplayModelTest, ShowsEveryPrintingOfAFilteredSet) +{ + db->addCard(makeCard("Multi", {{"TST", 2}, {"OTH", 1}})); + visualModel->refresh(); + + visualModel->setSetFilterTerms({"TST"}); + + ASSERT_EQ(2, visualModel->rowCount()); + for (int row = 0; row < visualModel->rowCount(); ++row) { + const ExactCard card = visualModel->cardAt(row); + ASSERT_TRUE(card) << "row " << row; + EXPECT_EQ("Multi", card.getName()); + ASSERT_FALSE(card.getPrinting().isEmpty()) << "row " << row; + EXPECT_EQ("TST", card.getPrinting().getSet()->getShortName()) << "row " << row; + } + EXPECT_NE(visualModel->cardAt(0).getPrinting(), visualModel->cardAt(1).getPrinting()); +} + +TEST_F(CardDatabaseVisualDisplayModelTest, HidesCardsWithoutAPrintingInTheFilteredSets) +{ + visualModel->setSetFilterTerms({"NO SUCH SET"}); + + EXPECT_EQ(0, visualModel->rowCount()); +} + +TEST_F(CardDatabaseVisualDisplayModelTest, RestoresOneRowPerCardWhenTheSetTermsClear) +{ + db->addCard(makeCard("Multi", {{"TST", 2}, {"OTH", 1}})); + visualModel->refresh(); + const int rowsBefore = visualModel->rowCount(); + + visualModel->setSetFilterTerms({"TST"}); + ASSERT_EQ(2, visualModel->rowCount()); + + visualModel->setSetFilterTerms({}); + EXPECT_EQ(rowsBefore, visualModel->rowCount()); +} + +TEST_F(CardDatabaseVisualDisplayModelTest, RebuildsByItselfWhenTheProxyGainsARow) +{ + const int rowsBefore = visualModel->rowCount(); + + db->addCard(makeCard("Late", {{"TST", 1}})); + waitForRebuild(); + + EXPECT_EQ(rowsBefore + 1, visualModel->rowCount()); +} + +TEST_F(CardDatabaseVisualDisplayModelTest, ReportsTheCardNameForDisplayAndAccessibleText) +{ + ASSERT_GT(visualModel->rowCount(), 0); + const QString expected = visualModel->cardAt(0).getName(); + + EXPECT_EQ(expected, visualModel->data(visualModel->index(0), Qt::DisplayRole).toString()); + EXPECT_EQ(expected, visualModel->data(visualModel->index(0), Qt::AccessibleTextRole).toString()); + EXPECT_FALSE(visualModel->data(QModelIndex(), Qt::DisplayRole).isValid()); +} +} // namespace diff --git a/vcpkg b/vcpkg index 9eec002df..9e593bb18 160000 --- a/vcpkg +++ b/vcpkg @@ -1 +1 @@ -Subproject commit 9eec002df1cd109f8f104cd966a763daf9751489 +Subproject commit 9e593bb18ea69cc5095e012465dcd675a822ed0d From 62e590d3005a0f8ca31ef45e97a9923a2552b085 Mon Sep 17 00:00:00 2001 From: RickyRister <42636155+RickyRister@users.noreply.github.com> Date: Mon, 2 Nov 2026 00:37:05 -0700 Subject: [PATCH 02/15] [ServerGame] Refactor out spectator/judge bools into RoleInfo (#7761) --- .../network/server/remote/CMakeLists.txt | 1 + .../network/server/remote/game/role_info.h | 13 +++++++ .../game/server_abstract_participant.cpp | 22 +++++------ .../remote/game/server_abstract_participant.h | 10 ++--- .../remote/game/server_abstract_player.cpp | 13 +++---- .../server/remote/game/server_game.cpp | 39 +++++++++++-------- .../network/server/remote/game/server_game.h | 7 ++-- .../server/remote/game/server_player.cpp | 6 +-- .../server/remote/game/server_spectator.cpp | 3 +- .../server/remote/server_protocolhandler.cpp | 3 +- .../network/server/remote/server_room.cpp | 9 +++-- tests/server_game_join_test.cpp | 31 ++++++++------- 12 files changed, 89 insertions(+), 68 deletions(-) create mode 100644 libcockatrice_network/libcockatrice/network/server/remote/game/role_info.h diff --git a/libcockatrice_network/libcockatrice/network/server/remote/CMakeLists.txt b/libcockatrice_network/libcockatrice/network/server/remote/CMakeLists.txt index 8586a7bc8..181218e1a 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/CMakeLists.txt +++ b/libcockatrice_network/libcockatrice/network/server/remote/CMakeLists.txt @@ -8,6 +8,7 @@ set(HEADERS game/server_cardzone.h game/server_counter.h game/game_config.h + game/role_info.h game/server_deck_validation_strategy.h game/server_game.h game/server_game_lifecycle_strategy.h diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/role_info.h b/libcockatrice_network/libcockatrice/network/server/remote/game/role_info.h new file mode 100644 index 000000000..ef8692361 --- /dev/null +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/role_info.h @@ -0,0 +1,13 @@ +#ifndef COCKATRICE_ROLE_INFO_H +#define COCKATRICE_ROLE_INFO_H + +/** + * @brief Represents information about a participant's roles. + */ +struct RoleInfo +{ + bool isJudge = false; ///< Whether the participant joined as a judge + bool isSpectator = false; ///< Whether the participant joined as a spectator +}; + +#endif // COCKATRICE_ROLE_INFO_H diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_participant.cpp b/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_participant.cpp index 5b8d901bb..ff8ea14f3 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_participant.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_participant.cpp @@ -61,10 +61,10 @@ Server_AbstractParticipant::Server_AbstractParticipant(Server_Game *_game, int _playerId, const ServerInfo_User &_userInfo, - bool _judge, + const RoleInfo &_roleInfo, Server_AbstractUserInterface *_userInterface) : ServerInfo_User_Container(_userInfo), game(_game), userInterface(_userInterface), pingTime(0), - playerId(_playerId), judge(_judge) + playerId(_playerId), roleInfo(_roleInfo) { } @@ -98,8 +98,8 @@ void Server_AbstractParticipant::getProperties(ServerInfo_PlayerProperties &resu if (withUserInfo) { copyUserInfo(*(result.mutable_user_info()), true); } - result.set_spectator(spectator); - result.set_judge(judge); + result.set_spectator(isSpectator()); + result.set_judge(isJudge()); result.set_ping_seconds(pingTime); getPlayerProperties(result); } @@ -176,7 +176,7 @@ Response::ResponseCode Server_AbstractParticipant::cmdUnconcede(const Command_Un Response::ResponseCode Server_AbstractParticipant::cmdJudge(const Command_Judge &cmd, ResponseContainer &rc, GameEventStorage &ges) { - if (!judge) { + if (!isJudge()) { return Response::RespFunctionNotAllowed; } @@ -204,7 +204,7 @@ Response::ResponseCode Server_AbstractParticipant::cmdReadyStart(const Command_R Response::ResponseCode Server_AbstractParticipant::cmdGameSay(const Command_GameSay &cmd, ResponseContainer & /*rc*/, GameEventStorage &ges) { - if (spectator) { + if (isSpectator()) { /* Spectators can only talk if: * (a) the game creator allows it * (b) the spectator is a moderator/administrator @@ -365,7 +365,7 @@ Response::ResponseCode Server_AbstractParticipant::cmdNextTurn(const Command_Nex return Response::RespGameNotStarted; } - if (!judge) { + if (!isJudge()) { return Response::RespFunctionNotAllowed; } @@ -381,7 +381,7 @@ Response::ResponseCode Server_AbstractParticipant::cmdSetActivePhase(const Comma return Response::RespGameNotStarted; } - if (!judge) { + if (!isJudge()) { return Response::RespFunctionNotAllowed; } @@ -415,8 +415,8 @@ Response::ResponseCode Server_AbstractParticipant::cmdReverseTurn(const Command_ ResponseContainer & /*rc*/, GameEventStorage &ges) { - if (!judge) { - if (spectator) { + if (!isJudge()) { + if (isSpectator()) { return Response::RespFunctionNotAllowed; } @@ -577,7 +577,7 @@ void Server_AbstractParticipant::setUserInterface(Server_AbstractUserInterface * void Server_AbstractParticipant::disconnectClient() { bool isRegistered = userInfo->user_level() & ServerInfo_User::IsRegistered; - if (!isRegistered || spectator) { + if (!isRegistered || isSpectator()) { game->removeParticipant(this, Event_Leave::USER_DISCONNECTED); } else { setUserInterface(nullptr); diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_participant.h b/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_participant.h index 195da9b7f..b6d4a8025 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_participant.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_participant.h @@ -2,6 +2,7 @@ #define ABSTRACT_PARTICIPANT_H #include "../serverinfo_user_container.h" +#include "role_info.h" #include "server_arrowtarget.h" #include @@ -61,8 +62,7 @@ protected: Server_AbstractUserInterface *userInterface; int pingTime; int playerId; - bool spectator; - bool judge; + RoleInfo roleInfo; virtual void getPlayerProperties(ServerInfo_PlayerProperties &result); mutable QMutex playerMutex; @@ -70,7 +70,7 @@ public: Server_AbstractParticipant(Server_Game *_game, int _playerId, const ServerInfo_User &_userInfo, - bool _judge, + const RoleInfo &_roleInfo, Server_AbstractUserInterface *_handler); ~Server_AbstractParticipant() override; virtual void prepareDestroy() @@ -91,11 +91,11 @@ public: } bool isSpectator() const { - return spectator; + return roleInfo.isSpectator; } bool isJudge() const { - return judge; + return roleInfo.isJudge; } Server_Game *getGame() const { diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_player.cpp b/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_player.cpp index c03f1e966..b5a620798 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_player.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_abstract_player.cpp @@ -81,10 +81,9 @@ Server_AbstractPlayer::Server_AbstractPlayer(Server_Game *_game, const ServerInfo_User &_userInfo, bool _judge, Server_AbstractUserInterface *_userInterface) - : Server_AbstractParticipant(_game, _playerId, _userInfo, _judge, _userInterface), conceded(false), deck(nullptr), - sideboardLocked(true), readyStart(false), nextCardId(0) + : Server_AbstractParticipant(_game, _playerId, _userInfo, {.isJudge = _judge}, _userInterface), conceded(false), + deck(nullptr), sideboardLocked(true), readyStart(false), nextCardId(0) { - spectator = false; } Server_AbstractPlayer::~Server_AbstractPlayer() = default; @@ -295,7 +294,7 @@ Response::ResponseCode Server_AbstractPlayer::moveCard(GameEventStorage &ges, { // Disallow controller change to other zones than the table. if (((targetzone->getType() != ServerInfo_Zone::PublicZone) || !targetzone->hasCoords()) && - (startzone->getPlayer() != targetzone->getPlayer()) && !judge) { + (startzone->getPlayer() != targetzone->getPlayer()) && !isJudge()) { return Response::RespContextError; } @@ -808,7 +807,7 @@ Server_AbstractPlayer::cmdMoveCard(const Command_MoveCard &cmd, ResponseContaine return Response::RespNameNotFound; } - if ((startPlayer != this) && (!startZone->getPlayersWithWritePermission().contains(playerId)) && !judge) { + if ((startPlayer != this) && (!startZone->getPlayersWithWritePermission().contains(playerId)) && !isJudge()) { return Response::RespContextError; } @@ -821,7 +820,7 @@ Server_AbstractPlayer::cmdMoveCard(const Command_MoveCard &cmd, ResponseContaine return Response::RespNameNotFound; } - if ((startPlayer != this) && (targetPlayer != this) && !judge) { + if ((startPlayer != this) && (targetPlayer != this) && !isJudge()) { return Response::RespContextError; } @@ -1598,7 +1597,7 @@ Server_AbstractPlayer::cmdRevealCards(const Command_RevealCards &cmd, ResponseCo zone->addWritePermission(cmd.player_id()); } - if (judge) { + if (isJudge()) { ges.setOverwriteOwnership(true); } diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.cpp b/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.cpp index b1863c91e..49db999f6 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.cpp @@ -41,6 +41,7 @@ #include "libcockatrice/protocol/pb/serverinfo_replay_match.pb.h" #include "libcockatrice/protocol/pb/serverinfo_user.pb.h" #include "libcockatrice/protocol/pb/session_event.pb.h" +#include "role_info.h" #include "server_abstract_player.h" #include "server_abstractuserinterface.h" #include "server_arrow.h" @@ -479,9 +480,8 @@ void Server_Game::stopGameIfFinished() Response::ResponseCode Server_Game::checkJoin(ServerInfo_User *user, const QString &_password, - bool spectator, - bool overrideRestrictions, - bool asJudge) + const RoleInfo &roleInfo, + bool overrideRestrictions) { Server_DatabaseInterface *databaseInterface = room->getServer()->getDatabaseInterface(); for (auto *participant : participants.values()) { @@ -490,11 +490,11 @@ Response::ResponseCode Server_Game::checkJoin(ServerInfo_User *user, } } - if (asJudge && !(user->user_level() & ServerInfo_User::IsJudge)) { + if (roleInfo.isJudge && !(user->user_level() & ServerInfo_User::IsJudge)) { return Response::RespUserLevelTooLow; } if (!(overrideRestrictions && (user->user_level() & (ServerInfo_User::IsModerator | ServerInfo_User::IsJudge)))) { - if ((_password != password) && !(spectator && !spectatorsNeedPassword)) { + if ((_password != password) && !(roleInfo.isSpectator && !spectatorsNeedPassword)) { return Response::RespWrongPassword; } if (!(user->user_level() & ServerInfo_User::IsRegistered) && onlyRegistered) { @@ -510,13 +510,13 @@ Response::ResponseCode Server_Game::checkJoin(ServerInfo_User *user, QString::fromStdString(user->name()))) { return Response::RespInIgnoreList; } - if (spectator) { + if (roleInfo.isSpectator) { if (!spectatorsAllowed) { return Response::RespSpectatorsNotAllowed; } } } - if (!spectator && (gameStarted || (getPlayerCount() >= getMaxPlayers()))) { + if (!roleInfo.isSpectator && (gameStarted || (getPlayerCount() >= getMaxPlayers()))) { return Response::RespGameFull; } @@ -537,19 +537,18 @@ bool Server_Game::containsUser(const QString &userName) const void Server_Game::addPlayer(Server_AbstractUserInterface *userInterface, ResponseContainer &rc, - bool spectator, - bool judge, + const RoleInfo &roleInfo, bool broadcastUpdate) { QMutexLocker locker(&gameMutex); Server_AbstractParticipant *newParticipant; - if (spectator) { + if (roleInfo.isSpectator) { newParticipant = new Server_Spectator(this, nextPlayerId++, userInterface->copyUserInfo(true, true, true), - judge, userInterface); + roleInfo.isJudge, userInterface); } else { - newParticipant = new Server_Player(this, nextPlayerId++, userInterface->copyUserInfo(true, true, true), judge, - userInterface); + newParticipant = new Server_Player(this, nextPlayerId++, userInterface->copyUserInfo(true, true, true), + roleInfo.isJudge, userInterface); } newParticipant->moveToThread(thread()); @@ -560,7 +559,7 @@ void Server_Game::addPlayer(Server_AbstractUserInterface *userInterface, const QString playerName = QString::fromStdString(newParticipant->getUserInfo()->name()); participants.insert(newParticipant->getPlayerId(), newParticipant); - if (spectator) { + if (roleInfo.isSpectator) { allSpectatorsEver.insert(playerName); } else { allPlayersEver.insert(playerName); @@ -579,7 +578,7 @@ void Server_Game::addPlayer(Server_AbstractUserInterface *userInterface, emit gameInfoChanged(gameInfo); } - if ((newParticipant->getUserInfo()->user_level() & ServerInfo_User::IsRegistered) && !spectator) { + if ((newParticipant->getUserInfo()->user_level() & ServerInfo_User::IsRegistered) && !roleInfo.isSpectator) { room->getServer()->addPersistentPlayer(playerName, room->getId(), gameId, newParticipant->getPlayerId()); } @@ -871,6 +870,13 @@ void Server_Game::createGameJoinedEvent(Server_AbstractParticipant *joiningParti } } +bool Server_Game::canSeePrivateEventsFor(const Server_AbstractParticipant *participant, int privatePlayerId) const +{ + return participant->getPlayerId() == privatePlayerId // same player + || participant->isJudge() // judges see everything + || (participant->isSpectator() && spectatorsSeeEverything); // spectators see everything +} + void Server_Game::sendGameEventContainer(GameEventContainer *cont, GameEventStorageItem::EventRecipients recipients, int privatePlayerId) @@ -879,8 +885,7 @@ void Server_Game::sendGameEventContainer(GameEventContainer *cont, cont->set_game_id(gameId); for (auto *participant : participants.values()) { - const bool playerPrivate = (participant->getPlayerId() == privatePlayerId) || participant->isJudge() || - (participant->isSpectator() && spectatorsSeeEverything); + const bool playerPrivate = canSeePrivateEventsFor(participant, privatePlayerId); if ((recipients.testFlag(GameEventStorageItem::SendToPrivate) && playerPrivate) || (recipients.testFlag(GameEventStorageItem::SendToOthers) && !playerPrivate)) { participant->sendGameEvent(*cont); diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.h b/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.h index c7f8d0929..363c0ca94 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_game.h @@ -62,6 +62,7 @@ class Message; } // namespace protobuf } // namespace google struct GameConfig; +struct RoleInfo; class Server_Game : public QObject { @@ -109,6 +110,7 @@ private: bool omniscient, bool withUserInfo); void storeGameInformation(); + bool canSeePrivateEventsFor(const Server_AbstractParticipant *participant, int privatePlayerId) const; signals: void sigStartGameIfReady(bool override); void gameInfoChanged(ServerInfo_Game gameInfo); @@ -188,12 +190,11 @@ public: return shareDecklistsOnLoad; } Response::ResponseCode - checkJoin(ServerInfo_User *user, const QString &_password, bool spectator, bool overrideRestrictions, bool asJudge); + checkJoin(ServerInfo_User *user, const QString &_password, const RoleInfo &roleInfo, bool overrideRestrictions); bool containsUser(const QString &userName) const; void addPlayer(Server_AbstractUserInterface *userInterface, ResponseContainer &rc, - bool spectator, - bool judge, + const RoleInfo &roleInfo, bool broadcastUpdate = true); void removeParticipant(Server_AbstractParticipant *participant, Event_Leave::LeaveReason reason); void removeArrowsRelatedToPlayer(GameEventStorage &ges, Server_AbstractPlayer *player); diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_player.cpp b/libcockatrice_network/libcockatrice/network/server/remote/game/server_player.cpp index 949d04b4b..b40dae09b 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_player.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_player.cpp @@ -573,7 +573,7 @@ Server_Player::cmdNextTurn(const Command_NextTurn & /*cmd*/, ResponseContainer & return Response::RespGameNotStarted; } - if (conceded && !judge) { + if (conceded && !isJudge()) { return Response::RespContextError; } @@ -589,7 +589,7 @@ Response::ResponseCode Server_Player::cmdSetActivePhase(const Command_SetActiveP return Response::RespGameNotStarted; } - if (!judge) { + if (!isJudge()) { if (conceded) { return Response::RespContextError; } @@ -623,7 +623,7 @@ Response::ResponseCode Server_Player::cmdReverseTurn(const Command_ReverseTurn &cmd, ResponseContainer &rc, GameEventStorage &ges) { - if (!judge && conceded) { + if (!isJudge() && conceded) { return Response::RespContextError; } return Server_AbstractParticipant::cmdReverseTurn(cmd, rc, ges); diff --git a/libcockatrice_network/libcockatrice/network/server/remote/game/server_spectator.cpp b/libcockatrice_network/libcockatrice/network/server/remote/game/server_spectator.cpp index d1c369949..164a4af5e 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/game/server_spectator.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/game/server_spectator.cpp @@ -11,7 +11,6 @@ Server_Spectator::Server_Spectator(Server_Game *_game, const ServerInfo_User &_userInfo, bool _judge, Server_AbstractUserInterface *_userInterface) - : Server_AbstractParticipant(_game, _playerId, _userInfo, _judge, _userInterface) + : Server_AbstractParticipant(_game, _playerId, _userInfo, {.isJudge = _judge, .isSpectator = true}, _userInterface) { - spectator = true; } diff --git a/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp b/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp index 39c1eb3e6..f9cadc9aa 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp @@ -1141,7 +1141,8 @@ Server_ProtocolHandler::cmdCreateGame(const Command_CreateGame &cmd, Server_Room auto *game = new Server_Game(config, room); - game->addPlayer(this, rc, asSpectator, asJudge, false); + RoleInfo roleInfo{.isJudge = asJudge, .isSpectator = asSpectator}; + game->addPlayer(this, rc, roleInfo, false); room->addGame(game); return Response::RespOk; diff --git a/libcockatrice_network/libcockatrice/network/server/remote/server_room.cpp b/libcockatrice_network/libcockatrice/network/server/remote/server_room.cpp index 68b137cd4..78ef2b289 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server_room.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/server_room.cpp @@ -1,5 +1,6 @@ #include "server_room.h" +#include "game/role_info.h" #include "game/server_game.h" #include "libcockatrice/protocol/pb/response.pb.h" #include "libcockatrice/protocol/pb/room_event.pb.h" @@ -372,11 +373,11 @@ Response::ResponseCode Server_Room::processJoinGameCommand(const Command_JoinGam QMutexLocker gameLocker(&game->gameMutex); - Response::ResponseCode result = - game->checkJoin(userInterface->getUserInfo(), QString::fromStdString(cmd.password()), cmd.spectator(), - cmd.override_restrictions(), cmd.join_as_judge()); + RoleInfo roleInfo{.isJudge = cmd.join_as_judge(), .isSpectator = cmd.spectator()}; + Response::ResponseCode result = game->checkJoin( + userInterface->getUserInfo(), QString::fromStdString(cmd.password()), roleInfo, cmd.override_restrictions()); if (result == Response::RespOk) { - game->addPlayer(userInterface, rc, cmd.spectator(), cmd.join_as_judge()); + game->addPlayer(userInterface, rc, roleInfo); } return result; diff --git a/tests/server_game_join_test.cpp b/tests/server_game_join_test.cpp index 8a2ae8c45..93895ab5c 100644 --- a/tests/server_game_join_test.cpp +++ b/tests/server_game_join_test.cpp @@ -4,6 +4,7 @@ */ #include "game/game_config.h" +#include "game/role_info.h" #include "game/server_game.h" #include "libcockatrice/protocol/pb/response.pb.h" #include "server.h" @@ -119,13 +120,13 @@ TEST_F(GameJoinOverrideTest, StaffBypassPasswordRestriction) game = makeGame(true, false, false, true); // A plain user cannot override the password even with the override flag set. - EXPECT_EQ(game->checkJoin(&plainUser, "wrong", false, true, false), Response::RespWrongPassword); + EXPECT_EQ(game->checkJoin(&plainUser, "wrong", {}, true), Response::RespWrongPassword); // Moderators and judges may enter any game regardless of the password. - EXPECT_EQ(game->checkJoin(&moderator, "wrong", false, true, false), Response::RespOk); - EXPECT_EQ(game->checkJoin(&judge, "wrong", false, true, false), Response::RespOk); + EXPECT_EQ(game->checkJoin(&moderator, "wrong", {}, true), Response::RespOk); + EXPECT_EQ(game->checkJoin(&judge, "wrong", {}, true), Response::RespOk); // Without the override flag judges are still subject to the password. - EXPECT_EQ(game->checkJoin(&judge, "wrong", false, false, true), Response::RespWrongPassword); - EXPECT_EQ(game->checkJoin(&judge, "secret", false, false, true), Response::RespOk); + EXPECT_EQ(game->checkJoin(&judge, "wrong", {.isJudge = true}, false), Response::RespWrongPassword); + EXPECT_EQ(game->checkJoin(&judge, "secret", {.isJudge = true}, false), Response::RespOk); } TEST_F(GameJoinOverrideTest, StaffBypassRegisteredOnlyRestriction) @@ -133,9 +134,9 @@ TEST_F(GameJoinOverrideTest, StaffBypassRegisteredOnlyRestriction) game = makeGame(false, true, false, true); // Without the override flag the only-registered restriction still applies. - EXPECT_EQ(game->checkJoin(&unregisteredJudge, QString(), false, false, false), Response::RespUserLevelTooLow); + EXPECT_EQ(game->checkJoin(&unregisteredJudge, QString(), {}, false), Response::RespUserLevelTooLow); // An unregistered judge may enter when overriding restrictions. - EXPECT_EQ(game->checkJoin(&unregisteredJudge, QString(), false, true, false), Response::RespOk); + EXPECT_EQ(game->checkJoin(&unregisteredJudge, QString(), {}, true), Response::RespOk); } TEST_F(GameJoinOverrideTest, StaffBypassBuddiesOnlyRestriction) @@ -143,10 +144,10 @@ TEST_F(GameJoinOverrideTest, StaffBypassBuddiesOnlyRestriction) game = makeGame(false, false, true, true); // A plain user who is not on the creator's buddy list gets rejected. - EXPECT_EQ(game->checkJoin(&plainUser, QString(), false, true, false), Response::RespOnlyBuddies); + EXPECT_EQ(game->checkJoin(&plainUser, QString(), {}, true), Response::RespOnlyBuddies); // Moderators and judges bypass the buddies-only restriction. - EXPECT_EQ(game->checkJoin(&moderator, QString(), false, true, false), Response::RespOk); - EXPECT_EQ(game->checkJoin(&judge, QString(), false, true, false), Response::RespOk); + EXPECT_EQ(game->checkJoin(&moderator, QString(), {}, true), Response::RespOk); + EXPECT_EQ(game->checkJoin(&judge, QString(), {}, true), Response::RespOk); } TEST_F(GameJoinOverrideTest, StaffBypassSpectatorsNotAllowedRestriction) @@ -154,11 +155,11 @@ TEST_F(GameJoinOverrideTest, StaffBypassSpectatorsNotAllowedRestriction) game = makeGame(false, false, false, false); // A plain user cannot spectate when the game disallows spectators. - EXPECT_EQ(game->checkJoin(&plainUser, QString(), true, false, false), Response::RespSpectatorsNotAllowed); + EXPECT_EQ(game->checkJoin(&plainUser, QString(), {.isSpectator = true}, false), Response::RespSpectatorsNotAllowed); // Moderators and judges may spectate any game regardless of the password // and the spectator restriction. - EXPECT_EQ(game->checkJoin(&moderator, "wrong", true, true, false), Response::RespOk); - EXPECT_EQ(game->checkJoin(&judge, "wrong", true, true, false), Response::RespOk); + EXPECT_EQ(game->checkJoin(&moderator, "wrong", {.isSpectator = true}, true), Response::RespOk); + EXPECT_EQ(game->checkJoin(&judge, "wrong", {.isSpectator = true}, true), Response::RespOk); } TEST_F(GameJoinOverrideTest, JudgeOverrideDoesNotGrantJudgeJoinToPlainUser) @@ -166,8 +167,8 @@ TEST_F(GameJoinOverrideTest, JudgeOverrideDoesNotGrantJudgeJoinToPlainUser) game = makeGame(false, false, false, true); // joining with join_as_judge still requires the judge flag even when overriding. - EXPECT_EQ(game->checkJoin(&plainUser, QString(), false, true, true), Response::RespUserLevelTooLow); - EXPECT_EQ(game->checkJoin(&judge, QString(), false, true, true), Response::RespOk); + EXPECT_EQ(game->checkJoin(&plainUser, QString(), {.isJudge = true}, true), Response::RespUserLevelTooLow); + EXPECT_EQ(game->checkJoin(&judge, QString(), {.isJudge = true}, true), Response::RespOk); } } // namespace From 8b0c980e85515c5e7d8a5e04a3889943b29ac756 Mon Sep 17 00:00:00 2001 From: "sentry[bot]" <39604003+sentry[bot]@users.noreply.github.com> Date: Mon, 2 Nov 2026 08:39:07 +0100 Subject: [PATCH 03/15] [Client] Guard dragged card ghost from painting or dropping deleted cards (#7688) * [Client] Guard dragged card ghost from painting or dropping deleted cards * [Game] Revert handling for cards destroyed mid-drag Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com> * [DeckView] Access pointer through QPointer in DeckViewCardDragItem --------- Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com> Co-authored-by: zeldazach <7460172+zeldazach@users.noreply.github.com> --- .../board/abstract_card_drag_item.cpp | 20 ++++++++++++++++++- .../board/abstract_card_drag_item.h | 10 +++++----- .../game_graphics/board/card_drag_item.cpp | 7 ++++--- .../src/game_graphics/deckview/deck_view.cpp | 2 +- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/cockatrice/src/game_graphics/board/abstract_card_drag_item.cpp b/cockatrice/src/game_graphics/board/abstract_card_drag_item.cpp index 2e18fca2d..c707b4022 100644 --- a/cockatrice/src/game_graphics/board/abstract_card_drag_item.cpp +++ b/cockatrice/src/game_graphics/board/abstract_card_drag_item.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -51,7 +52,20 @@ AbstractCardDragItem::AbstractCardDragItem(AbstractCardItem *_item, update(); }); - connect(item, &QObject::destroyed, this, &AbstractCardDragItem::deleteLater); + // The source card can be destroyed mid-drag (e.g. moved by a server event). deleteLater() alone is not enough: + // a scene repaint can run before the deferred delete and paint the dead card. Detach from the scene immediately. + connect(_item, &QObject::destroyed, this, [this]() { + hide(); + if (QGraphicsScene *sc = scene()) { + sc->removeItem(this); + } + deleteLater(); + }); +} + +AbstractCardItem *AbstractCardDragItem::getItem() const +{ + return item.data(); } QPainterPath AbstractCardDragItem::shape() const @@ -65,6 +79,10 @@ QPainterPath AbstractCardDragItem::shape() const void AbstractCardDragItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget) { + if (item.isNull()) { + return; + } + item->paint(painter, option, widget); // adds a mask to the card so it looks like the card hasnt been placed yet diff --git a/cockatrice/src/game_graphics/board/abstract_card_drag_item.h b/cockatrice/src/game_graphics/board/abstract_card_drag_item.h index 18afdcbb8..80e40ff1d 100644 --- a/cockatrice/src/game_graphics/board/abstract_card_drag_item.h +++ b/cockatrice/src/game_graphics/board/abstract_card_drag_item.h @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -24,7 +25,8 @@ class AbstractCardDragItem : public QObject, public QGraphicsItem Q_OBJECT Q_INTERFACES(QGraphicsItem) protected: - AbstractCardItem *item; + /// Guarded pointer: the source card may be destroyed while this drag item still exists. + QPointer item; QPointF hotSpot; QList childDrags; @@ -44,10 +46,8 @@ public: } [[nodiscard]] QPainterPath shape() const override; void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget) override; - [[nodiscard]] AbstractCardItem *getItem() const - { - return item; - } + /// @return the source card, or nullptr if it has already been destroyed. + [[nodiscard]] AbstractCardItem *getItem() const; [[nodiscard]] QPointF getHotSpot() const { return hotSpot; diff --git a/cockatrice/src/game_graphics/board/card_drag_item.cpp b/cockatrice/src/game_graphics/board/card_drag_item.cpp index 265ea759c..b263e8c92 100644 --- a/cockatrice/src/game_graphics/board/card_drag_item.cpp +++ b/cockatrice/src/game_graphics/board/card_drag_item.cpp @@ -130,8 +130,9 @@ void CardDragItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event) sc->removeItem(this); QList dragItemList; - CardZoneLogic *startZone = static_cast(item)->getZone(); - if (currentZone && !(static_cast(item)->getAttachedTo() && (startZone == currentZone->getLogic()))) { + CardZoneLogic *startZone = static_cast(item.data())->getZone(); + if (currentZone && + !(static_cast(item.data())->getAttachedTo() && (startZone == currentZone->getLogic()))) { if (!occupied) { dragItemList.append(this); } @@ -139,7 +140,7 @@ void CardDragItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event) for (int i = 0; i < childDrags.size(); i++) { CardDragItem *c = static_cast(childDrags[i]); if (!occupied && - !(static_cast(c->item)->getAttachedTo() && (startZone == currentZone->getLogic())) && + !(static_cast(c->item.data())->getAttachedTo() && (startZone == currentZone->getLogic())) && !c->occupied) { dragItemList.append(c); } diff --git a/cockatrice/src/game_graphics/deckview/deck_view.cpp b/cockatrice/src/game_graphics/deckview/deck_view.cpp index 630530792..6e0b0eaaa 100644 --- a/cockatrice/src/game_graphics/deckview/deck_view.cpp +++ b/cockatrice/src/game_graphics/deckview/deck_view.cpp @@ -77,7 +77,7 @@ void DeckViewCardDragItem::updatePosition(const QPointF &cursorScenePos) void DeckViewCardDragItem::handleDrop(DeckViewCardContainer *target) { - auto *card = static_cast(item); + auto *card = static_cast(item.data()); auto *start = static_cast(item->parentItem()); start->removeCard(card); target->addCard(card); From 9f30892dffb68a5551e20ecee65db5429be7f354 Mon Sep 17 00:00:00 2001 From: "sentry[bot]" <39604003+sentry[bot]@users.noreply.github.com> Date: Mon, 2 Nov 2026 13:28:23 +0100 Subject: [PATCH 04/15] [Client] Fix crash when local game commands are sent after shutdown (#7714) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Client] Fix crash when local game commands are sent after shutdown * [Network] Fix formatting in local client debug log * [CI] Keep the vcpkg submodule pointer on the manifest baseline vcpkg.json asks for builtin-baseline 9eec002 (vcpkg 2026.10.16), but the submodule gitlink was still at 9e593bb. The runners check submodules out shallowly, so vcpkg-tool cannot read versions/baseline.json out of the baseline commit and every vcpkg leg (Windows and all macOS) fails in CMake configure with: failed to `git show` versions/baseline.json git phrases that as "path ... exists on disk, but not in " even when the commit is simply absent from the shallow clone, so it reads like a tooling or network failure rather than a mismatched pointer. Point the submodule back at 9eec002, and add a cheap job that fails when the gitlink and builtin-baseline disagree, before the five build legs pay for it. --------- Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com> Co-authored-by: Lukas Brübach --- .github/workflows/desktop-build.yml | 32 +++- cockatrice/src/interface/window_main.cpp | 6 +- .../network/client/local/local_client.cpp | 10 +- .../network/client/local/local_client.h | 6 +- tests/CMakeLists.txt | 12 ++ tests/local_client_lifetime_test.cpp | 149 ++++++++++++++++++ vcpkg | 2 +- 7 files changed, 208 insertions(+), 9 deletions(-) create mode 100644 tests/local_client_lifetime_test.cpp diff --git a/.github/workflows/desktop-build.yml b/.github/workflows/desktop-build.yml index 54068f5a0..df054ea26 100644 --- a/.github/workflows/desktop-build.yml +++ b/.github/workflows/desktop-build.yml @@ -101,6 +101,36 @@ jobs: --title "$release_name" \ --notes-file "$body_path" + vcpkg-baseline: + name: vcpkg baseline + runs-on: ubuntu-slim + steps: + - name: "Checkout" + uses: actions/checkout@v7 + + # vcpkg-tool reads versions/baseline.json out of the commit named by + # `builtin-baseline`, through the `vcpkg` submodule's git history. A submodule + # pointer that lags the manifest - e.g. a stale `vcpkg/` worktree captured by + # `git add` in a PR - makes every vcpkg leg fail in configure with "failed to + # `git show` versions/baseline.json". That reads like a baseline or network + # problem, when the pointer is what has to move. Check the two agree here in + # seconds instead of on five build legs. + - name: "Check the submodule pointer against the manifest baseline" + shell: bash + run: | + pointer="$(git rev-parse HEAD:vcpkg 2>/dev/null || true)" + baseline="$(sed -nE 's/.*"builtin-baseline"[[:space:]]*:[[:space:]]*"([0-9a-f]{40})".*/\1/p' vcpkg.json)" + if [[ -z "$pointer" || -z "$baseline" ]]; then + echo "::error file=vcpkg.json::could not read the vcpkg submodule pointer or the builtin-baseline" + exit 1 + fi + if [[ "$pointer" != "$baseline" ]]; then + echo "::error file=vcpkg.json::vcpkg submodule is at $pointer but builtin-baseline is $baseline" + echo "Move them together: git -C vcpkg checkout $baseline && git add vcpkg" + exit 1 + fi + echo "vcpkg submodule and builtin-baseline agree on $baseline" + build-linux: strategy: fail-fast: false @@ -466,7 +496,7 @@ jobs: vcpkg_upload: 1 # only one leg per vcpkg_scope hands its archives over name: ${{ matrix.os }} ${{ matrix.target }}${{ matrix.soc == 'Intel' && ' Intel' || '' }}${{ matrix.type == 'Debug' && ' Debug' || '' }} - needs: configure + needs: [configure, vcpkg-baseline] runs-on: ${{ matrix.runner }} timeout-minutes: 100 env: diff --git a/cockatrice/src/interface/window_main.cpp b/cockatrice/src/interface/window_main.cpp index f8ab5d3f6..d546676bc 100644 --- a/cockatrice/src/interface/window_main.cpp +++ b/cockatrice/src/interface/window_main.cpp @@ -254,7 +254,11 @@ void MainWindow::actWatchReplay() void MainWindow::localGameEnded() { - delete localServer; + // Defer deletion so the LocalClients (already scheduled with deleteLater() in TabSupervisor::stop) + // and any queued commands are processed before the server and its interfaces are destroyed. + if (localServer) { + localServer->deleteLater(); + } localServer = nullptr; aConnect->setEnabled(true); diff --git a/libcockatrice_network/libcockatrice/network/client/local/local_client.cpp b/libcockatrice_network/libcockatrice/network/client/local/local_client.cpp index f7d8645e8..b9a1566f8 100644 --- a/libcockatrice_network/libcockatrice/network/client/local/local_client.cpp +++ b/libcockatrice_network/libcockatrice/network/client/local/local_client.cpp @@ -33,16 +33,18 @@ LocalClient::LocalClient(LocalServerInterface *_lsi, sendCommand(prepareSessionCommand(joinCmd)); } -LocalClient::~LocalClient() -{ -} - void LocalClient::sendCommandContainer(const CommandContainer &cont) { if (LocalClientLog().isDebugEnabled()) { qCDebug(LocalClientLog).noquote() << userName << "OUT" << getSafeDebugString(cont); } + if (lsi.isNull()) { + qCWarning(LocalClientLog).noquote() + << userName << "dropping command, local server interface no longer exists:" << getSafeDebugString(cont); + return; + } + lsi->itemFromClient(cont); } diff --git a/libcockatrice_network/libcockatrice/network/client/local/local_client.h b/libcockatrice_network/libcockatrice/network/client/local/local_client.h index 3ae570255..15b36d3db 100644 --- a/libcockatrice_network/libcockatrice/network/client/local/local_client.h +++ b/libcockatrice_network/libcockatrice/network/client/local/local_client.h @@ -10,6 +10,7 @@ #include "../abstract/abstract_client.h" #include +#include #include #include @@ -24,14 +25,15 @@ class LocalClient : public AbstractClient { Q_OBJECT private: - LocalServerInterface *lsi; + // Guarded pointer: the LocalServer (and its interfaces) may be destroyed while this client + // still has pending commands queued on the event loop. + QPointer lsi; public: LocalClient(LocalServerInterface *_lsi, const QString &_playerName, const QString &_clientId, QObject *parent = nullptr); - ~LocalClient() override; void sendCommandContainer(const CommandContainer &cont) override; private slots: diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 261900c01..0bb9654a1 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -350,6 +350,18 @@ cockatrice_add_test( LABELS ci ) +# The local game runs one client and one server in the same process. +# Either of them can be destroyed first; the survivor has to stay usable. +cockatrice_add_test( + NAME + local_client_lifetime_test + SOURCES + local_client_lifetime_test.cpp + LIBS + libcockatrice_network + LABELS + ci +) cockatrice_add_test( NAME server_room_channels_test diff --git a/tests/local_client_lifetime_test.cpp b/tests/local_client_lifetime_test.cpp new file mode 100644 index 000000000..95a2b1380 --- /dev/null +++ b/tests/local_client_lifetime_test.cpp @@ -0,0 +1,149 @@ +/** + * @file local_client_lifetime_test.cpp + * @brief Tests that a LocalClient survives the death of the LocalServer it talks to. + * @ingroup Tests + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +// The server library refers to this global; the real applications define it in main.cpp. +RNG_Abstract *rng = nullptr; + +namespace +{ + +// GTest's main() creates no QCoreApplication, which QObject::deleteLater() needs. +const QCoreApplication &application() +{ + static int argc = 1; + static char executable[] = "local_client_lifetime_test"; + static char *argv[] = {executable, nullptr}; + static const QCoreApplication app(argc, argv); + return app; +} + +/// Collects the warnings one logging category emits while this object is alive. +/// +/// The handler is process-global and Qt does not nest handlers, so only one capture can be +/// installed at a time; GTest running one case at a time is what makes that safe. +class CategoryWarningCapture +{ +public: + explicit CategoryWarningCapture(const char *_category) : category(_category) + { + previousHandler = qInstallMessageHandler(captureMessage); + s_active = this; + } + + ~CategoryWarningCapture() + { + qInstallMessageHandler(previousHandler); + s_active = nullptr; + } + + CategoryWarningCapture(const CategoryWarningCapture &) = delete; + CategoryWarningCapture &operator=(const CategoryWarningCapture &) = delete; + + const QStringList &warnings() const + { + return capturedWarnings; + } + +private: + static void captureMessage(QtMsgType type, const QMessageLogContext &context, const QString &message) + { + if (s_active != nullptr && type == QtWarningMsg && context.category != nullptr && + std::strcmp(s_active->category, context.category) == 0) { + s_active->capturedWarnings.append(message); + } + + // Forward everything else, so a failure elsewhere in the test is still visible. + if (previousHandler != nullptr) { + previousHandler(type, context, message); + return; + } + std::fprintf(stderr, "%s\n", qPrintable(qFormatLogMessage(type, context, message))); + } + + const char *category; + QStringList capturedWarnings; + + static CategoryWarningCapture *s_active; + static QtMessageHandler previousHandler; +}; + +CategoryWarningCapture *CategoryWarningCapture::s_active = nullptr; +QtMessageHandler CategoryWarningCapture::previousHandler = nullptr; + +class LocalClientLifetimeTest : public ::testing::Test +{ +protected: + void SetUp() override + { + (void)application(); + + server = std::make_unique(); + LocalServerInterface *lsi = server->newConnection(); + ASSERT_NE(lsi, nullptr); + // Building the client logs in and joins room 0 through that interface, exactly as + // MainWindow::startLocalGame() does. + client = std::make_unique(lsi, QStringLiteral("lifetime-test-player"), + QStringLiteral("lifetime-test-client-id")); + } + + void TearDown() override + { + // Either case may already have deleted one of these. + client.reset(); + server.reset(); + } + + std::unique_ptr server; + std::unique_ptr client; +}; + +TEST_F(LocalClientLifetimeTest, SendAfterServerDeletedIsDropped) +{ + server.reset(); // also destroys the LocalServerInterface the client was built on + + CategoryWarningCapture capture("local_client"); + // A default container stands in for a command still queued when the local game ended. + client->sendCommandContainer(CommandContainer()); + + ASSERT_EQ(capture.warnings().size(), 1); + EXPECT_TRUE(capture.warnings().first().contains(QStringLiteral("dropping command"))) + << capture.warnings().first().toStdString(); + + // The surviving client is intact: reading its state must not touch the deleted interface. + EXPECT_EQ(client->getUserName(), QStringLiteral("lifetime-test-player")); +} + +TEST_F(LocalClientLifetimeTest, DestroyingClientAfterServerIsSafe) +{ + server.reset(); + + CategoryWarningCapture capture("local_client"); + client.reset(); // must neither crash nor touch the destroyed interface + + // The destructor itself must have been silent. + EXPECT_TRUE(capture.warnings().isEmpty()); + + // A working capture proves the emptiness above is real rather than a broken handler. + qCWarning(LocalClientLog) << "local_client_lifetime_test capture self-check"; + EXPECT_EQ(capture.warnings().size(), 1); +} + +} // namespace diff --git a/vcpkg b/vcpkg index 9e593bb18..9eec002df 160000 --- a/vcpkg +++ b/vcpkg @@ -1 +1 @@ -Subproject commit 9e593bb18ea69cc5095e012465dcd675a822ed0d +Subproject commit 9eec002df1cd109f8f104cd966a763daf9751489 From 44d513818110343a61bbbd9e478502f0b9bcb69c Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Mon, 2 Nov 2026 18:18:19 +0100 Subject: [PATCH 05/15] [VisualDatabase] Keep the card grid scrolling on macOS (#7764) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [VisualDatabase] Keep the card grid scrolling on macOS The view ignored the wheel whenever the vertical scroll bar was not visible. On macOS, scroll bars are transient: once the fade-out finishes, QScrollbarStyleAnimation hides the scroll bar widget, so isVisible() then reports false even though the grid has more rows to scroll. Scrolling was dead until something else brought the bar back. Gate on the scroll bar's range instead: it says whether there is anything to scroll, while visibility is only about presentation. An event with no range to scroll is still ignored, so an enclosing scroll area can take it. * Update card_database_visual_view.cpp --------- Co-authored-by: Lukas Brübach --- .../visual_database_display/card_database_visual_view.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cockatrice/src/interface/widgets/visual_database_display/card_database_visual_view.cpp b/cockatrice/src/interface/widgets/visual_database_display/card_database_visual_view.cpp index 7b0d526f9..a8262c75a 100644 --- a/cockatrice/src/interface/widgets/visual_database_display/card_database_visual_view.cpp +++ b/cockatrice/src/interface/widgets/visual_database_display/card_database_visual_view.cpp @@ -130,8 +130,11 @@ void CardDatabaseVisualView::wheelEvent(QWheelEvent *event) return; } + // macOS uses transient (overlay) scroll bars that hide themselves + // once they fade out, so we need to use the range to determine if we ignore the wheel when there is nothing to + // scroll. This lets an enclosing scroll area take over. QScrollBar *bar = verticalScrollBar(); - if (!bar->isVisible()) { + if (bar->maximum() <= bar->minimum()) { event->ignore(); return; } From dd7ee3541c873beac3bf29df67835919c5657e0f Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:17:53 +0100 Subject: [PATCH 06/15] [PictureLoader] Keep the download log from going quadratic (#7766) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [PictureLoader] Keep the download log from going quadratic A fast scrollbar sweep over the visual card database requests a picture for every cell it paints, so one gesture can ask for thousands of downloads. The status bar built a seven-widget log entry for each of them and answered every completion with findChildren() over the whole, unbounded log, rebuilding url.toString() per child: O(n) per completion, O(n^2) per burst. Profiling a sweep on master put 47% of all cycles on the GUI thread, with qt_qFindChildren_helper at 12.4% of the process (a quarter of that thread) and QWidgetPrivate::reparentFocusChildren at 4.1%. LagMonitor reported stalls of 3-18 s, growing with the backlog. Keep at most MAX_LOG_ENTRIES entries and index them by URL, so a completion and the 1 Hz cleanup only visit the matching entries. The progress bar follows the entries that remain: dropping a finished entry moves both counters back, dropping a pending one only its maximum. The log is a diagnostic view, not a queue - the downloads themselves are unaffected. * fixup! [PictureLoader] Keep the download log from going quadratic Make the URL index a QMultiHash and say why: a deferred retry re-queues the same URL while its first log entry is still pending, so one URL can back several live entries. --------- Co-authored-by: Lukas Brübach --- .../card_picture_loader_status_bar.cpp | 93 ++++++++++++++++--- .../card_picture_loader_status_bar.h | 30 ++++++ 2 files changed, 108 insertions(+), 15 deletions(-) diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.cpp index 3b26c8033..a731d067a 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.cpp @@ -11,6 +11,7 @@ #include #include #include +#include class ExactCard; @@ -38,32 +39,94 @@ void CardPictureLoaderStatusBar::cleanOldEntries() if (!loadLog || !loadLog->popup) { return; } - for (CardPictureLoaderRequestStatusDisplayWidget *statusDisplayWidget : - loadLog->popup->findChildren()) { + + const QDateTime now = QDateTime::currentDateTime(); + // Removing an entry mutates the log, so walk a copy of it. + const QList entries = logEntries; + for (CardPictureLoaderRequestStatusDisplayWidget *statusDisplayWidget : entries) { statusDisplayWidget->queryElapsedSeconds(); - if (statusDisplayWidget->getFinished() && - QDateTime::fromString(statusDisplayWidget->getStartTime()).secsTo(QDateTime::currentDateTime()) > 10) { - loadLog->removeSettingsWidget(statusDisplayWidget); - progressBar->setMaximum(progressBar->maximum() - 1); - progressBar->setValue(progressBar->value() - 1); + if (!statusDisplayWidget->getFinished()) { + continue; + } + if (QDateTime::fromString(statusDisplayWidget->getStartTime()).secsTo(now) > LOG_ENTRY_LIFETIME_SECS) { + removeLogEntry(statusDisplayWidget); } } } void CardPictureLoaderStatusBar::addQueuedImageLoad(const QUrl &url, const ExactCard &card, const QString &setName) { - loadLog->addSettingsWidget(new CardPictureLoaderRequestStatusDisplayWidget(loadLog, url, card, setName)); + if (logEntries.size() >= MAX_LOG_ENTRIES) { + trimLog(); + } + + auto *entry = new CardPictureLoaderRequestStatusDisplayWidget(loadLog, url, card, setName); + loadLog->addSettingsWidget(entry); + logEntries.append(entry); + entriesByUrl.insert(entry->getUrl(), entry); progressBar->setMaximum(progressBar->maximum() + 1); } void CardPictureLoaderStatusBar::addSuccessfulImageLoad(const QUrl &url) { - progressBar->setValue(progressBar->value() + 1); - for (CardPictureLoaderRequestStatusDisplayWidget *statusDisplayWidget : - loadLog->popup->findChildren()) { - if (statusDisplayWidget->getUrl() == url.toString()) { - statusDisplayWidget->queryElapsedSeconds(); - statusDisplayWidget->setFinished(); + const QList entries = entriesByUrl.values(url.toString()); + if (entries.isEmpty()) { + // The entry may already have been dropped from the log; the download + // itself still succeeded, there is just nothing left to mark. + return; + } + + int finishedEntries = 0; + for (CardPictureLoaderRequestStatusDisplayWidget *statusDisplayWidget : entries) { + if (statusDisplayWidget->getFinished()) { + continue; + } + statusDisplayWidget->queryElapsedSeconds(); + statusDisplayWidget->setFinished(); + ++finishedEntries; + } + + // The bar counts the entries currently in the log, so only entries that are + // still listed move it. + progressBar->setValue(progressBar->value() + finishedEntries); +} + +void CardPictureLoaderStatusBar::trimLog() +{ + if (logEntries.isEmpty()) { + return; + } + + // Prefer evicting an entry that has already finished; only fall back to an + // in-flight request when the whole log is still pending. + CardPictureLoaderRequestStatusDisplayWidget *toRemove = nullptr; + for (CardPictureLoaderRequestStatusDisplayWidget *entry : logEntries) { + if (entry->getFinished()) { + toRemove = entry; + break; } } -} \ No newline at end of file + + removeLogEntry(toRemove != nullptr ? toRemove : logEntries.constFirst()); +} + +void CardPictureLoaderStatusBar::removeLogEntry(CardPictureLoaderRequestStatusDisplayWidget *entry) +{ + if (entry == nullptr) { + return; + } + + logEntries.removeOne(entry); + entriesByUrl.remove(entry->getUrl(), entry); + + // The progress bar's maximum counts the entries currently in the log and + // its value the finished ones, so dropping an entry moves both counters back. + // Move the value first: a maximum of 0 turns the bar into a busy indicator and + // resets its value. + if (entry->getFinished()) { + progressBar->setValue(qMax(0, progressBar->value() - 1)); + } + progressBar->setMaximum(qMax(0, progressBar->maximum() - 1)); + + loadLog->removeSettingsWidget(entry); +} diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.h b/cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.h index 2ba386c42..7f614e28f 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.h +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_status_bar.h @@ -1,10 +1,13 @@ #ifndef PICTURE_LOADER_STATUS_BAR_H #define PICTURE_LOADER_STATUS_BAR_H +#include +#include #include #include #include +class CardPictureLoaderRequestStatusDisplayWidget; class ExactCard; class QHBoxLayout; class QProgressBar; @@ -21,11 +24,20 @@ class SettingsButtonWidget; * - Shows overall progress of image downloads. * - Maintains a log of individual requests via a popup (SettingsButtonWidget). * - Cleans up finished request entries automatically after a delay. + * + * The log is a diagnostic view, not a queue: it keeps at most MAX_LOG_ENTRIES + * entries and indexes them by URL, so a burst of requests stays O(1) per event + * instead of scanning every entry the popup holds. */ class CardPictureLoaderStatusBar : public QWidget { Q_OBJECT public: + /** Entries kept in the log; older ones are evicted as new requests arrive. */ + static constexpr int MAX_LOG_ENTRIES = 50; + /** Seconds a finished entry stays in the log before it is cleaned up. */ + static constexpr int LOG_ENTRY_LIFETIME_SECS = 10; + /** * @brief Constructs a status bar with progress bar and log button. * @param parent Parent widget @@ -55,10 +67,28 @@ public slots: void cleanOldEntries(); private: + /** + * @brief Evicts the oldest finished entry (or the oldest entry if none finished) + * until the log fits within MAX_LOG_ENTRIES. + */ + void trimLog(); + + /** + * @brief Removes one entry from the log, its URL index and the popup, and keeps + * the progress bar counters in step with the entries that remain. + * @param entry Entry to remove + */ + void removeLogEntry(CardPictureLoaderRequestStatusDisplayWidget *entry); + QHBoxLayout *layout; ///< Horizontal layout containing progress bar and log button QProgressBar *progressBar; ///< Progress bar showing overall download progress SettingsButtonWidget *loadLog; ///< Popup log showing individual request statuses QTimer *cleaner; ///< Timer for periodically cleaning old log entries + QList logEntries; ///< Logged requests, oldest first + // A URL can back several live entries: a request parked for a deferred retry is re-queued while + // its first entry is still pending, so a completion must finish every entry for that URL. + QMultiHash + entriesByUrl; ///< Log entries indexed by their URL for O(1) completion lookup }; #endif // PICTURE_LOADER_STATUS_BAR_H From c5b776ca46792b4d9fbcc1c17362958e4e604705 Mon Sep 17 00:00:00 2001 From: "transifex-integration[bot]" <43880903+transifex-integration[bot]@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:31:12 +0100 Subject: [PATCH 07/15] Translate oracle/oracle_en@source.ts in de (#7767) 100% translated source file: 'oracle/oracle_en@source.ts' on 'de'. Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com> --- oracle/translations/oracle_de.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/oracle/translations/oracle_de.ts b/oracle/translations/oracle_de.ts index faf08d5ce..50a5f8e20 100644 --- a/oracle/translations/oracle_de.ts +++ b/oracle/translations/oracle_de.ts @@ -137,22 +137,22 @@ Downloading (0 MB) - + Herunterladen (0 MB) Downloading (%1 MB) - + Herunterladen (%1 MB) Parsing file (%1%) - + Datei wird verarbeitet (%1%) Extracting file... - + Extrahiere Datei... @@ -168,7 +168,7 @@ Parsing file (100%) - + Datei wird verarbeitet (100%) @@ -348,7 +348,7 @@ A Cockatrice card database file of %1 MB has been downloaded. - + Eine Cockatrice-Kartendatenbankdatei der Größe %1 MB wurde heruntergeladen. @@ -434,7 +434,7 @@ Downloading (0 MB) Downloading (0MB) - + Herunterladen (0 MB) From 381dd21e8c5441794a2df3f272037ff96641ffb8 Mon Sep 17 00:00:00 2001 From: PILIX123 <89544445+PILIX123@users.noreply.github.com> Date: Tue, 3 Nov 2026 04:33:21 -0500 Subject: [PATCH 08/15] [Client] Add option to load decks from ManaBox (#7642) * [Client] Add option to load decks from ManaBox * [Client] Fix PR comments * [Client] Fix UI and transaltion requests * [Client] Fix for UI and parser not doing what expected when loading maybe board * Add ManaBox support to docs As pointed out in review comments * [Client] Fix final PR comments --------- Co-authored-by: tooomm --- .../parsers/deck_link_to_api_transformer.cpp | 15 ++- .../parsers/deck_link_to_api_transformer.h | 1 + .../parsers/interface_json_deck_parser.h | 106 +++++++++++++++++- .../dialogs/dlg_load_deck_from_website.cpp | 16 ++- .../deck_management/importing_decks.md | 2 +- 5 files changed, 126 insertions(+), 14 deletions(-) diff --git a/cockatrice/src/client/network/parsers/deck_link_to_api_transformer.cpp b/cockatrice/src/client/network/parsers/deck_link_to_api_transformer.cpp index 7127b0a29..2b307735d 100644 --- a/cockatrice/src/client/network/parsers/deck_link_to_api_transformer.cpp +++ b/cockatrice/src/client/network/parsers/deck_link_to_api_transformer.cpp @@ -15,14 +15,18 @@ static const QString ARCHIDEKT_SUFFIX = "/?format=json"; static const QString MOXFIELD_BASE = "https://api.moxfield.com/v2/decks/all/"; static const QString MOXFIELD_SUFFIX = "/"; +static const QString DECKSTATS_BASE = "https://deckstats.net/decks/"; static const QString DECKSTATS_SUFFIX = "?include_comments=1&export_mtgarena=1"; +static const QString MANABOX_BASE = "https://cloud.manabox.app/decks/"; + bool parseDeckUrl(const QString &url, ParsedDeckInfo &outInfo) { static QRegularExpression rxTappedOut("tappedout\\.net/(?:mtg-decks/)?([^/?#]+)"); static QRegularExpression rxArchidekt("archidekt\\.com/decks/(\\d+)"); static QRegularExpression rxMoxfield("moxfield\\.com/decks/([a-zA-Z0-9_-]+)"); static QRegularExpression rxDeckstats("deckstats\\.net/decks/(\\d+/[a-zA-Z0-9_-]+)"); + static QRegularExpression rxManabox("manabox\\.app/decks/([a-zA-Z0-9_-]+)"); QRegularExpressionMatch match; @@ -49,11 +53,18 @@ bool parseDeckUrl(const QString &url, ParsedDeckInfo &outInfo) return true; } else if ((match = rxDeckstats.match(url)).hasMatch()) { QString deckPath = match.captured(1); - outInfo = ParsedDeckInfo{.baseUrl = "https://deckstats.net/decks/", + outInfo = ParsedDeckInfo{.baseUrl = DECKSTATS_BASE, .deckID = deckPath, - .fullUrl = "https://deckstats.net/decks/" + deckPath + DECKSTATS_SUFFIX, + .fullUrl = DECKSTATS_BASE + deckPath + DECKSTATS_SUFFIX, .provider = DeckProvider::Deckstats}; return true; + } else if ((match = rxManabox.match(url)).hasMatch()) { + QString deckPath = match.captured(1); + outInfo = ParsedDeckInfo{.baseUrl = MANABOX_BASE, + .deckID = deckPath, + .fullUrl = MANABOX_BASE + deckPath, + .provider = DeckProvider::Manabox}; + return true; } return false; diff --git a/cockatrice/src/client/network/parsers/deck_link_to_api_transformer.h b/cockatrice/src/client/network/parsers/deck_link_to_api_transformer.h index bbdd70c97..d3bff976f 100644 --- a/cockatrice/src/client/network/parsers/deck_link_to_api_transformer.h +++ b/cockatrice/src/client/network/parsers/deck_link_to_api_transformer.h @@ -15,6 +15,7 @@ enum class DeckProvider Archidekt, Moxfield, Deckstats, + Manabox, Unknown }; diff --git a/cockatrice/src/client/network/parsers/interface_json_deck_parser.h b/cockatrice/src/client/network/parsers/interface_json_deck_parser.h index 87fde4d54..8f1defb7a 100644 --- a/cockatrice/src/client/network/parsers/interface_json_deck_parser.h +++ b/cockatrice/src/client/network/parsers/interface_json_deck_parser.h @@ -8,12 +8,19 @@ #define INTERFACE_JSON_DECK_PARSER_H #include "../../../interface/deck_loader/card_node_function.h" +#include "libcockatrice/deck_list/tree/inner_deck_list_node.h" #include #include +#include +#include +#include +#include #include #include +inline Q_LOGGING_CATEGORY(InterfaceJsonDeckParserLog, "interface_json_deck_parser"); + class IJsonDeckParser { public: @@ -32,9 +39,6 @@ public: QString deckName = obj.value("name").toString(); QString deckDescription = obj.value("description").toString(); - deckList.setName(deckName); - deckList.setComments(deckDescription); - QString outputText; QTextStream outStream(&outputText); @@ -52,6 +56,8 @@ public: deckList.loadFromStream_Plain(outStream, false, CardNameNormalizer()); deckList.forEachCard(CardNodeFunction::ResolveProviderId()); + deckList.setName(deckName); + deckList.setComments(deckDescription); return deckList; } @@ -67,9 +73,6 @@ public: QString deckName = obj.value("name").toString(); QString deckDescription = obj.value("description").toString(); - deckList.setName(deckName); - deckList.setComments(deckDescription); - QString outputText; QTextStream outStream(&outputText); @@ -99,6 +102,8 @@ public: deckList.loadFromStream_Plain(outStream, false, CardNameNormalizer()); deckList.forEachCard(CardNodeFunction::ResolveProviderId()); + deckList.setName(deckName); + deckList.setComments(deckDescription); QJsonObject commandersObj = obj.value("commanders").toObject(); if (!commandersObj.isEmpty()) { @@ -118,4 +123,93 @@ public: } }; +class ManaboxJsonParser : public IJsonDeckParser +{ + enum BoardType : quint32 + { + MainBoard = 0, + SideBoard = 1, + Commander = 2, + MaybeBoard = 3, + }; + +public: + DeckList parse(const QJsonObject &obj) override + { + DeckList deckList; + + QString deckName = obj.value("name").toString(); + + QString outputText; + QString mainboardText; + QString sideboardText; + QTextStream mainboardStream(&mainboardText); + QTextStream sideboardStream(&sideboardText); + QTextStream outStream(&outputText); + + auto commanderList = QList(); + auto maybeboardList = QList(); + for (auto entry : obj.value("cards").toArray()) { + auto card = entry.toObject(); + + auto collectorNumber = card.value("collectorNumber").toString(); + auto cardName = card.value("name").toString(); + auto boardType = static_cast(card.value("boardCategory").toInt()); + auto setName = card.value("setId").toString(); + auto quantity = card.value("quantity").toInt(); + auto providerId = card.value("scryfallId").toString(); + + switch (boardType) { + case MainBoard: + mainboardStream << quantity << ' ' << cardName << " (" << setName << ") " << collectorNumber + << '\n'; + break; + case SideBoard: + sideboardStream << "SB: " << quantity << ' ' << cardName << " (" << setName << ") " + << collectorNumber << '\n'; + break; + case MaybeBoard: + maybeboardList.append(card); + break; + case Commander: + commanderList.append(card); + break; + default: + qCWarning(InterfaceJsonDeckParserLog) + << "Unknown ManaBox Zone" << card.value("boardCategory").toInt() << "for" << cardName; + break; + } + } + + outStream << mainboardText << sideboardText; + + deckList.loadFromStream_Plain(outStream, false, CardNameNormalizer()); + deckList.setName(deckName); + deckList.forEachCard(CardNodeFunction::ResolveProviderId()); + + for (const auto &maybeboardCard : maybeboardList) { + QString cardName = maybeboardCard.value("name").toString(); + QString setName = maybeboardCard.value("setId").toString().toUpper(); + QString collectorNumber = maybeboardCard.value("collectorNumber").toString(); + QString providerId = maybeboardCard.value("scryfallId").toString(); + + for (int x = 0; x < maybeboardCard.value("quantity").toInt(); x++) { + deckList.addCard(cardName, DECK_ZONE_MAYBEBOARD, -1, setName, collectorNumber, providerId); + } + } + for (const auto &commanderCard : commanderList) { + + QString commanderName = commanderCard.value("name").toString(); + QString setName = commanderCard.value("setId").toString().toUpper(); + QString collectorNumber = commanderCard.value("collectorNumber").toString(); + QString providerId = commanderCard.value("scryfallId").toString(); + + deckList.setBannerCard({commanderName, providerId}); + deckList.addCard(commanderName, DECK_ZONE_MAIN, -1, setName, collectorNumber, providerId); + } + + return deckList; + } +}; + #endif // INTERFACE_JSON_DECK_PARSER_H diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_website.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_website.cpp index 1c1ada302..04b0db2e3 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_website.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_website.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -33,6 +34,7 @@ class QWidget; +static const QStringList SUPPORTED_PROVIDERS = {"Archidekt", "Deckstats", "ManaBox", "Moxfield", "TappedOut"}; DlgLoadDeckFromWebsite::DlgLoadDeckFromWebsite(QWidget *parent) : QDialog(parent) { nam = NetworkManagerFactory::create(this); @@ -41,6 +43,7 @@ DlgLoadDeckFromWebsite::DlgLoadDeckFromWebsite(QWidget *parent) : QDialog(parent setLayout(layout); instructionLabel = new QLabel(this); + instructionLabel->setWordWrap(true); layout->addWidget(instructionLabel); urlEdit = new QLineEdit(this); @@ -64,8 +67,8 @@ DlgLoadDeckFromWebsite::DlgLoadDeckFromWebsite(QWidget *parent) : QDialog(parent void DlgLoadDeckFromWebsite::retranslateUi() { - instructionLabel->setText(tr("Paste a link to a decklist site here to import it.\n(Archidekt, Deckstats, Moxfield, " - "and TappedOut are supported.)")); + instructionLabel->setText(tr("Paste a link to a decklist site here to import it.") + "\n" + + tr("Supported services: %1").arg(SUPPORTED_PROVIDERS.join(", "))); } bool DlgLoadDeckFromWebsite::testValidUrl() @@ -84,8 +87,8 @@ void DlgLoadDeckFromWebsite::accept() if (!jsonParser && info.provider != DeckProvider::Deckstats && info.provider != DeckProvider::TappedOut) { qCWarning(DlgLoadDeckFromWebsiteLog) << "No parser found for provider"; QMessageBox::warning(this, tr("Load Deck from Website"), - tr("No parser available for this deck provider.\n (Archidekt, Deckstats, Moxfield, " - "and TappedOut are supported.)")); + tr("No parser available for this deck provider.") + "\n" + + tr("Supported services: %1").arg(SUPPORTED_PROVIDERS.join(", "))); QDialog::reject(); return; } @@ -152,6 +155,7 @@ void DlgLoadDeckFromWebsite::accept() "https://archidekt.com/decks/9999999\n" "https://deckstats.net/decks/99999/9999999-your-deck-name/en\n" "https://moxfield.com/decks/XYZxx-XYZ99Yyy-xyzXzzz\n" + "https://manabox.app/decks/XYZxxa-YZ99Yyyx_yzXzzz\n" "https://tappedout.net/mtg-decks/your-deck-name/")); QDialog::reject(); } @@ -164,7 +168,9 @@ QSharedPointer DlgLoadDeckFromWebsite::createParserForProvider( return QSharedPointer(new ArchidektJsonParser()); case DeckProvider::Moxfield: return QSharedPointer(new MoxfieldJsonParser()); + case DeckProvider::Manabox: + return QSharedPointer(new ManaboxJsonParser()); default: return QSharedPointer(nullptr); } -} \ No newline at end of file +} diff --git a/doc/doxygen/pages/user_documentation/deck_management/importing_decks.md b/doc/doxygen/pages/user_documentation/deck_management/importing_decks.md index 4aab7ad2c..fbfb8fe3c 100644 --- a/doc/doxygen/pages/user_documentation/deck_management/importing_decks.md +++ b/doc/doxygen/pages/user_documentation/deck_management/importing_decks.md @@ -70,4 +70,4 @@ confirm. The action will automatically import the deck from the online service without any other required user action. -Currently supported services are Archidekt, Deckstats, Moxfield, and TappedOut. \ No newline at end of file +Currently supported services are Archidekt, Deckstats, ManaBox, Moxfield, and TappedOut. From bfb53c5defeff356081789ff60bb103fd8fca75e Mon Sep 17 00:00:00 2001 From: Jurriaan Date: Tue, 3 Nov 2026 10:35:58 +0100 Subject: [PATCH 09/15] [Client] Visual Deck Editor: Added more mouse click modifiers to add/decrement cards to specific zones (#7589) * Make visual database mouse click modifiers properly add/decrement different card zones * Use event->modifiers(), refactor * Removed QApplication include * Added visual database display documentation page * Minor fixes * Added documentation for the mouse interaction of the table view --- .../tab_deck_editor_visual.cpp | 21 ++++++++++------ .../deck_management/editing_decks.md | 7 +++--- .../visual_database_display.md | 25 +++++++++++++++++++ 3 files changed, 43 insertions(+), 10 deletions(-) create mode 100644 doc/doxygen/pages/user_documentation/deck_management/visual_database_display.md diff --git a/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.cpp b/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.cpp index bf4e15aa6..84bf026f7 100644 --- a/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.cpp +++ b/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.cpp @@ -20,7 +20,6 @@ #include "tab_deck_editor_visual_tab_widget.h" #include -#include #include #include #include @@ -261,17 +260,25 @@ void TabDeckEditorVisual::processMainboardCardClick(const QMouseEvent *event, } } +/** @brief Get a zone name corresponding to keyboard modifiers. */ +static QString zoneForClickModifiers(const Qt::KeyboardModifiers &modifiers) +{ + if (modifiers.testFlag(Qt::ControlModifier)) { + return DECK_ZONE_SIDE; + } else if (modifiers.testFlag(Qt::ShiftModifier)) { + return DECK_ZONE_MAYBEBOARD; + } else { + return DECK_ZONE_MAIN; + } +} + /** @brief Handles clicks on cards in the database display. */ void TabDeckEditorVisual::processDatabaseCardClick(const QMouseEvent *event, const ExactCard &card) { if (event->button() == Qt::LeftButton) { - if (QApplication::keyboardModifiers() & Qt::ControlModifier) { - addCard(card, DECK_ZONE_SIDE); - } else { - addCard(card, DECK_ZONE_MAIN); - } + addCard(card, zoneForClickModifiers(event->modifiers())); } else if (event->button() == Qt::RightButton) { - decrementCard(card, DECK_ZONE_MAIN); + decrementCard(card, zoneForClickModifiers(event->modifiers())); } else if (event->button() == Qt::MiddleButton) { deckDockWidget->actRemoveCard(); } diff --git a/doc/doxygen/pages/user_documentation/deck_management/editing_decks.md b/doc/doxygen/pages/user_documentation/deck_management/editing_decks.md index 37b613461..04833282d 100644 --- a/doc/doxygen/pages/user_documentation/deck_management/editing_decks.md +++ b/doc/doxygen/pages/user_documentation/deck_management/editing_decks.md @@ -1,6 +1,7 @@ @page editing_decks Editing Decks - - @subpage editing_decks_classic - - @subpage editing_decks_visual +- @subpage editing_decks_classic +- @subpage editing_decks_visual - - @subpage editing_decks_printings +- @subpage visual_database_display +- @subpage editing_decks_printings diff --git a/doc/doxygen/pages/user_documentation/deck_management/visual_database_display.md b/doc/doxygen/pages/user_documentation/deck_management/visual_database_display.md new file mode 100644 index 000000000..097848ab8 --- /dev/null +++ b/doc/doxygen/pages/user_documentation/deck_management/visual_database_display.md @@ -0,0 +1,25 @@ +@page visual_database_display Visual Database Display + +# Purpose + +The VisualDatabaseDisplay presents all the cards from the enabled sets in the CardDatabase, providing a visual way to find and add cards to your DeckList. + +# Using the Visual Database Display + +The VisualDatabaseDisplay can be opened by itself using the Tabs section in the menu bar at the top of the screen, or by selecting the "Visual Database Display" tab in the visual deck editor. When opened as a part of the visual deck editor, the VisualDatabaseDisplay can be used to add and remove cards from your deck. See @ref editing_decks_visual for more information about modifying your deck. + +By default the cards are displayed as a grid of pictures. However, a table view using card names can be toggled by pressing the Visual/Table button at the top right of the widget. + +# Filtering cards + +The top section of the VisualDatabaseDisplay can be used to filter cards based on various card properties, as well as specifying in which way they should be sorted. Filters can be saved and reused at a later time by using the Save/Load button at the right of this panel. + +# Modifying the Deck list + +When the VisualDatabaseDisplay is opened as part of the visual deck editor, clicking on cards adds or removes them to and from your deck. + +In the picture view, left-clicking on a card will add that card to the main board of your deck, while right-clicking will remove that printing of the card from the main board, if it exists. When clicking cards, modifier keys can be used to add (or remove) the card to zones other than the main board. Holding a Ctrl key on Windows and Linux or a Command key on MacOS adds/removes cards to the sideboard, while the Shift keys add/remove cards to the maybeboard. + +In the table view, double-clicking on a card adds that card to the main board of your deck, while right-click opens the card menu. Holding the Ctrl key on Windows and Linux or the Command key on MacOS when double-clicking adds the card to the sideboard instead. + +Tokens are always added to the token zone, regardless of which view is open or which modifier key is pressed. From bc09ec6afe31cd0fcf7e19a4afaf4ceadc6b1f2c Mon Sep 17 00:00:00 2001 From: Jurriaan Date: Tue, 3 Nov 2026 10:37:17 +0100 Subject: [PATCH 10/15] [Client] Make DeckStateManager::decrementCard remove tokens from the token zone if the token is not in the given zone (#7588) * Fixed decrementCard for tokens not using the token zone * Removed dead function actDecrementCard Updated documentation * Fixed no-op, added documentation * Removed stray blank line * Allow finding card nodes even if no database entry exists for the given card * Added DeckStateManager tests * Added tokens on mainboard test * Added if checks and sorted sources Added if checks * Library include fix * Renamed zone to targetZone * Revert "Allow finding card nodes even if no database entry exists for the given card" This reverts commit 88578cd0d954f50907a19ea539c824e7e83dfa3d. * Combined loader_local_matching_test and deck_state_manager_test settings sources * Added settings sandbox, refactor * Format * Renamed return variable * Flattened if statement * Style fix --- .../deck_editor_deck_dock_widget.cpp | 12 -- .../deck_editor_deck_dock_widget.h | 1 - .../deck_editor/deck_state_manager.cpp | 22 +- .../widgets/deck_editor/deck_state_manager.h | 5 +- tests/CMakeLists.txt | 60 ++++-- tests/deck_state_manager_test.cpp | 190 ++++++++++++++++++ 6 files changed, 253 insertions(+), 37 deletions(-) create mode 100644 tests/deck_state_manager_test.cpp diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp index 37dd57459..626a1586f 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp +++ b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp @@ -762,18 +762,6 @@ void DeckEditorDeckDockWidget::actSwapSelection() update(); } -void DeckEditorDeckDockWidget::actDecrementCard(const ExactCard &card, QString zoneName) -{ - if (!card) { - return; - } - if (card.getInfo().getIsToken()) { - zoneName = DECK_ZONE_TOKENS; - } - - deckStateManager->decrementCard(card, zoneName); -} - void DeckEditorDeckDockWidget::actDecrementSelection() { auto selectedRows = getSelectedCardNodeSourceIndices(); diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h index 97ab8823e..fe03f02ae 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h +++ b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h @@ -70,7 +70,6 @@ public slots: void syncDisplayWidgetsToModel(); void actAddCard(const ExactCard &card, const QString &zoneName); void actIncrementSelection(); - void actDecrementCard(const ExactCard &card, QString zoneName); void actDecrementSelection(); void actSwapCard(const ExactCard &card, const QString &zoneName); void actSwapSelection(); diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp b/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp index a7e231ecf..cbf9dd3c4 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp +++ b/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp @@ -211,15 +211,15 @@ QModelIndex DeckStateManager::addCard(const ExactCard &card, const QString &zone return {}; } - QString zone = card.getInfo().getIsToken() ? DECK_ZONE_TOKENS : zoneName; + QString targetZone = card.getInfo().getIsToken() ? DECK_ZONE_TOKENS : zoneName; CardSetPtr set = card.getPrinting().getSet(); QString setName = set ? set->getCorrectedShortName() : ""; QString reason = - tr("Added (%1): %2 (%3) %4").arg(zone, card.getName(), setName, card.getPrinting().getProperty("num")); + tr("Added (%1): %2 (%3) %4").arg(targetZone, card.getName(), setName, card.getPrinting().getProperty("num")); - QModelIndex idx = modifyDeck(reason, [&card, &zone](auto model) { return model->addCard(card, zone); }); + QModelIndex idx = modifyDeck(reason, [&card, &targetZone](auto model) { return model->addCard(card, targetZone); }); if (idx.isValid()) { emit focusIndexChanged(idx, true); @@ -234,10 +234,22 @@ QModelIndex DeckStateManager::decrementCard(const ExactCard &card, const QString return {}; } + QString targetZone = zoneName; + QString providerId = card.getPrinting().getUuid(); QString collectorNumber = card.getPrinting().getProperty("num"); - QModelIndex idx = deckListModel->findCard(card.getName(), zoneName, providerId, collectorNumber); + QModelIndex idx = deckListModel->findCard(card.getName(), targetZone, providerId, collectorNumber); + + // Tokens are usually added to the token zone, but it is possible for imported/hand-edited + // decks to contain them in the main or sideboards. This is why we only try to remove the + // token from the token zone if it does not exist in the given zone, allowing us to still + // remove it from there. + if (!idx.isValid() && card.getInfo().getIsToken() && targetZone != DECK_ZONE_TOKENS) { + targetZone = DECK_ZONE_TOKENS; + idx = deckListModel->findCard(card.getName(), targetZone, providerId, collectorNumber); + } + if (!idx.isValid()) { return {}; } @@ -249,7 +261,7 @@ QModelIndex DeckStateManager::decrementCard(const ExactCard &card, const QString } // old index is no longer safe since rows could have been removed - QModelIndex newIdx = deckListModel->findCard(card.getName(), zoneName, providerId, collectorNumber); + QModelIndex newIdx = deckListModel->findCard(card.getName(), targetZone, providerId, collectorNumber); if (newIdx.isValid()) { emit focusIndexChanged(newIdx, true); } diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.h b/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.h index 54c59eb79..0612147eb 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.h +++ b/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.h @@ -188,17 +188,18 @@ public: ///@} /** - * @brief Adds the given card to the given zone. + * @brief Adds the given card to the given zone. Tokens always go to DECK_ZONE_TOKENS, ignoring the given zone. * Saves the operation to history if successful. * * @param card The card to add - * @param zoneName The zone to add the card to + * @param zoneName The zone to add the card to if the card is not a token * @return The index of the added card */ QModelIndex addCard(const ExactCard &card, const QString &zoneName); /** * @brief Removes 1 copy of the given card from the given zone. + * If the card is a token and is not in the given zone, then it is removed from DECK_ZONE_TOKENS instead. * Saves the operation to history if successful. * * @param card The card to remove diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0bb9654a1..cccff48c3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -412,23 +412,6 @@ cockatrice_add_test( INCLUDE_DIRS ${CMAKE_SOURCE_DIR}/cockatrice/src ) -cockatrice_add_test( - NAME - loader_local_matching_test - SOURCES - ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/card_picture_loader/card_picture_loader_local.cpp - ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/cache_settings.cpp - ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/card_counter_settings.cpp - ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/shortcuts_settings.cpp - ${CMAKE_SOURCE_DIR}/cockatrice/src/client/network/update/client/release_channel.cpp - ${VERSION_STRING_CPP} - loader_local_matching_test.cpp - LIBS - libcockatrice_network - libcockatrice_settings - INCLUDE_DIRS - ${CMAKE_SOURCE_DIR}/cockatrice/src -) cockatrice_add_test( NAME deck_hash_performance_test @@ -548,3 +531,46 @@ endif() if(WITH_ORACLE OR WITH_CLIENT) add_subdirectory(picture_loader_benchmark) endif() + +# loader_local_matching_test and deck_state_manager_test links libcockatrice_settings, +# which only exists when a client/UI-capable target is being built. +if(WITH_ORACLE OR WITH_CLIENT) + set(SETTINGS_SOURCES + ${CMAKE_SOURCE_DIR}/cockatrice/src/client/network/update/client/release_channel.cpp + ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/cache_settings.cpp + ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/card_counter_settings.cpp + ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/shortcuts_settings.cpp + ${VERSION_STRING_CPP} + ) + set(SETTINGS_LIBS libcockatrice_network libcockatrice_settings) + set(SETTINGS_INCLUDE_DIRS ${CMAKE_SOURCE_DIR}/cockatrice/src) + + cockatrice_add_test( + NAME + loader_local_matching_test + SOURCES + ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/card_picture_loader/card_picture_loader_local.cpp + ${SETTINGS_SOURCES} + loader_local_matching_test.cpp + LIBS + ${SETTINGS_LIBS} + INCLUDE_DIRS + ${SETTINGS_INCLUDE_DIRS} + ) + cockatrice_add_test( + NAME + deck_state_manager_test + SOURCES + ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp + ${SETTINGS_SOURCES} + deck_state_manager_test.cpp + LIBS + libcockatrice_card + libcockatrice_deck_list + libcockatrice_models + libcockatrice_utility + ${SETTINGS_LIBS} + INCLUDE_DIRS + ${SETTINGS_INCLUDE_DIRS} + ) +endif() diff --git a/tests/deck_state_manager_test.cpp b/tests/deck_state_manager_test.cpp new file mode 100644 index 000000000..ddd17912c --- /dev/null +++ b/tests/deck_state_manager_test.cpp @@ -0,0 +1,190 @@ +/** @file deck_state_manager_test.cpp + * @brief Tests for DeckStateManager operations. + * @ingroup Tests + */ + +#include "libcockatrice/card/database/card_database.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace +{ + +/** + * @brief Assert that the DeckStateManager's deck list contains only the given zone, + * and that that zone only contains the given number of cards, all with the given card name. + * + * @param manager the DeckStateManager + * @param restrictToZones In which zones to look for the zone and cards. Empty means use the entire deck + * @param zone The only zone that should exist in the deck + * @param cardName The name of the only card that should exist in the deck + * @param cardNumber The number of cards that should be in the deck + */ +static void containsExactly(const DeckStateManager &manager, + const QSet &restrictToZones, + QString zone, + QString cardName, + int cardNumber) +{ + QList zones = manager.getDeckList().getZoneNodes(restrictToZones); + ASSERT_EQ(zones.size(), 1); + ASSERT_EQ(zones[0]->getName(), zone); + ASSERT_EQ(zones[0]->size(), 1); + const DecklistCardNode *card = dynamic_cast((*zones[0])[0]); + ASSERT_NE(card, nullptr); + ASSERT_EQ(card->getName(), cardName); + ASSERT_EQ(card->getNumber(), cardNumber); +} + +/** @brief Return CardInfo of a token with the given name. */ +static CardInfoPtr makeToken(QString name) +{ + return CardInfo::newInstance(name, "", true, {}, {}, {}, {}, {}); +} + +// ===================================================================================================================== +// addCard +// ===================================================================================================================== + +TEST(DeckStateManager, AddCards) +{ + DeckStateManager manager; + manager.addCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), DECK_ZONE_MAIN); + containsExactly(manager, {}, DECK_ZONE_MAIN, "Lightning Bolt", 1); + manager.addCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), DECK_ZONE_MAIN); + containsExactly(manager, {}, DECK_ZONE_MAIN, "Lightning Bolt", 2); +} + +TEST(DeckStateManager, AddCardZones) +{ + DeckStateManager manager; + manager.addCard(ExactCard(CardInfo::newInstance("Grizzly Bears")), DECK_ZONE_MAIN); + manager.addCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), DECK_ZONE_SIDE); + manager.addCard(ExactCard(CardInfo::newInstance("Swords to Plowshares")), DECK_ZONE_MAYBEBOARD); + + // Check if the cards got put into the correct zones. + ASSERT_EQ(manager.getDeckList().getZoneNodes().size(), 3); + containsExactly(manager, {DECK_ZONE_MAIN}, DECK_ZONE_MAIN, "Grizzly Bears", 1); + containsExactly(manager, {DECK_ZONE_SIDE}, DECK_ZONE_SIDE, "Lightning Bolt", 1); + containsExactly(manager, {DECK_ZONE_MAYBEBOARD}, DECK_ZONE_MAYBEBOARD, "Swords to Plowshares", 1); +} + +TEST(DeckStateManager, AddTokens) +{ + DeckStateManager manager; + manager.addCard(ExactCard(makeToken("Beast")), DECK_ZONE_MAIN); + manager.addCard(ExactCard(makeToken("Beast")), DECK_ZONE_SIDE); + manager.addCard(ExactCard(makeToken("Beast")), DECK_ZONE_MAYBEBOARD); + manager.addCard(ExactCard(makeToken("Beast")), DECK_ZONE_TOKENS); + + // A token should always be put into the tokens zone. + containsExactly(manager, {}, DECK_ZONE_TOKENS, "Beast", 4); +} + +// ===================================================================================================================== +// decrementCard +// ===================================================================================================================== + +TEST(DeckStateManager, DecrementCards) +{ + DeckStateManager manager; + manager.addCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), DECK_ZONE_MAIN); + manager.addCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), DECK_ZONE_MAIN); + containsExactly(manager, {}, DECK_ZONE_MAIN, "Lightning Bolt", 2); + + // Removing from the sideboard should do nothing, as there is nothing in the side board. + manager.decrementCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), DECK_ZONE_SIDE); + containsExactly(manager, {}, DECK_ZONE_MAIN, "Lightning Bolt", 2); + + // Remove the first card. + manager.decrementCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), DECK_ZONE_MAIN); + containsExactly(manager, {}, DECK_ZONE_MAIN, "Lightning Bolt", 1); + + // Removing the second card should also remove the zone. + manager.decrementCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), DECK_ZONE_MAIN); + ASSERT_EQ(manager.getDeckList().getZoneNodes().size(), 0); +} + +TEST(DeckStateManager, DecrementTokens) +{ + DeckStateManager manager; + manager.addCard(ExactCard(makeToken("Beast")), DECK_ZONE_MAIN); + manager.addCard(ExactCard(makeToken("Beast")), DECK_ZONE_SIDE); + manager.addCard(ExactCard(makeToken("Beast")), DECK_ZONE_TOKENS); + containsExactly(manager, {}, DECK_ZONE_TOKENS, "Beast", 3); + + manager.decrementCard(ExactCard(makeToken("Beast")), DECK_ZONE_TOKENS); + containsExactly(manager, {}, DECK_ZONE_TOKENS, "Beast", 2); + + // Removing tokens from a given zone should remove them from the + // token zone if the given zone does not contain the token. + manager.decrementCard(ExactCard(makeToken("Beast")), DECK_ZONE_SIDE); + containsExactly(manager, {}, DECK_ZONE_TOKENS, "Beast", 1); + + manager.decrementCard(ExactCard(makeToken("Beast")), DECK_ZONE_MAIN); + ASSERT_EQ(manager.getDeckList().getZoneNodes().size(), 0); +} + +TEST(DeckStateManager, DecrementTokenMainboard) +{ + DeckStateManager manager; + + // Add cards to the deck list directly so tokens are not + // automatically added to the token zone. + manager.getDeckListShared()->addCard("Beast", DECK_ZONE_MAIN); + manager.getDeckListShared()->addCard("Beast", DECK_ZONE_TOKENS); + ASSERT_EQ(manager.getDeckList().getZoneNodes().size(), 2); + containsExactly(manager, {DECK_ZONE_MAIN}, DECK_ZONE_MAIN, "Beast", 1); + containsExactly(manager, {DECK_ZONE_TOKENS}, DECK_ZONE_TOKENS, "Beast", 1); + + // decrementCard uses the model to find the card, so we need to rebuild it. + manager.getModel()->rebuildTree(); + + // Tokens should be removed from the main zone first. + manager.decrementCard(ExactCard(makeToken("Beast")), DECK_ZONE_MAIN); + ASSERT_EQ(manager.getDeckList().getZoneNodes({DECK_ZONE_MAIN}).size(), 0); + containsExactly(manager, {DECK_ZONE_TOKENS}, DECK_ZONE_TOKENS, "Beast", 1); + + // If the main zone is empty they can be removed from the token zone. + manager.decrementCard(ExactCard(makeToken("Beast")), DECK_ZONE_MAIN); + ASSERT_EQ(manager.getDeckList().getZoneNodes().size(), 0); +} +} // namespace + +int main(int argc, char **argv) +{ + // Redirect SettingsCache reads/writes (app-data location) away from the real user profile. + QStandardPaths::setTestModeEnabled(true); + QCoreApplication app(argc, argv); + QCoreApplication::setOrganizationName(QStringLiteral("CockatriceTest")); + QCoreApplication::setApplicationName( + QStringLiteral("deck-state-manager-test-%1").arg(QCoreApplication::applicationPid())); + + ::testing::InitGoogleTest(&argc, argv); + + // Add cards to the card database + auto *db = CardDatabaseManager::getInstance(); + db->addCard(CardInfo::newInstance("Lightning Bolt")); + db->addCard(CardInfo::newInstance("Grizzly Bears")); + db->addCard(CardInfo::newInstance("Swords to Plowshares")); + db->addCard(makeToken("Beast")); + + int result = RUN_ALL_TESTS(); + + // Clean up the settings directory in ~/.qttest if the tests ran. + if (!::testing::GTEST_FLAG(list_tests)) { + QDir(SettingsCache::instance().getDataPath()).removeRecursively(); + QDir(SettingsCache::instance().getCachePath()).removeRecursively(); + } + + return result; +} From ada8f399f1a18152c1264943fd284c93ea605fac Mon Sep 17 00:00:00 2001 From: Bryce! Date: Tue, 3 Nov 2026 04:38:54 -0500 Subject: [PATCH 11/15] Log server identification and connection lifecycle (#7731) * Log remote server identification and connection lifecycle [skip ci] * Update libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp Co-authored-by: RickyRister <42636155+RickyRister@users.noreply.github.com> --------- Co-authored-by: BruebachL <44814898+BruebachL@users.noreply.github.com> Co-authored-by: RickyRister <42636155+RickyRister@users.noreply.github.com> --- .../network/client/remote/remote_client.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp index faacdc9f7..df1413dd2 100644 --- a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp +++ b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp @@ -137,6 +137,7 @@ RemoteClient::~RemoteClient() void RemoteClient::slotSocketError(QAbstractSocket::SocketError /*error*/) { QString errorString = socket->errorString(); + qCWarning(RemoteClientLog) << "Server connection error:" << errorString; doDisconnectFromServer(); emit socketError(errorString); } @@ -146,6 +147,7 @@ void RemoteClient::slotWebSocketError(QAbstractSocket::SocketError /*error*/) QString errorString = websocket->errorString(); if (getStatus() != ClientStatus::StatusDisconnected) { + qCWarning(RemoteClientLog) << "Server connection error:" << errorString; doDisconnectFromServer(); emit socketError(errorString); } @@ -153,6 +155,7 @@ void RemoteClient::slotWebSocketError(QAbstractSocket::SocketError /*error*/) void RemoteClient::slotConnected() { + qCInfo(RemoteClientLog) << "Server transport connected"; timeRunning = lastDataReceived = 0; timer->start(); @@ -166,6 +169,12 @@ void RemoteClient::slotConnected() void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentification &event) { + // Identification arrives before login, including when protocol versions differ. + // Quote the server-supplied strings in the log and limit their size. + const QString serverName = QString::fromStdString(event.server_name()).left(256); + const QString serverVersion = QString::fromStdString(event.server_version()).left(256); + qCInfo(RemoteClientLog) << "Server identification:" << serverName + << "version" << serverVersion << "protocol version" << event.protocol_version(); if (event.protocol_version() != protocolVersion) { emit protocolVersionMismatch(protocolVersion, event.protocol_version()); setStatus(StatusDisconnecting); @@ -698,6 +707,8 @@ void RemoteClient::sendCommandContainer(const CommandContainer &cont) void RemoteClient::connectToHost(const QString &hostname, unsigned int port) { usingWebSocket = port == 443 || port == 80 || port == 4748 || port == 8080; + qCInfo(RemoteClientLog) << "Connecting to server" << hostname << "port" << port + << (usingWebSocket ? "using WebSocket" : "using TCP"); if (usingWebSocket) { QUrl url(QString("%1://%2:%3/servatrice").arg(port == 443 ? "wss" : "ws").arg(hostname).arg(port)); websocket->open(url); From dc4aa19d23fa24dbbd61a7522d9baaee4ce695dd Mon Sep 17 00:00:00 2001 From: Bryce! Date: Tue, 3 Nov 2026 04:40:28 -0500 Subject: [PATCH 12/15] Import Moxfield signature spells into the deck [skip ci] (#7751) Co-authored-by: Codex --- .../parsers/interface_json_deck_parser.h | 26 ++++++ .../libcockatrice/deck_list/deck_list.cpp | 19 ++++- .../libcockatrice/deck_list/deck_list.h | 8 ++ tests/CMakeLists.txt | 13 +++ tests/moxfield_signature_spells_test.cpp | 84 +++++++++++++++++++ 5 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 tests/moxfield_signature_spells_test.cpp diff --git a/cockatrice/src/client/network/parsers/interface_json_deck_parser.h b/cockatrice/src/client/network/parsers/interface_json_deck_parser.h index 8f1defb7a..7e15d472f 100644 --- a/cockatrice/src/client/network/parsers/interface_json_deck_parser.h +++ b/cockatrice/src/client/network/parsers/interface_json_deck_parser.h @@ -54,7 +54,11 @@ public: outStream << quantity << ' ' << cardName << " (" << setName << ") " << collectorNumber << '\n'; } + outStream.flush(); + outStream.seek(0); deckList.loadFromStream_Plain(outStream, false, CardNameNormalizer()); + deckList.setName(deckName); + deckList.setComments(deckDescription); deckList.forEachCard(CardNodeFunction::ResolveProviderId()); deckList.setName(deckName); deckList.setComments(deckDescription); @@ -100,7 +104,11 @@ public: outStream << quantity << ' ' << cardName << " (" << setName << ") " << collectorNumber << '\n'; } + outStream.flush(); + outStream.seek(0); deckList.loadFromStream_Plain(outStream, false, CardNameNormalizer()); + deckList.setName(deckName); + deckList.setComments(deckDescription); deckList.forEachCard(CardNodeFunction::ResolveProviderId()); deckList.setName(deckName); deckList.setComments(deckDescription); @@ -119,6 +127,24 @@ public: } } + // Moxfield stores Oathbreaker signature spells in their own board, + // separate from both mainboard and commanders. They are playable deck + // cards, but must not replace the Oathbreaker as the banner card. + const QJsonObject signatureSpells = obj.value("signatureSpells").toObject(); + for (auto it = signatureSpells.begin(); it != signatureSpells.end(); ++it) { + const QJsonObject entry = it.value().toObject(); + const QJsonObject card = entry.value("card").toObject(); + const QString name = card.value("name").toString(); + const int quantity = entry.value("quantity").toInt(); + if (name.isEmpty() || quantity <= 0) { + continue; + } + const QString set = card.value("set").toString().toUpper(); + const QString collectorNumber = card.value("cn").toString(); + const QString providerId = card.value("scryfall_id").toString(); + deckList.addCards(name, quantity, DECK_ZONE_MAIN, -1, set, collectorNumber, providerId); + } + return deckList; } }; diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp index 821b53dff..2cd5ef914 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp @@ -423,8 +423,23 @@ DecklistCardNode *DeckList::addCard(const QString &cardName, const QString &cardProviderId, bool formatLegal) { - auto node = - tree.addCard(cardName, 1, zoneName, position, cardSetName, cardSetCollectorNumber, cardProviderId, formatLegal); + return addCards(cardName, 1, zoneName, position, cardSetName, cardSetCollectorNumber, cardProviderId, formatLegal); +} + +DecklistCardNode *DeckList::addCards(const QString &cardName, + int amount, + const QString &zoneName, + int position, + const QString &cardSetName, + const QString &cardSetCollectorNumber, + const QString &cardProviderId, + bool formatLegal) +{ + if (amount <= 0) { + return nullptr; + } + auto node = tree.addCard(cardName, amount, zoneName, position, cardSetName, cardSetCollectorNumber, cardProviderId, + formatLegal); refreshDeckHash(); return node; } diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h index 6796e9c33..9d0f20586 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h @@ -284,6 +284,14 @@ public: const QString &cardSetCollectorNumber = QString(), const QString &cardProviderId = QString(), const bool formatLegal = true); + DecklistCardNode *addCards(const QString &cardName, + int amount, + const QString &zoneName, + int position = -1, + const QString &cardSetName = QString(), + const QString &cardSetCollectorNumber = QString(), + const QString &cardProviderId = QString(), + const bool formatLegal = true); ///@} /** @name Deck identity */ diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index cccff48c3..6acd225c2 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -463,6 +463,19 @@ endif() # Guard must match the condition for src/single_instance_manager.cpp in cockatrice/CMakeLists.txt. if(WITH_CLIENT) + cockatrice_add_test( + NAME + moxfield_signature_spells_test + SOURCES + ${VERSION_STRING_CPP} + ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/deck_loader/card_node_function.cpp + moxfield_signature_spells_test.cpp + LIBS + libcockatrice_card + libcockatrice_deck_list + INCLUDE_DIRS + ${CMAKE_SOURCE_DIR}/cockatrice/src + ) cockatrice_add_test( NAME single_instance_manager_test diff --git a/tests/moxfield_signature_spells_test.cpp b/tests/moxfield_signature_spells_test.cpp new file mode 100644 index 000000000..54346a934 --- /dev/null +++ b/tests/moxfield_signature_spells_test.cpp @@ -0,0 +1,84 @@ +#include "client/network/parsers/interface_json_deck_parser.h" + +#include +#include +#include +#include + +TEST(MoxfieldImport, SignatureSpellIsAddedToMainWithoutReplacingCommanderBanner) +{ + // Moxfield v2 board objects map IDs to entries with quantity and card. + const auto input = QJsonDocument::fromJson(R"json({ + "name": "Oathbreaker example", "description": "Notes retained", + "mainboard": {"land": {"quantity": 1, "card": {"name": "Island", "set": "abc", "cn": "1"}}}, + "sideboard": {"side": {"quantity": 1, "card": {"name": "Mountain", "set": "abc", "cn": "2"}}}, + "commanders": {"walker": {"quantity": 1, "card": {"name": "Jace", "set": "abc", "cn": "3", "scryfall_id": "walker-id"}}}, + "signatureSpells": {"spell": {"quantity": 1, "card": {"name": "Lightning Bolt", "set": "xyz", "cn": "4", "scryfall_id": "spell-id"}}} + })json") + .object(); + MoxfieldJsonParser parser; + DeckList deck = parser.parse(input); + int mainSpellCount = 0; + int sideSpellCount = 0; + int mainLands = 0; + int sideLands = 0; + deck.forEachCard([&](InnerDecklistNode *zone, DecklistCardNode *card) { + if (card->getName() == "Island" && zone->getName() == DECK_ZONE_MAIN) { + mainLands += card->getNumber(); + } + if (card->getName() == "Mountain" && zone->getName() == DECK_ZONE_SIDE) { + sideLands += card->getNumber(); + } + if (card->getName() != "Lightning Bolt") { + return; + } + if (zone->getName() == DECK_ZONE_MAIN) { + mainSpellCount += card->getNumber(); + EXPECT_EQ(card->getCardSetShortName(), "XYZ"); + EXPECT_EQ(card->getCardCollectorNumber(), "4"); + EXPECT_EQ(card->getCardProviderId(), "spell-id"); + } else if (zone->getName() == DECK_ZONE_SIDE) { + sideSpellCount += card->getNumber(); + } + }); + EXPECT_EQ(mainSpellCount, 1); + EXPECT_EQ(sideSpellCount, 0); + EXPECT_EQ(mainLands, 1); + EXPECT_EQ(sideLands, 1); + EXPECT_EQ(deck.getName(), "Oathbreaker example"); + EXPECT_EQ(deck.getComments(), "Notes retained"); + EXPECT_EQ(deck.getBannerCard().name, "Jace"); + EXPECT_EQ(deck.getBannerCard().providerId, "walker-id"); +} + +TEST(MoxfieldImport, QuantityAwareDeckInsertionRetainsCountAndPrinting) +{ + DeckList deck; + auto *card = deck.addCards("Island", 2, DECK_ZONE_MAIN, -1, "ABC", "4", "id"); + ASSERT_NE(card, nullptr); + EXPECT_EQ(card->getNumber(), 2); + EXPECT_EQ(card->getCardProviderId(), "id"); + EXPECT_EQ(deck.addCards("Island", 0, DECK_ZONE_MAIN), nullptr); +} + +TEST(OnlineImport, ArchidektPreservesNameDescriptionAndCards) +{ + const auto input = QJsonDocument::fromJson(R"json({ + "name": "Online deck", "description": "Deck notes", + "cards": [{"quantity": 2, "card": { + "oracleCard": {"name": "Island"}, "edition": {"editioncode": "abc"}, "collectorNumber": "1" + }}] + })json") + .object(); + ArchidektJsonParser parser; + const DeckList deck = parser.parse(input); + int count = 0; + deck.forEachCard([&](InnerDecklistNode *zone, DecklistCardNode *card) { + if (zone->getName() == DECK_ZONE_MAIN && card->getName() == "Island") { + count += card->getNumber(); + } + }); + EXPECT_EQ(count, 2); + EXPECT_EQ(deck.getName(), "Online deck"); + EXPECT_EQ(deck.getComments(), "Deck notes"); +} From fd2a99e8973f4d44858b39a8539f25f6fe841c7e Mon Sep 17 00:00:00 2001 From: Bryce! Date: Tue, 3 Nov 2026 04:41:47 -0500 Subject: [PATCH 13/15] Respect nonempty Qt hints and explain discovery failures [skip ci] (#7733) --- CMakeLists.txt | 10 ++++++---- cmake/FindQtRuntime.cmake | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8a25fe2c6..677ef0c12 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -71,6 +71,8 @@ if(WIN32 OR USE_VCPKG) ) # Qt path set by user or env var if(QTDIR + OR QTDIR32 + OR QTDIR64 OR DEFINED ENV{QTDIR} OR DEFINED ENV{QTDIR32} OR DEFINED ENV{QTDIR64} @@ -236,13 +238,13 @@ else() set(_lib_suffix 32) endif() -if(DEFINED QTDIR${_lib_suffix}) +if(NOT "${QTDIR${_lib_suffix}}" STREQUAL "") list(APPEND CMAKE_PREFIX_PATH "${QTDIR${_lib_suffix}}") -elseif(DEFINED QTDIR) +elseif(NOT "${QTDIR}" STREQUAL "") list(APPEND CMAKE_PREFIX_PATH "${QTDIR}") -elseif(DEFINED ENV{QTDIR${_lib_suffix}}) +elseif(NOT "$ENV{QTDIR${_lib_suffix}}" STREQUAL "") list(APPEND CMAKE_PREFIX_PATH "$ENV{QTDIR${_lib_suffix}}") -elseif(DEFINED ENV{QTDIR}) +elseif(NOT "$ENV{QTDIR}" STREQUAL "") list(APPEND CMAKE_PREFIX_PATH "$ENV{QTDIR}") endif() diff --git a/cmake/FindQtRuntime.cmake b/cmake/FindQtRuntime.cmake index 2d74aca4d..7b644276d 100644 --- a/cmake/FindQtRuntime.cmake +++ b/cmake/FindQtRuntime.cmake @@ -1,6 +1,6 @@ # Find a compatible Qt version # Inputs: WITH_SERVER, WITH_CLIENT, WITH_ORACLE -# Optional Input: QT6_DIR -- Hint as to where Qt6 lives on the system +# Optional Input: Qt6_DIR -- Directory containing Qt6Config.cmake # Output: COCKATRICE_QT_VERSION_NAME -- Example values: Qt6 # Output: SERVATRICE_QT_MODULES # Output: COCKATRICE_QT_MODULES @@ -57,7 +57,17 @@ find_package( QUIET HINTS ${Qt6_DIR} ) if(NOT Qt6_FOUND) - message(FATAL_ERROR "No suitable version of Qt was found") + set(_QT_DIAGNOSTIC "") + if(Qt6_NOT_FOUND_MESSAGE) + set(_QT_DIAGNOSTIC "\nQt package diagnostic: ${Qt6_NOT_FOUND_MESSAGE}") + endif() + message(FATAL_ERROR + "No suitable version of Qt was found. Qt 6.4.2 or newer with these components is required: " + "${REQUIRED_QT_COMPONENTS};Linguist.\n" + "Set QTDIR or CMAKE_PREFIX_PATH to the Qt installation prefix, or Qt6_DIR to the directory " + "containing Qt6Config.cmake." + "${_QT_DIAGNOSTIC}" + ) endif() set(COCKATRICE_QT_VERSION_NAME Qt6) From 9588e1960d01817997bf8ea39c74fcf51b666ad2 Mon Sep 17 00:00:00 2001 From: Bryce! Date: Tue, 3 Nov 2026 05:10:50 -0500 Subject: [PATCH 14/15] Preserve saved deck formats through database reload and view refresh (#7745) --- .../deck_editor_deck_dock_widget.cpp | 49 +++++++++++-------- .../deck_editor_deck_dock_widget.h | 1 + 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp index 626a1586f..30fde1595 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp +++ b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp @@ -49,6 +49,7 @@ #include #include #include +#include #include #include #include @@ -187,10 +188,17 @@ void DeckEditorDeckDockWidget::createDeckDock() formatLabel = new QLabel(this); formatComboBox = new QComboBox(this); + formatComboBox->setObjectName("deckFormatComboBox"); formatComboBox->addItem(tr("Loading Database...")); formatComboBox->setEnabled(false); // Disable until loaded commanderBracketWidget = new CommanderBracketWidget(this); + connect(formatComboBox, QOverload::of(&QComboBox::currentIndexChanged), this, [this](int index) { + formatComboBox->setPlaceholderText(QString()); + formatComboBox->setToolTip(QString()); + deckStateManager->setFormat(index >= 0 ? formatComboBox->itemData(index).toString() : QString()); + commanderBracketWidget->setDeck(deckStateManager->getDeckListShared()); + }); commentsLabel = new QLabel(); commentsLabel->setObjectName("commentsLabel"); @@ -355,6 +363,9 @@ void DeckEditorDeckDockWidget::initializeFormats() { QStringList allFormats = CardDatabaseManager::query()->getAllFormatsWithCount().keys(); + // Rebuilding the list is a view update, not a user edit. In particular, + // clear() emits currentIndexChanged(-1), which must not clear the deck. + const QSignalBlocker blocker(formatComboBox); formatComboBox->clear(); // Remove "Loading Database..." formatComboBox->setEnabled(true); @@ -364,28 +375,25 @@ void DeckEditorDeckDockWidget::initializeFormats() formatComboBox->addItem(formatName, formatName); // store the raw key in itemData } - QString format = deckStateManager->getMetadata().gameFormat; - if (!format.isEmpty()) { - formatComboBox->setCurrentIndex(formatComboBox->findData(format)); - } else { - // Ensure no selection is visible initially - formatComboBox->setCurrentIndex(-1); - } - connect(formatComboBox, QOverload::of(&QComboBox::currentIndexChanged), this, [this](int index) { - QString formatKey; - if (index >= 0) { - formatKey = formatComboBox->itemData(index).toString(); - deckStateManager->setFormat(formatKey); - } else { - deckStateManager->setFormat(""); // clear format if deselected - } - - commanderBracketWidget->setDeck(deckStateManager->getDeckListShared()); - }); + syncFormatSelection(); commanderBracketWidget->setDeck(deckStateManager->getDeckListShared()); } +void DeckEditorDeckDockWidget::syncFormatSelection() +{ + if (!formatComboBox->isEnabled()) { + return; + } + const QSignalBlocker blocker(formatComboBox); + const QString format = deckStateManager->getMetadata().gameFormat; + const int index = format.isEmpty() ? -1 : formatComboBox->findData(format); + const QString unavailable = index < 0 && !format.isEmpty() ? tr("Unavailable: %1").arg(format) : QString(); + formatComboBox->setPlaceholderText(unavailable); + formatComboBox->setToolTip(unavailable); + formatComboBox->setCurrentIndex(index); +} + ExactCard DeckEditorDeckDockWidget::getCurrentCard() { QModelIndex current = deckView->selectionModel()->currentIndex(); @@ -596,9 +604,7 @@ void DeckEditorDeckDockWidget::syncDisplayWidgetsToModel() updatePlaymatLabel(); updateHash(); - formatComboBox->blockSignals(true); - formatComboBox->setCurrentIndex(formatComboBox->findData(deckStateManager->getMetadata().gameFormat)); - formatComboBox->blockSignals(false); + syncFormatSelection(); commanderBracketWidget->setDeck(deckStateManager->getDeckListShared()); @@ -1042,6 +1048,7 @@ void DeckEditorDeckDockWidget::retranslateUi() commentsLabel->setText(tr("&Comments:")); activeGroupCriteriaLabel->setText(tr("Group by:")); formatLabel->setText(tr("Format:")); + syncFormatSelection(); commanderBracketWidget->retranslateUi(); hashLabel1->setText(tr("Hash:")); diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h index fe03f02ae..02910e390 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h +++ b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h @@ -110,6 +110,7 @@ private: DeckListModel *getModel() const; [[nodiscard]] QModelIndexList getSelectedCardNodeSourceIndices() const; void offsetCountAtIndex(const QModelIndex &idx, bool isIncrement); + void syncFormatSelection(); void addMoveToZoneMenu(QMenu *menu, const QModelIndex &sourceCardIndex, const QString ¤tBoardName); void addChangeBoardMenu(QMenu *menu, const QString &zoneName); From c72192fd8105d24b21e3b089053418a145b74f71 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Tue, 3 Nov 2026 15:59:32 +0100 Subject: [PATCH 15/15] [Format] Small fixup (#7768) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Format] Small fixup. * [CardDB] Stop leaking the loader's mutexes. Both mutexes were heap-allocated through default member initializers and never freed, so every CardDatabase destruction leaked 16 bytes. LeakSanitizer flags this while discovering tests that create the database in main. Hold them by value instead, as the loader is a QObject and therefore neither copyable nor movable. --------- Co-authored-by: Lukas Brübach --- .../libcockatrice/card/database/card_database_loader.cpp | 4 ++-- .../libcockatrice/card/database/card_database_loader.h | 4 ++-- .../libcockatrice/network/client/remote/remote_client.cpp | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libcockatrice_card/libcockatrice/card/database/card_database_loader.cpp b/libcockatrice_card/libcockatrice/card/database/card_database_loader.cpp index 00b21f88e..7ca5ba433 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database_loader.cpp +++ b/libcockatrice_card/libcockatrice/card/database/card_database_loader.cpp @@ -83,7 +83,7 @@ LoadStatus CardDatabaseLoader::loadCardDatabase(const QString &path, CardDatabas auto startTime = QTime::currentTime(); LoadStatus tempLoadStatus = NotLoaded; if (!path.isEmpty()) { - QMutexLocker locker(loadFromFileMutex); + QMutexLocker locker(&loadFromFileMutex); tempLoadStatus = loadFromFile(path, data); } @@ -102,7 +102,7 @@ LoadStatus CardDatabaseLoader::loadCardDatabases(bool ignoreCache) LoadStatus CardDatabaseLoader::doLoadCardDatabases(bool ignoreCache) { - QMutexLocker locker(reloadDatabaseMutex); + QMutexLocker locker(&reloadDatabaseMutex); if (!database) { qCWarning(CardDatabaseLoadingLog) << "Loader has no database pointer"; diff --git a/libcockatrice_card/libcockatrice/card/database/card_database_loader.h b/libcockatrice_card/libcockatrice/card/database/card_database_loader.h index bfbaa0f51..72aeeff3e 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database_loader.h +++ b/libcockatrice_card/libcockatrice/card/database/card_database_loader.h @@ -177,8 +177,8 @@ private: ICardSetPriorityController *priorityController; /**< Controller for reconstructing set state. */ QList availableParsers; /**< List of available parsers for different formats. */ - QBasicMutex *loadFromFileMutex = new QBasicMutex(); /**< Mutex for single-file loading. */ - QBasicMutex *reloadDatabaseMutex = new QBasicMutex(); /**< Mutex for reloading entire database. */ + QBasicMutex loadFromFileMutex; /**< Mutex for single-file loading. */ + QBasicMutex reloadDatabaseMutex; /**< Mutex for reloading entire database. */ }; #endif // COCKATRICE_CARD_DATABASE_LOADER_H diff --git a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp index df1413dd2..fa75e356a 100644 --- a/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp +++ b/libcockatrice_network/libcockatrice/network/client/remote/remote_client.cpp @@ -173,8 +173,8 @@ void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentifica // Quote the server-supplied strings in the log and limit their size. const QString serverName = QString::fromStdString(event.server_name()).left(256); const QString serverVersion = QString::fromStdString(event.server_version()).left(256); - qCInfo(RemoteClientLog) << "Server identification:" << serverName - << "version" << serverVersion << "protocol version" << event.protocol_version(); + qCInfo(RemoteClientLog) << "Server identification:" << serverName << "version" << serverVersion + << "protocol version" << event.protocol_version(); if (event.protocol_version() != protocolVersion) { emit protocolVersionMismatch(protocolVersion, event.protocol_version()); setStatus(StatusDisconnecting);