[Picture Loader] Consider local images, remove some unused variables. (#5985)

* Consider local images, remove some unused variables.

* Move checking for local card images outside of card download loop, add NOT found debug line.

---------

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

cockatrice/src/client/ui/picture_loader/picture_loader_worker.cpp

@@ -15,17 +15,8 @@
 // Card back returned by gatherer when card is not found
 QStringList PictureLoaderWorker::md5Blacklist = QStringList() << "db0c48db407a907c16ade38de048a441";
 
-PictureLoaderWorker::PictureLoaderWorker()
+PictureLoaderWorker::PictureLoaderWorker() : QObject(nullptr), picDownload(SettingsCache::instance().getPicDownload())
-    : QObject(nullptr), picsPath(SettingsCache::instance().getPicsPath()),
-      customPicsPath(SettingsCache::instance().getCustomPicsPath()),
-      picDownload(SettingsCache::instance().getPicDownload()), downloadRunning(false), loadQueueRunning(false),
-      overrideAllCardArtWithPersonalPreference(SettingsCache::instance().getOverrideAllCardArtWithPersonalPreference())
 {
-    connect(&SettingsCache::instance(), SIGNAL(picsPathChanged()), this, SLOT(picsPathChanged()));
-    connect(&SettingsCache::instance(), SIGNAL(picDownloadChanged()), this, SLOT(picDownloadChanged()));
-    connect(&SettingsCache::instance(), &SettingsCache::overrideAllCardArtWithPersonalPreferenceChanged, this,
-            &PictureLoaderWorker::setOverrideAllCardArtWithPersonalPreference);
-
     networkManager = new QNetworkAccessManager(this);
     // We need a timeout to ensure requests don't hang indefinitely in case of
     // cache corruption, see related Qt bug: https://bugreports.qt.io/browse/QTBUG-111397
@@ -213,24 +204,6 @@ void PictureLoaderWorker::cleanStaleEntries()
     }
 }
 
-void PictureLoaderWorker::picDownloadChanged()
-{
-    QMutexLocker locker(&mutex);
-    picDownload = SettingsCache::instance().getPicDownload();
-}
-
-void PictureLoaderWorker::picsPathChanged()
-{
-    QMutexLocker locker(&mutex);
-    picsPath = SettingsCache::instance().getPicsPath();
-    customPicsPath = SettingsCache::instance().getCustomPicsPath();
-}
-
-void PictureLoaderWorker::setOverrideAllCardArtWithPersonalPreference(bool _overrideAllCardArtWithPersonalPreference)
-{
-    overrideAllCardArtWithPersonalPreference = _overrideAllCardArtWithPersonalPreference;
-}
-
 void PictureLoaderWorker::clearNetworkCache()
 {
     networkManager->cache()->clear();

cockatrice/src/client/ui/picture_loader/picture_loader_worker.h

@@ -29,9 +29,9 @@ class PictureLoaderWorker : public QObject
 public:
     explicit PictureLoaderWorker();
     ~PictureLoaderWorker() override;
-    void queueRequest(const QUrl &url, PictureLoaderWorkerWork *worker);
 
-    void enqueueImageLoad(const CardInfoPtr &card);
+    void enqueueImageLoad(const CardInfoPtr &card);                      // Starts a thread for the image to be loaded
+    void queueRequest(const QUrl &url, PictureLoaderWorkerWork *worker); // Queues network requests for load threads
     void clearNetworkCache();
 
 public slots:
@@ -43,22 +43,15 @@ private:
     static QStringList md5Blacklist;
 
     QThread *pictureLoaderThread;
-    QString picsPath, customPicsPath;
-    QList<PictureToLoad> loadQueue;
-    QMutex mutex;
     QNetworkAccessManager *networkManager;
     QNetworkDiskCache *cache;
     QHash<QUrl, QPair<QUrl, QDateTime>> redirectCache; // Stores redirect and timestamp
     QString cacheFilePath;                             // Path to persistent storage
     static constexpr int CacheTTLInDays = 30;          // TODO: Make user configurable
-    QList<PictureToLoad> cardsToDownload;
-    PictureToLoad cardBeingLoaded;
     PictureToLoad cardBeingDownloaded;
-    bool picDownload, downloadRunning, loadQueueRunning;
+    bool picDownload;
     QQueue<QPair<QUrl, PictureLoaderWorkerWork *>> requestLoadQueue;
-    QList<QPair<QUrl, PictureLoaderWorkerWork *>> requestQueue;
     QTimer requestTimer; // Timer for processing delayed requests
-    bool overrideAllCardArtWithPersonalPreference;
 
     void cacheRedirect(const QUrl &originalUrl, const QUrl &redirectUrl);
     QUrl getCachedRedirect(const QUrl &originalUrl) const;
@@ -66,13 +59,7 @@ private:
     void saveRedirectCache() const;
     void cleanStaleEntries();
 
-private slots:
-    void picDownloadChanged();
-    void picsPathChanged();
-    void setOverrideAllCardArtWithPersonalPreference(bool _overrideAllCardArtWithPersonalPreference);
-
 signals:
-    void startLoadQueue();
     void imageLoaded(CardInfoPtr card, const QImage &image);
     void imageLoadQueued(const QUrl &url, PictureLoaderWorkerWork *worker);
     void imageLoadSuccessful(const QUrl &url, PictureLoaderWorkerWork *worker);

cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.cpp

@@ -16,16 +16,27 @@
 QStringList PictureLoaderWorkerWork::md5Blacklist = QStringList() << "db0c48db407a907c16ade38de048a441";
 
 PictureLoaderWorkerWork::PictureLoaderWorkerWork(PictureLoaderWorker *_worker, const CardInfoPtr &toLoad)
-    : QThread(nullptr), worker(_worker), cardToDownload(toLoad)
+    : QThread(nullptr), worker(_worker), cardToDownload(toLoad), picsPath(SettingsCache::instance().getPicsPath()),
+      customPicsPath(SettingsCache::instance().getCustomPicsPath()),
+      overrideAllCardArtWithPersonalPreference(SettingsCache::instance().getOverrideAllCardArtWithPersonalPreference())
+
 {
+    // Hook up signals to the orchestrator
     connect(this, &PictureLoaderWorkerWork::requestImageDownload, worker, &PictureLoaderWorker::queueRequest,
             Qt::QueuedConnection);
     connect(this, &PictureLoaderWorkerWork::imageLoaded, worker, &PictureLoaderWorker::imageLoadedSuccessfully,
             Qt::QueuedConnection);
+
+    // Hook up signals to settings
+    connect(&SettingsCache::instance(), SIGNAL(picsPathChanged()), this, SLOT(picsPathChanged()));
+    connect(&SettingsCache::instance(), SIGNAL(picDownloadChanged()), this, SLOT(picDownloadChanged()));
+    connect(&SettingsCache::instance(), &SettingsCache::overrideAllCardArtWithPersonalPreferenceChanged, this,
+            &PictureLoaderWorkerWork::setOverrideAllCardArtWithPersonalPreference);
+
     pictureLoaderThread = new QThread;
     pictureLoaderThread->start(QThread::LowPriority);
     moveToThread(pictureLoaderThread);
-    startNextPicDownload();
+    startWork();
 }
 
 PictureLoaderWorkerWork::~PictureLoaderWorkerWork()
@@ -33,32 +44,66 @@ PictureLoaderWorkerWork::~PictureLoaderWorkerWork()
     pictureLoaderThread->deleteLater();
 }
 
-bool PictureLoaderWorkerWork::cardImageExistsOnDisk(QString &setName, QString &correctedCardname)
+void PictureLoaderWorkerWork::startWork()
+{
+    QString setName = cardToDownload.getSetName();
+    QString cardName = cardToDownload.getCard()->getName();
+    QString correctedCardName = cardToDownload.getCard()->getCorrectedName();
+
+    qCDebug(PictureLoaderWorkerLog).nospace()
+        << "[card: " << cardName << " set: " << setName << "]: Trying to load picture";
+
+    // FIXME: This is a hack so that to keep old Cockatrice behavior
+    // (ignoring provider ID) when the "override all card art with personal
+    // preference" is set.
+    //
+    // Figure out a proper way to integrate the two systems at some point.
+    //
+    // Note: need to go through a member for
+    // overrideAllCardArtWithPersonalPreference as reading from the
+    // SettingsCache instance from the PictureLoaderWorker thread could
+    // cause race conditions.
+    //
+    // XXX: Reading from the CardDatabaseManager instance from the
+    // PictureLoaderWorker thread might not be safe either
+    bool searchCustomPics = overrideAllCardArtWithPersonalPreference ||
+                            CardDatabaseManager::getInstance()->isProviderIdForPreferredPrinting(
+                                cardName, cardToDownload.getCard()->getPixmapCacheKey());
+    if (!(searchCustomPics && cardImageExistsOnDisk(setName, correctedCardName, searchCustomPics))) {
+        startNextPicDownload();
+    }
+}
+
+bool PictureLoaderWorkerWork::cardImageExistsOnDisk(const QString &setName,
+                                                    const QString &correctedCardName,
+                                                    const bool searchCustomPics)
 {
     QImage image;
     QImageReader imgReader;
     imgReader.setDecideFormatFromContent(true);
     QList<QString> picsPaths = QList<QString>();
-    QDirIterator it(SettingsCache::instance().getCustomPicsPath(),
-                    QDirIterator::Subdirectories | QDirIterator::FollowSymlinks);
 
-    // Recursively check all subdirectories of the CUSTOM folder
+    if (searchCustomPics) {
-    while (it.hasNext()) {
+        QDirIterator it(customPicsPath, QDirIterator::Subdirectories | QDirIterator::FollowSymlinks);
-        QString thisPath(it.next());
-        QFileInfo thisFileInfo(thisPath);
 
-        if (thisFileInfo.isFile() &&
+        // Recursively check all subdirectories of the CUSTOM folder
-            (thisFileInfo.fileName() == correctedCardname || thisFileInfo.completeBaseName() == correctedCardname ||
+        while (it.hasNext()) {
-             thisFileInfo.baseName() == correctedCardname)) {
+            QString thisPath(it.next());
-            picsPaths << thisPath; // Card found in the CUSTOM directory, somewhere
+            QFileInfo thisFileInfo(thisPath);
+
+            if (thisFileInfo.isFile() &&
+                (thisFileInfo.fileName() == correctedCardName || thisFileInfo.completeBaseName() == correctedCardName ||
+                 thisFileInfo.baseName() == correctedCardName)) {
+                picsPaths << thisPath; // Card found in the CUSTOM directory, somewhere
+            }
         }
     }
 
     if (!setName.isEmpty()) {
-        picsPaths << SettingsCache::instance().getPicsPath() + "/" + setName + "/" + correctedCardname
+        picsPaths << picsPath + "/" + setName + "/" + correctedCardName
                   // We no longer store downloaded images there, but don't just ignore
                   // stuff that old versions have put there.
-                  << SettingsCache::instance().getPicsPath() + "/downloadedPics/" + setName + "/" + correctedCardname;
+                  << picsPath + "/downloadedPics/" + setName + "/" + correctedCardName;
     }
 
     // Iterates through the list of paths, searching for images with the desired
@@ -67,27 +112,28 @@ bool PictureLoaderWorkerWork::cardImageExistsOnDisk(QString &setName, QString &c
     for (const auto &_picsPath : picsPaths) {
         imgReader.setFileName(_picsPath);
         if (imgReader.read(&image)) {
-            qCDebug(PictureLoaderWorkerWorkLog).nospace()
+            qCDebug(PictureLoaderWorkerLog).nospace()
-                << "PictureLoader: [card: " << correctedCardname << " set: " << setName << "]: Picture found on disk.";
+                << "[card: " << correctedCardName << " set: " << setName << "]: Picture found on disk.";
             imageLoaded(cardToDownload.getCard(), image);
             return true;
         }
         imgReader.setFileName(_picsPath + ".full");
         if (imgReader.read(&image)) {
-            qCDebug(PictureLoaderWorkerWorkLog).nospace() << "PictureLoader: [card: " << correctedCardname
+            qCDebug(PictureLoaderWorkerLog).nospace()
-                                                          << " set: " << setName << "]: Picture.full found on disk.";
+                << "[card: " << correctedCardName << " set: " << setName << "]: Picture.full found on disk.";
             imageLoaded(cardToDownload.getCard(), image);
             return true;
         }
         imgReader.setFileName(_picsPath + ".xlhq");
         if (imgReader.read(&image)) {
-            qCDebug(PictureLoaderWorkerWorkLog).nospace() << "PictureLoader: [card: " << correctedCardname
+            qCDebug(PictureLoaderWorkerLog).nospace()
-                                                          << " set: " << setName << "]: Picture.xlhq found on disk.";
+                << "[card: " << correctedCardName << " set: " << setName << "]: Picture.xlhq found on disk.";
             imageLoaded(cardToDownload.getCard(), image);
             return true;
         }
     }
-
+    qCDebug(PictureLoaderWorkerLog).nospace()
+        << "[card: " << correctedCardName << " set: " << setName << "]: Picture NOT found on disk.";
     return false;
 }
 
@@ -229,6 +275,23 @@ void PictureLoaderWorkerWork::picDownloadFinished(QNetworkReply *reply)
     reply->deleteLater();
 }
 
+void PictureLoaderWorkerWork::picDownloadChanged()
+{
+    picDownload = SettingsCache::instance().getPicDownload();
+}
+
+void PictureLoaderWorkerWork::picsPathChanged()
+{
+    picsPath = SettingsCache::instance().getPicsPath();
+    customPicsPath = SettingsCache::instance().getCustomPicsPath();
+}
+
+void PictureLoaderWorkerWork::setOverrideAllCardArtWithPersonalPreference(
+    bool _overrideAllCardArtWithPersonalPreference)
+{
+    overrideAllCardArtWithPersonalPreference = _overrideAllCardArtWithPersonalPreference;
+}
+
 bool PictureLoaderWorkerWork::imageIsBlackListed(const QByteArray &picData)
 {
     QString md5sum = QCryptographicHash::hash(picData, QCryptographicHash::Md5).toHex();

cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.h

@@ -26,21 +26,31 @@ class PictureLoaderWorkerWork : public QThread
 public:
     explicit PictureLoaderWorkerWork(PictureLoaderWorker *worker, const CardInfoPtr &toLoad);
     ~PictureLoaderWorkerWork() override;
+    void startWork();
     PictureLoaderWorker *worker;
     PictureToLoad cardToDownload;
+
 public slots:
     void picDownloadFinished(QNetworkReply *reply);
     void picDownloadFailed();
 
 private:
+    QString picsPath, customPicsPath;
+    bool overrideAllCardArtWithPersonalPreference;
     static QStringList md5Blacklist;
     QThread *pictureLoaderThread;
     QNetworkAccessManager *networkManager;
     bool picDownload, downloadRunning, loadQueueRunning;
+
     void startNextPicDownload();
-    bool cardImageExistsOnDisk(QString &setName, QString &correctedCardName);
+    bool cardImageExistsOnDisk(const QString &setName, const QString &correctedCardName, bool searchCustomPics);
     bool imageIsBlackListed(const QByteArray &);
 
+private slots:
+    void picDownloadChanged();
+    void picsPathChanged();
+    void setOverrideAllCardArtWithPersonalPreference(bool _overrideAllCardArtWithPersonalPreference);
+
 signals:
     void startLoadQueue();
     void imageLoaded(CardInfoPtr card, const QImage &image);

cockatrice/src/client/ui/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.h

@@ -6,6 +6,8 @@
 
 #include <QWidget>
 
+inline bool confirmOverwriteIfExists(QWidget *parent, const QString &filePath);
+
 class DeckPreviewWidget;
 class DeckPreviewDeckTagsDisplayWidget : public QWidget
 {