Dart SDK compatibility

[benni-tec]

What could be improved

Remove requirement for dart:ui and therefore the Flutter SDK

Why should this be improved

Currently this package/parser is only usable with the Flutter SDK. However I believe there are valid use-cases for the Dart SDK. For example I'm currently working on a CLI to optimize tmx to only include the tiles actually used in a map in it's tilesets. This is not possible using the Flutter SDK.

Any risks?

This improvement would require the removal of all dart:ui dependencies. However there are only two in this project, which I replaced in the "#69-dart-sdk branch" on my fork:

• Color, which I replaced with color_models and a little bit of code for hex parsing
• Rect, which I simply replaced by dart:math's Rectangle

More information

These changes would break public contracts and therefore require some work on the flame_tiled plugin.

However I believe this would be minimal. I did not look into this further, but from what I changed this would only require converting Rect to Rectangle and casting ColorModel to Model, which is implemented in flutter_color_models.

I believe this would be worth it and I am prepared to submit PRs both here and in flame_tiled to make these rather simple changes. However if you would consider this to be too invasive, I am happy to just keep this as a fork!

[spydon]

Overall it sounds like a good idea, we've had this plan before but never done the implementation. I'm was thinking that maybe it is a better idea to pull in the color and rect class directly in tiled.dart instead of pulling it from the third party library so that we don't have to make yet another breaking change if that library become unmaintained and we have to update something, but on the other hand we could just pull in those classes at that time then instead.

[spydon]

@kurtome @ufrshubham @jtmcdole do you have any input on this, #70 and #71?

[benni-tec]

Overall it sounds like a good idea, we've had this plan before but never done the implementation. I'm was thinking that maybe it is a better idea to pull in the color and rect class directly in tiled.dart instead of pulling it from the third party library so that we don't have to make yet another breaking change if that library become unmaintained and we have to update something, but on the other hand we could just pull in those classes at that time then instead.

Maybe I'm missing something, but nothing is ever done with both Rect and Color right? Could Color than not just store RGBA values and thats it? There would be no need for any library at all. (Don't know why I did not think of this before when I implemented this)

Since Rectangle comes from dart:math I think its just find to depend on it!

[benni-tec]

Oh and I forgot to mention that I needed to replace flutter_test with test but I don't think thats going to be problem since their APIs seem to be the same

[spydon]

Maybe I'm missing something, but nothing is ever done with both Rect and Color right? Could Color than not just store RGBA values and thats it? There would be no need for any library at all. (Don't know why I did not think of this before when I implemented this)

Ah yes, if that works it would be great!

Oh and I forgot to mention that I needed to replace flutter_test with test but I don't think thats going to be problem since their APIs seem to be the same

:+1:

[kurtome]

My only thoughts are: make sure the Color primitives play nice with Dart's canvas methods. If I remember correctly I was having issues with the hex string being a pain to deal with, so I put the hex parsing in the tiled.dart library so we could access the dart:ui color value, but also left the hex string on the structs because (in my opinion) we want to leave let users of this library access values from the tmx file

[kurtome]

Could Color than not just store RGBA values and thats it?

I think that works because then you can make a dart:ui Color easily, the problem is the hex string didn't have a built in parser in dart:ui so it was annoying to use

[signmotion]

I needed to include Rect, Color, and Size in some of my Dart projects, therefore this library - pure_dart_ui - came into being.

Also, I needed to parse the TMX file to pure Dart and I am very grateful for your Tiled package. The pure_dart_ui package was included in Tiled, and all tests passed. If you don't mind this option, I will prepare 2 PRs:

1. To close this issue WITHOUT a breaking change.

2. To close the Map Export issue.

[benni-tec]

What do you mean by „WITHOUT a breaking change“? Wouldn’t changing the types be a breaking change anyway?

Also I don‘t think it‘s smart to depend on another library when all that is needed is one Type: Color

Secondly how would you propose a PR for #71 ? Did you already write an Exporter as well? I have already opened #78 with a finished implementation!

[signmotion]

What do you mean by „WITHOUT a breaking change“? Wouldn’t changing the types be a breaking change anyway?

I mean that the types from pure_dart_ui are 100% copied from Flutter dart.ui: all the functionality remains unchanged.

Also I don‘t think it‘s smart to depend on another library when all that is needed is one Type: Color

Wrapping classes in a library is a solution that makes the library more stable, but not inflexible. These classes can be used for many other pure Dart projects and can be extended with some useful extensions, as Flame has already done.

Secondly how would you propose a PR for https://github.com/flame-engine/tiled.dart/issues/71 ? Did you already write an Exporter as well? I have already opened https://github.com/flame-engine/tiled.dart/pull/78 with a finished implementation!

Nice!

[benni-tec]

I mean that the types from pure_dart_ui are 100% copied from Flutter dart.ui: all the functionality remains unchanged.

So does my ColorData class, however both are breaking changes as conversion methods are required in order to adapt to for example dart:ui's Color that is needed by flutter/flame! Or am I missing something?

Wrapping classes in a library is a solution that makes the library more stable, but not inflexible. These classes can be used for many other pure Dart projects and can be extended with some useful extensions, as Flame has already done.

While you are correct I don't believe that is worth it for only a single class, however that is something that should be decided by this projects maintainers @spydon

[signmotion]

So does my ColorData class, however both are breaking changes as conversion methods are required in order to adapt to for example dart:ui's Color that is needed by flutter/flame! Or am I missing something?

The pure_dart_ui contains only the copied classes from dart:ui with all dependencies to make it work. See below for details.

While you are correct I don't believe that is worth it for only a single class, however that is something that should be decided by this projects maintainers

Color, Offset, Rect, Size, and we can add all the necessary classes and functions later. So we can unbind from dart.ui and maintain compatibility with pure Dart for all Flame packages without rewriting the code except for these lines:

* import 'dart:ui' hide Color, Offset, Rect, Size;
+ import 'package:pure_dart_ui/pure_dart_ui.dart';

[spydon]

While you are correct I don't believe that is worth it for only a single class, however that is something that should be decided by this projects maintainers @spydon

I agree here, especially since the class name will clash with everywhere where people are using the Flutter classes. So let's go with the ColorData class.

So we can unbind from dart.ui and maintain compatibility with pure Dart for all Flame packages without rewriting the code except for these lines:

It will still be a breaking change even though no code except the imports need to change, the classes won't be compatible with the ones in dart:ui for our users, even though they are copies.

[signmotion]

Ok with Color / ColorData. What about the other classes: Size, Rect, Offset?

[spydon]

Ok with Color / ColorData. What about the other classes: Size, Rect, Offset?

They are not used here afaik. And the problem would be the same with them.