[commited] Replace the generateCollisionTree that still uses int instead of enum

There is one method left in com.jme.bounding.CollisionTreeManager that is still using an int for the type.

It has the following signature:

public void generateCollisionTree(int type, Spatial object, boolean protect)



Beside the int the current method signature leads to an infinite loop as it calls itself without advancing in the hierarchy.



The current one should be removed and the following method should take over its former responsibility:



    /**
     * creates a new collision tree for the provided spatial. If the spatial is
     * a node, it recursively calls generateCollisionTree for each child. If it
     * is a TriMesh, a call to generateCollisionTree is made for each mesh. If
     * this tree(s) is to be protected, i.e. not deleted by the
     * CollisionTreeController, set protect to true.
     *
     * @param type
     *            the type of collision tree to generate.
     * @param object
     *            the Spatial to generate tree(s) for.
     * @param protect
     *            true to keep these trees from being removed, false otherwise.
     */
   public void generateCollisionTree(CollisionTree.Type type,
            Spatial mesh, boolean protect) {
      if (mesh instanceof TriMesh)
         CollisionTreeManager.getInstance().generateCollisionTree(type, (TriMesh)mesh, protect);
      if (mesh instanceof Node) {
         if (((Node)mesh).getQuantity() > 0)
            for (Spatial sp : ((Node)mesh).getChildren())
               generateCollisionTree(type, sp, protect);
      }
   }



committed rev 4034.

1. change int type to enum, that's for sure
2. refactor 'object' to 'spatial', instead of 'mesh' as you wrote in the above code

but, 3. where is it you say that the method calls itself? I can't spot that...
Galun said:

Beside the int the current method signature leads to an infinite loop as it calls itself without advancing in the hierarchy.


Thats intended, its called 'recursion'  ;)

The original function (with an int argument) is recursive as mine is, but the problem is that in its current state it will recurse infinitely. Just check it in the current jme2 source.



Yes, Spatial would be better indeed - I'll change it.

Galun said:

, but the problem is that in its current state it will recurse infinitely. Just check it in the current jme2 source.


you are right, sorry

I thought this discussion would go on, as there should be some things clarified :?



What's the use of this patch btw? The method was not and is not used anyways. It was there for backward compatibility, so changing it to the enum stuff makes no sense. What about deleting it?

well, it hasn't made things worse  :wink:



maybe we should just delete it

Most propably it was not used, yes. But if somebody tried so he would have started an endless loop. Maybe it was thought to be for backward compatibility but it couldn't serve as that.



There was no replacement to recursively update the collision tree and it looked very like this function was just forgotten.



I haven't read any objections which could not be cleared so I committed after around three days. Is there anything wrong with that?


dhdd said:

maybe we should just delete it

yep
dhdd said:

well, it hasn't made things worse  :wink:

maybe we should just delete it


If "it" means the old and broken function then this is done already.

But if "it" means the new recursive function then we would force everybody into implementing their own function to do that. In jME 2 there was no alternative to the broken method.
Just to repeat: the other current create methods are not recursive but act on a single TriMesh. There are recursive functions to update and to remove collision trees - so why should we remove the method to create collision trees recursively?
Galun said:

so why should we remove the method to create collision trees recursively?


ok, then someone is going to have to fix it, so it wont recurse infinitly. Since i dont know anything about collision trees, i'm not fit for the job.

any takers?

Err, maybe I wasn't clear enough. The committed fix already stops the infinite recursion. The one with the altered arguments (enum instead of int) is recursive but not infinite. The infinite recursion problem was in the previous method because there was no other one fitting the arguments thus calling itself with the same arguments.

This was a result of this left-over int-argument method.



In short words: It's fixed now.

Galun said:

Err, maybe I wasn't clear enough.

hehe, yeah, seems that was the only problem :)