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

feat!: turning List<Property> into CustomProperties #55

Merged
merged 9 commits into from
Oct 31, 2022
203 changes: 193 additions & 10 deletions packages/tiled/lib/src/common/property.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,35 +12,218 @@ part of tiled;
/// (default string is “”, default number is 0, default boolean is “false”,
/// default color is #00000000, default file is “.” (the current file’s
/// parent directory))
class Property {
class Property<T> {
String name;
PropertyType type;
// TODO(luan): support other property types
String value;
T value;

Property({
required this.name,
required this.type,
required this.value,
});

Property.parse(Parser parser)
: this(
name: parser.getString('name'),
type: parser.getPropertyType('type', defaults: PropertyType.string),
value: parser.getString('value'),
static Property<Object> parse(Parser parser) {
final name = parser.getString('name');
final type = parser.getPropertyType('type', defaults: PropertyType.string);

switch (type) {
case PropertyType.object:
return ObjectProperty(
name: name,
value: parser.getInt('value', defaults: 0),
);

case PropertyType.color:
return ColorProperty(
name: name,
value: parser.getColor('value', defaults: const Color(0x00000000)),
hexValue: parser.getString('value', defaults: '#00000000'),
);

case PropertyType.bool:
return BoolProperty(
name: name,
value: parser.getBool('value', defaults: false),
);

case PropertyType.float:
return FloatProperty(
name: name,
value: parser.getDouble('value', defaults: 0),
);

case PropertyType.int:
return IntProperty(
name: name,
value: parser.getInt('value', defaults: 0),
);

case PropertyType.file:
return FileProperty(
name: name,
value: parser.getString('value', defaults: '.'),
);

case PropertyType.string:
final value = parser.formatSpecificParsing((json) {
return json.getString('value', defaults: '');
}, (xml) {
final attrString = parser.getStringOrNull('value');
if (attrString != null) {
return attrString;
} else {
// In tmx files, multi-line text property values can be stored
// inside the <property> node itself instead of in the 'value'
// attribute
return xml.element.innerText;
}
});

return StringProperty(
name: name,
value: value,
);
}
}
}

/// A wrapper for a Tiled property set
///
/// Accessing an int value
/// ```dart
/// properties.get<int>('foo') == 3
/// ```
///
/// Accessing an int property:
/// ```dart
/// properties.getProperty<IntProperty>('foo') ==
/// IntProperty(name: 'foo', value: 3);
/// ```
///
/// You can also use an accessor:
/// ```dart
/// properties['foo'] == IntProperty(name: 'foo', value: 3);
/// ```
class CustomProperties extends Iterable<Property<Object>> {
static const empty = CustomProperties({});

/// The properties, indexed by name
final Map<String, Property<Object>> byName;

const CustomProperties(this.byName);

/// Get a property value by its name.
///
/// [T] must be the type of the value, which depends on the property type.
/// The following Tiled properties map to the follow Dart types:
/// - int property -> int
/// - float property -> double
/// - boolean property -> bool
/// - string property -> string
/// - color property -> ui.Color
/// - file property -> string (path)
/// - object property -> int (ID)
T? getValue<T>(String name) {
return getProperty(name)?.value as T?;
}

/// Get a typed property by its name
T? getProperty<T extends Property<Object>>(String name) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
T? getProperty<T extends Property<Object>>(String name) {
T? getProperty<T extends Property<T>>(String name) {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

T is the type of the property here, not the value, so this change would require that the Property has a value of Property recursively. We'd have to add a second generic to the method, K, which would be awkward:
getProperty<IntProperty, int>('foo')

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, of course. Wouldn't one want to just want to specify the type of the value instead, and then the return type can be Property<T>??

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well the intent was to make it simple to get an IntProperty or ColorProperty, otherwise you still have to cast it to one of those if you want the actual property type, which would be more relevant if the property has unique methods on it. Otherwise there is really no reason to be grabbing the property itself at all, you can just grab the value directly

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, what is the real point of having the property wrapper actually? Can't we just offer extensions on the value type instead? Like toHex on Color for example?

Copy link
Collaborator Author

@kurtome kurtome Sep 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may not need it, but it's not entirely clear to me that every property can just be a simple name/value pair where the value doesn't need custom behavior, if so we should probably ditch the entire Property class

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For all the value based use cases your describing, that's what getValue<T> is for, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, are there any usecases for having this wrapper class? @ufrshubham @jtmcdole

Copy link
Collaborator Author

@kurtome kurtome Sep 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll also say that there is some value to me to making the class structure match the Tiled data structures for consistency with Tiled itself. Also, we really do want to know if a property is a file string or a text string, etc... which is an argument for keeping the Property class

return byName[name] as T?;
}

/// Get a property by its name
Property<Object>? operator [](String name) {
return byName[name];
}

/// Returns whether or not a property with [name] exists.
bool has(String name) {
return byName.containsKey(name);
}

@override
Iterator<Property<Object>> get iterator => byName.values.iterator;
}

/// [value] is the ID of the object
class ObjectProperty extends Property<int> {
spydon marked this conversation as resolved.
Show resolved Hide resolved
ObjectProperty({
required super.name,
required super.value,
}) : super(type: PropertyType.object);
}

/// [value] is the color
class ColorProperty extends Property<Color> {
final String hexValue;

ColorProperty({
required super.name,
required super.value,
required this.hexValue,
}) : super(type: PropertyType.color);
}

/// [value] is the string text
class StringProperty extends Property<String> {
StringProperty({
required super.name,
required super.value,
}) : super(type: PropertyType.string);
}

/// [value] is the path to the file
class FileProperty extends Property<String> {
FileProperty({
required super.name,
required super.value,
}) : super(type: PropertyType.file);
}

/// [value] is the integer number
class IntProperty extends Property<int> {
IntProperty({
required super.name,
required super.value,
}) : super(type: PropertyType.int);
}

/// [value] is the double-percision floating-point number
class FloatProperty extends Property<double> {
FloatProperty({
required super.name,
required super.value,
}) : super(type: PropertyType.float);
}

/// [value] is the boolean
class BoolProperty extends Property<bool> {
BoolProperty({
required super.name,
required super.value,
}) : super(type: PropertyType.bool);
}

extension PropertiesParser on Parser {
List<Property> getProperties() {
return formatSpecificParsing(
CustomProperties getProperties() {
final properties = formatSpecificParsing(
(json) => json.getChildrenAs('properties', Property.parse),
(xml) =>
xml
.getSingleChildOrNull('properties')
?.getChildrenAs('property', Property.parse) ??
[],
);

// NOTE: two properties should never have the same name, if they do
// one will simply override the other
final byName =
properties.groupFoldBy((prop) => prop.name, (previous, element) {
return element;
});

return CustomProperties(byName);
}
}
4 changes: 2 additions & 2 deletions packages/tiled/lib/src/layer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ abstract class Layer {
bool visible;

/// List of [Property].
List<Property> properties;
CustomProperties properties;

Layer({
this.id,
Expand All @@ -109,7 +109,7 @@ abstract class Layer {
this.tintColor,
this.opacity = 1,
this.visible = true,
this.properties = const [],
this.properties = CustomProperties.empty,
});

static Layer parse(Parser parser) {
Expand Down
4 changes: 2 additions & 2 deletions packages/tiled/lib/src/objects/tiled_object.dart
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class TiledObject {

List<Point> polygon;
List<Point> polyline;
List<Property> properties;
CustomProperties properties;

/// The "Class" property, a.k.a "Type" prior to Tiled 1.9.
/// Will be same as [type].
Expand All @@ -89,7 +89,7 @@ class TiledObject {
this.visible = true,
this.polygon = const [],
this.polyline = const [],
this.properties = const [],
this.properties = CustomProperties.empty,
});

bool get isPolyline => polyline.isNotEmpty;
Expand Down
4 changes: 2 additions & 2 deletions packages/tiled/lib/src/tiled_map.dart
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class TiledMap {
RenderOrder renderOrder;

List<EditorSetting> editorSettings;
List<Property> properties;
CustomProperties properties;

// only for hexagonal maps:
int? hexSideLength;
Expand Down Expand Up @@ -118,7 +118,7 @@ class TiledMap {
this.staggerAxis,
this.staggerIndex,
this.editorSettings = const [],
this.properties = const [],
this.properties = CustomProperties.empty,
});

/// Takes a string [contents] and converts it to a [TiledMap] with the help of
Expand Down
4 changes: 2 additions & 2 deletions packages/tiled/lib/src/tileset/terrain.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ part of tiled;
class Terrain {
String name;
int tile;
List<Property> properties;
CustomProperties properties;

Terrain({
required this.name,
required this.tile,
this.properties = const [],
this.properties = CustomProperties.empty,
});

Terrain.parse(Parser parser)
Expand Down
4 changes: 2 additions & 2 deletions packages/tiled/lib/src/tileset/tile.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class Tile {
TiledImage? image;
Layer? objectGroup;
List<Frame> animation;
List<Property> properties;
CustomProperties properties;

Tile({
required this.localId,
Expand All @@ -39,7 +39,7 @@ class Tile {
this.image,
this.objectGroup,
this.animation = const [],
this.properties = const [],
this.properties = CustomProperties.empty,
});

bool get isEmpty => localId == -1;
Expand Down
6 changes: 3 additions & 3 deletions packages/tiled/lib/src/tileset/tileset.dart
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class Tileset {
TiledImage? image;
TileOffset? tileOffset;
Grid? grid;
List<Property> properties = [];
CustomProperties properties;
List<Terrain> terrains = [];
List<WangSet> wangSets = [];

Expand All @@ -85,7 +85,7 @@ class Tileset {
this.image,
this.tileOffset,
this.grid,
this.properties = const [],
this.properties = CustomProperties.empty,
this.terrains = const [],
this.wangSets = const [],
this.version = '1.0',
Expand Down Expand Up @@ -185,7 +185,7 @@ class Tileset {
tileWidth = tileset.tileWidth ?? tileWidth;
transparentColor = tileset.transparentColor ?? transparentColor;
// Add List-Attributes
properties.addAll(tileset.properties);
properties.byName.addAll(tileset.properties.byName);
terrains.addAll(tileset.terrains);
tiles.addAll(tileset.tiles);
wangSets.addAll(tileset.wangSets);
Expand Down
4 changes: 2 additions & 2 deletions packages/tiled/lib/src/tileset/wang/wang_color.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ class WangColor {
int tile;
double probability;

List<Property> properties;
CustomProperties properties;

WangColor({
required this.name,
required this.color,
required this.tile,
this.probability = 0,
this.properties = const [],
this.properties = CustomProperties.empty,
});

WangColor.parse(Parser parser)
Expand Down
4 changes: 2 additions & 2 deletions packages/tiled/lib/src/tileset/wang/wang_set.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ class WangSet {
List<WangColor> cornerColors;
List<WangColor> edgeColors;
List<WangTile> wangTiles;
List<Property> properties;
CustomProperties properties;

WangSet({
required this.name,
required this.tile,
this.cornerColors = const [],
this.edgeColors = const [],
this.wangTiles = const [],
this.properties = const [],
this.properties = CustomProperties.empty,
});

factory WangSet.parse(Parser parser) {
Expand Down
10 changes: 9 additions & 1 deletion packages/tiled/test/fixtures/test.tmx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,15 @@
<map version="1.2" tiledversion="1.2.1" orientation="orthogonal" renderorder="right-down" width="10" height="10" tilewidth="32" tileheight="32" infinite="0" nextlayerid="2" nextobjectid="1" backgroundcolor="#ccddaaff">
<tileset firstgid="1" name="basketball" tilewidth="32" tileheight="32" tilecount="2" columns="1">
<properties>
<property name="test_property" value="test_value"/>
<property name="string property" value="test_value"/>
<property name="multiline string">Hello,
World</property>
<property name="integer property" type="int" value="42"/>
<property name="bool property" type="bool" value="true"/>
<property name="color property" type="color" value="#00112233"/>
<property name="float property" type="float" value="1.56"/>
<property name="file property" type="file" value="./icons.png"/>
<property name="object property" type="object" value="32"/>
</properties>
<image source="icons.png" width="32" height="32"/>
<tile id="0" class="tile0Class">
Expand Down
Loading