From 53a1e97d4fd0ac7728c8ed1feaa4c691cecc0009 Mon Sep 17 00:00:00 2001 From: Alejandro Perea <alexpro820@gmail.com> Date: Mon, 7 Mar 2022 11:59:08 +0100 Subject: [PATCH] Better macros (#184) * Optional `optionals` in `get_attrs!` * Improve `get_attrs` * Improve Map error * Simplify `parse_tag` internals --- src/animation.rs | 3 +- src/image.rs | 2 +- src/layers/mod.rs | 11 +----- src/layers/object.rs | 7 +--- src/layers/tile/finite.rs | 6 +-- src/layers/tile/infinite.rs | 9 ++--- src/layers/tile/mod.rs | 4 +- src/map.rs | 2 +- src/objects.rs | 6 +-- src/tileset.rs | 3 +- src/util.rs | 75 ++++++++++++++++++++++++------------- 11 files changed, 66 insertions(+), 62 deletions(-) diff --git a/src/animation.rs b/src/animation.rs index cb8d55e..2cf352c 100644 --- a/src/animation.rs +++ b/src/animation.rs @@ -21,9 +21,8 @@ pub struct Frame { impl Frame { pub(crate) fn new(attrs: Vec<OwnedAttribute>) -> Result<Frame, TiledError> { - let ((), (tile_id, duration)) = get_attrs!( + let (tile_id, duration) = get_attrs!( attrs, - optionals: [], required: [ ("tileid", tile_id, |v:String| v.parse().ok()), ("duration", duration, |v:String| v.parse().ok()), diff --git a/src/image.rs b/src/image.rs index c22b57a..9220a93 100644 --- a/src/image.rs +++ b/src/image.rs @@ -43,7 +43,7 @@ impl Image { TiledError::MalformedAttributes("Image must have a source, width and height with correct types".to_string()) ); - parse_tag!(parser, "image", { "" => |_| Ok(()) }); + parse_tag!(parser, "image", { }); Ok(Image { source: path_relative_to.as_ref().join(s), width: w, diff --git a/src/layers/mod.rs b/src/layers/mod.rs index 56db87e..5ce61b2 100644 --- a/src/layers/mod.rs +++ b/src/layers/mod.rs @@ -53,10 +53,7 @@ impl LayerData { map_path: &Path, tilesets: &[MapTilesetGid], ) -> Result<Self, TiledError> { - let ( - (opacity, tint_color, visible, offset_x, offset_y, parallax_x, parallax_y, name, id), - (), - ) = get_attrs!( + let (opacity, tint_color, visible, offset_x, offset_y, parallax_x, parallax_y, name, id) = get_attrs!( attrs, optionals: [ ("opacity", opacity, |v:String| v.parse().ok()), @@ -68,11 +65,7 @@ impl LayerData { ("parallaxy", parallax_y, |v:String| v.parse().ok()), ("name", name, |v| Some(v)), ("id", id, |v:String| v.parse().ok()), - ], - required: [ - ], - - TiledError::MalformedAttributes("layer parsing error, no id attribute found".to_string()) + ] ); let (ty, properties) = match tag { diff --git a/src/layers/object.rs b/src/layers/object.rs index 500c7f6..5939bfa 100644 --- a/src/layers/object.rs +++ b/src/layers/object.rs @@ -25,14 +25,11 @@ impl ObjectLayerData { attrs: Vec<OwnedAttribute>, tilesets: Option<&[MapTilesetGid]>, ) -> Result<(ObjectLayerData, Properties), TiledError> { - let (c, ()) = get_attrs!( + let c = get_attrs!( attrs, optionals: [ ("color", colour, |v:String| v.parse().ok()), - ], - required: [], - // this error should never happen since there are no required attrs - TiledError::MalformedAttributes("object group parsing error".to_string()) + ] ); let mut objects = Vec::new(); let mut properties = HashMap::new(); diff --git a/src/layers/tile/finite.rs b/src/layers/tile/finite.rs index e76eabe..ca6f768 100644 --- a/src/layers/tile/finite.rs +++ b/src/layers/tile/finite.rs @@ -32,14 +32,12 @@ impl FiniteTileLayerData { height: u32, tilesets: &[MapTilesetGid], ) -> Result<Self, TiledError> { - let ((e, c), ()) = get_attrs!( + let (e, c) = get_attrs!( attrs, optionals: [ ("encoding", encoding, |v| Some(v)), ("compression", compression, |v| Some(v)), - ], - required: [], - TiledError::MalformedAttributes("data must have an encoding and a compression".to_string()) + ] ); let tiles = parse_data_line(e, c, parser, tilesets)?; diff --git a/src/layers/tile/infinite.rs b/src/layers/tile/infinite.rs index f7d70fc..39f5715 100644 --- a/src/layers/tile/infinite.rs +++ b/src/layers/tile/infinite.rs @@ -26,14 +26,12 @@ impl InfiniteTileLayerData { attrs: Vec<OwnedAttribute>, tilesets: &[MapTilesetGid], ) -> Result<Self, TiledError> { - let ((e, c), ()) = get_attrs!( + let (e, c) = get_attrs!( attrs, optionals: [ ("encoding", encoding, |v| Some(v)), ("compression", compression, |v| Some(v)), - ], - required: [], - TiledError::MalformedAttributes("data must have an encoding and a compression".to_string()) + ] ); let mut chunks = HashMap::<(i32, i32), Chunk>::new(); @@ -126,9 +124,8 @@ impl InternalChunk { compression: Option<String>, tilesets: &[MapTilesetGid], ) -> Result<Self, TiledError> { - let ((), (x, y, width, height)) = get_attrs!( + let (x, y, width, height) = get_attrs!( attrs, - optionals: [], required: [ ("x", x, |v: String| v.parse().ok()), ("y", y, |v: String| v.parse().ok()), diff --git a/src/layers/tile/mod.rs b/src/layers/tile/mod.rs index 0257752..c9746d7 100644 --- a/src/layers/tile/mod.rs +++ b/src/layers/tile/mod.rs @@ -75,10 +75,8 @@ impl TileLayerData { infinite: bool, tilesets: &[MapTilesetGid], ) -> Result<(Self, Properties), TiledError> { - let ((), (width, height)) = get_attrs!( + let (width, height) = get_attrs!( attrs, - optionals: [ - ], required: [ ("width", width, |v: String| v.parse().ok()), ("height", height, |v: String| v.parse().ok()), diff --git a/src/map.rs b/src/map.rs index 96dc9f0..17504b9 100644 --- a/src/map.rs +++ b/src/map.rs @@ -170,7 +170,7 @@ impl Map { ("tilewidth", tile_width, |v:String| v.parse().ok()), ("tileheight", tile_height, |v:String| v.parse().ok()), ], - TiledError::MalformedAttributes("map must have a version, width and height with correct types".to_string()) + TiledError::MalformedAttributes("map must have version, width, height, tilewidth, tileheight and orientation with correct types".to_string()) ); let infinite = infinite.unwrap_or(false); diff --git a/src/objects.rs b/src/objects.rs index 55c5a70..f59df80 100644 --- a/src/objects.rs +++ b/src/objects.rs @@ -145,9 +145,8 @@ impl ObjectData { impl ObjectData { fn new_polyline(attrs: Vec<OwnedAttribute>) -> Result<ObjectShape, TiledError> { - let ((), s) = get_attrs!( + let s = get_attrs!( attrs, - optionals: [], required: [ ("points", points, |v| Some(v)), ], @@ -158,9 +157,8 @@ impl ObjectData { } fn new_polygon(attrs: Vec<OwnedAttribute>) -> Result<ObjectShape, TiledError> { - let ((), s) = get_attrs!( + let s = get_attrs!( attrs, - optionals: [], required: [ ("points", points, |v| Some(v)), ], diff --git a/src/tileset.rs b/src/tileset.rs index 3c6a3ed..146844a 100644 --- a/src/tileset.rs +++ b/src/tileset.rs @@ -197,9 +197,8 @@ impl Tileset { attrs: &Vec<OwnedAttribute>, map_path: &Path, ) -> Result<EmbeddedParseResult, TiledError> { - let ((), (first_gid, source)) = get_attrs!( + let (first_gid, source) = get_attrs!( attrs, - optionals: [], required: [ ("firstgid", first_gid, |v:String| v.parse().ok().map(|n| Gid(n))), ("source", name, |v| Some(v)), diff --git a/src/util.rs b/src/util.rs index 8ea2803..aca93cd 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,26 +1,51 @@ /// Loops through the attributes once and pulls out the ones we ask it to. It /// will check that the required ones are there. This could have been done with /// attrs.find but that would be inefficient. -/// -/// This is probably a really terrible way to do this. It does cut down on lines -/// though which is nice. macro_rules! get_attrs { - ($attrs:expr, optionals: [$(($oName:pat, $oVar:ident, $oMethod:expr)),* $(,)*], - required: [$(($name:pat, $var:ident, $method:expr)),* $(,)*], $err:expr) => { + ($attrs:expr, optionals: [$(($oName:pat, $oVar:ident, $oMethod:expr)),+ $(,)*] + , required: [$(($name:pat, $var:ident, $method:expr)),+ $(,)*], $err:expr) => { { $(let mut $oVar = None;)* $(let mut $var = None;)* - for attr in $attrs.iter() { - match attr.name.local_name.as_ref() { - $($oName => $oVar = $oMethod(attr.value.clone()),)* - $($name => $var = $method(attr.value.clone()),)* - _ => {} - } + $crate::util::match_attrs!($attrs, match: [$(($oName, $oVar, $oMethod)),+, $(($name, $var, $method)),+]); + + if !(true $(&& $var.is_some())*) { + return Err($err); } + ( + ($($oVar),*), + ($($var.unwrap()),*) + ) + } + }; + ($attrs:expr, optionals: [$(($oName:pat, $oVar:ident, $oMethod:expr)),+ $(,)*]) => { + { + $(let mut $oVar = None;)+ + $crate::util::match_attrs!($attrs, match: [$(($oName, $oVar, $oMethod)),+]); + ($($oVar),*) + } + }; + ($attrs:expr, required: [$(($name:pat, $var:ident, $method:expr)),+ $(,)*], $err:expr) => { + { + $(let mut $var = None;)* + $crate::util::match_attrs!($attrs, match: [$(($name, $var, $method)),+]); + if !(true $(&& $var.is_some())*) { return Err($err); } - (($($oVar),*), ($($var.unwrap()),*)) + + ($($var.unwrap()),*) + } + }; +} + +macro_rules! match_attrs { + ($attrs:expr, match: [$(($name:pat, $var:ident, $method:expr)),*]) => { + for attr in $attrs.iter() { + match <String as AsRef<str>>::as_ref(&attr.name.local_name) { + $($name => $var = $method(attr.value.clone()),)* + _ => {} + } } } } @@ -31,21 +56,20 @@ macro_rules! parse_tag { ($parser:expr, $close_tag:expr, {$($open_tag:expr => $open_method:expr),* $(,)*}) => { while let Some(next) = $parser.next() { match next.map_err(TiledError::XmlDecodingError)? { - xml::reader::XmlEvent::StartElement {name, attributes, ..} => { - if false {} - $(else if name.local_name == $open_tag { - match $open_method(attributes) { - Ok(()) => {}, - Err(e) => return Err(e) - }; - })* + #[allow(unused_variables)] + $( + xml::reader::XmlEvent::StartElement {name, attributes, ..} + if name.local_name == $open_tag => $open_method(attributes)?, + )* + + + xml::reader::XmlEvent::EndElement {name, ..} => if name.local_name == $close_tag { + break; } - xml::reader::XmlEvent::EndElement {name, ..} => { - if name.local_name == $close_tag { - break; - } + + xml::reader::XmlEvent::EndDocument => { + return Err(TiledError::PrematureEnd("Document ended before we expected.".to_string())); } - xml::reader::XmlEvent::EndDocument => return Err(TiledError::PrematureEnd("Document ended before we expected.".to_string())), _ => {} } } @@ -79,6 +103,7 @@ macro_rules! map_wrapper { pub(crate) use get_attrs; pub(crate) use map_wrapper; +pub(crate) use match_attrs; pub(crate) use parse_tag; use crate::{Gid, MapTilesetGid}; -- GitLab