More test coverage improvements

pytiled_parser/parsers/json/tiled_map.py

@@ -75,13 +75,7 @@ def parse(file: Path) -> TiledMap:
             tileset_path = Path(parent_dir / raw_tileset["source"])
             parser = check_format(tileset_path)
             with open(tileset_path) as raw_tileset_file:
-                if parser == "json":
-                    tilesets[raw_tileset["firstgid"]] = parse_json_tileset(
-                        json.load(raw_tileset_file),
-                        raw_tileset["firstgid"],
-                        external_path=tileset_path.parent,
-                    )
-                elif parser == "tmx":
+                if parser == "tmx":
                     raw_tileset_external = etree.parse(raw_tileset_file).getroot()
                     tilesets[raw_tileset["firstgid"]] = parse_tmx_tileset(
                         raw_tileset_external,
@@ -89,10 +83,17 @@ def parse(file: Path) -> TiledMap:
                         external_path=tileset_path.parent,
                     )
                 else:
-                    raise UnknownFormat(
-                        "Unkown Tileset format, please use either the TSX or JSON format."
-                    )
-
+                    try:
+                        tilesets[raw_tileset["firstgid"]] = parse_json_tileset(
+                            json.load(raw_tileset_file),
+                            raw_tileset["firstgid"],
+                            external_path=tileset_path.parent,
+                        )
+                    except ValueError:
+                        raise UnknownFormat(
+                            "Unknown Tileset Format, please use either the TSX or JSON format. "
+                            "This message could also mean your tileset file is invalid or corrupted."
+                        )
         else:
             # Is an embedded Tileset
             raw_tileset = cast(RawTileSet, raw_tileset)
@@ -100,7 +101,7 @@ def parse(file: Path) -> TiledMap:
                 raw_tileset, raw_tileset["firstgid"]
             )
 
-    if isinstance(raw_tiled_map["version"], float):
+    if isinstance(raw_tiled_map["version"], float): # pragma: no cover
         version = str(raw_tiled_map["version"])
     else:
         version = raw_tiled_map["version"]

pytiled_parser/parsers/json/tileset.py

@@ -178,10 +178,14 @@ def _parse_tile(raw_tile: RawTile, external_path: Optional[Path] = None) -> Tile
         else:
             tile.image = Path(raw_tile["image"])
 
-    if raw_tile.get("imagewidth") is not None:
+    # These are ignored from coverage because there does not exist a scenario where
+    # image is set, but these aren't, so the branches will never fully be hit.
+    # However, leaving these checks in place is nice to prevent fatal errors on 
+    # a manually edited map that has an "incorrect" but not "unusable" structure
+    if raw_tile.get("imagewidth") is not None: # pragma: no cover
         tile.image_width = raw_tile["imagewidth"]
 
-    if raw_tile.get("imageheight") is not None:
+    if raw_tile.get("imageheight") is not None: # pragma: no cover
         tile.image_height = raw_tile["imageheight"]
 
     if raw_tile.get("type") is not None:
@@ -218,7 +222,11 @@ def parse(
     )
 
     if raw_tileset.get("version") is not None:
-        if isinstance(raw_tileset["version"], float):
+        # This is here to support old versions of Tiled Maps. It's a pain
+        # to keep old versions in the test data and not update them with the
+        # rest so I'm excluding this from coverage. In reality it's probably
+        # not needed. Tiled hasn't been using floats for the version for a long time
+        if isinstance(raw_tileset["version"], float): # pragma: no cover
             tileset.version = str(raw_tileset["version"])
         else:
             tileset.version = raw_tileset["version"]
@@ -234,10 +242,12 @@ def parse(
         else:
             tileset.image = Path(raw_tileset["image"])
 
-    if raw_tileset.get("imagewidth") is not None:
+    # See above note about imagewidth and imageheight on parse_tile function
+    # for an explanation on why these are ignored
+    if raw_tileset.get("imagewidth") is not None: # pragma: no cover
         tileset.image_width = raw_tileset["imagewidth"]
 
-    if raw_tileset.get("imageheight") is not None:
+    if raw_tileset.get("imageheight") is not None: # pragma: no cover
         tileset.image_height = raw_tileset["imageheight"]
 
     if raw_tileset.get("objectalignment") is not None:

tests/test_data/map_tests/cross_format_tileset/expected.py

@@ -0,0 +1,45 @@
+from pathlib import Path
+
+from pytiled_parser import common_types, tiled_map, tileset
+
+EXPECTED = tiled_map.TiledMap(
+    infinite=False,
+    layers=[],
+    map_size=common_types.Size(8, 6),
+    next_layer_id=2,
+    next_object_id=1,
+    orientation="orthogonal",
+    render_order="right-down",
+    tiled_version="1.6.0",
+    tile_size=common_types.Size(32, 32),
+    version="1.6",
+    background_color=common_types.Color(255, 0, 4, 255),
+    tilesets={
+        1: tileset.Tileset(
+            columns=8,
+            image=Path(Path(__file__).parent / "../../images/tmw_desert_spacing.png")
+            .absolute()
+            .resolve(),
+            image_width=265,
+            image_height=199,
+            firstgid=1,
+            margin=1,
+            spacing=1,
+            name="tile_set_image",
+            tile_count=48,
+            tiled_version="1.6.0",
+            tile_height=32,
+            tile_width=32,
+            version="1.6",
+            type="tileset",
+        )
+    },
+    properties={
+        "bool property - true": True,
+        "color property": common_types.Color(73, 252, 255, 255),
+        "file property": Path("../../../../../../var/log/syslog"),
+        "float property": 1.23456789,
+        "int property": 13,
+        "string property": "Hello, World!!",
+    },
+)

tests/test_data/map_tests/cross_format_tileset/map.json

@@ -0,0 +1,52 @@
+{ "backgroundcolor":"#ff0004",
+ "compressionlevel":0,
+ "height":6,
+ "infinite":false,
+ "layers":[],
+ "nextlayerid":2,
+ "nextobjectid":1,
+ "orientation":"orthogonal",
+ "properties":[
+        {
+         "name":"bool property - true",
+         "type":"bool",
+         "value":true
+        }, 
+        {
+         "name":"color property",
+         "type":"color",
+         "value":"#ff49fcff"
+        }, 
+        {
+         "name":"file property",
+         "type":"file",
+         "value":"..\/..\/..\/..\/..\/..\/var\/log\/syslog"
+        }, 
+        {
+         "name":"float property",
+         "type":"float",
+         "value":1.23456789
+        }, 
+        {
+         "name":"int property",
+         "type":"int",
+         "value":13
+        }, 
+        {
+         "name":"string property",
+         "type":"string",
+         "value":"Hello, World!!"
+        }],
+ "renderorder":"right-down",
+ "tiledversion":"1.6.0",
+ "tileheight":32,
+ "tilesets":[
+        {
+         "firstgid":1,
+         "source":"tileset.tsx"
+        }],
+ "tilewidth":32,
+ "type":"map",
+ "version":"1.6",
+ "width":8
+}

tests/test_data/map_tests/cross_format_tileset/map.tmx

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<map version="1.5" tiledversion="1.7.0" orientation="orthogonal" renderorder="right-down" compressionlevel="0" width="8" height="6" tilewidth="32" tileheight="32" infinite="0" backgroundcolor="#ff0004" nextlayerid="2" nextobjectid="1">
+ <properties>
+  <property name="bool property - true" type="bool" value="true"/>
+  <property name="color property" type="color" value="#ff49fcff"/>
+  <property name="file property" type="file" value="../../../../../../var/log/syslog"/>
+  <property name="float property" type="float" value="1.23456789"/>
+  <property name="int property" type="int" value="13"/>
+  <property name="string property" value="Hello, World!!"/>
+ </properties>
+ <tileset firstgid="1" source="tileset.json"/>
+</map>

tests/test_data/map_tests/cross_format_tileset/tileset.json

@@ -0,0 +1,14 @@
+{ "columns":8,
+ "image":"..\/..\/images\/tmw_desert_spacing.png",
+ "imageheight":199,
+ "imagewidth":265,
+ "margin":1,
+ "name":"tile_set_image",
+ "spacing":1,
+ "tilecount":48,
+ "tiledversion":"1.6.0",
+ "tileheight":32,
+ "tilewidth":32,
+ "type":"tileset",
+ "version":"1.6"
+}

tests/test_data/map_tests/cross_format_tileset/tileset.tsx

@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<tileset version="1.6" tiledversion="1.6.0" name="tile_set_image" tilewidth="32" tileheight="32" spacing="1" margin="1" tilecount="48" columns="8">
+ <image source="../../images/tmw_desert_spacing.png" width="265" height="199"/>
+</tileset>

tests/test_data/map_tests/json_invalid_tileset/map.json

@@ -0,0 +1,52 @@
+{ "backgroundcolor":"#ff0004",
+ "compressionlevel":0,
+ "height":6,
+ "infinite":false,
+ "layers":[],
+ "nextlayerid":2,
+ "nextobjectid":1,
+ "orientation":"orthogonal",
+ "properties":[
+        {
+         "name":"bool property - true",
+         "type":"bool",
+         "value":true
+        }, 
+        {
+         "name":"color property",
+         "type":"color",
+         "value":"#ff49fcff"
+        }, 
+        {
+         "name":"file property",
+         "type":"file",
+         "value":"..\/..\/..\/..\/..\/..\/var\/log\/syslog"
+        }, 
+        {
+         "name":"float property",
+         "type":"float",
+         "value":1.23456789
+        }, 
+        {
+         "name":"int property",
+         "type":"int",
+         "value":13
+        }, 
+        {
+         "name":"string property",
+         "type":"string",
+         "value":"Hello, World!!"
+        }],
+ "renderorder":"right-down",
+ "tiledversion":"1.6.0",
+ "tileheight":32,
+ "tilesets":[
+        {
+         "firstgid":1,
+         "source":"tileset.garbage"
+        }],
+ "tilewidth":32,
+ "type":"map",
+ "version":"1.6",
+ "width":8
+}

tests/test_data/map_tests/json_invalid_tileset/tileset.garbage

@@ -0,0 +1 @@
+|||thisisgarbage|||||itwillbreak|||butthatswhatwewant||

tests/test_data/tilesets/image/expected.py

@@ -12,9 +12,10 @@ EXPECTED = tileset.Tileset(
     spacing=1,
     name="tile_set_image",
     tile_count=48,
-    tiled_version="1.6.0",
+    tiled_version="1.8.5",
     tile_height=32,
     tile_width=32,
-    version="1.6",
+    version="1.8",
     type="tileset",
+    alignment="topleft"
 )

tests/test_data/tilesets/image/tileset.json

@@ -4,11 +4,12 @@
  "imagewidth":265,
  "margin":1,
  "name":"tile_set_image",
+ "objectalignment":"topleft",
  "spacing":1,
  "tilecount":48,
- "tiledversion":"1.6.0",
+ "tiledversion":"1.8.5",
  "tileheight":32,
  "tilewidth":32,
  "type":"tileset",
- "version":"1.6"
+ "version":"1.8"
 }

tests/test_data/tilesets/image/tileset.tsx

@@ -1,4 +1,4 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<tileset version="1.5" tiledversion="1.7.0" name="tile_set_image" tilewidth="32" tileheight="32" spacing="1" margin="1" tilecount="48" columns="8">
+<tileset version="1.8" tiledversion="1.8.5" name="tile_set_image" tilewidth="32" tileheight="32" spacing="1" margin="1" tilecount="48" columns="8" objectalignment="topleft">
  <image source="../../images/tmw_desert_spacing.png" width="265" height="199"/>
 </tileset>

tests/test_map.py

@@ -5,7 +5,7 @@ from pathlib import Path
 
 import pytest
 
-from pytiled_parser import parse_map
+from pytiled_parser import UnknownFormat, parse_map
 from pytiled_parser.common_types import OrderedPair, Size
 
 TESTS_DIR = Path(os.path.dirname(os.path.abspath(__file__)))
@@ -19,8 +19,10 @@ ALL_MAP_TESTS = [
     MAP_TESTS / "hexagonal",
     MAP_TESTS / "embedded_tileset",
     MAP_TESTS / "template",
+    MAP_TESTS / "cross_format_tileset",
 ]
 
+JSON_INVALID_TILESET = MAP_TESTS / "json_invalid_tileset"
 
 def fix_object(my_object):
     my_object.coordinates = OrderedPair(
@@ -83,3 +85,9 @@ def test_map_integration(parser_type, map_test):
     fix_map(expected.EXPECTED)
     fix_map(casted_map)
     assert casted_map == expected.EXPECTED
+
+def test_json_invalid_tileset():
+    raw_map_path = JSON_INVALID_TILESET / "map.json"
+
+    with pytest.raises(UnknownFormat):
+        parse_map(raw_map_path)