Skip to content

Keep blended animation targets and settings with the cloned model #3004

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
toaster0123 wants to merge 2 commits into
base: master
Choose a base branch
from
+510 −3
Open

Keep blended animation targets and settings with the cloned model #3004

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
469fcb5
Clone blended animation targets and default blend-space ownership
toaster0123 Oct 2, 2026
6b21010
Remove non-blend and uncloned animation controls
toaster0123 Oct 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions jme3-core/src/main/java/com/jme3/anim/tween/action/BlendAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
import com.jme3.anim.util.HasLocalTransform;
import com.jme3.math.FastMath;
import com.jme3.math.Transform;
import com.jme3.util.clone.Cloner;
import com.jme3.util.clone.JmeCloneable;

import java.util.Arrays;
import java.util.Collection;
Expand All @@ -44,11 +46,11 @@ public class BlendAction extends BlendableAction {

private int firstActiveIndex;
private int secondActiveIndex;
final private BlendSpace blendSpace;
private BlendSpace blendSpace;
private float blendWeight;
final private double[] timeFactor;
private double[] speedFactors;
final private Map<HasLocalTransform, Transform> targetMap = new HashMap<>();
private Map<HasLocalTransform, Transform> targetMap = new HashMap<>();

public BlendAction(BlendSpace blendSpace, BlendableAction... actions) {
super(actions);
Expand Down Expand Up @@ -203,6 +205,35 @@ public void collectTransform(HasLocalTransform target, Transform t, float weight
}
}

/**
* Resolve the cloned child targets and copy mutable blending state.
* Blend spaces implementing {@link JmeCloneable}, with an existing cloner
* mapping, or with a registered clone function use the same cloner. Other
* spaces remain shared and are not rebound.
*
* @param cloner the cloner that's cloning this action (not null)
* @param original the action from which this action was shallow-cloned
*/
@Override
public void cloneFields(Cloner cloner, Object original) {
super.cloneFields(cloner, original);
Map<HasLocalTransform, Transform> clonedTargets = new HashMap<>();
for (Map.Entry<HasLocalTransform, Transform> entry : targetMap.entrySet()) {
HasLocalTransform target = entry.getKey();
// A custom child action may deliberately retain a non-cloneable target.
if (cloner.isCloned(target)) {
target = cloner.clone(target);
}
clonedTargets.put(target, cloner.clone(entry.getValue()));
}
targetMap = clonedTargets;
speedFactors = cloner.clone(speedFactors);
if (blendSpace instanceof JmeCloneable || cloner.isCloned(blendSpace)
|| cloner.getCloneFunction(blendSpace.getClass()) != null) {
blendSpace = cloner.clone(blendSpace);
}
}

private void collect(HasLocalTransform target, Transform tr) {
if (collectTransformDelegate != null) {
collectTransformDelegate.collectTransform(target, tr, this.getWeight(), this);
Expand Down
30 changes: 29 additions & 1 deletion jme3-core/src/main/java/com/jme3/anim/tween/action/LinearBlendSpace.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package com.jme3.anim.tween.action;

public class LinearBlendSpace implements BlendSpace {
import com.jme3.util.clone.Cloner;
import com.jme3.util.clone.JmeCloneable;

public class LinearBlendSpace implements BlendSpace, JmeCloneable {

@jaime-jmebot jaime-jmebot Oct 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heads-up on a subtle behavior change worth calling out in the description: previously Cloner returned the same LinearBlendSpace instance for every clone, so anyone deliberately sharing one blend space across several models (a common trick to sync them) keeps working — but now each clone silently gets its own copy with its own value. That's the right behavior for this fix, just worth mentioning so it isn't a surprise for existing users.

@jaime-jmebot jaime-jmebot Oct 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more thing to double-check while we're here: cloneFields only rebinds action, and the shallow jmeClone() copies the steps array by reference. So the original and the copy end up sharing the same float[]. That's fine as long as steps is never mutated after construction, but if anything can write into it, the copy would silently corrupt the original — the exact class of bug this PR is fixing. If there's any mutator, dropping final on steps and adding steps = steps.clone(); in cloneFields would keep the state genuinely independent.

@toaster0123 toaster0123 Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an explicit compatibility note to the description: the default LinearBlendSpace now has its own value and owner after cloning. Code that wants synchronized blend values should update each copied action's blend space.

For the second point, this revision has a scalar float step, not a steps array: fields at lines 8–12. setBlendAction recomputes that scalar, and the shallow clone copies it by value. There is no array storage to share or clone here, so no production change is needed.

@jaime-jmebot jaime-jmebot Oct 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, and thanks for the correction — my note was based on a field shape that isn't in this revision. Here it's a plain float step, setBlendAction recomputes it from the actions' length, and the shallow clone copies it by value. Nothing to share, nothing to fix. My earlier "steps array" wording was simply mistaken; the approval stands as-is.

The compatibility note in the description covers the remaining behavior change nicely. 👍


private BlendAction action;
private float value;
Expand All @@ -13,6 +16,31 @@ public LinearBlendSpace(float minValue, float maxValue) {
this.minValue = minValue;
}

/**
* Create a shallow clone for the JME cloner.
*
* @return a new blend space (not null)
*/
@Override
public LinearBlendSpace jmeClone() {
try {
return (LinearBlendSpace) super.clone();
} catch (CloneNotSupportedException exception) {
throw new RuntimeException(exception);
}
}

/**
* Resolve the owning action through the same cloner as this blend space.
*
* @param cloner the cloner that's cloning this blend space (not null)
* @param original the blend space from which this one was shallow-cloned (unused)
*/
@Override
public void cloneFields(Cloner cloner, Object original) {
action = cloner.clone(action);
}

@Override
public void setBlendAction(BlendAction action) {
this.action = action;
Expand Down
Loading