Question about new Animation System part 3 - Engine v.3.4.0-beta1

Why do you think this needs to be refactored?

Interesting observations. I propose we:

  • protect the no-arg constructor for TransformTrack
  • modify getRotations() and getTranslations() in TransformTrack to return null if the corresponding CompactArray is null.
1 Like

Because, they aren’t used in any other code scopes, & also naming of convention of BlendSpace interface field methods seems a little bit weird, like having getters without setters or object definitions & more above that “inside an interface”, I am lost inside the code & I think anyone who will try to contribute or fix bugs, he will get lost too.

I think if we care more about the inside code design, we had better not to return null directly, is it better to just return an instance of CompactArray, & the user could use getCompactIndex(i) to fetch by index ?

1 Like

Beware of blind reciprocity.

Obviously it’s fine to have getters without setters. Some getters are read-only. Some are calculated. And so on.

On interfaces, it’s also fine to have setters with no getters if nothing about the API requires getting the value back. Once you add the getter to the interface then you force ALL implementations to implement that method even if it doesn’t make sense. Maybe there is some implementation that takes a value in and would find it complicated to give it back… why should it then store the value just to return it?

Missing on the interface doesn’t mean the implementation can’t implement a getter. It just means that it’s not required to.

I think all of these methods need javadoc and that would clear a lot of things up. In the mean time, the fact that one thing has only a setter and another has only a getter is a very valuable clue.

Don’t erase the clues before the API has been documented unless there is a legitimate reason.

2 Likes

BlendSpace.setValue() is implemented in LinearBlendSpace and used in TestAnimMigration. What changes do you have in mind?

1 Like

I suspect the design intention was to hide CompactArray from the application programmer. Hiding such implementation details allows future re-implementation without affecting the API. I recommend keeping them hidden.

To provide a concrete basis for further discussion, I’ve gone ahead and submitted PRs 1531 and 1532.

4 Likes

Oh, i haven’t noticed that blendSpace is implemented by another class, i was implementing the BlendSpace into my state class, passing it into the BlendAction constructor & using the getWeight directly, not sure if it was the right use…

Yeh i supposed there’s no a call for setValue(value:Float) inside the BlendableAction API (ie : there’s no blendSpace.getValue() or blendSpace.value), but now i see that LinearBlendSpace is using setValue() & using var value inside getWeight() which in turn get called inside the BlendableAction,

but if you remove setValue from the interface BlendSpace, nothing would be changed, & the API still wouldn’t be affected, that is what i am trying to say…, except for the TestAnimMigration that would get broken at action.getBlendSpace().setValue(x) :grin: , but you still can use a global variable of LinearBlendSpace like linearBlendSpace.setValue(x);

At the end, it is just an opinion (not argument at all) about code design, so normal users like me won’t get lost, …that change that i have mentioned would render no change in code functionality, hopefully if there’s something else that i donot know about, but anyway if that’s documented properly, right there shouldn’t be a change…

EDIT : There may be a blueprint design, that illustrates why the author is doing this, may be having a BlendSpace & BlendSpace properties (fields of interface) for each single blendAction that controls the interpolation values & those controls get all enclosed inside an Entity named BlendSpace interface

1 Like

The whole point of a blend space is to be able to set a value and have it blend. It may be a linear space, it may be a cubic space, spherical space… whatever. Application code and/or other actions will be inputting some “value” to blend.

Having a setValue() method is then a good indication of the intent of the interface. And your confusion is then deeper than “Huh, why is this method here?”

Do not confuse a badly documented API for a badly designed one. And if you don’t understand the use for some method then you may be in a poor position to decide which is which.

2 Likes

Yeh, you were right, i started to understand when i have seen the LinearBlendSpace, as i mentioned above, at first i was implementing the BlendSpace inside my class, so i was seeing no use for setValue().

1 Like

And just in case a typical example would be the idle/walk/run blend animation.

Based on the character move speed you may set the blend value in [0, 1] to sync animation state with character move speed.

3 Likes

hi everyone, I am trying to complete the transition to the new animation system in some of my projects. Could you tell me how to convert these instructions? Thanks for your help.

  • Case A: assuming animComposer is running a looped animation
private SkeletonControl skeletonControl;

@Deprecated
private void resetBoneTransform() {
	int boneCount = skeletonControl.getSkeleton().getBoneCount();
	for (int i = 0; i < boneCount; i++) {
		Bone bone = skeletonControl.getSkeleton().getBone(i);
		if (bone.hasUserControl()) {
			bone.setUserControl(false);
			System.out.println("Reset User Control: " + bone.getName());
		}
	}
}

private SkinningControl skinningControl;

private void resetBoneTransform() {
	int boneCount = skinningControl.getArmature().getJointCount();
	for (int i = 0; i < boneCount; i++) {
		Joint bone = skinningControl.getArmature().getJoint(i);
		
		/* ???
		if (bone.hasUserControl()) {
			bone.setUserControl(false);
			System.out.println("Reset User Control: " + bone.getName());
		}
		*/
	}
}
  • Case B: assuming animComposer is running a looped animation
private AnimComposer animComposer;
private Camera camera;
private boolean isAiming;
private Bone spineBone;
private Quaternion tempRotation = new Quaternion();
private float[] angles = new float[3];

...
    
@Override
protected void controlUpdate(float tpf) {
	updateBoneIK(tpf);
	updateWeaponAiming(tpf);
}

/**
 * rotates the spine according to the angle of the camera.
 * @param tpf
 */
private void updateBoneIK(float tpf) {
	if (isAiming) {
		camera.getRotation().toAngles(angles);
		float rx = FastMath.clamp(angles[0], -0.1f, 0.75f);
		tempRotation.fromAngles(0, 0, -rx);
        // ???
		spineBone.setUserControl(true);
		spineBone.setUserTransforms(Vector3f.ZERO, tempRotation, Vector3f.UNIT_XYZ);

	} else {
		spineBone.setUserControl(false);
	}
}

Edit case A and B

1 Like

I don’t know what setUserControl(bool:Boolean) does, but I believe there are multiple ways to do it،

Since, Joint implements HasLocalTransform, then setting the local transform would work :

//stop the current Action
animComposer.removeAction("Running");
//fetch default start animation tracks or use const attributes 
Vector3f defaultTranslation = transformTrack.getTranslations[0];
Vector3f defaultScale = transformTrack.getScales()[0];
Quaternion defaultQuat = transfromTrack.getRotations()[0];

Transform transform = new Transform(defaultTranslations, defaultRotations, defaultScales);
//teleport animation to start 
joint.setLocalTransform(defaultTransform);

But, warning this is a teleport reset…not an animated reset.

I believe there are better ways, I will search more…& you will need to stop the currentAction detach & reattach to stop the animation, if you are planning to reset the animation during the tracks play…

At the end, It depends on what you want to do.

So, you want the equivalent of that code in the new system ? , I think if that is the case, you would follow similar steps like any normal animation.

Maybe this helps

Sorry for spamming, I think those package-private methods modifiers could be settled to protected explicitly :

Here :

And Here :

2 Likes

@Ali_RS @Pavl_G I take some time to think about it. thank you.

2 Likes

I’ve opened a PR for the package-private methods: PR 1536.

2 Likes

@capdevon If i were not wrong, i think you are searching for something like this :

To do an animation reset on a multiLayer AnimComposer Control :

  • make an Animation layer :
  • Bind the BaseAppState class containing your Animation Code to the AnimComposer Control :

For Enabling :

For Disabling & Stopping Animation Action inside that layer :

  • Inside your AnimationFactory class Util do this to attach the animations :
  • Inside the AnimtionFactory to play Animations Attached do :
  • To reset the All the Animations :
1 Like

Today, i was trying to build a BlendSpace class that fits my needs, so i though first i should study LinearBlendSpace, So noticed using setFirstActiveIndex(), setSecondActiveIndex() to swap between Actions inside that blendSpace, but i also noticed they have protected access so only the classes within same package & the subclasses can use it, so why not making them public access so we can build our own blendSpaces without interruption ?

Those methods setFirstActiveIndex(), setSecondActiveIndex() that controls the indices of the current Actions inside a BlendAction should have a public access :

2 Likes

That sounds reasonable. Go ahead and submit a pull request.

2 Likes

Hi there, long time isn’t it, i have figured it out through Inheritance & Composition, but i am still not sure if this was the right way to do custom blendspaces, and i cannot see a major difference between a radial blendSpace or any other type of BlendSpaces & the linear one since proper weight values are between zero & one :

package com.scrappers.dbtraining.mainScreens.prefaceScreen.renderer.animationWrapper.customBlendAction;

import com.jme3.anim.tween.action.Action;
import com.jme3.anim.tween.action.BlendAction;
import com.jme3.anim.tween.action.BlendSpace;
import com.jme3.anim.tween.action.BlendableAction;
import com.jme3.math.FastMath;

/**
 * A new runnable blendSpace test through a blendAction implementation utility, using the composition & concrete classes.
 * @author pavl_g.
 */
public final class CustomBlendAction extends BlendAction {
    public CustomBlendAction(BlendSpace blendSpace, BlendableAction... actions) {
        super(blendSpace, actions);
    }

    @Override
    protected void setFirstActiveIndex(int index) {
        super.setFirstActiveIndex(index);
    }

    @Override
    protected void setSecondActiveIndex(int index) {
        super.setSecondActiveIndex(index);
    }

    @Override
    protected Action[] getActions() {
        return super.getActions();
    }

    /**
     * Radial BlendSpace testcase.
     * @author pavl_g.
     */
    public static class RadialBlendSpace implements BlendSpace {

        //Concrete Composition level-2
        private CustomBlendAction blendAction;
        private float minRadius;
        private float maxRadius;
        private float minCircumference;
        private float maxCircumference;
        private float step;

        public RadialBlendSpace(final float minRadius, final float maxRadius){
            this.minRadius = minRadius;
            this.maxRadius = maxRadius;
        }
        @Override
        public void setBlendAction(BlendAction action) {
            if(action instanceof CustomBlendAction){
                this.blendAction = (CustomBlendAction) action;
                //other useful assigns.
                //1) getting the minCircum & the maxCircum.
                minCircumference = 2 * FastMath.PI * minRadius;
                maxCircumference = 2 * FastMath.PI * maxRadius;
                //2)finding the step length to be used for shuffling between animations, based on a circle arcs(subscribed circles areas).
                final float numberOfSpacesBetweenActions = ((CustomBlendAction) action).getActions().length - 1;
                final float lengthOfBlendSpace = (maxCircumference - minCircumference);
                //3)the step used for blending between the blendActions.
                step = lengthOfBlendSpace / numberOfSpacesBetweenActions;
            }else{
                throw new IllegalStateException("BlendAction Should be of type " + CustomBlendAction.class.getName());
            }
        }

        /**
         * Get the weight for the current blendSpace.
         * @apiNote the blendSpace weight : is the scale of blendSpace.
         * @return a number from zero to one representing the scale of the blendingAction.
         */
        @Override
        public float getWeight() {
            final int numberOfActiveActions = blendAction.getActions().length;
            float lowStep = minCircumference, highStep = minCircumference;
            int firstActiveAction = 0;
            for(int activeIndex = 0; activeIndex < numberOfActiveActions && highStep < maxCircumference;
                                     activeIndex++, highStep += step){
                //get the firstActiveAction
                firstActiveAction = activeIndex;
                //assign the lowStep to get the value of old highStep
                lowStep = highStep;
                //log the values
                System.out.println(firstActiveAction + " "+lowStep+" "+highStep);
            }
            //set the active indices for the runnable actions, indices get collected based on the circum steps.
            blendAction.setFirstActiveIndex(firstActiveAction);
            blendAction.setSecondActiveIndex(firstActiveAction + 1);
            //handle the redundant steps
            if(lowStep == highStep){
                return 0;
            }
            //return the ratio between the 2 circle waves (i.e : maxCircumference & minCircumference), NB : the 2 values are adjustable via their radii
            // (a circle perimeter represent the direction of propagation of the animation waves)
            return minCircumference / maxCircumference;
        }

        /**
         * Set the max value for the blending animation.
         * @param maxRadius the max value.
         */
        @Override
        public void setValue(float maxRadius) {
            this.maxRadius = maxRadius;
            //re-calculate the maxWave
            maxCircumference = 2 * FastMath.PI * maxRadius;
            //recalculate values
            setBlendAction(blendAction);
        }

        public void setMaxRadius(float maxRadius) {
            this.maxRadius = maxRadius;
            //re-calculate the maxWave
            maxCircumference = 2 * FastMath.PI * maxRadius;
            //recalculate values
            setBlendAction(blendAction);
        }

        public void setMinRadius(float minRadius) {
            this.minRadius = minRadius;
            //recalculate values
            minCircumference = 2 * FastMath.PI * minRadius;
            //recalculate values
            setBlendAction(blendAction);
        }

        public void shuffleActionIndices(){
            blendAction.setFirstActiveIndex(1);
            blendAction.setSecondActiveIndex(0);
        }
    }
    //...........Other useful blendSpaces...........

}

EDIT :
Implementation Code :

Shuffling/blending between actions manually through update :