Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/docs/essentials/custom-properties.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
}
]
};
}
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
@@ -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>
1 change: 1 addition & 0 deletions src/DotTiled.Tests/UnitTests/Serialization/TestData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>()],
Expand Down
52 changes: 35 additions & 17 deletions src/DotTiled/Serialization/Tmj/TmjReaderBase.Properties.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
};
}

Expand All @@ -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))
Expand Down Expand Up @@ -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");
Expand All @@ -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)
Expand Down
23 changes: 10 additions & 13 deletions src/DotTiled/Serialization/Tmx/TmxReaderBase.Properties.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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)
Expand Down