Skip to content

FlxU.rotatePoint() #53

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

Open
FlixelCommunityBot opened this issue Sep 13, 2012 · 9 comments
Open

FlxU.rotatePoint() #53

FlixelCommunityBot opened this issue Sep 13, 2012 · 9 comments
Assignees
Labels

Comments

@FlixelCommunityBot
Copy link

Issue #178 by: scaven

line 569:
var dy:Number = PivotY+Y; //Y axis is inverted in flash, normally this would be a subtract operation

should clearly be subtract operation (err.. maybe normality changed recently?)

change to:
var dy:Number = PivotY-Y;

@FlixelCommunityBot
Copy link
Author

Comment by: Beeblerox

This should be
var dy:Float = Y - PivotY;

@FlixelCommunityBot
Copy link
Author

Comment by: axcho

This is a major issue. I believe that @scaven is correct in his proposed solution:
var dy:Number = PivotY-Y;

Update:
The original Flixel code is consistent with its uses in pathfinding and FlxU.getAngle() and such. However, I still suspect that Flixel's angle calculations (including FlxU.getAngle() and elsewhere) are incorrect, because supplying FlxU.rotatePoint() with my own numbers results in some very strange angles...

@FlixelCommunityBot
Copy link
Author

Comment by: avoxgames

Simple test: rotatePoint(X, Y, 0, 0, 0) should return (X, Y), not (X, -Y)

@IQAndreas
Copy link
Member

I have been playing around with this, and I pushed some test code to the Sandbox.

The fix var dy:Number = PivotY-Y; fixes the problem.

(though, Adam's math is confusing me a bit, and I'm not a fan of all those arbitrary values just thrown in, but I'll live. Also the accuracy is off by a bit, but the difference is tiny, and is understandable if you are incrementing gradually thousands of times rather than setting rotations by a specific value. Developers should expect this.)

I still suspect that Flixel's angle calculations (including FlxU.getAngle() and elsewhere) are incorrect

I have tested, and getAngle() seems to work as expected. There may be some mix-ups since getAngle() expects an object at rotation 0 to be pointing at "the 12 on a clock and going clockwise", whereas in math class we were brought up to see rotation on a circle as starting "at the 3 on a clock and going counter-clockwise".

getAngle() works as intended, and fits in very well with the Flash coordinate system as you can see with the following line: https://github.com/FlixelCommunity/FlixelSandbox/blob/master/src/TestAngles.as#L54

@IQAndreas
Copy link
Member

Alternatively (and this I believe looks more "symmetrical" code-wise) we can use this code which also fixes the issue:

var dx:Number = X-PivotX;
var dy:Number = Y-PivotY;
Point.x = PivotX + cos*dx + sin*dy;
Point.y = PivotY - sin*dx + cos*dy;

@moly
Copy link
Member

moly commented May 1, 2013

There's already a pull request with your proposed fix: #128
It's not been accepted yet as it seems to cause problems with pathfinding.

@IQAndreas
Copy link
Member

There's already a pull request with your proposed fix

Yep, but it seems like SeiferTim is a tad busy at the moment, which is why I'm picking this issue up. I was going to try my hand at fixing the pathfinding, and there also seemed to be some line break issues in the pull request which I would see if I could avoid.

Would you happen to have any of the "testing code" you used for the pathfinding left?

@moly
Copy link
Member

moly commented May 3, 2013

Any pathfinding code should show the problem. You can use the flixel pathfinding demo: https://code.google.com/p/mightiesthero-flash-game-dev-tips/source/browse/FlxPathFinding/src/PlayState.as

@ghost ghost assigned IQAndreas May 4, 2013
@Dovyski
Copy link
Member

Dovyski commented Aug 12, 2013

I'm moving this issue to the "future release" milestone.

If we fix FlxU.rotatePoint() and all internal Flixel references, Flixel should work with the new approach, but the change to FlxU.rotatePoint() will inevitably break the code of everyone using that method expecting the old results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants