[VDS] Decouple tag filter and fix reordered-chips crash

cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_color_identity_filter_widget.cpp

@@ -1,11 +1,10 @@
 #include "deck_preview_color_identity_filter_widget.h"
 
 #include "../../cards/additional_info/mana_symbol_widget.h"
-#include "../visual_deck_storage_widget.h"
 
 #include <QSet>
 
-DeckPreviewColorIdentityFilterWidget::DeckPreviewColorIdentityFilterWidget(VisualDeckStorageWidget *parent)
+DeckPreviewColorIdentityFilterWidget::DeckPreviewColorIdentityFilterWidget(QWidget *parent)
     : QWidget(parent), layout(new QHBoxLayout(this))
 {
     setLayout(layout);

cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_color_identity_filter_widget.h

@@ -14,14 +14,12 @@
 #include <QSet>
 #include <QWidget>
 
-class VisualDeckStorageWidget;
-
 class DeckPreviewColorIdentityFilterWidget : public QWidget
 {
     Q_OBJECT
 
 public:
-    explicit DeckPreviewColorIdentityFilterWidget(VisualDeckStorageWidget *parent);
+    explicit DeckPreviewColorIdentityFilterWidget(QWidget *parent = nullptr);
     void retranslateUi();
 
     /**

cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_search_widget.cpp

@@ -39,3 +39,8 @@ VisualDeckStorageSearchWidget::VisualDeckStorageSearchWidget(QWidget *parent) :
 
     connect(searchDebounceTimer, &QTimer::timeout, this, [this] { emit searchTextChanged(searchBar->text()); });
 }
+
+void VisualDeckStorageSearchWidget::setPlaceholderText(const QString &text)
+{
+    searchBar->setPlaceholderText(text);
+}

cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_search_widget.h

@@ -19,6 +19,8 @@ class VisualDeckStorageSearchWidget : public QWidget
 public:
     explicit VisualDeckStorageSearchWidget(QWidget *parent);
 
+    void setPlaceholderText(const QString &text);
+
 signals:
     /**
      * Emitted once the debounce timer fires after the user stopped typing.

cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_tag_filter_widget.cpp

@@ -2,14 +2,10 @@
 
 #include "../general/layout_containers/flow_widget.h"
 #include "deck_preview/deck_preview_tag_display_widget.h"
-#include "visual_deck_storage_model.h"
-#include "visual_deck_storage_sort_filter_proxy_model.h"
-#include "visual_deck_storage_widget.h"
 
 #include <QHBoxLayout>
 
-VisualDeckStorageTagFilterWidget::VisualDeckStorageTagFilterWidget(VisualDeckStorageWidget *_parent)
-    : QWidget(_parent), parent(_parent)
+VisualDeckStorageTagFilterWidget::VisualDeckStorageTagFilterWidget(QWidget *parent) : QWidget(parent)
 {
 
     setSizePolicy(QSizePolicy::Minimum, QSizePolicy::Minimum);
@@ -25,97 +21,62 @@ VisualDeckStorageTagFilterWidget::VisualDeckStorageTagFilterWidget(VisualDeckSto
     layout->addWidget(flowWidget);
 }
 
+void VisualDeckStorageTagFilterWidget::setAllTagsProvider(const std::function<QSet<QString>()> &provider)
+{
+    allTagsProvider = provider;
+}
+
 void VisualDeckStorageTagFilterWidget::showEvent(QShowEvent *event)
 {
     QWidget::showEvent(event);
     refreshTags();
 }
 
-/**
- * @brief The tags of all decks currently accepted by the proxy model.
- */
-QSet<QString> VisualDeckStorageTagFilterWidget::gatherAllTags() const
-{
-    QSet<QString> allTags;
-    auto *proxy = parent->proxyModel();
-
-    for (int proxyRow = 0; proxyRow < proxy->rowCount(); ++proxyRow) {
-        const QModelIndex index = proxy->index(proxyRow, 0);
-        if (!index.data(VisualDeckStorageRoles::FilterMatchRole).toBool()) {
-            continue;
-        }
-        const QStringList deckTags = index.data(VisualDeckStorageRoles::TagsRole).toStringList();
-        for (const QString &tag : deckTags) {
-            allTags.insert(tag);
-        }
-    }
-
-    return allTags;
-}
-
 void VisualDeckStorageTagFilterWidget::refreshTags()
 {
-    QSet<QString> allTags = gatherAllTags();
-    removeTagsNotInList(allTags);
-    addTagsIfNotPresent(allTags);
-    sortTags();
-}
+    const QSet<QString> allTags = allTagsProvider ? allTagsProvider() : QSet<QString>();
 
-void VisualDeckStorageTagFilterWidget::removeTagsNotInList(const QSet<QString> &tags)
-{
+    // Existing chips survive if their tag is still part of the deck set, or if the chip
+    // is currently selected/excluded. Everything else is dropped. Dropped chips must NOT
+    // be re-added to the layout afterwards: they are scheduled for a deferred delete, and
+    // the flow layout would keep a dangling reference to them once the deletion runs on
+    // the next event-loop cycle.
+    QList<DeckPreviewTagDisplayWidget *> chips;
     for (DeckPreviewTagDisplayWidget *tagWidget : findChildren<DeckPreviewTagDisplayWidget *>()) {
-        const QString &tagName = tagWidget->getTagName();
-
-        // Keep the tag widget if it is either selected or excluded
-        if (!tags.contains(tagName) && tagWidget->getState() == TagState::NotSelected) {
+        if (tagWidget->getState() != TagState::NotSelected || allTags.contains(tagWidget->getTagName())) {
+            chips.append(tagWidget);
+        } else {
             flowWidget->removeWidget(tagWidget);
             tagWidget->deleteLater();
         }
     }
-}
 
-void VisualDeckStorageTagFilterWidget::addTagsIfNotPresent(const QSet<QString> &tags)
-{
-    for (const QString &tag : tags) {
-        addTagIfNotPresent(tag);
-    }
-}
-
-void VisualDeckStorageTagFilterWidget::addTagIfNotPresent(const QString &tag)
-{
-    // Check if the tag already exists in the flow widget
-    bool tagExists = false;
-    for (DeckPreviewTagDisplayWidget *tagWidget : findChildren<DeckPreviewTagDisplayWidget *>()) {
-        if (tagWidget->getTagName() == tag) {
-            tagExists = true;
-            break;
+    // Add chips for tags that are not shown yet.
+    for (const QString &tag : allTags) {
+        bool tagExists = false;
+        for (DeckPreviewTagDisplayWidget *tagWidget : chips) {
+            if (tagWidget->getTagName() == tag) {
+                tagExists = true;
+                break;
+            }
+        }
+        if (!tagExists) {
+            auto *newTagWidget = new DeckPreviewTagDisplayWidget(this, tag);
+            connect(newTagWidget, &DeckPreviewTagDisplayWidget::tagClicked, this,
+                    &VisualDeckStorageTagFilterWidget::filterChanged);
+            flowWidget->addWidget(newTagWidget);
+            chips.append(newTagWidget);
         }
     }
 
-    // If the tag doesn't exist, add a new DeckPreviewTagDisplayWidget
-    if (!tagExists) {
-        auto *newTagWidget = new DeckPreviewTagDisplayWidget(this, tag);
-        connect(newTagWidget, &DeckPreviewTagDisplayWidget::tagClicked, parent,
-                &VisualDeckStorageWidget::updateTagFilter);
-        flowWidget->addWidget(newTagWidget);
-    }
-}
-
-void VisualDeckStorageTagFilterWidget::sortTags()
-{
-    // Get all tag widgets
-    QList<DeckPreviewTagDisplayWidget *> tagWidgets = findChildren<DeckPreviewTagDisplayWidget *>();
-
-    // Sort widgets by tag name
-    std::sort(tagWidgets.begin(), tagWidgets.end(), [](DeckPreviewTagDisplayWidget *a, DeckPreviewTagDisplayWidget *b) {
+    // Clear and re-add the chips in sorted order.
+    std::sort(chips.begin(), chips.end(), [](DeckPreviewTagDisplayWidget *a, DeckPreviewTagDisplayWidget *b) {
         return a->getTagName().toLower() < b->getTagName().toLower();
     });
-
-    // Clear and re-add widgets in sorted order
-    for (DeckPreviewTagDisplayWidget *tagWidget : tagWidgets) {
+    for (DeckPreviewTagDisplayWidget *tagWidget : chips) {
         flowWidget->removeWidget(tagWidget);
     }
-    for (DeckPreviewTagDisplayWidget *tagWidget : tagWidgets) {
+    for (DeckPreviewTagDisplayWidget *tagWidget : chips) {
         flowWidget->addWidget(tagWidget);
     }
 }

cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_tag_filter_widget.h

@@ -9,26 +9,27 @@
 #include <QSet>
 #include <QStringList>
 #include <QWidget>
+#include <functional>
 
 class FlowWidget;
 class VisualDeckStorageWidget;
+
 class VisualDeckStorageTagFilterWidget : public QWidget
 {
     Q_OBJECT
 
-    VisualDeckStorageWidget *parent;
     FlowWidget *flowWidget;
-
-    [[nodiscard]] QSet<QString> gatherAllTags() const;
-    void removeTagsNotInList(const QSet<QString> &tags);
-    void addTagsIfNotPresent(const QSet<QString> &tags);
-    void addTagIfNotPresent(const QString &tag);
-    void sortTags();
+    std::function<QSet<QString>()> allTagsProvider;
 
 public:
-    explicit VisualDeckStorageTagFilterWidget(VisualDeckStorageWidget *_parent);
+    explicit VisualDeckStorageTagFilterWidget(QWidget *parent = nullptr);
     [[nodiscard]] QStringList getAllKnownTags() const;
 
+    /**
+     * @brief Sets a provider for the full set of tags to draw chips from.
+     */
+    void setAllTagsProvider(const std::function<QSet<QString>()> &provider);
+
     /**
      * @brief The tags currently in "selected" state.
      */
@@ -39,9 +40,15 @@ public:
      */
     [[nodiscard]] QStringList excludedTags() const;
 
+signals:
+    /**
+     * Emitted whenever a chip's selection/exclusion state changes.
+     */
+    void filterChanged();
+
 public slots:
     /**
-     * @brief Rebuilds the tag chips from the tags of the currently visible decks.
+     * @brief Rebuilds the tag chips from the currently available tags.
      */
     void refreshTags();
     void showEvent(QShowEvent *event) override;

cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_widget.cpp

@@ -61,6 +61,9 @@ VisualDeckStorageWidget::VisualDeckStorageWidget(QWidget *parent) : QWidget(pare
 
     // tag filter box
     tagFilterWidget = new VisualDeckStorageTagFilterWidget(this);
+    tagFilterWidget->setAllTagsProvider([this] { return gatherVisibleTags(); });
+    connect(tagFilterWidget, &VisualDeckStorageTagFilterWidget::filterChanged, this,
+            &VisualDeckStorageWidget::updateTagFilter);
     updateTagsVisibility(SettingsCache::instance().visualDeckStorage().getVisualDeckStorageShowTagFilter());
 
     deckPreviewSelectionAnimationEnabled =
@@ -216,6 +219,25 @@ void VisualDeckStorageWidget::updateTagFilter()
     tagFilterWidget->refreshTags();
 }
 
+/**
+ * @brief The tags of all decks currently accepted by the proxy model.
+ */
+QSet<QString> VisualDeckStorageWidget::gatherVisibleTags() const
+{
+    QSet<QString> allTags;
+    for (int proxyRow = 0; proxyRow < storageProxyModel->rowCount(); ++proxyRow) {
+        const QModelIndex index = storageProxyModel->index(proxyRow, 0);
+        if (!index.data(VisualDeckStorageRoles::FilterMatchRole).toBool()) {
+            continue;
+        }
+        const QStringList deckTags = index.data(VisualDeckStorageRoles::TagsRole).toStringList();
+        for (const QString &tag : deckTags) {
+            allTags.insert(tag);
+        }
+    }
+    return allTags;
+}
+
 /**
  * Pushes the color identity filter widget's state into the proxy model.
  */

cockatrice/src/interface/widgets/visual_deck_storage/visual_deck_storage_widget.h

@@ -70,6 +70,7 @@ protected:
 
 private:
     void reapplySortAndFilters();
+    [[nodiscard]] QSet<QString> gatherVisibleTags() const;
 
 private:
     QVBoxLayout *layout;