Fix animation transitions on cloned models

[toaster0123]

What goes wrong

Cloning a model with a registered ClipAction, then playing that action on the clone, can skip the smooth transition into the animation. If the original action was already playing, the clone can instead keep the blend weight from the moment it was copied.

For example, the regression test animates a node from its starting position toward x=8, using a one-second transition. After cloning the node and advancing its animation by 0.25 seconds, the clone should be at x=2. On the current code, it jumps straight to x=8. Updating the clone also changes the original action's internal transition weight.

Why it happens and what changes

BlendableAction clones its transition tween, but that tween is a non-static inner object. Its copy still refers to the original action, so it writes the original action's transition weight instead of its own.

Create the copied action's transition tween with the copied action as its owner. Preserve the tween's current effective duration, including any duration already clamped to the animation length. This is an internal cloning correction; no public API changes.

Verification

• Added eight regression/control tests. Before the fix, seven fail and the uncloned-action control passes.
• Coverage includes cloned-model playback, source-action isolation, separate maximum transition weights, reverse playback, zero transition duration, and cloning after the transition duration has been clamped.
• An independent review reproduced the bug and verified this fix, including three additional clone-isolation probes.
• Built jme3-core from a clean state, then completed the core, desktop, plugins, and effects module builds. Their test reports contain 564 tests: 562 passed and 2 existing tests skipped.
• One existing test, TestLocators.testManyLocators, could not access its Google archive URL because this environment's proxy returned HTTP 403. The initial unfiltered core run recorded that failure; the successful build excluded only that test through an external test configuration, without changing repository files.
• The new test class also passed the repository's Checkstyle configuration. Existing renderer-style warnings remain outside this change.

Reproduction: run ./gradlew :jme3-core:test --tests com.jme3.anim.tween.action.BlendableActionCloneTest.

The tests use real animation tracks and scene nodes without a renderer. GPU/rendered-animation tests and the full engine build were not run; LWJGL2 and NiftyUI were excluded.

[jaime-jmebot]

Nice catch on the inner-class binding — creating the tween here correctly rebinds it to the clone, and preserving getLength() keeps the clamped duration.

One small robustness nit: the old cloner.clone(transition) was null-safe (the Cloner returns null for a null input), but this line dereferences transition unconditionally. transition is only created lazily, so cloning a BlendableAction that never had setTransitionLength() called would now throw an NPE. A guard keeps the behavior safe for that path:

[jaime-jmebot]

Concretely, this is equivalent to the old line plus the null check:

Suggested change

	collectTransformDelegate = cloner.clone(collectTransformDelegate);
	transition = cloner.clone(transition);
	// Rebind the inner tween to this action instead of retaining its original owner.
	transition = new TransitionTween(transition.getLength());
	if (transition != null) {
	// Rebind the inner tween to this action instead of retaining its original owner.
	transition = new TransitionTween(transition.getLength());
	}

[toaster0123]

I checked the never-configured case. The transition is created eagerly by the field initializer, before construction finishes. setTransitionLength() only changes that existing tween's duration. The field is private, and neither construction nor cloning assigns it null.

I also tested fresh ClipAction, BlendAction, and a minimal BlendableAction subclass before any setTransitionLength() or interpolation call. Cloning succeeds. The ClipAction and subclass then use the default 0.4-second transition correctly, without changing the source action's weight. All eight regression tests still pass at ab4cb9f.

I'm keeping the current code: the proposed never-configured path already has a non-null transition, so it doesn't need this guard.

[jaime-jmebot]

You're right, and thanks for checking it properly rather than just telling me to add the guard. I was wrong about transition being lazily created — it's assigned in the field initializer, which runs before the constructor body, so it's always non-null. setTransitionLength() only mutates the existing tween, and nothing can assign it back to null.

Good call keeping the code as-is. An always-true guard would just be noise, and the reasoning is documented here for the next reader. Approving.

[jaime-jmebot]

Thanks for the clear write-up — the root-cause analysis was spot on, and the tests are the strongest part of this PR. They pin down the actual reported symptom rather than just the internal symptom, and the isolation assertions (source action untouched, separate max weights) guard against the fix regressing into a different aliasing bug.

Highlights:

• Fix is minimal and correct. Rebinding the inner tween to the clone is the right repair, and preserving getLength() correctly carries over the clamped duration.
• Test coverage is well targeted — cloned playback, isolation, reverse playback, zero duration, and post-clamp cloning give good protection.
• One small nit inline: the new line drops the null-safety that cloner.clone(null) used to give, so a guard would avoid a possible NPE on a never-transitioned action.

No blocking concerns — happy to approve once that guard lands (or if you'd prefer to keep it as-is and follow up separately, just say so and I'll take another look).

[jaime-jmebot]

Checked the field initializer myself — transition is assigned eagerly at declaration, so it's never null and my earlier NPE concern doesn't hold. Nice work verifying that on real subclasses too.

This is a clean, well-scoped fix:

• Root cause is correct. TransitionTween is a non-static inner class, so a copied instance kept writing to the original action's transitionWeight. Rebinding it to the clone is the right repair.
• getLength() is the right value to carry over — it's the clamped duration, so post-clamp clones stay correct.
• Tests are the standout. They assert the user-visible symptom (clone lands at x=2, not x=8) rather than just internal state, and cover isolation, reverse playback, zero duration, and post-clamp cloning. That's solid protection against the fix regressing into a different aliasing bug.

Approved.