PrimaryFeather / Sparrow-Framework

The Open Source Game Engine for iOS
http://www.sparrow-framework.org
Other
538 stars 173 forks source link

SPDisplayObject setRotation can hang. #1083

Closed smeans closed 11 years ago

smeans commented 11 years ago

I inadvertently set the rotation on a sprite to a huge double (something in the 1.0e+9 range) and my program appeared to hang. I modified my fork like so:

Just hoping to save someone some head scratching.

PrimaryFeather commented 11 years ago

Hi Scott!

Mhm, setting such a high (or low) number will be problematic, because Starling executes a very long loop then. However, clamping is not perfect, either; in the current code, you can e.g. do an endless rotation like that:

in each frame, do
    object.rotation += 0.1;

That will rotate the object multiple times; when clamping, on the other hand, it will stop at 180 degrees. That's why it was implemented like this -- and shouldn't be changed, or code relying on this would break.

Nevertheless, thanks for the heads-up, and for taking the time to create the issue!

smeans commented 11 years ago

Ok, but my code has the same behaviour without looping. fmod() is a floating point modulus operation, so it returns floating point remainder. I use it to do continuous rotation without any problem. Just FYI.

PrimaryFeather commented 11 years ago

Aaaaah! Sorry, I didn't read the code that carefully -- I just read "clamp" (which is actually, a wrong comment in my code!), and didn't know the fmod function. That makes a lot of sense, then! I'll make that change, then.

Sorry for the misunderstanding, and thanks for making me aware of this method! =)

smeans commented 11 years ago

No problem Here's the ugly truth: this is my first contribution to someone else's repo. I'm a pretty old-school developer and although I've been using source control for 20+ years but I really never got the knack of contributing to open source. I've decided it's time to start, though. I've got a couple of other suggestions I'll send over when they're ready for prime time. Thanks for re-opening!

PrimaryFeather commented 11 years ago

Thanks a lot, Scott! I'm happy to hear that Sparrow motivated you to do that. =)

To be honest, I'm not much different: before working on Sparrow, I didn't commit many changes to other open source projects, neither. ;-)

In any case, Git and GitHub really make this easier than it used to be.

PrimaryFeather commented 11 years ago

I gave it a short try, but it's not easy to find an (elegant) solution that returns the same values as before. The old method moves the values between -PI and +PI, including both those values. I know that's a little odd, but Flash does the same, and I tried to mimic that. I can't spend any time on it right now, but if you want, you can of course do that. ;-) When the unit tests run through, it's implemented correctly.

PrimaryFeather commented 11 years ago

OK, I chose the easy path of keeping the old range checks, but added the fmod call at the beginning. Thanks again for making me aware of this!