Spatial.clone breaks rigidbodies

[riccardobl]

It seems that the user object is never set for cloned rigidbodies.

Test code:

import com.jme3.app.SimpleApplication;
import com.jme3.bullet.BulletAppState;
import com.jme3.bullet.control.RigidBodyControl;
import com.jme3.material.Material;
import com.jme3.scene.Geometry;
import com.jme3.scene.shape.Box;

public class Test  extends SimpleApplication{
    public static void main(String[] args) {
        Test t=new Test();
        t.setShowSettings(false);
        t.start();
    }   

    Geometry cloned;
    Geometry original;
    float DELAY=2.f;

    @Override
    public void simpleInitApp() {
        BulletAppState bas=new BulletAppState();
        stateManager.attach(bas);        
        Box b=new Box(10,10,10);
        Material mat=new Material(assetManager,"Common/MatDefs/Misc/Unshaded.j3md");

        original=new Geometry("Test",b);
        original.setMaterial(mat);
        original.addControl(new RigidBodyControl(0));
        rootNode.attachChild(original);
        bas.getPhysicsSpace().addAll(original);

        cloned=original.clone();
        bas.getPhysicsSpace().addAll(cloned);
        rootNode.attachChild(cloned);

    }

    @Override
    public void simpleUpdate(float tpf){
        DELAY-=tpf;
        if(DELAY<=0){
            DELAY=2.f;
            if(cloned.getControl(RigidBodyControl.class).getUserObject()==null){
                System.out.println("/!\\ Cloned RigidBody has null user object");
            }else{
                System.out.println("Everything is fine");
            }
        }
    }
}

Full gradle project: http://bit.ly/2APrAGN

$ gradle execute
[...]
/!\ Cloned RigidBody has null user object
/!\ Cloned RigidBody has null user object
/!\ Cloned RigidBody has null user object
/!\ Cloned RigidBody has null user object

[pspeed42]

I don't know bullet well but the getUserObject() name implies that this would be a user object you'd supply by calling setUserObject()... which I don't see anywhere in the code. Can you confirm that your original.getUserObject() is not null?

It's possible that it's not cloned properly but your example wouldn't show this, I think.

[pspeed42]

Oh, I see... it should be the spatial.

In that case, why not just get the spatial? (I mean, it's a bug but it seems like a simple workaround to me.)

[riccardobl]

Yes indeed, getUserObject in com.jme3.bullet.control.* classes could just return spatial.

It used to be set from setSpatial, but the new cloning doesn't call it anymore, i wonder if a similiar issue could affect other controls that relies on the old behaviour 🤔

[pspeed42]

I think PhysicsObject or whatever just needs to override cloneFields() and do the right thing with its local fields.

[Nehon]

The KinematicRagdollControl put something that is not a spatial in userObject. Why not just call setUserObject when cloning the controls? here for example https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-bullet/src/common/java/com/jme3/bullet/control/RigidBodyControl.java#L154 So that each control manages what should go in user object

[thoced]

A PR is proposed with a correction. Can you check if this seems correct ?

[stephengold]

I'm in agreement with @pspeed42 that PhysicsCollisionObject should override cloneFields(). Unfortunately, PhysicsCollisionObject currently does not implement JmeCloneable.

If RigidBodyControl implements JmeCloneable, then so should PhysicsRigidBody (both versions) and PhysicsCollisionObject (both versions).

And since the user object could be either a Spatial or a PhysicsBoneLink, then PhysicsBoneLink should also implement JmeCloneable so that its userObject field can be cloned in PhysicsCollisionObject.cloneFields().