diff --git a/cockatrice/src/game_graphics/game_view.cpp b/cockatrice/src/game_graphics/game_view.cpp index 2f8af3e1b..7a51fc97f 100644 --- a/cockatrice/src/game_graphics/game_view.cpp +++ b/cockatrice/src/game_graphics/game_view.cpp @@ -117,11 +117,10 @@ GameView::GameView(GameScene *scene, QWidget *parent) : QGraphicsView(scene, par tallyLayout->setSpacing(2); tallyContainer->hide(); - applyDeckListFont(); - connect(themeManager, &ThemeManager::fontsChanged, this, &GameView::applyDeckListFont); + applyDeckOverlayFonts(); } -void GameView::applyDeckListFont() +void GameView::applyDeckOverlayFonts() { dragCountLabel->setFont(deckOverlayFont(14, true)); totalCountLabel->setFont(deckOverlayFont(16, true)); diff --git a/cockatrice/src/game_graphics/game_view.h b/cockatrice/src/game_graphics/game_view.h index 0fca224b2..99a73a0a8 100644 --- a/cockatrice/src/game_graphics/game_view.h +++ b/cockatrice/src/game_graphics/game_view.h @@ -50,7 +50,7 @@ private slots: void refreshShortcuts(); void updateTotalSelectionCount(const QSize &viewSize = QSize()); void setFocusDisabled(bool disabled); - void applyDeckListFont(); + void applyDeckOverlayFonts(); public slots: void updateSceneRect(const QRectF &rect); diff --git a/cockatrice/src/interface/palette_editor/font_grid_widget.cpp b/cockatrice/src/interface/palette_editor/font_grid_widget.cpp index f5a5beb45..c4b81de86 100644 --- a/cockatrice/src/interface/palette_editor/font_grid_widget.cpp +++ b/cockatrice/src/interface/palette_editor/font_grid_widget.cpp @@ -14,6 +14,7 @@ static constexpr int MIN_FONT_SIZE = 6; static constexpr int MAX_FONT_SIZE = 72; static constexpr int CARD_TEXT_MIN_SIZE = 1; +static constexpr int CARD_TEXT_MAX_SIZE = 12; static constexpr int FALLBACK_FONT_SIZE = 10; static const QMap APP_FONT_DESCRIPTIONS = { @@ -107,13 +108,45 @@ void FontGridWidget::loadFonts(const FontConfig &cfg) auto role = static_cast(fontEnum.value(i)); const FontRow &row = fontRows[role]; - // Roles absent from the file resolve to the startup font, not to the - // live preview: appFont() would return the previewed config, so Reset - // and a first load would both show the edited font instead of the - // theme's real default. - const QFont font = cfg.fonts.value(role, themeManager->defaultAppFont(role)); + // Roles absent from the file resolve the same way the runtime does: an unset + // role falls back to the config's InterfaceDefault, and only then to the + // startup font. appFont() would return the live preview instead, so Reset and + // a first load must not measure against it. + const QFont fallback = cfg.fonts.value(AppFont::InterfaceDefault, themeManager->defaultAppFont(role)); + const QFont font = cfg.fonts.value(role, fallback); row.familyCombo->setCurrentFont(font); + if (role == AppFont::CardText && row.minSpin != nullptr) { + // A previous load may have narrowed the linked bounds (min 20 pulls + // max up to 20), which would clamp the values written below. Reset + // both to the full range first; the valueChanged lambdas re-link + // them as the values are set. + row.minSpin->setRange(CARD_TEXT_MIN_SIZE, MAX_FONT_SIZE); + row.sizeSpin->setRange(MIN_FONT_SIZE, MAX_FONT_SIZE); + + // The max spin is a size-only override. Without CardTextMaxSize it + // follows the CardText family size, and without CardText at all it + // must match cardTextMaxSize()'s default rather than the system + // startup size, which currentFontConfig() would otherwise bake in. + const int fallbackMax = cfg.fonts.contains(role) ? font.pointSize() : CARD_TEXT_MAX_SIZE; + int maxSize = cfg.cardTextMaxSize > 0 ? cfg.cardTextMaxSize : fallbackMax; + if (maxSize <= 0) { + maxSize = CARD_TEXT_MAX_SIZE; + } + int minSize = cfg.cardTextMinSize > 0 ? cfg.cardTextMinSize : themeManager->cardTextMinSize(); + minSize = qBound(CARD_TEXT_MIN_SIZE, minSize, MAX_FONT_SIZE); + maxSize = qBound(MIN_FONT_SIZE, maxSize, MAX_FONT_SIZE); + if (maxSize < minSize) { + maxSize = minSize; + } + + // Max first, then min: each value re-applies the linked bound, so + // the pair ends up consistent instead of clamping the smaller one. + row.sizeSpin->setValue(maxSize); + row.minSpin->setValue(minSize); + continue; + } + int size = font.pointSize(); if (size <= 0) { size = qApp->font().pointSize(); @@ -122,11 +155,6 @@ void FontGridWidget::loadFonts(const FontConfig &cfg) size = FALLBACK_FONT_SIZE; } row.sizeSpin->setValue(qBound(MIN_FONT_SIZE, size, MAX_FONT_SIZE)); - - if (role == AppFont::CardText && row.minSpin != nullptr) { - int minSize = cfg.cardTextMinSize > 0 ? cfg.cardTextMinSize : themeManager->cardTextMinSize(); - row.minSpin->setValue(qBound(CARD_TEXT_MIN_SIZE, minSize, MAX_FONT_SIZE)); - } } blockSignals(false); } @@ -147,6 +175,7 @@ FontConfig FontGridWidget::currentFontConfig() const if (role == AppFont::CardText && row.minSpin != nullptr) { cfg.cardTextMinSize = row.minSpin->value(); + cfg.cardTextMaxSize = row.sizeSpin->value(); } } return cfg; diff --git a/cockatrice/src/interface/palette_editor/palette_editor_dialog.cpp b/cockatrice/src/interface/palette_editor/palette_editor_dialog.cpp index de5e5e737..e3bcd5046 100644 --- a/cockatrice/src/interface/palette_editor/palette_editor_dialog.cpp +++ b/cockatrice/src/interface/palette_editor/palette_editor_dialog.cpp @@ -286,9 +286,7 @@ void PaletteEditorDialog::loadSchemes() void PaletteEditorDialog::loadFonts() { savedFontConfig = ThemeManager::loadDefaultFontConfig(themeDirPath, themeName); - fontGrid->blockSignals(true); fontGrid->loadFonts(savedFontConfig); - fontGrid->blockSignals(false); fontsDirty = false; } @@ -395,7 +393,6 @@ void PaletteEditorDialog::onSave() void PaletteEditorDialog::onReset() { - const bool hadFontPreview = fontsDirty; workingConfig[loadedScheme] = savedConfig[loadedScheme]; paletteGrid->loadPalette(savedConfig[loadedScheme]); // Restore the live theme before re-reading the grid: loadFonts resolves @@ -404,9 +401,6 @@ void PaletteEditorDialog::onReset() themeManager->reloadCurrentTheme(); fontGrid->loadFonts(savedFontConfig); fontsDirty = false; - if (hadFontPreview) { - themeManager->previewFonts(savedFontConfig); - } seedAccentFromScheme(loadedScheme); } diff --git a/cockatrice/src/interface/palette_editor/palette_editor_dialog.h b/cockatrice/src/interface/palette_editor/palette_editor_dialog.h index cf2321f80..5e47536b5 100644 --- a/cockatrice/src/interface/palette_editor/palette_editor_dialog.h +++ b/cockatrice/src/interface/palette_editor/palette_editor_dialog.h @@ -19,7 +19,6 @@ class FontGridWidget; class QuickSetupPanel; class QFrame; class QWidget; -class QCloseEvent; class PaletteEditorDialog : public QDialog { diff --git a/cockatrice/src/interface/theme_config.cpp b/cockatrice/src/interface/theme_config.cpp index 3ef3a8870..a75330db4 100644 --- a/cockatrice/src/interface/theme_config.cpp +++ b/cockatrice/src/interface/theme_config.cpp @@ -308,7 +308,7 @@ QPalette PaletteConfig::apply(QPalette base) const bool FontConfig::hasFonts() const { - return !fonts.isEmpty() || cardTextMinSize != 1; + return !fonts.isEmpty() || cardTextMinSize != 1 || cardTextMaxSize > 0; } QString FontConfig::toToml() const @@ -338,6 +338,10 @@ QString FontConfig::toToml() const entries += QString("%1 = %2\n").arg(QStringLiteral("CardTextMinSize"), -20).arg(cardTextMinSize); } + if (cardTextMaxSize > 0) { + entries += QString("%1 = %2\n").arg(QStringLiteral("CardTextMaxSize"), -20).arg(cardTextMaxSize); + } + if (entries.isEmpty()) { return {}; } @@ -405,13 +409,13 @@ FontConfig FontConfig::fromFile(const QString &filePath) value = value.mid(1, value.size() - 2).trimmed(); } - // CardText floor: bare integer, e.g. CardTextMinSize = 2. Tolerate - // quoting like the family entries; skip invalid lines. - QString minKey = key; - if (minKey.startsWith("AppFont::")) { - minKey = minKey.mid(QStringLiteral("AppFont::").size()); + // CardText size bounds: bare integers, e.g. CardTextMinSize = 2. + // Tolerate quoting like the family entries; skip invalid lines. + QString sizeKey = key; + if (sizeKey.startsWith("AppFont::")) { + sizeKey = sizeKey.mid(QStringLiteral("AppFont::").size()); } - if (minKey.compare("CardTextMinSize", Qt::CaseInsensitive) == 0) { + if (sizeKey.compare("CardTextMinSize", Qt::CaseInsensitive) == 0) { bool ok = false; int minSize = value.trimmed().toInt(&ok); if (!ok || minSize <= 0) { @@ -420,6 +424,15 @@ FontConfig FontConfig::fromFile(const QString &filePath) cfg.cardTextMinSize = minSize; continue; } + if (sizeKey.compare("CardTextMaxSize", Qt::CaseInsensitive) == 0) { + bool ok = false; + int maxSize = value.trimmed().toInt(&ok); + if (!ok || maxSize <= 0) { + continue; + } + cfg.cardTextMaxSize = maxSize; + continue; + } int comma = value.lastIndexOf(','); diff --git a/cockatrice/src/interface/theme_config.h b/cockatrice/src/interface/theme_config.h index 9481bf3a7..ebad7ef46 100644 --- a/cockatrice/src/interface/theme_config.h +++ b/cockatrice/src/interface/theme_config.h @@ -94,10 +94,13 @@ struct FontConfig { QMap fonts; int cardTextMinSize = 1; + /// Optional size-only maximum for CardText, used by the retired maxFontSize + /// migration so seeding it does not have to pin a family. + int cardTextMaxSize = 0; bool operator==(const FontConfig &rhs) const { - return fonts == rhs.fonts && cardTextMinSize == rhs.cardTextMinSize; + return fonts == rhs.fonts && cardTextMinSize == rhs.cardTextMinSize && cardTextMaxSize == rhs.cardTextMaxSize; } bool operator!=(const FontConfig &rhs) const { diff --git a/cockatrice/src/interface/theme_manager.cpp b/cockatrice/src/interface/theme_manager.cpp index 70dcf2946..6b4328d5c 100644 --- a/cockatrice/src/interface/theme_manager.cpp +++ b/cockatrice/src/interface/theme_manager.cpp @@ -657,23 +657,12 @@ QFont ThemeManager::appFont(AppFont::Role role) const } // Role fallbacks preserve the pre-theme defaults when no fonts.toml is - // present: chat logs rendered monospace (matching the report dialogs) and - // every other role resolves to the app font. The surfaces that hardcoded a - // different family ask for it explicitly: counter painters through - // cardTextFont(), the deck view headers and printed deck list through - // deckListFont(). Only the family basis is fixed there; sizes stay - // caller-side. - if (role == AppFont::ChatLog) { - // ChatView never set a font before the theme role existed, so an unset - // ChatLog must keep resolving to the application font. The report - // dialogs that used to hardcode monospace ask for it explicitly - // (report_utils::reportLogFont) instead of through this fallback. - return qApp->font(); - } - // No per-role scaling: an unset role resolves to the application font, so - // with no fonts.toml present the result is identical to before. The counter - // painters that used to hardcode Serif ask for it through cardTextFont(), - // and the two deck-list sites that did through deckListFont(). + // present: an unset role resolves to the application font. Surfaces that + // hardcoded a different family ask for it explicitly: counter painters + // through cardTextFont(), the deck view headers and printed deck list + // through deckListFont(), the report dialogs through + // report_utils::reportLogFont(). Only the family basis is fixed there; + // sizes stay caller-side. return qApp->font(); } @@ -713,6 +702,9 @@ QFont ThemeManager::defaultAppFont(AppFont::Role /*role*/) const int ThemeManager::cardTextMaxSize() const { + if (currentFonts.cardTextMaxSize > 0) { + return currentFonts.cardTextMaxSize; + } const auto it = currentFonts.fonts.constFind(AppFont::CardText); if (it != currentFonts.fonts.constEnd()) { const int size = it.value().pointSize(); diff --git a/cockatrice/src/interface/theme_manager.h b/cockatrice/src/interface/theme_manager.h index 22fbed86e..0cd44125e 100644 --- a/cockatrice/src/interface/theme_manager.h +++ b/cockatrice/src/interface/theme_manager.h @@ -169,7 +169,8 @@ public: * "what this role resolves to" must use this, because appFont() * reflects a live preview while the editor is open. */ QFont defaultAppFont(AppFont::Role role) const; - /** @brief CardText max size: stored CardText point size, else 12. */ + /** @brief CardText max size: stored CardTextMaxSize, else the CardText + * point size, else 12. */ int cardTextMaxSize() const; /** @brief CardText shrink-to-fit floor: stored CardTextMinSize, else 1. */ int cardTextMinSize() const; diff --git a/libcockatrice_settings/libcockatrice/settings/settings_migration.cpp b/libcockatrice_settings/libcockatrice/settings/settings_migration.cpp index 312f5933f..3be3c0bf2 100644 --- a/libcockatrice_settings/libcockatrice/settings/settings_migration.cpp +++ b/libcockatrice_settings/libcockatrice/settings/settings_migration.cpp @@ -480,6 +480,9 @@ static bool fontsTomlHasExplicitCardTextMax(const QString &filePath) if (key.startsWith("AppFont::")) { key = key.mid(QStringLiteral("AppFont::").size()); } + if (key.compare("CardTextMaxSize", Qt::CaseInsensitive) == 0) { + return true; + } if (key.compare("CardText", Qt::CaseInsensitive) != 0) { continue; } @@ -525,8 +528,11 @@ static bool seedCardTextMaxIntoThemeDir(const QString &themeDirPath, int maxSize existing = QString::fromUtf8(in.readAll()); } - const QString entry = - QString("%1 = \"%2, %3\"\n").arg(QStringLiteral("CardText"), -20).arg(QStringLiteral("Serif")).arg(maxSize); + // Size-only key: seeding the CardText family here would pin it to Serif + // for the whole board (names, player labels, the completer), which is not + // what the retired maxFontSize setting meant. Let the family keep its + // existing resolution and only constrain the size. + const QString entry = QString("%1 = %2\n").arg(QStringLiteral("CardTextMaxSize"), -20).arg(maxSize); QString out; if (existing.isEmpty()) { diff --git a/tests/font_config_test.cpp b/tests/font_config_test.cpp index 75ea62daf..93c9cefb6 100644 --- a/tests/font_config_test.cpp +++ b/tests/font_config_test.cpp @@ -246,6 +246,73 @@ TEST(FontConfigTest, MalformedMinIsSkipped) EXPECT_EQ(FontConfig::fromFile(filePath).cardTextMinSize, 1); } +TEST(FontConfigTest, CardTextMaxRoundTrip) +{ + QTemporaryDir tmp; + ASSERT_TRUE(tmp.isValid()); + + FontConfig cfg; + cfg.fonts[AppFont::CardText] = makeFont("Noto Serif", 14); + cfg.cardTextMaxSize = 18; + ASSERT_TRUE(cfg.hasFonts()); + + const QString filePath = writeTempFile(tmp, "fonts.toml", cfg.toToml()); + const FontConfig loaded = FontConfig::fromFile(filePath); + + EXPECT_EQ(loaded, cfg); + EXPECT_EQ(loaded.cardTextMaxSize, 18); + EXPECT_EQ(loaded.fonts[AppFont::CardText].pointSize(), 14); +} + +TEST(FontConfigTest, CardTextMaxSizeWithoutFamily) +{ + // The retired maxFontSize setting migrates to a size-only key, so the + // family keeps its existing resolution instead of being pinned to Serif. + FontConfig cfg; + cfg.cardTextMaxSize = 18; + ASSERT_TRUE(cfg.hasFonts()); + + QTemporaryDir tmp; + ASSERT_TRUE(tmp.isValid()); + const QString filePath = writeTempFile(tmp, "fonts.toml", cfg.toToml()); + const FontConfig loaded = FontConfig::fromFile(filePath); + + EXPECT_TRUE(loaded.fonts.isEmpty()); + EXPECT_EQ(loaded.cardTextMaxSize, 18); + EXPECT_EQ(loaded, cfg); +} + +TEST(FontConfigTest, CardTextMaxDefaultIsOmitted) +{ + FontConfig cfg; + cfg.fonts[AppFont::CardText] = makeFont("Noto Serif", 12); + EXPECT_EQ(cfg.cardTextMaxSize, 0); + + const QString toml = cfg.toToml(); + EXPECT_FALSE(toml.contains(QStringLiteral("CardTextMaxSize"))); + + QTemporaryDir tmp; + ASSERT_TRUE(tmp.isValid()); + const QString filePath = writeTempFile(tmp, "fonts.toml", toml); + EXPECT_EQ(FontConfig::fromFile(filePath).cardTextMaxSize, 0); +} + +TEST(FontConfigTest, MalformedMaxIsSkipped) +{ + QTemporaryDir tmp; + ASSERT_TRUE(tmp.isValid()); + + const QStringList badValues = {"big", "0", "-3", "", "Inter, 18"}; + for (const QString &bad : badValues) { + const QString badPath = writeTempFile( + tmp, "bad.toml", QString("[Fonts]\nCardText = \"Noto Serif, 14\"\nCardTextMaxSize = %1\n").arg(bad)); + const FontConfig loaded = FontConfig::fromFile(badPath); + EXPECT_EQ(loaded.cardTextMaxSize, 0) << "bad value: " << bad.toStdString(); + ASSERT_TRUE(loaded.fonts.contains(AppFont::CardText)); + EXPECT_EQ(loaded.fonts[AppFont::CardText].pointSize(), 14); + } +} + TEST(FontConfigTest, InequalityComparesMinSize) { FontConfig left; @@ -257,6 +324,17 @@ TEST(FontConfigTest, InequalityComparesMinSize) EXPECT_NE(left, right); } +TEST(FontConfigTest, InequalityComparesMaxSize) +{ + FontConfig left; + left.fonts[AppFont::CardText] = makeFont("Noto Serif", 12); + left.cardTextMaxSize = 12; + + FontConfig right = left; + right.cardTextMaxSize = 18; + EXPECT_NE(left, right); +} + // Every test brings its own main(): GTest is not guaranteed to be found on the build machines, and // the fallback then links the bare gtest library without gtest_main. int main(int argc, char **argv) diff --git a/tests/settings/settings_migration_test.cpp b/tests/settings/settings_migration_test.cpp index 0d0ad999a..4fea3c085 100644 --- a/tests/settings/settings_migration_test.cpp +++ b/tests/settings/settings_migration_test.cpp @@ -1008,9 +1008,11 @@ TEST_F(SettingsMigrationTest, AppearanceMaxFontSize_NonDefaultSeedsThemeAndDelet QFile f(fontsPath); ASSERT_TRUE(f.open(QIODevice::ReadOnly | QIODevice::Text)); const QString content = QString::fromUtf8(f.readAll()); - EXPECT_TRUE(content.contains("CardText")); + EXPECT_TRUE(content.contains("CardTextMaxSize")); EXPECT_TRUE(content.contains("18")); - EXPECT_TRUE(content.contains("Serif")); + // Size-only: the migration must not pin the family, or every CardText + // surface switches to Serif for users who never chose it. + EXPECT_FALSE(content.contains("Serif")); ASSERT_FALSE(SettingsMigration::migrateAppearanceMaxFontSizeToTheme(settingsPath)); } @@ -1042,7 +1044,7 @@ TEST_F(SettingsMigrationTest, AppearanceMaxFontSize_SeedsActiveStockTheme) QFile f(fontsPath); ASSERT_TRUE(f.open(QIODevice::ReadOnly | QIODevice::Text)); const QString content = QString::fromUtf8(f.readAll()); - EXPECT_TRUE(content.contains("CardText")); + EXPECT_TRUE(content.contains("CardTextMaxSize")); EXPECT_TRUE(content.contains("18")); } @@ -1154,7 +1156,7 @@ TEST_F(SettingsMigrationTest, AppearanceMaxFontSize_MergesWithOtherRoles) ASSERT_TRUE(f.open(QIODevice::ReadOnly | QIODevice::Text)); const QString content = QString::fromUtf8(f.readAll()); EXPECT_TRUE(content.contains("InterfaceDefault")); - EXPECT_TRUE(content.contains("CardText")); + EXPECT_TRUE(content.contains("CardTextMaxSize")); EXPECT_TRUE(content.contains("20")); }