Keep cloned tween sequences on their copied actions

[toaster0123]

Problem

Playing a cloned model's registered tween sequence can animate the original model instead.

Reproduction: create two clip actions with AnimComposer.actionSequence, clone the model, explicitly select the sequence on the copied composer, and advance it by half a second. In the regression, the original target moves to x=5 and the copied target stays at x=0. The expected result is the reverse.

BaseAction clones its collected child actions, but its wrapped tween still refers to the original graph. Fixing Sequence alone is insufficient: the same problem occurs when the sequence contains built-in parallel, stretch, loop, invert, or curve wrappers.

Fix

Clone the wrapped graph and playback state for all six built-in container types: Sequence, Parallel, Stretch, Loop, Invert, and Curve.

The copied containers use the same Cloner as their actions. They preserve shared delegate-array identities, copy Parallel's mutable completion flags, and honor existing mappings and registered clone functions, including array policies.

This changes cloning, not the interpolation algorithms.

Custom tweens with JME cloning support or explicit Cloner policies participate. This includes inherited AbstractTween cloning, which shallow-copies subclass fields; referenced targets remain shared unless cloneFields or a Cloner policy remaps them. An override is not required simply to preserve field values. Other custom tweens retain existing sharing. Callback recipients and argument payloads also retain their existing references; this patch does not infer how application callbacks should be retargeted. Tween and ContainsTweens retain their existing interface requirements.

Null values introduced by later delegate-array mutation or explicit Cloner mappings are preserved during cloning rather than causing a new clone-time exception. This does not make null delegates valid for playback.

Validation

• The original 72-case suite gives 65 failures and 7 passing controls on upstream, then 72/72 passes after the container repair
• The review adds 21 cases for inherited custom-tween fields, explicit target remapping, and null policies. All 93 cases pass on the final head; the initial published head af6357b passes 85 and fails the eight new null-handling regressions
• Tests cover each container, nested wrappers, copied-model playback before/during/later clips, interleaved playback, cursor/completion-state independence, array aliases, explicit Cloner mappings/functions, and custom/callback sharing controls
• Exact Fix animation transitions on cloned models #2992 composition passes 101/101, including its eight unchanged transition regressions
• Independent source review and standalone/composed fixture replays found no blocker; all six public wrapper probes animate the copy while leaving the source target unchanged
• Final core/desktop/effects/plugins check tasks pass: 650 reported tests, zero failures/errors, two existing/environment skips. The earlier container-only head also passed core build; Javadoc generation was not repeated for the review refinement
• Both new test files have zero Checkstyle warnings; no new production diagnostics

Scope

Based directly on upstream master. The transition-owner repair in #2992 and blended-action correction in #3004 are separate, tested compositions rather than bundled changes. Existing Curve mask discovery is unchanged.

Validation is CPU animation/transform and clone-state testing, not GPU or rendered-pixel certification. Unsupported custom tweens are not promised automatic deep cloning.

The skips are the existing FastMath counterclockwise test and a desktop noexec-filesystem assumption. The local whole-repository Android build remains unverified because the SDK is unavailable.

[jaime-jmebot]

Thanks for taking the time on this — routing the six built-in containers through the same Cloner used for the actions (instead of hand-rolled copies) is a clean shape, and preserving shared delegate arrays / Parallel's completion flags keeps playback state per-copy.

Before this goes in, one thing I'd like you to check:

• instanceof JmeCloneable is a very permissive opt-in here. AbstractTween itself implements Cloneable/JmeCloneable and its cloneFields is an explicit no-op, so the test matches basically every tween built on it. For a custom tween that extends AbstractTween, keeps its target in a field, and doesn't override cloneFields, Cloner.clone() will hand back a fresh, field-less instance — previously the original was simply shared, so those tweens worked on cloned models. Could you either narrow the condition so only tweens that genuinely opt in are copied, or add a regression test that clones a model using such a minimal custom tween and asserts the copy still animates?
• Small hardening: tween.getClass() will NPE if a container was built with a null delegate entry — a tween != null && guard keeps that failure out of clone time (where it would now surface) and leaves it where it belongs, at interpolation.
• Docs: the description says custom tweens keep existing sharing, but the practical rule is "copied only if they override cloneFields or are registered with the Cloner". Worth one javadoc line on Tween/Cloneable so users aren't surprised.

Tests around aliasing, cursor independence and nested wrappers look like exactly the right coverage.

[toaster0123]

@jaime-jmebot Addressed this in bcf7a5b.

The minimal AbstractTween subclass does retain its fields: jmeClone calls super.clone, so an inherited no-op cloneFields leaves a shallow copy, not an uninitialized instance. Added paired model-cloning tests for both direct BaseAction and actionSequence:

• The inherited implementation retains target, payload, duration and scalar values, and still executes its target operation. Its target reference stays shared, as it did before this PR
• An explicit cloneFields target remap animates the copied model's own target and leaves the original unchanged

The Javadocs now state that distinction. An override is needed when referenced fields should be cloned/remapped, not merely to preserve their values, so I kept the existing JmeCloneable gate.

Direct null construction already fails, but I reproduced the reachable cases through later delegate-array mutation and explicit null Cloner mappings. Null guards now cover the delegate, delegate-array and BaseAction paths, including cloning a null-mapped result again. Null remains invalid for interpolation; cloning no longer fails earlier on these paths.

The final focused suite passes 93/93; the prior published head passes 85 and fails the eight null regressions. Exact #2992 composition passes 101/101. Final core/desktop/effects/plugins checks pass with 650 tests, zero failures and two existing/environment skips; independent review found no blocker.

[jaime-jmebot]

That answers all three, thanks — checking the new head:

• instanceof JmeCloneable permissiveness — you're right, and I was too pessimistic. jmeClone() calling super.clone() means an inherited no-op cloneFields leaves a shallow copy, not a field-less instance. What I actually care about was "does a minimal custom tween still work on a cloned model?", and minimalCustomTweenStillRunsAgainstItsSharedTarget now locks that in for both the direct BaseAction and the actionSequence path. The paired RetargetedTween test is a nice touch — it shows the opt-in boundary from the other side.
• Null guard — tween != null && (...) in BaseAction covers it, and cloneTheNullMappedResultAgainIsSafe (setClonedValue → clone → Cloner.deepClone again) is the exact case I was worried about. Keeping assertThrows(NullPointerException.class, () -> ...interpolate(0.25)) as the documented end state reads well — clone stops being the place that complains.
• Docs — good, the "override to clone/remap, not just to preserve values" phrasing is the right distinction to put in writing.

One note on process rather than code: no CI checks are reporting on this head from my side, so I'm going off your local runs here. If you flip this out of draft, it'd be worth having the GitHub build confirm the 93/93 before merge.

Otherwise this looks good to me — no blockers from my side. The aliasing and cursor-independence coverage is the part I'm most glad to see, since that was the part this fix could most easily have broken.

[jaime-jmebot]

My three earlier points are resolved in bcf7a5b, so I have nothing blocking here.

• The JmeCloneable gate reads as more permissive than it is — jmeClone() calls super.clone(), so an inherited no-op cloneFields gives a shallow copy rather than a blank instance. The paired minimal-subclass tests now pin that down, and the cloneFields remap test shows the opt-in boundary from the other side.
• The tween != null && guard is in, and the null-mapped-result re-clone case is covered with the NPE correctly deferred to interpolate.
• Javadoc now distinguishes "override to clone/remap" from "override to preserve values."

Worth having the GitHub build confirm the 93/93 when this leaves draft — I'm not seeing checks on this head, so this is based on the local runs you reported.

Aliasing and cursor-independence coverage was the part I most wanted to see, and it landed well.