[PaletteEditor] Ensure directory is writeable, don't fall back to alternate palette (#7048)

* [PaletteEditor] Ensure directory is writeable, don't fall back to alternate palette.

Took 17 minutes

* [ThemeManager] Extract system theme dir path resolution to helper, use it in default palette fetching

Took 8 minutes

* [ThemeManager] Expose styling again

Took 3 minutes

* [ThemeManager] Capture app palette before theme is applied for reverting

* [ThemeManager] Simply delete custom palette instead of reverting to default

* [ThemeManager] Generate and apply immediately on change.

* [PaletteEditor] Block grid signals during initial loading.

Took 5 minutes

* Address comments

Took 8 minutes

* Seed accent from scheme on reset and revert

Took 2 minutes

---------

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

cockatrice/src/interface/palette_editor/palette_editor_dialog.cpp

@@ -1,5 +1,6 @@
 #include "palette_editor_dialog.h"
 
+#include "../../client/settings/cache_settings.h"
 #include "../theme_manager.h"
 #include "palette_generator.h"
 #include "palette_grid_widget.h"
@@ -8,20 +9,51 @@
 #include <QApplication>
 #include <QComboBox>
 #include <QDialogButtonBox>
+#include <QDir>
+#include <QFile>
+#include <QFileInfo>
 #include <QFrame>
 #include <QGuiApplication>
 #include <QLabel>
+#include <QLoggingCategory>
 #include <QMessageBox>
 #include <QPushButton>
 #include <QStyleHints>
 #include <QTimer>
 
+// 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)
 {
     setMinimumSize(740, 220);
     setupUi();
 
+    // 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().getThemesPath()).absoluteFilePath(themeName);
+        if (!QDir().mkpath(saveDir)) {
+            qWarning() << "Failed to create palette save directory:" << saveDir;
+        }
+    }
+
     // Load both scheme configs upfront so switching is instant
     loadSchemes();
 
@@ -31,7 +63,9 @@ PaletteEditorDialog::PaletteEditorDialog(const QString &_themeDirPath, const QSt
     schemeComboBox->setCurrentText(loadedScheme);
     schemeComboBox->blockSignals(false);
 
+    paletteGrid->blockSignals(true);
     paletteGrid->loadPalette(workingConfig[loadedScheme]);
+    paletteGrid->blockSignals(false);
     seedAccentFromScheme(loadedScheme);
 
     retranslateUi();
@@ -124,8 +158,7 @@ void PaletteEditorDialog::setupUi()
 
     buttonBox = new QDialogButtonBox;
     resetBtn = buttonBox->addButton(tr("Reset"), QDialogButtonBox::ResetRole);
-    applyBtn = buttonBox->addButton(tr("Apply"), QDialogButtonBox::ApplyRole);
-    saveBtn = buttonBox->addButton(tr("Save && Apply"), QDialogButtonBox::AcceptRole);
+    saveBtn = buttonBox->addButton(tr("Save"), QDialogButtonBox::AcceptRole);
     closeBtn = buttonBox->addButton(QDialogButtonBox::Close);
 
     footerLayout->addWidget(revertButton);
@@ -135,10 +168,14 @@ void PaletteEditorDialog::setupUi()
 
     // Connections
     connect(schemeComboBox, &QComboBox::currentTextChanged, this, &PaletteEditorDialog::onSchemeChanged);
-    connect(quickSetupPanel, &QuickSetupPanel::generateRequested, this, &PaletteEditorDialog::onGenerateFromAccent);
+    autoApplyTimer = new QTimer(this);
+    autoApplyTimer->setSingleShot(true);
+    autoApplyTimer->setInterval(150);
+    connect(autoApplyTimer, &QTimer::timeout, this, &PaletteEditorDialog::onApply);
+    connect(quickSetupPanel, &QuickSetupPanel::valueChanged, this, &PaletteEditorDialog::onGenerateFromAccent);
+    connect(paletteGrid, &PaletteGridWidget::paletteChanged, this, [this] { autoApplyTimer->start(); });
     connect(revertButton, &QPushButton::clicked, this, &PaletteEditorDialog::onRevertToDefault);
     connect(resetBtn, &QPushButton::clicked, this, &PaletteEditorDialog::onReset);
-    connect(applyBtn, &QPushButton::clicked, this, &PaletteEditorDialog::onApply);
     connect(saveBtn, &QPushButton::clicked, this, &PaletteEditorDialog::onSave);
     connect(closeBtn, &QPushButton::clicked, this, &QDialog::reject);
 
@@ -162,15 +199,8 @@ void PaletteEditorDialog::retranslateUi()
     setWindowTitle(tr("Palette Editor — %1").arg(themeName));
     titleLabel->setText(tr("<b>Palette Editor</b> &nbsp;·&nbsp; %1").arg(themeName));
 
-    // Revert button only makes sense when the theme ships default palette files
-    const bool hasDefault = PaletteConfig::fromDefault(themeDirPath, "Light").hasPalette() ||
-                            PaletteConfig::fromDefault(themeDirPath, "Dark").hasPalette();
-    revertButton->setEnabled(hasDefault);
-    if (!hasDefault) {
-        revertButton->setToolTip(tr("This theme ships no default palette files"));
-    } else {
-        revertButton->setToolTip(tr("Replace current colours with the theme author's defaults"));
-    }
+    revertButton->setToolTip(
+        tr("Delete this scheme's custom palette and revert to the theme default (or the application palette)"));
 
     schemeComboBox->setToolTip(tr("Switch between the light and dark palette files"));
     editingLabel->setText(tr("Editing:"));
@@ -179,15 +209,13 @@ void PaletteEditorDialog::retranslateUi()
     revertButton->setText(tr("↺  Revert to theme default"));
 
     resetBtn->setText(tr("Reset"));
-    applyBtn->setText(tr("Apply"));
-    saveBtn->setText(tr("Save && Apply"));
+    saveBtn->setText(tr("Save"));
     resetBtn->setToolTip(tr("Discard unsaved edits and restore the last saved palette"));
-    applyBtn->setToolTip(tr("Preview this palette without saving to disk"));
     saveBtn->setToolTip(tr("Write palette-%1.toml and reload the theme").arg(loadedScheme.toLower()));
 
-    if (themeDirPath.isEmpty()) {
+    if (saveDir.isEmpty() || !isDirReallyWritable(saveDir)) {
         saveBtn->setEnabled(false);
-        saveBtn->setToolTip(tr("Cannot save: this theme has no directory on disk"));
+        saveBtn->setToolTip(tr("Cannot save: this theme has no writable directory"));
     }
 }
 
@@ -198,7 +226,7 @@ void PaletteEditorDialog::loadSchemes()
         PaletteConfig cfg = PaletteConfig::fromScheme(themeDirPath, scheme);
 
         if (!cfg.hasPalette()) {
-            cfg = PaletteConfig::fromDefault(themeDirPath, scheme);
+            cfg = ThemeManager::loadDefaultPaletteConfig(themeDirPath, themeName, scheme);
         }
 
         if (!cfg.hasPalette()) {
@@ -235,7 +263,6 @@ void PaletteEditorDialog::onSchemeChanged(const QString &scheme)
     loadedScheme = scheme;
     paletteGrid->loadPalette(workingConfig.value(scheme));
     seedAccentFromScheme(scheme);
-    onApply();
 }
 
 void PaletteEditorDialog::onGenerateFromAccent(const QColor &accent, int intensity)
@@ -256,20 +283,34 @@ void PaletteEditorDialog::onSave()
         return;
     }
 
-    PaletteConfig cfg = paletteGrid->currentPaletteConfig();
+    // Snapshot the currently displayed scheme so unsaved edits are not lost.
+    workingConfig[loadedScheme] = paletteGrid->currentPaletteConfig();
 
-    if (!ThemeManager::savePaletteConfig(themeDirPath, loadedScheme, cfg)) {
-        QMessageBox::warning(this, tr("Save failed"),
-                             tr("Could not write %1 to:\n%2").arg(PaletteConfig::fileName(loadedScheme), themeDirPath));
-        return;
+    // Persist every scheme that changed, not just the one on screen. Each scheme
+    // has its own file, so edits to the non-active scheme would otherwise be
+    // silently discarded when the dialog closes.
+    for (auto it = workingConfig.begin(); it != workingConfig.end(); ++it) {
+        const QString &scheme = it.key();
+        if (it.value().colors == savedConfig.value(scheme).colors) {
+            continue; // unchanged — leave the on-disk file alone
+        }
+
+        if (!ThemeManager::savePaletteConfig(saveDir, scheme, it.value())) {
+            QMessageBox::warning(this, tr("Save failed"),
+                                 tr("Could not write %1 to:\n%2").arg(PaletteConfig::fileName(scheme), saveDir));
+            return;
+        }
     }
 
-    ThemeConfig globalCfg = ThemeConfig::fromThemeDir(themeDirPath);
-    globalCfg.colorScheme = loadedScheme;
-    globalCfg.save(themeDirPath);
+    // Keep the saved snapshot in sync so Reset behaves correctly afterwards.
+    for (auto it = workingConfig.begin(); it != workingConfig.end(); ++it) {
+        savedConfig[it.key()] = it.value();
+    }
+
+    ThemeConfig globalCfg = ThemeConfig::fromThemeDir(saveDir);
+    globalCfg.colorScheme = loadedScheme;
+    globalCfg.save(saveDir);
 
-    savedConfig[loadedScheme] = cfg;
-    workingConfig[loadedScheme] = cfg;
     themeManager->reloadCurrentTheme();
     accept();
 }
@@ -278,18 +319,40 @@ void PaletteEditorDialog::onReset()
 {
     workingConfig[loadedScheme] = savedConfig[loadedScheme];
     paletteGrid->loadPalette(savedConfig[loadedScheme]);
+    seedAccentFromScheme(loadedScheme);
 }
 
 void PaletteEditorDialog::onRevertToDefault()
 {
-    PaletteConfig def = PaletteConfig::fromDefault(themeDirPath, loadedScheme);
+    // Delete this scheme's custom palette file so the theme falls back to its
+    // default (or, when it ships none, the application palette).
+    // Note: shipped defaults use palette-default-<scheme>.toml so this only
+    // removes user-written custom palette files; the theme author's defaults
+    // are left untouched.
+    QFile::remove(QDir(saveDir).absoluteFilePath(PaletteConfig::fileName(loadedScheme)));
+
+    // Reload the live theme so the revert takes effect immediately.
+    themeManager->reloadCurrentTheme();
+
+    // Reflect the resolved palette (theme default, else current app palette) in
+    // the editor so it no longer shows the deleted custom colours.
+    PaletteConfig def = ThemeManager::loadDefaultPaletteConfig(themeDirPath, themeName, loadedScheme);
     if (!def.hasPalette()) {
-        QMessageBox::information(this, tr("No default found"),
-                                 tr("No default palette file found for the \"%1\" scheme.").arg(loadedScheme));
-        return;
+        const QPalette appPal = qApp->palette();
+        for (auto group : {QPalette::Active, QPalette::Disabled, QPalette::Inactive}) {
+            for (int i = 0; i < QPalette::NColorRoles; ++i) {
+                auto role = static_cast<QPalette::ColorRole>(i);
+                if (role != QPalette::NoRole) {
+                    def.colors[group][role] = appPal.color(group, role);
+                }
+            }
+        }
     }
+
+    savedConfig[loadedScheme] = def;
     workingConfig[loadedScheme] = def;
     paletteGrid->loadPalette(def);
+    seedAccentFromScheme(loadedScheme);
 }
 
 void PaletteEditorDialog::changeEvent(QEvent *e)

cockatrice/src/interface/palette_editor/palette_editor_dialog.h

@@ -7,6 +7,8 @@
 #include <QFrame>
 #include <QMap>
 
+class QTimer;
+
 class QLabel;
 class QComboBox;
 class QDialogButtonBox;
@@ -48,7 +50,6 @@ private:
     QComboBox *schemeComboBox = nullptr;
     QDialogButtonBox *buttonBox = nullptr;
     QPushButton *resetBtn = nullptr;
-    QPushButton *applyBtn = nullptr;
     QPushButton *saveBtn = nullptr;
     QPushButton *closeBtn = nullptr;
     QPushButton *revertButton = nullptr;
@@ -57,10 +58,15 @@ private:
     QString themeDirPath;
     QString themeName;
     QString loadedScheme;
+    // Directory writes are directed to; may differ from themeDirPath when the
+    // latter is read-only (e.g. a built-in / system theme directory).
+    QString saveDir;
 
     QMap<QString, PaletteConfig> workingConfig;
     QMap<QString, PaletteConfig> savedConfig;
 
+    QTimer *autoApplyTimer = nullptr;
+
 protected:
     void changeEvent(QEvent *e) override;
 };

cockatrice/src/interface/palette_editor/palette_grid_widget.cpp

@@ -117,6 +117,7 @@ void PaletteGridWidget::buildGrid(QWidget *host)
         for (int col = 0; col < 3; ++col) {
             auto group = ALL_GROUPS[col];
             auto *btn = new ColorButton(host);
+            connect(btn, &ColorButton::colorChanged, this, [this] { emit paletteChanged(); });
             colorButtons[group][role] = btn;
             grid->addWidget(btn, row + 1, col + 1, Qt::AlignHCenter | Qt::AlignVCenter);
         }

cockatrice/src/interface/palette_editor/palette_grid_widget.h

@@ -22,6 +22,9 @@ public:
     void loadPalette(const PaletteConfig &cfg);
     PaletteConfig currentPaletteConfig() const;
 
+signals:
+    void paletteChanged();
+
 private:
     void buildGrid(QWidget *host);
     void changeEvent(QEvent *e);

cockatrice/src/interface/palette_editor/quick_setup_panel.cpp

@@ -3,7 +3,6 @@
 #include <QApplication>
 #include <QHBoxLayout>
 #include <QLabel>
-#include <QPushButton>
 #include <QSlider>
 
 QuickSetupPanel::QuickSetupPanel(QWidget *parent) : QWidget(parent)
@@ -41,8 +40,6 @@ QuickSetupPanel::QuickSetupPanel(QWidget *parent) : QWidget(parent)
     intensityPercentageLabel->setFixedWidth(34);
     intensityPercentageLabel->setAlignment(Qt::AlignRight | Qt::AlignVCenter);
 
-    generateButton = new QPushButton(this);
-
     layout->addWidget(heading);
     layout->addSpacing(6);
     layout->addWidget(accentLabel);
@@ -54,12 +51,13 @@ QuickSetupPanel::QuickSetupPanel(QWidget *parent) : QWidget(parent)
     layout->addWidget(labelHigh);
     layout->addWidget(intensityPercentageLabel);
     layout->addStretch();
-    layout->addWidget(generateButton);
 
-    connect(intensitySlider, &QSlider::valueChanged, this,
-            [this](int v) { intensityPercentageLabel->setText(tr("%1%").arg(v)); });
-    connect(generateButton, &QPushButton::clicked, this,
-            [this] { emit generateRequested(accentButton->getColor(), intensitySlider->value()); });
+    connect(intensitySlider, &QSlider::valueChanged, this, [this](int v) {
+        intensityPercentageLabel->setText(tr("%1%").arg(v));
+        emit valueChanged(accentButton->getColor(), v);
+    });
+    connect(accentButton, &ColorButton::colorChanged, this,
+            [this](const QColor &c) { emit valueChanged(c, intensitySlider->value()); });
     retranslateUi();
 }
 
@@ -77,10 +75,6 @@ void QuickSetupPanel::retranslateUi()
                                    "30–70  Accented — buttons, tooltips, and borders join in\n"
                                    "70–100 Full colour — backgrounds, everything"));
     intensityPercentageLabel->setText(tr("70%"));
-
-    generateButton->setText(tr("Generate ↓"));
-    generateButton->setToolTip(tr("Derive all palette roles from the accent colour above.\n"
-                                  "Fine-tune individual colours in the grid afterwards."));
 }
 
 QColor QuickSetupPanel::accentColor() const
@@ -95,5 +89,7 @@ int QuickSetupPanel::intensity() const
 
 void QuickSetupPanel::setAccentColor(const QColor &c)
 {
+    accentButton->blockSignals(true);
     accentButton->setColor(c);
-}
+    accentButton->blockSignals(false);
+}

cockatrice/src/interface/palette_editor/quick_setup_panel.h

@@ -5,7 +5,6 @@
 
 #include <QWidget>
 
-class QPushButton;
 class QHBoxLayout;
 class QLabel;
 class QSlider;
@@ -16,12 +15,11 @@ class QSlider;
  *
  * The panel contains:
  * - an accent color picker,
- * - an intensity slider,
- * - and a generate button.
+ * - an intensity slider.
  *
- * When the user clicks the generate button, the panel emits
- * generateRequested() with the currently selected accent color
- * and intensity value.
+ * Whenever either value changes the panel emits valueChanged() with
+ * the current accent colour and intensity, which the parent dialog
+ * uses to auto-apply the generated palette.
  *
  * Typically used together with PaletteGenerator::fromAccent()
  * to quickly generate color schemes from a chosen accent color.
@@ -71,12 +69,12 @@ public:
 
 signals:
     /**
-     * @brief Emitted when the user requests palette generation.
+     * @brief Emitted whenever the accent colour or intensity changes.
      *
-     * @param accent The selected accent color.
-     * @param intensity The selected intensity value.
+     * The parent dialog consumes this to auto-apply the generated palette
+     * without requiring an explicit Generate click.
      */
-    void generateRequested(QColor accent, int intensity);
+    void valueChanged(QColor accent, int intensity);
 
 private:
     QHBoxLayout *layout;
@@ -88,7 +86,6 @@ private:
     QLabel *labelHigh;
     QSlider *intensitySlider;
     QLabel *intensityPercentageLabel;
-    QPushButton *generateButton;
 };
 
-#endif // COCKATRICE_QUICK_SETUP_PANEL_H
+#endif // COCKATRICE_QUICK_SETUP_PANEL_H