diff --git a/cockatrice/src/client/crash/crash_reporter.cpp b/cockatrice/src/client/crash/crash_reporter.cpp index 07a0389c7..cba84994c 100644 --- a/cockatrice/src/client/crash/crash_reporter.cpp +++ b/cockatrice/src/client/crash/crash_reporter.cpp @@ -614,3 +614,19 @@ QString CrashReporter::redactCockatriceUrls(const QString &line) redacted.append(line.mid(pos)); return redacted; } + +void CrashReporter::addBreadcrumb(const char *category, const char *message) +{ +#if COCKATRICE_WITH_CRASH_REPORTING + if (category == nullptr || message == nullptr) { + return; + } + sentry_value_t crumb = sentry_value_new_breadcrumb("default", message); + sentry_value_set_by_key(crumb, "category", sentry_value_new_string(category)); + sentry_value_set_by_key(crumb, "level", sentry_value_new_string("info")); + sentry_add_breadcrumb(crumb); +#else + Q_UNUSED(category); + Q_UNUSED(message); +#endif +} diff --git a/cockatrice/src/client/crash/crash_reporter.h b/cockatrice/src/client/crash/crash_reporter.h index aecc6ade3..f06103188 100644 --- a/cockatrice/src/client/crash/crash_reporter.h +++ b/cockatrice/src/client/crash/crash_reporter.h @@ -167,6 +167,17 @@ public: static QString sanitizeLogLine(const QString &line); static QString redactCockatriceUrls(const QString &line); + /** + * @brief Record a fixed-text lifecycle breadcrumb on the active Sentry scope. + * + * Native crashes whose stack walk loses the in-app frames (e.g. a QMap lookup + * that dies inside Qt's string comparison) otherwise arrive with no hint of + * what the client was doing. Only pass compile-time string literals: no user + * names, card names, paths or URLs, so no personal data can leak through this + * channel. Thread-safe; a no-op in OFF builds or before init(). + */ + static void addBreadcrumb(const char *category, const char *message); + private: bool ready = false; QString sentryDatabasePath; diff --git a/cockatrice/src/game_graphics/board/abstract_card_item.cpp b/cockatrice/src/game_graphics/board/abstract_card_item.cpp index f7ae1ff71..f2c0966e4 100644 --- a/cockatrice/src/game_graphics/board/abstract_card_item.cpp +++ b/cockatrice/src/game_graphics/board/abstract_card_item.cpp @@ -48,8 +48,8 @@ class QSizeF; // Width the hovered card smooth-zooms to: exactly the hover panel picture size, so // all four corners coincide when the panel swaps in over it. static constexpr qreal HOVER_ZOOM_WIDTH = 270.0; -// The pre-panel hover enlargement. The smooth zoom is opt-in, so this stays the hover -// behavior whenever no zoom trigger is configured (see setHoverHighlighted). +// The pre-panel hover enlargement. This stays the hover behavior when the smooth zoom is +// turned off (see setHoverHighlighted). static constexpr qreal LEGACY_HOVER_SCALE = 1.1; static constexpr int HOVER_ZOOM_IN_MS = 220; static constexpr int HOVER_ZOOM_OUT_MS = 140; @@ -356,11 +356,9 @@ void AbstractCardItem::setHoverHighlighted(bool highlighted) setZValue(ZValues::HOVERED_CARD); } - // Fallback to the historical hover scale: the smooth hover zoom is opt-in and its - // trigger defaults to Off, so when hovering drives neither the zoom nor the panel - // (zoom trigger Off, panel not on hover) contact alone must still enlarge the card - // to the old 1.1x while scaleCards is on. Otherwise every existing user silently - // loses the hover enlargement on upgrade. + // Fallback to the historical hover scale when the smooth hover zoom is turned off and + // the panel is not driven by hover: contact alone must still enlarge the card to the + // old 1.1x while scaleCards is on. auto &display = SettingsCache::instance().cardsDisplay(); const bool legacyHoverScale = display.getScaleCards() && display.getHoverZoomTrigger() == CardsDisplaySettings::HoverPanelTriggerOff && diff --git a/cockatrice/src/interface/widgets/server/game_selector.cpp b/cockatrice/src/interface/widgets/server/game_selector.cpp index c3bc7499f..775e58297 100644 --- a/cockatrice/src/interface/widgets/server/game_selector.cpp +++ b/cockatrice/src/interface/widgets/server/game_selector.cpp @@ -182,42 +182,35 @@ GameSelector::GameSelector(AbstractClient *_client, hostGameAction->setShortcutContext(Qt::WidgetWithChildrenShortcut); connect(hostGameAction, &QAction::triggered, this, &GameSelector::actQuickHost); - // A plain QPushButton, so the quick host sits in the same control family as the - // join buttons. A QPushButton does not take its text, tooltip or enabled state from - // an action, so those are mirrored by hand below and in updateHostControlsEnabled(). - hostGameButton = new QPushButton(this); - hostGameButton->setObjectName("hostGameButton"); - connect(hostGameButton, &QPushButton::clicked, hostGameAction, &QAction::trigger); - hostGameMenu = new QMenu(this); hostGameMenu->setObjectName("hostGameMenu"); - hostGameMenu->addAction(hostGameAction); gameSettingsAction = hostGameMenu->addAction(QString()); gameSettingsAction->setObjectName("gameSettingsAction"); gameSettingsAction->setShortcutContext(Qt::WidgetWithChildrenShortcut); connect(gameSettingsAction, &QAction::triggered, this, &GameSelector::actCreate); - // The secondary control: a flat arrow next to the primary button on styles that - // support a split control. The native macOS style does not, so there it is a plain - // sibling button instead (see applyHostGameStyle()). - hostGameMenuButton = new QToolButton(this); - hostGameMenuButton->setObjectName("hostGameMenuButton"); - hostGameMenuButton->setArrowType(Qt::DownArrow); - hostGameMenuButton->setAutoRaise(true); - hostGameMenuButton->setPopupMode(QToolButton::InstantPopup); - hostGameMenuButton->setToolButtonStyle(Qt::ToolButtonIconOnly); + // A split QToolButton in its native style: the primary half quick-hosts, and the + // arrow opens the menu with the full form. This is the control on every style that + // can draw one; QMacStyle draws the menu half as a second square bezel instead. + hostGameSplitButton = new QToolButton(this); + hostGameSplitButton->setObjectName("hostGameSplitButton"); + hostGameSplitButton->setDefaultAction(hostGameAction); + hostGameSplitButton->setPopupMode(QToolButton::MenuButtonPopup); + hostGameSplitButton->setToolButtonStyle(Qt::ToolButtonTextOnly); + hostGameSplitButton->setMenu(hostGameMenu); + + // The native macOS style has no split rendering, so there the primary stays a plain + // QPushButton next to a plain settings button. A QPushButton does not take its text, + // tooltip or enabled state from an action, so those are mirrored by hand below and + // in updateHostControlsEnabled(). + hostGameButton = new QPushButton(this); + hostGameButton->setObjectName("hostGameButton"); + connect(hostGameButton, &QPushButton::clicked, hostGameAction, &QAction::trigger); hostGameSettingsButton = new QPushButton(this); hostGameSettingsButton->setObjectName("hostGameSettingsButton"); connect(hostGameSettingsButton, &QPushButton::clicked, gameSettingsAction, &QAction::trigger); - hostGameButtonWidget = new QWidget(this); - auto *hostGameButtonLayout = new QHBoxLayout(hostGameButtonWidget); - hostGameButtonLayout->setContentsMargins(0, 0, 0, 0); - hostGameButtonLayout->setSpacing(0); - hostGameButtonLayout->addWidget(hostGameButton); - hostGameButtonLayout->addWidget(hostGameMenuButton); - applyHostGameStyle(); // The shortcuts belong to the whole room tab, not just the game list, so they also @@ -244,7 +237,9 @@ GameSelector::GameSelector(AbstractClient *_client, } buttonLayout->addStretch(); if (room) { - buttonLayout->addWidget(hostGameButtonWidget); + // Only the form that matches the active style is visible; hidden widgets take no room. + buttonLayout->addWidget(hostGameSplitButton); + buttonLayout->addWidget(hostGameButton); buttonLayout->addWidget(hostGameSettingsButton); } buttonLayout->addWidget(joinButton); @@ -435,23 +430,17 @@ void GameSelector::clearHostError() void GameSelector::applyHostGameStyle() { - if (hostGameButton == nullptr || hostGameMenu == nullptr || hostGameMenuButton == nullptr || - hostGameSettingsButton == nullptr) { + if (hostGameSplitButton == nullptr || hostGameButton == nullptr || hostGameSettingsButton == nullptr) { return; } - // The primary button always quick-hosts. The native macOS style has no split-button - // rendering (a QToolButton menu becomes a second square bezel), so the secondary - // action is a plain sibling button there; every other style gets the split arrow. - if (usesMacStyle()) { - hostGameMenuButton->setMenu(nullptr); - hostGameMenuButton->setVisible(false); - hostGameSettingsButton->setVisible(true); - } else { - hostGameMenuButton->setMenu(hostGameMenu); - hostGameMenuButton->setVisible(true); - hostGameSettingsButton->setVisible(false); - } + // Every style that can draw a split button gets one. The native macOS style cannot + // (QMacStyle draws the menu half as a second square bezel), so it gets the plain + // primary button and a sibling settings button instead. + const bool macStyle = usesMacStyle(); + hostGameSplitButton->setVisible(!macStyle); + hostGameButton->setVisible(macStyle); + hostGameSettingsButton->setVisible(macStyle); // The appearance settings can switch the style while the room is open. updateHostGameToolTip(); @@ -482,8 +471,8 @@ void GameSelector::updateHostControlsEnabled() if (hostGameButton) { hostGameButton->setEnabled(hostControlsEnabled); } - if (hostGameMenuButton) { - hostGameMenuButton->setEnabled(hostControlsEnabled); + if (hostGameSplitButton) { + hostGameSplitButton->setEnabled(hostControlsEnabled); } if (hostGameSettingsButton) { hostGameSettingsButton->setEnabled(hostControlsEnabled); @@ -740,10 +729,10 @@ void GameSelector::retranslateUi() filterButton->setText(tr("&Filter games")); clearFilterButton->setText(tr("C&lear filter")); if (hostGameAction) { - hostGameAction->setText(tr("Host with last settings")); + hostGameAction->setText(tr("Host game")); if (hostGameButton) { - // A QPushButton does not draw text from an action; the button keeps the short - // label while the menu item spells out what the remembered settings do. + // A QPushButton does not draw text from an action; on macOS this button is the + // primary action, so it carries the same label. hostGameButton->setText(tr("Host game")); } updateHostGameToolTip(); @@ -755,11 +744,6 @@ void GameSelector::retranslateUi() // secondary action, so it carries the same label. hostGameSettingsButton->setText(tr("Game settings…")); } - if (hostGameMenuButton) { - // The split arrow has no text of its own; name it for the menu it opens. - hostGameMenuButton->setToolTip(tr("More host options")); - hostGameMenuButton->setAccessibleName(tr("More host options")); - } } joinButton->setText(tr("&Join")); joinAsJudgeButton->setText(tr("Join as judge")); @@ -794,6 +778,9 @@ void GameSelector::updateHostGameToolTip() if (hostGameButton) { hostGameButton->setToolTip(quickHostTooltip); } + if (hostGameSplitButton) { + hostGameSplitButton->setToolTip(quickHostTooltip); + } if (hostGameSettingsButton) { hostGameSettingsButton->setToolTip(tr("Open the game settings before hosting")); } diff --git a/cockatrice/src/interface/widgets/server/game_selector.h b/cockatrice/src/interface/widgets/server/game_selector.h index 72c6fabfe..bf51239f1 100644 --- a/cockatrice/src/interface/widgets/server/game_selector.h +++ b/cockatrice/src/interface/widgets/server/game_selector.h @@ -164,10 +164,9 @@ private: QPushButton *filterButton; /**< Button to open the filter dialog. */ QPushButton *clearFilterButton; /**< Button to clear active filters. */ - QPushButton *hostGameButton = nullptr; /**< Host button, in the same family as the join buttons. */ + QToolButton *hostGameSplitButton = nullptr; /**< Split host button, on every style that can draw one. */ + QPushButton *hostGameButton = nullptr; /**< Primary quick-host button, on the native macOS style only. */ QPushButton *hostGameSettingsButton = nullptr; /**< Opens the host form; a sibling button on macOS. */ - QToolButton *hostGameMenuButton = nullptr; /**< Split arrow that opens the host menu (non-macOS styles only). */ - QWidget *hostGameButtonWidget = nullptr; /**< Container that keeps the split button and its arrow together. */ QMenu *hostGameMenu = nullptr; /**< Menu behind the split arrow (non-macOS styles only). */ QAction *hostGameAction = nullptr; /**< Quick-host action, carrying the keyboard shortcut. */ QAction *gameSettingsAction = nullptr; /**< Opens the host form with the current settings. */ @@ -193,9 +192,9 @@ private: /** * @brief Rebuilds the host control for the active style. * - * The primary button always quick-hosts. Every other style gets the split button, - * whose arrow opens the menu; the native macOS style has no split control, so it - * gets a plain sibling button for the secondary action instead. + * The primary button always quick-hosts. Every style that can draw a split control + * gets one, whose arrow opens the menu; the native macOS style has no split control, + * so it gets a plain sibling button for the secondary action instead. */ void applyHostGameStyle(); /** @brief Marks a quick-host command as in flight or finished, then updates the host controls. */ diff --git a/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp b/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp index 943b4d5de..b7f4614c9 100644 --- a/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp @@ -1,5 +1,6 @@ #include "user_list_widget.h" +#include "../../../../client/crash/crash_reporter.h" #include "../../../../client/settings/cache_settings.h" #include "../../interface/pixel_map_generator.h" #include "../../interface/theme_manager.h" @@ -1221,6 +1222,7 @@ void UserListWidget::processUserInfo(Section section, const ServerInfo_User &use bool UserListWidget::deleteUser(const QString &userName) { + CrashReporter::addBreadcrumb("users", "user list row removed"); if (sectioned) { // The user may own several rows (one per section). Drop them all. bool removed = false; diff --git a/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp b/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp index 7647b8ca8..c5cb7fda6 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp @@ -1,5 +1,6 @@ #include "tab_supervisor.h" +#include "../../../client/crash/crash_reporter.h" #include "../../../client/settings/cache_settings.h" #include "../../../client/settings/shortcuts_settings.h" #include "../../intents/contexts/context_connect_to_server.h" @@ -593,6 +594,7 @@ void TabSupervisor::startLocal(const QList &_clients) */ void TabSupervisor::stop() { + CrashReporter::addBreadcrumb("tabs", "tab supervisor stopping"); if ((!client) && localClients.isEmpty()) { return; } @@ -1003,6 +1005,7 @@ void TabSupervisor::updateLatencyTooltip(const LatencyTracker::Stats &stats) void TabSupervisor::gameJoined(const Event_GameJoined &event) { + CrashReporter::addBreadcrumb("tabs", "game joined"); QMap roomGameTypes; TabRoom *room = roomTabs.value(event.game_info().room_id()); if (room) { @@ -1055,6 +1058,7 @@ void TabSupervisor::gameLeft(TabGame *tab) void TabSupervisor::addRoomTab(const ServerInfo_Room &info, bool setCurrent) { + CrashReporter::addBreadcrumb("tabs", "room tab added"); auto *tab = new TabRoom(this, client, userInfo, info); connect(tab, &TabRoom::maximizeClient, this, &TabSupervisor::maximizeMainWindow); connect(tab, &TabRoom::roomClosing, this, &TabSupervisor::roomLeft); @@ -1472,6 +1476,7 @@ void TabSupervisor::actShowPopup(const QString &message) void TabSupervisor::processUserLeft(const QString &userName) { + CrashReporter::addBreadcrumb("users", "user left"); TabMessage *tab = messageTabs.value(userName); if (tab) { tab->processUserLeft(); diff --git a/cockatrice/src/main.cpp b/cockatrice/src/main.cpp index b4dc0610f..e82867f3f 100644 --- a/cockatrice/src/main.cpp +++ b/cockatrice/src/main.cpp @@ -427,6 +427,23 @@ int main(int argc, char *argv[]) CardDatabaseManager::setCardDatabasePathProvider(&SettingsCache::instance()); CardDatabaseManager::setCardSetPriorityController(&SettingsCache::instance().cardDatabase()); + // Lifecycle breadcrumbs for native crash reports (see COCKATRICE-DESKTOP-1R): the live + // card maps are replaced wholesale on reset, so knowing whether a crash followed a reload + // narrows down which QMap was being read. Direct connections: the SDK call is thread-safe + // and loading signals may come from a QtConcurrent worker. + { + CardDatabase *crumbDb = CardDatabaseManager::getInstance(); + QObject::connect( + crumbDb, &CardDatabase::cardDatabaseReset, crumbDb, + [] { CrashReporter::addBreadcrumb("card_db", "card database snapshot swapped in"); }, Qt::DirectConnection); + QObject::connect( + crumbDb, &CardDatabase::cardDatabaseLoadingFinished, crumbDb, + [] { CrashReporter::addBreadcrumb("card_db", "card database loading finished"); }, Qt::DirectConnection); + QObject::connect( + crumbDb, &CardDatabase::cardDatabaseLoadingFailed, crumbDb, + [] { CrashReporter::addBreadcrumb("card_db", "card database loading failed"); }, Qt::DirectConnection); + } + qCInfo(MainLog) << "Starting main program"; // Front-load the card database before constructing the main window. The diff --git a/libcockatrice_settings/libcockatrice/settings/cards_display_settings.cpp b/libcockatrice_settings/libcockatrice/settings/cards_display_settings.cpp index 910e3f09d..bdd494646 100644 --- a/libcockatrice_settings/libcockatrice/settings/cards_display_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/cards_display_settings.cpp @@ -70,16 +70,14 @@ bool CardsDisplaySettings::getScaleCards() const int CardsDisplaySettings::getHoverPanelTrigger() const { - // Off by default: the panel is new, so it is opt-in. - return getValue("hoverPanelTrigger", QString(), QString(), static_cast(HoverPanelTriggerOff)).toInt(); + return getValue("hoverPanelTrigger", QString(), QString(), static_cast(HoverPanelTriggerHover)).toInt(); } int CardsDisplaySettings::getHoverZoomTrigger() const { - // Off by default: the smooth hover zoom is opt-in. With no trigger configured, - // hovering keeps the historical 1.1x scale (AbstractCardItem::setHoverHighlighted) - // instead of the smooth zoom to the hover panel size. - return getValue("hoverZoomTrigger", QString(), QString(), static_cast(HoverPanelTriggerOff)).toInt(); + // Hover is the default: hovering first zooms the card, then the tab panel shows up. + // Either trigger can still be changed or turned off in the interface settings. + return getValue("hoverZoomTrigger", QString(), QString(), static_cast(HoverPanelTriggerHover)).toInt(); } bool CardsDisplaySettings::getHoverPanelDoubleConfirm() const diff --git a/libcockatrice_settings/libcockatrice/settings/cards_display_settings.h b/libcockatrice_settings/libcockatrice/settings/cards_display_settings.h index 59a513e8f..d8ced83a4 100644 --- a/libcockatrice_settings/libcockatrice/settings/cards_display_settings.h +++ b/libcockatrice_settings/libcockatrice/settings/cards_display_settings.h @@ -23,7 +23,7 @@ public: HoverPanelTriggerRightClick = 1, HoverPanelTriggerWheel = 2, HoverPanelTriggerShortcut = 3, - /// Never fires. The hover zoom and panel are opt-in, so this is their default. + /// Never fires when selected; lets the user turn the hover zoom and panel off. HoverPanelTriggerOff = 4 }; diff --git a/tests/crash_reporter_test.cpp b/tests/crash_reporter_test.cpp index e44a34a60..208812a4b 100644 --- a/tests/crash_reporter_test.cpp +++ b/tests/crash_reporter_test.cpp @@ -418,6 +418,16 @@ TEST(CrashReporterTest, BeginSessionEndSessionRoundTripReportsClean) reporter.endSession(); } +TEST(CrashReporterTest, AddBreadcrumbIsSafeWithoutInitAndWithNullArguments) +{ + // Breadcrumbs are emitted from GUI slots and QtConcurrent workers regardless of whether + // the SDK was initialised (empty DSN, Never consent, OFF builds); none of these may crash. + CrashReporter::addBreadcrumb("card_db", "card database snapshot swapped in"); + CrashReporter::addBreadcrumb(nullptr, "message"); + CrashReporter::addBreadcrumb("category", nullptr); + SUCCEED(); +} + } // namespace int main(int argc, char **argv) diff --git a/tests/settings/settings_defaults_test.cpp b/tests/settings/settings_defaults_test.cpp index 4ca440567..acffae9a8 100644 --- a/tests/settings/settings_defaults_test.cpp +++ b/tests/settings/settings_defaults_test.cpp @@ -686,8 +686,7 @@ TEST_F(SettingsDefaultsTest, CardsDisplay_ArrowDrawAnimation_Default) TEST_F(SettingsDefaultsTest, CardsDisplay_HoverPanelTrigger_Default) { CardsDisplaySettings s(settingsPath, nullptr); - // Off by default: the panel is opt-in on upgrade. - ASSERT_EQ(s.getHoverPanelTrigger(), CardsDisplaySettings::HoverPanelTriggerOff); + ASSERT_EQ(s.getHoverPanelTrigger(), CardsDisplaySettings::HoverPanelTriggerHover); } TEST_F(SettingsDefaultsTest, CardsDisplay_HoverPanelTrigger_SetAndGet) @@ -702,8 +701,7 @@ TEST_F(SettingsDefaultsTest, CardsDisplay_HoverPanelTrigger_SetAndGet) TEST_F(SettingsDefaultsTest, CardsDisplay_HoverZoomTrigger_Default) { CardsDisplaySettings s(settingsPath, nullptr); - // Off by default: the smooth hover zoom is opt-in on upgrade. - ASSERT_EQ(s.getHoverZoomTrigger(), CardsDisplaySettings::HoverPanelTriggerOff); + ASSERT_EQ(s.getHoverZoomTrigger(), CardsDisplaySettings::HoverPanelTriggerHover); } TEST_F(SettingsDefaultsTest, CardsDisplay_HoverZoomTrigger_SetAndGet)