Dart SDK compatibility about tiled.dart

[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

👍

[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 #71 ? Did you already write an Exporter as well? I have already opened #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.