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 @@ private static string ConvertPathToAssemblyResourcePath(string path) =>
     [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 @@ internal ClassProperty ReadClassProperty(JsonElement element)
       {
         Name = name,
         PropertyType = propertyType,
-        Value = ReadPropertiesInsideClass(valueElement, new CustomClassDefinition
-        {
-          Name = propertyType,
-          Members = []
-        })
+        Value = ReadPropertiesInsideClass(valueElement, null)
       };
     }
 
@@ -104,6 +100,31 @@ internal List<IProperty> ReadPropertiesInsideClass(
   {
     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 @@ internal List<IProperty> ReadPropertiesInsideClass(
     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 @@ internal EnumProperty ReadEnumProperty(JsonElement element)
 
     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");
-        var values = value.Split(',').Select(v => v.Trim()).ToHashSet();
-        return new EnumProperty { Name = name, PropertyType = propertyType, Value = values };
-      }
-      else
-      {
-        var value = element.GetRequiredProperty<int>("value");
-        var values = new HashSet<string> { value.ToString(CultureInfo.InvariantCulture) };
-        return new EnumProperty { Name = name, PropertyType = propertyType, Value = values };
-      }
+        PropertyType.String => new StringProperty { Name = name, Value = element.GetRequiredProperty<string>("value") },
+        PropertyType.Int => new IntProperty { Name = name, Value = element.GetRequiredProperty<int>("value") },
+      };
+#pragma warning restore IDE0072 // Add missing cases
+#pragma warning restore CS8509 // The switch expression does not handle all possible values of its input type (it is not exhaustive).
     }
 
     if (customTypeDef.Value is not CustomEnumDefinition ced)

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

@@ -123,7 +123,7 @@ internal ClassProperty ReadClassProperty()
     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 @@ internal EnumProperty ReadEnumProperty()
       "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");
-        var values = value.Split(',').Select(v => v.Trim()).ToHashSet();
-        return new EnumProperty { Name = name, PropertyType = propertyType, Value = values };
-      }
-      else
-      {
-        var value = _reader.GetRequiredAttributeParseable<int>("value");
-        var values = new HashSet<string> { value.ToString(CultureInfo.InvariantCulture) };
-        return new EnumProperty { Name = name, PropertyType = propertyType, Value = values };
-      }
+        PropertyType.String => new StringProperty { Name = name, Value = _reader.GetRequiredAttribute("value") },
+        PropertyType.Int => new IntProperty { Name = name, Value = _reader.GetRequiredAttributeParseable<int>("value") },
+      };
+#pragma warning restore IDE0072 // Add missing cases
+#pragma warning restore CS8509 // The switch expression does not handle all possible values of its input type (it is not exhaustive).
     }
 
     if (customTypeDef.Value is not CustomEnumDefinition ced)