diff --git a/FOLLOW_UP_2.md b/FOLLOW_UP_2.md index e853d51..4e8ce06 100644 --- a/FOLLOW_UP_2.md +++ b/FOLLOW_UP_2.md @@ -37,15 +37,14 @@ The OperationGuard ordering fix in `handlers.rs` is the structurally most import missing-sentinel, and local-path cases. - TempDir test helper: peer tests now share `test_support::TempDir` instead of carrying per-module copies. +- Typed availability: `lanspread-db::Game::availability` now uses the shared + serde enum, with `lanspread-proto` re-exporting it for wire summaries. The + JSON spelling stays `"Ready" | "Downloading" | "LocalOnly"`. ## Still open ### Code hygiene -#### 9. `availability` field is `String`, not a typed enum - -`lanspread-db::Game::availability: String` with `AVAILABILITY_*` consts works but allows invalid values. `remote_peer::summary_from_game` already has a defensive fallback for unknown strings. A typed enum + serde would be tighter, but is a wire-format change — coordinate if/when other peers are upgraded. - #### 10. `save_library_index` non-atomic `local_games.rs:141-148` writes without temp+rename. Corrupt index → next scan rebuilds, so blast radius is small. Match the intent-log atomic pattern if touching that module. diff --git a/crates/lanspread-compat/src/eti.rs b/crates/lanspread-compat/src/eti.rs index 9fbd191..5ed6b6f 100644 --- a/crates/lanspread-compat/src/eti.rs +++ b/crates/lanspread-compat/src/eti.rs @@ -1,6 +1,6 @@ use std::path::Path; -use lanspread_db::db::{AVAILABILITY_LOCAL_ONLY, Game}; +use lanspread_db::db::{Availability, Game}; use serde::{Deserialize, Serialize}; use sqlx::sqlite::{SqliteConnectOptions, SqlitePoolOptions}; @@ -63,7 +63,7 @@ impl From for Game { size: (eti_game.game_size * 1024.0 * 1024.0 * 1024.0) as u64, downloaded: false, installed: false, - availability: AVAILABILITY_LOCAL_ONLY.to_string(), + availability: Availability::LocalOnly, eti_game_version: None, local_version: None, peer_count: 0, // ETI games start with 0 peers until peer system discovers them diff --git a/crates/lanspread-db/src/db.rs b/crates/lanspread-db/src/db.rs index 776399a..97468f0 100644 --- a/crates/lanspread-db/src/db.rs +++ b/crates/lanspread-db/src/db.rs @@ -5,10 +5,6 @@ use std::{collections::HashMap, fmt, path::Path}; use serde::{Deserialize, Serialize}; -pub const AVAILABILITY_READY: &str = "Ready"; -pub const AVAILABILITY_DOWNLOADING: &str = "Downloading"; -pub const AVAILABILITY_LOCAL_ONLY: &str = "LocalOnly"; - /// Read version from version.ini file /// # Errors /// Returns error if file cannot be read or parsed @@ -34,8 +30,30 @@ pub fn read_version_from_ini(game_dir: &Path) -> eyre::Result> { } } -fn default_availability() -> String { - AVAILABILITY_LOCAL_ONLY.to_string() +#[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq, Hash)] +pub enum Availability { + Ready, + /// Wire-compatible transitional state. Local library summaries currently + /// suppress active operations instead of advertising this value. + Downloading, + #[default] + LocalOnly, +} + +impl Availability { + #[must_use] + pub fn from_downloaded(downloaded: bool) -> Self { + if downloaded { + Self::Ready + } else { + Self::LocalOnly + } + } + + #[must_use] + pub fn is_downloaded(&self) -> bool { + matches!(self, Self::Ready) + } } /// A game @@ -66,8 +84,8 @@ pub struct Game { #[serde(default)] pub installed: bool, /// Backend-reported availability state for this game's local or peer summary. - #[serde(default = "default_availability")] - pub availability: String, + #[serde(default)] + pub availability: Availability, /// ETI game version from version.ini (YYYYMMDD format) (server) pub eti_game_version: Option, /// Local game version from version.ini (YYYYMMDD format) @@ -76,6 +94,26 @@ pub struct Game { pub peer_count: u32, } +impl Game { + /// Sets sentinel-derived download state and collapses any non-ready + /// availability, including `Downloading`, back to `LocalOnly`. + pub fn set_downloaded(&mut self, downloaded: bool) { + self.downloaded = downloaded; + self.availability = Availability::from_downloaded(downloaded); + } + + #[must_use] + pub fn normalized_availability(&self) -> Availability { + if self.downloaded { + Availability::Ready + } else if self.availability.is_downloaded() { + Availability::LocalOnly + } else { + self.availability.clone() + } + } +} + impl fmt::Debug for Game { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( @@ -166,9 +204,8 @@ impl GameDB { pub fn set_all_uninstalled(&mut self) { for game in self.games.values_mut() { - game.downloaded = false; + game.set_downloaded(false); game.installed = false; - game.availability = AVAILABILITY_LOCAL_ONLY.to_string(); game.local_version = None; } } @@ -219,10 +256,30 @@ impl fmt::Debug for GameFileDescription { mod tests { use serde_json::json; - use super::{AVAILABILITY_LOCAL_ONLY, Game, GameFileDescription}; + use super::{Availability, Game, GameFileDescription}; + + fn game_fixture() -> Game { + Game { + id: "aoe2".to_string(), + name: "Age of Empires II".to_string(), + description: "desc".to_string(), + release_year: "1999".to_string(), + publisher: "Microsoft".to_string(), + max_players: 8, + version: "1.0".to_string(), + genre: "RTS".to_string(), + size: 123_456, + downloaded: false, + installed: false, + availability: Availability::LocalOnly, + eti_game_version: None, + local_version: None, + peer_count: 0, + } + } #[test] - fn installed_defaults_to_false_when_missing() { + fn missing_client_state_defaults_to_false_and_local_only() { let raw = json!({ "id": "aoe2", "name": "Age of Empires II", @@ -246,7 +303,39 @@ mod tests { !game.installed, "missing installed flag should default to false" ); - assert_eq!(game.availability, AVAILABILITY_LOCAL_ONLY); + assert_eq!(game.availability, Availability::LocalOnly); + } + + #[test] + fn download_state_helpers_keep_ready_in_lockstep() { + assert_eq!(Availability::from_downloaded(true), Availability::Ready); + assert_eq!( + Availability::from_downloaded(false), + Availability::LocalOnly + ); + + let mut game = game_fixture(); + game.set_downloaded(true); + assert!(game.downloaded); + assert_eq!(game.availability, Availability::Ready); + + game.set_downloaded(false); + assert!(!game.downloaded); + assert_eq!(game.availability, Availability::LocalOnly); + + game.availability = Availability::Ready; + assert_eq!(game.normalized_availability(), Availability::LocalOnly); + + game.availability = Availability::Downloading; + game.set_downloaded(false); + assert!(!game.downloaded); + assert_eq!(game.availability, Availability::LocalOnly); + + game.availability = Availability::Downloading; + assert_eq!(game.normalized_availability(), Availability::Downloading); + + game.downloaded = true; + assert_eq!(game.normalized_availability(), Availability::Ready); } #[test] diff --git a/crates/lanspread-peer/src/local_games.rs b/crates/lanspread-peer/src/local_games.rs index 39d5993..7217cfd 100644 --- a/crates/lanspread-peer/src/local_games.rs +++ b/crates/lanspread-peer/src/local_games.rs @@ -8,14 +8,7 @@ use std::{ time::{SystemTime, UNIX_EPOCH}, }; -use lanspread_db::db::{ - AVAILABILITY_DOWNLOADING, - AVAILABILITY_LOCAL_ONLY, - AVAILABILITY_READY, - Game, - GameDB, - GameFileDescription, -}; +use lanspread_db::db::{Game, GameDB, GameFileDescription}; use lanspread_proto::{Availability, GameSummary}; use serde::{Deserialize, Serialize}; use tokio::io::AsyncWriteExt; @@ -436,21 +429,13 @@ pub(crate) fn game_from_summary(summary: &GameSummary) -> Game { size: summary.size, downloaded: summary.downloaded, installed: summary.installed, - availability: availability_label(&summary.availability).to_string(), + availability: summary.availability.clone(), eti_game_version: summary.eti_version.clone(), local_version: summary.eti_version.clone(), peer_count: 0, } } -pub(crate) fn availability_label(availability: &Availability) -> &'static str { - match availability { - Availability::Ready => AVAILABILITY_READY, - Availability::Downloading => AVAILABILITY_DOWNLOADING, - Availability::LocalOnly => AVAILABILITY_LOCAL_ONLY, - } -} - struct IndexUpdate { summary: Option, changed: bool, diff --git a/crates/lanspread-peer/src/peer_db.rs b/crates/lanspread-peer/src/peer_db.rs index ec72d04..0c71b42 100644 --- a/crates/lanspread-peer/src/peer_db.rs +++ b/crates/lanspread-peer/src/peer_db.rs @@ -7,10 +7,12 @@ use std::{ time::{Duration, Instant}, }; -use lanspread_db::db::{AVAILABILITY_READY, Game, GameFileDescription}; -use lanspread_proto::{Availability, GameSummary, LibraryDelta, LibrarySnapshot}; +#[cfg(test)] +use lanspread_db::db::Availability; +use lanspread_db::db::{Game, GameFileDescription}; +use lanspread_proto::{GameSummary, LibraryDelta, LibrarySnapshot}; -use crate::{library::compute_library_digest, local_games::availability_label}; +use crate::library::compute_library_digest; pub type PeerId = String; /// Information about a discovered peer. @@ -292,11 +294,9 @@ impl PeerGameDB { existing.size = game.size; } if game_is_ready(game) { - existing.downloaded = true; - existing.availability = AVAILABILITY_READY.to_string(); + existing.set_downloaded(true); } else if !existing.downloaded { - existing.availability = - availability_label(&game.availability).to_string(); + existing.availability = game.availability.clone(); } if game.installed { existing.installed = true; @@ -305,7 +305,6 @@ impl PeerGameDB { .or_insert_with(|| { let mut game_clone = summary_to_game(game); game_clone.peer_count = *peer_counts.get(&game.id).unwrap_or(&0); - game_clone.downloaded = game_is_ready(game); game_clone }); } @@ -728,7 +727,7 @@ fn create_peer_whitelist(peer_scores: HashMap) -> Vec bool { - summary.availability == Availability::Ready + summary.availability.is_downloaded() } fn summary_to_game(summary: &GameSummary) -> Game { @@ -746,9 +745,9 @@ fn summary_to_game(summary: &GameSummary) -> Game { version: "1.0".to_string(), genre: String::new(), size: summary.size, - downloaded: summary.downloaded, + downloaded: summary.availability.is_downloaded(), installed: summary.installed, - availability: availability_label(&summary.availability).to_string(), + availability: summary.availability.clone(), eti_game_version, local_version: None, peer_count: 0, @@ -759,8 +758,6 @@ fn summary_to_game(summary: &GameSummary) -> Game { mod tests { use std::net::SocketAddr; - use lanspread_db::db::AVAILABILITY_LOCAL_ONLY; - use super::*; fn addr(port: u16) -> SocketAddr { @@ -824,7 +821,7 @@ mod tests { assert_eq!(games.len(), 1); assert_eq!(games[0].peer_count, 0); assert!(!games[0].downloaded); - assert_eq!(games[0].availability, AVAILABILITY_LOCAL_ONLY); + assert_eq!(games[0].availability, Availability::LocalOnly); assert_eq!(games[0].eti_game_version, None); assert!(db.peers_with_game("game").is_empty()); diff --git a/crates/lanspread-peer/src/remote_peer.rs b/crates/lanspread-peer/src/remote_peer.rs index 868f2f0..548f207 100644 --- a/crates/lanspread-peer/src/remote_peer.rs +++ b/crates/lanspread-peer/src/remote_peer.rs @@ -2,13 +2,8 @@ use std::{collections::HashMap, net::SocketAddr, sync::Arc}; -use lanspread_db::db::{ - AVAILABILITY_DOWNLOADING, - AVAILABILITY_LOCAL_ONLY, - AVAILABILITY_READY, - Game, -}; -use lanspread_proto::{Availability, GameSummary}; +use lanspread_db::db::Game; +use lanspread_proto::GameSummary; use tokio::sync::RwLock; use crate::{ @@ -31,14 +26,6 @@ pub async fn ensure_peer_id_for_addr( } pub fn summary_from_game(game: &Game) -> GameSummary { - let availability = match game.availability.as_str() { - AVAILABILITY_READY => Availability::Ready, - AVAILABILITY_DOWNLOADING => Availability::Downloading, - AVAILABILITY_LOCAL_ONLY => Availability::LocalOnly, - _ if game.downloaded => Availability::Ready, - _ => Availability::LocalOnly, - }; - GameSummary { id: game.id.clone(), name: game.name.clone(), @@ -47,7 +34,7 @@ pub fn summary_from_game(game: &Game) -> GameSummary { installed: game.installed, eti_version: game.eti_game_version.clone(), manifest_hash: 0, - availability, + availability: game.normalized_availability(), } } diff --git a/crates/lanspread-proto/src/lib.rs b/crates/lanspread-proto/src/lib.rs index 6d5b46f..966e25e 100644 --- a/crates/lanspread-proto/src/lib.rs +++ b/crates/lanspread-proto/src/lib.rs @@ -4,14 +4,7 @@ use serde::{Deserialize, Serialize}; pub const PROTOCOL_VERSION: u32 = 2; -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Hash)] -pub enum Availability { - Ready, - /// Wire-compatible transitional state. Local library summaries currently - /// suppress active operations instead of advertising this value. - Downloading, - LocalOnly, -} +pub use lanspread_db::db::Availability; #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Hash)] pub struct GameSummary { diff --git a/crates/lanspread-tauri-deno-ts/src-tauri/src/lib.rs b/crates/lanspread-tauri-deno-ts/src-tauri/src/lib.rs index e28cc42..864f557 100644 --- a/crates/lanspread-tauri-deno-ts/src-tauri/src/lib.rs +++ b/crates/lanspread-tauri-deno-ts/src-tauri/src/lib.rs @@ -9,13 +9,7 @@ use std::{ use eyre::bail; use lanspread_compat::eti::get_games; -use lanspread_db::db::{ - AVAILABILITY_LOCAL_ONLY, - AVAILABILITY_READY, - Game, - GameDB, - GameFileDescription, -}; +use lanspread_db::db::{Game, GameDB, GameFileDescription}; use lanspread_peer::{ ActiveOperation, ActiveOperationKind, @@ -372,15 +366,10 @@ fn update_game_installation_state(game: &mut Game, games_root: &Path) { } let downloaded = game_path.join("version.ini").is_file(); - game.downloaded = downloaded; + game.set_downloaded(downloaded); let installed = local_install_is_present(&game_path); game.installed = installed; - game.availability = if downloaded { - AVAILABILITY_READY.to_string() - } else { - AVAILABILITY_LOCAL_ONLY.to_string() - }; // Size stays anchored to bundled game.db; skip expensive recalculation. @@ -687,11 +676,8 @@ async fn update_local_games_in_db(local_games: Vec, app: AppHandle) { // Update installation status for games that exist locally for local_game in &local_games { if let Some(existing_game) = game_db.get_mut_game_by_id(&local_game.id) { - existing_game.downloaded = local_game.downloaded; + existing_game.set_downloaded(local_game.downloaded); existing_game.installed = local_game.installed; - existing_game - .availability - .clone_from(&local_game.availability); existing_game .local_version .clone_from(&local_game.local_version); @@ -707,9 +693,8 @@ async fn update_local_games_in_db(local_games: Vec, app: AppHandle) { "Game {} no longer exists locally, marking as uninstalled", game.id ); - game.downloaded = false; + game.set_downloaded(false); game.installed = false; - game.availability = AVAILABILITY_LOCAL_ONLY.to_string(); game.local_version = None; } }