feat!: Dart SDK compatibility #77
Conversation
--> replaced dart:ui Color with color_model package --> custom Rect instead of dart:ui --> replaced flutter_test with test package
…/ added tests for hex decoding in ColorData
| /// Basic data class holding a Color in ARGB format. | ||
| /// This can be converted to dart:ui's Color using the flame_tiled package | ||
| class ColorData { | ||
| static int _sub(int hex, int index) => (hex >> index * 8) & 0x000000ff; |
There was a problem hiding this comment.
I just saw this in the other PR; is there a mix up between the two? If so; my comment there is valid here as well: why not store the "hex" as one value and do bit shifting in setters?
This is what Color was doing before and so the pattern would match and the memory footprint should be the same.
There was a problem hiding this comment.
Yes you are right that would be more efficient! This change does belong here, but I based the PR‘s branch on this one so I don‘t have to integrate the new types later!
| green = _sub(hex, 1), | ||
| blue = _sub(hex, 0); | ||
|
|
||
| const ColorData.rgb(this.red, this.green, this.blue, [this.alpha = 255]) |
There was a problem hiding this comment.
"rgba"? and why deviate from "argb"?
There was a problem hiding this comment.
I don’t really know, it just felt more natural. For most colors the alpha value is just 0xff so it can have a default value at the end. CSS also puts it at the end #rrggbbaa, I believe for the same reason.
This constructor is meant for users and not used internally, so I think this way is a better experience. However I‘m happy to change it!
There was a problem hiding this comment.
Right, I was trying to stick with the dart:ui color for familiarity.
|
Would this help here? https://flutter.dev/go/move-flutter-agnostic-types If yes, help me understand how. |
Yes! That would be exactly what we need here, it migrates all the types that we need out of flutter. Thanks! |
# Conflicts: # packages/tiled/pubspec.yaml
# Conflicts: # packages/tiled/pubspec.yaml # packages/tiled/test/parser_test.dart
|
@benni-tec is this ready to be merged? |
|
I believe it should be ready to merge now, I adjusted Color to fit @jtmcdole's suggestions. |
Description
This PR makes this package compatible with the Dart SDK. In order to achieve this I changed 3 things:
dart:ui's Color with a simple ColorData classRectfromflamewithRectanglefromdart:mathflutter_testwithtestpackageThe
ColorDataclass is used excatly once for which I prepared a flame_tiled PR (however I need to wait until this is merged right) including conversion extensions.No
Rectangleattribute is ever read inflame_tiled, evensoflamealready contains conversion extensions!Checklist
fix:,feat:,docs:etc).-->
melos run analyzefails due toXmlData.valuehowever I think this requires another issue/PRdocsand added dartdoc comments with///.examples.--> I don't think there are any exmaples necessary for this
Breaking Change
I think this should result in a minor version bump to 0.11.0 to indicate these breaking changes!
In order to achieve dart comaptibilty only two things changed:
flame_tiledcontains a conversion extensiondart:math's Rectangle --> Useflame'stoRect()method to convertRelated Issues