Since the upper bound for the for loop is computed once before running the loop, the application crashes when a child is removed while the loop is executed.
In my case, i have a Controller that removes a child from its parent after a specific amount of time has passed.
I would think this should be handled just like in Spatial.updateWorldData() (adding a try-catch block and catching IndexOutOfBoundExceptions)
Don't know the performance impacts, please get back to me
Index: src/com/jme/scene/Node.java
===================================================================
--- src/com/jme/scene/Node.java (revision 4048)
+++ src/com/jme/scene/Node.java (working copy)
@@ -566,18 +566,23 @@
}
BoundingVolume worldBound = null;
for (int i = 0, cSize = children.size(); i < cSize; i++) {
- Spatial child = children.get(i);
- if (child != null) {
- if (worldBound != null) {
- // merge current world bound with child world bound
- worldBound.mergeLocal(child.getWorldBound());
+ try {
+ Spatial child = children.get(i);
+ if (child != null) {
+ if (worldBound != null) {
+ // merge current world bound with child world bound
+ worldBound.mergeLocal(child.getWorldBound());
- } else {
- // set world bound to first non-null child world bound
- if (child.getWorldBound() != null) {
- worldBound = child.getWorldBound().clone(this.worldBound);
- }
- }
+ } else {
+ // set world bound to first non-null child world bound
+ if (child.getWorldBound() != null) {
+ worldBound = child.getWorldBound().clone(this.worldBound);
+ }
+ }
+ }
+ } catch (IndexOutOfBoundsException e) {
+ // a child was removed while iterating over this nodes children
+ break;
}
}
this.worldBound = worldBound;
The current code of Spatial.updateWorldData() is made for the possibility of removing Controllers while updating. Why shouldn't it be possible to do so with Spatials? Keep in mind that StandardGame is made for multithreading and other threads might want to remove parts of the scenegraph.
I didn't know that the StandardGame is made for the multithreading.
Sorry. I should have investigated about that.
But IMHO, it is the responsibility of application to make it error-free, not engine.
And, please print log message when error occurs.
I find more dangerous that you may be missing updates. I think that if the child removed is in the part of the array that has been processed already, indexes would shift and the index would be incremented anyway.
As this is quite an abnormal situation of which a programer should be aware of, I reckon it should be left up to the user to catch. That's the purpose of UncheckedExceptions anyway, and JME is an API and as such shouldn't catch that exception.
I think child shouldn't be removed when update call is being processed.
If one want to do this, he may insert try-catch block on his own update method.
The current code of Spatial.updateWorldData() is made for the possibility of removing Controllers while updating. Why shouldn't it be possible to do so with Spatials? Keep in mind that StandardGame is made for multithreading and other threads might want to remove parts of the scenegraph.
Also, placing a try-catch block in the method that calls update() does not solve this problem, since the exception would cause the update cycle to end, whereas the patch proposed above would have the update cycle continue normally after the exception.
Looks ok to me, regarding performance aspects. Still mulov is partly right - removing controllers in the update method is still a potential problem. But your patch does not make it worse
What is makes worse is the indentation - please check that.
I would say this version still could jump updates, and it won't even launch an Exception.
Missing an update is still an exceptional situation, and the programmer should receive an Exception.
What I do in your situation, by the way, is to queue children removals or otherwise overcome the need to remove children from the scenegraph while it is being walked. I don't think doing that makes too much sense anyway.
I still prefer to receive the exception. Missing updates is completely unnaceptable to me.
What I do in your situation, by the way, is to queue children removals or otherwise overcome the need to remove children from the scenegraph while it is being walked. I don't think doing that makes too much sense anyway.
That's what I did too.
When I used 'simple firework' (http://www.jmonkeyengine.com/wiki/doku.php?id=simple_fireworks&s[]=firework)
it threw IndexOutOfBoundsException frequently and degraded performance.
So I registered disappearing particles to linked list and removed it after every update if exists.
'Queuing and removal at the end of the update' strategy may be a good way to avoid error.
