BlendAction: resolve slow-motion side effect caused by stretching actions

[Ali-RS]

This resolves slow-motion side effects caused by BlendAction stretching any animation that doesn't have the same length. It generates speed factors for each child animation that are dynamically interpolated and applied to base speed based on the blend weight taken from blend space.

This is a breaking change, as the speed returned by BlendAction.getSpeed() used to be a static value but now it returns a dynamic value that is changed as blend weight is changed. Unless speedFactors is cleared out or factors are equal. (i.e. clips have the same length).

[Ali-RS]

Please let me know if something is vague about Javadoc or method naming.

[Ali-RS]

I made a demo video:

Old behavior (not using dynamic speed): https://youtu.be/jZY2u9GU_L8

New behavior (using dynamic speed): https://youtu.be/1TuevW1yick

[capdevon]

Hi Ali, may I ask you why the calculateSpeedFactors method is public static and not private? :)

[Ali-RS]

Hi @capdevon :)

Thanks so much for your interest.

Well, this PR is something that I rewrote multiple times in different ways because I was not sure which one is good. I am interested to hear other ideas. I wanted to indicate that users can modify the default factors to something else or even clear it and be able to set them back to the default ones so I made calculateSpeedFactors a utility method for that case (flexibility).

I would also like to hear from @pspeed42 about this.

[pspeed42]

I don't have a problem with the public static utility method. I'm not sure I'd think to use it, either... but I think it's ok.

[Ali-RS]

@MeFisto94 I would like to hear your opinion as well regarding this PR

Do you want to add anything here in regards to #1303?

[capdevon]

@ali

users can modify the default factors to something else or even clear it and be able to set them back to the default ones

@pspeed

I'm not sure I'd think to use it, either...

What do you think of this proposal? :)
how about highlighting the 'default' keyword?

//before
public static double[] calculateSpeedFactors(BlendAction blendAction) {
	Action[] actions = blendAction.getActions();
	return Arrays.stream(actions)
			.mapToDouble(action -> blendAction.getLength() / action.getLength())
			.toArray();
}

//usage example
BlendAction ba = ...;
ba.clearSpeedFactors();
ba.setSpeedFactors(BlendAction.calculateSpeedFactors(ba));

//after (more compact and self consistent version)
public void setDefaultSpeedFactors() {
	Action[] actions = getActions();
	this.speedFactors = Arrays.stream(actions)
			.mapToDouble(action -> getLength() / action.getLength())
			.toArray();
}

//usage example:
BlendAction ba = ...;
ba.clearSpeedFactors();
// I deleted the previous values ​​for some reason, and now I want to set the defaults again.
ba.setDefaultSpeedFactors();

I have an API on top of the current animation system, where I use BlendActions with animations of different lengths. To solve the slow motion problem, I dynamically vary the speed using an algorithm inspired by Unity and Godot. What impact can this PR have on those who have already implemented this type of solution before, since the speedFactors are calculated in the constructor?

I saw that in the TestAnimMigration class, you removed the instructions that change the speed.

//action.setSpeed(blendValue);

should I call the clearSpeedFactors() method throughout my API after creating a BlendAction?

[Ali-RS]

how about highlighting the 'default' keyword?

I am fine with using setDefaultSpeedFactors() instead of a static utility method. Let's see what does @pspeed42 think?

I have an API on top of the current animation system, where I use BlendActions with animations of different lengths. To solve the slow motion problem, I dynamically vary the speed using an algorithm inspired by Unity and Godot. What impact can this PR have on those who have already implemented this type of solution before, since the speedFactors are calculated in the constructor?

Will break them. If you want to handle this using your API you must unset the speed factors using (clearSpeedFactors).

Again I would like to ask @pspeed42 what does he think about it?

should I call the clearSpeedFactors() method throughout my API after creating a BlendAction?

Yes, if you want to handle speed using your API.

[pspeed42]

I'm doing my best to follow along but I don't have much experience in this area. So while I can prevent "obviously wrong" general API things from happening, I don't have a strong opinion on how the details work out.

If the speed can vary based on blending and different people want to blend things different ways, I wonder if a strategy object/function would be appropriate? At some point it becomes over-engineered for the problem space, though.

[Ali-RS]

To solve the slow motion problem, I dynamically vary the speed using an algorithm inspired by Unity and Godot.

@capdevon can you please show me the code you are using?

Edit:
Never mind, I think I found it on your repo. It's the AnimatorState, yes?

I think if you want to make this compatible with your API you need to remove this:

https://github.com/capdevon/jme-capdevon-examples/blob/3d6133d9dc7abd85f80229bc315c33c56db6d6f3/src/main/java/com/capdevon/anim/fsm/AnimatorState.java#L146

and

https://github.com/capdevon/jme-capdevon-examples/blob/3d6133d9dc7abd85f80229bc315c33c56db6d6f3/src/main/java/com/capdevon/anim/fsm/AnimatorState.java#L160

and instead set the childMotion.timeScale as the speed factor into the BlendAction.

[Ali-RS]

@capdevon
I changed
public static double[] calculateSpeedFactors()
to
public void applyDefaultSpeedFactors().

[capdevon]

I've done some testing with my API and everything looks ok. I have no problem modifying it. I'll do some more testing when version 3.6.0-alpha is released. Another step forward for the engine. Good job! ;)

[Ali-RS]

That's cool.

[Ali-RS]

Unless there's substantial discussion, I plan to integrate this in about 24 hours.

[MeFisto94]

So from what I remember, on the surface, there are three types of ways you can combine two animations in parallel:

1. "Non Looping", i.e. they just run with their different lengths, start at the same point, end different and then when the full cycle is complete, they start again.
2. Looping them with their natural speed, i.e. if one animation only has half the length, it is just played twice as often
3. Looping them by stretching to the slowest animation (i.e. they end at the same time/loop roll over is at the same time)

So in theory, being able to choose between those may be nice, however I think option 2 (that is implemented by that PR) is the most common, especially considering animations are developed for a specific length (so 3 doesn't make too much sense) and option 1 can probably be done by clever tweening?

For the implementation I don't know the animation API good enough any more, I just knew that I was thinking on how you could get them to the same speed by just seleting a clever interpolator value (t), but setting a "fake" speed is probably a lot easier here (or the only possible way)

So yeah, TLDR: Nothing relevant :D

[Ali-RS]

Thanks, @MeFisto94

Well, the looping idea did not turn out well, unfortunately, otherwise, it would definitely be my pick. The blending looks bad, you can see it in this demo video:
https://youtu.be/DJHwDaTy79o

That's why I opted for the second solution, which is dynamically adjusting the speed. Also, this is what the original author was doing in the TestAnimMigration, by externally setting the speed when blending. This PR does this process internally by automatically adjusting the speed. Blending looks much smoother in this approach.