From a44757bd11a587eb37affa08c24aa333e62bf58c Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Fri, 9 Oct 2026 11:07:48 +0200 Subject: [PATCH] [Client] Improve card database update visibility (#7458) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Client] Improve card database update visibility Addresses Cockatrice/Cockatrice#7431. Cockatrice previously offered no in-client feedback while the card database updater ran: the only progress UI lived in the first-run wizard, stderr/stdout were not parsed in interactive runs, and starting a second update with Oracle already open or with a hidden background run active just showed a confusing 'already running' guard - even though Oracle is launched as a separate process with no lifecycle tracking. - Add a status-bar widget (CardDatabaseUpdateStatusBar) that appears while an update runs and shows the Downloading/Parsing/Importing stages with percent progress; it is also used in the quit-confirm prompt so users see an unfinished update before cancelling it. - Parse the updater's PROGRESS lines (new CardUpdateProgressParser, unit-tested) in both foreground and background runs and log stage changes to the new card_database.update category, throttled to at most one line per second; forward Oracle stderr to the logs. - Show a tray balloon when a background update starts and on finish or failure instead of staying silent. - Request a running Oracle instance to raise itself (QLocalServer CockatriceOracleRaise-) when the user starts another update, and demote the 'already running' guard to an informative message only when no running instance can be found. - Emit download/scan/import progress unconditionally from Oracle so interactive and background runs behave the same. - Document the new log category in the troubleshooting guide. * [Client] Address card database update review feedback * [Client] Add setting to control card database update tray notifications * [Client] Address remaining card database update review nits - Rename card_update_progress_parser.{h,cpp} to card_update_progress.{h,cpp} - Store the CardUpdateProgress struct in the status bar widget * Lint. --------- Co-authored-by: Lukas Brübach --- cockatrice/CMakeLists.txt | 4 + .../card_database_update_status_bar.cpp | 117 ++++++++++++++ .../card_database_update_status_bar.h | 68 ++++++++ .../card_update_progress.cpp | 59 +++++++ .../card_update_progress.h | 42 +++++ .../widgets/onboarding/first_run_wizard.cpp | 4 +- .../widgets/onboarding/first_run_wizard.h | 4 +- .../pages/card_database_setup_page.cpp | 22 ++- .../pages/card_database_setup_page.h | 3 +- .../settings_page/general_settings_page.cpp | 7 + .../settings_page/general_settings_page.h | 1 + cockatrice/src/interface/window_main.cpp | 149 +++++++++++++++--- cockatrice/src/interface/window_main.h | 29 +++- cockatrice/src/single_instance_manager.cpp | 15 +- .../troubleshooting/enabling_debug_logs.md | 2 + .../settings/updates_settings.cpp | 10 ++ .../libcockatrice/settings/updates_settings.h | 2 + libcockatrice_utility/CMakeLists.txt | 1 + .../libcockatrice/utility/local_server_name.h | 28 ++++ oracle/src/oracleimporter.h | 13 +- oracle/src/oraclewizard.cpp | 53 +++++++ oracle/src/oraclewizard.h | 9 ++ oracle/src/pages.cpp | 19 +-- tests/CMakeLists.txt | 9 ++ tests/card_update_progress_test.cpp | 93 +++++++++++ tests/settings/settings_defaults_test.cpp | 6 + 26 files changed, 702 insertions(+), 67 deletions(-) create mode 100644 cockatrice/src/interface/card_database_update/card_database_update_status_bar.cpp create mode 100644 cockatrice/src/interface/card_database_update/card_database_update_status_bar.h create mode 100644 cockatrice/src/interface/card_database_update/card_update_progress.cpp create mode 100644 cockatrice/src/interface/card_database_update/card_update_progress.h create mode 100644 libcockatrice_utility/libcockatrice/utility/local_server_name.h create mode 100644 tests/card_update_progress_test.cpp diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index 1760d86b8..14258fad2 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -140,6 +140,10 @@ set(cockatrice_SOURCES src/game_graphics/zones/view_zone.cpp src/game_graphics/zones/view_zone_widget.cpp src/game_graphics/board/abstract_graphics_item.cpp + src/interface/card_database_update/card_database_update_status_bar.cpp + src/interface/card_database_update/card_database_update_status_bar.h + src/interface/card_database_update/card_update_progress.cpp + src/interface/card_database_update/card_update_progress.h src/interface/card_picture_loader/card_picture_loader.cpp src/interface/card_picture_loader/card_picture_loader_local.cpp src/interface/card_picture_loader/card_picture_loader_request_status_display_widget.cpp diff --git a/cockatrice/src/interface/card_database_update/card_database_update_status_bar.cpp b/cockatrice/src/interface/card_database_update/card_database_update_status_bar.cpp new file mode 100644 index 000000000..721fdd6c8 --- /dev/null +++ b/cockatrice/src/interface/card_database_update/card_database_update_status_bar.cpp @@ -0,0 +1,117 @@ +#include "card_database_update_status_bar.h" + +#include +#include +#include +#include +#include +#include +#include + +namespace +{ +/** + * @brief Translated name of one updater stage. + * @param fallback shown when the stage is unknown or not reported yet + */ +QString stageName(CardUpdateStage stage, const QString &fallback) +{ + switch (stage) { + case CardUpdateStage::Download: + return QCoreApplication::translate("CardDatabaseUpdateStatusBar", "Downloading"); + case CardUpdateStage::Scan: + return QCoreApplication::translate("CardDatabaseUpdateStatusBar", "Parsing"); + case CardUpdateStage::Import: + return QCoreApplication::translate("CardDatabaseUpdateStatusBar", "Importing"); + case CardUpdateStage::Unknown: + return fallback; + } + return fallback; +} +} // namespace + +CardDatabaseUpdateStatusBar::CardDatabaseUpdateStatusBar(QWidget *parent) : QWidget(parent) +{ + layout = new QHBoxLayout(this); + layout->setContentsMargins(0, 0, 0, 0); + + stageLabel = new QLabel(this); + progressBar = new QProgressBar(this); + progressBar->setMinimumWidth(120); + + layout->addWidget(stageLabel); + layout->addWidget(progressBar); + + setLayout(layout); + refresh(); + hide(); +} + +QString CardDatabaseUpdateStatusBar::statusText() const +{ + if (!active) { + return {}; + } + const QString name = stageName(progress.stage, tr("Updating the card database")); + if (progress.total <= 0) { + return name; + } + const int percent = static_cast((100.0 * progress.done) / progress.total); + return tr("%1 (%2%)").arg(name).arg(percent); +} + +void CardDatabaseUpdateStatusBar::updateStarted() +{ + active = true; + progress = {}; + refresh(); + show(); +} + +void CardDatabaseUpdateStatusBar::updateProgress(const CardUpdateProgress &progress) +{ + active = true; + this->progress = progress; + refresh(); +} + +void CardDatabaseUpdateStatusBar::updateFinished() +{ + active = false; + progress = {}; + refresh(); + hide(); +} + +void CardDatabaseUpdateStatusBar::changeEvent(QEvent *event) +{ + if (event->type() == QEvent::LanguageChange) { + refresh(); + } + QWidget::changeEvent(event); +} + +void CardDatabaseUpdateStatusBar::refresh() +{ + const QString name = stageName(progress.stage, tr("Updating the card database")); + stageLabel->setText(name); + + const qint64 cappedTotal = qMin(progress.total, INT_MAX); + const qint64 cappedDone = qMin(progress.done, INT_MAX); + const int minimum = 0; + const int maximum = static_cast(cappedTotal); + if (progressBar->minimum() != minimum || progressBar->maximum() != maximum) { + // A zero range is the busy indicator, used until the first report arrives. + progressBar->setRange(minimum, maximum); + } + progressBar->setValue(static_cast(cappedDone)); + + if (active && progress.total > 0) { + const int percent = static_cast((100.0 * progress.done) / progress.total); + setToolTip(tr("Card database update in progress: %1 (%2%)").arg(name).arg(percent)); + } else if (active) { + setToolTip(tr("Card database update in progress.")); + } else { + setToolTip({}); + } +} diff --git a/cockatrice/src/interface/card_database_update/card_database_update_status_bar.h b/cockatrice/src/interface/card_database_update/card_database_update_status_bar.h new file mode 100644 index 000000000..6db3bd225 --- /dev/null +++ b/cockatrice/src/interface/card_database_update/card_database_update_status_bar.h @@ -0,0 +1,68 @@ +#ifndef CARD_DATABASE_UPDATE_STATUS_BAR_H +#define CARD_DATABASE_UPDATE_STATUS_BAR_H + +#include "card_update_progress.h" + +#include +#include +#include + +class QEvent; +class QHBoxLayout; +class QLabel; +class QProgressBar; + +/** + * @class CardDatabaseUpdateStatusBar + * @brief Status bar widget showing what the card database updater (Oracle) is doing. + * + * The updater runs as a separate process, so this widget is the in-client view of + * its progress: it appears when an update starts, tracks the download/scan/import + * stages reported by the updater, and disappears when the update finishes. + */ +class CardDatabaseUpdateStatusBar : public QWidget +{ + Q_OBJECT +public: + /** + * @brief Constructs a hidden progress widget. + * @param parent Parent widget + */ + explicit CardDatabaseUpdateStatusBar(QWidget *parent = nullptr); + + /** + * @brief Human readable description of the running stage, e.g. "Downloading (42%)". + * @return the description, or an empty string when no update is running + */ + QString statusText() const; + +public slots: + /** + * @brief Shows the widget and resets it to the indeterminate state. + */ + void updateStarted(); + + /** + * @brief Shows the progress of one update stage. + */ + void updateProgress(const CardUpdateProgress &progress); + + /** + * @brief Hides the widget and clears the reported progress. + */ + void updateFinished(); + +protected: + void changeEvent(QEvent *event) override; + +private: + void refresh(); + + QHBoxLayout *layout; ///< Horizontal layout holding the stage label and the progress bar + QLabel *stageLabel; ///< Name of the stage currently running + QProgressBar *progressBar; ///< Progress within the current stage + CardUpdateProgress progress; ///< Last reported progress + bool active = false; ///< Whether an update is currently running +}; + +#endif // CARD_DATABASE_UPDATE_STATUS_BAR_H diff --git a/cockatrice/src/interface/card_database_update/card_update_progress.cpp b/cockatrice/src/interface/card_database_update/card_update_progress.cpp new file mode 100644 index 000000000..1f8e500d0 --- /dev/null +++ b/cockatrice/src/interface/card_database_update/card_update_progress.cpp @@ -0,0 +1,59 @@ +#include "card_update_progress.h" + +#include + +namespace +{ +CardUpdateStage parseStage(const QByteArray &token) +{ + if (token == "download") { + return CardUpdateStage::Download; + } + if (token == "scan") { + return CardUpdateStage::Scan; + } + if (token == "import") { + return CardUpdateStage::Import; + } + return CardUpdateStage::Unknown; +} +} // namespace + +std::optional CardUpdateProgress::fromProtocolLine(const QByteArray &line) +{ + const QByteArray trimmed = line.trimmed(); + if (!trimmed.startsWith("PROGRESS ")) { + return std::nullopt; + } + const QList parts = trimmed.split(' '); + if (parts.size() != 4) { + return std::nullopt; + } + bool doneOk = false; + bool totalOk = false; + const qint64 done = parts.at(2).toLongLong(&doneOk); + const qint64 total = parts.at(3).toLongLong(&totalOk); + if (!doneOk || !totalOk || done < 0 || total < 0) { + return std::nullopt; + } + CardUpdateProgress progress; + progress.stage = parseStage(parts.at(1)); + progress.done = done; + progress.total = total; + return progress; +} + +QString CardUpdateProgress::stageToken() const +{ + switch (stage) { + case CardUpdateStage::Download: + return QStringLiteral("download"); + case CardUpdateStage::Scan: + return QStringLiteral("scan"); + case CardUpdateStage::Import: + return QStringLiteral("import"); + case CardUpdateStage::Unknown: + return QStringLiteral("unknown"); + } + return QStringLiteral("unknown"); +} diff --git a/cockatrice/src/interface/card_database_update/card_update_progress.h b/cockatrice/src/interface/card_database_update/card_update_progress.h new file mode 100644 index 000000000..743f32209 --- /dev/null +++ b/cockatrice/src/interface/card_database_update/card_update_progress.h @@ -0,0 +1,42 @@ +#ifndef CARD_UPDATE_PROGRESS_H +#define CARD_UPDATE_PROGRESS_H + +#include +#include +#include +#include + +/** + * @brief One stage of the card database updater's progress protocol. + */ +enum class CardUpdateStage +{ + Download, ///< downloading the source data + Scan, ///< parsing the downloaded data + Import, ///< importing cards into the database + Unknown, ///< a stage the client does not recognize +}; + +/** + * @brief One progress line read from the card database updater's stdout. + */ +struct CardUpdateProgress +{ + /** + * @brief Parses one line of the updater stdout protocol: "PROGRESS ". + * @param line a single stdout line, with or without its trailing newline + * @return the parsed progress, or @c std::nullopt when the line is not a valid progress line + */ + static std::optional fromProtocolLine(const QByteArray &line); + + /** + * @brief Protocol token of the stage ("download", "scan", "import" or "unknown"), for logging. + */ + QString stageToken() const; + + CardUpdateStage stage = CardUpdateStage::Unknown; + qint64 done = 0; + qint64 total = 0; +}; + +#endif // CARD_UPDATE_PROGRESS_H diff --git a/cockatrice/src/interface/widgets/onboarding/first_run_wizard.cpp b/cockatrice/src/interface/widgets/onboarding/first_run_wizard.cpp index 6441a7864..426811ec2 100644 --- a/cockatrice/src/interface/widgets/onboarding/first_run_wizard.cpp +++ b/cockatrice/src/interface/widgets/onboarding/first_run_wizard.cpp @@ -185,10 +185,10 @@ void FirstRunWizard::onCardDatabaseUpdateFinished(bool success) } } -void FirstRunWizard::onCardDatabaseUpdateProgress(const QString &stage, qint64 done, qint64 total) +void FirstRunWizard::onCardDatabaseUpdateProgress(const CardUpdateProgress &progress) { if (cardDatabasePage) { - cardDatabasePage->onUpdateProgress(stage, done, total); + cardDatabasePage->onUpdateProgress(progress); } } diff --git a/cockatrice/src/interface/widgets/onboarding/first_run_wizard.h b/cockatrice/src/interface/widgets/onboarding/first_run_wizard.h index bd0e79679..812985a9f 100644 --- a/cockatrice/src/interface/widgets/onboarding/first_run_wizard.h +++ b/cockatrice/src/interface/widgets/onboarding/first_run_wizard.h @@ -1,6 +1,8 @@ #ifndef FIRST_RUN_WIZARD_H #define FIRST_RUN_WIZARD_H +#include "../../card_database_update/card_update_progress.h" + #include #include #include @@ -42,7 +44,7 @@ public slots: void onCardDatabaseUpdateFinished(bool success); /** @brief Forwarded from MainWindow while the background card database update process runs. */ - void onCardDatabaseUpdateProgress(const QString &stage, qint64 done, qint64 total); + void onCardDatabaseUpdateProgress(const CardUpdateProgress &progress); protected: void closeEvent(QCloseEvent *event) override; diff --git a/cockatrice/src/interface/widgets/onboarding/pages/card_database_setup_page.cpp b/cockatrice/src/interface/widgets/onboarding/pages/card_database_setup_page.cpp index feff78088..cb53a25f3 100644 --- a/cockatrice/src/interface/widgets/onboarding/pages/card_database_setup_page.cpp +++ b/cockatrice/src/interface/widgets/onboarding/pages/card_database_setup_page.cpp @@ -188,21 +188,29 @@ void CardDatabaseSetupPage::onUpdateFinished(bool success) } } -void CardDatabaseSetupPage::onUpdateProgress(const QString &stage, qint64 done, qint64 total) +void CardDatabaseSetupPage::onUpdateProgress(const CardUpdateProgress &progress) { if (state != State::Running) { return; } + const qint64 total = progress.total; + const qint64 done = progress.done; progressBar->setRange(0, total > 0 ? static_cast(qMin(total, INT_MAX)) : 0); progressBar->setValue(static_cast(qMin(done, INT_MAX))); if (total > 0) { const int percent = static_cast((100.0 * done) / total); - if (stage == QLatin1String("download")) { - statusLabel->setText(tr("Downloading the card database (%1%)…").arg(percent)); - } else if (stage == QLatin1String("scan")) { - statusLabel->setText(tr("Parsing the card database (%1%)…").arg(percent)); - } else if (stage == QLatin1String("import")) { - statusLabel->setText(tr("Importing cards (%1%)…").arg(percent)); + switch (progress.stage) { + case CardUpdateStage::Download: + statusLabel->setText(tr("Downloading the card database (%1%)…").arg(percent)); + break; + case CardUpdateStage::Scan: + statusLabel->setText(tr("Parsing the card database (%1%)…").arg(percent)); + break; + case CardUpdateStage::Import: + statusLabel->setText(tr("Importing cards (%1%)…").arg(percent)); + break; + case CardUpdateStage::Unknown: + break; } } } diff --git a/cockatrice/src/interface/widgets/onboarding/pages/card_database_setup_page.h b/cockatrice/src/interface/widgets/onboarding/pages/card_database_setup_page.h index 77631dc77..a84223497 100644 --- a/cockatrice/src/interface/widgets/onboarding/pages/card_database_setup_page.h +++ b/cockatrice/src/interface/widgets/onboarding/pages/card_database_setup_page.h @@ -1,6 +1,7 @@ #ifndef CARD_DATABASE_SETUP_PAGE_H #define CARD_DATABASE_SETUP_PAGE_H +#include "../../../card_database_update/card_update_progress.h" #include "../first_run_wizard_page.h" #include @@ -33,7 +34,7 @@ public: void retranslateUi() override; void onUpdateFinished(bool success); - void onUpdateProgress(const QString &stage, qint64 done, qint64 total); + void onUpdateProgress(const CardUpdateProgress &progress); signals: void updateRequested(); diff --git a/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp b/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp index a4597014a..73a0f7be2 100644 --- a/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp +++ b/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp @@ -161,12 +161,18 @@ GeneralSettingsPage::GeneralSettingsPage() newVersionOracleCheckBox.setChecked(settings.updates().getNotifyAboutNewVersion()); + cardDatabaseUpdateTrayNotificationCheckBox.setChecked(settings.updates().getCardDatabaseUpdateTrayNotification()); + + connect(&cardDatabaseUpdateTrayNotificationCheckBox, &QCheckBox::QT_STATE_CHANGED, &settings.updates(), + &UpdatesSettings::setCardDatabaseUpdateTrayNotification); + auto *cardDatabaseGrid = new QGridLayout; cardDatabaseGrid->addWidget(&startupCardUpdateCheckBehaviorLabel, 0, 0); cardDatabaseGrid->addWidget(&startupCardUpdateCheckBehaviorSelector, 0, 1); cardDatabaseGrid->addWidget(&cardUpdateCheckIntervalLabel, 1, 0); cardDatabaseGrid->addWidget(&cardUpdateCheckIntervalSpinBox, 1, 1); cardDatabaseGrid->addWidget(&lastCardUpdateCheckDateLabel, 2, 1); + cardDatabaseGrid->addWidget(&cardDatabaseUpdateTrayNotificationCheckBox, 3, 0, 1, 2); cardDatabaseGroupBox = new QGroupBox; cardDatabaseGroupBox->setLayout(cardDatabaseGrid); @@ -565,6 +571,7 @@ void GeneralSettingsPage::retranslateUi() tr("Always update in the background")); cardUpdateCheckIntervalLabel.setText(tr("Check for card database updates every")); cardUpdateCheckIntervalSpinBox.setSuffix(tr(" days")); + cardDatabaseUpdateTrayNotificationCheckBox.setText(tr("Notify in the system tray while the card database updates")); QDate lastCheckDate = settings.updates().getLastCardUpdateCheck(); int daysAgo = lastCheckDate.daysTo(QDate::currentDate()); diff --git a/cockatrice/src/interface/widgets/settings_page/general_settings_page.h b/cockatrice/src/interface/widgets/settings_page/general_settings_page.h index fca300670..fcf1c78d0 100644 --- a/cockatrice/src/interface/widgets/settings_page/general_settings_page.h +++ b/cockatrice/src/interface/widgets/settings_page/general_settings_page.h @@ -75,6 +75,7 @@ private: QLabel cardUpdateCheckIntervalLabel; QSpinBox cardUpdateCheckIntervalSpinBox; QLabel lastCardUpdateCheckDateLabel; + QCheckBox cardDatabaseUpdateTrayNotificationCheckBox; QCheckBox showTipsOnStartup; QLabel startupTabLabel; diff --git a/cockatrice/src/interface/window_main.cpp b/cockatrice/src/interface/window_main.cpp index 889e81bfe..189585d84 100644 --- a/cockatrice/src/interface/window_main.cpp +++ b/cockatrice/src/interface/window_main.cpp @@ -37,6 +37,8 @@ #include "../interface/widgets/tabs/tab_supervisor.h" #include "../main.h" #include "abstract_client.h" +#include "card_database_update/card_database_update_status_bar.h" +#include "card_database_update/card_update_progress.h" #include "intents/contexts/context_connect_to_server.h" #include "intents/contexts/context_join_room.h" #include "intents/intent.h" @@ -67,6 +69,7 @@ #include #include #include +#include #include #include #include @@ -103,6 +106,7 @@ #include #include #include +#include #include #include #include @@ -574,6 +578,13 @@ MainWindow::MainWindow(QWidget *parent) connect(connectionController, &ConnectionController::pingStatsUpdated, latencyStatus, &LatencyStatusWidget::updateData); + cardUpdateStatusBar = new CardDatabaseUpdateStatusBar(this); + statusBar()->addPermanentWidget(cardUpdateStatusBar); + connect(this, &MainWindow::cardDatabaseUpdateProgress, cardUpdateStatusBar, + &CardDatabaseUpdateStatusBar::updateProgress); + connect(this, &MainWindow::cardDatabaseUpdateFinished, cardUpdateStatusBar, + &CardDatabaseUpdateStatusBar::updateFinished); + connect(&SettingsCache::instance().shortcuts(), &ShortcutsSettings::shortCutChanged, this, &MainWindow::refreshShortcuts); refreshShortcuts(); @@ -896,10 +907,15 @@ void MainWindow::closeEvent(QCloseEvent *event) bClosingDown = true; if (cardUpdateProcess && cardUpdateProcess->state() != QProcess::NotRunning) { - if (QMessageBox::question(this, tr("Are you sure?"), - tr("A card database update is still running. Quitting now will cancel it.\n" - "Are you sure you want to quit?"), - QMessageBox::Yes | QMessageBox::No, QMessageBox::No) == QMessageBox::No) { + const QString runningFor = cardUpdateStatusBar ? cardUpdateStatusBar->statusText() : QString(); + const QString message = runningFor.isEmpty() + ? tr("A card database update is still running. Quitting now will cancel it.\n" + "Are you sure you want to quit?") + : tr("A card database update is still running: %1.\nQuitting now will cancel it.\n" + "Are you sure you want to quit?") + .arg(runningFor); + if (QMessageBox::question(this, tr("Are you sure?"), message, QMessageBox::Yes | QMessageBox::No, + QMessageBox::No) == QMessageBox::No) { event->ignore(); bClosingDown = false; return; @@ -1116,16 +1132,28 @@ void MainWindow::actCheckCardUpdatesBackground() void MainWindow::createCardUpdateProcess(bool background) { if (cardUpdateProcess) { - QMessageBox::information(this, tr("Information"), tr("A card database update is already running.")); + qCDebug(CardDatabaseUpdateLog) << "A card database update is already running; asking it to come to the front"; + if (requestCardUpdateRaise()) { + return; + } + if (background) { + // A hidden run has nothing to raise: keep the user informed without blocking. + announceCardUpdateRunning(); + return; + } + QMessageBox::information(this, tr("Information"), + tr("The card database updater is already open.\n" + "Look for the Oracle window - it may be behind this one.")); return; } cardUpdateProcess = new QProcess(this); connect(cardUpdateProcess, &QProcess::errorOccurred, this, &MainWindow::cardUpdateError); - connect(cardUpdateProcess, qOverload(&QProcess::finished), this, &MainWindow::cardUpdateFinished); + connect(cardUpdateProcess, &QProcess::readyReadStandardOutput, this, &MainWindow::cardUpdateProgressOutput); + connect(cardUpdateProcess, &QProcess::readyReadStandardError, this, &MainWindow::cardUpdateStandardError); // full "run the update" command; leave empty if not present QString updaterCmd; @@ -1170,6 +1198,8 @@ void MainWindow::createCardUpdateProcess(bool background) } if (updaterCmd.isEmpty()) { + qCWarning(CardDatabaseUpdateLog) << "Unable to locate the card database updater; looked for" + << dir.absoluteFilePath(binaryName); QMessageBox::warning(this, tr("Error"), tr("Unable to run the card database updater: ") + dir.absoluteFilePath(binaryName)); exitCardDatabaseUpdate(); @@ -1177,13 +1207,24 @@ void MainWindow::createCardUpdateProcess(bool background) return; } + cardUpdateOutputBuffer.clear(); + cardUpdateErrorOutput.clear(); + cardUpdateErrorPartial.clear(); + cardUpdateLoggedStage = CardUpdateStage::Unknown; + cardUpdateProgressLogTimer.start(); + + qCInfo(CardDatabaseUpdateLog) << "Starting the card database updater" << updaterCmd << "background:" << background; + cardUpdateStatusBar->updateStarted(); + if (!background) { cardUpdateProcess->start(updaterCmd, QStringList()); } else { - cardUpdateOutputBuffer.clear(); - connect(cardUpdateProcess, &QProcess::readyReadStandardOutput, this, &MainWindow::cardUpdateProgressOutput); cardUpdateProcess->start(updaterCmd, QStringList("-b")); statusBar()->showMessage(tr("Card database update running.")); + if (trayIcon && SettingsCache::instance().updates().getCardDatabaseUpdateTrayNotification()) { + trayIcon->showMessage(tr("Card database update started"), + tr("Cockatrice is updating the card database in the background.")); + } } } @@ -1198,24 +1239,43 @@ void MainWindow::cardUpdateProgressOutput() if (newline < 0) { break; } - const QByteArray line = cardUpdateOutputBuffer.left(newline).trimmed(); + const QByteArray line = cardUpdateOutputBuffer.left(newline); cardUpdateOutputBuffer.remove(0, newline + 1); - // Protocol emitted by `oracle -b`: "PROGRESS " - if (!line.startsWith("PROGRESS ")) { + // Protocol emitted by `oracle`: "PROGRESS " + const std::optional progress = CardUpdateProgress::fromProtocolLine(line); + if (!progress.has_value()) { continue; } - const QList parts = line.split(' '); - if (parts.size() != 4) { - continue; + // Log stage changes and, while a stage runs, one line per second at most, so the + // debug log shows liveness without flooding it. + if (progress->stage != cardUpdateLoggedStage || cardUpdateProgressLogTimer.elapsed() >= 1000) { + qCDebug(CardDatabaseUpdateLog) + << "Card database update" << progress->stageToken() << progress->done << "/" << progress->total; + cardUpdateLoggedStage = progress->stage; + cardUpdateProgressLogTimer.restart(); } - bool doneOk = false; - bool totalOk = false; - const qint64 done = parts.at(2).toLongLong(&doneOk); - const qint64 total = parts.at(3).toLongLong(&totalOk); - if (!doneOk || !totalOk || done < 0 || total < 0) { - continue; + emit cardDatabaseUpdateProgress(progress.value()); + } +} + +void MainWindow::cardUpdateStandardError() +{ + if (!cardUpdateProcess) { + return; + } + const QByteArray chunk = cardUpdateProcess->readAllStandardError(); + if (chunk.isEmpty()) { + return; + } + cardUpdateErrorOutput.append(chunk); + cardUpdateErrorPartial.append(chunk); + int newline; + while ((newline = cardUpdateErrorPartial.indexOf('\n')) >= 0) { + const QByteArray line = cardUpdateErrorPartial.left(newline).trimmed(); + cardUpdateErrorPartial.remove(0, newline + 1); + if (!line.isEmpty()) { + qCWarning(CardDatabaseUpdateLog) << "Card database updater:" << line; } - emit cardDatabaseUpdateProgress(QString::fromLatin1(parts.at(1)), done, total); } } @@ -1240,7 +1300,6 @@ void MainWindow::cardUpdateError(QProcess::ProcessError err) break; case QProcess::Crashed: error = tr("The process crashed some time after starting successfully."); - error += "\n\nError output:\n" + cardUpdateProcess->readAllStandardError(); break; case QProcess::Timedout: error = tr("Timed out. The process took too long to respond. The last waitFor...() function timed out."); @@ -1259,6 +1318,12 @@ void MainWindow::cardUpdateError(QProcess::ProcessError err) break; } + if (!cardUpdateErrorOutput.isEmpty()) { + error += "\n\n" + tr("Error output:") + "\n" + QString::fromUtf8(cardUpdateErrorOutput); + cardUpdateErrorOutput.clear(); + } + + qCWarning(CardDatabaseUpdateLog) << "The card database updater exited with an error:" << error; exitCardDatabaseUpdate(); QMessageBox::warning(this, tr("Error"), tr("The card database updater exited with an error:\n%1").arg(error)); emit cardDatabaseUpdateFinished(false); @@ -1266,16 +1331,56 @@ void MainWindow::cardUpdateError(QProcess::ProcessError err) void MainWindow::cardUpdateFinished(int exitCode, QProcess::ExitStatus exitStatus) { + if (!cardUpdateProcess) { + return; + } cardUpdateProgressOutput(); // drain any progress lines not yet parsed const bool success = (exitStatus == QProcess::NormalExit) && (exitCode == 0); if (exitStatus == QProcess::NormalExit) { SettingsCache::instance().updates().setLastCardUpdateCheck(QDateTime::currentDateTime().date()); } + qCInfo(CardDatabaseUpdateLog) << "The card database updater finished with exit code" << exitCode << "exit status" + << static_cast(exitStatus) << "success:" << success; exitCardDatabaseUpdate(); + notifyCardUpdateFinishedFromTray(success); emit cardDatabaseUpdateFinished(success); } +bool MainWindow::requestCardUpdateRaise() +{ + const QString serverName = scopedLocalServerName(QStringLiteral("CockatriceOracleRaise")); + QLocalSocket socket; + socket.connectToServer(serverName); + if (!socket.waitForConnected(250)) { + return false; + } + socket.write("RAISE\n"); + const bool acked = socket.waitForBytesWritten(250); + socket.disconnectFromServer(); + return acked; +} + +void MainWindow::announceCardUpdateRunning() +{ + if (SettingsCache::instance().userInterface().getShowStatusBar()) { + statusBar()->showMessage(tr("Card database update running.")); + } else if (trayIcon && SettingsCache::instance().updates().getCardDatabaseUpdateTrayNotification()) { + trayIcon->showMessage(tr("Card database update running"), + tr("Cockatrice is updating the card database in the background.")); + } +} + +void MainWindow::notifyCardUpdateFinishedFromTray(bool success) +{ + if (SettingsCache::instance().userInterface().getShowStatusBar() || !trayIcon || + !SettingsCache::instance().updates().getCardDatabaseUpdateTrayNotification()) { + return; + } + trayIcon->showMessage(tr("Card database update finished"), + success ? tr("The card database is up to date.") : tr("The card database update failed.")); +} + void MainWindow::actCheckServerUpdates() { auto hps = new HandlePublicServers(this); diff --git a/cockatrice/src/interface/window_main.h b/cockatrice/src/interface/window_main.h index 5223332a0..bf68ed72a 100644 --- a/cockatrice/src/interface/window_main.h +++ b/cockatrice/src/interface/window_main.h @@ -27,10 +27,12 @@ #include "../client/lag_monitor.h" #include "../client/network/update/client/release_channel.h" +#include "card_database_update/card_update_progress.h" #include "connection_controller/remote_connection_controller.h" #include #include +#include #include #include #include @@ -52,11 +54,13 @@ inline Q_LOGGING_CATEGORY(WindowMainStartupLog, "window_main.startup"); inline Q_LOGGING_CATEGORY(WindowMainStartupVersionLog, "window_main.startup.version"); inline Q_LOGGING_CATEGORY(WindowMainStartupShortcutsLog, "window_main.startup.shortcuts"); inline Q_LOGGING_CATEGORY(WindowMainStartupAutoconnectLog, "window_main.startup.autoconnect"); +inline Q_LOGGING_CATEGORY(CardDatabaseUpdateLog, "card_database.update"); class DlgViewLog; class GameReplay; class LocalServer; class LatencyStatusWidget; +class CardDatabaseUpdateStatusBar; class RemoteClient; class TabSupervisor; class WndSets; @@ -68,13 +72,13 @@ class MainWindow : public QMainWindow { Q_OBJECT signals: - /** @brief Emitted after the background card-database update subprocess exits. */ + /** @brief Emitted after the card-database update subprocess exits. */ void cardDatabaseUpdateFinished(bool success); - /** @brief Emitted while the background card-database update subprocess runs. - * @p stage is one of "download", "scan" or "import"; @p done/@p total - * are byte counts for the first two stages and set indices for "import". */ - void cardDatabaseUpdateProgress(const QString &stage, qint64 done, qint64 total); + /** @brief Emitted while the card-database update subprocess runs. + * @p progress.done/@p progress.total are byte counts for the first two stages + * and set indices for "import". */ + void cardDatabaseUpdateProgress(const CardUpdateProgress &progress); public slots: void actCheckCardUpdates(); @@ -106,6 +110,7 @@ private slots: void cardUpdateError(QProcess::ProcessError err); void cardUpdateFinished(int exitCode, QProcess::ExitStatus exitStatus); void cardUpdateProgressOutput(); + void cardUpdateStandardError(); void refreshShortcuts(); void cardDatabaseLoadingFailed(); void cardDatabaseNewSetsFound(int numUnknownSets, QStringList unknownSetsNames); @@ -150,6 +155,15 @@ private: } void createCardUpdateProcess(bool background = false); void exitCardDatabaseUpdate(); + /** + * @brief Tries to bring the open card database updater window to the front. + * @return true when the updater acknowledged the request + */ + bool requestCardUpdateRaise(); + /** @brief Tells the user an update is running without blocking, honoring a hidden status bar. */ + void announceCardUpdateRunning(); + /** @brief Posts a tray notification when the status bar cannot show progress. */ + void notifyCardUpdateFinishedFromTray(bool success); void startLocalGame(const LocalGameOptions &options); @@ -176,6 +190,11 @@ private: bool firstRunWizardActive = false; QProcess *cardUpdateProcess; QByteArray cardUpdateOutputBuffer; + QByteArray cardUpdateErrorOutput; ///< diagnostic output collected from the updater's stderr + QByteArray cardUpdateErrorPartial; ///< incomplete trailing stderr line, kept for line-based logging + CardUpdateStage cardUpdateLoggedStage = CardUpdateStage::Unknown; ///< stage of the last logged progress update + QElapsedTimer cardUpdateProgressLogTimer; ///< throttles progress logging to the running stage + CardDatabaseUpdateStatusBar *cardUpdateStatusBar = nullptr; ///< status bar widget tracking the card database update DlgViewLog *logviewDialog; GameReplay *replay; DlgTipOfTheDay *tip; diff --git a/cockatrice/src/single_instance_manager.cpp b/cockatrice/src/single_instance_manager.cpp index 89fc6b578..08294d72c 100644 --- a/cockatrice/src/single_instance_manager.cpp +++ b/cockatrice/src/single_instance_manager.cpp @@ -2,15 +2,14 @@ #include #include -#include #include #include #include #include #include #include -#include #include +#include namespace { @@ -26,17 +25,9 @@ SingleInstanceManager::SingleInstanceManager(QObject *parent) : QObject(parent) bool SingleInstanceManager::tryRun(const QStringList &filesToSend) { - // Scope the socket name to the current user. On Linux the default abstract - // namespace is system-wide, so a plain name would let one user's instance + // Scope the socket name to the current user so one user's instance cannot // hijack another user's session. - QString userName = qEnvironmentVariable("USER"); - if (userName.isEmpty()) { - userName = qEnvironmentVariable("USERNAME"); - } - if (userName.isEmpty()) { - userName = QDir::home().dirName(); - } - serverName = QStringLiteral("CockatriceSingleInstance-%1").arg(userName); + serverName = scopedLocalServerName(QStringLiteral("CockatriceSingleInstance")); // Hand off to an already-running primary instance if one exists. Never steal // the socket of a busy primary: it is alive and will act on the payload. diff --git a/doc/doxygen/extra-pages/user_documentation/troubleshooting/enabling_debug_logs.md b/doc/doxygen/extra-pages/user_documentation/troubleshooting/enabling_debug_logs.md index 7e4196a50..27c4272f5 100644 --- a/doc/doxygen/extra-pages/user_documentation/troubleshooting/enabling_debug_logs.md +++ b/doc/doxygen/extra-pages/user_documentation/troubleshooting/enabling_debug_logs.md @@ -110,6 +110,7 @@ suffix, e.g. `card_picture_loader.*`, to include all sub categories). | ------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------ | | Card picture loading (URLs, local file hits/misses, downloads, redirects) | `card_picture_loader.*` | | Card database loading and parsing | `card_database`, `card_database.loading`, `card_database.loading.success_or_failure`, `cockatrice_xml.*` | +| Card database update (running Oracle, progress, exit status, errors) | `card_database.update` | | Card, set, and deck information | `card_info`, `card_list`, `deck_loader` | | Startup sequence and update checks | `window_main.startup.*`, `release_channel`, `spoiler_background_updater` | | User interface and themes | `theme_manager`, `sound_engine`, `flow_layout`, `flow_widget.*`, `pixel_map_generator`, `card_info_picture_widget` | @@ -128,6 +129,7 @@ For example, to investigate why a card database update seems to fail, enable the card_database = true card_database.loading = true card_database.loading.success_or_failure = true +card_database.update = true cockatrice_xml.* = true ``` diff --git a/libcockatrice_settings/libcockatrice/settings/updates_settings.cpp b/libcockatrice_settings/libcockatrice/settings/updates_settings.cpp index f1d638a99..9ea22ed7c 100644 --- a/libcockatrice_settings/libcockatrice/settings/updates_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/updates_settings.cpp @@ -60,6 +60,11 @@ bool UpdatesSettings::getNotifyAboutNewVersion() const return getValue("newVersionNotification", QString(), QString(), true).toBool(); } +bool UpdatesSettings::getCardDatabaseUpdateTrayNotification() const +{ + return getValue("cardDatabaseUpdateTrayNotification", QString(), QString(), true).toBool(); +} + int UpdatesSettings::getUpdateReleaseChannelIndex() const { return getValue("updateReleaseChannel", QString(), QString(), 0).toInt(); @@ -105,6 +110,11 @@ void UpdatesSettings::setNotifyAboutNewVersion(bool _notifyaboutnewversion) setValue(_notifyaboutnewversion, "newVersionNotification"); } +void UpdatesSettings::setCardDatabaseUpdateTrayNotification(bool value) +{ + setValue(value, "cardDatabaseUpdateTrayNotification"); +} + void UpdatesSettings::setUpdateReleaseChannelIndex(int value) { setValue(value, "updateReleaseChannel"); diff --git a/libcockatrice_settings/libcockatrice/settings/updates_settings.h b/libcockatrice_settings/libcockatrice/settings/updates_settings.h index 76bb07e86..490dca14e 100644 --- a/libcockatrice_settings/libcockatrice/settings/updates_settings.h +++ b/libcockatrice_settings/libcockatrice/settings/updates_settings.h @@ -25,6 +25,7 @@ public: [[nodiscard]] bool getAlwaysEnableNewSets() const override; [[nodiscard]] bool getNotifyAboutUpdates() const override; [[nodiscard]] bool getNotifyAboutNewVersion() const override; + [[nodiscard]] bool getCardDatabaseUpdateTrayNotification() const; [[nodiscard]] int getUpdateReleaseChannelIndex() const override; void setCheckUpdatesOnStartup(bool value); @@ -35,6 +36,7 @@ public: void setAlwaysEnableNewSets(bool value); void setNotifyAboutUpdates(bool _notifyaboutupdate); void setNotifyAboutNewVersion(bool _notifyaboutnewversion); + void setCardDatabaseUpdateTrayNotification(bool value); void setUpdateReleaseChannelIndex(int value); public: diff --git a/libcockatrice_utility/CMakeLists.txt b/libcockatrice_utility/CMakeLists.txt index 41f1bfa0f..d986214d7 100644 --- a/libcockatrice_utility/CMakeLists.txt +++ b/libcockatrice_utility/CMakeLists.txt @@ -22,6 +22,7 @@ set(UTILITY_HEADERS libcockatrice/utility/cryptoutil.h libcockatrice/utility/expression.h libcockatrice/utility/levenshtein.h + libcockatrice/utility/local_server_name.h libcockatrice/utility/macros.h libcockatrice/utility/passwordhasher.h libcockatrice/utility/playmat_params.h diff --git a/libcockatrice_utility/libcockatrice/utility/local_server_name.h b/libcockatrice_utility/libcockatrice/utility/local_server_name.h new file mode 100644 index 000000000..a5797fba1 --- /dev/null +++ b/libcockatrice_utility/libcockatrice/utility/local_server_name.h @@ -0,0 +1,28 @@ +#ifndef COCKATRICE_LOCAL_SERVER_NAME_H +#define COCKATRICE_LOCAL_SERVER_NAME_H + +#include +#include +#include + +/** + * @brief Builds a QLocalServer/QLocalSocket name scoped to the current user. + * + * On Linux the default local socket namespace is system-wide, so an unscoped name + * would let one user's process talk to — or hijack — another user's session. + * @param prefix application specific socket name prefix + * @return the scoped socket name + */ +inline QString scopedLocalServerName(const QString &prefix) +{ + QString userName = qEnvironmentVariable("USER"); + if (userName.isEmpty()) { + userName = qEnvironmentVariable("USERNAME"); + } + if (userName.isEmpty()) { + userName = QDir::home().dirName(); + } + return prefix + QLatin1Char('-') + userName; +} + +#endif // COCKATRICE_LOCAL_SERVER_NAME_H diff --git a/oracle/src/oracleimporter.h b/oracle/src/oracleimporter.h index 4057144c1..d872e184e 100644 --- a/oracle/src/oracleimporter.h +++ b/oracle/src/oracleimporter.h @@ -185,9 +185,10 @@ private: /** * Whether readSetsFromByteArray() should report scan progress via - * dataReadProgress. A background run routes that signal to stdout (for the - * hosting Cockatrice client to parse); the flag exists to skip the scanner - * instrumentation entirely when no consumer needs it. + * dataReadProgress. The wizard streams that signal to stdout (for the + * hosting Cockatrice client to parse) in both interactive and background + * runs; the flag exists to skip the scanner instrumentation entirely when + * no consumer needs it. */ bool progressReporting = true; @@ -255,9 +256,9 @@ public: * @brief Controls whether readSetsFromByteArray() instruments the raw scan. * * When enabled (the default) the raw scanner reports progress via - * dataReadProgress(), which an interactive wizard shows on its progress bar - * and a background run routes to stdout for the hosting client. Switch it - * off only when nothing will consume scan progress. + * dataReadProgress(), which the wizard streams to stdout (so the hosting + * Cockatrice client can show it) and also displays on its own progress bar. + * Switch it off only when nothing will consume scan progress. */ void setProgressReporting(bool enabled) { diff --git a/oracle/src/oraclewizard.cpp b/oracle/src/oraclewizard.cpp index b92feae8f..05f08ce64 100644 --- a/oracle/src/oraclewizard.cpp +++ b/oracle/src/oraclewizard.cpp @@ -14,13 +14,17 @@ #include #include #include +#include +#include #include #include #include +#include #include #include #include #include +#include #include class QWidget; @@ -71,6 +75,7 @@ OracleWizard::OracleWizard(QWidget *parent) : QWizard(parent) } retranslateUi(); + startRaiseServer(); } /** @@ -139,6 +144,54 @@ void OracleWizard::runInBackground() currentPage()->initializePage(); } +void OracleWizard::startRaiseServer() +{ + // Scope the socket name to the current user, mirroring the hosting client. + raiseServer = new QLocalServer(this); + connect(raiseServer, &QLocalServer::newConnection, this, &OracleWizard::handleRaiseRequest); + const QString serverName = scopedLocalServerName(QStringLiteral("CockatriceOracleRaise")); + if (raiseServer->listen(serverName)) { + return; + } + // A stale socket left behind by a crashed run holds the name: take it back. + QLocalServer::removeServer(serverName); + if (!raiseServer->listen(serverName)) { + qWarning() << "Oracle: could not listen for raise requests on" << serverName; + delete raiseServer; + raiseServer = nullptr; + } +} + +void OracleWizard::handleRaiseRequest() +{ + while (raiseServer && raiseServer->hasPendingConnections()) { + QLocalSocket *socket = raiseServer->nextPendingConnection(); + connect(socket, &QLocalSocket::readyRead, this, [this, socket]() { + socket->readAll(); + socket->deleteLater(); + raiseWizard(); + }); + connect(socket, &QLocalSocket::disconnected, socket, &QLocalSocket::deleteLater); + } + raiseWizard(); +} + +void OracleWizard::raiseWizard() +{ + // An ongoing background run is hidden; the user asked to see it, so promote it to a window. + if (backgroundMode && !isVisible()) { + show(); + } + if (isMinimized()) { + showNormal(); + } + raise(); + activateWindow(); + if (windowHandle()) { + windowHandle()->requestActivate(); + } +} + void OracleWizard::enableButtons() { button(QWizard::NextButton)->setDisabled(false); diff --git a/oracle/src/oraclewizard.h b/oracle/src/oraclewizard.h index 2dfcc2cdc..ea652bdc5 100644 --- a/oracle/src/oraclewizard.h +++ b/oracle/src/oraclewizard.h @@ -9,6 +9,7 @@ class QNetworkAccessManager; class OracleImporter; +class QLocalServer; class QSettings; class QWidget; @@ -60,6 +61,7 @@ public: private slots: void updateLanguage(); + void handleRaiseRequest(); private: QByteArray tokensData; @@ -67,6 +69,13 @@ private: QString cardSourceVersion; void migrateOracleSettings(); + /** + * @brief Starts the local socket the hosting Cockatrice client uses to bring this wizard forward. + */ + void startRaiseServer(); + /** @brief Shows, raises and activates the wizard window. */ + void raiseWizard(); + QLocalServer *raiseServer = nullptr; ///< listens for raise requests from the hosting client protected: void changeEvent(QEvent *event) override; diff --git a/oracle/src/pages.cpp b/oracle/src/pages.cpp index 43c863495..8c286fb46 100644 --- a/oracle/src/pages.cpp +++ b/oracle/src/pages.cpp @@ -82,13 +82,12 @@ class QWidget; #endif /** - * @brief Emits one machine-readable background-run progress line to stdout. + * @brief Emits one machine-readable progress line to stdout. * - * Used only in background mode, so the hosting Cockatrice client can parse these - * lines to drive a determinate progress bar. stderr stays reserved for - * human-readable log output. + * The hosting Cockatrice client parses these lines to drive a determinate + * progress bar. stderr stays reserved for human-readable log output. */ -static void emitBackgroundProgress(const char *stage, qint64 done, qint64 total) +static void emitProgress(const char *stage, qint64 done, qint64 total) { QTextStream out(stdout); out << "PROGRESS " << stage << ' ' << done << ' ' << total << '\n'; @@ -473,9 +472,7 @@ void LoadSetsPage::downloadSetsFile(const QUrl &url) void LoadSetsPage::actDownloadProgressSetsFile(qint64 received, qint64 total) { - if (wizard()->backgroundMode) { - emitBackgroundProgress("download", received, total); - } + emitProgress("download", received, total); if (total > 0) { progressBar->setMaximum(static_cast(total)); progressBar->setValue(static_cast(received)); @@ -534,7 +531,7 @@ void LoadSetsPage::updateParsingProgress(int bytesRead, int totalBytes) void LoadSetsPage::scanProgressToStdout(int bytesRead, int totalBytes) { - emitBackgroundProgress("scan", bytesRead, totalBytes); + emitProgress("scan", bytesRead, totalBytes); } void LoadSetsPage::beginLoadSets(bool compressedFile) @@ -841,18 +838,18 @@ void SaveSetsPage::updateTotalProgress(int cardsImported, int setIndex, const QS const int cardCount = wizard()->importer->getCardList().size(); if (wizard()->backgroundMode) { qInfo() << tr("Import finished: %1 cards.").arg(cardCount); - emitBackgroundProgress("import", totalSets, totalSets); } else { messageLog->append("" + tr("Import finished: %1 cards.").arg(cardCount) + ""); } + emitProgress("import", totalSets, totalSets); } else { progressBar->setValue(setIndex); if (wizard()->backgroundMode) { qInfo() << tr("%1: %2 cards imported").arg(setName).arg(cardsImported); - emitBackgroundProgress("import", setIndex, totalSets); } else { messageLog->append(tr("%1: %2 cards imported").arg(setName).arg(cardsImported)); } + emitProgress("import", setIndex, totalSets); } messageLog->verticalScrollBar()->setValue(messageLog->verticalScrollBar()->maximum()); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b51d10560..2c288c6ce 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -22,6 +22,7 @@ add_test(NAME report_categories_test COMMAND report_categories_test) add_test(NAME lag_monitor_test COMMAND lag_monitor_test) add_test(NAME latency_tracker_test COMMAND latency_tracker_test) add_test(NAME metrics_registry_test COMMAND metrics_registry_test) +add_test(NAME card_update_progress_test COMMAND card_update_progress_test) add_test(NAME loader_local_matching_test COMMAND loader_local_matching_test) add_test(NAME deck_hash_performance_test COMMAND deck_hash_performance_test) @@ -75,6 +76,11 @@ add_executable(lag_monitor_test ${CMAKE_SOURCE_DIR}/cockatrice/src/client/lag_mo target_include_directories(lag_monitor_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src) add_executable(latency_tracker_test latency_tracker_test.cpp) add_executable(metrics_registry_test ../servatrice/src/metrics_registry.cpp metrics_registry_test.cpp) +add_executable( + card_update_progress_test ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/card_database_update/card_update_progress.cpp + card_update_progress_test.cpp +) +target_include_directories(card_update_progress_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src) add_executable( loader_local_matching_test ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/card_picture_loader/card_picture_loader_local.cpp @@ -94,6 +100,7 @@ if(WITH_CLIENT) single_instance_manager_test.cpp ) target_include_directories(single_instance_manager_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src) + target_include_directories(single_instance_manager_test PRIVATE ${CMAKE_SOURCE_DIR}/libcockatrice_utility) add_test(NAME single_instance_manager_test COMMAND single_instance_manager_test) endif() @@ -143,6 +150,7 @@ if(NOT GTEST_FOUND) add_dependencies(lag_monitor_test gtest) add_dependencies(latency_tracker_test gtest) add_dependencies(metrics_registry_test gtest) + add_dependencies(card_update_progress_test gtest) add_dependencies(loader_local_matching_test gtest) add_dependencies(single_instance_manager_test gtest) endif() @@ -201,6 +209,7 @@ target_link_libraries( ) target_include_directories(metrics_registry_test PRIVATE ${CMAKE_SOURCE_DIR}/servatrice/src) target_link_libraries(metrics_registry_test ${TEST_QT_MODULES} Threads::Threads ${GTEST_BOTH_LIBRARIES}) +target_link_libraries(card_update_progress_test Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}) target_link_libraries( loader_local_matching_test libcockatrice_settings Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES} ) diff --git a/tests/card_update_progress_test.cpp b/tests/card_update_progress_test.cpp new file mode 100644 index 000000000..849cf623c --- /dev/null +++ b/tests/card_update_progress_test.cpp @@ -0,0 +1,93 @@ +#include "interface/card_database_update/card_update_progress.h" + +#include +#include +#include + +namespace +{ +std::optional parseLine(const QByteArray &line) +{ + return CardUpdateProgress::fromProtocolLine(line); +} +} // namespace + +TEST(CardUpdateProgressTest, ParsesAValidDownloadLine) +{ + const std::optional progress = parseLine("PROGRESS download 42 100"); + ASSERT_TRUE(progress.has_value()); + EXPECT_EQ(CardUpdateStage::Download, progress->stage); + EXPECT_EQ(42, progress->done); + EXPECT_EQ(100, progress->total); +} + +TEST(CardUpdateProgressTest, ParsesTrailingNewline) +{ + const std::optional progress = parseLine("PROGRESS scan 7 99\n"); + ASSERT_TRUE(progress.has_value()); + EXPECT_EQ(CardUpdateStage::Scan, progress->stage); + EXPECT_EQ(7, progress->done); + EXPECT_EQ(99, progress->total); +} + +TEST(CardUpdateProgressTest, ParsesWithSurroundingWhitespace) +{ + const std::optional progress = parseLine(QByteArray(" PROGRESS import 3 10 ")); + ASSERT_TRUE(progress.has_value()); + EXPECT_EQ(CardUpdateStage::Import, progress->stage); + EXPECT_EQ(3, progress->done); + EXPECT_EQ(10, progress->total); +} + +TEST(CardUpdateProgressTest, AcceptsZeroProgress) +{ + const std::optional progress = parseLine("PROGRESS download 0 0"); + ASSERT_TRUE(progress.has_value()); + EXPECT_EQ(0, progress->done); + EXPECT_EQ(0, progress->total); +} + +TEST(CardUpdateProgressTest, RejectsLinesWithoutProgressPrefix) +{ + EXPECT_FALSE(parseLine("download 42 100").has_value()); + EXPECT_FALSE(parseLine("PROGRESSdownload 42 100").has_value()); + EXPECT_FALSE(parseLine("").has_value()); +} + +TEST(CardUpdateProgressTest, RejectsMalformedLines) +{ + EXPECT_FALSE(parseLine("PROGRESS").has_value()); + EXPECT_FALSE(parseLine("PROGRESS download").has_value()); + EXPECT_FALSE(parseLine("PROGRESS download 42 100 extra").has_value()); + EXPECT_FALSE(parseLine("PROGRESS download 42").has_value()); + EXPECT_FALSE(parseLine("PROGRESS download notANumber 100").has_value()); +} + +TEST(CardUpdateProgressTest, RejectsNegativeValues) +{ + EXPECT_FALSE(parseLine("PROGRESS download -1 100").has_value()); + EXPECT_FALSE(parseLine("PROGRESS download 42 -100").has_value()); +} + +TEST(CardUpdateProgressTest, AcceptsUnknownStagesSchemalessly) +{ + const std::optional progress = parseLine("PROGRESS spoilers 1 2"); + ASSERT_TRUE(progress.has_value()); + EXPECT_EQ(CardUpdateStage::Unknown, progress->stage); + EXPECT_EQ(1, progress->done); + EXPECT_EQ(2, progress->total); +} + +TEST(CardUpdateProgressTest, StageTokensRoundTrip) +{ + EXPECT_EQ("download", CardUpdateProgress{CardUpdateStage::Download}.stageToken()); + EXPECT_EQ("scan", CardUpdateProgress{CardUpdateStage::Scan}.stageToken()); + EXPECT_EQ("import", CardUpdateProgress{CardUpdateStage::Import}.stageToken()); + EXPECT_EQ("unknown", CardUpdateProgress{CardUpdateStage::Unknown}.stageToken()); +} + +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} \ No newline at end of file diff --git a/tests/settings/settings_defaults_test.cpp b/tests/settings/settings_defaults_test.cpp index 03de21e8a..65e41af76 100644 --- a/tests/settings/settings_defaults_test.cpp +++ b/tests/settings/settings_defaults_test.cpp @@ -166,6 +166,12 @@ TEST_F(SettingsDefaultsTest, Updates_NotifyAboutNewVersion_Default) ASSERT_EQ(s.getNotifyAboutNewVersion(), true); } +TEST_F(SettingsDefaultsTest, Updates_CardDatabaseUpdateTrayNotification_Default) +{ + UpdatesSettings s(settingsPath, nullptr); + ASSERT_EQ(s.getCardDatabaseUpdateTrayNotification(), true); +} + TEST_F(SettingsDefaultsTest, Updates_UpdateReleaseChannelIndex_Default) { UpdatesSettings s(settingsPath, nullptr);