[Client] Show card names in the game log and [card] tags in the chosen card language (#7387)

The game log translates its sentences, zone names and counter names, but every
card name was rendered as the raw canonical name straight from the server.
The card board and the card info text already went through
CardLocalization::displayName(), so the log was the odd one out.

cardLink() used one string for both the card:// href and the visible label,
which left no room for a localized label. Split the two: the href keeps the
canonical name because ChatView::mousePressEvent feeds it straight to
showCardInfoPopup for lookup, while the label goes through the display
language. Same split for [card] chat tags in ChatView::appendCardTag.

Sanitize the name while building the link. logUndoDraw already did this by
hand and had drifted into its own non-italic copy of cardLink(); route it back
through the helper.

Add CardLocalization::displayNameFor() for callers that only hold a canonical
name and need no CardInfoPtr of their own.

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>

cockatrice/src/game_graphics/log/message_log_widget.cpp

@@ -6,6 +6,7 @@
 #include "../../game/game_state.h"
 #include "../../game/phase.h"
 #include "../../game/player/player_logic.h"
+#include "../../interface/card_localization.h"
 #include "../../interface/widgets/tabs/tab_game.h"
 #include "../board/card_item.h"
 #include "../board/translate_counter_name.h"
@@ -23,7 +24,11 @@ static QString sanitizeHtml(QString dirty)
 
 static QString cardLink(const QString &cardName)
 {
-    return QString("<i><a href=\"card://%1\">%2</a></i>").arg(cardName).arg(cardName);
+    // The href keeps the canonical name so the card popup can resolve it, while the label shows the name
+    // in the configured card language.
+    return QString("<i><a href=\"card://%1\">%2</a></i>")
+        .arg(sanitizeHtml(cardName))
+        .arg(sanitizeHtml(CardLocalization::displayNameFor(cardName)));
 }
 
 QPair<QString, QString>
@@ -803,10 +808,9 @@ void MessageLogWidget::logUndoDraw(PlayerLogic *player, QString cardName)
     if (cardName.isEmpty()) {
         appendHtmlServerMessage(tr("%1 undoes their last draw.").arg(sanitizeHtml(player->getPlayerInfo()->getName())));
     } else {
-        appendHtmlServerMessage(
-            tr("%1 undoes their last draw (%2).")
-                .arg(sanitizeHtml(player->getPlayerInfo()->getName()))
-                .arg(QString("<a href=\"card://%1\">%2</a>").arg(sanitizeHtml(cardName)).arg(sanitizeHtml(cardName))));
+        appendHtmlServerMessage(tr("%1 undoes their last draw (%2).")
+                                    .arg(sanitizeHtml(player->getPlayerInfo()->getName()))
+                                    .arg(cardLink(std::move(cardName))));
     }
 }
 

cockatrice/src/interface/card_localization.h

@@ -5,6 +5,7 @@
 
 #include <QString>
 #include <libcockatrice/card/card_info.h>
+#include <libcockatrice/card/database/card_database_manager.h>
 #include <libcockatrice/settings/cards_display_settings.h>
 
 namespace CardLocalization
@@ -54,6 +55,25 @@ inline QString displayText(const CardInfo &card)
 {
     return card.getLocalizedText(displayLang());
 }
+
+/**
+ * @brief Looks a card up by its canonical name and returns its name in the configured display language.
+ *
+ * Use this where only the canonical name is known, such as the game log or [card] chat tags. Callers that
+ * already hold a resolved card and need to keep the canonical name around (as a link target, say) should
+ * use displayName() directly instead.
+ *
+ * @param cardName The canonical card name.
+ * @return The localized name, or cardName unchanged when the card is unknown to the database.
+ */
+inline QString displayNameFor(const QString &cardName)
+{
+    if (cardName.isEmpty()) {
+        return cardName;
+    }
+    const QString localizedName = displayName(CardDatabaseManager::query()->getCardInfo(cardName));
+    return localizedName.isEmpty() ? cardName : localizedName;
+}
 } // namespace CardLocalization
 
 #endif // COCKATRICE_CARD_LOCALIZATION_H

cockatrice/src/interface/theme_manager.cpp

@@ -23,6 +23,7 @@
 #include <libcockatrice/settings/paths_settings.h>
 
 #define SYSTEM_THEME_NAME "System"
+#define LEGACY_SYSTEM_THEME_NAME "Default"
 #define FUSION_THEME_NAME "Fusion"
 #define STYLE_CSS_NAME "style.css"
 #define HANDZONE_BG_NAME "handzone"
@@ -122,12 +123,84 @@ ThemeManager::ThemeManager(QObject *parent) : QObject(parent)
     themeChangedSlot();
 }
 
+// Copy every file of sourceDir into targetDir without ever overwriting an
+// existing file, recursing into subdirectories (zones/, backgrounds/, ...).
+// Returns false as soon as an entry could not be copied, leaving the source
+// untouched so the caller can retry later.
+static bool mergeDirWithoutOverwrite(const QString &sourceDir, const QString &targetDir)
+{
+    const QDir target(targetDir);
+    bool merged = true;
+
+    const QDir::Filters filters = QDir::AllEntries | QDir::NoDotAndDotDot | QDir::Hidden;
+    for (const QFileInfo &entry : QDir(sourceDir).entryInfoList(filters)) {
+        const QString targetEntry = target.absoluteFilePath(entry.fileName());
+        if (entry.isDir()) {
+            if (!QDir().mkpath(targetEntry) || !mergeDirWithoutOverwrite(entry.absoluteFilePath(), targetEntry)) {
+                merged = false;
+            }
+        } else if (!target.exists(entry.fileName()) && !QFile::copy(entry.absoluteFilePath(), targetEntry)) {
+            qCWarning(ThemeManagerLog) << "Could not migrate theme file:" << entry.absoluteFilePath();
+            merged = false;
+        }
+    }
+
+    return merged;
+}
+
+void ThemeManager::migrateLegacyThemeDir(const QString &themesPath, const QString &currentThemeName)
+{
+    const QString legacyPath = QDir(themesPath).absoluteFilePath(LEGACY_SYSTEM_THEME_NAME);
+    if (!QDir(legacyPath).exists()) {
+        return;
+    }
+
+    // Only the renamed theme's own directory is migrated. A user sitting on a
+    // different theme may well have hand-made a "Default" folder of their own,
+    // and silently folding it into "System" would be rude. The check accepts
+    // both the pre-rename name and the post-rename one, since a user who
+    // already ran the rename keeps their settings on "System" while their
+    // customised directory is still called "Default".
+    if (currentThemeName != LEGACY_SYSTEM_THEME_NAME && currentThemeName != SYSTEM_THEME_NAME) {
+        return;
+    }
+
+    const QString targetPath = QDir(themesPath).absoluteFilePath(SYSTEM_THEME_NAME);
+    if (!QDir(targetPath).exists() && QDir().rename(legacyPath, targetPath)) {
+        qCInfo(ThemeManagerLog) << "Migrated customised theme directory" << LEGACY_SYSTEM_THEME_NAME << "to"
+                                << SYSTEM_THEME_NAME;
+        return;
+    }
+
+    // The rename is only possible when the target does not exist yet. When it
+    // does (a partial earlier upgrade, or a user who re-customised the theme
+    // after the rename), fall back to merging so nothing is lost: existing
+    // "System" files win, legacy files only fill gaps. Because every legacy
+    // file either already exists at the target or is copied there, removing
+    // the legacy directory afterwards cannot drop user data.
+    if (!mergeDirWithoutOverwrite(legacyPath, targetPath)) {
+        qCWarning(ThemeManagerLog) << "Could not fully migrate theme directory" << legacyPath << "; keeping it for a"
+                                   << "later retry";
+        return;
+    }
+    if (!QDir(legacyPath).removeRecursively()) {
+        qCWarning(ThemeManagerLog) << "Migrated theme directory but could not remove the legacy one:" << legacyPath;
+        return;
+    }
+    qCInfo(ThemeManagerLog) << "Merged customised theme directory" << LEGACY_SYSTEM_THEME_NAME << "into"
+                            << SYSTEM_THEME_NAME;
+}
+
 void ThemeManager::ensureThemeDirectoryExists()
 {
     auto &settings = SettingsCache::instance();
 
+    // Carry the user's theme customisations (zone graphics, stylesheets,
+    // palettes, theme.cfg) across before the stored name is rewritten.
+    migrateLegacyThemeDir(settings.paths().getThemesPath(), settings.getThemeName());
+
     // Migrate the old "Default" theme name to "System"
-    if (settings.getThemeName() == "Default") {
+    if (settings.getThemeName() == LEGACY_SYSTEM_THEME_NAME) {
         settings.setThemeName(SYSTEM_THEME_NAME);
     }
 

cockatrice/src/interface/theme_manager.h

@@ -71,6 +71,14 @@ public:
     // read-only system themes fall back to the user themes directory, creating
     // it if needed, so customisations never get lost on upgrade.
     static QString writableThemeDir(const QString &themeName);
+    // Carries a user profile's customised "Default" theme directory over to the
+    // "System" directory the theme was renamed to, so zone graphics,
+    // stylesheets, palettes and theme.cfg survive the rename. Renames the
+    // directory when "System" does not exist yet, otherwise merges file by
+    // file without overwriting what "System" already has. No-op unless the
+    // active theme is one of the two names, so a hand-made "Default" folder of
+    // a user on another theme is left alone.
+    static void migrateLegacyThemeDir(const QString &themesPath, const QString &currentThemeName);
     // Probe whether a directory is truly writable by trying to create and remove
     // a temporary file. QFileInfo::isWritable() on a directory is unreliable
     // (notably on Windows where UAC VirtualStore can make a system dir appear

cockatrice/src/interface/widgets/server/chat_view/chat_view.cpp

@@ -2,6 +2,7 @@
 
 #include "../../../../client/settings/cache_settings.h"
 #include "../../client/sound_engine.h"
+#include "../../interface/card_localization.h"
 #include "../../interface/pixel_map_generator.h"
 #include "../../interface/widgets/tabs/tab_account.h"
 #include "../user/user_context_menu.h"
@@ -190,7 +191,8 @@ void ChatView::appendCardTag(QTextCursor &cursor, const QString &cardName)
     QTextCharFormat cardFormat = oldFormat;
     cardFormat.setFontItalic(true);
 
-    if (!CardDatabaseManager::query()->lookupCardByName(cardName)) {
+    const CardInfoPtr card = CardDatabaseManager::query()->lookupCardByName(cardName);
+    if (card.isNull()) {
         cardFormat.setForeground(unresolvedCardTagColor);
         cursor.setCharFormat(cardFormat);
         cursor.insertText(cardName);
@@ -200,10 +202,12 @@ void ChatView::appendCardTag(QTextCursor &cursor, const QString &cardName)
 
     cardFormat.setForeground(linkColor);
     cardFormat.setAnchor(true);
+    // The href keeps the canonical name so the card popup can resolve it, while the text shows the name
+    // in the configured card language.
     cardFormat.setAnchorHref("card://" + cardName);
 
     cursor.setCharFormat(cardFormat);
-    cursor.insertText(cardName);
+    cursor.insertText(CardLocalization::displayName(card));
     cursor.setCharFormat(oldFormat);
 }
 

tests/CMakeLists.txt

@@ -178,6 +178,11 @@ add_subdirectory(movecard_tests)
 add_subdirectory(oracle)
 add_subdirectory(settings)
 
+# theme_migration_test compiles client sources, whose headers are only reachable from a client build.
+if(WITH_CLIENT)
+  add_subdirectory(theme_migration)
+endif()
+
 # picture_loader_benchmark links libcockatrice_settings, which only exists when
 # a client/UI-capable target is being built.
 if(WITH_ORACLE OR WITH_CLIENT)

tests/settings/settings_migration_test.cpp

@@ -542,6 +542,34 @@ TEST_F(SettingsMigrationTest, PartialMigrationOnlyMigratesPresentKeys)
     ASSERT_FALSE(fileExists("chat.ini"));
 }
 
+TEST_F(SettingsMigrationTest, ThemeNameDefaultIsRewrittenToSystem)
+{
+    {
+        QSettings g(settingsPath + "global.ini", QSettings::IniFormat);
+        g.setValue("theme/name", "Default");
+        g.sync();
+    }
+
+    ASSERT_TRUE(SettingsMigration::migrateSettingsFromGlobalIni(settingsPath));
+
+    ASSERT_TRUE(fileExists("appearance.ini"));
+    ASSERT_EQ(readFromIni("appearance.ini", "appearance/themeName"), QVariant("System"));
+}
+
+TEST_F(SettingsMigrationTest, OtherThemeNamesAreLeftAlone)
+{
+    {
+        QSettings g(settingsPath + "global.ini", QSettings::IniFormat);
+        g.setValue("theme/name", "VelvetMarble");
+        g.sync();
+    }
+
+    ASSERT_TRUE(SettingsMigration::migrateSettingsFromGlobalIni(settingsPath));
+
+    ASSERT_TRUE(fileExists("appearance.ini"));
+    ASSERT_EQ(readFromIni("appearance.ini", "appearance/themeName"), QVariant("VelvetMarble"));
+}
+
 TEST_F(SettingsMigrationTest, KeyMapTranslationIsCorrect)
 {
     {

tests/theme_migration/CMakeLists.txt

@@ -0,0 +1,30 @@
+add_executable(
+  theme_migration_test
+  ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/theme_manager.cpp
+  ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/theme_config.cpp
+  ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/pixel_map_generator.cpp
+  ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/cache_settings.cpp
+  ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/card_counter_settings.cpp
+  ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/shortcuts_settings.cpp
+  ${CMAKE_SOURCE_DIR}/cockatrice/src/client/network/update/client/release_channel.cpp
+  ${VERSION_STRING_CPP}
+  theme_migration_test.cpp
+)
+target_include_directories(theme_migration_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src)
+
+target_link_libraries(
+  theme_migration_test
+  libcockatrice_settings
+  libcockatrice_network
+  libcockatrice_network_server_remote
+  libcockatrice_protocol
+  Threads::Threads
+  ${GTEST_BOTH_LIBRARIES}
+  ${TEST_QT_MODULES}
+)
+
+add_test(NAME theme_migration_test COMMAND theme_migration_test)
+
+if(NOT GTEST_FOUND)
+  add_dependencies(theme_migration_test gtest)
+endif()

tests/theme_migration/theme_migration_test.cpp

@@ -0,0 +1,154 @@
+#include "interface/theme_manager.h"
+
+#include <QDir>
+#include <QFile>
+#include <QFileInfo>
+#include <QString>
+#include <QTemporaryDir>
+#include <gtest/gtest.h>
+
+// Owned by main.cpp in the real application; pixel_map_generator.cpp references
+// it, so the test binary needs a definition even though it never renders.
+ThemeManager *themeManager = nullptr;
+
+namespace
+{
+const QString LEGACY = QStringLiteral("Default");
+const QString CURRENT = QStringLiteral("System");
+
+// Writes content at <themesPath>/<theme>/<relativePath>, creating parents.
+void writeThemeFile(const QString &themesPath,
+                    const QString &theme,
+                    const QString &relativePath,
+                    const QByteArray &content)
+{
+    const QString filePath = QDir(QDir(themesPath).absoluteFilePath(theme)).absoluteFilePath(relativePath);
+    ASSERT_TRUE(QDir().mkpath(QFileInfo(filePath).absolutePath()));
+    QFile file(filePath);
+    ASSERT_TRUE(file.open(QIODevice::WriteOnly));
+    file.write(content);
+    file.close();
+}
+
+QByteArray readThemeFile(const QString &themesPath, const QString &theme, const QString &relativePath)
+{
+    QFile file(QDir(QDir(themesPath).absoluteFilePath(theme)).absoluteFilePath(relativePath));
+    if (!file.open(QIODevice::ReadOnly)) {
+        return QByteArray();
+    }
+    return file.readAll();
+}
+
+class ThemeMigrationTest : public ::testing::Test
+{
+protected:
+    QTemporaryDir tmp;
+    QString themesPath;
+
+    void SetUp() override
+    {
+        ASSERT_TRUE(tmp.isValid());
+        themesPath = QDir(tmp.path()).absoluteFilePath("themes");
+        ASSERT_TRUE(QDir().mkpath(themesPath));
+    }
+};
+
+} // namespace
+
+TEST_F(ThemeMigrationTest, NoLegacyDirectoryIsANoOp)
+{
+    writeThemeFile(themesPath, CURRENT, "theme.cfg", "[Appearance]\n");
+
+    ThemeManager::migrateLegacyThemeDir(themesPath, CURRENT);
+
+    EXPECT_TRUE(QDir(QDir(themesPath).absoluteFilePath(CURRENT)).exists());
+    EXPECT_TRUE(QDir(QDir(themesPath).absoluteFilePath(LEGACY)).exists() == false);
+    EXPECT_EQ(readThemeFile(themesPath, CURRENT, "theme.cfg").toStdString(), std::string("[Appearance]\n"));
+}
+
+TEST_F(ThemeMigrationTest, RenamesDirectoryWhenTargetIsFree)
+{
+    // A customised "Default": stylesheet, zone graphics in a subdirectory and a
+    // palette, i.e. everything a user is liable to have personalised.
+    writeThemeFile(themesPath, LEGACY, "style.css", "QPushButton { color: red; }");
+    writeThemeFile(themesPath, LEGACY, "zones/handzone.png", "handzone-bytes");
+    writeThemeFile(themesPath, LEGACY, "zones/tablezone-dark.svg", "tablezone-bytes");
+    writeThemeFile(themesPath, LEGACY, "palette-dark.toml", "palette");
+
+    ThemeManager::migrateLegacyThemeDir(themesPath, LEGACY);
+
+    EXPECT_FALSE(QDir(QDir(themesPath).absoluteFilePath(LEGACY)).exists());
+    EXPECT_EQ(readThemeFile(themesPath, CURRENT, "style.css").toStdString(),
+              std::string("QPushButton { color: red; }"));
+    EXPECT_EQ(readThemeFile(themesPath, CURRENT, "zones/handzone.png").toStdString(), std::string("handzone-bytes"));
+    EXPECT_EQ(readThemeFile(themesPath, CURRENT, "zones/tablezone-dark.svg").toStdString(),
+              std::string("tablezone-bytes"));
+    EXPECT_EQ(readThemeFile(themesPath, CURRENT, "palette-dark.toml").toStdString(), std::string("palette"));
+}
+
+TEST_F(ThemeMigrationTest, RenamesForAnAlreadyMigratedUserWhoseDirectoryWasLeftBehind)
+{
+    // The settings value was rewritten to "System" on an earlier run, but the
+    // customised directory is still called "Default". The customisations are
+    // still reachable and must still be carried over.
+    writeThemeFile(themesPath, LEGACY, "style.css", "legacy-css");
+
+    ThemeManager::migrateLegacyThemeDir(themesPath, CURRENT);
+
+    EXPECT_FALSE(QDir(QDir(themesPath).absoluteFilePath(LEGACY)).exists());
+    EXPECT_EQ(readThemeFile(themesPath, CURRENT, "style.css").toStdString(), std::string("legacy-css"));
+}
+
+TEST_F(ThemeMigrationTest, MergesIntoAnExistingTargetWithoutOverwriting)
+{
+    // Post-rename re-customisation: "System" already exists and holds newer
+    // files. Those win; the legacy directory only fills the gaps.
+    writeThemeFile(themesPath, CURRENT, "style.css", "newer-css");
+    writeThemeFile(themesPath, CURRENT, "zones/handzone.png", "newer-handzone");
+    writeThemeFile(themesPath, LEGACY, "style.css", "legacy-css");
+    writeThemeFile(themesPath, LEGACY, "zones/handzone.png", "legacy-handzone");
+    writeThemeFile(themesPath, LEGACY, "zones/stackzone.png", "legacy-stackzone");
+    writeThemeFile(themesPath, LEGACY, "palette-light.toml", "legacy-palette");
+
+    ThemeManager::migrateLegacyThemeDir(themesPath, CURRENT);
+
+    EXPECT_FALSE(QDir(QDir(themesPath).absoluteFilePath(LEGACY)).exists());
+    EXPECT_EQ(readThemeFile(themesPath, CURRENT, "style.css").toStdString(), std::string("newer-css"));
+    EXPECT_EQ(readThemeFile(themesPath, CURRENT, "zones/handzone.png").toStdString(), std::string("newer-handzone"));
+    EXPECT_EQ(readThemeFile(themesPath, CURRENT, "zones/stackzone.png").toStdString(), std::string("legacy-stackzone"));
+    EXPECT_EQ(readThemeFile(themesPath, CURRENT, "palette-light.toml").toStdString(), std::string("legacy-palette"));
+}
+
+TEST_F(ThemeMigrationTest, LeavesAnotherThemesDefaultDirectoryAlone)
+{
+    // A user on an unrelated theme may legitimately have made a "Default"
+    // folder of their own; it must not be folded into "System".
+    writeThemeFile(themesPath, LEGACY, "style.css", "unrelated");
+
+    ThemeManager::migrateLegacyThemeDir(themesPath, QStringLiteral("Leather"));
+
+    EXPECT_TRUE(QDir(QDir(themesPath).absoluteFilePath(LEGACY)).exists());
+    EXPECT_EQ(readThemeFile(themesPath, LEGACY, "style.css").toStdString(), std::string("unrelated"));
+    EXPECT_FALSE(QDir(QDir(themesPath).absoluteFilePath(CURRENT)).exists());
+}
+
+TEST_F(ThemeMigrationTest, IsIdempotent)
+{
+    writeThemeFile(themesPath, LEGACY, "style.css", "css");
+
+    ThemeManager::migrateLegacyThemeDir(themesPath, LEGACY);
+    ASSERT_EQ(readThemeFile(themesPath, CURRENT, "style.css").toStdString(), std::string("css"));
+
+    ThemeManager::migrateLegacyThemeDir(themesPath, CURRENT);
+    ThemeManager::migrateLegacyThemeDir(themesPath, LEGACY);
+
+    EXPECT_EQ(readThemeFile(themesPath, CURRENT, "style.css").toStdString(), std::string("css"));
+}
+
+// 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)
+{
+    ::testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}