Node.attachChild(at) with keeping the current worldtranslation

archive contribution-depot-jme2
1

Hola, I have to problem that I want to hand over a Spatial from one to another node. As

this Spatial's localtranslation is not (0,0,0) the effect is that you see of course the effect as

the worldtranslation in another branch of the scenegraph is of course not the same.



To avoid this I added a method attachChild(Spatial sp,boolean keepWorldTranslation) and

attachChildAt(Spatial,Int,boolean)



The old behavior of attachChild(Sp) and attachChildAt(Sp) stays the same as before.



Here is the Patch: (A test comes at the bottom of the post)


### Eclipse Workspace Patch 1.0
#P jme2
Index: src/com/jme/scene/Node.java
===================================================================
--- src/com/jme/scene/Node.java (revision 4780)
+++ src/com/jme/scene/Node.java (working copy)
@@ -40,11 +40,11 @@
import java.util.Stack;
import java.util.logging.Level;
import java.util.logging.Logger;
-
import com.jme.bounding.BoundingVolume;
import com.jme.intersection.CollisionResults;
import com.jme.intersection.PickResults;
import com.jme.math.Ray;
+import com.jme.math.Vector3f;
import com.jme.renderer.Renderer;
import com.jme.scene.state.RenderState;
import com.jme.util.export.JMEExporter;
@@ -77,6 +77,7 @@
        setCollisionMask(-1,false);
    }

+
    /**
      * Constructor instantiates a new <code>Node</code> with a default empty
      * list for containing children.
@@ -156,9 +157,34 @@
      * @return the number of children maintained by this node.
      */
    public int attachChild(Spatial child) {
-        if (child != null) {
+    return attachChild(child,false);
+    }
+
+   
+    /**
+    *
+    * <code>attachChild</code> attaches a child to this node. This node
+    * becomes the child's parent. The current number of children maintained is
+    * returned.
+    * <br>
+    * If the child already had a parent it is detached from that former parent.
+    *
+    * @param child
+    *            the child to attach to this node.
+    * @param keepWorldTranslation
+    *            modifies the local-translation that the world-translation stays the same
+    * @return the number of children maintained by this node.
+    */   
+    public int attachChild(Spatial child,boolean keepWorldTranslation) {
+    if (child != null) {
            if (child.getParent() != this) {
-                if (child.getParent() != null) {
+                Vector3f lastWorldTranslation=null;
+            if (child.getParent() != null) {
+                if (keepWorldTranslation)
+                {
+                child.updateWorldTranslation();
+                lastWorldTranslation = child.getWorldTranslation().clone();
+                }
                    child.getParent().detachChild(child);
                }
                child.setParent(this);
@@ -166,6 +192,10 @@
                    children = Collections.synchronizedList(new ArrayList<Spatial>(1)); 
                }
                children.add(child);
+                if (lastWorldTranslation!=null)
+                {
+                child.setLocalTranslation(worldToLocal(lastWorldTranslation, null));
+                }
                logger.log(Level.FINE,
                        "Child "{0}" attached to this node "{1}"",
                        new String[] {child.getName(), getName()});
@@ -189,16 +219,43 @@
      * @return the number of children maintained by this node.
      */
    public int attachChildAt(Spatial child, int index) {
+    return attachChildAt(child,index,false);
+    }
+
+    /**
+    *
+    * <code>attachChildAt</code> attaches a child to this node at an index. This node
+    * becomes the child's parent. The current number of children maintained is
+    * returned.
+    * <br>
+    * If the child already had a parent it is detached from that former parent.
+    *
+    * @param child
+    *            the child to attach to this node.
+    * @param keepWorldTranslation
+    *            modifies the local-translation that the world-translation stays the same
+    * @return the number of children maintained by this node.
+    */
+    public int attachChildAt(Spatial child, int index, boolean keepWorldTranslation) {
        if (child != null) {
            if (child.getParent() != this) {
-                if (child.getParent() != null) {
+            Vector3f lastWorldTranslation=null;
+            if (child.getParent() != null) {
+                if (keepWorldTranslation)
+                {
+                child.updateWorldTranslation();
+                lastWorldTranslation = child.getWorldTranslation().clone();
+                }
                    child.getParent().detachChild(child);
                }
-                child.setParent(this);
                if(children == null) {
                    children = Collections.synchronizedList(new ArrayList<Spatial>(1)); 
                }
                children.add(index, child);
+                if (lastWorldTranslation!=null)
+                {
+                child.setLocalTranslation(worldToLocal(lastWorldTranslation, null));
+                }           
                logger.log(Level.FINE,
                        "Child "{0}" attached to this node "{1}"",
                        new String[] {child.getName(), getName()});


And here is a test to show the effect with and without keepWorldTranslation. There are two Nodes,one box and one sphere.
There are 3 states.
1)box and sphere are attachedTo the rootNode
2)attachedTo(node1) where the box is attached with keepWorldTranslation
3)attachedTo(node2) where the box is attached with keepWorldTranslation

You can jump from state to state by hitting space:


import com.jme.app.SimpleGame;
import com.jme.input.KeyBindingManager;
import com.jme.input.KeyInput;
import com.jme.input.MouseInput;
import com.jme.math.Vector3f;
import com.jme.renderer.ColorRGBA;
import com.jme.scene.Node;
import com.jme.scene.shape.Box;
import com.jme.scene.shape.Sphere;

public class TestAttach extends SimpleGame {

   private int state = 0;
   
   Box b1;
   Sphere b2;
   Node b_parent1,b_parent2;
   
   @Override
   protected void simpleInitGame() {
      MouseInput.get().setCursorVisible(true);
      b1 = new Box("b",Vector3f.ZERO,Vector3f.UNIT_XYZ);
      b1.getLocalTranslation().addLocal(-5,5,0);
      b2 = new Sphere("b2",16,16,1);
      b2.getLocalTranslation().addLocal(-5,5,0);
      b2.setDefaultColor(ColorRGBA.blue);
      b_parent2 = new Node();
      b_parent2.getLocalTranslation().addLocal(5,0,0);
      b_parent1 = new Node();
      b_parent1.getLocalTranslation().addLocal(-5,0,0);

      rootNode.attachChild(b1);
      rootNode.attachChild(b2);
      rootNode.attachChild(b_parent1);
      rootNode.attachChild(b_parent2);
      
      KeyBindingManager.getKeyBindingManager().set("next", KeyInput.KEY_SPACE);
      
   }
   
   public void nextState()
   {
      state++;
      if (state>2)
         state=0;
      if (state==0)
      {
         rootNode.attachChild(b1,true);
         rootNode.attachChild(b2);
         System.out.println("NO PARENT!!"+" "+b1.getWorldTranslation());
      }
      else if (state==1)
      {
         b_parent1.attachChild(b1,true);
         b_parent1.attachChild(b2);
         System.out.println("b_p1 parent"+" "+b1.getWorldTranslation());
      }
      else if (state==2)
      {
         b_parent2.attachChild(b1,true);
         b_parent2.attachChild(b2);
         System.out.println("b_p2 parent"+" "+b1.getWorldTranslation());
      }
   }

   @Override
   protected void simpleUpdate() {
      super.simpleUpdate();
      if (KeyBindingManager.getKeyBindingManager().isValidCommand("next",false))
      {
         nextState();
      }
   }

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

}

2

IMO the large addition of code to a very basic class isn't justified by this far-from-common use case.  It's idiosyncratic in that you are adding a translation-specific parameter to this otherwise general method signature.  Why should there be a general demand to preserve world translations but no such demand to preserve world scales or rotations?  Case-in-point:  A Node with non-default translation+scale+rotation, which is parent to your model with relative translation+scale+rotation.  Upon re-attachment, one would expect either the world translation+scale+rotation to be preserved or to be lost, but not for translation to be preserved but not scale+rotation.



Concern 1:  If you're going to generalize attachment methods to be able to preserve world states, then do it generally.



Concern 2:  Large additions to core classes must be justified by either general demand or something that will benefit a large proportion of users.  I don't see either justification here.

3

What I think would be much more unobtrusive and general than this would be a general callback feature for attaching.  Something like



    public interface PreAttachAction {

        public void preAttach(Node newParent, Node newChild);



and



    public class Node…

        public void addPreAttachAction(PreAttachAction newAttachAction) {…



and same for PostAttachActions.  This supports use cases like yours, to automatically restore world states; as well as for game scene management (like to update lists of tracked Spatials of various types).  This would cost about 10 lines of code in Node and 2 tiny interfaces to the code base, but support more use cases.



I would have added something like this long ago, but one should be conservative about adding stuff to core classes, and I don't know if the average user would appreciate the benefits.

4

I actually see the demand (as it is there for me) If you can provide another (better?) way

go for it. I would appreciate it.


Upon re-attachment, one would expect either the world translation+scale+rotation to be preserved or to be lost, but not for translation to be preserved but not scale+rotation.


Yes, you are right! This should also be preserved and taken in account (forgot about it).

Ok...no problem for me. I thought it would be a good feature and wanted to share it with the community. But as you said without scale and rotation it is worthless.