flame-engine/tiled.dart

Make it easier and more efficient to access a property by name

kurtome opened this issue · 5 comments

kurtome commented

What could be improved

Properties on tiles and objects are unique, but are returned as List<Property>, which makes it difficult to access a property by name and requires iterating through the entire list each time.

So, I'm suggesting changing tile.properties and object.properties (and anywhere else applicable) to Map<String, Property>, for better access.

Why should this be improved

The Tiled editor doesn't allow multiple properties with the same name, and accessing the properties by name would better match the Tiled editor. In addition, if the properties are keyed by name it will be more efficient.

Any risks?

This would be a breaking change.

spydon commented

Sounds very sensible, and since tiled is pre-v1, it's definitely worth the breaking change.
It'll break for flame_tiled users too ofc, but I think it is worth doing these quality of life fixes. flame_tiled wasn't supposed to be >v1, we accidentally released that as that when we released Flame v1.
Should I assign you to the issue?

  • 👍2
kurtome commented

Sounds very sensible, and since tiled is pre-v1, it's definitely worth the breaking change.

It'll break for flame_tiled users too ofc, but I think it is worth doing these quality of life fixes. flame_tiled wasn't supposed to be >v1, we accidentally released that as that when we released Flame v1.

Should I assign you to the issue?

Yup, I can make this change

jtmcdole commented

Can we avoid breaking by using:

  1. a new sensible name
  2. @deprecated('use sensible name') the properties today?
kurtome commented

Can we avoid breaking by using:

  1. a new sensible name

  2. @deprecated('use sensible name') the properties today?

We could! But if we don't really care about backwards compatibility, might as well keep the simple name "properties"?

spydon commented

Can we avoid breaking by using:

  1. a new sensible name

  2. @deprecated('use sensible name') the properties today?

We could! But if we don't really care about backwards compatibility, might as well keep the simple name "properties"?

Is there something called properties in tiled? If that is the case I'd definitely keep it. And as you said, we're don't care that much about breaking changes here.