fix(engine): unify light/dark theme manifest URL resolution
Test / test (pull_request) Successful in 13m4s

- set_theme resolved every id to themes:// — the bundled dark/classic
  themes live at embedded://, so switching to them via the public API
  would NotFound and silently keep the old theme. All three load paths
  now share one theme_manifest_url resolver.
- load_initial_theme's settings-absent fallback was "classic", a
  leftover from v0.33 when classic was the default; it now derives from
  Settings::default() so it can't drift from the data crate again, with
  a test pinning the default id to a bundled theme.
- settings.rs doc no longer claims "classic" is migrated to "dark";
  only the pre-rename "default" id is rewritten.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
funman300
2026-07-07 12:25:35 -07:00
parent abf1312cf5
commit 018b69285d
2 changed files with 64 additions and 27 deletions
+4 -2
View File
@@ -161,8 +161,10 @@ pub struct Settings {
/// Identifier of the active card-art theme. Matches `meta.id` from /// Identifier of the active card-art theme. Matches `meta.id` from
/// the theme's `theme.ron` manifest. `"dark"` and `"classic"` are /// the theme's `theme.ron` manifest. `"dark"` and `"classic"` are
/// always present; user-supplied themes register under their own ids. /// always present; user-supplied themes register under their own ids.
/// Older `settings.json` files that stored `"default"` or `"classic"` /// Older `settings.json` files that stored `"default"` (the
/// are migrated to `"dark"` by [`Settings::sanitized`]. /// pre-rename id of the dark theme) are migrated to `"dark"` by
/// [`Settings::sanitized`]; `"classic"` is a valid player choice
/// and is never rewritten.
#[serde(default = "default_theme_id")] #[serde(default = "default_theme_id")]
pub selected_theme_id: String, pub selected_theme_id: String,
/// Set to `true` once the achievement-onboarding info-toast has been /// Set to `true` once the achievement-onboarding info-toast has been
+60 -25
View File
@@ -116,24 +116,32 @@ impl Plugin for ThemePlugin {
} }
} }
/// Resolves the manifest URL for `theme_id`: the bundled themes
/// (`"dark"`, `"classic"`) load from the embedded source; any other id
/// is a user theme served from the `themes://` directory source.
fn theme_manifest_url(theme_id: &str) -> String {
bundled_theme_url(theme_id)
.map(str::to_string)
.unwrap_or_else(|| format!("themes://{theme_id}/theme.ron"))
}
/// Kicks off the initial theme load — the one named by /// Kicks off the initial theme load — the one named by
/// `Settings::selected_theme_id` if available, falling back to the /// `Settings::selected_theme_id` if available, falling back to the
/// embedded default. The actual rasterisation runs asynchronously on /// fresh-install default from `solitaire_data::Settings`. The actual
/// the asset task pool; the sync system below picks up the /// rasterisation runs asynchronously on the asset task pool; the sync
/// `LoadedWithDependencies` event when every face + back is ready. /// system below picks up the `LoadedWithDependencies` event when every
/// face + back is ready.
fn load_initial_theme( fn load_initial_theme(
asset_server: Res<AssetServer>, asset_server: Res<AssetServer>,
settings: Option<Res<crate::settings_plugin::SettingsResource>>, settings: Option<Res<crate::settings_plugin::SettingsResource>>,
mut commands: Commands, mut commands: Commands,
) { ) {
let default_id = solitaire_data::Settings::default().selected_theme_id;
let id = settings let id = settings
.as_deref() .as_deref()
.map(|s| s.0.selected_theme_id.as_str()) .map(|s| s.0.selected_theme_id.as_str())
.unwrap_or("classic"); .unwrap_or(&default_id);
let url = bundled_theme_url(id) let handle: Handle<CardTheme> = asset_server.load(theme_manifest_url(id));
.map(str::to_string)
.unwrap_or_else(|| format!("themes://{id}/theme.ron"));
let handle: Handle<CardTheme> = asset_server.load(url);
commands.insert_resource(ActiveTheme(handle)); commands.insert_resource(ActiveTheme(handle));
} }
@@ -162,10 +170,7 @@ fn react_to_settings_theme_change(
return; return;
} }
let url = bundled_theme_url(new_id) let handle: Handle<CardTheme> = asset_server.load(theme_manifest_url(new_id));
.map(str::to_string)
.unwrap_or_else(|| format!("themes://{new_id}/theme.ron"));
let handle: Handle<CardTheme> = asset_server.load(url);
commands.insert_resource(ActiveTheme(handle)); commands.insert_resource(ActiveTheme(handle));
} }
@@ -252,10 +257,11 @@ fn apply_theme_to_card_image_set(theme: &CardTheme, image_set: &mut CardImageSet
image_set.theme_back = Some(theme.back.clone()); image_set.theme_back = Some(theme.back.clone());
} }
/// Switches the active theme to the one served at /// Switches the active theme to `theme_id` — the embedded source for
/// `themes://<theme_id>/theme.ron`. Returns the new `Handle<CardTheme>` /// the bundled `"dark"` / `"classic"` themes, `themes://` for user
/// so callers can poll `Assets<CardTheme>` if they want to wait for /// themes. Returns the new `Handle<CardTheme>` so callers can poll
/// the load before changing UI state. /// `Assets<CardTheme>` if they want to wait for the load before
/// changing UI state.
/// ///
/// The handle is also written to the [`ActiveTheme`] resource — the /// The handle is also written to the [`ActiveTheme`] resource — the
/// per-frame sync system picks up the `LoadedWithDependencies` event /// per-frame sync system picks up the `LoadedWithDependencies` event
@@ -266,8 +272,7 @@ pub fn set_theme(
asset_server: &AssetServer, asset_server: &AssetServer,
theme_id: &str, theme_id: &str,
) -> Handle<CardTheme> { ) -> Handle<CardTheme> {
let url = format!("themes://{theme_id}/theme.ron"); let handle: Handle<CardTheme> = asset_server.load(theme_manifest_url(theme_id));
let handle: Handle<CardTheme> = asset_server.load(url);
commands.insert_resource(ActiveTheme(handle.clone())); commands.insert_resource(ActiveTheme(handle.clone()));
handle handle
} }
@@ -475,13 +480,43 @@ mod tests {
} }
#[test] #[test]
fn set_theme_url_format_matches_themes_source() { fn theme_manifest_url_routes_bundled_ids_to_embedded_source() {
// The format string is the only behavioural surface of // The bundled themes live in the binary, not in the user
// set_theme that doesn't require an App. We assert the URL // themes directory — resolving them to `themes://` would
// shape so a future refactor doesn't accidentally change the // NotFound and silently leave the previous theme active.
// path layout. assert_eq!(
let url = format!("themes://{}/theme.ron", "user_uploaded"); theme_manifest_url("dark"),
assert_eq!(url, "themes://user_uploaded/theme.ron"); crate::assets::DARK_THEME_MANIFEST_URL
);
assert_eq!(
theme_manifest_url("classic"),
crate::assets::CLASSIC_THEME_MANIFEST_URL
);
}
#[test]
fn theme_manifest_url_routes_user_ids_to_themes_source() {
// We assert the URL shape so a future refactor doesn't
// accidentally change the path layout of the user-themes
// source.
assert_eq!(
theme_manifest_url("user_uploaded"),
"themes://user_uploaded/theme.ron"
);
}
#[test]
fn initial_theme_fallback_matches_settings_default() {
// `load_initial_theme` falls back to the data-crate default
// when `SettingsResource` is absent. That default must always
// name a bundled theme, or a fresh minimal setup would resolve
// to a nonexistent user theme (see the v0.33.0 "classic"
// fallback drift in CHANGELOG.md).
let default_id = solitaire_data::Settings::default().selected_theme_id;
assert!(
bundled_theme_url(&default_id).is_some(),
"default theme id {default_id:?} must be a bundled theme"
);
} }
/// Test 1: the bundled dark theme always has embedded SVG bytes /// Test 1: the bundled dark theme always has embedded SVG bytes