Skip to content

DefaultParticleInfluencer: incorrect cloning of variables #2407

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.

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
wants to merge 8 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2009-2021 jMonkeyEngine
* Copyright (c) 2009-2025 jMonkeyEngine
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
Expand Down Expand Up @@ -40,6 +40,7 @@
import com.jme3.math.FastMath;
import com.jme3.math.Vector3f;
import com.jme3.util.clone.Cloner;

import java.io.IOException;

/**
Expand Down Expand Up @@ -101,13 +102,10 @@ public void read(JmeImporter im) throws IOException {

@Override
public DefaultParticleInfluencer clone() {
try {
DefaultParticleInfluencer clone = (DefaultParticleInfluencer) super.clone();
clone.initialVelocity = initialVelocity.clone();
return clone;
} catch (CloneNotSupportedException e) {
throw new AssertionError();
}
// Set up the cloner for the type of cloning we want to do.
Cloner cloner = new Cloner();
DefaultParticleInfluencer clone = cloner.clone(this);
return clone;
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2009-2012 jMonkeyEngine
* Copyright (c) 2009-2025 jMonkeyEngine
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
Expand Down Expand Up @@ -142,17 +142,6 @@ protected void applyVelocityVariation(Particle particle) {
particle.velocity.addLocal(temp);
}

@Override
public NewtonianParticleInfluencer clone() {
NewtonianParticleInfluencer result = new NewtonianParticleInfluencer();
result.normalVelocity = normalVelocity;
result.initialVelocity = initialVelocity;
result.velocityVariation = velocityVariation;
result.surfaceTangentFactor = surfaceTangentFactor;
result.surfaceTangentRotation = surfaceTangentRotation;
return result;
}

@Override
public void write(JmeExporter ex) throws IOException {
super.write(ex);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2009-2018 jMonkeyEngine
* Copyright (c) 2009-2025 jMonkeyEngine
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
Expand Down Expand Up @@ -42,7 +42,7 @@
* An interface that defines the methods to affect initial velocity of the particles.
* @author Marcin Roguski (Kaelthas)
*/
public interface ParticleInfluencer extends Savable, Cloneable, JmeCloneable {
public interface ParticleInfluencer extends Savable, JmeCloneable {

/**
* This method influences the particle.
Expand All @@ -57,7 +57,7 @@ public interface ParticleInfluencer extends Savable, Cloneable, JmeCloneable {
* This method clones the influencer instance.
* @return cloned instance
*/
public ParticleInfluencer clone();
ParticleInfluencer clone();

/**
* @param initialVelocity
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package com.jme3.effect.influencers;

import com.jme3.asset.AssetManager;
import com.jme3.asset.DesktopAssetManager;
import com.jme3.export.binary.BinaryExporter;
import com.jme3.math.Vector3f;
import org.junit.Assert;
import org.junit.Test;

/**
* Automated tests for the {@code ParticleInfluencer} class.
*
* @author capdevon
*/
public class ParticleInfluencerTest {

/**
* Tests cloning, serialization and de-serialization of a {@code NewtonianParticleInfluencer}.
*/
@Test
public void testNewtonianParticleInfluencer() {
AssetManager assetManager = new DesktopAssetManager(true);

NewtonianParticleInfluencer inf = new NewtonianParticleInfluencer();
inf.setNormalVelocity(1);
inf.setSurfaceTangentFactor(0.5f);
inf.setSurfaceTangentRotation(2.5f);
inf.setInitialVelocity(new Vector3f(0, 1, 0));
inf.setVelocityVariation(2f);

NewtonianParticleInfluencer clone = (NewtonianParticleInfluencer) inf.clone();
assertEquals(inf, clone);
Assert.assertNotSame(inf.temp, clone.temp);

NewtonianParticleInfluencer copy = BinaryExporter.saveAndLoad(assetManager, inf);
assertEquals(inf, copy);
}

private void assertEquals(NewtonianParticleInfluencer inf, NewtonianParticleInfluencer clone) {
Assert.assertEquals(inf.getNormalVelocity(), clone.getNormalVelocity(), 0.001f);
Assert.assertEquals(inf.getSurfaceTangentFactor(), clone.getSurfaceTangentFactor(), 0.001f);
Assert.assertEquals(inf.getSurfaceTangentRotation(), clone.getSurfaceTangentRotation(), 0.001f);
Assert.assertEquals(inf.getInitialVelocity(), clone.getInitialVelocity());
Assert.assertEquals(inf.getVelocityVariation(), clone.getVelocityVariation(), 0.001f);
}

/**
* Tests cloning, serialization and de-serialization of a {@code RadialParticleInfluencer}.
*/
@Test
public void testRadialParticleInfluencer() {
AssetManager assetManager = new DesktopAssetManager(true);

RadialParticleInfluencer inf = new RadialParticleInfluencer();
inf.setHorizontal(true);
inf.setOrigin(new Vector3f(0, 1, 0));
inf.setRadialVelocity(2f);
inf.setInitialVelocity(new Vector3f(0, 1, 0));
inf.setVelocityVariation(2f);

RadialParticleInfluencer clone = (RadialParticleInfluencer) inf.clone();
assertEquals(inf, clone);
Assert.assertNotSame(inf.temp, clone.temp);
Assert.assertNotSame(inf.getOrigin(), clone.getOrigin());

RadialParticleInfluencer copy = BinaryExporter.saveAndLoad(assetManager, inf);
assertEquals(inf, copy);
}

private void assertEquals(RadialParticleInfluencer inf, RadialParticleInfluencer clone) {
Assert.assertEquals(inf.isHorizontal(), clone.isHorizontal());
Assert.assertEquals(inf.getOrigin(), clone.getOrigin());
Assert.assertEquals(inf.getRadialVelocity(), clone.getRadialVelocity(), 0.001f);
Assert.assertEquals(inf.getInitialVelocity(), clone.getInitialVelocity());
Assert.assertEquals(inf.getVelocityVariation(), clone.getVelocityVariation(), 0.001f);
}

}