Questions about clone() and cloneFields()

Development Engine Development
1

Hey everyone,

Lately, I’ve been looking into how jMonkeyEngine objects are cloned, and it seems there might be some inconsistencies. It looks like some classes are mixing native Java cloning with the Cloner utility which uses cloneFields() method.

While I haven’t had a chance to dig into every class yet, I suspect there might be some issues with DefaultParticleInfluencer and AudioNode. What do you guys think?

Okay, so diving a bit deeper:

It appears that the AudioNode class calls its superclass Spatial’s clone() method. This clone() method then utilizes the Cloner object to clone the object’s fields by calling cloneFields(). However, this cloning process currently fails because the LowPassFilter class isn’t properly set up for cloning – definitely a bug to address.

Similarly, in the case of DefaultParticleInfluencer, it seems to be calling the native clone() method from the Object superclass. Because of this, the temp Vector3f variable isn’t being cloned at that stage. It looks like the temp variable is cloned later on when the cloneFields() method of the Cloner is used. This discrepancy also seems like a bug that needs fixing.

Can you confirm my analysis?

2

DefaultParticleInfluencer’s clone method is legitimately busted. It should be probably be using the cloner like other classes… especially since it’s already setup for it.

But note: this issue only affects code that is directly cloning a DefaultParticleInfluencer. Anything cloning the actual Geometry/ParicleEmitter/etc that holds it will be doing it through the JME cloner which will never call this particular clone method.

3

this is the current implementation

    @Override
    public DefaultParticleInfluencer clone() {
        try {
            DefaultParticleInfluencer clone = (DefaultParticleInfluencer) super.clone();
            clone.initialVelocity = initialVelocity.clone();
            return clone;
        } catch (CloneNotSupportedException e) {
            throw new AssertionError();
        }
    }

the correct one should be this one, right? I tested it and it works.

    @Override
    public DefaultParticleInfluencer clone() {
            // Set up the cloner for the type of cloning we want to do.
            Cloner cloner = new Cloner();
            DefaultParticleInfluencer clone = cloner.clone(this);
            return clone;
    }
4

Looks right to me.

Note that this unlikely to affect anyone… so while it’s definitely good to fix it, it shouldn’t hold up any releases or anything. (in case it comes up)

1 Like
5

ok, thanks for the feedback, @pspeed. I’ve addressed the first issue with ParticleInfluencers. I’ll leave it to @yaRnMcDonuts to decide if it should be included in the current or the next engine release. Now, I’m going to shift my focus to the AudioNode issue related to the LowPassFilter. I’ll keep you updated on my progress with that. :wink:

1 Like
6

@pspeed ,
I’m running into a bit of a roadblock with the AudioNode cloning issue. There seems to be some overlap and confusion between the different clone() methods and the Cloner utility, making it tricky to pinpoint the right solution. Do you have any thoughts or suggestions on how to best approach this?

Here’s the test case:

7

Approach what? The line of code that was highlighted doesn’t tell me what the issue is. Seems right to me.

Is the issue still that LowPassFilter is not cloneable? It can’t be cloned unless it’s cloneable. Because only Cloneable things can be cloned.

If you need to copy the object because it’s impossible to clone it then it needs to be copied and not cloned.

Edit: though note that I think things can be “JME cloneable” in the sense that they will work with the cloner even if they are not “Java Cloneable”… but then it needs to implement the jmeClone() method to make a copy.

8
1 Like