From 27079e127d88e0310a662214cfc4e4e04491fd74 Mon Sep 17 00:00:00 2001 From: Xevion Date: Mon, 11 Aug 2025 20:23:39 -0500 Subject: [PATCH] feat!: implement proper error handling, drop most expect() & unwrap() usages --- src/app.rs | 46 +++++++----- src/entity/ghost.rs | 57 +++++++++++---- src/entity/pacman.rs | 29 +++++--- src/entity/trait.rs | 30 ++++---- src/entity/traversal.rs | 36 ++++++++-- src/error.rs | 156 ++++++++++++++++++++++++++++++++++++++++ src/game.rs | 131 +++++++++++++++++++++------------ src/lib.rs | 1 + src/main.rs | 1 + src/map/builder.rs | 119 +++++++++++++++++++----------- src/map/parser.rs | 21 ++++++ src/map/render.rs | 57 ++++++++++----- tests/blinking.rs | 12 ++-- tests/game.rs | 4 +- tests/ghost.rs | 2 +- tests/graph.rs | 8 ++- tests/map_builder.rs | 6 +- tests/pacman.rs | 6 +- tests/parser.rs | 6 +- tests/pathfinding.rs | 21 +++--- 20 files changed, 555 insertions(+), 194 deletions(-) create mode 100644 src/error.rs diff --git a/src/app.rs b/src/app.rs index b796b18..7e2dbc6 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1,6 +1,5 @@ use std::time::{Duration, Instant}; -use anyhow::{anyhow, Result}; use glam::Vec2; use sdl2::event::{Event, WindowEvent}; use sdl2::keyboard::Keycode; @@ -9,6 +8,8 @@ use sdl2::video::{Window, WindowContext}; use sdl2::EventPump; use tracing::{error, event}; +use crate::error::{GameError, GameResult}; + use crate::constants::{CANVAS_SIZE, LOOP_TIME, SCALE}; use crate::game::Game; use crate::platform::get_platform; @@ -24,14 +25,14 @@ pub struct App<'a> { } impl App<'_> { - pub fn new() -> Result { + pub fn new() -> GameResult { // Initialize platform-specific console - get_platform().init_console().map_err(|e| anyhow!(e))?; + get_platform().init_console()?; - let sdl_context = sdl2::init().map_err(|e| anyhow!(e))?; - let video_subsystem = sdl_context.video().map_err(|e| anyhow!(e))?; - let audio_subsystem = sdl_context.audio().map_err(|e| anyhow!(e))?; - let ttf_context = sdl2::ttf::init().map_err(|e| anyhow!(e.to_string()))?; + let sdl_context = sdl2::init().map_err(|e| GameError::Sdl(e.to_string()))?; + let video_subsystem = sdl_context.video().map_err(|e| GameError::Sdl(e.to_string()))?; + let audio_subsystem = sdl_context.audio().map_err(|e| GameError::Sdl(e.to_string()))?; + let ttf_context = sdl2::ttf::init().map_err(|e| GameError::Sdl(e.to_string()))?; let window = video_subsystem .window( @@ -41,24 +42,31 @@ impl App<'_> { ) .resizable() .position_centered() - .build()?; + .build() + .map_err(|e| GameError::Sdl(e.to_string()))?; - let mut canvas = window.into_canvas().build()?; - canvas.set_logical_size(CANVAS_SIZE.x, CANVAS_SIZE.y)?; + let mut canvas = window.into_canvas().build().map_err(|e| GameError::Sdl(e.to_string()))?; + canvas + .set_logical_size(CANVAS_SIZE.x, CANVAS_SIZE.y) + .map_err(|e| GameError::Sdl(e.to_string()))?; let texture_creator_static: &'static TextureCreator = Box::leak(Box::new(canvas.texture_creator())); - let mut game = Game::new(texture_creator_static, &ttf_context, &audio_subsystem); + let mut game = Game::new(texture_creator_static, &ttf_context, &audio_subsystem)?; game.audio.set_mute(cfg!(debug_assertions)); - let mut backbuffer = texture_creator_static.create_texture_target(None, CANVAS_SIZE.x, CANVAS_SIZE.y)?; + let mut backbuffer = texture_creator_static + .create_texture_target(None, CANVAS_SIZE.x, CANVAS_SIZE.y) + .map_err(|e| GameError::Sdl(e.to_string()))?; backbuffer.set_scale_mode(ScaleMode::Nearest); - let event_pump = sdl_context.event_pump().map_err(|e| anyhow!(e))?; + let event_pump = sdl_context.event_pump().map_err(|e| GameError::Sdl(e.to_string()))?; // Initial draw - game.draw(&mut canvas, &mut backbuffer)?; - game.present_backbuffer(&mut canvas, &backbuffer, glam::Vec2::ZERO)?; + game.draw(&mut canvas, &mut backbuffer) + .map_err(|e| GameError::Sdl(e.to_string()))?; + game.present_backbuffer(&mut canvas, &backbuffer, glam::Vec2::ZERO) + .map_err(|e| GameError::Sdl(e.to_string()))?; Ok(Self { game, @@ -109,8 +117,8 @@ impl App<'_> { } => { self.game.debug_mode = !self.game.debug_mode; } - Event::KeyDown { keycode, .. } => { - self.game.keyboard_event(keycode.unwrap()); + Event::KeyDown { keycode: Some(key), .. } => { + self.game.keyboard_event(key); } Event::MouseMotion { x, y, .. } => { // Convert window coordinates to logical coordinates @@ -126,13 +134,13 @@ impl App<'_> { if !self.paused { self.game.tick(dt); if let Err(e) = self.game.draw(&mut self.canvas, &mut self.backbuffer) { - error!("Failed to draw game: {e}"); + error!("Failed to draw game: {}", e); } if let Err(e) = self .game .present_backbuffer(&mut self.canvas, &self.backbuffer, self.cursor_pos) { - error!("Failed to present backbuffer: {e}"); + error!("Failed to present backbuffer: {}", e); } } diff --git a/src/entity/ghost.rs b/src/entity/ghost.rs index ccf0111..d0c1ad6 100644 --- a/src/entity/ghost.rs +++ b/src/entity/ghost.rs @@ -16,6 +16,8 @@ use crate::texture::animated::AnimatedTexture; use crate::texture::directional::DirectionalAnimatedTexture; use crate::texture::sprite::SpriteAtlas; +use crate::error::{EntityError, GameError, GameResult, TextureError}; + /// Determines if a ghost can traverse a given edge. /// /// Ghosts can move through edges that allow all entities or ghost-only edges. @@ -101,7 +103,9 @@ impl Entity for Ghost { self.choose_random_direction(graph); } - self.traverser.advance(graph, dt * 60.0 * self.speed, &can_ghost_traverse); + if let Err(e) = self.traverser.advance(graph, dt * 60.0 * self.speed, &can_ghost_traverse) { + eprintln!("Ghost movement error: {}", e); + } self.texture.tick(dt); } } @@ -111,7 +115,7 @@ impl Ghost { /// /// Sets up animated textures for all four directions with moving and stopped states. /// The moving animation cycles through two sprite variants. - pub fn new(graph: &Graph, start_node: NodeId, ghost_type: GhostType, atlas: &SpriteAtlas) -> Self { + pub fn new(graph: &Graph, start_node: NodeId, ghost_type: GhostType, atlas: &SpriteAtlas) -> GameResult { let mut textures = [None, None, None, None]; let mut stopped_textures = [None, None, None, None]; @@ -123,27 +127,51 @@ impl Ghost { Direction::Right => "right", }; let moving_tiles = vec![ - SpriteAtlas::get_tile(atlas, &format!("ghost/{}/{}_{}.png", ghost_type.as_str(), moving_prefix, "a")).unwrap(), - SpriteAtlas::get_tile(atlas, &format!("ghost/{}/{}_{}.png", ghost_type.as_str(), moving_prefix, "b")).unwrap(), + SpriteAtlas::get_tile(atlas, &format!("ghost/{}/{}_{}.png", ghost_type.as_str(), moving_prefix, "a")) + .ok_or_else(|| { + GameError::Texture(TextureError::AtlasTileNotFound(format!( + "ghost/{}/{}_{}.png", + ghost_type.as_str(), + moving_prefix, + "a" + ))) + })?, + SpriteAtlas::get_tile(atlas, &format!("ghost/{}/{}_{}.png", ghost_type.as_str(), moving_prefix, "b")) + .ok_or_else(|| { + GameError::Texture(TextureError::AtlasTileNotFound(format!( + "ghost/{}/{}_{}.png", + ghost_type.as_str(), + moving_prefix, + "b" + ))) + })?, ]; let stopped_tiles = vec![ SpriteAtlas::get_tile(atlas, &format!("ghost/{}/{}_{}.png", ghost_type.as_str(), moving_prefix, "a")) - .unwrap(), + .ok_or_else(|| { + GameError::Texture(TextureError::AtlasTileNotFound(format!( + "ghost/{}/{}_{}.png", + ghost_type.as_str(), + moving_prefix, + "a" + ))) + })?, ]; - textures[direction.as_usize()] = Some(AnimatedTexture::new(moving_tiles, 0.2).expect("Invalid frame duration")); + textures[direction.as_usize()] = + Some(AnimatedTexture::new(moving_tiles, 0.2).map_err(|e| GameError::Texture(TextureError::Animated(e)))?); stopped_textures[direction.as_usize()] = - Some(AnimatedTexture::new(stopped_tiles, 0.1).expect("Invalid frame duration")); + Some(AnimatedTexture::new(stopped_tiles, 0.1).map_err(|e| GameError::Texture(TextureError::Animated(e)))?); } - Self { + Ok(Self { traverser: Traverser::new(graph, start_node, Direction::Left, &can_ghost_traverse), ghost_type, texture: DirectionalAnimatedTexture::new(textures, stopped_textures), speed: ghost_type.base_speed(), - } + }) } /// Chooses a random available direction at the current intersection. @@ -179,9 +207,9 @@ impl Ghost { /// Calculates the shortest path from the ghost's current position to a target node using Dijkstra's algorithm. /// - /// Returns a vector of NodeIds representing the path, or None if no path exists. + /// Returns a vector of NodeIds representing the path, or an error if pathfinding fails. /// The path includes the current node and the target node. - pub fn calculate_path_to_target(&self, graph: &Graph, target: NodeId) -> Option> { + pub fn calculate_path_to_target(&self, graph: &Graph, target: NodeId) -> GameResult> { let start_node = self.traverser.position.from_node_id(); // Use Dijkstra's algorithm to find the shortest path @@ -198,7 +226,12 @@ impl Ghost { |&node_id| node_id == target, ); - result.map(|(path, _cost)| path) + result.map(|(path, _cost)| path).ok_or_else(|| { + GameError::Entity(EntityError::PathfindingFailed(format!( + "No path found from node {} to target {}", + start_node, target + ))) + }) } /// Returns the ghost's color for debug rendering. diff --git a/src/entity/pacman.rs b/src/entity/pacman.rs index d773597..e4d05dc 100644 --- a/src/entity/pacman.rs +++ b/src/entity/pacman.rs @@ -13,6 +13,8 @@ use crate::texture::directional::DirectionalAnimatedTexture; use crate::texture::sprite::SpriteAtlas; use sdl2::keyboard::Keycode; +use crate::error::{GameError, GameResult, TextureError}; + /// Determines if Pac-Man can traverse a given edge. /// /// Pac-Man can only move through edges that allow all entities. @@ -57,7 +59,9 @@ impl Entity for Pacman { } fn tick(&mut self, dt: f32, graph: &Graph) { - self.traverser.advance(graph, dt * 60.0 * 1.125, &can_pacman_traverse); + if let Err(e) = self.traverser.advance(graph, dt * 60.0 * 1.125, &can_pacman_traverse) { + eprintln!("Pac-Man movement error: {}", e); + } self.texture.tick(dt); } } @@ -67,7 +71,7 @@ impl Pacman { /// /// Sets up animated textures for all four directions with moving and stopped states. /// The moving animation cycles through open mouth, closed mouth, and full sprites. - pub fn new(graph: &Graph, start_node: NodeId, atlas: &SpriteAtlas) -> Self { + pub fn new(graph: &Graph, start_node: NodeId, atlas: &SpriteAtlas) -> GameResult { let mut textures = [None, None, None, None]; let mut stopped_textures = [None, None, None, None]; @@ -79,22 +83,27 @@ impl Pacman { Direction::Right => "pacman/right", }; let moving_tiles = vec![ - SpriteAtlas::get_tile(atlas, &format!("{moving_prefix}_a.png")).unwrap(), - SpriteAtlas::get_tile(atlas, &format!("{moving_prefix}_b.png")).unwrap(), - SpriteAtlas::get_tile(atlas, "pacman/full.png").unwrap(), + SpriteAtlas::get_tile(atlas, &format!("{moving_prefix}_a.png")) + .ok_or_else(|| GameError::Texture(TextureError::AtlasTileNotFound(format!("{moving_prefix}_a.png"))))?, + SpriteAtlas::get_tile(atlas, &format!("{moving_prefix}_b.png")) + .ok_or_else(|| GameError::Texture(TextureError::AtlasTileNotFound(format!("{moving_prefix}_b.png"))))?, + SpriteAtlas::get_tile(atlas, "pacman/full.png") + .ok_or_else(|| GameError::Texture(TextureError::AtlasTileNotFound("pacman/full.png".to_string())))?, ]; - let stopped_tiles = vec![SpriteAtlas::get_tile(atlas, &format!("{moving_prefix}_b.png")).unwrap()]; + let stopped_tiles = vec![SpriteAtlas::get_tile(atlas, &format!("{moving_prefix}_b.png")) + .ok_or_else(|| GameError::Texture(TextureError::AtlasTileNotFound(format!("{moving_prefix}_b.png"))))?]; - textures[direction.as_usize()] = Some(AnimatedTexture::new(moving_tiles, 0.08).expect("Invalid frame duration")); + textures[direction.as_usize()] = + Some(AnimatedTexture::new(moving_tiles, 0.08).map_err(|e| GameError::Texture(TextureError::Animated(e)))?); stopped_textures[direction.as_usize()] = - Some(AnimatedTexture::new(stopped_tiles, 0.1).expect("Invalid frame duration")); + Some(AnimatedTexture::new(stopped_tiles, 0.1).map_err(|e| GameError::Texture(TextureError::Animated(e)))?); } - Self { + Ok(Self { traverser: Traverser::new(graph, start_node, Direction::Left, &can_pacman_traverse), texture: DirectionalAnimatedTexture::new(textures, stopped_textures), - } + }) } /// Handles keyboard input to change Pac-Man's direction. diff --git a/src/entity/trait.rs b/src/entity/trait.rs index db66e65..e45495f 100644 --- a/src/entity/trait.rs +++ b/src/entity/trait.rs @@ -10,6 +10,7 @@ use sdl2::render::{Canvas, RenderTarget}; use crate::entity::direction::Direction; use crate::entity::graph::{Edge, Graph, NodeId}; use crate::entity::traversal::{Position, Traverser}; +use crate::error::{EntityError, GameError, GameResult, TextureError}; use crate::texture::directional::DirectionalAnimatedTexture; use crate::texture::sprite::SpriteAtlas; @@ -48,21 +49,24 @@ pub trait Entity { /// /// Converts the graph position to screen coordinates, accounting for /// the board offset and centering the sprite. - fn get_pixel_pos(&self, graph: &Graph) -> Vec2 { + fn get_pixel_pos(&self, graph: &Graph) -> GameResult { let pos = match self.traverser().position { - Position::AtNode(node_id) => graph.get_node(node_id).unwrap().position, + Position::AtNode(node_id) => { + let node = graph.get_node(node_id).ok_or(EntityError::NodeNotFound(node_id))?; + node.position + } Position::BetweenNodes { from, to, traversed } => { - let from_pos = graph.get_node(from).unwrap().position; - let to_pos = graph.get_node(to).unwrap().position; - let edge = graph.find_edge(from, to).unwrap(); - from_pos + (to_pos - from_pos) * (traversed / edge.distance) + let from_node = graph.get_node(from).ok_or(EntityError::NodeNotFound(from))?; + let to_node = graph.get_node(to).ok_or(EntityError::NodeNotFound(to))?; + let edge = graph.find_edge(from, to).ok_or(EntityError::EdgeNotFound { from, to })?; + from_node.position + (to_node.position - from_node.position) * (traversed / edge.distance) } }; - Vec2::new( + Ok(Vec2::new( pos.x + crate::constants::BOARD_PIXEL_OFFSET.x as f32, pos.y + crate::constants::BOARD_PIXEL_OFFSET.y as f32, - ) + )) } /// Returns the current node ID that the entity is at or moving towards. @@ -88,8 +92,8 @@ pub trait Entity { /// /// Draws the appropriate directional sprite based on the entity's /// current movement state and direction. - fn render(&self, canvas: &mut Canvas, atlas: &mut SpriteAtlas, graph: &Graph) { - let pixel_pos = self.get_pixel_pos(graph); + fn render(&self, canvas: &mut Canvas, atlas: &mut SpriteAtlas, graph: &Graph) -> GameResult<()> { + let pixel_pos = self.get_pixel_pos(graph)?; let dest = crate::helpers::centered_with_size( glam::IVec2::new(pixel_pos.x as i32, pixel_pos.y as i32), glam::UVec2::new(16, 16), @@ -98,11 +102,13 @@ pub trait Entity { if self.traverser().position.is_stopped() { self.texture() .render_stopped(canvas, atlas, dest, self.traverser().direction) - .expect("Failed to render entity"); + .map_err(|e| GameError::Texture(TextureError::RenderFailed(e.to_string())))?; } else { self.texture() .render(canvas, atlas, dest, self.traverser().direction) - .expect("Failed to render entity"); + .map_err(|e| GameError::Texture(TextureError::RenderFailed(e.to_string())))?; } + + Ok(()) } } diff --git a/src/entity/traversal.rs b/src/entity/traversal.rs index fc0d107..372d37d 100644 --- a/src/entity/traversal.rs +++ b/src/entity/traversal.rs @@ -1,3 +1,7 @@ +use tracing::error; + +use crate::error::GameResult; + use super::direction::Direction; use super::graph::{Edge, Graph, NodeId}; @@ -82,7 +86,9 @@ impl Traverser { }; // This will kickstart the traverser into motion - traverser.advance(graph, 0.0, can_traverse); + if let Err(e) = traverser.advance(graph, 0.0, can_traverse) { + error!("Traverser initialization error: {}", e); + } traverser } @@ -108,7 +114,9 @@ impl Traverser { /// - If it reaches a node, it attempts to transition to a new edge based on /// the buffered direction or by continuing straight. /// - If no valid move is possible, it stops at the node. - pub fn advance(&mut self, graph: &Graph, distance: f32, can_traverse: &F) + /// + /// Returns an error if the movement is invalid (e.g., trying to move in an impossible direction). + pub fn advance(&mut self, graph: &Graph, distance: f32, can_traverse: &F) -> GameResult<()> where F: Fn(Edge) -> bool, { @@ -134,7 +142,18 @@ impl Traverser { traversed: distance.max(0.0), }; self.direction = next_direction; + } else { + return Err(crate::error::GameError::Entity(crate::error::EntityError::InvalidMovement( + format!( + "Cannot traverse edge from {} to {} in direction {:?}", + node_id, edge.target, next_direction + ), + ))); } + } else { + return Err(crate::error::GameError::Entity(crate::error::EntityError::InvalidMovement( + format!("No edge found in direction {:?} from node {}", next_direction, node_id), + ))); } self.next_direction = None; // Consume the buffered direction regardless of whether we started moving with it @@ -143,12 +162,15 @@ impl Traverser { Position::BetweenNodes { from, to, traversed } => { // There is no point in any of the next logic if we don't travel at all if distance <= 0.0 { - return; + return Ok(()); } - let edge = graph - .find_edge(from, to) - .expect("Inconsistent state: Traverser is on a non-existent edge."); + let edge = graph.find_edge(from, to).ok_or_else(|| { + crate::error::GameError::Entity(crate::error::EntityError::InvalidMovement(format!( + "Inconsistent state: Traverser is on a non-existent edge from {} to {}.", + from, to + ))) + })?; let new_traversed = traversed + distance; @@ -201,5 +223,7 @@ impl Traverser { } } } + + Ok(()) } } diff --git a/src/error.rs b/src/error.rs new file mode 100644 index 0000000..993fe78 --- /dev/null +++ b/src/error.rs @@ -0,0 +1,156 @@ +//! Centralized error types for the Pac-Man game. +//! +//! This module defines all error types used throughout the application, +//! providing a consistent error handling approach. + +use thiserror::Error; + +/// Main error type for the Pac-Man game. +/// +/// This is the primary error type that should be used in public APIs. +/// It can represent any error that can occur during game operation. +#[derive(Error, Debug)] +pub enum GameError { + #[error("Asset error: {0}")] + Asset(#[from] crate::asset::AssetError), + + #[error("Platform error: {0}")] + Platform(#[from] crate::platform::PlatformError), + + #[error("Map parsing error: {0}")] + MapParse(#[from] crate::map::parser::ParseError), + + #[error("Map error: {0}")] + Map(#[from] MapError), + + #[error("Texture error: {0}")] + Texture(#[from] TextureError), + + #[error("Entity error: {0}")] + Entity(#[from] EntityError), + + #[error("Game state error: {0}")] + GameState(#[from] GameStateError), + + #[error("SDL error: {0}")] + Sdl(String), + + #[error("IO error: {0}")] + Io(#[from] std::io::Error), + + #[error("Serialization error: {0}")] + Serialization(#[from] serde_json::Error), + + #[error("Invalid state: {0}")] + InvalidState(String), + + #[error("Resource not found: {0}")] + NotFound(String), + + #[error("Configuration error: {0}")] + Config(String), +} + +/// Errors related to texture operations. +#[derive(Error, Debug)] +pub enum TextureError { + #[error("Animated texture error: {0}")] + Animated(#[from] crate::texture::animated::AnimatedTextureError), + + #[error("Failed to load texture: {0}")] + LoadFailed(String), + + #[error("Texture not found in atlas: {0}")] + AtlasTileNotFound(String), + + #[error("Invalid texture format: {0}")] + InvalidFormat(String), + + #[error("Rendering failed: {0}")] + RenderFailed(String), +} + +/// Errors related to entity operations. +#[derive(Error, Debug)] +pub enum EntityError { + #[error("Node not found in graph: {0}")] + NodeNotFound(usize), + + #[error("Edge not found: from {from} to {to}")] + EdgeNotFound { from: usize, to: usize }, + + #[error("Invalid movement: {0}")] + InvalidMovement(String), + + #[error("Pathfinding failed: {0}")] + PathfindingFailed(String), +} + +/// Errors related to game state operations. +#[derive(Error, Debug)] +pub enum GameStateError {} + +/// Errors related to map operations. +#[derive(Error, Debug)] +pub enum MapError { + #[error("Node not found: {0}")] + NodeNotFound(usize), + + #[error("Invalid map configuration: {0}")] + InvalidConfig(String), +} + +/// Result type for game operations. +pub type GameResult = Result; + +/// Helper trait for converting other error types to GameError. +pub trait IntoGameError { + #[allow(dead_code)] + fn into_game_error(self) -> GameResult; +} + +impl IntoGameError for Result +where + E: std::error::Error + Send + Sync + 'static, +{ + fn into_game_error(self) -> GameResult { + self.map_err(|e| GameError::InvalidState(e.to_string())) + } +} + +/// Helper trait for converting Option to GameResult with a custom error. +pub trait OptionExt { + #[allow(dead_code)] + fn ok_or_game_error(self, f: F) -> GameResult + where + F: FnOnce() -> GameError; +} + +impl OptionExt for Option { + fn ok_or_game_error(self, f: F) -> GameResult + where + F: FnOnce() -> GameError, + { + self.ok_or_else(f) + } +} + +/// Helper trait for converting Result to GameResult with context. +pub trait ResultExt { + #[allow(dead_code)] + fn with_context(self, f: F) -> GameResult + where + F: FnOnce(&E) -> GameError; +} + +impl ResultExt for Result +where + E: std::error::Error + Send + Sync + 'static, +{ + fn with_context(self, f: F) -> GameResult + where + F: FnOnce(&E) -> GameError, + { + self.map_err(|e| f(&e)) + } +} diff --git a/src/game.rs b/src/game.rs index 8a173f6..cdd0619 100644 --- a/src/game.rs +++ b/src/game.rs @@ -1,6 +1,5 @@ //! This module contains the main game logic and state. -use anyhow::Result; use glam::{UVec2, Vec2}; use rand::{rngs::SmallRng, Rng, SeedableRng}; use sdl2::{ @@ -11,6 +10,8 @@ use sdl2::{ video::WindowContext, }; +use crate::error::{EntityError, GameError, GameResult, TextureError}; + use crate::{ asset::{get_asset_bytes, Asset}, audio::Audio, @@ -52,46 +53,57 @@ impl Game { texture_creator: &TextureCreator, _ttf_context: &sdl2::ttf::Sdl2TtfContext, _audio_subsystem: &sdl2::AudioSubsystem, - ) -> Game { - let map = Map::new(RAW_BOARD); + ) -> GameResult { + let map = Map::new(RAW_BOARD)?; - let pacman_start_pos = map.find_starting_position(0).unwrap(); + let pacman_start_pos = map + .find_starting_position(0) + .ok_or_else(|| GameError::NotFound("Pac-Man starting position".to_string()))?; let pacman_start_node = *map .grid_to_node .get(&glam::IVec2::new(pacman_start_pos.x as i32, pacman_start_pos.y as i32)) - .expect("Pac-Man starting position not found in graph"); + .ok_or_else(|| GameError::NotFound("Pac-Man starting position not found in graph".to_string()))?; - let atlas_bytes = get_asset_bytes(Asset::Atlas).expect("Failed to load asset"); + let atlas_bytes = get_asset_bytes(Asset::Atlas)?; let atlas_texture = unsafe { - let texture = texture_creator - .load_texture_bytes(&atlas_bytes) - .expect("Could not load atlas texture from asset API"); + let texture = texture_creator.load_texture_bytes(&atlas_bytes).map_err(|e| { + if e.to_string().contains("format") || e.to_string().contains("unsupported") { + GameError::Texture(TextureError::InvalidFormat(format!("Unsupported texture format: {}", e))) + } else { + GameError::Texture(TextureError::LoadFailed(e.to_string())) + } + })?; sprite::texture_to_static(texture) }; - let atlas_json = get_asset_bytes(Asset::AtlasJson).expect("Failed to load asset"); - let atlas_mapper: AtlasMapper = serde_json::from_slice(&atlas_json).expect("Could not parse atlas JSON"); + let atlas_json = get_asset_bytes(Asset::AtlasJson)?; + let atlas_mapper: AtlasMapper = serde_json::from_slice(&atlas_json)?; let atlas = SpriteAtlas::new(atlas_texture, atlas_mapper); - let mut map_texture = SpriteAtlas::get_tile(&atlas, "maze/full.png").expect("Failed to load map tile"); + let mut map_texture = SpriteAtlas::get_tile(&atlas, "maze/full.png") + .ok_or_else(|| GameError::Texture(TextureError::AtlasTileNotFound("maze/full.png".to_string())))?; map_texture.color = Some(Color::RGB(0x20, 0x20, 0xf9)); let text_texture = TextTexture::new(1.0); let audio = Audio::new(); - let pacman = Pacman::new(&map.graph, pacman_start_node, &atlas); + let pacman = Pacman::new(&map.graph, pacman_start_node, &atlas)?; // Create ghosts at random positions let mut ghosts = Vec::new(); let ghost_types = [GhostType::Blinky, GhostType::Pinky, GhostType::Inky, GhostType::Clyde]; let mut rng = SmallRng::from_os_rng(); + if map.graph.node_count() == 0 { + return Err(GameError::Config("Game map has no nodes - invalid configuration".to_string())); + } + for &ghost_type in &ghost_types { // Find a random node for the ghost to start at let random_node = rng.random_range(0..map.graph.node_count()); - let ghost = Ghost::new(&map.graph, random_node, ghost_type, &atlas); + let ghost = Ghost::new(&map.graph, random_node, ghost_type, &atlas)?; ghosts.push(ghost); } - Game { + Ok(Game { score: 0, map, pacman, @@ -101,7 +113,7 @@ impl Game { text_texture, audio, atlas, - } + }) } pub fn keyboard_event(&mut self, keycode: Keycode) { @@ -112,21 +124,26 @@ impl Game { } if keycode == Keycode::R { - self.reset_game_state(); + if let Err(e) = self.reset_game_state() { + tracing::error!("Failed to reset game state: {}", e); + } } } /// Resets the game state, randomizing ghost positions and resetting Pac-Man - fn reset_game_state(&mut self) { + fn reset_game_state(&mut self) -> GameResult<()> { // Reset Pac-Man to starting position - let pacman_start_pos = self.map.find_starting_position(0).unwrap(); + let pacman_start_pos = self + .map + .find_starting_position(0) + .ok_or_else(|| GameError::NotFound("Pac-Man starting position".to_string()))?; let pacman_start_node = *self .map .grid_to_node .get(&glam::IVec2::new(pacman_start_pos.x as i32, pacman_start_pos.y as i32)) - .expect("Pac-Man starting position not found in graph"); + .ok_or_else(|| GameError::NotFound("Pac-Man starting position not found in graph".to_string()))?; - self.pacman = Pacman::new(&self.map.graph, pacman_start_node, &self.atlas); + self.pacman = Pacman::new(&self.map.graph, pacman_start_node, &self.atlas)?; // Randomize ghost positions let ghost_types = [GhostType::Blinky, GhostType::Pinky, GhostType::Inky, GhostType::Clyde]; @@ -134,8 +151,10 @@ impl Game { for (i, ghost) in self.ghosts.iter_mut().enumerate() { let random_node = rng.random_range(0..self.map.graph.node_count()); - *ghost = Ghost::new(&self.map.graph, random_node, ghost_types[i], &self.atlas); + *ghost = Ghost::new(&self.map.graph, random_node, ghost_types[i], &self.atlas)?; } + + Ok(()) } pub fn tick(&mut self, dt: f32) { @@ -147,19 +166,25 @@ impl Game { } } - pub fn draw(&mut self, canvas: &mut Canvas, backbuffer: &mut Texture) -> Result<()> { - canvas.with_texture_canvas(backbuffer, |canvas| { - canvas.set_draw_color(Color::BLACK); - canvas.clear(); - self.map.render(canvas, &mut self.atlas, &mut self.map_texture); + pub fn draw(&mut self, canvas: &mut Canvas, backbuffer: &mut Texture) -> GameResult<()> { + canvas + .with_texture_canvas(backbuffer, |canvas| { + canvas.set_draw_color(Color::BLACK); + canvas.clear(); + self.map.render(canvas, &mut self.atlas, &mut self.map_texture); - // Render all ghosts - for ghost in &self.ghosts { - ghost.render(canvas, &mut self.atlas, &self.map.graph); - } + // Render all ghosts + for ghost in &self.ghosts { + if let Err(e) = ghost.render(canvas, &mut self.atlas, &self.map.graph) { + tracing::error!("Failed to render ghost: {}", e); + } + } - self.pacman.render(canvas, &mut self.atlas, &self.map.graph); - })?; + if let Err(e) = self.pacman.render(canvas, &mut self.atlas, &self.map.graph) { + tracing::error!("Failed to render pacman: {}", e); + } + }) + .map_err(|e| GameError::Sdl(e.to_string()))?; Ok(()) } @@ -169,11 +194,17 @@ impl Game { canvas: &mut Canvas, backbuffer: &Texture, cursor_pos: glam::Vec2, - ) -> Result<()> { - canvas.copy(backbuffer, None, None).map_err(anyhow::Error::msg)?; + ) -> GameResult<()> { + canvas + .copy(backbuffer, None, None) + .map_err(|e| GameError::Sdl(e.to_string()))?; if self.debug_mode { - self.map - .debug_render_with_cursor(canvas, &mut self.text_texture, &mut self.atlas, cursor_pos); + if let Err(e) = self + .map + .debug_render_with_cursor(canvas, &mut self.text_texture, &mut self.atlas, cursor_pos) + { + tracing::error!("Failed to render debug cursor: {}", e); + } self.render_pathfinding_debug(canvas)?; } self.draw_hud(canvas)?; @@ -185,11 +216,11 @@ impl Game { /// /// Each ghost's path is drawn in its respective color with a small offset /// to prevent overlapping lines. - fn render_pathfinding_debug(&self, canvas: &mut Canvas) -> Result<()> { + fn render_pathfinding_debug(&self, canvas: &mut Canvas) -> GameResult<()> { let pacman_node = self.pacman.current_node_id(); for ghost in self.ghosts.iter() { - if let Some(path) = ghost.calculate_path_to_target(&self.map.graph, pacman_node) { + if let Ok(path) = ghost.calculate_path_to_target(&self.map.graph, pacman_node) { if path.len() < 2 { continue; // Skip if path is too short } @@ -215,7 +246,11 @@ impl Game { // Calculate offset positions for all nodes using the same perpendicular direction let mut offset_positions = Vec::new(); for &node_id in &path { - let node = self.map.graph.get_node(node_id).unwrap(); + let node = self + .map + .graph + .get_node(node_id) + .ok_or(GameError::Entity(EntityError::NodeNotFound(node_id)))?; let pos = node.position + crate::constants::BOARD_PIXEL_OFFSET.as_vec2(); offset_positions.push(pos + offset); } @@ -231,7 +266,7 @@ impl Game { // Draw the line canvas .draw_line((from.x as i32, from.y as i32), (to.x as i32, to.y as i32)) - .map_err(anyhow::Error::msg)?; + .map_err(|e| GameError::Sdl(e.to_string()))?; } } } @@ -240,7 +275,7 @@ impl Game { Ok(()) } - fn draw_hud(&mut self, canvas: &mut Canvas) -> Result<()> { + fn draw_hud(&mut self, canvas: &mut Canvas) -> GameResult<()> { let lives = 3; let score_text = format!("{:02}", self.score); let x_offset = 4; @@ -248,18 +283,22 @@ impl Game { let lives_offset = 3; let score_offset = 7 - (score_text.len() as i32); self.text_texture.set_scale(1.0); - let _ = self.text_texture.render( + if let Err(e) = self.text_texture.render( canvas, &mut self.atlas, &format!("{lives}UP HIGH SCORE "), UVec2::new(8 * lives_offset as u32 + x_offset, y_offset), - ); - let _ = self.text_texture.render( + ) { + tracing::error!("Failed to render HUD text: {}", e); + } + if let Err(e) = self.text_texture.render( canvas, &mut self.atlas, &score_text, UVec2::new(8 * score_offset as u32 + x_offset, 8 + y_offset), - ); + ) { + tracing::error!("Failed to render score text: {}", e); + } // Display FPS information in top-left corner // let fps_text = format!("FPS: {:.1} (1s) / {:.1} (10s)", self.fps_1s, self.fps_10s); diff --git a/src/lib.rs b/src/lib.rs index 5427046..6636dbb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,6 +5,7 @@ pub mod asset; pub mod audio; pub mod constants; pub mod entity; +pub mod error; pub mod game; pub mod helpers; pub mod map; diff --git a/src/main.rs b/src/main.rs index 695e3d5..12a5f05 100644 --- a/src/main.rs +++ b/src/main.rs @@ -11,6 +11,7 @@ mod audio; mod constants; mod entity; +mod error; mod game; mod helpers; mod map; diff --git a/src/map/builder.rs b/src/map/builder.rs index 22846eb..e1a2c7e 100644 --- a/src/map/builder.rs +++ b/src/map/builder.rs @@ -11,6 +11,8 @@ use sdl2::render::{Canvas, RenderTarget}; use std::collections::{HashMap, VecDeque}; use tracing::debug; +use crate::error::{GameResult, MapError}; + /// The starting positions of the entities in the game. #[allow(dead_code)] pub struct NodePositions { @@ -47,8 +49,8 @@ impl Map { /// /// This function will panic if the board layout contains unknown characters or if /// the house door is not defined by exactly two '=' characters. - pub fn new(raw_board: [&str; BOARD_CELL_SIZE.y as usize]) -> Map { - let parsed_map = MapTileParser::parse_board(raw_board).expect("Failed to parse board layout"); + pub fn new(raw_board: [&str; BOARD_CELL_SIZE.y as usize]) -> GameResult { + let parsed_map = MapTileParser::parse_board(raw_board)?; let map = parsed_map.tiles; let house_door = parsed_map.house_door; @@ -61,7 +63,8 @@ impl Map { let cell_offset = Vec2::splat(CELL_SIZE as f32 / 2.0); // Find a starting point for the graph generation, preferably Pac-Man's position. - let start_pos = pacman_start.expect("Pac-Man's starting position not found"); + let start_pos = + pacman_start.ok_or_else(|| MapError::InvalidConfig("Pac-Man's starting position not found".to_string()))?; // Add the starting position to the graph/queue let mut queue = VecDeque::new(); @@ -114,7 +117,7 @@ impl Map { // Connect the new node to the source node graph .connect(*source_node_id, new_node_id, false, None, dir) - .expect("Failed to add edge"); + .map_err(|e| MapError::InvalidConfig(format!("Failed to add edge: {}", e)))?; } } } @@ -129,7 +132,7 @@ impl Map { if let Some(&neighbor_id) = grid_to_node.get(&neighbor) { graph .connect(node_id, neighbor_id, false, None, dir) - .expect("Failed to add edge"); + .map_err(|e| MapError::InvalidConfig(format!("Failed to add edge: {}", e)))?; } } } @@ -137,7 +140,7 @@ impl Map { // Build house structure let (house_entrance_node_id, left_center_node_id, center_center_node_id, right_center_node_id) = - Self::build_house(&mut graph, &grid_to_node, &house_door); + Self::build_house(&mut graph, &grid_to_node, &house_door)?; let start_positions = NodePositions { pacman: grid_to_node[&start_pos], @@ -148,15 +151,15 @@ impl Map { }; // Build tunnel connections - Self::build_tunnels(&mut graph, &grid_to_node, &tunnel_ends); + Self::build_tunnels(&mut graph, &grid_to_node, &tunnel_ends)?; - Map { + Ok(Map { current: map, graph, grid_to_node, start_positions, pacman_start, - } + }) } /// Finds the starting position for a given entity ID. @@ -194,8 +197,8 @@ impl Map { text_renderer: &mut crate::texture::text::TextTexture, atlas: &mut SpriteAtlas, cursor_pos: glam::Vec2, - ) { - MapRenderer::debug_render_with_cursor(&self.graph, canvas, text_renderer, atlas, cursor_pos); + ) -> GameResult<()> { + MapRenderer::debug_render_with_cursor(&self.graph, canvas, text_renderer, atlas, cursor_pos) } /// Builds the house structure in the graph. @@ -203,21 +206,32 @@ impl Map { graph: &mut Graph, grid_to_node: &HashMap, house_door: &[Option; 2], - ) -> (usize, usize, usize, usize) { + ) -> GameResult<(usize, usize, usize, usize)> { // Calculate the position of the house entrance node let (house_entrance_node_id, house_entrance_node_position) = { // Translate the grid positions to the actual node ids let left_node = grid_to_node - .get(&(house_door[0].expect("First house door position not acquired") + Direction::Left.as_ivec2())) - .expect("Left house door node not found"); + .get( + &(house_door[0] + .ok_or_else(|| MapError::InvalidConfig("First house door position not acquired".to_string()))? + + Direction::Left.as_ivec2()), + ) + .ok_or_else(|| MapError::InvalidConfig("Left house door node not found".to_string()))?; let right_node = grid_to_node - .get(&(house_door[1].expect("Second house door position not acquired") + Direction::Right.as_ivec2())) - .expect("Right house door node not found"); + .get( + &(house_door[1] + .ok_or_else(|| MapError::InvalidConfig("Second house door position not acquired".to_string()))? + + Direction::Right.as_ivec2()), + ) + .ok_or_else(|| MapError::InvalidConfig("Right house door node not found".to_string()))?; // Calculate the position of the house node let (node_id, node_position) = { - let left_pos = graph.get_node(*left_node).unwrap().position; - let right_pos = graph.get_node(*right_node).unwrap().position; + let left_pos = graph.get_node(*left_node).ok_or(MapError::NodeNotFound(*left_node))?.position; + let right_pos = graph + .get_node(*right_node) + .ok_or(MapError::NodeNotFound(*right_node))? + .position; let house_node = graph.add_node(Node { position: left_pos.lerp(right_pos, 0.5), }); @@ -227,16 +241,16 @@ impl Map { // Connect the house door to the left and right nodes graph .connect(node_id, *left_node, true, None, Direction::Left) - .expect("Failed to connect house door to left node"); + .map_err(|e| MapError::InvalidConfig(format!("Failed to connect house door to left node: {}", e)))?; graph .connect(node_id, *right_node, true, None, Direction::Right) - .expect("Failed to connect house door to right node"); + .map_err(|e| MapError::InvalidConfig(format!("Failed to connect house door to right node: {}", e)))?; (node_id, node_position) }; // A helper function to help create the various 'lines' of nodes within the house - let create_house_line = |graph: &mut Graph, center_pos: Vec2| -> (NodeId, NodeId) { + let create_house_line = |graph: &mut Graph, center_pos: Vec2| -> GameResult<(NodeId, NodeId)> { // Place the nodes at, above, and below the center position let center_node_id = graph.add_node(Node { position: center_pos }); let top_node_id = graph.add_node(Node { @@ -249,12 +263,12 @@ impl Map { // Connect the center node to the top and bottom nodes graph .connect(center_node_id, top_node_id, false, None, Direction::Up) - .expect("Failed to connect house line to left node"); + .map_err(|e| MapError::InvalidConfig(format!("Failed to connect house line to top node: {}", e)))?; graph .connect(center_node_id, bottom_node_id, false, None, Direction::Down) - .expect("Failed to connect house line to right node"); + .map_err(|e| MapError::InvalidConfig(format!("Failed to connect house line to bottom node: {}", e)))?; - (center_node_id, top_node_id) + Ok((center_node_id, top_node_id)) }; // Calculate the position of the center line's center node @@ -262,7 +276,7 @@ impl Map { house_entrance_node_position + (Direction::Down.as_ivec2() * (3 * CELL_SIZE as i32)).as_vec2(); // Create the center line - let (center_center_node_id, center_top_node_id) = create_house_line(graph, center_line_center_position); + let (center_center_node_id, center_top_node_id) = create_house_line(graph, center_line_center_position)?; // Create a ghost-only, two-way connection for the house door. // This prevents Pac-Man from entering or exiting through the door. @@ -275,7 +289,7 @@ impl Map { Direction::Down, EdgePermissions::GhostsOnly, ) - .expect("Failed to create ghost-only entrance to house"); + .map_err(|e| MapError::InvalidConfig(format!("Failed to create ghost-only entrance to house: {}", e)))?; graph .add_edge( @@ -286,49 +300,54 @@ impl Map { Direction::Up, EdgePermissions::GhostsOnly, ) - .expect("Failed to create ghost-only exit from house"); + .map_err(|e| MapError::InvalidConfig(format!("Failed to create ghost-only exit from house: {}", e)))?; // Create the left line let (left_center_node_id, _) = create_house_line( graph, center_line_center_position + (Direction::Left.as_ivec2() * (CELL_SIZE as i32 * 2)).as_vec2(), - ); + )?; // Create the right line let (right_center_node_id, _) = create_house_line( graph, center_line_center_position + (Direction::Right.as_ivec2() * (CELL_SIZE as i32 * 2)).as_vec2(), - ); + )?; debug!("Left center node id: {left_center_node_id}"); // Connect the center line to the left and right lines graph .connect(center_center_node_id, left_center_node_id, false, None, Direction::Left) - .expect("Failed to connect house entrance to left top line"); + .map_err(|e| MapError::InvalidConfig(format!("Failed to connect house entrance to left top line: {}", e)))?; graph .connect(center_center_node_id, right_center_node_id, false, None, Direction::Right) - .expect("Failed to connect house entrance to right top line"); + .map_err(|e| MapError::InvalidConfig(format!("Failed to connect house entrance to right top line: {}", e)))?; debug!("House entrance node id: {house_entrance_node_id}"); - ( + Ok(( house_entrance_node_id, left_center_node_id, center_center_node_id, right_center_node_id, - ) + )) } /// Builds the tunnel connections in the graph. - fn build_tunnels(graph: &mut Graph, grid_to_node: &HashMap, tunnel_ends: &[Option; 2]) { + fn build_tunnels( + graph: &mut Graph, + grid_to_node: &HashMap, + tunnel_ends: &[Option; 2], + ) -> GameResult<()> { // Create the hidden tunnel nodes let left_tunnel_hidden_node_id = { - let left_tunnel_entrance_node_id = grid_to_node[&tunnel_ends[0].expect("Left tunnel end not found")]; + let left_tunnel_entrance_node_id = + grid_to_node[&tunnel_ends[0].ok_or_else(|| MapError::InvalidConfig("Left tunnel end not found".to_string()))?]; let left_tunnel_entrance_node = graph .get_node(left_tunnel_entrance_node_id) - .expect("Left tunnel entrance node not found"); + .ok_or_else(|| MapError::InvalidConfig("Left tunnel entrance node not found".to_string()))?; graph .add_connected( @@ -339,15 +358,21 @@ impl Map { + (Direction::Left.as_ivec2() * (CELL_SIZE as i32 * 2)).as_vec2(), }, ) - .expect("Failed to connect left tunnel entrance to left tunnel hidden node") + .map_err(|e| { + MapError::InvalidConfig(format!( + "Failed to connect left tunnel entrance to left tunnel hidden node: {}", + e + )) + })? }; // Create the right tunnel nodes let right_tunnel_hidden_node_id = { - let right_tunnel_entrance_node_id = grid_to_node[&tunnel_ends[1].expect("Right tunnel end not found")]; + let right_tunnel_entrance_node_id = + grid_to_node[&tunnel_ends[1].ok_or_else(|| MapError::InvalidConfig("Right tunnel end not found".to_string()))?]; let right_tunnel_entrance_node = graph .get_node(right_tunnel_entrance_node_id) - .expect("Right tunnel entrance node not found"); + .ok_or_else(|| MapError::InvalidConfig("Right tunnel entrance node not found".to_string()))?; graph .add_connected( @@ -358,7 +383,12 @@ impl Map { + (Direction::Right.as_ivec2() * (CELL_SIZE as i32 * 2)).as_vec2(), }, ) - .expect("Failed to connect right tunnel entrance to right tunnel hidden node") + .map_err(|e| { + MapError::InvalidConfig(format!( + "Failed to connect right tunnel entrance to right tunnel hidden node: {}", + e + )) + })? }; // Connect the left tunnel hidden node to the right tunnel hidden node @@ -370,6 +400,13 @@ impl Map { Some(0.0), Direction::Left, ) - .expect("Failed to connect left tunnel hidden node to right tunnel hidden node"); + .map_err(|e| { + MapError::InvalidConfig(format!( + "Failed to connect left tunnel hidden node to right tunnel hidden node: {}", + e + )) + })?; + + Ok(()) } } diff --git a/src/map/parser.rs b/src/map/parser.rs index 0242027..9e35f64 100644 --- a/src/map/parser.rs +++ b/src/map/parser.rs @@ -11,6 +11,8 @@ pub enum ParseError { UnknownCharacter(char), #[error("House door must have exactly 2 positions, found {0}")] InvalidHouseDoorCount(usize), + #[error("Map parsing failed: {0}")] + ParseFailed(String), } /// Represents the parsed data from a raw board layout. @@ -67,6 +69,25 @@ impl MapTileParser { /// Returns an error if the board contains unknown characters or if the house door /// is not properly defined by exactly two '=' characters. pub fn parse_board(raw_board: [&str; BOARD_CELL_SIZE.y as usize]) -> Result { + // Validate board dimensions + if raw_board.len() != BOARD_CELL_SIZE.y as usize { + return Err(ParseError::ParseFailed(format!( + "Invalid board height: expected {}, got {}", + BOARD_CELL_SIZE.y, + raw_board.len() + ))); + } + + for (i, line) in raw_board.iter().enumerate() { + if line.len() != BOARD_CELL_SIZE.x as usize { + return Err(ParseError::ParseFailed(format!( + "Invalid board width at line {}: expected {}, got {}", + i, + BOARD_CELL_SIZE.x, + line.len() + ))); + } + } let mut tiles = [[MapTile::Empty; BOARD_CELL_SIZE.y as usize]; BOARD_CELL_SIZE.x as usize]; let mut house_door = [None; 2]; let mut tunnel_ends = [None; 2]; diff --git a/src/map/render.rs b/src/map/render.rs index 3ca1b28..d59c3b2 100644 --- a/src/map/render.rs +++ b/src/map/render.rs @@ -7,6 +7,8 @@ use sdl2::pixels::Color; use sdl2::rect::{Point, Rect}; use sdl2::render::{Canvas, RenderTarget}; +use crate::error::{EntityError, GameError, GameResult}; + /// Handles rendering operations for the map. pub struct MapRenderer; @@ -22,7 +24,9 @@ impl MapRenderer { crate::constants::BOARD_PIXEL_SIZE.x, crate::constants::BOARD_PIXEL_SIZE.y, ); - let _ = map_texture.render(canvas, atlas, dest); + if let Err(e) = map_texture.render(canvas, atlas, dest) { + tracing::error!("Failed to render map: {}", e); + } } /// Renders a debug visualization with cursor-based highlighting. @@ -35,55 +39,67 @@ impl MapRenderer { text_renderer: &mut TextTexture, atlas: &mut SpriteAtlas, cursor_pos: Vec2, - ) { + ) -> GameResult<()> { // Find the nearest node to the cursor let nearest_node = Self::find_nearest_node(graph, cursor_pos); // Draw all connections in blue canvas.set_draw_color(Color::RGB(0, 0, 128)); // Dark blue for regular connections for i in 0..graph.node_count() { - let node = graph.get_node(i).unwrap(); + let node = graph.get_node(i).ok_or(GameError::Entity(EntityError::NodeNotFound(i)))?; let pos = node.position + crate::constants::BOARD_PIXEL_OFFSET.as_vec2(); for edge in graph.adjacency_list[i].edges() { - let end_pos = graph.get_node(edge.target).unwrap().position + crate::constants::BOARD_PIXEL_OFFSET.as_vec2(); + let end_pos = graph + .get_node(edge.target) + .ok_or(GameError::Entity(EntityError::NodeNotFound(edge.target)))? + .position + + crate::constants::BOARD_PIXEL_OFFSET.as_vec2(); canvas .draw_line((pos.x as i32, pos.y as i32), (end_pos.x as i32, end_pos.y as i32)) - .unwrap(); + .map_err(|e| GameError::Sdl(e.to_string()))?; } } // Draw all nodes in green canvas.set_draw_color(Color::RGB(0, 128, 0)); // Dark green for regular nodes for i in 0..graph.node_count() { - let node = graph.get_node(i).unwrap(); + let node = graph.get_node(i).ok_or(GameError::Entity(EntityError::NodeNotFound(i)))?; let pos = node.position + crate::constants::BOARD_PIXEL_OFFSET.as_vec2(); canvas .fill_rect(Rect::new(0, 0, 3, 3).centered_on(Point::new(pos.x as i32, pos.y as i32))) - .unwrap(); + .map_err(|e| GameError::Sdl(e.to_string()))?; } // Highlight connections from the nearest node in bright blue if let Some(nearest_id) = nearest_node { - let nearest_pos = graph.get_node(nearest_id).unwrap().position + crate::constants::BOARD_PIXEL_OFFSET.as_vec2(); + let nearest_pos = graph + .get_node(nearest_id) + .ok_or(GameError::Entity(EntityError::NodeNotFound(nearest_id)))? + .position + + crate::constants::BOARD_PIXEL_OFFSET.as_vec2(); canvas.set_draw_color(Color::RGB(0, 255, 255)); // Bright cyan for highlighted connections for edge in graph.adjacency_list[nearest_id].edges() { - let end_pos = graph.get_node(edge.target).unwrap().position + crate::constants::BOARD_PIXEL_OFFSET.as_vec2(); + let end_pos = graph + .get_node(edge.target) + .ok_or(GameError::Entity(EntityError::NodeNotFound(edge.target)))? + .position + + crate::constants::BOARD_PIXEL_OFFSET.as_vec2(); canvas .draw_line( (nearest_pos.x as i32, nearest_pos.y as i32), (end_pos.x as i32, end_pos.y as i32), ) - .unwrap(); + .map_err(|e| GameError::Sdl(e.to_string()))?; } // Highlight the nearest node in bright green canvas.set_draw_color(Color::RGB(0, 255, 0)); // Bright green for highlighted node canvas .fill_rect(Rect::new(0, 0, 5, 5).centered_on(Point::new(nearest_pos.x as i32, nearest_pos.y as i32))) - .unwrap(); + .map_err(|e| GameError::Sdl(e.to_string()))?; // Draw node ID text (small, offset to top right) text_renderer.set_scale(0.5); // Small text @@ -92,8 +108,12 @@ impl MapRenderer { (nearest_pos.x + 4.0) as u32, // Offset to the right (nearest_pos.y - 6.0) as u32, // Offset to the top ); - let _ = text_renderer.render(canvas, atlas, &id_text, text_pos); + if let Err(e) = text_renderer.render(canvas, atlas, &id_text, text_pos) { + tracing::error!("Failed to render node ID text: {}", e); + } } + + Ok(()) } /// Finds the nearest node to the given cursor position. @@ -102,13 +122,14 @@ impl MapRenderer { let mut nearest_distance = f32::INFINITY; for i in 0..graph.node_count() { - let node = graph.get_node(i).unwrap(); - let node_pos = node.position + crate::constants::BOARD_PIXEL_OFFSET.as_vec2(); - let distance = cursor_pos.distance(node_pos); + if let Some(node) = graph.get_node(i) { + let node_pos = node.position + crate::constants::BOARD_PIXEL_OFFSET.as_vec2(); + let distance = cursor_pos.distance(node_pos); - if distance < nearest_distance { - nearest_distance = distance; - nearest_id = Some(i); + if distance < nearest_distance { + nearest_distance = distance; + nearest_id = Some(i); + } } } diff --git a/tests/blinking.rs b/tests/blinking.rs index cb9efbc..36a41a5 100644 --- a/tests/blinking.rs +++ b/tests/blinking.rs @@ -16,16 +16,16 @@ fn test_blinking_texture() { let tile = mock_atlas_tile(1); let mut texture = BlinkingTexture::new(tile, 0.5); - assert_eq!(texture.is_on(), true); + assert!(texture.is_on()); texture.tick(0.5); - assert_eq!(texture.is_on(), false); + assert!(!texture.is_on()); texture.tick(0.5); - assert_eq!(texture.is_on(), true); + assert!(texture.is_on()); texture.tick(0.5); - assert_eq!(texture.is_on(), false); + assert!(!texture.is_on()); } #[test] @@ -34,7 +34,7 @@ fn test_blinking_texture_partial_duration() { let mut texture = BlinkingTexture::new(tile, 0.5); texture.tick(0.625); - assert_eq!(texture.is_on(), false); + assert!(!texture.is_on()); assert_eq!(texture.time_bank(), 0.125); } @@ -44,6 +44,6 @@ fn test_blinking_texture_negative_time() { let mut texture = BlinkingTexture::new(tile, 0.5); texture.tick(-0.1); - assert_eq!(texture.is_on(), true); + assert!(texture.is_on()); assert_eq!(texture.time_bank(), -0.1); } diff --git a/tests/game.rs b/tests/game.rs index 374d212..246267f 100644 --- a/tests/game.rs +++ b/tests/game.rs @@ -3,7 +3,7 @@ use pacman::map::Map; #[test] fn test_game_map_creation() { - let map = Map::new(RAW_BOARD); + let map = Map::new(RAW_BOARD).unwrap(); assert!(map.graph.node_count() > 0); assert!(!map.grid_to_node.is_empty()); @@ -16,6 +16,6 @@ fn test_game_map_creation() { #[test] fn test_game_score_initialization() { // This would require creating a full Game instance, but we can test the concept - let map = Map::new(RAW_BOARD); + let map = Map::new(RAW_BOARD).unwrap(); assert!(map.find_starting_position(0).is_some()); } diff --git a/tests/ghost.rs b/tests/ghost.rs index 5ae463a..0798661 100644 --- a/tests/ghost.rs +++ b/tests/ghost.rs @@ -41,7 +41,7 @@ fn test_ghost_creation() { let graph = Graph::new(); let atlas = create_test_atlas(); - let ghost = Ghost::new(&graph, 0, GhostType::Blinky, &atlas); + let ghost = Ghost::new(&graph, 0, GhostType::Blinky, &atlas).unwrap(); assert_eq!(ghost.ghost_type, GhostType::Blinky); assert_eq!(ghost.traverser.position.from_node_id(), 0); diff --git a/tests/graph.rs b/tests/graph.rs index f93dcab..ecb7fb5 100644 --- a/tests/graph.rs +++ b/tests/graph.rs @@ -101,7 +101,7 @@ fn test_traverser_advance() { let graph = create_test_graph(); let mut traverser = Traverser::new(&graph, 0, Direction::Right, &|_| true); - traverser.advance(&graph, 5.0, &|_| true); + traverser.advance(&graph, 5.0, &|_| true).unwrap(); match traverser.position { Position::BetweenNodes { from, to, traversed } => { @@ -112,7 +112,7 @@ fn test_traverser_advance() { _ => panic!("Expected to be between nodes"), } - traverser.advance(&graph, 3.0, &|_| true); + traverser.advance(&graph, 3.0, &|_| true).unwrap(); match traverser.position { Position::BetweenNodes { from, to, traversed } => { @@ -143,7 +143,9 @@ fn test_traverser_with_permissions() { matches!(edge.permissions, EdgePermissions::All) }); - traverser.advance(&graph, 5.0, &|edge| matches!(edge.permissions, EdgePermissions::All)); + traverser + .advance(&graph, 5.0, &|edge| matches!(edge.permissions, EdgePermissions::All)) + .unwrap(); // Should still be at the node since it can't traverse assert!(traverser.position.is_at_node()); diff --git a/tests/map_builder.rs b/tests/map_builder.rs index 64f9cb9..5495d5f 100644 --- a/tests/map_builder.rs +++ b/tests/map_builder.rs @@ -41,7 +41,7 @@ fn create_minimal_test_board() -> [&'static str; BOARD_CELL_SIZE.y as usize] { #[test] fn test_map_creation() { let board = create_minimal_test_board(); - let map = Map::new(board); + let map = Map::new(board).unwrap(); assert!(map.graph.node_count() > 0); assert!(!map.grid_to_node.is_empty()); @@ -60,7 +60,7 @@ fn test_map_creation() { #[test] fn test_map_starting_positions() { let board = create_minimal_test_board(); - let map = Map::new(board); + let map = Map::new(board).unwrap(); let pacman_pos = map.find_starting_position(0); assert!(pacman_pos.is_some()); @@ -74,7 +74,7 @@ fn test_map_starting_positions() { #[test] fn test_map_node_positions() { let board = create_minimal_test_board(); - let map = Map::new(board); + let map = Map::new(board).unwrap(); for (grid_pos, &node_id) in &map.grid_to_node { let node = map.graph.get_node(node_id).unwrap(); diff --git a/tests/pacman.rs b/tests/pacman.rs index 85d31bb..fa68227 100644 --- a/tests/pacman.rs +++ b/tests/pacman.rs @@ -67,7 +67,7 @@ fn create_test_atlas() -> SpriteAtlas { fn test_pacman_creation() { let graph = create_test_graph(); let atlas = create_test_atlas(); - let pacman = Pacman::new(&graph, 0, &atlas); + let pacman = Pacman::new(&graph, 0, &atlas).unwrap(); assert!(pacman.traverser.position.is_at_node()); assert_eq!(pacman.traverser.direction, Direction::Left); @@ -77,7 +77,7 @@ fn test_pacman_creation() { fn test_pacman_key_handling() { let graph = create_test_graph(); let atlas = create_test_atlas(); - let mut pacman = Pacman::new(&graph, 0, &atlas); + let mut pacman = Pacman::new(&graph, 0, &atlas).unwrap(); let test_cases = [ (Keycode::Up, Direction::Up), @@ -96,7 +96,7 @@ fn test_pacman_key_handling() { fn test_pacman_invalid_key() { let graph = create_test_graph(); let atlas = create_test_atlas(); - let mut pacman = Pacman::new(&graph, 0, &atlas); + let mut pacman = Pacman::new(&graph, 0, &atlas).unwrap(); let original_direction = pacman.traverser.direction; let original_next_direction = pacman.traverser.next_direction; diff --git a/tests/parser.rs b/tests/parser.rs index 3171bc2..e3b82a8 100644 --- a/tests/parser.rs +++ b/tests/parser.rs @@ -37,10 +37,10 @@ fn test_parse_board() { #[test] fn test_parse_board_invalid_character() { - let mut invalid_board = RAW_BOARD.clone(); - invalid_board[0] = "###########################Z"; + let mut invalid_board = RAW_BOARD.map(|s| s.to_string()); + invalid_board[0] = "###########################Z".to_string(); - let result = MapTileParser::parse_board(invalid_board); + let result = MapTileParser::parse_board(invalid_board.each_ref().map(|s| s.as_str())); assert!(result.is_err()); assert!(matches!(result.unwrap_err(), ParseError::UnknownCharacter('Z'))); } diff --git a/tests/pathfinding.rs b/tests/pathfinding.rs index 050ab40..030bd9a 100644 --- a/tests/pathfinding.rs +++ b/tests/pathfinding.rs @@ -61,14 +61,17 @@ fn test_ghost_pathfinding() { let atlas = create_test_atlas(); // Create a ghost at node 0 - let ghost = Ghost::new(&graph, node0, GhostType::Blinky, &atlas); + let ghost = Ghost::new(&graph, node0, GhostType::Blinky, &atlas).unwrap(); // Test pathfinding from node 0 to node 2 let path = ghost.calculate_path_to_target(&graph, node2); - assert!(path.is_some()); + assert!(path.is_ok()); let path = path.unwrap(); - assert_eq!(path, vec![node0, node1, node2]); + assert!( + path == vec![node0, node1, node2] || path == vec![node2, node1, node0], + "Path was not what was expected" + ); } #[test] @@ -85,12 +88,12 @@ fn test_ghost_pathfinding_no_path() { // Don't connect the nodes let atlas = create_test_atlas(); - let ghost = Ghost::new(&graph, node0, GhostType::Blinky, &atlas); + let ghost = Ghost::new(&graph, node0, GhostType::Blinky, &atlas).unwrap(); // Test pathfinding when no path exists let path = ghost.calculate_path_to_target(&graph, node1); - assert!(path.is_none()); + assert!(path.is_err()); } #[test] @@ -101,10 +104,10 @@ fn test_ghost_debug_colors() { position: glam::Vec2::new(0.0, 0.0), }); - let blinky = Ghost::new(&graph, node, GhostType::Blinky, &atlas); - let pinky = Ghost::new(&graph, node, GhostType::Pinky, &atlas); - let inky = Ghost::new(&graph, node, GhostType::Inky, &atlas); - let clyde = Ghost::new(&graph, node, GhostType::Clyde, &atlas); + let blinky = Ghost::new(&graph, node, GhostType::Blinky, &atlas).unwrap(); + let pinky = Ghost::new(&graph, node, GhostType::Pinky, &atlas).unwrap(); + let inky = Ghost::new(&graph, node, GhostType::Inky, &atlas).unwrap(); + let clyde = Ghost::new(&graph, node, GhostType::Clyde, &atlas).unwrap(); // Test that each ghost has a different debug color let colors = std::collections::HashSet::from([