diff --git a/solitaire_data/src/settings.rs b/solitaire_data/src/settings.rs index 8e8b63b..c670712 100644 --- a/solitaire_data/src/settings.rs +++ b/solitaire_data/src/settings.rs @@ -161,8 +161,10 @@ pub struct Settings { /// Identifier of the active card-art theme. Matches `meta.id` from /// the theme's `theme.ron` manifest. `"dark"` and `"classic"` are /// always present; user-supplied themes register under their own ids. - /// Older `settings.json` files that stored `"default"` or `"classic"` - /// are migrated to `"dark"` by [`Settings::sanitized`]. + /// Older `settings.json` files that stored `"default"` (the + /// 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")] pub selected_theme_id: String, /// Set to `true` once the achievement-onboarding info-toast has been diff --git a/solitaire_engine/src/theme/plugin.rs b/solitaire_engine/src/theme/plugin.rs index 9d400d5..8225908 100644 --- a/solitaire_engine/src/theme/plugin.rs +++ b/solitaire_engine/src/theme/plugin.rs @@ -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 /// `Settings::selected_theme_id` if available, falling back to the -/// embedded default. The actual rasterisation runs asynchronously on -/// the asset task pool; the sync system below picks up the -/// `LoadedWithDependencies` event when every face + back is ready. +/// fresh-install default from `solitaire_data::Settings`. The actual +/// rasterisation runs asynchronously on the asset task pool; the sync +/// system below picks up the `LoadedWithDependencies` event when every +/// face + back is ready. fn load_initial_theme( asset_server: Res, settings: Option>, mut commands: Commands, ) { + let default_id = solitaire_data::Settings::default().selected_theme_id; let id = settings .as_deref() .map(|s| s.0.selected_theme_id.as_str()) - .unwrap_or("classic"); - let url = bundled_theme_url(id) - .map(str::to_string) - .unwrap_or_else(|| format!("themes://{id}/theme.ron")); - let handle: Handle = asset_server.load(url); + .unwrap_or(&default_id); + let handle: Handle = asset_server.load(theme_manifest_url(id)); commands.insert_resource(ActiveTheme(handle)); } @@ -162,10 +170,7 @@ fn react_to_settings_theme_change( return; } - let url = bundled_theme_url(new_id) - .map(str::to_string) - .unwrap_or_else(|| format!("themes://{new_id}/theme.ron")); - let handle: Handle = asset_server.load(url); + let handle: Handle = asset_server.load(theme_manifest_url(new_id)); 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()); } -/// Switches the active theme to the one served at -/// `themes:///theme.ron`. Returns the new `Handle` -/// so callers can poll `Assets` if they want to wait for -/// the load before changing UI state. +/// Switches the active theme to `theme_id` — the embedded source for +/// the bundled `"dark"` / `"classic"` themes, `themes://` for user +/// themes. Returns the new `Handle` so callers can poll +/// `Assets` if they want to wait for the load before +/// changing UI state. /// /// The handle is also written to the [`ActiveTheme`] resource — the /// per-frame sync system picks up the `LoadedWithDependencies` event @@ -266,8 +272,7 @@ pub fn set_theme( asset_server: &AssetServer, theme_id: &str, ) -> Handle { - let url = format!("themes://{theme_id}/theme.ron"); - let handle: Handle = asset_server.load(url); + let handle: Handle = asset_server.load(theme_manifest_url(theme_id)); commands.insert_resource(ActiveTheme(handle.clone())); handle } @@ -475,13 +480,43 @@ mod tests { } #[test] - fn set_theme_url_format_matches_themes_source() { - // The format string is the only behavioural surface of - // set_theme that doesn't require an App. We assert the URL - // shape so a future refactor doesn't accidentally change the - // path layout. - let url = format!("themes://{}/theme.ron", "user_uploaded"); - assert_eq!(url, "themes://user_uploaded/theme.ron"); + fn theme_manifest_url_routes_bundled_ids_to_embedded_source() { + // The bundled themes live in the binary, not in the user + // themes directory — resolving them to `themes://` would + // NotFound and silently leave the previous theme active. + assert_eq!( + theme_manifest_url("dark"), + 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