Modified to use the new SafeArrayList for children and controls.

This means it is possible for a control to modify its own
node hierarchy in a way that might have caused random skipping
at best and index out of bounds exceptions at worse.
Also, users can iterate over children and detach them at the
same time using standard for each constructs.
Performance is the same for me though I'd expected it to be
at least slightly faster given that most inner loops now use
direct array access.  My scenes must not exploit this much.


git-svn-id: https://jmonkeyengine.googlecode.com/svn/trunk@7857 75d07b2b-3a1a-0410-a2c5-0572b91ccdca

engine/src/core/com/jme3/scene/AssetLinkNode.java

@@ -40,6 +40,7 @@ import com.jme3.export.JmeExporter;
 import com.jme3.export.JmeImporter;
 import com.jme3.export.OutputCapsule;
 import com.jme3.export.binary.BinaryImporter;
+import com.jme3.util.SafeArrayList;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -177,8 +178,8 @@ public class AssetLinkNode extends Node {
 
     @Override
     public void write(JmeExporter e) throws IOException {
-        ArrayList<Spatial> childs = children;
-        children = new ArrayList<Spatial>();
+        SafeArrayList<Spatial> childs = children;
+        children = new SafeArrayList<Spatial>(Spatial.class);
         super.write(e);
         OutputCapsule capsule = e.getCapsule(this);
         capsule.writeSavableArrayList(assetLoaderKeys, "assetLoaderKeyList", null);

engine/src/core/com/jme3/scene/Node.java

@@ -39,6 +39,7 @@ import com.jme3.export.JmeExporter;
 import com.jme3.export.JmeImporter;
 import com.jme3.export.Savable;
 import com.jme3.material.Material;
+import com.jme3.util.SafeArrayList;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
@@ -65,7 +66,7 @@ public class Node extends Spatial implements Savable {
     /** 
      * This node's children.
      */
-    protected ArrayList<Spatial> children = new ArrayList<Spatial>(1);
+    protected SafeArrayList<Spatial> children = new SafeArrayList<Spatial>(Spatial.class);
 
     /**
      * Serialization only. Do not use.
@@ -99,7 +100,7 @@ public class Node extends Spatial implements Savable {
     @Override
     protected void setTransformRefresh(){
         super.setTransformRefresh();
-        for (Spatial child : children){
+        for (Spatial child : children.getArray()){
             if ((child.refreshFlags & RF_TRANSFORM) != 0)
                 continue;
 
@@ -110,7 +111,7 @@ public class Node extends Spatial implements Savable {
     @Override
     protected void setLightListRefresh(){
         super.setLightListRefresh();
-        for (Spatial child : children){
+        for (Spatial child : children.getArray()){
             if ((child.refreshFlags & RF_LIGHTLIST) != 0)
                 continue;
 
@@ -121,11 +122,11 @@ public class Node extends Spatial implements Savable {
     @Override
     protected void updateWorldBound(){
         super.updateWorldBound();
+        
         // for a node, the world bound is a combination of all it's children
         // bounds
         BoundingVolume resultBound = null;
-        for (int i = 0, cSize = children.size(); i < cSize; i++) {
-            Spatial child = children.get(i);
+        for (Spatial child : children.getArray()) {
             // child bound is assumed to be updated
             assert (child.refreshFlags & RF_BOUND) == 0;
             if (resultBound != null) {
@@ -145,11 +146,11 @@ public class Node extends Spatial implements Savable {
     public void updateLogicalState(float tpf){
         super.updateLogicalState(tpf);
 
-        // FIXME: Iterating through the children list backwards
-        // to avoid IndexOutOfBoundsException. This is sometimes unreliable,
-        // a more robust solution is needed.
-        for (int i = children.size()-1; i >= 0; i--){
-            Spatial child = children.get(i);
+        if (children.isEmpty()) {
+            return;
+        }
+        
+        for (Spatial child : children.getArray()) {
             child.updateLogicalState(tpf);
         }
     }
@@ -166,15 +167,16 @@ public class Node extends Spatial implements Savable {
             updateWorldTransforms();
         }
 
-        // the important part- make sure child geometric state is refreshed
-        // first before updating own world bound. This saves
-        // a round-trip later on.
-        // NOTE 9/19/09
-        // Although it does save a round trip,
-        for (int i = 0, cSize = children.size(); i < cSize; i++) {
-            Spatial child = children.get(i);
-            child.updateGeometricState();
-        }
+        if (!children.isEmpty()) {
+            // the important part- make sure child geometric state is refreshed
+            // first before updating own world bound. This saves
+            // a round-trip later on.
+            // NOTE 9/19/09
+            // Although it does save a round trip,
+            for (Spatial child : children.getArray()) {
+                child.updateGeometricState();
+            }
+        }            
 
         if ((refreshFlags & RF_BOUND) != 0){
             updateWorldBound();
@@ -428,8 +430,7 @@ public class Node extends Spatial implements Savable {
         if (name == null) 
             return null;
 
-        for (int x = 0, cSize = getQuantity(); x < cSize; x++) {
-            Spatial child = children.get(x);
+        for (Spatial child : children.getArray()) {
             if (name.equals(child.getName())) {
                 return child;
             } else if(child instanceof Node) {
@@ -454,8 +455,7 @@ public class Node extends Spatial implements Savable {
         if (children.contains(spat))
             return true;
 
-        for (int i = 0, max = getQuantity(); i < max; i++) {
-            Spatial child =  children.get(i);
+        for (Spatial child : children.getArray()) {
             if (child instanceof Node && ((Node) child).hasChild(spat))
                 return true;
         }
@@ -483,14 +483,14 @@ public class Node extends Spatial implements Savable {
     @Override
     public void setLodLevel(int lod){
         super.setLodLevel(lod);
-        for (int i = 0; i < children.size(); i++){
-            children.get(i).setLodLevel(lod);
+        for (Spatial child : children.getArray()) {
+            child.setLodLevel(lod);
         }
     }
 
     public int collideWith(Collidable other, CollisionResults results){
         int total = 0;
-        for (Spatial child : children){
+        for (Spatial child : children.getArray()){
             total += child.collideWith(other, results);
         }
         return total;
@@ -575,7 +575,7 @@ public class Node extends Spatial implements Savable {
     @Override
     public Spatial deepClone(){
         Node nodeClone = (Node) super.clone();
-        nodeClone.children = new ArrayList<Spatial>();
+        nodeClone.children = new SafeArrayList<Spatial>(Spatial.class);
         for (Spatial child : children){
             Spatial childClone = child.deepClone();
             childClone.parent = nodeClone;
@@ -587,7 +587,7 @@ public class Node extends Spatial implements Savable {
     @Override
     public void write(JmeExporter e) throws IOException {
         super.write(e);
-        e.getCapsule(this).writeSavableArrayList(children, "children", null);
+        e.getCapsule(this).writeSavableArrayList(new ArrayList(children), "children", null);
     }
 
     @Override
@@ -596,12 +596,12 @@ public class Node extends Spatial implements Savable {
         // This prevents empty children list if controls query
         // it in Control.setSpatial().
         
-        children = e.getCapsule(this).readSavableArrayList("children", null);
+        children = new SafeArrayList( Spatial.class, 
+                                      e.getCapsule(this).readSavableArrayList("children", null) );
 
         // go through children and set parent to this node
         if (children != null) {
-            for (int x = 0, cSize = children.size(); x < cSize; x++) {
-                Spatial child = children.get(x);
+            for (Spatial child : children.getArray()) {
                 child.parent = this;
             }
         }
@@ -612,8 +612,8 @@ public class Node extends Spatial implements Savable {
     @Override
     public void setModelBound(BoundingVolume modelBound) {
         if(children != null) {
-            for(int i = 0, max = children.size(); i < max; i++) {
-                children.get(i).setModelBound(modelBound != null ? modelBound.clone(null) : null);
+            for (Spatial child : children.getArray()) {
+                child.setModelBound(modelBound != null ? modelBound.clone(null) : null);
             }
         }
     }
@@ -621,16 +621,16 @@ public class Node extends Spatial implements Savable {
     @Override
     public void updateModelBound() {
         if(children != null) {
-            for(int i = 0, max = children.size(); i < max; i++) {
-                children.get(i).updateModelBound();
+            for (Spatial child : children.getArray()) {
+                child.updateModelBound();
             }
         }
     }
     
     @Override
     public void depthFirstTraversal(SceneGraphVisitor visitor) {
-        for(int i = 0, max = children.size(); i < max; i++) {
-            children.get(i).depthFirstTraversal(visitor);
+        for (Spatial child : children.getArray()) {
+            child.depthFirstTraversal(visitor);
         }
         visitor.visit(this);
     }

engine/src/core/com/jme3/scene/Spatial.java

@@ -53,6 +53,7 @@ import com.jme3.renderer.queue.RenderQueue;
 import com.jme3.renderer.queue.RenderQueue.Bucket;
 import com.jme3.renderer.queue.RenderQueue.ShadowMode;
 import com.jme3.scene.control.Control;
+import com.jme3.util.SafeArrayList;
 import com.jme3.util.TempVars;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -131,7 +132,7 @@ public abstract class Spatial implements Savable, Cloneable, Collidable {
     public transient float queueDistance = Float.NEGATIVE_INFINITY;
     protected Transform localTransform;
     protected Transform worldTransform;
-    protected ArrayList<Control> controls = new ArrayList<Control>(1);
+    protected SafeArrayList<Control> controls = new SafeArrayList<Control>(Control.class);
     protected HashMap<String, Savable> userData = null;
     /** 
      * Spatial's parent, or null if it has none.
@@ -512,8 +513,8 @@ public abstract class Spatial implements Savable, Cloneable, Collidable {
             return;
         }
 
-        for (int i = 0; i < controls.size(); i++) {
-            controls.get(i).update(tpf);
+        for (Control c : controls.getArray()) {
+            c.update(tpf);
         }
     }
 
@@ -532,8 +533,8 @@ public abstract class Spatial implements Savable, Cloneable, Collidable {
             return;
         }
 
-        for (int i = 0; i < controls.size(); i++) {
-            controls.get(i).render(rm, vp);
+        for (Control c : controls.getArray() ) {
+            c.render(rm, vp);
         }
     }
 
@@ -590,9 +591,9 @@ public abstract class Spatial implements Savable, Cloneable, Collidable {
      * @see Spatial#addControl(com.jme3.scene.control.Control) 
      */
     public <T extends Control> T getControl(Class<T> controlType) {
-        for (int i = 0; i < controls.size(); i++) {
-            if (controlType.isAssignableFrom(controls.get(i).getClass())) {
-                return (T) controls.get(i);
+        for (Control c : controls.getArray()) {
+            if (controlType.isAssignableFrom(c.getClass())) {
+                return (T)c;
             }
         }
         return null;
@@ -1114,7 +1115,7 @@ public abstract class Spatial implements Savable, Cloneable, Collidable {
             if (clone instanceof Node) {
                 Node node = (Node) this;
                 Node nodeClone = (Node) clone;
-                nodeClone.children = new ArrayList<Spatial>();
+                nodeClone.children = new SafeArrayList<Spatial>(Spatial.class);
                 for (Spatial child : node.children) {
                     Spatial childClone = child.clone(cloneMaterial);
                     childClone.parent = nodeClone;
@@ -1127,7 +1128,7 @@ public abstract class Spatial implements Savable, Cloneable, Collidable {
             clone.setTransformRefresh();
             clone.setLightListRefresh();
 
-            clone.controls = new ArrayList<Control>();
+            clone.controls = new SafeArrayList<Control>(Control.class);
             for (int i = 0; i < controls.size(); i++) {
                 clone.controls.add(controls.get(i).cloneForSpatial(clone));
             }
@@ -1240,7 +1241,9 @@ public abstract class Spatial implements Savable, Cloneable, Collidable {
         capsule.write(shadowMode, "shadow_mode", ShadowMode.Inherit);
         capsule.write(localTransform, "transform", Transform.IDENTITY);
         capsule.write(localLights, "lights", null);
-        capsule.writeSavableArrayList(controls, "controlsList", null);
+        
+        // Shallow clone the controls array to convert its type.
+        capsule.writeSavableArrayList( new ArrayList(controls), "controlsList", null);
         capsule.writeStringSavableMap(userData, "user_data", null);
     }