From dfe845840cd36cfdfad515f1ed75d33e28f3d97a Mon Sep 17 00:00:00 2001 From: Alejandro Perea <alexpro820@gmail.com> Date: Thu, 10 Feb 2022 21:00:58 +0100 Subject: [PATCH] Better errors (#152) * Close #124 * Add `TiledError::InvalidEncodingFormat` * Rename `DecompressingError` to `IoError` * Add `InvalidPropertyValue` & `UnknownPropertyType` * Remove `Other` * Remove `ParseTileError` * Misc comment improvements * Revert `IoError` back to `DecompressingError` - Use `CouldNotOpenFile` for `Map` * Address PR comments --- src/error.rs | 39 ++++++++++++++++++++++++++++++--------- src/layers/tile/util.rs | 39 ++++++++++++++++++++++----------------- src/map.rs | 14 ++++++++------ src/properties.rs | 29 ++++++++++++++++++----------- 4 files changed, 78 insertions(+), 43 deletions(-) diff --git a/src/error.rs b/src/error.rs index 3b0f789..b42727d 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,13 +1,8 @@ use std::{fmt, path::PathBuf}; -#[derive(Debug, Copy, Clone)] -pub enum ParseTileError { - ColorError, - OrientationError, -} - /// Errors which occured when parsing the file #[derive(Debug)] +#[non_exhaustive] pub enum TiledError { /// A attribute was missing, had the wrong type of wasn't formated /// correctly. @@ -31,7 +26,19 @@ pub enum TiledError { }, /// There was an invalid tile in the map parsed. InvalidTileFound, - Other(String), + /// Unknown encoding or compression format or invalid combination of both (for tile layers) + InvalidEncodingFormat { + encoding: Option<String>, + compression: Option<String>, + }, + /// There was an error parsing the value of a [`PropertyValue`]. + /// + /// [`PropertyValue`]: crate::PropertyValue + InvalidPropertyValue{description: String}, + /// Found an unknown property value type while parsing a [`PropertyValue`]. + /// + /// [`PropertyValue`]: crate::PropertyValue + UnknownPropertyType{name: String}, } impl fmt::Display for TiledError { @@ -62,12 +69,26 @@ impl fmt::Display for TiledError { ) } TiledError::InvalidTileFound => write!(fmt, "Invalid tile found in map being parsed"), - TiledError::Other(s) => write!(fmt, "{}", s), + TiledError::InvalidEncodingFormat { encoding: None, compression: None } => + write!( + fmt, + "Deprecated combination of encoding and compression" + ), + TiledError::InvalidEncodingFormat { encoding, compression } => + write!( + fmt, + "Unknown encoding or compression format or invalid combination of both (for tile layers): {} encoding with {} compression", + encoding.as_deref().unwrap_or("no"), + compression.as_deref().unwrap_or("no") + ), + TiledError::InvalidPropertyValue{description} => + write!(fmt, "Invalid property value: {}", description), + TiledError::UnknownPropertyType { name } => + write!(fmt, "Unknown property value type '{}'", name), } } } -// This is a skeleton implementation, which should probably be extended in the future. impl std::error::Error for TiledError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { diff --git a/src/layers/tile/util.rs b/src/layers/tile/util.rs index b3c3a00..8937a31 100644 --- a/src/layers/tile/util.rs +++ b/src/layers/tile/util.rs @@ -10,18 +10,18 @@ pub(crate) fn parse_data_line( parser: &mut impl Iterator<Item = XmlEventResult>, tilesets: &[MapTilesetGid], ) -> Result<Vec<Option<LayerTileData>>, TiledError> { - match (encoding, compression) { - (None, None) => { - return Err(TiledError::Other( - "XML format is currently not supported".to_string(), - )) + match (encoding.as_deref(), compression.as_deref()) { + (Some("base64"), None) => { + return parse_base64(parser).map(|v| convert_to_tiles(&v, tilesets)) } - (Some(e), None) => match e.as_ref() { - "base64" => return parse_base64(parser).map(|v| convert_to_tiles(&v, tilesets)), - "csv" => return decode_csv(parser, tilesets), - e => return Err(TiledError::Other(format!("Unknown encoding format {}", e))), - }, - (Some(e), Some(c)) => match (e.as_ref(), c.as_ref()) { + (Some("csv"), None) => return decode_csv(parser, tilesets), + (Some(_), None) => { + return Err(TiledError::InvalidEncodingFormat { + encoding, + compression, + }) + } + (Some(e), Some(c)) => match (e, c) { ("base64", "zlib") => { return parse_base64(parser) .and_then(decode_zlib) @@ -38,14 +38,19 @@ pub(crate) fn parse_data_line( .and_then(decode_zstd) .map(|v| convert_to_tiles(&v, tilesets)) } - (e, c) => { - return Err(TiledError::Other(format!( - "Unknown combination of {} encoding and {} compression", - e, c - ))) + _ => { + return Err(TiledError::InvalidEncodingFormat { + encoding, + compression, + }) } }, - _ => return Err(TiledError::Other("Missing encoding format".to_string())), + _ => { + return Err(TiledError::InvalidEncodingFormat { + encoding, + compression, + }) + } }; } diff --git a/src/map.rs b/src/map.rs index 9497470..20ae246 100644 --- a/src/map.rs +++ b/src/map.rs @@ -3,7 +3,7 @@ use std::{collections::HashMap, fmt, fs::File, io::Read, path::Path, rc::Rc, str use xml::{attribute::OwnedAttribute, reader::XmlEvent, EventReader}; use crate::{ - error::{ParseTileError, TiledError}, + error::TiledError, layers::{LayerData, LayerTag}, properties::{parse_properties, Color, Properties}, tileset::Tileset, @@ -90,8 +90,10 @@ impl Map { path: impl AsRef<Path>, cache: &mut impl ResourceCache, ) -> Result<Self, TiledError> { - let reader = File::open(path.as_ref()) - .map_err(|_| TiledError::Other(format!("Map file not found: {:?}", path.as_ref())))?; + let reader = File::open(path.as_ref()).map_err(|err| TiledError::CouldNotOpenFile { + path: path.as_ref().to_owned(), + err, + })?; Self::parse_reader(reader, path.as_ref(), cache) } } @@ -268,15 +270,15 @@ pub enum Orientation { } impl FromStr for Orientation { - type Err = ParseTileError; + type Err = (); - fn from_str(s: &str) -> Result<Orientation, ParseTileError> { + fn from_str(s: &str) -> Result<Self, Self::Err> { match s { "orthogonal" => Ok(Orientation::Orthogonal), "isometric" => Ok(Orientation::Isometric), "staggered" => Ok(Orientation::Staggered), "hexagonal" => Ok(Orientation::Hexagonal), - _ => Err(ParseTileError::OrientationError), + _ => Err(()), } } } diff --git a/src/properties.rs b/src/properties.rs index 928b7f9..0625c20 100644 --- a/src/properties.rs +++ b/src/properties.rs @@ -74,32 +74,39 @@ impl PropertyValue { match property_type.as_str() { "bool" => match value.parse() { Ok(val) => Ok(PropertyValue::BoolValue(val)), - Err(err) => Err(TiledError::Other(err.to_string())), + Err(err) => Err(TiledError::InvalidPropertyValue { + description: err.to_string(), + }), }, "float" => match value.parse() { Ok(val) => Ok(PropertyValue::FloatValue(val)), - Err(err) => Err(TiledError::Other(err.to_string())), + Err(err) => Err(TiledError::InvalidPropertyValue { + description: err.to_string(), + }), }, "int" => match value.parse() { Ok(val) => Ok(PropertyValue::IntValue(val)), - Err(err) => Err(TiledError::Other(err.to_string())), + Err(err) => Err(TiledError::InvalidPropertyValue { + description: err.to_string(), + }), }, "color" if value.len() > 1 => match u32::from_str_radix(&value[1..], 16) { Ok(color) => Ok(PropertyValue::ColorValue(color)), - Err(_) => Err(TiledError::Other(format!( - "Improperly formatted color property" - ))), + Err(err) => Err(TiledError::InvalidPropertyValue { + description: err.to_string(), + }), }, "string" => Ok(PropertyValue::StringValue(value)), "object" => match value.parse() { Ok(val) => Ok(PropertyValue::ObjectValue(val)), - Err(err) => Err(TiledError::Other(err.to_string())), + Err(err) => Err(TiledError::InvalidPropertyValue { + description: err.to_string(), + }), }, "file" => Ok(PropertyValue::FileValue(value)), - _ => Err(TiledError::Other(format!( - "Unknown property type \"{}\"", - property_type - ))), + _ => Err(TiledError::UnknownPropertyType { + name: property_type, + }), } } } -- GitLab