diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp index 5ab097bc4..fab651d72 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp @@ -24,6 +24,7 @@ static constexpr qint64 QUOTA_RESET_INTERVAL_MS = 1000; ///< Interval at which t CardPictureLoaderWorker::CardPictureLoaderWorker() : QObject(nullptr), picDownload(SettingsCache::instance().downloads().getPicDownload()), + requestQuota(MAX_REQUESTS_PER_SEC), hostRequestLimits(SettingsCache::instance().downloads().getHostRequestLimits()) { networkManager = new QNetworkAccessManager(this); @@ -112,14 +113,6 @@ QNetworkReply *CardPictureLoaderWorker::makeRequest(const QUrl &url, CardPicture QUrl cachedRedirect = getCachedRedirect(url); if (!cachedRedirect.isEmpty()) { emit imageRequestSucceeded(url); - // The redirect target is a different host, which may itself be in 429 backoff; hand the - // entry back to its worker so it waits the backoff out instead of dispatching straight - // onto the backed-off host. - if (CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(cachedRedirect.host(), - QDateTime::currentDateTime())) { - worker->scheduleDeferredRetry(); - return nullptr; - } return makeRequest(cachedRedirect, worker); } @@ -140,43 +133,31 @@ QNetworkReply *CardPictureLoaderWorker::makeRequest(const QUrl &url, CardPicture QNetworkReply *reply = networkManager->get(req); - // Track in-flight replies per host so the unlocked fast path can bound how many requests it - // issues at once, instead of creating replies that time out before Qt opens a connection. - const QString host = url.host(); - hostInFlight.insert(host, hostInFlight.value(host) + 1); - // Connect reply handling - connect(reply, &QNetworkReply::finished, worker, [this, reply, worker, host] { - hostInFlight.insert(host, qMax(0, hostInFlight.value(host) - 1)); - worker->handleNetworkReply(reply); - }); + connect(reply, &QNetworkReply::finished, worker, [reply, worker] { worker->handleNetworkReply(reply); }); return reply; } void CardPictureLoaderWorker::resetRequestQuota() { - // Allowances are seeded lazily per host in processSingleRequest() when a request is first - // looked at in a new second, so a host that enters the queue mid-second now gets its reduced - // per-host allowance instead of falling through to the full per-second default. + requestQuota = MAX_REQUESTS_PER_SEC; + // Allowances are seeded per host on demand in processSingleRequest(), so a + // rate-limited host never gets a fresh full quota mid-second. hostQuotaRemaining.clear(); QDateTime now = QDateTime::currentDateTime(); for (auto it = hostRequestQuota.begin(); it != hostRequestQuota.end();) { + // An unlocked host has no per-host allowance; drop any stale entry instead of + // recovering it towards the UNLIMITED_HOST_QUOTA sentinel, which would poison it. + if (hostAllowanceCeiling(it.key()) == DownloadSettings::UNLIMITED_HOST_QUOTA) { + it = hostRequestQuota.erase(it); + continue; + } if (!hostLast429.contains(it.key()) || now.msecsTo(hostLast429.value(it.key())) < -QUOTA_RECOVER_MS) { - if (hostAllowanceCeiling(it.key()) == DownloadSettings::UNLIMITED_HOST_QUOTA) { - // A developer-unlocked host that fell back after a 429 recovers towards the default - // allowance; once it gets there it becomes unlocked (fast-path) again. - if (it.value() + 1 >= DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT) { - it = hostRequestQuota.erase(it); - continue; - } - it.value() += 1; - } else { - // Recover towards the host's effective allowance ceiling, which may be - // lowered by the user's per-host request limits. - it.value() = qMin(hostAllowanceCeiling(it.key()), it.value() + 1); - } + // Recover towards the host's effective allowance ceiling, which may be + // lowered by the user's per-host request limits. + it.value() = qMin(hostAllowanceCeiling(it.key()), it.value() + 1); } ++it; } @@ -186,76 +167,49 @@ void CardPictureLoaderWorker::resetRequestQuota() void CardPictureLoaderWorker::processQueuedRequests() { - Q_ASSERT(thread() == QThread::currentThread()); - if (requestLoadQueue.isEmpty()) { dispatchTimer.stop(); - requestTimer.stop(); return; } // Start lazily from the worker's own thread: QTimer must be started in the thread it lives in. if (!requestTimer.isActive()) { requestTimer.start(); } - // Restarting an active timer would reset the pacing countdown, so a burst of enqueues could - // keep starving the dispatcher; only start it when it has actually stopped. - if (!dispatchTimer.isActive()) { - dispatchTimer.start(); - } + dispatchTimer.start(); } void CardPictureLoaderWorker::dispatchQueuedRequest() { if (requestLoadQueue.isEmpty()) { - // All queued requests have been dispatched; stop the pacing and quota-reset timers. dispatchTimer.stop(); - requestTimer.stop(); return; } + // Unlocked hosts (developer cap UNLIMITED_HOST_QUOTA) skip the dispatch pacing and the + // global per-second quota: dispatch every queued request for them back-to-back, bounded + // only by their 429 backoff window and Qt's per-host connection pool. QDateTime now = QDateTime::currentDateTime(); - bool dispatched = false; - // Set while an unlocked host still has queued work blocked only by the in-flight cap; the - // timer must keep running so it gets another try as soon as a slot frees. A host blocked by - // its 429 backoff instead waits for the next quota-reset tick to restart the dispatcher. - bool unlockedCapped = false; - - // Unlocked hosts (developer cap UNLIMITED_HOST_QUOTA) skip the pacing and the per-host - // allowance: dispatch their queued requests back-to-back, bounded by their 429 backoff and the - // per-host in-flight cap so a large burst can't queue replies that time out before Qt opens a - // connection for them. for (int i = 0; i < requestLoadQueue.size();) { const auto &request = requestLoadQueue.at(i); const QString host = request.first.host(); - if (isUnlockedHost(host)) { - if (CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) { - ++i; - continue; - } - if (hostInFlight.value(host) < MAX_IN_FLIGHT_PER_HOST) { - makeRequest(request.first, request.second); - requestLoadQueue.removeAt(i); - dispatched = true; - continue; - } - unlockedCapped = true; + if (hostAllowanceCeiling(host) == DownloadSettings::UNLIMITED_HOST_QUOTA && + !CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) { + makeRequest(request.first, request.second); + requestLoadQueue.removeAt(i); + } else { + ++i; } - ++i; } - if (requestLoadQueue.isEmpty()) { + if (requestLoadQueue.isEmpty() || requestQuota <= 0) { dispatchTimer.stop(); - requestTimer.stop(); return; } if (processSingleRequest()) { - dispatched = true; - } - - // Keep the timer running while there is progress to make or unlocked work waiting on a free - // in-flight slot; otherwise no host has allowance left this second, so wait for the quota reset. - if (!dispatched && !unlockedCapped) { + --requestQuota; + } else { + // No queued host currently has allowance left in this second; wait for the quota reset. dispatchTimer.stop(); } } @@ -266,28 +220,14 @@ bool CardPictureLoaderWorker::processSingleRequest() for (int i = 0; i < requestLoadQueue.size(); ++i) { const auto &request = requestLoadQueue.at(i); const QString host = request.first.host(); - // Don't dispatch requests to a host that is currently in its 429 backoff; hand the entry - // back to its worker so it can wait the backoff out or fall through to another source, - // instead of leaving it parked in the queue with no reply pending. + // Don't dispatch requests to a host that is currently in its 429 backoff. if (CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) { - requestLoadQueue.removeAt(i); - request.second->startNextPicDownload(); - return true; - } - // Unlocked hosts are handled by dispatchQueuedRequest's fast path, bounded by the in-flight - // cap; they must not fall through to the per-host allowance arithmetic below. - if (isUnlockedHost(host)) { continue; } - int ceiling = hostAllowanceCeiling(host); - if (ceiling == DownloadSettings::UNLIMITED_HOST_QUOTA) { - // A 429 dropped this unlocked host out of the fast path and installed a concrete - // allowance; pace it against that allowance until the recovery loop unlocks it again. - ceiling = hostRequestQuota.value(host, DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT); - } - // Seed the allowance lazily so a host that enters the queue mid-second gets its reduced - // per-host allowance, clamped against the ceiling so a lowered user cap applies from this - // second onward. + const int ceiling = hostAllowanceCeiling(host); + // Seed the allowance only now, so a host that was rate limited last second + // doesn't get a fresh full quota the moment it is queried mid-second. Clamp + // against the ceiling so a lowered user cap applies from this second onward. if (!hostQuotaRemaining.contains(host)) { hostQuotaRemaining.insert(host, qMin(ceiling, hostRequestQuota.value(host, ceiling))); } @@ -312,20 +252,15 @@ int CardPictureLoaderWorker::hostAllowanceCeiling(const QString &host) const return SettingsCache::instance().downloads().clampHostRequestLimit(host, requested); } -bool CardPictureLoaderWorker::isUnlockedHost(const QString &host) const -{ - return hostAllowanceCeiling(host) == DownloadSettings::UNLIMITED_HOST_QUOTA && !hostRequestQuota.contains(host); -} - void CardPictureLoaderWorker::onHostRateLimited(const QString &host) { const int ceiling = hostAllowanceCeiling(host); - // An unlocked host has no per-host allowance to halve. Install one instead so it drops out of - // the unlocked fast path and is paced like a throttled host; the recovery loop in - // resetRequestQuota() then walks it back up and unlocks it again. - const int base = - ceiling == DownloadSettings::UNLIMITED_HOST_QUOTA ? DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT : ceiling; - hostRequestQuota.insert(host, qMax(MIN_HOST_QUOTA, hostRequestQuota.value(host, base) / 2)); + if (ceiling == DownloadSettings::UNLIMITED_HOST_QUOTA) { + // Unlocked hosts have no per-host allowance to halve; the shared backoff + // window tracked by the rate limiter still paces them. + return; + } + hostRequestQuota.insert(host, qMax(MIN_HOST_QUOTA, hostRequestQuota.value(host, ceiling) / 2)); hostLast429.insert(host, QDateTime::currentDateTime()); } diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h index 1f4bebb53..70bc36419 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h @@ -86,7 +86,7 @@ public slots: */ QNetworkReply *makeRequest(const QUrl &url, CardPictureLoaderWorkerWork *workThread); - /** @brief Starts the pacing timers if there is queued work, stops them when the queue is empty. */ + /** @brief Processes all queued requests respecting the request quota. */ void processQueuedRequests(); /** @brief Chooses a request from the queue and starts it, respecting the quota and pacing. */ @@ -121,16 +121,13 @@ private: bool picDownload; ///< Whether downloading images from network is enabled QQueue> requestLoadQueue; ///< Queue of pending network requests + int requestQuota; ///< Remaining requests allowed per second QTimer requestTimer; ///< Timer to reset the request quota QTimer dispatchTimer; ///< Timer pacing individual network requests QHash hostRequestQuota; ///< Sustained per-host request allowance QHash hostRequestLimits; ///< User-set per-host request allowances QHash hostQuotaRemaining; ///< Per-host allowance left in the current second QHash hostLast429; ///< When each host was last rate limited - QHash hostInFlight; ///< Network replies currently in flight, per host - - /** @brief Maximum concurrent in-flight network replies per host. */ - static constexpr int MAX_IN_FLIGHT_PER_HOST = 6; CardPictureLoaderLocal *localLoader; ///< Loader for local images QSet currentlyLoading; ///< Deduplication: contains pixmapCacheKey currently being loaded @@ -143,14 +140,6 @@ private: */ [[nodiscard]] int hostAllowanceCeiling(const QString &host) const; - /** - * @brief Whether a host may skip dispatch pacing and per-host allowance entirely. - * - * A host is unlocked while it has no user limit and no reduced allowance installed by a 429. - * A 429 drops it out of the fast path until resetRequestQuota() walks the allowance back up. - */ - [[nodiscard]] bool isUnlockedHost(const QString &host) const; - /** @brief Returns cached redirect URL for the given original URL, if available. */ [[nodiscard]] QUrl getCachedRedirect(const QUrl &originalUrl) const; diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp index e4a36ab8c..072a919d7 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp @@ -22,7 +22,7 @@ static const QStringList MD5_BLACKLIST = { "fbc7d763c08771c260b39e2115414eeb" // Current card back hash }; -const ServerRateLimiter &CardPictureLoaderWorkerWork::rateLimiter() +ServerRateLimiter &CardPictureLoaderWorkerWork::rateLimiter() { return s_rateLimiter; } diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h index f05d727de..8490cb3ac 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h @@ -44,27 +44,7 @@ public: CardPictureToLoad cardToDownload; ///< The card and associated URLs to try downloading /** @brief Shared per-server 429 backoff state. */ - static const ServerRateLimiter &rateLimiter(); - - /** - * @brief Starts downloading the next URL for this card. - * - * Skips URLs whose server is currently in 429 backoff, either waiting the - * backoff out or falling through to the other configured sources. Also used by - * the dispatch machinery to hand an entry back after it was removed from the - * request queue when its host turned out to be backed off. - */ - void startNextPicDownload(); - - /** - * @brief Schedules a deferred retry after the relevant server backoff expires. - * - * Waits on the current URL's server when it is the reason we are blocked, - * otherwise on the earliest active backoff. If no servers are in backoff, - * concludes with failure. Otherwise resets the CardPictureToLoad indices and - * retries after the backoff period. - */ - void scheduleDeferredRetry(); + static ServerRateLimiter &rateLimiter(); public slots: /** @@ -78,6 +58,9 @@ private: static ServerRateLimiter s_rateLimiter; ///< Shared per-server 429 backoff state + /** @brief Starts downloading the next URL for this card. */ + void startNextPicDownload(); + /** @brief Called when all URLs have been exhausted or download failed. */ void picDownloadFailed(); @@ -102,6 +85,16 @@ private: */ void concludeImageLoad(const QImage &image); + /** + * @brief Schedules a deferred retry after the relevant server backoff expires. + * + * Waits on the current URL's server when it is the reason we are blocked, + * otherwise on the earliest active backoff. If no servers are in backoff, + * concludes with failure. Otherwise resets the CardPictureToLoad indices and + * retries after the backoff period. + */ + void scheduleDeferredRetry(); + private slots: /** @brief Updates the picDownload setting when it changes. */ void picDownloadChanged(); diff --git a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp index 8628aa5a8..6223cd480 100644 --- a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp +++ b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -18,6 +17,8 @@ #include #include +static constexpr int UNLOCKED_HOST_LIMIT_MAX = 50; ///< Upper bound for rate limits on hosts unlocked by the developer + DeckEditorSettingsPage::DeckEditorSettingsPage() { picDownloadCheckBox.setChecked(SettingsCache::instance().downloads().getPicDownload()); @@ -53,9 +54,7 @@ DeckEditorSettingsPage::DeckEditorSettingsPage() urlList->setDragDropMode(QAbstractItemView::InternalMove); connect(urlList->model(), &QAbstractItemModel::rowsMoved, this, &DeckEditorSettingsPage::urlListChanged); - for (const QString &url : SettingsCache::instance().downloads().getAllURLs()) { - addUrlItem(url); - } + urlList->addItems(SettingsCache::instance().downloads().getAllURLs()); aAdd = new QAction(this); aAdd->setIcon(themePixmap(QStringLiteral("icons/increment"))); @@ -126,9 +125,7 @@ void DeckEditorSettingsPage::resetDownloadedURLsButtonClicked() { SettingsCache::instance().downloads().resetToDefaultURLs(); urlList->clear(); - for (const QString &url : SettingsCache::instance().downloads().getAllURLs()) { - addUrlItem(url); - } + urlList->addItems(SettingsCache::instance().downloads().getAllURLs()); QMessageBox::information(this, tr("Success"), tr("Download URLs have been reset.")); } @@ -137,7 +134,7 @@ void DeckEditorSettingsPage::actAddURL() bool ok; QString msg = QInputDialog::getText(this, tr("Add URL"), tr("URL:"), QLineEdit::Normal, QString(), &ok); if (ok) { - addUrlItem(msg); + urlList->addItem(msg); storeSettings(); } } @@ -152,14 +149,12 @@ void DeckEditorSettingsPage::actRemoveURL() void DeckEditorSettingsPage::actEditURL() { - QListWidgetItem *item = urlList->currentItem(); - if (item) { - const QString oldText = urlForItem(item); + if (urlList->currentItem()) { + QString oldText = urlList->currentItem()->text(); bool ok; QString msg = QInputDialog::getText(this, tr("Edit URL"), tr("URL:"), QLineEdit::Normal, oldText, &ok); if (ok) { - item->setData(Qt::UserRole, msg); - item->setText(urlLabel(msg)); + urlList->currentItem()->setText(msg); storeSettings(); } } @@ -171,77 +166,10 @@ void DeckEditorSettingsPage::storeSettings() QStringList downloadUrls; for (int i = 0; i < urlList->count(); i++) { - const QString url = urlForItem(urlList->item(i)); - qInfo() << "Priority" << i << ":" << url; - downloadUrls << url; + qInfo() << "Priority" << i << ":" << urlList->item(i)->text(); + downloadUrls << urlList->item(i)->text(); } SettingsCache::instance().downloads().setDownloadUrls(downloadUrls); - - // Drop per-host limits whose host is no longer referenced by any configured URL, so removing - // a URL doesn't leave a stale throttle behind that reactivates if the host is re-added. - QSet usedHosts; - for (const QString &url : downloadUrls) { - const QString host = QUrl(url).host(); - if (!host.isEmpty()) { - usedHosts.insert(host); - } - } - QHash limits = SettingsCache::instance().downloads().getHostRequestLimits(); - bool limitsChanged = false; - for (auto it = limits.begin(); it != limits.end();) { - if (!usedHosts.contains(it.key())) { - it = limits.erase(it); - limitsChanged = true; - } else { - ++it; - } - } - if (limitsChanged) { - SettingsCache::instance().downloads().setHostRequestLimits(limits); - } - - refreshUrlItems(); -} - -QListWidgetItem *DeckEditorSettingsPage::addUrlItem(const QString &url) -{ - auto *item = new QListWidgetItem(urlLabel(url)); - item->setData(Qt::UserRole, url); - urlList->addItem(item); - return item; -} - -QString DeckEditorSettingsPage::urlForItem(const QListWidgetItem *item) const -{ - return item->data(Qt::UserRole).toString(); -} - -QString DeckEditorSettingsPage::urlLabel(const QString &url) const -{ - const QString host = QUrl(url).host(); - if (host.isEmpty()) { - return url; - } - - const QHash limits = SettingsCache::instance().downloads().getHostRequestLimits(); - const int devCap = - DownloadSettings::getDeveloperHostCaps().value(host, DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT); - if (devCap == DownloadSettings::UNLIMITED_HOST_QUOTA && !limits.contains(host)) { - return tr("%1 (unlimited)").arg(url); - } - - const int requested = limits.value( - host, devCap == DownloadSettings::UNLIMITED_HOST_QUOTA ? DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT : devCap); - const int effective = SettingsCache::instance().downloads().clampHostRequestLimit(host, requested); - return tr("%1 (%2/s)").arg(url).arg(effective); -} - -void DeckEditorSettingsPage::refreshUrlItems() -{ - for (int i = 0; i < urlList->count(); ++i) { - QListWidgetItem *item = urlList->item(i); - item->setText(urlLabel(urlForItem(item))); - } } void DeckEditorSettingsPage::actAdjustRateLimit() @@ -251,7 +179,7 @@ void DeckEditorSettingsPage::actAdjustRateLimit() return; } - const QString host = QUrl(urlForItem(urlList->currentItem())).host(); + const QString host = QUrl(urlList->currentItem()->text()).host(); if (host.isEmpty()) { QMessageBox::information(this, tr("Adjust Rate Limit"), tr("The selected URL does not have a valid host.")); return; @@ -266,21 +194,19 @@ void DeckEditorSettingsPage::actAdjustRateLimit() int minimum; int maximum; int defaultValue; - QString prompt; if (unlocked) { minimum = 0; // 0 means "unlimited" - maximum = DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT; + maximum = UNLOCKED_HOST_LIMIT_MAX; defaultValue = currentLimits.value(host, 0); - prompt = tr("Requests per second (0 = unlimited, fastest; up to %1):").arg(maximum); } else { minimum = DownloadSettings::MIN_HOST_REQUEST_LIMIT; maximum = devCap; defaultValue = currentLimits.value(host, devCap); - prompt = tr("Requests per second (developer maximum is %1):").arg(maximum); } - const int value = QInputDialog::getInt(this, tr("Adjust Rate Limit for %1").arg(host), prompt, defaultValue, - minimum, maximum, 1, &ok); + const int value = QInputDialog::getInt(this, tr("Adjust Rate Limit for %1").arg(host), + tr("Requests per second (developer maximum is %1):").arg(maximum), + defaultValue, minimum, maximum, 1, &ok); if (!ok) { return; } @@ -292,7 +218,6 @@ void DeckEditorSettingsPage::actAdjustRateLimit() limits.insert(host, value); } SettingsCache::instance().downloads().setHostRequestLimits(limits); - refreshUrlItems(); } void DeckEditorSettingsPage::urlListChanged(const QModelIndex &, int, int, const QModelIndex &, int) @@ -376,7 +301,4 @@ void DeckEditorSettingsPage::retranslateUi() aEdit->setText(tr("Edit URL")); aRemove->setText(tr("Remove URL")); aRateLimit->setText(tr("Adjust Rate Limit")); - - // The per-URL rate limit suffixes are translated, so refresh them when the language changes. - refreshUrlItems(); } diff --git a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h index 23e33fa20..57de5699e 100644 --- a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h +++ b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h @@ -47,18 +47,6 @@ private: QLabel infoOnSpoilersLabel; QPushButton *mpSpoilerPathButton; QPushButton *updateNowButton; - - /** @brief Adds a list item for the given URL, storing the raw URL alongside its displayed label. */ - QListWidgetItem *addUrlItem(const QString &url); - - /** @brief Returns the raw URL stored on a list item. */ - [[nodiscard]] QString urlForItem(const QListWidgetItem *item) const; - - /** @brief Returns the display label for a URL, including its current effective rate limit. */ - [[nodiscard]] QString urlLabel(const QString &url) const; - - /** @brief Refreshes the displayed label of every URL item after limits or settings change. */ - void refreshUrlItems(); }; #endif // COCKATRICE_DECK_EDITOR_SETTINGS_PAGE_H diff --git a/libcockatrice_settings/libcockatrice/settings/download_settings.cpp b/libcockatrice_settings/libcockatrice/settings/download_settings.cpp index a321f0bd0..5293e390b 100644 --- a/libcockatrice_settings/libcockatrice/settings/download_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/download_settings.cpp @@ -69,46 +69,21 @@ void DownloadSettings::setDownloadSpoilerStatus(bool _spoilerStatus) QHash DownloadSettings::getHostRequestLimits() const { - auto settings = getSettings(); - if (!defaultGroup.isEmpty()) { - settings.beginGroup(defaultGroup); - } - settings.beginGroup("hostRequestLimits"); - + const QVariantMap stored = getValue("hostRequestLimits").toMap(); QHash hostRequestLimits; - const QStringList hosts = settings.childKeys(); - for (const QString &host : hosts) { - hostRequestLimits.insert(host, settings.value(host).toInt()); - } - - settings.endGroup(); - if (!defaultGroup.isEmpty()) { - settings.endGroup(); + for (auto it = stored.cbegin(); it != stored.cend(); ++it) { + hostRequestLimits.insert(it.key(), it.value().toInt()); } return hostRequestLimits; } void DownloadSettings::setHostRequestLimits(const QHash &hostRequestLimits) { - auto settings = getSettings(); - if (!defaultGroup.isEmpty()) { - settings.beginGroup(defaultGroup); - } - - // Drop the legacy single-key form (an opaque @Variant blob) written by earlier builds so each - // host is stored as a plain, hand-editable key in its own subgroup. - settings.remove("hostRequestLimits"); - settings.beginGroup("hostRequestLimits"); - settings.remove(QString()); + QVariantMap stored; for (auto it = hostRequestLimits.cbegin(); it != hostRequestLimits.cend(); ++it) { - settings.setValue(it.key(), it.value()); + stored.insert(it.key(), it.value()); } - settings.endGroup(); - - if (!defaultGroup.isEmpty()) { - settings.endGroup(); - } - settings.sync(); + setValue(stored, "hostRequestLimits"); emit hostRequestLimitsChanged(); }