fix(data): route data_dir() through a per-platform shim so Android persists

dirs::data_dir() returns None on Android, which silently disabled
every persistence path (settings, stats, achievements, replays,
game-state, time-attack sessions, user themes). New
solitaire_data::platform::data_dir() shim falls through to
dirs::data_dir() on desktop and returns the per-app sandbox at
/data/data/com.solitairequest.app/files on Android — no JNI needed,
since the package id is pinned in
[package.metadata.android].

CLAUDE.md §10 already flagged this as a known pitfall; the shim
pays it down at the one chokepoint instead of per feature.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
funman300
2026-05-07 17:55:49 -07:00
parent f2d2119db5
commit 4b51e50203
8 changed files with 148 additions and 79 deletions
+1 -1
View File
@@ -15,7 +15,7 @@ const FILE_NAME: &str = "achievements.json";
/// Platform-specific default path for `achievements.json`. /// Platform-specific default path for `achievements.json`.
pub fn achievements_file_path() -> Option<PathBuf> { pub fn achievements_file_path() -> Option<PathBuf> {
dirs::data_dir().map(|d| d.join(APP_DIR_NAME).join(FILE_NAME)) crate::data_dir().map(|d| d.join(APP_DIR_NAME).join(FILE_NAME))
} }
/// Load achievements from an explicit path. Returns `Vec::new()` if the file /// Load achievements from an explicit path. Returns `Vec::new()` if the file
+3
View File
@@ -163,3 +163,6 @@ pub use replay::{
replay_history_path, save_replay_history_to, Replay, ReplayHistory, ReplayMove, replay_history_path, save_replay_history_to, Replay, ReplayHistory, ReplayMove,
REPLAY_HISTORY_CAP, REPLAY_HISTORY_SCHEMA_VERSION, REPLAY_SCHEMA_VERSION, REPLAY_HISTORY_CAP, REPLAY_HISTORY_SCHEMA_VERSION, REPLAY_SCHEMA_VERSION,
}; };
pub mod platform;
pub use platform::data_dir;
+92
View File
@@ -0,0 +1,92 @@
//! Per-platform resolution of the per-user data directory.
//!
//! The rest of `solitaire_data` (settings, stats, achievements,
//! replays, progress, game state) and the engine's user-themes
//! discovery all need a base path under which to nest
//! `solitaire_quest/<file>`. On desktop the right answer is
//! `dirs::data_dir()` (which resolves to platform-appropriate
//! locations: `~/.local/share` on Linux, `~/Library/Application
//! Support` on macOS, `%APPDATA%` on Windows). On Android the
//! `dirs` crate returns `None`, which would silently disable
//! every persistence path — settings, stats, replays, the lot.
//!
//! [`data_dir`] is a thin shim that returns the right base path
//! per target. Callers continue to append
//! `solitaire_quest/<file>` themselves, so the on-disk layout is
//! identical across platforms (the per-app Android sandbox makes
//! the extra `solitaire_quest/` segment harmless, and a `tar`
//! export from one platform deserialises cleanly on another).
//!
//! # Why hardcode on Android?
//!
//! The "proper" Android answer is JNI: call back into Java to
//! invoke `Activity.getFilesDir()`. That requires plumbing an
//! `AndroidApp` context through Bevy's startup hooks and a
//! per-call JNI bridge — meaningfully more code than the
//! sandbox-guaranteed `/data/data/<package>/files` path. The
//! package name `com.solitairequest.app` is fixed at compile
//! time in `solitaire_app/Cargo.toml`'s
//! `[package.metadata.android]` block, so a hardcoded path is
//! safe until that ever changes (at which point this constant
//! moves with it).
use std::path::PathBuf;
/// Hardcoded per-app private files directory on Android.
///
/// Matches `[package.metadata.android]` in `solitaire_app/Cargo.toml`.
/// The Android sandbox guarantees this path exists, is writable,
/// and is private to the app — no JNI needed. Update both this
/// constant and the Cargo metadata together if the package id
/// ever changes.
#[cfg(target_os = "android")]
const ANDROID_APP_FILES_DIR: &str = "/data/data/com.solitairequest.app/files";
/// Returns the per-user data directory for the current target,
/// or `None` if the platform doesn't expose one (rare; usually
/// indicates a broken `$HOME` or `$XDG_*` configuration on a
/// minimal Linux container).
///
/// Callers append `solitaire_quest/<file>` themselves. See the
/// module-level doc comment for the per-platform behaviour and
/// why Android uses a hardcoded path.
pub fn data_dir() -> Option<PathBuf> {
#[cfg(target_os = "android")]
{
Some(PathBuf::from(ANDROID_APP_FILES_DIR))
}
#[cfg(not(target_os = "android"))]
{
dirs::data_dir()
}
}
#[cfg(test)]
mod tests {
use super::*;
/// On every supported desktop target the OS reports a usable
/// data directory. This test only runs on desktop because the
/// Android branch returns a fixed string regardless of host
/// state, and asserting on a fixed string is a tautology.
#[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))]
#[test]
fn data_dir_returns_some_on_desktop_targets() {
let dir = data_dir().expect("desktop targets must report a data dir");
assert!(
dir.is_absolute(),
"data_dir() must return an absolute path on desktop, got {dir:?}",
);
}
/// On Android the hardcoded path matches the package id pinned
/// in `solitaire_app/Cargo.toml`'s `[package.metadata.android]`.
/// If a future change rotates that id, this test fails loudly
/// so the path constant moves with it.
#[cfg(target_os = "android")]
#[test]
fn data_dir_returns_sandbox_path_on_android() {
let dir = data_dir().expect("android must report a data dir");
assert_eq!(dir, PathBuf::from("/data/data/com.solitairequest.app/files"));
}
}
+1 -1
View File
@@ -46,7 +46,7 @@ pub fn xp_for_win(time_seconds: u64, used_undo: bool) -> u64 {
/// Platform-specific default path for `progress.json`. /// Platform-specific default path for `progress.json`.
pub fn progress_file_path() -> Option<PathBuf> { pub fn progress_file_path() -> Option<PathBuf> {
dirs::data_dir().map(|d| d.join(APP_DIR_NAME).join(FILE_NAME)) crate::data_dir().map(|d| d.join(APP_DIR_NAME).join(FILE_NAME))
} }
/// Load progress from an explicit path. Returns `default()` if missing/corrupt. /// Load progress from an explicit path. Returns `default()` if missing/corrupt.
+4 -4
View File
@@ -230,21 +230,21 @@ impl ReplayHistory {
} }
/// Returns the platform-specific path to `latest_replay.json`, or `None` /// Returns the platform-specific path to `latest_replay.json`, or `None`
/// if `dirs::data_dir()` is unavailable (e.g. minimal Linux containers). /// if `crate::data_dir()` is unavailable (e.g. minimal Linux containers).
#[deprecated( #[deprecated(
note = "single-slot replay storage replaced by the rolling history at \ note = "single-slot replay storage replaced by the rolling history at \
replay_history_path(); kept for the one-shot legacy migration \ replay_history_path(); kept for the one-shot legacy migration \
in migrate_legacy_latest_replay" in migrate_legacy_latest_replay"
)] )]
pub fn latest_replay_path() -> Option<PathBuf> { pub fn latest_replay_path() -> Option<PathBuf> {
dirs::data_dir().map(|d| d.join(APP_DIR_NAME).join(LATEST_REPLAY_FILE_NAME)) crate::data_dir().map(|d| d.join(APP_DIR_NAME).join(LATEST_REPLAY_FILE_NAME))
} }
/// Returns the platform-specific path to `replays.json`, the rolling /// Returns the platform-specific path to `replays.json`, the rolling
/// history file, or `None` if `dirs::data_dir()` is unavailable (e.g. /// history file, or `None` if `crate::data_dir()` is unavailable (e.g.
/// minimal Linux containers). /// minimal Linux containers).
pub fn replay_history_path() -> Option<PathBuf> { pub fn replay_history_path() -> Option<PathBuf> {
dirs::data_dir().map(|d| d.join(APP_DIR_NAME).join(REPLAY_HISTORY_FILE_NAME)) crate::data_dir().map(|d| d.join(APP_DIR_NAME).join(REPLAY_HISTORY_FILE_NAME))
} }
/// Save a [`Replay`] atomically to `path` using the standard `.tmp` → /// Save a [`Replay`] atomically to `path` using the standard `.tmp` →
+2 -2
View File
@@ -400,9 +400,9 @@ impl Settings {
} }
/// Returns the platform-specific path to `settings.json`, or `None` if /// Returns the platform-specific path to `settings.json`, or `None` if
/// `dirs::data_dir()` is unavailable. /// the platform's data directory is unavailable.
pub fn settings_file_path() -> Option<PathBuf> { pub fn settings_file_path() -> Option<PathBuf> {
dirs::data_dir().map(|d| d.join(APP_DIR_NAME).join(SETTINGS_FILE_NAME)) crate::data_dir().map(|d| d.join(APP_DIR_NAME).join(SETTINGS_FILE_NAME))
} }
/// Load settings from an explicit path. Returns `Settings::default()` if the /// Load settings from an explicit path. Returns `Settings::default()` if the
+7 -7
View File
@@ -19,9 +19,9 @@ const GAME_STATE_FILE_NAME: &str = "game_state.json";
const TIME_ATTACK_SESSION_FILE_NAME: &str = "time_attack_session.json"; const TIME_ATTACK_SESSION_FILE_NAME: &str = "time_attack_session.json";
/// Returns the platform-specific path to `stats.json`, or `None` if /// Returns the platform-specific path to `stats.json`, or `None` if
/// `dirs::data_dir()` is unavailable (e.g. minimal Linux containers). /// `crate::data_dir()` is unavailable (e.g. minimal Linux containers).
pub fn stats_file_path() -> Option<PathBuf> { pub fn stats_file_path() -> Option<PathBuf> {
dirs::data_dir().map(|d| d.join(APP_DIR_NAME).join(STATS_FILE_NAME)) crate::data_dir().map(|d| d.join(APP_DIR_NAME).join(STATS_FILE_NAME))
} }
/// Load stats from an explicit path. Returns `StatsSnapshot::default()` if /// Load stats from an explicit path. Returns `StatsSnapshot::default()` if
@@ -69,9 +69,9 @@ pub fn save_stats(stats: &StatsSnapshot) -> io::Result<()> {
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
/// Returns the platform-specific path to `game_state.json`, or `None` if /// Returns the platform-specific path to `game_state.json`, or `None` if
/// `dirs::data_dir()` is unavailable. /// `crate::data_dir()` is unavailable.
pub fn game_state_file_path() -> Option<PathBuf> { pub fn game_state_file_path() -> Option<PathBuf> {
dirs::data_dir().map(|d| d.join(APP_DIR_NAME).join(GAME_STATE_FILE_NAME)) crate::data_dir().map(|d| d.join(APP_DIR_NAME).join(GAME_STATE_FILE_NAME))
} }
/// Load an in-progress `GameState` from `path`. Returns `None` if the file is /// Load an in-progress `GameState` from `path`. Returns `None` if the file is
@@ -129,7 +129,7 @@ pub fn delete_game_state_at(path: &Path) -> io::Result<()> {
/// in an atomic save. Safe to call on startup; missing or unreadable entries /// in an atomic save. Safe to call on startup; missing or unreadable entries
/// are silently skipped. /// are silently skipped.
pub fn cleanup_orphaned_tmp_files() -> io::Result<()> { pub fn cleanup_orphaned_tmp_files() -> io::Result<()> {
let dir = match dirs::data_dir() { let dir = match crate::data_dir() {
Some(d) => d.join(APP_DIR_NAME), Some(d) => d.join(APP_DIR_NAME),
None => return Ok(()), None => return Ok(()),
}; };
@@ -179,9 +179,9 @@ pub struct TimeAttackSession {
} }
/// Returns the platform-specific path to `time_attack_session.json`, or /// Returns the platform-specific path to `time_attack_session.json`, or
/// `None` if `dirs::data_dir()` is unavailable. /// `None` if `crate::data_dir()` is unavailable.
pub fn time_attack_session_path() -> Option<PathBuf> { pub fn time_attack_session_path() -> Option<PathBuf> {
dirs::data_dir().map(|d| d.join(APP_DIR_NAME).join(TIME_ATTACK_SESSION_FILE_NAME)) crate::data_dir().map(|d| d.join(APP_DIR_NAME).join(TIME_ATTACK_SESSION_FILE_NAME))
} }
/// Save a Time Attack session atomically. Mirrors `save_game_state_to`'s /// Save a Time Attack session atomically. Mirrors `save_game_state_to`'s
+32 -58
View File
@@ -1,13 +1,12 @@
//! Per-platform resolution of the user-themes directory. //! Per-platform resolution of the user-themes directory.
//! //!
//! The path is determined exactly once and exposed via //! The path is determined exactly once and exposed via
//! [`user_theme_dir`]. On desktop platforms it is derived from //! [`user_theme_dir`]. The base directory comes from
//! `dirs::data_dir()` (matching the rest of the project's //! [`solitaire_data::data_dir`] (desktop: `dirs::data_dir()`;
//! per-app-storage convention); on mobile it must be supplied by the //! Android: the hardcoded `/data/data/<package>/files` sandbox
//! platform entry point via [`set_user_theme_dir`] before any code //! path). Mobile entry points may still override the path via
//! that needs the path executes — there is deliberately no silent //! [`set_user_theme_dir`] when they need to point at a non-default
//! fallback because mobile sandboxing makes any guess we'd hard-code //! location (e.g. tests, custom AssetManager wiring).
//! wrong.
//! //!
//! # Why panic instead of returning Result? //! # Why panic instead of returning Result?
//! //!
@@ -35,17 +34,18 @@ const APP_DIR_NAME: &str = "solitaire_quest";
/// Sub-folder under [`APP_DIR_NAME`] dedicated to user themes. /// Sub-folder under [`APP_DIR_NAME`] dedicated to user themes.
const THEME_DIR_NAME: &str = "themes"; const THEME_DIR_NAME: &str = "themes";
/// Sets the user-themes directory at runtime — mobile-only API. /// Sets the user-themes directory at runtime — escape hatch for
/// embedders or tests that need to override the platform default.
/// ///
/// Returns `Err` containing the rejected path if the override has /// Returns `Err` containing the rejected path if the override has
/// already been set. The first caller wins and subsequent calls are /// already been set. The first caller wins and subsequent calls are
/// silently a no-op-with-feedback so a mis-configured embedder can't /// silently a no-op-with-feedback so a mis-configured embedder can't
/// flip the path mid-session. /// flip the path mid-session.
/// ///
/// On desktop platforms this is functional but unnecessary — /// Mostly unnecessary now that [`solitaire_data::data_dir`] handles
/// [`user_theme_dir`] derives the path from `dirs::data_dir` directly /// every supported target — the override is kept for tests and for
/// and ignores the override. Setting it on desktop is harmless but /// embedders that want a non-default location (e.g. a sandboxed
/// nearly always a sign of confusion. /// AssetManager root on a future iOS port).
pub fn set_user_theme_dir(path: PathBuf) -> Result<(), PathBuf> { pub fn set_user_theme_dir(path: PathBuf) -> Result<(), PathBuf> {
USER_THEME_DIR_OVERRIDE.set(path) USER_THEME_DIR_OVERRIDE.set(path)
} }
@@ -55,16 +55,10 @@ pub fn set_user_theme_dir(path: PathBuf) -> Result<(), PathBuf> {
/// ///
/// # Panics /// # Panics
/// ///
/// Panics on: /// Panics if [`solitaire_data::data_dir`] returns `None`, which on
/// /// desktop indicates a broken `$HOME` / `$XDG_*` configuration.
/// - Desktop, if `dirs::data_dir()` returns `None` (rare; usually /// Android always returns `Some`. The panic message names the
/// indicates a broken `$HOME` or `$XDG_*` configuration). /// supported workaround ([`set_user_theme_dir`]).
/// - Mobile, if no entry point has called [`set_user_theme_dir`] yet.
/// - Any other target, where the embedder is required to supply the
/// path manually.
///
/// The panic message names the missing piece so the failure is
/// immediately actionable.
pub fn user_theme_dir() -> PathBuf { pub fn user_theme_dir() -> PathBuf {
if let Some(p) = USER_THEME_DIR_OVERRIDE.get() { if let Some(p) = USER_THEME_DIR_OVERRIDE.get() {
return p.clone(); return p.clone();
@@ -79,12 +73,15 @@ fn user_theme_dir_for(data_dir: PathBuf) -> PathBuf {
data_dir.join(APP_DIR_NAME).join(THEME_DIR_NAME) data_dir.join(APP_DIR_NAME).join(THEME_DIR_NAME)
} }
/// Per-target-os resolution of the platform's data dir. Split out so /// Per-target-os resolution of the platform's data dir. Delegates
/// mobile branches can grow without disturbing desktop behaviour. /// to [`solitaire_data::data_dir`] which encapsulates the
/// per-target shape (desktop: `dirs::data_dir()`; android: the
/// hardcoded `/data/data/<package>/files` sandbox path). Panics
/// only when the underlying resolver returns `None`, which on
/// desktop indicates a broken `$HOME` / `$XDG_*` configuration —
/// the panic message names the supported workaround.
fn detected_platform_data_dir() -> PathBuf { fn detected_platform_data_dir() -> PathBuf {
#[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] solitaire_data::data_dir().unwrap_or_else(|| {
{
dirs::data_dir().unwrap_or_else(|| {
panic!( panic!(
"user_theme_dir(): platform data directory is unavailable. \ "user_theme_dir(): platform data directory is unavailable. \
On Linux check $XDG_DATA_HOME or $HOME; on macOS / Windows \ On Linux check $XDG_DATA_HOME or $HOME; on macOS / Windows \
@@ -93,31 +90,6 @@ fn detected_platform_data_dir() -> PathBuf {
set_user_theme_dir() before App::run()." set_user_theme_dir() before App::run()."
) )
}) })
}
#[cfg(any(target_os = "android", target_os = "ios"))]
{
panic!(
"user_theme_dir(): mobile entry point must call \
solitaire_engine::assets::user_dir::set_user_theme_dir() \
before App::run() — there is no platform default."
)
}
#[cfg(not(any(
target_os = "linux",
target_os = "macos",
target_os = "windows",
target_os = "android",
target_os = "ios"
)))]
{
panic!(
"user_theme_dir(): unsupported platform; call \
solitaire_engine::assets::user_dir::set_user_theme_dir() \
from your entry point before App::run()."
)
}
} }
#[cfg(test)] #[cfg(test)]
@@ -140,14 +112,16 @@ mod tests {
assert_eq!(dir, PathBuf::from("solitaire_quest/themes")); assert_eq!(dir, PathBuf::from("solitaire_quest/themes"));
} }
#[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))]
#[test] #[test]
fn detected_data_dir_yields_a_path_with_a_parent() { fn detected_data_dir_yields_a_path_with_a_parent() {
// On every supported desktop platform the OS reports a // On every supported target the platform resolver
// user-writable data directory; the test machine already has // (`solitaire_data::data_dir`) returns a usable directory:
// one for `dirs::data_dir()` to discover. We don't pin the // desktop targets via `dirs::data_dir()` (the test machine
// exact value because it depends on the user's $HOME, but it // already has a `$HOME` for it to discover), Android via
// must at least be a non-empty path with a parent component. // the hardcoded `/data/data/<package>/files` sandbox path.
// We don't pin the exact value because it depends on the
// user's `$HOME` on desktop, but it must at least be a
// non-empty path with a parent component.
let dir = detected_platform_data_dir(); let dir = detected_platform_data_dir();
assert!(dir.parent().is_some(), "data dir {dir:?} should be absolute"); assert!(dir.parent().is_some(), "data dir {dir:?} should be absolute");
} }