From af55dd3da0d4f6f419566b78d5439a4983e1c558 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Fri, 11 Sep 2026 20:34:48 +0200 Subject: [PATCH] [Themes] Route theme writes to the user themes directory setColorScheme()/setStyleName() and the palette editor wrote directly to the resolved theme directory, which for built-in themes is the read-only system (install) location. Changes therefore landed in the install dir and were lost on upgrade. Add ThemeManager::writableThemeDir(), which always resolves to the user themes directory, and route all theme writes through it. The palette editor reuses the same helper, dropping its private writability probe. --- .../palette_editor/palette_editor_dialog.cpp | 32 ++----------------- cockatrice/src/interface/theme_manager.cpp | 32 ++++++++++++++++--- cockatrice/src/interface/theme_manager.h | 11 ++++++- 3 files changed, 39 insertions(+), 36 deletions(-) diff --git a/cockatrice/src/interface/palette_editor/palette_editor_dialog.cpp b/cockatrice/src/interface/palette_editor/palette_editor_dialog.cpp index dc1e96901..d5a168708 100644 --- a/cockatrice/src/interface/palette_editor/palette_editor_dialog.cpp +++ b/cockatrice/src/interface/palette_editor/palette_editor_dialog.cpp @@ -1,6 +1,5 @@ #include "palette_editor_dialog.h" -#include "../../client/settings/cache_settings.h" #include "../theme_manager.h" #include "palette_generator.h" #include "palette_grid_widget.h" @@ -11,31 +10,11 @@ #include #include #include -#include #include -#include #include -#include #include #include -#include #include -#include - -// 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 writable). -static bool isDirReallyWritable(const QString &dirPath) -{ - const QString probe = QDir(dirPath).absoluteFilePath(".cockatrice_write_test"); - QFile f(probe); - if (!f.open(QIODevice::WriteOnly)) { - return false; - } - f.close(); - f.remove(); - return true; -} PaletteEditorDialog::PaletteEditorDialog(const QString &_themeDirPath, const QString &_themeName, QWidget *parent) : QDialog(parent), themeDirPath(_themeDirPath), themeName(_themeName) @@ -46,14 +25,7 @@ PaletteEditorDialog::PaletteEditorDialog(const QString &_themeDirPath, const QSt // Resolve a writable directory for saving. Built-in (Default / Fusion) and // other read-only theme directories must be customised in the user-writable // themes directory; otherwise the write would fail or be lost on upgrade. - if (!themeDirPath.isEmpty() && isDirReallyWritable(themeDirPath)) { - saveDir = themeDirPath; - } else { - saveDir = QDir(SettingsCache::instance().paths().getThemesPath()).absoluteFilePath(themeName); - if (!QDir().mkpath(saveDir)) { - qWarning() << "Failed to create palette save directory:" << saveDir; - } - } + saveDir = ThemeManager::writableThemeDir(themeName); // Load both scheme configs upfront so switching is instant loadSchemes(); @@ -214,7 +186,7 @@ void PaletteEditorDialog::retranslateUi() resetBtn->setToolTip(tr("Discard unsaved edits and restore the last saved palette")); saveBtn->setToolTip(tr("Write palette-%1.toml and reload the theme").arg(loadedScheme.toLower())); - if (saveDir.isEmpty() || !isDirReallyWritable(saveDir)) { + if (saveDir.isEmpty() || !ThemeManager::isDirReallyWritable(saveDir)) { saveBtn->setEnabled(false); saveBtn->setToolTip(tr("Cannot save: this theme has no writable directory")); } diff --git a/cockatrice/src/interface/theme_manager.cpp b/cockatrice/src/interface/theme_manager.cpp index d1bccbc0b..da9f14408 100644 --- a/cockatrice/src/interface/theme_manager.cpp +++ b/cockatrice/src/interface/theme_manager.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -184,11 +185,32 @@ QString ThemeManager::assetPath(QStringView prefix) const return resolvedPlain.isEmpty() ? prefix.toString() : resolvedPlain; } -bool ThemeManager::isBuiltInTheme() +// 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 writable). +bool ThemeManager::isDirReallyWritable(const QString &dirPath) { - const auto themeName = SettingsCache::instance().getThemeName(); + const QString probe = QDir(dirPath).absoluteFilePath(".cockatrice_write_test"); + QFile f(probe); + if (!f.open(QIODevice::WriteOnly)) { + return false; + } + f.close(); + f.remove(); + return true; +} - return themeName == NONE_THEME_NAME || themeName == FUSION_THEME_NAME; +QString ThemeManager::writableThemeDir(const QString &themeName) +{ + // All theme writes go to the user themes directory regardless of whether + // the resolved (system) theme directory happens to be writable. Even when a + // write would succeed in-place, routing it to the user directory keeps the + // install intact and guarantees changes survive upgrades. + const QString dirPath = QDir(SettingsCache::instance().paths().getThemesPath()).absoluteFilePath(themeName); + if (!QDir().mkpath(dirPath)) { + qWarning() << "Failed to create theme save directory:" << dirPath; + } + return dirPath; } // System (read-only) themes location, relative to the application binary. @@ -331,7 +353,7 @@ bool ThemeManager::commitPalette(const QString &themeDirPath, const QString &col void ThemeManager::setColorScheme(const QString &scheme) { - const QString dirPath = getAvailableThemes().value(SettingsCache::instance().getThemeName()); + const QString dirPath = writableThemeDir(SettingsCache::instance().getThemeName()); ThemeConfig cfg = ThemeConfig::fromThemeDir(dirPath); cfg.colorScheme = scheme; @@ -342,7 +364,7 @@ void ThemeManager::setColorScheme(const QString &scheme) void ThemeManager::setStyleName(const QString &styleName) { - const QString dirPath = getAvailableThemes().value(SettingsCache::instance().getThemeName()); + const QString dirPath = writableThemeDir(SettingsCache::instance().getThemeName()); ThemeConfig cfg = ThemeConfig::fromThemeDir(dirPath); cfg.styleName = styleName; diff --git a/cockatrice/src/interface/theme_manager.h b/cockatrice/src/interface/theme_manager.h index 4503e824b..aadb38ee9 100644 --- a/cockatrice/src/interface/theme_manager.h +++ b/cockatrice/src/interface/theme_manager.h @@ -66,7 +66,16 @@ protected: const QString &activeScheme); public: - bool isBuiltInTheme(); + // Resolves the directory to write theme changes to for the given theme + // name. The resolved theme dir (user or system) is used when writable; + // 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); + // 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 + // writable). + static bool isDirReallyWritable(const QString &dirPath); // Explicit color scheme of the theme: theme.cfg's ColorScheme setting // (Dark/Light), falling back to the OS color scheme when it is "System". bool isDarkMode(const QString &themeDirPath) const;