Merge pull request #65 from dcronqvist/enum-bug

Enum properties were not being properly parsed due to incorrect usage of Optional

docs/docs/essentials/custom-properties.md

@@ -177,6 +177,9 @@ For enum definitions, the <xref:System.FlagsAttribute> can be used to indicate t
 > [!NOTE]
 > Tiled supports enums which can store their values as either strings or integers, and depending on the storage type you have specified in Tiled, you must make sure to have the same storage type in your <xref:DotTiled.CustomEnumDefinition>. This can be done by setting the `StorageType` property to either `CustomEnumStorageType.String` or `CustomEnumStorageType.Int` when creating the definition, or by passing the storage type as an argument to the <xref:DotTiled.CustomEnumDefinition.FromEnum``1(DotTiled.CustomEnumStorageType)> method. To be consistent with Tiled, <xref:DotTiled.CustomEnumDefinition.FromEnum``1(DotTiled.CustomEnumStorageType)> will default to `CustomEnumStorageType.String` for the storage type parameter.
 
+> [!WARNING]
+> If you have a custom enum type in Tiled, but do not define it in DotTiled, you must be aware that the type of the parsed property will be either <xref:DotTiled.StringProperty> or <xref:IntProperty>. It is not possible to determine the correct way to parse the enum property without the custom enum definition, which is why you will instead be given a property of type `string` or `int` when accessing the property in DotTiled. This can lead to inconsistencies between the map in Tiled and the loaded map with DotTiled. It is therefore recommended to define your custom enum types in DotTiled if you want to access the properties as <xref:EnumProperty> instances.
+
 ## Mapping properties to C# classes or enums
 
 So far, we have only discussed how to define custom property types in DotTiled, and why they are needed. However, the most important part is how you can map properties inside your maps to their corresponding C# classes or enums. 

src/DotTiled.Tests/TestData/Maps/map-with-custom-type-props-without-defs/map-with-custom-type-props-without-defs.cs

@@ -0,0 +1,85 @@
+using System.Globalization;
+
+namespace DotTiled.Tests;
+
+public partial class TestData
+{
+  public static Map MapWithCustomTypePropsWithoutDefs() => new Map
+  {
+    Class = "",
+    Orientation = MapOrientation.Orthogonal,
+    Width = 5,
+    Height = 5,
+    TileWidth = 32,
+    TileHeight = 32,
+    Infinite = false,
+    ParallaxOriginX = 0,
+    ParallaxOriginY = 0,
+    RenderOrder = RenderOrder.RightDown,
+    CompressionLevel = -1,
+    BackgroundColor = Color.Parse("#00000000", CultureInfo.InvariantCulture),
+    Version = "1.10",
+    TiledVersion = "1.11.0",
+    NextLayerID = 2,
+    NextObjectID = 1,
+    Layers = [
+      new TileLayer
+      {
+        ID = 1,
+        Name = "Tile Layer 1",
+        Width = 5,
+        Height = 5,
+        Data = new Data
+        {
+          Encoding = DataEncoding.Csv,
+          GlobalTileIDs = new Optional<uint[]>([
+            0, 0, 0, 0, 0,
+            0, 0, 0, 0, 0,
+            0, 0, 0, 0, 0,
+            0, 0, 0, 0, 0,
+            0, 0, 0, 0, 0
+          ]),
+          FlippingFlags = new Optional<FlippingFlags[]>([
+            FlippingFlags.None, FlippingFlags.None, FlippingFlags.None, FlippingFlags.None, FlippingFlags.None,
+            FlippingFlags.None, FlippingFlags.None, FlippingFlags.None, FlippingFlags.None, FlippingFlags.None,
+            FlippingFlags.None, FlippingFlags.None, FlippingFlags.None, FlippingFlags.None, FlippingFlags.None,
+            FlippingFlags.None, FlippingFlags.None, FlippingFlags.None, FlippingFlags.None, FlippingFlags.None,
+            FlippingFlags.None, FlippingFlags.None, FlippingFlags.None, FlippingFlags.None, FlippingFlags.None
+          ])
+        }
+      }
+    ],
+    Properties = [
+      new ClassProperty
+      {
+        Name = "customclassprop",
+        PropertyType = "CustomClass",
+        Value = [
+          new BoolProperty { Name = "boolinclass", Value = true },
+          new FloatProperty { Name = "floatinclass", Value = 13.37f },
+          new StringProperty { Name = "stringinclass", Value = "This is a set string" }
+        ]
+      },
+      new IntProperty
+      {
+        Name = "customenumintflagsprop",
+        Value = 6
+      },
+      new IntProperty
+      {
+        Name = "customenumintprop",
+        Value = 3
+      },
+      new StringProperty
+      {
+        Name = "customenumstringprop",
+        Value = "CustomEnumString_2"
+      },
+      new StringProperty
+      {
+        Name = "customenumstringflagsprop",
+        Value = "CustomEnumStringFlags_1,CustomEnumStringFlags_2"
+      }
+    ]
+  };
+}

src/DotTiled.Tests/TestData/Maps/map-with-custom-type-props-without-defs/map-with-custom-type-props-without-defs.tmj

@@ -0,0 +1,68 @@
+{ "compressionlevel":-1,
+ "height":5,
+ "infinite":false,
+ "layers":[
+        {
+         "data":[0, 0, 0, 0, 0,
+            0, 0, 0, 0, 0,
+            0, 0, 0, 0, 0,
+            0, 0, 0, 0, 0,
+            0, 0, 0, 0, 0],
+         "height":5,
+         "id":1,
+         "name":"Tile Layer 1",
+         "opacity":1,
+         "type":"tilelayer",
+         "visible":true,
+         "width":5,
+         "x":0,
+         "y":0
+        }],
+ "nextlayerid":2,
+ "nextobjectid":1,
+ "orientation":"orthogonal",
+ "properties":[
+        {
+         "name":"customclassprop",
+         "propertytype":"CustomClass",
+         "type":"class",
+         "value":
+            {
+             "boolinclass":true,
+             "floatinclass":13.37,
+             "stringinclass":"This is a set string"
+            }
+        }, 
+        {
+         "name":"customenumintflagsprop",
+         "propertytype":"CustomEnumIntFlags",
+         "type":"int",
+         "value":6
+        }, 
+        {
+         "name":"customenumintprop",
+         "propertytype":"CustomEnumInt",
+         "type":"int",
+         "value":3
+        }, 
+        {
+         "name":"customenumstringflagsprop",
+         "propertytype":"CustomEnumStringFlags",
+         "type":"string",
+         "value":"CustomEnumStringFlags_1,CustomEnumStringFlags_2"
+        }, 
+        {
+         "name":"customenumstringprop",
+         "propertytype":"CustomEnumString",
+         "type":"string",
+         "value":"CustomEnumString_2"
+        }],
+ "renderorder":"right-down",
+ "tiledversion":"1.11.0",
+ "tileheight":32,
+ "tilesets":[],
+ "tilewidth":32,
+ "type":"map",
+ "version":"1.10",
+ "width":5
+}

src/DotTiled.Tests/TestData/Maps/map-with-custom-type-props-without-defs/map-with-custom-type-props-without-defs.tmx

@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<map version="1.10" tiledversion="1.11.0" orientation="orthogonal" renderorder="right-down" width="5" height="5" tilewidth="32" tileheight="32" infinite="0" nextlayerid="2" nextobjectid="1">
+ <properties>
+  <property name="customclassprop" type="class" propertytype="CustomClass">
+   <properties>
+    <property name="boolinclass" type="bool" value="true"/>
+    <property name="floatinclass" type="float" value="13.37"/>
+    <property name="stringinclass" value="This is a set string"/>
+   </properties>
+  </property>
+  <property name="customenumintflagsprop" type="int" propertytype="CustomEnumIntFlags" value="6"/>
+  <property name="customenumintprop" type="int" propertytype="CustomEnumInt" value="3"/>
+  <property name="customenumstringflagsprop" propertytype="CustomEnumStringFlags" value="CustomEnumStringFlags_1,CustomEnumStringFlags_2"/>
+  <property name="customenumstringprop" propertytype="CustomEnumString" value="CustomEnumString_2"/>
+ </properties>
+ <layer id="1" name="Tile Layer 1" width="5" height="5">
+  <data encoding="csv">
+0,0,0,0,0,
+0,0,0,0,0,
+0,0,0,0,0,
+0,0,0,0,0,
+0,0,0,0,0
+</data>
+ </layer>
+</map>

src/DotTiled.Tests/UnitTests/Serialization/TestData.cs

@@ -36,6 +36,7 @@ public static partial class TestData
     [GetMapPath("default-map"), (string f) => DefaultMap(), Array.Empty<ICustomTypeDefinition>()],
     [GetMapPath("map-with-common-props"), (string f) => MapWithCommonProps(), Array.Empty<ICustomTypeDefinition>()],
     [GetMapPath("map-with-custom-type-props"), (string f) => MapWithCustomTypeProps(), MapWithCustomTypePropsCustomTypeDefinitions()],
+    [GetMapPath("map-with-custom-type-props-without-defs"), (string f) => MapWithCustomTypePropsWithoutDefs(), Array.Empty<ICustomTypeDefinition>()],
     [GetMapPath("map-with-embedded-tileset"), (string f) => MapWithEmbeddedTileset(), Array.Empty<ICustomTypeDefinition>()],
     [GetMapPath("map-with-external-tileset"), (string f) => MapWithExternalTileset(f), Array.Empty<ICustomTypeDefinition>()],
     [GetMapPath("map-with-flippingflags"), (string f) => MapWithFlippingFlags(f), Array.Empty<ICustomTypeDefinition>()],

src/DotTiled/Serialization/Tmj/TmjReaderBase.Properties.cs

@@ -75,11 +75,7 @@ public abstract partial class TmjReaderBase
       {
         Name = name,
         PropertyType = propertyType,
-        Value = ReadPropertiesInsideClass(valueElement, new CustomClassDefinition
+        Value = ReadPropertiesInsideClass(valueElement, null)
-        {
-          Name = propertyType,
-          Members = []
-        })
       };
     }
 
@@ -104,6 +100,31 @@ public abstract partial class TmjReaderBase
   {
     List<IProperty> resultingProps = [];
 
+    if (customClassDefinition is null)
+    {
+      foreach (var prop in element.EnumerateObject())
+      {
+        var name = prop.Name;
+        var value = prop.Value;
+
+#pragma warning disable IDE0072 // Add missing cases
+        IProperty property = value.ValueKind switch
+        {
+          JsonValueKind.String => new StringProperty { Name = name, Value = value.GetString() },
+          JsonValueKind.Number => value.TryGetInt32(out var intValue) ? new IntProperty { Name = name, Value = intValue } : new FloatProperty { Name = name, Value = value.GetSingle() },
+          JsonValueKind.True => new BoolProperty { Name = name, Value = true },
+          JsonValueKind.False => new BoolProperty { Name = name, Value = false },
+          JsonValueKind.Object => new ClassProperty { Name = name, PropertyType = "", Value = ReadPropertiesInsideClass(value, null) },
+          _ => throw new JsonException("Invalid property type")
+        };
+#pragma warning restore IDE0072 // Add missing cases
+
+        resultingProps.Add(property);
+      }
+
+      return resultingProps;
+    }
+
     foreach (var prop in customClassDefinition.Members)
     {
       if (!element.TryGetProperty(prop.Name, out var propElement))
@@ -156,7 +177,7 @@ public abstract partial class TmjReaderBase
     return resultingProps;
   }
 
-  internal EnumProperty ReadEnumProperty(JsonElement element)
+  internal IProperty ReadEnumProperty(JsonElement element)
   {
     var name = element.GetRequiredProperty<string>("name");
     var propertyType = element.GetRequiredProperty<string>("propertytype");
@@ -170,18 +191,15 @@ public abstract partial class TmjReaderBase
 
     if (!customTypeDef.HasValue)
     {
-      if (typeInJson == PropertyType.String)
+#pragma warning disable CS8509 // The switch expression does not handle all possible values of its input type (it is not exhaustive).
+#pragma warning disable IDE0072 // Add missing cases
+      return typeInJson switch
       {
-        var value = element.GetRequiredProperty<string>("value");
+        PropertyType.String => new StringProperty { Name = name, Value = element.GetRequiredProperty<string>("value") },
-        var values = value.Split(',').Select(v => v.Trim()).ToHashSet();
+        PropertyType.Int => new IntProperty { Name = name, Value = element.GetRequiredProperty<int>("value") },
-        return new EnumProperty { Name = name, PropertyType = propertyType, Value = values };
+      };
-      }
+#pragma warning restore IDE0072 // Add missing cases
-      else
+#pragma warning restore CS8509 // The switch expression does not handle all possible values of its input type (it is not exhaustive).
-      {
-        var value = element.GetRequiredProperty<int>("value");
-        var values = new HashSet<string> { value.ToString(CultureInfo.InvariantCulture) };
-        return new EnumProperty { Name = name, PropertyType = propertyType, Value = values };
-      }
     }
 
     if (customTypeDef.Value is not CustomEnumDefinition ced)

src/DotTiled/Serialization/Tmx/TmxReaderBase.Properties.cs

@@ -123,7 +123,7 @@ public abstract partial class TmxReaderBase
     return new ClassProperty { Name = name, PropertyType = propertyType, Value = propsInType };
   }
 
-  internal EnumProperty ReadEnumProperty()
+  internal IProperty ReadEnumProperty()
   {
     var name = _reader.GetRequiredAttribute("name");
     var propertyType = _reader.GetRequiredAttribute("propertytype");
@@ -132,25 +132,22 @@ public abstract partial class TmxReaderBase
       "string" => PropertyType.String,
       "int" => PropertyType.Int,
       _ => throw new XmlException("Invalid property type")
-    }) ?? PropertyType.String;
+    }).GetValueOr(PropertyType.String);
     var customTypeDef = _customTypeResolver(propertyType);
 
     // If the custom enum definition is not found,
     // we assume an empty enum definition.
     if (!customTypeDef.HasValue)
     {
-      if (typeInXml == PropertyType.String)
+#pragma warning disable CS8509 // The switch expression does not handle all possible values of its input type (it is not exhaustive).
+#pragma warning disable IDE0072 // Add missing cases
+      return typeInXml switch
       {
-        var value = _reader.GetRequiredAttribute("value");
+        PropertyType.String => new StringProperty { Name = name, Value = _reader.GetRequiredAttribute("value") },
-        var values = value.Split(',').Select(v => v.Trim()).ToHashSet();
+        PropertyType.Int => new IntProperty { Name = name, Value = _reader.GetRequiredAttributeParseable<int>("value") },
-        return new EnumProperty { Name = name, PropertyType = propertyType, Value = values };
+      };
-      }
+#pragma warning restore IDE0072 // Add missing cases
-      else
+#pragma warning restore CS8509 // The switch expression does not handle all possible values of its input type (it is not exhaustive).
-      {
-        var value = _reader.GetRequiredAttributeParseable<int>("value");
-        var values = new HashSet<string> { value.ToString(CultureInfo.InvariantCulture) };
-        return new EnumProperty { Name = name, PropertyType = propertyType, Value = values };
-      }
     }
 
     if (customTypeDef.Value is not CustomEnumDefinition ced)