Action#forward is a redundant field

Development
1

Hello, there were questions raised lately around the real usage of the Action#forward boolean field, and whether it will be better to make it a public field.

I don’t really see a good use of it, other than just a shortcut, otherwise, it’s considered a redundant field, if anyone noticed an essential use, please kindly report on this thread here.

1 Like
2

Perhaps the commit it was added is enlightening:

I don’t know if it’s really needed but it did have a purpose when it was added.

2 Likes
3

I see, thanks for sharing. I still not convinced by these changes back then, the problem is the commit adds the field, but didn’t use it right. For example, if someone tries to use Action#setForward(true) in an implementation of a BlendableAction, what will be the next move? You didn’t actually change the speed direction. The next question is ‘What is the reason for dissociating the speed direction into a separate boolean, and associating it only when using Action#setSpeed(), that’s a very weird relationship? Couldn’t it just be speed direction only?’.

4

I agree with sgold that this is simply redundant:

  if(isForward()) {
      transition.interpolate(t);
  } else {
      float v = Math.max((float)(getLength() - t), 0f);
      transition.interpolate(v);
  }

And, it appears only in BlendableAction, it seems that this PR is for a BlendableAction compatibility, but complicated things more than it should be.

5

The only thing I can think of would be “What direction is 0?”

I don’t know if it ever matters to know the difference or if the code is literally treating them the same now. It’s just that when there are two separate variables and one is ambiguous and the other isn’t, that’s what I think of.

1 Like
6

Well, zero is really a different case, and it is treated as ‘stop’, I know it’s kind weird, but AnimComposer can explain this.

7

My point is that the AnimComposer forwards the interpolation to the Action objects; after which a condition is raised {if the Action is a BlendableAction} then the speed direction is dissociated into a boolean, and that could lead potentially to a bug.

An example situation is if in case the forwarded interpolation time into the Action objects is a negative value, then, the forward flag will act as another negative sign, which means if it’s true in this case, the result will be backward (and not forward) and vice versa. I fear that this relation is true.

8

That field is used for activating smooth animation transition. When action is forward then smooth transition is applied at the start of the animation and when it is backward then smooth transition is applied at the end of the animation.
(However, I think instead of defining a new field, we could check that with speed > 0)

But note that the idea of using negative speed for playing action backward is not compatible with other tweens like the Sequence tween.

If you make a sequence of tweens (e.g. CallMethod(“m1”) + walkAction + CallMethod(“m2”)) and play it backward (set negative speed) the result won’t be as you expected.

In that case, we should use the Invert tween for playing action backward. So our sequence would be like this

(CallMethod(“m1”) + Invert(walkAction) + CallMethod(“m2”))

2 Likes
9

So, I guess we have a misnomer issue here. I think we should decouple speed, direction, and smooth transitions into separate fields. Each field should have only and only one action, do you guys agree with this? If that is the case, then please help me to know the prerequisites for this change.

EDIT:
Maybe speed and direction can be coupled into a single field, but I don’t think it’s a good idea at some point to use multiple fields and refer to the same thing, and then re-using the field again for different totally irrelevant function.

1 Like