domhofmann / PRTween

Lightweight tween library for iOS
BSD 2-Clause "Simplified" License
459 stars 63 forks source link

Timing function updates #10

Open typeoneerror opened 12 years ago

typeoneerror commented 12 years ago

Seems like the timing functions have been locked down and removed some of the original parameters from Penner's methods.

CGFloat PRTweenTimingFunctionElasticOut (CGFloat t, CGFloat b, CGFloat c, CGFloat d) {
    CGFloat p = d*.3;
    CGFloat s, a;
    if (t==0) return b;  if ((t/=d)==1) return b+c;
    if (!a || a < ABS(c)) { a=c; s=p/4; }
    else s = p/(2*M_PI) * asin (c/a);
    return (a*pow(2,-10*t) * sin( (t*d-s)*(2*M_PI)/p ) + c + b);
}

This for example is missing the amplitude and period params, so much of the logic testing for a isn't needed if you're going to hard code those values right? Just noticed this because when running Analyze you get "a evaluates to a garbage value." Any reason that you got rid of those parameters?

domhofmann commented 12 years ago

It's a remnant from a quick port job of Penner's curves. I think ultimately we want to leave those parameters in, as they can be quite useful and it bothers me that they're hard coded now. There's a question hanging in the air about best approach to this.

One thought is to modify the functions to look like this:

CGFloat PRTweenTimingFunctionElasticOut (CGFloat t, CGFloat b, CGFloat c, CGFloat d, NSDictionary *options) {
    ...
}

In this case I think we'd expose it through a property on PRTweenOperation (call it timingFunctionOptions or something similar.)

With shorthands, it could be handled in two ways. We'd get chaining for free:

[PRTween tween:&someValue from:0 to:100 duration:1 timingFunction:&PRTweenTimingFunctionElasticOut].timingFunctionOptions = options;

Or we could add/modify the method signature to include an options argument, something like [PRTween tween:from:to:duration:timingFunction:options:]

What are your thoughts?

typeoneerror commented 12 years ago

I'd say add a new method signature (not modify) like the last one that allows you to send options dictionary, and have the original method just pass nil to that method. I think 9.9 times out of 10 people will just use the default values that are currently hard-coded, but I always prefer a "complete" API, ya know? Seems like a low priority update.

domhofmann commented 12 years ago

Working on this and it should be up soon.

As a side note, don't hit E by accident on the issues page.

typeoneerror commented 12 years ago

lol, I had to hit 'e' to test, naturally.

typeoneerror commented 12 years ago

Looking forward to the update. I submitted a quick pull request for a typo and a weird "/" I found inside one of the transition functions.