Reload card db and notify enabled sets change on "Manage Sets" dialog save (#6837)

* Reload card db and notify enabled sets change on "Manage Sets" dialog save

Took 1 hour 18 minutes

Took 6 seconds

* Extract to method, also notify on "Reload db" and "new sets found"

Took 3 minutes

Took 4 seconds

* Add an "always enable new sets" fuse to "new sets found" dialog

Took 11 minutes

* Always debounce modelDirty() with dirty() timer.

Took 29 minutes

Took 3 minutes

* Performance improvements for settings by not constructing a new settings object on every single set() call (this forced a sync to/from fs but it seems fine to just rely on Qts own periodic sync?)

Took 23 minutes

Took 3 seconds

---------

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

cockatrice/src/client/settings/cache_settings.cpp

@@ -211,6 +211,7 @@ SettingsCache::SettingsCache()
     startupCardUpdateCheckAlwaysUpdate = settings->value("personal/startupCardUpdateCheckAlwaysUpdate", false).toBool();
     cardUpdateCheckInterval = settings->value("personal/cardUpdateCheckInterval", 7).toInt();
     lastCardUpdateCheck = settings->value("personal/lastCardUpdateCheck", QDateTime::currentDateTime().date()).toDate();
+    alwaysEnableNewSets = settings->value("personal/alwaysEnableNewSets", false).toBool();
     notifyAboutUpdates = settings->value("personal/updatenotification", true).toBool();
     notifyAboutNewVersion = settings->value("personal/newversionnotification", true).toBool();
 
@@ -1246,6 +1247,12 @@ void SettingsCache::setLastCardUpdateCheck(QDate value)
     settings->setValue("personal/lastCardUpdateCheck", lastCardUpdateCheck);
 }
 
+void SettingsCache::setAlwaysEnableNewSets(bool value)
+{
+    alwaysEnableNewSets = value;
+    settings->setValue("personal/alwaysEnableNewSets", alwaysEnableNewSets);
+}
+
 void SettingsCache::setRememberGameSettings(const bool _rememberGameSettings)
 {
     rememberGameSettings = _rememberGameSettings;

cockatrice/src/client/settings/cache_settings.h

@@ -216,6 +216,7 @@ private:
     bool checkCardUpdatesOnStartup;
     int cardUpdateCheckInterval;
     QDate lastCardUpdateCheck;
+    bool alwaysEnableNewSets;
     bool notifyAboutUpdates;
     bool notifyAboutNewVersion;
     bool showTipsOnStartup;
@@ -502,6 +503,10 @@ public:
         return getLastCardUpdateCheck().daysTo(QDateTime::currentDateTime().date()) >= getCardUpdateCheckInterval() &&
                getLastCardUpdateCheck() != QDateTime::currentDateTime().date();
     }
+    [[nodiscard]] bool getAlwaysEnableNewSets() const
+    {
+        return alwaysEnableNewSets;
+    }
     [[nodiscard]] bool getNotifyAboutUpdates() const override
     {
         return notifyAboutUpdates;
@@ -1125,6 +1130,7 @@ public slots:
     void setStartupCardUpdateCheckAlwaysUpdate(bool value);
     void setCardUpdateCheckInterval(int value);
     void setLastCardUpdateCheck(QDate value);
+    void setAlwaysEnableNewSets(bool value);
     void setNotifyAboutUpdate(QT_STATE_CHANGED_T _notifyaboutupdate);
     void setNotifyAboutNewVersion(QT_STATE_CHANGED_T _notifyaboutnewversion);
     void setUpdateReleaseChannelIndex(int value);

cockatrice/src/client/settings/card_counter_settings.cpp

@@ -11,8 +11,6 @@ CardCounterSettings::CardCounterSettings(const QString &settingsPath, QObject *p
 
 void CardCounterSettings::setColor(int counterId, const QColor &color)
 {
-    QSettings settings = getSettings();
-
     QString key = QString("cards/counters/%1/color").arg(counterId);
 
     if (settings.value(key).value<QColor>() == color)
@@ -38,7 +36,7 @@ QColor CardCounterSettings::color(int counterId) const
         defaultColor = QColor::fromHsv(h, s, v);
     }
 
-    return getSettings().value(QString("cards/counters/%1/color").arg(counterId), defaultColor).value<QColor>();
+    return settings.value(QString("cards/counters/%1/color").arg(counterId), defaultColor).value<QColor>();
 }
 
 QString CardCounterSettings::displayName(int counterId) const

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

@@ -16,6 +16,7 @@
 #include <QPushButton>
 #include <QToolBar>
 #include <QTreeView>
+#include <QtConcurrentRun>
 #include <algorithm>
 #include <libcockatrice/card/database/card_database_manager.h>
 #include <libcockatrice/models/database/card_set/card_sets_model.h>
@@ -253,6 +254,11 @@ void WndSets::actSave()
     model->save(CardDatabaseManager::getInstance());
     SettingsCache::instance().setIncludeRebalancedCards(includeRebalancedCards);
     CardPictureLoader::clearPixmapCache();
+    const auto reloadOk1 = QtConcurrent::run([] {
+        CardDatabaseManager::getInstance()->reloadCardDatabasesAndNotify();
+
+        SettingsCache::instance().downloads().sync();
+    });
     close();
 }
 

cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.cpp

@@ -29,6 +29,11 @@ VisualDatabaseDisplayWidget::VisualDatabaseDisplayWidget(QWidget *parent,
     : QWidget(parent), deckEditor(_deckEditor), databaseModel(database_model),
       databaseDisplayModel(database_display_model)
 {
+    debounceTimer = new QTimer(this);
+    debounceTimer->setSingleShot(true); // Ensure it only fires once after the timeout
+
+    connect(debounceTimer, &QTimer::timeout, this, &VisualDatabaseDisplayWidget::onSearchModelChanged);
+
     cards = new QList<ExactCard>;
     connect(databaseDisplayModel, &CardDatabaseDisplayModel::modelDirty, this,
             &VisualDatabaseDisplayWidget::modelDirty);
@@ -157,11 +162,6 @@ void VisualDatabaseDisplayWidget::initialize()
 
     mainLayout->addWidget(cardSizeWidget);
 
-    debounceTimer = new QTimer(this);
-    debounceTimer->setSingleShot(true); // Ensure it only fires once after the timeout
-
-    connect(debounceTimer, &QTimer::timeout, this, &VisualDatabaseDisplayWidget::onSearchModelChanged);
-
     databaseDisplayModel->setFilterTree(filterModel->filterTree());
 
     connect(filterModel, &FilterTreeModel::layoutChanged, this, &VisualDatabaseDisplayWidget::onSearchModelChanged);
@@ -286,7 +286,9 @@ void VisualDatabaseDisplayWidget::loadNextPage()
     }
 
     // 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)

cockatrice/src/interface/window_main.cpp

@@ -1173,6 +1173,13 @@ void MainWindow::cardDatabaseLoadingFailed()
 
 void MainWindow::cardDatabaseNewSetsFound(int numUnknownSets, QStringList unknownSetsNames)
 {
+    if (SettingsCache::instance().getAlwaysEnableNewSets()) {
+        CardDatabaseManager::getInstance()->enableAllUnknownSets();
+        const auto reloadOk1 =
+            QtConcurrent::run([] { CardDatabaseManager::getInstance()->reloadCardDatabasesAndNotify(); });
+        return;
+    }
+
     QMessageBox msgBox(this);
     msgBox.setWindowTitle(tr("New sets found"));
     msgBox.setIcon(QMessageBox::Question);
@@ -1183,6 +1190,7 @@ void MainWindow::cardDatabaseNewSetsFound(int numUnknownSets, QStringList unknow
                        .arg(unknownSetsNames.join(", ")));
 
     QPushButton *yesButton = msgBox.addButton(tr("Yes"), QMessageBox::YesRole);
+    QPushButton *yesAlwaysButton = msgBox.addButton(tr("Yes, always enable"), QMessageBox::YesRole);
     QPushButton *noButton = msgBox.addButton(tr("No"), QMessageBox::NoRole);
     QPushButton *settingsButton = msgBox.addButton(tr("View sets"), QMessageBox::ActionRole);
     msgBox.setDefaultButton(yesButton);
@@ -1191,7 +1199,13 @@ void MainWindow::cardDatabaseNewSetsFound(int numUnknownSets, QStringList unknow
 
     if (msgBox.clickedButton() == yesButton) {
         CardDatabaseManager::getInstance()->enableAllUnknownSets();
-        const auto reloadOk1 = QtConcurrent::run([] { CardDatabaseManager::getInstance()->loadCardDatabases(); });
+        const auto reloadOk1 =
+            QtConcurrent::run([] { CardDatabaseManager::getInstance()->reloadCardDatabasesAndNotify(); });
+    } else if (msgBox.clickedButton() == yesAlwaysButton) {
+        CardDatabaseManager::getInstance()->enableAllUnknownSets();
+        const auto reloadOk1 =
+            QtConcurrent::run([] { CardDatabaseManager::getInstance()->reloadCardDatabasesAndNotify(); });
+        SettingsCache::instance().setAlwaysEnableNewSets(true);
     } else if (msgBox.clickedButton() == noButton) {
         CardDatabaseManager::getInstance()->markAllSetsAsKnown();
     } else if (msgBox.clickedButton() == settingsButton) {
@@ -1473,7 +1487,7 @@ int MainWindow::getNextCustomSetPrefix(QDir dataDir)
 void MainWindow::actReloadCardDatabase()
 {
     const auto reloadOk1 = QtConcurrent::run([] {
-        CardDatabaseManager::getInstance()->loadCardDatabases();
+        CardDatabaseManager::getInstance()->reloadCardDatabasesAndNotify();
         SettingsCache::instance().downloads().sync();
     });
 }

libcockatrice_card/libcockatrice/card/database/card_database.cpp

@@ -1,6 +1,7 @@
 #include "card_database.h"
 
 #include "../relation/card_relation.h"
+#include "card_database_manager.h"
 #include "parser/cockatrice_xml_4.h"
 
 #include <QCryptographicHash>
@@ -60,6 +61,17 @@ void CardDatabase::loadCardDatabases()
     loadStatus = loader->loadCardDatabases();
 }
 
+void CardDatabase::reloadCardDatabasesAndNotify()
+{
+    loadCardDatabases();
+
+    QMetaObject::Connection conn;
+    conn = connect(this, &CardDatabase::cardDatabaseLoadingFinished, this, [conn, this]() mutable {
+        notifyEnabledSetsChanged();
+        QObject::disconnect(conn);
+    });
+}
+
 bool CardDatabase::saveCustomTokensToFile()
 {
     return loader->saveCustomTokensToFile();

libcockatrice_card/libcockatrice/card/database/card_database.h

@@ -147,6 +147,7 @@ public slots:
 
     /** @brief Loads card databases from configured paths. */
     void loadCardDatabases();
+    void reloadCardDatabasesAndNotify();
 
     /** @brief Saves custom tokens to file.
      * @return True if successful.

libcockatrice_models/libcockatrice/models/database/card_database_display_model.cpp

@@ -10,11 +10,28 @@ CardDatabaseDisplayModel::CardDatabaseDisplayModel(QObject *parent)
     setSortCaseSensitivity(Qt::CaseInsensitive);
 
     dirtyTimer.setSingleShot(true);
-    connect(&dirtyTimer, &QTimer::timeout, this, &CardDatabaseDisplayModel::invalidate);
+    connect(&dirtyTimer, &QTimer::timeout, this, [this]() {
+        invalidate();
+        emit modelDirty();
+    });
 
     loadedRowCount = 0;
 }
 
+void CardDatabaseDisplayModel::setSourceModel(QAbstractItemModel *model)
+{
+    QSortFilterProxyModel::setSourceModel(model);
+
+    connect(model, &QAbstractItemModel::rowsInserted, this, [this]() { dirty(); });
+
+    connect(model, &QAbstractItemModel::rowsRemoved, this, [this]() { dirty(); });
+
+    connect(model, &QAbstractItemModel::modelReset, this, [this]() {
+        loadedRowCount = 0;
+        dirty();
+    });
+}
+
 QMap<wchar_t, wchar_t> CardDatabaseDisplayModel::characterTranslation = {{L'“', L'\"'},
                                                                          {L'”', L'\"'},
                                                                          {L'‘', L'\''},

libcockatrice_models/libcockatrice/models/database/card_database_display_model.h

@@ -38,11 +38,11 @@ private:
 
 public:
     explicit CardDatabaseDisplayModel(QObject *parent = nullptr);
+    void setSourceModel(QAbstractItemModel *model) override;
     void setFilterTree(FilterTree *_filterTree);
     void setIsToken(FilterBool _isToken)
     {
         isToken = _isToken;
-        emit modelDirty();
         dirty();
     }
 
@@ -53,20 +53,17 @@ public:
             filterString = nullptr;
         }
         cardName = sanitizeCardName(_cardName, characterTranslation);
-        emit modelDirty();
         dirty();
     }
     void setStringFilter(const QString &_src)
     {
         delete filterString;
         filterString = new FilterString(_src);
-        emit modelDirty();
         dirty();
     }
     void setCardNameSet(const QSet<QString> &_cardNameSet)
     {
         cardNameSet = _cardNameSet;
-        emit modelDirty();
         dirty();
     }
 

libcockatrice_settings/libcockatrice/settings/settings_manager.cpp

@@ -4,19 +4,13 @@ SettingsManager::SettingsManager(const QString &_settingPath,
                                  const QString &_defaultGroup,
                                  const QString &_defaultSubGroup,
                                  QObject *parent)
-    : QObject(parent), settingPath(_settingPath), defaultGroup(_defaultGroup), defaultSubGroup(_defaultSubGroup)
+    : QObject(parent), defaultGroup(_defaultGroup), defaultSubGroup(_defaultSubGroup),
+      settings(_settingPath, QSettings::IniFormat)
 {
 }
 
-QSettings SettingsManager::getSettings() const
-{
-    return QSettings(settingPath, QSettings::IniFormat);
-}
-
 void SettingsManager::setValue(const QVariant &value, const QString &name)
 {
-    auto settings = getSettings();
-
     if (!defaultGroup.isEmpty()) {
         settings.beginGroup(defaultGroup);
     }
@@ -41,8 +35,6 @@ void SettingsManager::setValue(const QVariant &value,
                                const QString &group,
                                const QString &subGroup)
 {
-    auto settings = getSettings();
-
     if (!group.isEmpty()) {
         settings.beginGroup(group);
     }
@@ -64,8 +56,6 @@ void SettingsManager::setValue(const QVariant &value,
 
 void SettingsManager::deleteValue(const QString &name)
 {
-    auto settings = getSettings();
-
     if (!defaultGroup.isEmpty()) {
         settings.beginGroup(defaultGroup);
     }
@@ -87,8 +77,6 @@ void SettingsManager::deleteValue(const QString &name)
 
 void SettingsManager::deleteValue(const QString &name, const QString &group, const QString &subGroup)
 {
-    auto settings = getSettings();
-
     if (!group.isEmpty()) {
         settings.beginGroup(group);
     }
@@ -110,8 +98,6 @@ void SettingsManager::deleteValue(const QString &name, const QString &group, con
 
 QVariant SettingsManager::getValue(const QString &name) const
 {
-    auto settings = getSettings();
-
     if (!defaultGroup.isEmpty()) {
         settings.beginGroup(defaultGroup);
     }
@@ -135,8 +121,6 @@ QVariant SettingsManager::getValue(const QString &name) const
 
 QVariant SettingsManager::getValue(const QString &name, const QString &group, const QString &subGroup) const
 {
-    auto settings = getSettings();
-
     if (!group.isEmpty()) {
         settings.beginGroup(group);
     }
@@ -163,7 +147,5 @@ QVariant SettingsManager::getValue(const QString &name, const QString &group, co
  */
 void SettingsManager::sync()
 {
-    auto settings = getSettings();
-
     settings.sync();
 }

libcockatrice_settings/libcockatrice/settings/settings_manager.h

@@ -14,26 +14,31 @@
 class SettingsManager : public QObject
 {
     Q_OBJECT
+
 public:
     explicit SettingsManager(const QString &settingPath,
                              const QString &defaultGroup = QString(),
                              const QString &defaultSubGroup = QString(),
                              QObject *parent = nullptr);
+
     QVariant getValue(const QString &name) const;
     QVariant getValue(const QString &name, const QString &group, const QString &subGroup = QString()) const;
+
     void sync();
 
 protected:
-    QString settingPath;
     QString defaultGroup;
     QString defaultSubGroup;
 
-    QSettings getSettings() const;
+    mutable QSettings settings;
 
     void setValue(const QVariant &value, const QString &name);
+
     void
     setValue(const QVariant &value, const QString &name, const QString &group, const QString &subGroup = QString());
+
     void deleteValue(const QString &name);
+
     void deleteValue(const QString &name, const QString &group, const QString &subGroup = QString());
 };