From 6bed5d6f67f82fc5dfe5c863def8e9e6d5b393a8 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Fri, 9 Oct 2026 11:07:22 +0200 Subject: [PATCH 1/3] [Style] Warn about the broken Windows native styles and stop resolving System to them (#7461) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Style] Fall back to the built-in windows style instead of windowsvista The Windows Vista native style is broken the same way as the Windows 11 one, so the sanitizer now catches both and falls back to the built-in QWindowsStyle instead of the Vista plugin. The style picker no longer offers either. * [Style] Keep Windows native styles selectable but warn about them --------- Co-authored-by: Lukas Brübach --- cockatrice/src/interface/theme_manager.cpp | 22 +++++++------ .../appearance_settings_page.cpp | 31 ++++++++++++++----- .../settings_page/appearance_settings_page.h | 2 ++ 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/cockatrice/src/interface/theme_manager.cpp b/cockatrice/src/interface/theme_manager.cpp index cbf869639..9a96b3e45 100644 --- a/cockatrice/src/interface/theme_manager.cpp +++ b/cockatrice/src/interface/theme_manager.cpp @@ -111,12 +111,17 @@ struct PaletteColorInfo static QString usableDefaultStyle(const QString &style) { - // The Windows 11 native style is broken: dragging cards across zones can + // Only the OS/default ("System") style is sanitized: the Windows 11 and + // Windows Vista native styles are broken (dragging cards across zones can // shrink the board to a tiny grey window that is unfixable without - // rejoining. It is never usable, so guard against it no matter how it was - // requested (OS default or an explicit "windows11" theme choice) and fall - // back to the Vista style. - return style.compare("windows11", Qt::CaseInsensitive) == 0 ? QStringLiteral("windowsvista") : style; + // rejoining), so a "System" choice falls through to the built-in "windows" + // style (QWindowsStyle) instead. They remain selectable by hand, where the + // settings page warns about them. + if (style.compare("windows11", Qt::CaseInsensitive) == 0 || + style.compare("windowsvista", Qt::CaseInsensitive) == 0) { + return QStringLiteral("windows"); + } + return style; } ThemeManager::ThemeManager(QObject *parent) : QObject(parent) @@ -496,14 +501,13 @@ void ThemeManager::applyStyleAndPalette(const QString &themeName, if (themeName == FUSION_THEME_NAME) { styleName = "Fusion"; } else { + // "System" resolves through the guard, which steers the broken + // Windows native styles to "windows". An explicitly chosen style + // name is honored as-is. styleName = usableDefaultStyle(defaultStyleName); } } - // The Windows 11 style is broken even when selected explicitly in a theme, - // so sanitize the resolved name here rather than trusting the theme config. - styleName = usableDefaultStyle(styleName); - QStyle *style = QStyleFactory::create(styleName); if (!style) { style = QStyleFactory::create(usableDefaultStyle(defaultStyleName)); diff --git a/cockatrice/src/interface/widgets/settings_page/appearance_settings_page.cpp b/cockatrice/src/interface/widgets/settings_page/appearance_settings_page.cpp index 159db767d..4c38459c6 100644 --- a/cockatrice/src/interface/widgets/settings_page/appearance_settings_page.cpp +++ b/cockatrice/src/interface/widgets/settings_page/appearance_settings_page.cpp @@ -74,15 +74,10 @@ AppearanceSettingsPage::AppearanceSettingsPage() connect(&schemeCombo, &QComboBox::currentIndexChanged, this, [this] { themeManager->setColorScheme(schemeCombo.currentData().toString()); }); - // Qt widget style; "System" lets the application decide + // Qt widget style; "System" lets the application decide. The broken Windows + // native styles are still offered, but selecting one raises a warning. styleCombo.addItem(tr("System"), QStringLiteral("System")); for (const QString &key : QStyleFactory::keys()) { - // The Windows 11 native style is broken (board rendering glitches when - // moving cards), so never offer it; it is already sanitized at apply - // time in ThemeManager. - if (key.compare("windows11", Qt::CaseInsensitive) == 0) { - continue; - } styleCombo.addItem(key, key); } @@ -92,6 +87,7 @@ AppearanceSettingsPage::AppearanceSettingsPage() connect(&styleCombo, &QComboBox::currentIndexChanged, this, [this] { themeManager->setStyleName(styleCombo.currentData().toString()); }); + connect(&styleCombo, &QComboBox::currentIndexChanged, this, &AppearanceSettingsPage::updateStyleWarning); connect(themeManager, &ThemeManager::themeChanged, this, [this, dirPath] { const QString newDir = themeManager->getAvailableThemes().value(SettingsCache::instance().getThemeName()); @@ -108,6 +104,8 @@ AppearanceSettingsPage::AppearanceSettingsPage() const int styleIdx = currentStyle.isEmpty() ? 0 : styleCombo.findData(currentStyle); styleCombo.setCurrentIndex(styleIdx >= 0 ? styleIdx : 0); styleCombo.blockSignals(false); + + updateStyleWarning(); }); connect(&editPaletteButton, &QPushButton::clicked, this, &AppearanceSettingsPage::editPalette); @@ -120,7 +118,11 @@ AppearanceSettingsPage::AppearanceSettingsPage() themeGrid->addWidget(&schemeCombo, 2, 1); themeGrid->addWidget(&styleComboLabel, 3, 0); themeGrid->addWidget(&styleCombo, 3, 1); - themeGrid->addWidget(&editPaletteButton, 4, 1); + themeGrid->addWidget(&styleWarningLabel, 4, 0, 1, 2); + themeGrid->addWidget(&editPaletteButton, 5, 1); + + styleWarningLabel.setWordWrap(true); + styleWarningLabel.setAlignment(Qt::AlignLeft | Qt::AlignVCenter); themeGroupBox = new QGroupBox; themeGroupBox->setLayout(themeGrid); @@ -465,6 +467,14 @@ void AppearanceSettingsPage::updateHomeTabSettingsVisibility() homeTabDisplayCardNameCheckBox.setVisible(visible); } +void AppearanceSettingsPage::updateStyleWarning() +{ + const QString style = styleCombo.currentData().toString(); + const bool broken = + style.compare("windows11", Qt::CaseInsensitive) == 0 || style.compare("windowsvista", Qt::CaseInsensitive) == 0; + styleWarningLabel.setVisible(broken); +} + void AppearanceSettingsPage::showShortcutsChanged(QT_STATE_CHANGED_T value) { SettingsCache::instance().userInterface().setShowShortcuts(value); @@ -527,6 +537,11 @@ void AppearanceSettingsPage::retranslateUi() schemeComboLabel.setText(tr("Active theme palette:")); styleComboLabel.setText(tr("Active theme style:")); styleCombo.setToolTip(tr("Qt widget style saved to this theme (\"System\" lets the application decide)")); + styleWarningLabel.setText( + tr("Warning: the Windows 11 and Windows Vista styles are buggy and can render controls such as combo boxes " + "unreadable, especially in dark mode. Fusion is the recommended style on every platform: it is Qt's " + "cross-platform style and the one Cockatrice is developed against.")); + updateStyleWarning(); editPaletteButton.setText(tr("Edit theme palette")); homeTabGroupBox->setTitle(tr("Home tab settings")); diff --git a/cockatrice/src/interface/widgets/settings_page/appearance_settings_page.h b/cockatrice/src/interface/widgets/settings_page/appearance_settings_page.h index 096ba0915..8afd326a3 100644 --- a/cockatrice/src/interface/widgets/settings_page/appearance_settings_page.h +++ b/cockatrice/src/interface/widgets/settings_page/appearance_settings_page.h @@ -22,6 +22,7 @@ private slots: void openThemeLocation(); void editPalette(); void updateHomeTabSettingsVisibility(); + void updateStyleWarning(); void showShortcutsChanged(QT_STATE_CHANGED_T enabled); void overrideAllCardArtWithPersonalPreferenceToggled(QT_STATE_CHANGED_T enabled); @@ -37,6 +38,7 @@ private: QComboBox schemeCombo; QLabel styleComboLabel; QComboBox styleCombo; + QLabel styleWarningLabel; QPushButton editPaletteButton; QLabel homeTabBackgroundSourceLabel; 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 2/3] [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); From bb61617d425c61784e474b6c4c4f52094bfc2f2a Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Fri, 9 Oct 2026 11:35:35 +0200 Subject: [PATCH 3/3] [Versioning] Bump version number for 3.1.1 beta release. (#7464) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Lukas Brübach --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4923f9999..bb1fa6174 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -86,7 +86,7 @@ endif() # A project name is needed for CPack # Version can be overriden by git tags, see cmake/getversion.cmake -project("Cockatrice" VERSION 3.1.0) +project("Cockatrice" VERSION 3.1.1) # Set release name if not provided via env/cmake var if(NOT DEFINED GIT_TAG_RELEASENAME)