Horde3D

Hi guys,

I propose to add some new API functions relative to animation area.

For example, currently it looks impossible to retrieve info about the current weight and elapsed time of an animation.

There's a "h3dSetModelAnimParams(...)" function but not a "h3dgetModelAnimParams()" one.

My proposal is to do the following change:



We can still keep the old "h3dSetModelAnimParams" for retro-compatibility but deprecating its use (and remove it in a future version). This way the set/get process is consistent with the rest of the API.

Another plus is that we could add more functions to cover other types of params (int, str, etc.) if we need them.

I agree but would propose to break compatibility to not clutter up the API. Although the version names of Horde3D are not optimal (as discussed before), one could see the Beta version number as version minor number, and I don't have a problem with changing APIs slightly when changing the minor version.

I changed a little bit my original intention with the following approach (which has also less impact on the API):



Basically, that's it!

Adding these new values, let us to use existing interface to query for these parameters.

For example:



Obviously, changes to the internal API are necessary (to "Model" and "AnimationController" classes).

What do you think about this solution?

PS: A question. Why "Model" doesn't inherit from (or, better, why doesn't integrate the API of) "AnimationController"? What's the point to have this level of indirection?

I merged your pull request, but I'm not sure if the proposed patch is the best solution. The h3dSetModelAnimParams could be called very often (e.g. the Chicago sample). Having it split up into three function calls all doing the same validation on the passed parameters, seems not the best solution to me. Maybe we should add the getters the way you prosed it in your pull request, but leave the h3dSetModelAnimParams. Having both ways (setter functions and the separate h3dSetModelAnimParams function) may be confusing. What do others think?

The problem is that both solutions could be "confusing":

1) Leave "h3dSetModelAnimParams" together with new setters: redundant (Current solution).

2) Remove setters but leave the new param values for getters: really inconsistent (the enum values are there but you can not use them with usual "h3dSetNodeParamF").

IMHO, the best solution is the first one. Redundancy is better than inconsistency. We can simply document that, due to performance reasons, is always preferable use "h3dSetModelAnimParams" over normal setters when you have to set both params at the same time (that's why I "removed" the deprecation of "h3dSetModelAnimParams").

EDIT: There's also a 3) option:

3) Remove new "AnimTimeF" and "AnimWeightF" values from "H3DModel" enum, but keep new "AnimCountI". Then add a new "void h3dGetModelAnimParams(H3DNode model, int stage, float* time, float* weight)" function to retrieve both parameters with one function call (in this case there is no big performance gain on the getter, but we keep the consistency between setter and getter without doubling the overhead on setter as in the current implementation). This is the same approach of "h3dGetNodeTransform(...)" function, for example.