Merge remote-tracking branch 'upstream/master' into guidelines

assets/folder/tiled_relative_paths.tmx

@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<map version="1.8" tiledversion="1.8.2" orientation="orthogonal" renderorder="right-down" width="16" height="16" tilewidth="32" tileheight="32" infinite="0" nextlayerid="3" nextobjectid="1">
+ <tileset firstgid="1" source="../tilesheet.tsx"/>
+ <layer id="1" name="Tile Layer 1" width="16" height="16">
+  <data encoding="csv">
+21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,
+21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,
+21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,
+21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,
+21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,
+21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,
+21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,
+21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,
+21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,
+21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,
+21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,
+21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,
+21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,
+21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,
+21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,
+21,21,21,21,21,21,21,21,21,21,21,21,21,21,21,21
+</data>
+ </layer>
+ <imagelayer id="2" name="image">
+  <image source="../tilesheet.png" width="448" height="192"/>
+ </imagelayer>
+</map>

src/animation.rs

@@ -21,9 +21,8 @@ pub struct Frame {
 
 impl Frame {
     pub(crate) fn new(attrs: Vec<OwnedAttribute>) -> Result<Frame> {
-        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()),

src/image.rs

@@ -2,19 +2,72 @@ use std::path::{Path, PathBuf};
 
 use xml::attribute::OwnedAttribute;
 
-use crate::{error::{Error, Result}, properties::Color, util::*};
+use crate::{
+    error::{Error, Result},
+    properties::Color,
+    util::*,
+};
 
 /// A reference to an image stored somewhere within the filesystem.
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub struct Image {
-    /// The filepath of the image.
+    /// The **uncanonicalized** filepath of the image, starting from the path given to load the file
+    /// this image is in. See the example for more details.
     ///
     /// ## Note
     /// The crate does not currently support embedded images (Even though Tiled
     /// does not allow creating maps with embedded image data, the TMX format does; [source])
     ///
+    /// Currently, the crate is not prepared to handle anything but OS paths. Using VFS is a hard
+    /// task that involves a lot of messy path manipulation. [Tracking issue]
+    ///
     /// [source]: https://doc.mapeditor.org/en/stable/reference/tmx-map-format/#image
+    /// [Tracking issue]: https://github.com/mapeditor/rs-tiled/issues/37
+    ///
+    /// ## Example
+    /// ```
+    /// use std::path::Path;
+    /// use std::fs::File;
+    /// use tiled::*;
+    ///
+    /// # fn main() -> std::result::Result<(), Box<dyn std::error::Error>> {
+    /// let map = Map::parse_file(
+    ///     "assets/folder/tiled_relative_paths.tmx",
+    ///     &mut FilesystemResourceCache::new(),
+    /// )?;
+    ///
+    /// let image_layer = match map
+    ///     .layers()
+    ///     .find(|layer| layer.name() == "image")
+    ///     .unwrap()
+    ///     .layer_type()
+    /// {
+    ///     LayerType::ImageLayer(layer) => layer,
+    ///     _ => panic!(),
+    /// };
+    ///
+    /// // Image layer has an image with the source attribute set to "../tilesheet.png"
+    /// // Given the information we gave to the `parse_file` function, the image source should be
+    /// // "assets/folder/../tilesheet.png". The filepath is not canonicalized.
+    /// let image_source = &image_layer.image().unwrap().source;
+    ///
+    /// assert_eq!(
+    ///     image_source,
+    ///     Path::new("assets/folder/../tilesheet.png")
+    /// );
+    ///
+    /// // If you are using the OS's filesystem, figuring out the real path of the image is as easy
+    /// // as canonicalizing the path. If you are using some sort of VFS, this task is much harder
+    /// // since std::path is meant to be used with the OS. This will be fixed in the future!
+    /// let image_source = image_source.canonicalize()?;
+    /// assert!(File::open(image_source).is_ok());
+    /// # Ok(())
+    /// # }
+    /// ```
+    /// Check the assets/tiled_relative_paths.tmx file at the crate root to see the structure of the
+    /// file this example is referring to.
     // TODO: Embedded images
+    // TODO: Figure out how to serve crate users paths in a better way
     pub source: PathBuf,
     /// The width in pixels of the image.
     pub width: i32,
@@ -43,7 +96,7 @@ impl Image {
             Error::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,

src/layers/mod.rs

@@ -58,10 +58,7 @@ impl LayerData {
         map_path: &Path,
         tilesets: &[MapTilesetGid],
     ) -> Result<Self> {
-        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()),
@@ -73,11 +70,7 @@ impl LayerData {
                 ("parallaxy", parallax_y, |v:String| v.parse().ok()),
                 ("name", name, |v| Some(v)),
                 ("id", id, |v:String| v.parse().ok()),
-            ],
-            required: [
-            ],
-
-            Error::MalformedAttributes("layer parsing error, no id attribute found".to_string())
+            ]
         );
 
         let (ty, properties) = match tag {

src/layers/object.rs

@@ -25,14 +25,11 @@ impl ObjectLayerData {
         attrs: Vec<OwnedAttribute>,
         tilesets: Option<&[MapTilesetGid]>,
     ) -> Result<(ObjectLayerData, Properties), Error> {
-        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
-            Error::MalformedAttributes("object group parsing error".to_string())
+            ]
         );
         let mut objects = Vec::new();
         let mut properties = HashMap::new();

src/layers/tile/finite.rs

@@ -2,7 +2,7 @@ use xml::attribute::OwnedAttribute;
 
 use crate::{
     util::{get_attrs, map_wrapper, XmlEventResult},
-    Error, LayerTile, LayerTileData, MapTilesetGid, Result,
+    LayerTile, LayerTileData, MapTilesetGid, Result,
 };
 
 use super::util::parse_data_line;
@@ -32,14 +32,12 @@ impl FiniteTileLayerData {
         height: u32,
         tilesets: &[MapTilesetGid],
     ) -> Result<Self> {
-        let ((e, c), ()) = get_attrs!(
+        let (e, c) = get_attrs!(
             attrs,
             optionals: [
                 ("encoding", encoding, |v| Some(v)),
                 ("compression", compression, |v| Some(v)),
-            ],
-            required: [],
-            Error::MalformedAttributes("data must have an encoding and a compression".to_string())
+            ]
         );
 
         let tiles = parse_data_line(e, c, parser, tilesets)?;

src/layers/tile/infinite.rs

@@ -26,14 +26,12 @@ impl InfiniteTileLayerData {
         attrs: Vec<OwnedAttribute>,
         tilesets: &[MapTilesetGid],
     ) -> Result<Self> {
-        let ((e, c), ()) = get_attrs!(
+        let (e, c) = get_attrs!(
             attrs,
             optionals: [
                 ("encoding", encoding, |v| Some(v)),
                 ("compression", compression, |v| Some(v)),
-            ],
-            required: [],
-            Error::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> {
-        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()),

src/layers/tile/mod.rs

@@ -5,7 +5,7 @@ use xml::attribute::OwnedAttribute;
 use crate::{
     parse_properties,
     util::{get_attrs, map_wrapper, parse_tag, XmlEventResult},
-    Error, Gid, Map, MapTilesetGid, Properties, Tile, TileId, Tileset,
+    Error, Gid, Map, MapTilesetGid, Properties, Result, Tile, TileId, Tileset,
 };
 
 mod finite;
@@ -74,11 +74,9 @@ impl TileLayerData {
         attrs: Vec<OwnedAttribute>,
         infinite: bool,
         tilesets: &[MapTilesetGid],
-    ) -> Result<(Self, Properties), Error> {
-        let ((), (width, height)) = get_attrs!(
+    ) -> Result<(Self, Properties)> {
+        let (width, height) = get_attrs!(
             attrs,
-            optionals: [
-            ],
             required: [
                 ("width", width, |v: String| v.parse().ok()),
                 ("height", height, |v: String| v.parse().ok()),

src/map.rs

@@ -167,7 +167,7 @@ impl Map {
                 ("tilewidth", tile_width, |v:String| v.parse().ok()),
                 ("tileheight", tile_height, |v:String| v.parse().ok()),
             ],
-            Error::MalformedAttributes("map must have a version, width and height with correct types".to_string())
+            Error::MalformedAttributes("map must have version, width, height, tilewidth, tileheight and orientation with correct types".to_string())
         );
 
         let infinite = infinite.unwrap_or(false);

src/objects.rs

@@ -145,9 +145,8 @@ impl ObjectData {
 
 impl ObjectData {
     fn new_polyline(attrs: Vec<OwnedAttribute>) -> Result<ObjectShape> {
-        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> {
-        let ((), s) = get_attrs!(
+        let s = get_attrs!(
             attrs,
-            optionals: [],
             required: [
                 ("points", points, |v| Some(v)),
             ],

src/tileset.rs

@@ -191,9 +191,8 @@ impl Tileset {
         attrs: &Vec<OwnedAttribute>,
         map_path: &Path,
     ) -> Result<EmbeddedParseResult> {
-        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)),

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(Error::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(Error::PrematureEnd("Document ended before we expected.".to_string()));
                 }
-                xml::reader::XmlEvent::EndDocument => return Err(Error::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};