From e70533a9bc55864a6cdfcecd790e84c9e2aaf3f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Fri, 15 Nov 2024 15:39:51 +0100 Subject: [PATCH] Address pull request comments (nullptr checks and additional comments, mostly.) --- cockatrice/src/client/ui/picture_loader.cpp | 108 ++++++++++---------- common/decklist.cpp | 6 +- 2 files changed, 57 insertions(+), 57 deletions(-) diff --git a/cockatrice/src/client/ui/picture_loader.cpp b/cockatrice/src/client/ui/picture_loader.cpp index 2953d760d..e30e5fc58 100644 --- a/cockatrice/src/client/ui/picture_loader.cpp +++ b/cockatrice/src/client/ui/picture_loader.cpp @@ -175,8 +175,8 @@ void PictureLoaderWorker::processLoadQueue() QString cardName = cardBeingLoaded.getCard()->getName(); QString correctedCardName = cardBeingLoaded.getCard()->getCorrectedName(); - //qDebug().nospace() << "PictureLoader: [card: " << cardName << " set: " << setName - // << "]: Trying to load picture"; + qDebug().nospace() << "PictureLoader: [card: " << cardName << " set: " << setName + << "]: Trying to load picture"; if (CardDatabaseManager::getInstance()->isUuidForPreferredPrinting( cardName, cardBeingLoaded.getCard()->getPixmapCacheKey())) { @@ -185,8 +185,8 @@ void PictureLoaderWorker::processLoadQueue() } } - //qDebug().nospace() << "PictureLoader: [card: " << cardName << " set: " << setName - // << "]: No custom picture, trying to download"; + qDebug().nospace() << "PictureLoader: [card: " << cardName << " set: " << setName + << "]: No custom picture, trying to download"; cardsToDownload.append(cardBeingLoaded); cardBeingLoaded.clear(); if (!downloadRunning) { @@ -228,22 +228,22 @@ bool PictureLoaderWorker::cardImageExistsOnDisk(QString &setName, QString &corre for (const auto &_picsPath : picsPaths) { imgReader.setFileName(_picsPath); if (imgReader.read(&image)) { - //qDebug().nospace() << "PictureLoader: [card: " << correctedCardname << " set: " << setName - // << "]: Picture found on disk."; + qDebug().nospace() << "PictureLoader: [card: " << correctedCardname << " set: " << setName + << "]: Picture found on disk."; imageLoaded(cardBeingLoaded.getCard(), image); return true; } imgReader.setFileName(_picsPath + ".full"); if (imgReader.read(&image)) { - //qDebug().nospace() << "PictureLoader: [card: " << correctedCardname << " set: " << setName - // << "]: Picture.full found on disk."; + qDebug().nospace() << "PictureLoader: [card: " << correctedCardname << " set: " << setName + << "]: Picture.full found on disk."; imageLoaded(cardBeingLoaded.getCard(), image); return true; } imgReader.setFileName(_picsPath + ".xlhq"); if (imgReader.read(&image)) { - //qDebug().nospace() << "PictureLoader: [card: " << correctedCardname << " set: " << setName - // << "]: Picture.xlhq found on disk."; + qDebug().nospace() << "PictureLoader: [card: " << correctedCardname << " set: " << setName + << "]: Picture.xlhq found on disk."; imageLoaded(cardBeingLoaded.getCard(), image); return true; } @@ -259,8 +259,6 @@ static int parse(const QString &urlTemplate, std::function getProperty, QMap &transformMap) { - (void) cardName; - (void) setName; static const QRegularExpression rxFillWith("^(.+)_fill_with_(.+)$"); static const QRegularExpression rxSubStr("^(.+)_substr_(\\d+)_(\\d+)$"); @@ -291,18 +289,18 @@ static int parse(const QString &urlTemplate, } QString propertyValue = getProperty(cardPropertyName); if (propertyValue.isEmpty()) { - //qDebug().nospace() << "PictureLoader: [card: " << cardName << " set: " << setName << "]: Requested " - // << propType << "property (" << cardPropertyName << ") for Url template (" << urlTemplate - // << ") is not available"; + qDebug().nospace() << "PictureLoader: [card: " << cardName << " set: " << setName << "]: Requested " + << propType << "property (" << cardPropertyName << ") for Url template (" << urlTemplate + << ") is not available"; return 1; } else { int propLength = propertyValue.length(); if (subStrLen > 0) { if (subStrPos + subStrLen > propLength) { - //qDebug().nospace() << "PictureLoader: [card: " << cardName << " set: " << setName << "]: Requested " - // << propType << " property (" << cardPropertyName << ") for Url template (" - // << urlTemplate << ") is smaller than substr specification (" << subStrPos - // << " + " << subStrLen << " > " << propLength << ")"; + qDebug().nospace() << "PictureLoader: [card: " << cardName << " set: " << setName << "]: Requested " + << propType << " property (" << cardPropertyName << ") for Url template (" + << urlTemplate << ") is smaller than substr specification (" << subStrPos + << " + " << subStrLen << " > " << propLength << ")"; return 1; } else { propertyValue = propertyValue.mid(subStrPos, subStrLen); @@ -313,9 +311,9 @@ static int parse(const QString &urlTemplate, if (!fillWith.isEmpty()) { int fillLength = fillWith.length(); if (fillLength < propLength) { - //qDebug().nospace() << "PictureLoader: [card: " << cardName << " set: " << setName << "]: Requested " - // << propType << " property (" << cardPropertyName << ") for Url template (" - // << urlTemplate << ") is longer than fill specification (" << fillWith << ")"; + qDebug().nospace() << "PictureLoader: [card: " << cardName << " set: " << setName << "]: Requested " + << propType << " property (" << cardPropertyName << ") for Url template (" + << urlTemplate << ") is longer than fill specification (" << fillWith << ")"; return 1; } else { @@ -382,9 +380,9 @@ QString PictureToLoad::transformUrl(const QString &urlTemplate) const * populated in this card, so it should return an empty string, * indicating an invalid Url. */ - //qDebug().nospace() << "PictureLoader: [card: " << cardName << " set: " << setName - // << "]: Requested information (" << prop << ") for Url template (" << urlTemplate - // << ") is not available"; + qDebug().nospace() << "PictureLoader: [card: " << cardName << " set: " << setName + << "]: Requested information (" << prop << ") for Url template (" << urlTemplate + << ") is not available"; return QString(); } } @@ -412,9 +410,9 @@ void PictureLoaderWorker::startNextPicDownload() picDownloadFailed(); } else { QUrl url(picUrl); - //qDebug().nospace() << "PictureLoader: [card: " << cardBeingDownloaded.getCard()->getCorrectedName() - // << " set: " << cardBeingDownloaded.getSetName() << "]: Trying to fetch picture from url " - // << url.toDisplayString(); + qDebug().nospace() << "PictureLoader: [card: " << cardBeingDownloaded.getCard()->getCorrectedName() + << " set: " << cardBeingDownloaded.getSetName() << "]: Trying to fetch picture from url " + << url.toDisplayString(); makeRequest(url); } } @@ -430,10 +428,10 @@ void PictureLoaderWorker::picDownloadFailed() loadQueue.prepend(cardBeingDownloaded); mutex.unlock(); } else { - //qDebug().nospace() << "PictureLoader: [card: " << cardBeingDownloaded.getCard()->getCorrectedName() - // << " set: " << cardBeingDownloaded.getSetName() << "]: Picture NOT found, " - // << (picDownload ? "download failed" : "downloads disabled") - // << ", no more url combinations to try: BAILING OUT"; + qDebug().nospace() << "PictureLoader: [card: " << cardBeingDownloaded.getCard()->getCorrectedName() + << " set: " << cardBeingDownloaded.getSetName() << "]: Picture NOT found, " + << (picDownload ? "download failed" : "downloads disabled") + << ", no more url combinations to try: BAILING OUT"; imageLoaded(cardBeingDownloaded.getCard(), QImage()); cardBeingDownloaded.clear(); } @@ -461,19 +459,19 @@ void PictureLoaderWorker::picDownloadFinished(QNetworkReply *reply) if (reply->error()) { if (isFromCache) { - //qDebug().nospace() << "PictureLoader: [card: " << cardBeingDownloaded.getCard()->getName() - // << " set: " << cardBeingDownloaded.getSetName() - // << "]: Removing corrupted cache file for url " << reply->url().toDisplayString() - // << " and retrying (" << reply->errorString() << ")"; + qDebug().nospace() << "PictureLoader: [card: " << cardBeingDownloaded.getCard()->getName() + << " set: " << cardBeingDownloaded.getSetName() + << "]: Removing corrupted cache file for url " << reply->url().toDisplayString() + << " and retrying (" << reply->errorString() << ")"; networkManager->cache()->remove(reply->url()); makeRequest(reply->url()); } else { - //qDebug().nospace() << "PictureLoader: [card: " << cardBeingDownloaded.getCard()->getName() - // << " set: " << cardBeingDownloaded.getSetName() - // << "]: " << (picDownload ? "Download" : "Cache search") << " failed for url " - // << reply->url().toDisplayString() << " (" << reply->errorString() << ")"; + qDebug().nospace() << "PictureLoader: [card: " << cardBeingDownloaded.getCard()->getName() + << " set: " << cardBeingDownloaded.getSetName() + << "]: " << (picDownload ? "Download" : "Cache search") << " failed for url " + << reply->url().toDisplayString() << " (" << reply->errorString() << ")"; picDownloadFailed(); startNextPicDownload(); @@ -488,9 +486,9 @@ void PictureLoaderWorker::picDownloadFinished(QNetworkReply *reply) if (statusCode == 301 || statusCode == 302 || statusCode == 303 || statusCode == 305 || statusCode == 307 || statusCode == 308) { QUrl redirectUrl = reply->header(QNetworkRequest::LocationHeader).toUrl(); - //qDebug().nospace() << "PictureLoader: [card: " << cardBeingDownloaded.getCard()->getName() - // << " set: " << cardBeingDownloaded.getSetName() << "]: following " - // << (isFromCache ? "cached redirect" : "redirect") << " to " << redirectUrl.toDisplayString(); + qDebug().nospace() << "PictureLoader: [card: " << cardBeingDownloaded.getCard()->getName() + << " set: " << cardBeingDownloaded.getSetName() << "]: following " + << (isFromCache ? "cached redirect" : "redirect") << " to " << redirectUrl.toDisplayString(); makeRequest(redirectUrl); reply->deleteLater(); return; @@ -500,9 +498,9 @@ void PictureLoaderWorker::picDownloadFinished(QNetworkReply *reply) const QByteArray &picData = reply->peek(reply->size()); if (imageIsBlackListed(picData)) { - //qDebug().nospace() << "PictureLoader: [card: " << cardBeingDownloaded.getCard()->getName() - // << " set: " << cardBeingDownloaded.getSetName() - // << "]: Picture found, but blacklisted, will consider it as not found"; + qDebug().nospace() << "PictureLoader: [card: " << cardBeingDownloaded.getCard()->getName() + << " set: " << cardBeingDownloaded.getSetName() + << "]: Picture found, but blacklisted, will consider it as not found"; picDownloadFailed(); reply->deleteLater(); @@ -535,19 +533,19 @@ void PictureLoaderWorker::picDownloadFinished(QNetworkReply *reply) imageLoaded(cardBeingDownloaded.getCard(), testImage); logSuccessMessage = true; } else { - //qDebug().nospace() << "PictureLoader: [card: " << cardBeingDownloaded.getCard()->getName() - // << " set: " << cardBeingDownloaded.getSetName() << "]: Possible " - // << (isFromCache ? "cached" : "downloaded") << " picture at " - // << reply->url().toDisplayString() << " could not be loaded: " << reply->errorString(); + qDebug().nospace() << "PictureLoader: [card: " << cardBeingDownloaded.getCard()->getName() + << " set: " << cardBeingDownloaded.getSetName() << "]: Possible " + << (isFromCache ? "cached" : "downloaded") << " picture at " + << reply->url().toDisplayString() << " could not be loaded: " << reply->errorString(); picDownloadFailed(); } if (logSuccessMessage) { - //qDebug().nospace() << "PictureLoader: [card: " << cardBeingDownloaded.getCard()->getName() - // << " set: " << cardBeingDownloaded.getSetName() << "]: Image successfully " - // << (isFromCache ? "loaded from cached" : "downloaded from") << " url " - // << reply->url().toDisplayString(); + qDebug().nospace() << "PictureLoader: [card: " << cardBeingDownloaded.getCard()->getName() + << " set: " << cardBeingDownloaded.getSetName() << "]: Image successfully " + << (isFromCache ? "loaded from cached" : "downloaded from") << " url " + << reply->url().toDisplayString(); } reply->deleteLater(); @@ -614,7 +612,7 @@ void PictureLoader::getCardBackPixmap(QPixmap &pixmap, QSize size) { QString backCacheKey = "_trice_card_back_" + QString::number(size.width()) + QString::number(size.height()); if (!QPixmapCache::find(backCacheKey, &pixmap)) { - //qDebug() << "PictureLoader: cache fail for" << backCacheKey; + qDebug() << "PictureLoader: cache fail for" << backCacheKey; pixmap = QPixmap("theme:cardback").scaled(size, Qt::KeepAspectRatio, Qt::SmoothTransformation); QPixmapCache::insert(backCacheKey, pixmap); } diff --git a/common/decklist.cpp b/common/decklist.cpp index baddbf0f4..0d757c831 100644 --- a/common/decklist.cpp +++ b/common/decklist.cpp @@ -160,8 +160,10 @@ AbstractDecklistNode *InnerDecklistNode::findChild(const QString &_name) AbstractDecklistNode *InnerDecklistNode::findCardChildByNameAndUUID(const QString &_name, const QString &_uuid) { for (int i = 0; i < size(); i++) { - if (at(i)->getName() == _name && at(i)->getCardUuid() == _uuid) { - return at(i); + if (at(i) != nullptr) { + if (at(i)->getName() == _name && at(i)->getCardUuid() == _uuid) { + return at(i); + } } } return nullptr;