From c499923f276113a3f689673a2237e5cc467faba3 Mon Sep 17 00:00:00 2001
From: Alejandro Perea <alexpro820@gmail.com>
Date: Sat, 26 Mar 2022 18:18:07 +0100
Subject: [PATCH] Common layer interface (#197)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Add width & height properties to LayerData

* Expose get_tile_data methods

* Update changelog

* Apply suggestions from code review

Co-authored-by: Thorbjørn Lindeijer <bjorn@lindeijer.nl>
---
 CHANGELOG.md                |  5 +++
 src/layers/tile/finite.rs   |  9 ++++-
 src/layers/tile/infinite.rs |  9 ++++-
 src/layers/tile/mod.rs      | 73 +++++++++++++++++++++++++++++++++++++
 4 files changed, 92 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c38e7d5..746f6d6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.10.2]
+### Added
+- `TileLayer::width` & `TileLayer::height` for ergonomic access of width/height.
+- `FiniteTileLayerData::get_tile_data`, `InfiniteTileLayerData::get_tile_data`.
+
 ## [0.10.1]
 ### Added
 - `Loader` type for loading map and tileset files without having to necessarily mention the cache
diff --git a/src/layers/tile/finite.rs b/src/layers/tile/finite.rs
index 771ee29..87e6773 100644
--- a/src/layers/tile/finite.rs
+++ b/src/layers/tile/finite.rs
@@ -62,7 +62,12 @@ impl FiniteTileLayerData {
         })
     }
 
-    pub(crate) fn get_tile(&self, x: i32, y: i32) -> Option<&LayerTileData> {
+    /// Obtains the tile data present at the position given.
+    ///
+    /// If the position given is invalid or the position is empty, this function will return [`None`].
+    ///
+    /// If you want to get a [`Tile`](`crate::Tile`) instead, use [`FiniteTileLayer::get_tile()`].
+    pub fn get_tile_data(&self, x: i32, y: i32) -> Option<&LayerTileData> {
         if x < self.width as i32 && y < self.height as i32 && x >= 0 && y >= 0 {
             self.tiles[x as usize + y as usize * self.width as usize].as_ref()
         } else {
@@ -82,7 +87,7 @@ impl<'map> FiniteTileLayer<'map> {
     /// If the position given is invalid or the position is empty, this function will return [`None`].
     pub fn get_tile(&self, x: i32, y: i32) -> Option<LayerTile> {
         self.data
-            .get_tile(x, y)
+            .get_tile_data(x, y)
             .and_then(|data| Some(LayerTile::new(self.map(), data)))
     }
 }
diff --git a/src/layers/tile/infinite.rs b/src/layers/tile/infinite.rs
index bf5bac7..c6e1886 100644
--- a/src/layers/tile/infinite.rs
+++ b/src/layers/tile/infinite.rs
@@ -57,7 +57,12 @@ impl InfiniteTileLayerData {
         Ok(Self { chunks })
     }
 
-    pub(crate) fn get_tile(&self, x: i32, y: i32) -> Option<&LayerTileData> {
+    /// Obtains the tile data present at the position given.
+    ///
+    /// If the position given is invalid or the position is empty, this function will return [`None`].
+    ///
+    /// If you want to get a [`Tile`](`crate::Tile`) instead, use [`InfiniteTileLayer::get_tile()`].
+    pub fn get_tile_data(&self, x: i32, y: i32) -> Option<&LayerTileData> {
         let chunk_pos = tile_to_chunk_pos(x, y);
         self.chunks
             .get(&chunk_pos)
@@ -159,7 +164,7 @@ impl<'map> InfiniteTileLayer<'map> {
     /// If the position is empty, this function will return [`None`].
     pub fn get_tile(&self, x: i32, y: i32) -> Option<LayerTile> {
         self.data
-            .get_tile(x, y)
+            .get_tile_data(x, y)
             .and_then(|data| Some(LayerTile::new(self.map, data)))
     }
 }
diff --git a/src/layers/tile/mod.rs b/src/layers/tile/mod.rs
index b699374..ff32120 100644
--- a/src/layers/tile/mod.rs
+++ b/src/layers/tile/mod.rs
@@ -82,6 +82,11 @@ impl LayerTileData {
     }
 }
 
+/// The raw data of a [`TileLayer`]. Does not include a reference to its parent [`Map`](crate::Map).
+///
+/// The reason this data is not public is because with the current interface there is no way to
+/// dereference [`TileLayer`] into this structure, and even if we could, it wouldn't make much
+/// sense, since we can already deref from the finite/infinite tile layers themselves.
 #[derive(Debug, PartialEq, Clone)]
 pub(crate) enum TileLayerData {
     Finite(FiniteTileLayerData),
@@ -169,4 +174,72 @@ impl<'map> TileLayer<'map> {
             TileLayer::Infinite(infinite) => infinite.get_tile(x, y),
         }
     }
+
+    /// The width of this layer, if finite, or `None` if infinite.
+    ///
+    /// ## Example
+    /// ```
+    /// use tiled::LayerType;
+    /// use tiled::Loader;
+    ///
+    /// # fn main() {
+    /// # let map = Loader::new()
+    /// #     .load_tmx_map("assets/tiled_base64_zlib.tmx")
+    /// #     .unwrap();
+    /// # let layer = match map.get_layer(0).unwrap().layer_type() {
+    /// #     LayerType::TileLayer(layer) => layer,
+    /// #     _ => panic!("Layer #0 is not a tile layer"),
+    /// # };
+    /// #
+    /// let width = match layer {
+    ///     tiled::TileLayer::Finite(finite) => Some(finite.width()),
+    ///     _ => None,
+    /// };
+    ///
+    /// // These are both equal, and they achieve the same thing; However, matching on the layer
+    /// // type is significantly more verbose. If you already know the layer type, then it is
+    /// // slighly faster to use its respective width method.
+    /// assert_eq!(width, layer.width());
+    /// # }
+    /// ```
+    pub fn width(&self) -> Option<u32> {
+        match self {
+            TileLayer::Finite(finite) => Some(finite.width()),
+            TileLayer::Infinite(_infinite) => None,
+        }
+    }
+
+    /// The height of this layer, if finite, or `None` if infinite.
+    ///
+    /// ## Example
+    /// ```
+    /// use tiled::LayerType;
+    /// use tiled::Loader;
+    ///
+    /// # fn main() {
+    /// # let map = Loader::new()
+    /// #     .load_tmx_map("assets/tiled_base64_zlib.tmx")
+    /// #     .unwrap();
+    /// # let layer = match map.get_layer(0).unwrap().layer_type() {
+    /// #     LayerType::TileLayer(layer) => layer,
+    /// #     _ => panic!("Layer #0 is not a tile layer"),
+    /// # };
+    /// #
+    /// let height = match layer {
+    ///     tiled::TileLayer::Finite(finite) => Some(finite.height()),
+    ///     _ => None,
+    /// };
+    ///
+    /// // These are both equal, and they achieve the same thing; However, matching on the layer
+    /// // type is significantly more verbose. If you already know the layer type, then it is
+    /// // slighly faster to use its respective height method.
+    /// assert_eq!(height, layer.height());
+    /// # }
+    /// ```
+    pub fn height(&self) -> Option<u32> {
+        match self {
+            TileLayer::Finite(finite) => Some(finite.height()),
+            TileLayer::Infinite(_infinite) => None,
+        }
+    }
 }
-- 
GitLab