Re: [boost] [tweener] Preliminary Submission

Subject: Re: [boost] [tweener] Preliminary Submission
From: Klaim - Joël Lamotte (mjklaim_at_[hidden])
Date: 2013-03-02 07:04:23

Hi, some comments on your remarks Micheal:

On Sat, Mar 2, 2013 at 10:28 AM, Michael Marcin <mike.marcin_at_[hidden]>wrote:

>
> How do you do looping tweens? (Both N and infinite loops)
>
> How do you do relative tweens, i.e. instead of tween from -> to, tween by.
> I guess this might not matter as it seems the tweener only tweens its own
> internal value. In many cases it would seem nice to update a memory
> location directly. For instance a member variable of the class that owns
> the tween.
>

Looks to me like advanced features that are not necessary for a first
version.
In particular, in my experience checking a value in memory have been less
useful (and harder to make safe) than keeping a copy of the target value,
but allow it to be updated from user code.

>
> domain_type feels awkard. It's used as both a time_point and duration to
> borrow boost::chrono terminology.
>
>
I think you assume that tweening/interpolation is relative to time. It is
not. This library should never depends on time.
It could provide extensions later to work with time but I feel it's
unnecessary.

> Why does base_tweener have a virtual destructor, virtual do_clone and
> virtual do_is_finished members? It seems you could easily just use CRTP
> here.
>
>
Not sure it's possible in this case, but that would be cool.

> I think tweener_sequence/tweener_group should be a container adapter or at
> least switched to use a std::vector. I know I wouldn't want to pay for a
> std::list here in any use case I can imagine.
>
> FWIW if you're using a std::list (like in tweener_group.ipp) instead of:
> const iterator_type tmp(it);
> ++it;
> m_tweeners.erase(tmp);
> you can do:
> it = m_tweeners.erase(it);
>
> I think the order of tween updates for sequences/groups can be important.
> There should be a guarantee of first-in first-update order and I think the
> name insert should change to push_back to reflect the ordered nature of the
> operation.
>
>
I agree that vector instead of list would be far better.

> It seems all tweeners are one shot in that you cannot restart them.
> tween_sequence and tweener_groups do_update is destructive and prevents
> being able to restart the group.
>
>
I'm not sure to understand what you mean here.

> Why are all the callbacks boost::functions? It's already a template can't
> I just use my own callback type without paying for the type erasure and
> dispatch of a boost::function if I don't need it?
>
>
I think the functions should be template but inside they still have to use
boost::function for storing the callback.

> There appears to be no interface to set the current time_point (m_date)
> directly you can only move it a relative amount. There is also no interface
> to query it. This is a very useful feature, it allows you for instance to
> bind your tween to a slider and scrub back and forth through it.
>
>
I don't understand why you are talking about time and then about non-time
based interpolation.
Do you mean a way to set manually the interpolation state from outside?

> Supporting multiple callbacks for on_finished seems unnecessarily
> expensive an inconsistent (only one update callback supported).
>
>
Not  sure about this.

> init/end seems like a strange name pairing. Maybe from/to, begin/end,
> initial/final? Although using end here at all seems a bit dubious as it is
> so often used in the as a semi-open interval [begin,end) whereas here it is
> a closed interval [init,end].
>
>
I quite agree, I'm just not sure begin/end wouldnt be misleading. I tend to
prefer "target" as the end value name, as it should be modifiable (moving
target).

> I personally think a method chaining interface ala HOTween's TweenParms is
> very nice for configuring tweeners.
>
>
I agree.

>
> As someone who has used TweenLite, iTween, HOTween, and AniMate as well as
> rolled his own (poorly) I'm very happy to have such a library proposed to
> Boost.
>
> Thank you.
>

Same here, I've rolled my own and used some others from different libraries.
It would be very useful to have this in boost.

I wanted to try in one of my projects a previous version of this lib but I
had to cancel the project.
I think I might have an opportunity to use it in the coming month, I'll
give feedback after then.

Joel Lamotte