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

FlxAngle.rotatePoint() -> FlxPoint#rotate(), FlxAngle.getAngle() -> FlxPoint#angleBetween() #1143

Merged
merged 2 commits into from
Jun 4, 2014

Conversation

Gama11
Copy link
Member

@Gama11 Gama11 commented May 31, 2014

This makes sense because these are functions that operate on FlxPoint. There is no reason for these to be located in a static utility class.

Also, by taking x and y as parameters separately, you end up having to pass the x and y of a point around individually, which kind of defeats the point (no pun intended).

Moving these to FlxPoint greatly reduces their function signature (rotatePoint() went from 6 parameters (one optional) to only 2 (!).

I think it makes for much more readable code. The code snippet I posted here now becomes this:

package;

import flixel.FlxG;
import flixel.FlxSprite;
import flixel.FlxState;
import flixel.math.FlxAngle;
import flixel.system.scaleModes.FixedScaleMode;
using flixel.util.FlxSpriteUtil;

class PlayState extends FlxState
{   
    var sprite = new FlxSprite();
    var pivot = new FlxSprite(200, 200);

    override public function create():Void 
    {
        FlxG.scaleMode = new FixedScaleMode();
        add(sprite.screenCenter());
        add(pivot);
    }

    override public function update():Void 
    {
        super.update();

        var spritePosition = sprite.toPoint();
        spritePosition.rotate(pivot.toPoint(), 3);
        sprite.setPosition(spritePosition.x, spritePosition.y);
    }
}

As you can see, this comboes very nicely with FlxObject#toPoint() I added here. The only thing that's bothering me a little here is that there's no way to set the position of a FlxObject via a point. Maybe we should add FlxObject#setPositionByPoint()? That strikes me as too verbose though.

@Gama11
Copy link
Member Author

Gama11 commented May 31, 2014

The other thing I'm not sure about is whether or not these functions should be inlined. I left the keyword in there for now, but they are quite lenghty, so it might not be beneficial at all.

* @param Angle Rotate the point by this many degrees clockwise.
* @return A FlxPoint containing the coordinates of the rotated point.
*/
public inline function rotate(Pivot:FlxPoint, Angle:Float):FlxPoint
Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely not a good inline candidate (loops and if statements).

@gamedevsam
Copy link
Contributor

This is excellent! 👍

Gama11 added a commit that referenced this pull request Jun 4, 2014
FlxAngle.rotatePoint() -> FlxPoint#rotate(), FlxAngle.getAngle() -> FlxPoint#angleBetween()
@Gama11 Gama11 merged commit b804097 into HaxeFlixel:dev Jun 4, 2014
Gama11 added a commit to HaxeFlixel/flixel-demos that referenced this pull request Jun 4, 2014
@Gama11 Gama11 deleted the FlxAnglePoint branch May 11, 2016 14:43
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.

2 participants