-
-
Notifications
You must be signed in to change notification settings - Fork 31
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!: Dart SDK compatibility #77
Changes from 7 commits
c37b7cf
e389a08
8f38c1c
f697800
e104efd
6b60e92
6a8e362
ab5611f
4d3786c
82a2b34
8acf697
d68fefc
38101fb
f0721be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
part of tiled; | ||
|
||
/// 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; | ||
|
||
final int red; | ||
final int green; | ||
final int blue; | ||
final int alpha; | ||
|
||
/// Parses the Color from an int using the lower 32-bits and tiled's format: 0xaarrggbb | ||
ColorData.hex(int hex) | ||
: alpha = _sub(hex, 3), | ||
red = _sub(hex, 2), | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "rgba"? and why deviate from "argb"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, I was trying to stick with the dart:ui color for familiarity. |
||
: assert(red >= 0 && red <= 255), | ||
assert(green >= 0 && green <= 255), | ||
assert(blue >= 0 && blue <= 255), | ||
assert(alpha >= 0 && alpha <= 255); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import 'dart:math'; | ||
|
||
import 'package:test/expect.dart'; | ||
import 'package:test/scaffolding.dart'; | ||
import 'package:tiled/tiled.dart'; | ||
|
||
void main() { | ||
group("ColorData.hex", () { | ||
test("parse", () { | ||
final random = Random(); | ||
final red = random.nextInt(256); | ||
final green = random.nextInt(256); | ||
final blue = random.nextInt(256); | ||
final alpha = random.nextInt(256); | ||
|
||
final hex = alpha << 24 | red << 16 | green << 8 | blue << 0; | ||
final data = ColorData.hex(hex); | ||
|
||
expect(data.alpha, equals(alpha)); | ||
expect(data.red, equals(red)); | ||
expect(data.green, equals(green)); | ||
expect(data.blue, equals(blue)); | ||
}); | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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!