Fix SkeletonDebugger deserializing

[tonihele]

Resolves #2226

Prevents NPE after deserializing SkeletonDebugger. The solution is not that pretty. But it works. The children are written/read properly, this just then finds them by the fixed names set.

[tonihele]

Here is the code I used to test this. The model is here https://sketchfab.com/3d-models/silver-dragonkin-mir4-89ead4e87cdc4b70840f748383f0998f, get it as FBX:

package jme3test.scene;

import com.jme3.app.SimpleApplication;
import com.jme3.asset.plugins.FileLocator;
import com.jme3.asset.plugins.HttpZipLocator;
import com.jme3.asset.plugins.ZipLocator;
import com.jme3.export.binary.BinaryExporter;
import com.jme3.light.AmbientLight;
import com.jme3.light.DirectionalLight;
import com.jme3.math.ColorRGBA;
import com.jme3.math.Vector3f;
import com.jme3.scene.Geometry;
import com.jme3.scene.Spatial;
import com.jme3.scene.shape.Sphere;
import com.jme3.util.SkyFactory;
import java.io.File;
import java.io.IOException;

public class TestSceneLoading extends SimpleApplication {

    final private Sphere sphereMesh = new Sphere(32, 32, 10, false, true);
    final private Geometry sphere = new Geometry("Sky", sphereMesh);
    private static boolean useHttp = false;

    public static void main(String[] args) {
     
        TestSceneLoading app = new TestSceneLoading();
        app.start();
    }

    @Override
    public void simpleUpdate(float tpf){
        sphere.setLocalTranslation(cam.getLocation());
    }

    @Override
    public void simpleInitApp() {
        File file = new File("wildhouse.zip");
        if (!file.exists()) {
            useHttp = true;
        }

        assetManager.registerLocator("C:\\temp\\dragon\\source", FileLocator.class);
        
        this.flyCam.setMoveSpeed(10);

        // load sky
        rootNode.attachChild(SkyFactory.createSky(assetManager, 
                "Textures/Sky/Bright/BrightSky.dds", 
                SkyFactory.EnvMapType.CubeMap));
        
        // create the geometry and attach it
        // load the level from zip or http zip
/*        if (useHttp) {
            assetManager.registerLocator("https://storage.googleapis.com/google-code-archive-downloads/v2/code.google.com/jmonkeyengine/wildhouse.zip", HttpZipLocator.class);
        } else {
            assetManager.registerLocator("wildhouse.zip", ZipLocator.class);
        }*/
        Spatial scene = assetManager.loadModel("Mon_BlackDragon31_Skeleton.FBX");
        BinaryExporter exporter = BinaryExporter.getInstance();
        File f = new File("C:\\temp\\dragon\\source\\MyModel.j3o");
        try {
            exporter.save(scene, f);
        } catch (IOException ex) {

        }

        scene = assetManager.loadModel("MyModel.j3o");

        AmbientLight al = new AmbientLight();
        scene.addLight(al);

        DirectionalLight sun = new DirectionalLight();
        sun.setDirection(new Vector3f(0.69077975f, -0.6277887f, -0.35875428f).normalizeLocal());
        sun.setColor(ColorRGBA.White.clone().multLocal(2));
        scene.addLight(sun);

        rootNode.attachChild(scene);
    }
}

[capdevon]

Hi @tonihele ,
you are right, I downloaded the dragon model and ran your code on my local pc getting the following error.

java.lang.NullPointerException
	at com.jme3.scene.debug.SkeletonDebugger.updateLogicalState(SkeletonDebugger.java:99)
	at com.jme3.scene.Node.updateLogicalState(Node.java:239)
	at com.jme3.app.SimpleApplication.update(SimpleApplication.java:268)
	at com.jme3.system.lwjgl.LwjglWindow.runLoop(LwjglWindow.java:631)
	at com.jme3.system.lwjgl.LwjglWindow.run(LwjglWindow.java:721)
	at java.base/java.lang.Thread.run(Thread.java:834)

Effectively jme's stock FBXLoader still uses the old animation system. The strange thing is that it adds the SkeletonDebugger to the model after loading it.

https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-plugins/src/fbx/java/com/jme3/scene/plugins/fbx/node/FbxNode.java#L525

...
        if (fbxNode.skeleton != null) { 
            jmeSpatial.addControl(new AnimControl(fbxNode.skeleton));
            jmeSpatial.addControl(new SkeletonControl(fbxNode.skeleton));
            
// comment out this code
            /*SkeletonDebugger sd = new SkeletonDebugger("debug", fbxNode.skeleton);
            Material mat = new Material(fbxNode.assetManager, "Common/MatDefs/Misc/Unshaded.j3md");
            mat.getAdditionalRenderState().setWireframe(true);
            mat.getAdditionalRenderState().setDepthTest(false);
            mat.setColor("Color", ColorRGBA.Green);
            sd.setMaterial(mat);
            
            ((Node)jmeSpatial).attachChild(sd);*/
        }

I don't know if the SkeletonDebugger is meant to be serialized or to be used for debugging purposes only in test applications. The solutions could be either to comment out the SkeletonDebugger from the FBXLoader or to fix the SkeletonDebugger as you did.

[tonihele]

I don't know if the SkeletonDebugger is meant to be serialized or to be used for debugging purposes only in test applications. The solutions could be either to comment out the SkeletonDebugger from the FBXLoader or to fix the SkeletonDebugger as you did.

SkeletonDebugger is serializable. So IMO it should work, regardless what FBXLoader or anyone else thinks of it. We have no way of deducting how it will be used but it should work flawlessly as it promises to.

FBXLoader is another issue. Yes, it feels that it should not be in the master branch yet since it seems to use some debug data. It should not probably use this SkeletonDebugger. There is also another issue about the FBXLoader using the old animation system.

[capdevon]

SkeletonDebugger is serializable. So IMO it should work, regardless what FBXLoader or anyone else thinks of it. We have no way of deducting how it will be used but it should work flawlessly as it promises to.

I agree with you.