Skip to content
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

FlxSpriteUtil curveTo + fixes, etc #1263

Merged
merged 4 commits into from Aug 15, 2014
Merged

FlxSpriteUtil curveTo + fixes, etc #1263

merged 4 commits into from Aug 15, 2014

Conversation

MSGhero
Copy link
Member

@MSGhero MSGhero commented Aug 9, 2014

Added curveTo(). The default filling behavior caused the curves to always be filled with a line drawn from end to start to complete the figure (graphics.beginFill + graphics.curveTo default). Changed filling to create a default fillStyle if null in the drawShapes, which maintains the previous behavior of a null fillStyle. A null fillStyle now means beginFill doesn't get called--happens in drawLine and optionally in drawCurve.

Breaking: removed the Color param of beginDraw. Either fillStyle.color was overriding it, or it didn't get used. I doubt anyone used this function (should be private imo), so it's not a big deal. Added fillStyle.alpha to the fill which probably was forgotten in the FlxColor rework.

Consistency in drawCircle params.

I don't like Color and FillStyle both as params, and some params start with a caps letter while others don't (class names), but those are issues for another time.

beginFill only if a fillStyle is specified or assumed
@@ -280,7 +313,12 @@ class FlxSpriteUtil
public static function drawRect(sprite:FlxSprite, X:Float, Y:Float, Width:Float, Height:Float, Color:FlxColor,
?lineStyle:LineStyle, ?fillStyle:FillStyle, ?drawStyle:DrawStyle):FlxSprite
{
beginDraw(Color, lineStyle, fillStyle);
if (fillStyle == null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems extremely redundant to have this in every function. Why not move it to beginDraw()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I'll work with that.

The Color param makes fillStyles moot.  Added a default line thickness to setLineStyle.
@MSGhero
Copy link
Member Author

MSGhero commented Aug 9, 2014

Gonna look through the demos now...

Edit: The addons package has a bunch, sifting through those now.

@@ -529,6 +551,7 @@ class FlxSpriteUtil
if (lineStyle != null)
{
var color = (lineStyle.color == null) ? FlxColor.BLACK : lineStyle.color;
if (lineStyle.thickness == null) { lineStyle.thickness = 1; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you create a lineStyle object, I feel like 99% of the time you want it to have a thickness. {color:FlxColor.RED} doesn't do anything right now since the default thickness is 0.

FillColor instead of Color
@MSGhero
Copy link
Member Author

MSGhero commented Aug 9, 2014

"Calculator" is failing in the Travis CI, but I don't see that demo anywhere...?

@MSGhero
Copy link
Member Author

MSGhero commented Aug 13, 2014

Found it and fixed it.

Gama11 added a commit that referenced this pull request Aug 15, 2014
FlxSpriteUtil curveTo + fixes, etc
@Gama11 Gama11 merged commit c6aedc4 into HaxeFlixel:dev Aug 15, 2014
Gama11 added a commit to HaxeFlixel/flixel-demos that referenced this pull request Aug 15, 2014
Gama11 added a commit to HaxeFlixel/flixel-addons that referenced this pull request Aug 20, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants