From 4126d69cbf0ce31285488b8c79b798effda3b6d9 Mon Sep 17 00:00:00 2001 From: will wade Date: Sat, 20 Jun 2026 05:47:21 +0100 Subject: [PATCH] Rework RFC 0007 appearance model: stateful mode + dual prefs, fix persistence leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback (PR #24) identified three problems with the v1 stateless "switch to my companion" design: 1. Persistence bug: dasher_set_appearance called dasher_set_palette, which writes the persistent SP_COLOUR_ID, so the user's explicit palette choice was clobbered by every auto-switch and lost across restarts. 2. No System/Light/Dark mode concept — every frontend had to reinvent the mode toggle. 3. 1:1 companion pairing locked users into (Rainbow <-> Rainbow Dark) with no way to mix e.g. Rainbow light + TurboLUT Dark. This reworks the model to own appearance state at the C API layer (not in the engine Parameters system — appearance is a shell/canvas concern): - dasher_get/set_appearance_mode: SYSTEM (default) / LIGHT / DARK. - dasher_get/set_system_appearance: transient OS appearance input (SYSTEM mode). - dasher_get/set_light_palette + dasher_get/set_dark_palette: two independent persisted preferences (enables mixing). - dasher_set_user_palette: sets the current-side pref and defaults the other side to the chosen palette's companion. - Resolution (mode + system + prefs -> SP_COLOUR_ID) lives in DasherCore and runs at create + on every change, so dasher_get_current_palette always reflects the resolved palette. - State persists to /appearance_settings.xml (pugixml sidecar); the preferences — not the transient active palette — are the source of truth, so auto-switching can never overwrite the user's choice (bug #1 fixed). - dasher_set_palette now routes through dasher_set_user_palette so existing pickers stay correct within the model. dasher_set_appearance is removed. Kept (not redundant): the palette appearance/companion XML metadata and dasher_get_palette_appearance / dasher_find_companion_palette — still needed for defaulting the dark preference and grouping the picker. Tests: mode-system, forced-mode, independent-prefs, and a persistence regression that asserts the user's light choice survives a dark auto-switch across a restart. Full suite green (21/21); clang-format clean. RFC text in dasher-project/governance#9 revised to match. Signed-off-by: will wade --- docs/C_API.md | 28 +++- src/CAPI.cpp | 260 ++++++++++++++++++++++++++++++-------- src/dasher.h | 53 +++++--- tests/test_color_math.cpp | 115 ++++++++++++++--- 4 files changed, 360 insertions(+), 96 deletions(-) diff --git a/docs/C_API.md b/docs/C_API.md index 10e41430..f95f4ac7 100644 --- a/docs/C_API.md +++ b/docs/C_API.md @@ -440,20 +440,36 @@ void dasher_set_palette(dasher_ctx* ctx, const char* palette_name); ### Appearance / dark mode (RFC 0007) -Palettes may declare an `appearance` (`light`/`dark`) and a `companion` (their opposite-appearance partner) in their XML. Use these to follow the OS light/dark preference without hardcoding name pairs per frontend. +DasherCore owns a light/dark appearance model so frontends don't each reinvent the System/Light/Dark toggle, the companion lookup, or the palette-preference storage. State persists to `/appearance_settings.xml`. The active palette (returned by `dasher_get_current_palette`) is *derived* from mode + system input + preferences, so an auto-switch can never overwrite the user's explicit choice across restarts. ```c -int dasher_get_palette_appearance(dasher_ctx* ctx, int index); // 0=unspecified, 1=light, 2=dark, -1=oor +int dasher_get_palette_appearance(dasher_ctx* ctx, int index); // 0=unspecified,1=light,2=dark,-1=oor const char* dasher_find_companion_palette(dasher_ctx* ctx, const char* palette_name); // NULL if none -int dasher_set_appearance(dasher_ctx* ctx, int appearance); // 1=light, 2=dark; 0 on success, -1 if no companion + +int dasher_get_appearance_mode(dasher_ctx* ctx); // 0=system,1=light,2=dark +void dasher_set_appearance_mode(dasher_ctx* ctx, int mode); +int dasher_get_system_appearance(dasher_ctx* ctx); // 1=light,2=dark (transient) +void dasher_set_system_appearance(dasher_ctx* ctx, int appearance); + +const char* dasher_get_light_palette(dasher_ctx* ctx); // persisted preferences +const char* dasher_get_dark_palette(dasher_ctx* ctx); +void dasher_set_light_palette(dasher_ctx* ctx, const char* name); +void dasher_set_dark_palette(dasher_ctx* ctx, const char* name); +void dasher_set_user_palette(dasher_ctx* ctx, const char* name); // sets current side + defaults other ``` -`dasher_set_appearance` switches the current palette to its companion that matches the requested appearance; if the current palette already matches it is a no-op. If no companion is available it returns `-1` and leaves the palette unchanged (the user's explicit choice is respected). Companion lookup is bidirectional, so legacy palettes without metadata are still paired with a dark companion that names them. +- **Mode** `SYSTEM` follows `dasher_set_system_appearance`; `LIGHT`/`DARK` force that side. +- **Two preferences** (`light_palette`, `dark_palette`) are stored independently, so a user can mix — e.g. Rainbow for light, TurboLUT Dark for dark. `dasher_set_user_palette` sets the current effective side and defaults the other to the chosen palette's companion. +- `dasher_set_palette` routes through `dasher_set_user_palette`, so existing pickers stay correct within the model. -Typical frontend usage on an OS appearance change: +Typical frontend usage: ```c -dasher_set_appearance(ctx, is_dark ? 2 : 1); +// On launch and whenever the OS appearance changes: +dasher_set_system_appearance(ctx, os_is_dark ? 2 : 1); + +// Settings UI: System / Light / Dark control +dasher_set_appearance_mode(ctx, mode); // 0=system,1=light,2=dark ``` ## Game Mode diff --git a/src/CAPI.cpp b/src/CAPI.cpp index 26ba06a7..4550770d 100644 --- a/src/CAPI.cpp +++ b/src/CAPI.cpp @@ -26,6 +26,8 @@ #include #include +#include "pugixml.hpp" + // ── UTF-8 / text boundary helpers ────────────────────────────────────────── namespace { @@ -339,6 +341,17 @@ struct dasher_ctx { std::string dataDir; std::string userDir; std::string stringBuf; + + // Appearance model state (RFC 0007). Lives at the C API layer — appearance + // is a shell/canvas concern, not a DasherCore engine parameter. Persisted to + // /appearance_settings.xml. The active palette (SP_COLOUR_ID) is + // derived from these via resolveAppearance(), so an auto-switch can never + // overwrite the user's explicit preference. + int appearanceMode = 0; // 0=system, 1=light, 2=dark + int systemAppearance = 1; // transient OS input: 1=light, 2=dark + std::string lightPalette; // user's preferred palette for light appearance + std::string darkPalette; // user's preferred palette for dark appearance + bool appearanceLoaded = false; dasher_output_callback outputCb = nullptr; void* outputCbUserData = nullptr; dasher_message_callback messageCb = nullptr; @@ -492,6 +505,111 @@ struct dasher_ctx { // ── C API implementation ────────────────────────────────────────────────── +// Appearance model helpers (RFC 0007). Defined outside `extern "C"` because they +// use C++ types (std::string, pugixml). Called by the C-linkage API functions +// and by dasher_create below. +namespace { +// Bidirectional companion lookup. Returns the opposite-appearance partner +// palette, or nullptr if none. Explicit `companion` first; then a reverse scan +// so legacy palettes without metadata are still paired. +const Dasher::ColorPalette* companionLookup(Dasher::CColorIO* colorIO, const std::string& name) { + if (!colorIO) return nullptr; + const Dasher::ColorPalette* p = colorIO->FindPalette(name); + if (!p || p->PaletteName != name) return nullptr; // FindPalette falls back to default + + if (!p->CompanionName.empty()) { + const Dasher::ColorPalette* q = colorIO->FindPalette(p->CompanionName); + if (q && q->PaletteName == p->CompanionName && q != p) return q; + } + const auto* all = colorIO->GetKnownPalettes(); + for (const auto& [n, q] : *all) { + if (q == p) continue; + if (q->CompanionName == name) return q; + } + return nullptr; +} + +// Effective appearance (1=light, 2=dark) from mode + transient system input. +int effectiveAppearanceValue(const dasher_ctx* ctx) { + if (ctx->appearanceMode == 1) return 1; // forced light + if (ctx->appearanceMode == 2) return 2; // forced dark + return ctx->systemAppearance; // follow system (defaults to light) +} + +// Recompute the active palette from mode + system + preferences and write it to +// SP_COLOUR_ID (what the canvas renders). The persisted preferences are the +// source of truth, so this can never clobber the user's explicit choice. +void resolveAppearance(dasher_ctx* ctx) { + if (!ctx || !ctx->intf) return; + int eff = effectiveAppearanceValue(ctx); + std::string target = (eff == 1) ? ctx->lightPalette : ctx->darkPalette; + if (target.empty()) target = (eff == 1) ? ctx->darkPalette : ctx->lightPalette; // other side + if (target.empty()) return; // nothing chosen yet; leave the engine default + + std::string current = ctx->intf->GetStringParameter(Dasher::SP_COLOUR_ID); + if (current != target) ctx->intf->SetStringParameter(Dasher::SP_COLOUR_ID, target); +} + +std::string appearanceSettingsPath(const dasher_ctx* ctx) { + std::string p = ctx->userDir; +#ifdef _WIN32 + p += "\\appearance_settings.xml"; +#else + p += "/appearance_settings.xml"; +#endif + return p; +} + +// Load mode + light/dark preferences from the sidecar. Non-fatal on any error. +void loadAppearanceSettings(dasher_ctx* ctx) { + if (ctx->appearanceLoaded) return; + ctx->appearanceLoaded = true; + std::string path = appearanceSettingsPath(ctx); + pugi::xml_document doc; + pugi::xml_parse_result res = doc.load_file(path.c_str()); + if (!res) return; // missing/unreadable: leave defaults + pugi::xml_node root = doc.child("appearance"); + if (!root) return; + ctx->appearanceMode = root.attribute("mode").as_int(0); + ctx->lightPalette = root.attribute("light").as_string(""); + ctx->darkPalette = root.attribute("dark").as_string(""); + if (ctx->appearanceMode < 0 || ctx->appearanceMode > 2) ctx->appearanceMode = 0; +} + +// Persist mode + light/dark preferences to the sidecar. Non-fatal on any error. +void saveAppearanceSettings(dasher_ctx* ctx) { + if (!ctx || ctx->userDir.empty()) return; + pugi::xml_document doc; + pugi::xml_node root = doc.append_child("appearance"); + root.append_attribute("mode") = ctx->appearanceMode; + root.append_attribute("light") = ctx->lightPalette.c_str(); + root.append_attribute("dark") = ctx->darkPalette.c_str(); + doc.save_file(appearanceSettingsPath(ctx).c_str()); +} + +// Ensure the model is initialised: on first use, seed the light preference from +// the engine's current palette and default the dark side to its companion. +void ensureAppearanceInitialised(dasher_ctx* ctx) { + if (ctx->appearanceLoaded) { + resolveAppearance(ctx); + return; + } + loadAppearanceSettings(ctx); + if (ctx->lightPalette.empty() && ctx->darkPalette.empty()) { + // Fresh start: adopt whatever palette the engine loaded as the light + // preference, and default the dark side to its companion. + std::string current = ctx->intf->GetStringParameter(Dasher::SP_COLOUR_ID); + ctx->lightPalette = current; + if (auto* colorIO = ctx->intf->GetColorIO()) { + if (const Dasher::ColorPalette* comp = companionLookup(colorIO, current)) + ctx->darkPalette = comp->PaletteName; + } + saveAppearanceSettings(ctx); + } + resolveAppearance(ctx); +} +} // namespace + extern "C" { static std::string s_errorString; @@ -506,7 +624,6 @@ DASHER_API dasher_ctx* dasher_create(const char* data_dir, const char* user_dir, if (out_error) *out_error = s_errorString.data(); return nullptr; } - #ifdef _WIN32 setlocale(LC_CTYPE, ".UTF8"); #endif @@ -544,6 +661,11 @@ DASHER_API dasher_ctx* dasher_create(const char* data_dir, const char* user_dir, return nullptr; } + // Load persisted appearance preferences and resolve the active palette, so + // the user's explicit choice is reflected immediately on startup (RFC 0007). + loadAppearanceSettings(ctx); + resolveAppearance(ctx); + return ctx; } @@ -986,46 +1108,17 @@ DASHER_API void dasher_set_palette(dasher_ctx* ctx, const char* palette_name) { ctx->intf->KeyUp(nowMs(), Dasher::Keys::Primary_Input); ctx->mouseDown = false; } - ctx->intf->SetStringParameter(Dasher::SP_COLOUR_ID, std::string(palette_name)); + // Route through the appearance model (RFC 0007): this sets the user's + // preference for the current effective appearance (and defaults the other + // side to the companion), then resolves. This keeps palette selection + // consistent with light/dark mode and prevents the persistence leak that + // direct SP_COLOUR_ID writes would cause. If the model hasn't been touched + // yet, ensureAppearanceInitialised seeds it first. + dasher_set_user_palette(ctx, palette_name); } // ── Appearance / dark mode (RFC 0007) ────────────────────────────────────── - -namespace { -// Bidirectional companion lookup (RFC 0007). Returns the opposite-appearance -// partner palette, or nullptr if none. If the palette declares an explicit -// `companion`, that is used; otherwise we scan for a palette that declares this -// one as its companion (so legacy palettes without metadata are still paired). -const Dasher::ColorPalette* companionLookup(Dasher::CColorIO* colorIO, const std::string& name) { - if (!colorIO) return nullptr; - const Dasher::ColorPalette* p = colorIO->FindPalette(name); - if (!p || p->PaletteName != name) return nullptr; // FindPalette falls back to default - - // Forward: explicit companion declared and resolvable. - if (!p->CompanionName.empty()) { - const Dasher::ColorPalette* q = colorIO->FindPalette(p->CompanionName); - if (q && q->PaletteName == p->CompanionName && q != p) return q; - } - // Reverse: some other palette declares this one as its companion. - const auto* all = colorIO->GetKnownPalettes(); - for (const auto& [n, q] : *all) { - if (q == p) continue; - if (q->CompanionName == name) return q; - } - return nullptr; -} - -// Effective appearance, treating an unspecified palette whose companion is dark -// as effectively light (the common legacy-palette case). -Dasher::ColorPalette::Appearance effectiveAppearance(Dasher::CColorIO* colorIO, const Dasher::ColorPalette* p) { - using App = Dasher::ColorPalette::Appearance; - if (!p) return App::Unspecified; - if (p->AppearanceValue != App::Unspecified) return p->AppearanceValue; - const Dasher::ColorPalette* comp = companionLookup(colorIO, p->PaletteName); - if (comp && comp->AppearanceValue == App::Dark) return App::Light; - return App::Unspecified; -} -} // namespace +// (Helpers live above `extern "C"` — they return std::string / use C++ types.) DASHER_API int dasher_get_palette_appearance(dasher_ctx* ctx, int index) { if (!ctx || !ctx->intf) return -1; @@ -1048,27 +1141,83 @@ DASHER_API const char* dasher_find_companion_palette(dasher_ctx* ctx, const char return ctx->tlString.c_str(); } -DASHER_API int dasher_set_appearance(dasher_ctx* ctx, int appearance) { - if (!ctx || !ctx->intf) return -1; - if (appearance != 1 && appearance != 2) return -1; - using App = Dasher::ColorPalette::Appearance; - App target = (appearance == 1) ? App::Light : App::Dark; +DASHER_API int dasher_get_appearance_mode(dasher_ctx* ctx) { + if (!ctx) return 0; + return ctx->appearanceMode; +} - auto colorIO = ctx->intf->GetColorIO(); - if (!colorIO) return -1; - std::string currentName = ctx->intf->GetStringParameter(Dasher::SP_COLOUR_ID); - const Dasher::ColorPalette* current = colorIO->FindPalette(currentName); +DASHER_API void dasher_set_appearance_mode(dasher_ctx* ctx, int mode) { + if (!ctx || mode < 0 || mode > 2) return; + ensureAppearanceInitialised(ctx); + if (ctx->appearanceMode == mode) return; + ctx->appearanceMode = mode; + saveAppearanceSettings(ctx); + resolveAppearance(ctx); +} - // Candidate 1: the current palette already matches. - if (current && effectiveAppearance(colorIO, current) == target) return 0; +DASHER_API int dasher_get_system_appearance(dasher_ctx* ctx) { + if (!ctx) return 1; + return ctx->systemAppearance; +} - // Candidate 2: the current palette's companion matches. - const Dasher::ColorPalette* comp = companionLookup(colorIO, currentName); - if (comp && effectiveAppearance(colorIO, comp) == target) { - dasher_set_palette(ctx, comp->PaletteName.c_str()); - return 0; +DASHER_API void dasher_set_system_appearance(dasher_ctx* ctx, int appearance) { + if (!ctx || (appearance != 1 && appearance != 2)) return; + ensureAppearanceInitialised(ctx); + if (ctx->systemAppearance == appearance) return; + ctx->systemAppearance = appearance; + // Only matters in SYSTEM mode, but resolve is cheap and keeps state consistent. + if (ctx->appearanceMode == 0) resolveAppearance(ctx); +} + +DASHER_API const char* dasher_get_light_palette(dasher_ctx* ctx) { + if (!ctx || !ctx->intf) return ""; + ensureAppearanceInitialised(ctx); + ctx->tlString = ctx->lightPalette; + return ctx->tlString.c_str(); +} + +DASHER_API const char* dasher_get_dark_palette(dasher_ctx* ctx) { + if (!ctx || !ctx->intf) return ""; + ensureAppearanceInitialised(ctx); + ctx->tlString = ctx->darkPalette; + return ctx->tlString.c_str(); +} + +DASHER_API void dasher_set_light_palette(dasher_ctx* ctx, const char* name) { + if (!ctx || !ctx->intf || !name) return; + ensureAppearanceInitialised(ctx); + ctx->lightPalette = name; + saveAppearanceSettings(ctx); + resolveAppearance(ctx); +} + +DASHER_API void dasher_set_dark_palette(dasher_ctx* ctx, const char* name) { + if (!ctx || !ctx->intf || !name) return; + ensureAppearanceInitialised(ctx); + ctx->darkPalette = name; + saveAppearanceSettings(ctx); + resolveAppearance(ctx); +} + +DASHER_API void dasher_set_user_palette(dasher_ctx* ctx, const char* name) { + if (!ctx || !ctx->intf || !name) return; + ensureAppearanceInitialised(ctx); + int eff = effectiveAppearanceValue(ctx); + if (eff == 1) + ctx->lightPalette = name; + else + ctx->darkPalette = name; + + // Default the other side to the chosen palette's companion if unset, so the + // user gets a sensible matching variant without configuring both sides. + std::string& other = (eff == 1) ? ctx->darkPalette : ctx->lightPalette; + if (other.empty() || other == ctx->lightPalette || other == ctx->darkPalette) { + if (auto* colorIO = ctx->intf->GetColorIO()) { + if (const Dasher::ColorPalette* comp = companionLookup(colorIO, name)) other = comp->PaletteName; + } } - return -1; // no suitable companion; leave the current palette unchanged + saveAppearanceSettings(ctx); + resolveAppearance(ctx); } // ── Alphabets ────────────────────────────────────────────────────────────── @@ -1158,6 +1307,7 @@ DASHER_API const char* dasher_game_get_wrong_text(dasher_ctx* ctx) { DASHER_API void dasher_save_settings(dasher_ctx* ctx) { if (!ctx || !ctx->settings) return; ctx->settings->Save(); + if (ctx->appearanceLoaded) saveAppearanceSettings(ctx); // RFC 0007 sidecar } // ── Localization ────────────────────────────────────────────────────────── diff --git a/src/dasher.h b/src/dasher.h index c834cd34..5779e400 100644 --- a/src/dasher.h +++ b/src/dasher.h @@ -223,30 +223,47 @@ DASHER_API void dasher_set_palette(dasher_ctx* ctx, const char* palette_name); // ── Appearance / dark mode (RFC 0007) ───────────────────────────────────── // -// Palettes may declare an `appearance` ("light" or "dark") and a `companion` -// (the name of their opposite-appearance partner) in their XML. Frontends can -// use these to follow the OS light/dark preference by switching to the current -// palette's companion, without each frontend hardcoding name pairs. +// DasherCore owns a light/dark appearance model at the C API layer so frontends +// don't each reinvent the System/Light/Dark toggle, the companion lookup, or +// the palette-preference storage. State persists to / +// appearance_settings.xml. The active palette (returned by +// dasher_get_current_palette) is *derived* from the mode + system input + +// preferences, so an auto-switch can never overwrite the user's explicit choice. // -// Legacy palettes without metadata report appearance 0 (unspecified) but are -// still paired via a bidirectional companion lookup: if palette A declares -// companion="B", then B's effective companion is A. +// Palettes may declare `appearance` ("light"/"dark") and a `companion` (their +// opposite-appearance partner) in their XML. Companion lookup is bidirectional, +// so legacy palettes without metadata are still paired with a dark companion +// that names them. -// Get the appearance classification of a palette. -// Returns: 0 = unspecified, 1 = light, 2 = dark, -1 = index out of range. +// Classify a palette. Returns: 0 = unspecified, 1 = light, 2 = dark, -1 = oor. DASHER_API int dasher_get_palette_appearance(dasher_ctx* ctx, int index); -// Find the companion (opposite-appearance) palette for the given palette name. -// Lookup is bidirectional (see above). Returns the companion name (valid until -// the next API call), or NULL if the palette has no companion. +// Find the companion (opposite-appearance) palette for the given name. +// Bidirectional lookup. Returns the companion name (valid until the next API +// call), or NULL if the palette has no companion. DASHER_API const char* dasher_find_companion_palette(dasher_ctx* ctx, const char* palette_name); -// Switch to the companion of the current palette that matches the requested -// appearance. appearance: 1 = light, 2 = dark. -// If the current palette already matches, this is a no-op and returns 0. -// Returns 0 on success, -1 if no suitable companion is available (in which case -// the current palette is left unchanged — the user's explicit choice is respected). -DASHER_API int dasher_set_appearance(dasher_ctx* ctx, int appearance); +// Appearance mode (persisted). SYSTEM follows dasher_set_system_appearance; +// LIGHT/DARK are explicit overrides. Returns 0=system, 1=light, 2=dark. +DASHER_API int dasher_get_appearance_mode(dasher_ctx* ctx); +DASHER_API void dasher_set_appearance_mode(dasher_ctx* ctx, int mode); + +// Transient OS appearance input (not persisted). Frontends call this when the +// OS reports a change. Consulted only when mode == SYSTEM. 1=light, 2=dark. +DASHER_API int dasher_get_system_appearance(dasher_ctx* ctx); +DASHER_API void dasher_set_system_appearance(dasher_ctx* ctx, int appearance); + +// User's preferred palette for each appearance (persisted). A picker should set +// the side matching the current effective appearance (see dasher_set_user_palette). +DASHER_API const char* dasher_get_light_palette(dasher_ctx* ctx); +DASHER_API const char* dasher_get_dark_palette(dasher_ctx* ctx); +DASHER_API void dasher_set_light_palette(dasher_ctx* ctx, const char* name); +DASHER_API void dasher_set_dark_palette(dasher_ctx* ctx, const char* name); + +// Convenience for the picker: sets the preference for the current effective +// appearance, and defaults the other side to the chosen palette's companion if +// that side has not been customised. (dasher_set_palette routes through this.) +DASHER_API void dasher_set_user_palette(dasher_ctx* ctx, const char* name); // ── Alphabets ───────────────────────────────────────────────────────────── diff --git a/tests/test_color_math.cpp b/tests/test_color_math.cpp index a981e191..54f43c5f 100644 --- a/tests/test_color_math.cpp +++ b/tests/test_color_math.cpp @@ -163,36 +163,114 @@ TEST(color_palette_companion_lookup) { printf("v color_palette_companion_lookup passed\n"); } -TEST(color_palette_set_appearance) { +TEST(color_palette_appearance_mode_system) { dasher_ctx* ctx = create_isolated_context(); ASSERT(ctx); dasher_set_screen_size(ctx, 800, 600); - dasher_set_palette(ctx, "Rainbow"); + // Fresh: mode defaults to SYSTEM (0), system appearance to LIGHT (1). + ASSERT_EQ(dasher_get_appearance_mode(ctx), 0); + ASSERT_EQ(dasher_get_system_appearance(ctx), 1); + + // Picker sets the user palette. In SYSTEM+light the light side is "Rainbow"; + // the dark side defaults to its companion "Rainbow Dark". + dasher_set_user_palette(ctx, "Rainbow"); + ASSERT_STR_EQ(dasher_get_light_palette(ctx), "Rainbow"); + ASSERT_STR_EQ(dasher_get_dark_palette(ctx), "Rainbow Dark"); ASSERT_STR_EQ(dasher_get_current_palette(ctx), "Rainbow"); - // Request dark -> switch to Rainbow Dark. - ASSERT_EQ(dasher_set_appearance(ctx, 2), 0); + // OS goes dark -> active switches to the dark preference. + dasher_set_system_appearance(ctx, 2); ASSERT_STR_EQ(dasher_get_current_palette(ctx), "Rainbow Dark"); + // The light preference is NOT clobbered by the auto-switch (the bug fix). + ASSERT_STR_EQ(dasher_get_light_palette(ctx), "Rainbow"); - // Requesting dark again is a no-op (already on the dark variant). - ASSERT_EQ(dasher_set_appearance(ctx, 2), 0); - ASSERT_STR_EQ(dasher_get_current_palette(ctx), "Rainbow Dark"); + // OS goes light -> back to Rainbow. + dasher_set_system_appearance(ctx, 1); + ASSERT_STR_EQ(dasher_get_current_palette(ctx), "Rainbow"); - // Request light -> switch back to Rainbow. - ASSERT_EQ(dasher_set_appearance(ctx, 1), 0); + dasher_destroy(ctx); + printf("v color_palette_appearance_mode_system passed\n"); +} + +TEST(color_palette_appearance_forced_mode) { + dasher_ctx* ctx = create_isolated_context(); + ASSERT(ctx); + dasher_set_screen_size(ctx, 800, 600); + + dasher_set_user_palette(ctx, "Rainbow"); // light=Rainbow, dark=Rainbow Dark + dasher_set_system_appearance(ctx, 2); // system dark + + // Force LIGHT even though system is dark. + dasher_set_appearance_mode(ctx, 1); + ASSERT_EQ(dasher_get_appearance_mode(ctx), 1); ASSERT_STR_EQ(dasher_get_current_palette(ctx), "Rainbow"); - // A palette with no companion cannot switch -> -1, and is left unchanged. - dasher_set_palette(ctx, "Yellow on Blue"); - ASSERT_EQ(dasher_set_appearance(ctx, 1), -1); - ASSERT_STR_EQ(dasher_get_current_palette(ctx), "Yellow on Blue"); + // Force DARK. + dasher_set_appearance_mode(ctx, 2); + ASSERT_STR_EQ(dasher_get_current_palette(ctx), "Rainbow Dark"); + + // Back to SYSTEM follows the (still dark) system input. + dasher_set_appearance_mode(ctx, 0); + ASSERT_STR_EQ(dasher_get_current_palette(ctx), "Rainbow Dark"); - // Invalid appearance value -> -1. - ASSERT_EQ(dasher_set_appearance(ctx, 9), -1); + // Invalid mode is rejected. + dasher_set_appearance_mode(ctx, 9); + ASSERT_EQ(dasher_get_appearance_mode(ctx), 0); dasher_destroy(ctx); - printf("v color_palette_set_appearance passed\n"); + printf("v color_palette_appearance_forced_mode passed\n"); +} + +TEST(color_palette_appearance_independent_prefs) { + dasher_ctx* ctx = create_isolated_context(); + ASSERT(ctx); + dasher_set_screen_size(ctx, 800, 600); + + // The user may mix: Rainbow for light, TurboLUT Dark for dark (not 1:1). + dasher_set_light_palette(ctx, "Rainbow"); + dasher_set_dark_palette(ctx, "TurboLUT Dark"); + + dasher_set_appearance_mode(ctx, 1); // light + ASSERT_STR_EQ(dasher_get_current_palette(ctx), "Rainbow"); + dasher_set_appearance_mode(ctx, 2); // dark + ASSERT_STR_EQ(dasher_get_current_palette(ctx), "TurboLUT Dark"); + + dasher_destroy(ctx); + printf("v color_palette_appearance_independent_prefs passed\n"); +} + +TEST(color_palette_appearance_persistence) { + // Two contexts sharing one user dir prove the user's explicit choice + // survives an auto-switch across restarts (the persistence-bug regression). + char sharedDir[256]; + snprintf(sharedDir, sizeof(sharedDir), "%s/dasher_persist_%d_%d", dasher_temp_dir(), dasher_getpid(), + (int)(uintptr_t)sharedDir); + dasher_mkdir(sharedDir); + + // Session 1: user picks Rainbow (light); system goes dark so active becomes + // Rainbow Dark at close. + dasher_ctx* c1 = dasher_create(TEST_DATA_DIR, sharedDir, nullptr); + ASSERT(c1); + dasher_set_screen_size(c1, 800, 600); + dasher_set_user_palette(c1, "Rainbow"); + dasher_set_system_appearance(c1, 2); + ASSERT_STR_EQ(dasher_get_current_palette(c1), "Rainbow Dark"); + dasher_save_settings(c1); + dasher_destroy(c1); + + // Session 2: preferences must reload; active must re-resolve to the light + // preference (system input is transient and defaults to light), NOT stay + // stuck on the leaked "Rainbow Dark". + dasher_ctx* c2 = dasher_create(TEST_DATA_DIR, sharedDir, nullptr); + ASSERT(c2); + dasher_set_screen_size(c2, 800, 600); + ASSERT_STR_EQ(dasher_get_light_palette(c2), "Rainbow"); + ASSERT_STR_EQ(dasher_get_dark_palette(c2), "Rainbow Dark"); + ASSERT_STR_EQ(dasher_get_current_palette(c2), "Rainbow"); // original choice restored + dasher_destroy(c2); + + printf("v color_palette_appearance_persistence passed\n"); } int main() { @@ -208,7 +286,10 @@ int main() { test_color_palette_switch(); test_color_palette_appearance_classification(); test_color_palette_companion_lookup(); - test_color_palette_set_appearance(); + test_color_palette_appearance_mode_system(); + test_color_palette_appearance_forced_mode(); + test_color_palette_appearance_independent_prefs(); + test_color_palette_appearance_persistence(); printf("\nAll color math tests passed!\n"); return 0;