Spatial/ray collision detection fails in JME3.1

Troubleshooting SDK
1

I have just tried v3.1
collision detection now fails for me.
I am using some code I c&p from an example or test a long time ago.
The code detects the spatial I click on with the mouse.
It worked fine in jme3.0
It fails in jme3.1
I used the debugger to find out why, it is because of some ‘optimization’ added in JME3.1

I hacked out the ‘optimization’ code, added to Node.java in JME3.1 (see below)
It now works.

package com.jme3.scene;
…
public class Node extends Spatial {
…
//THIS FAILS, ALWAYS RETURNS ZERO
public int collideWith(Collidable other, CollisionResults results){
int total = 0;

    // optimization: try collideWith BoundingVolume to avoid possibly redundant tests on children
    // number 4 in condition is somewhat arbitrary. When there is only one child, the boundingVolume test is redundant at all. 
    // The idea is when there are few children, it can be too expensive to test boundingVolume first.
    if (children.size() > 4)
    {
      BoundingVolume bv = this.getWorldBound();
      if (bv==null) return 0;

      // collideWith without CollisionResults parameter used to avoid allocation when possible
      if (bv.collideWith(other) == 0) return 0;
    }
    for (Spatial child : children.getArray()){
        total += child.collideWith(other, results);
    }
    return total;
}

//THIS WORKS - REMOVED THE OPTIMIZATION ADDED IN v3.1
public int collideWith(Collidable other, CollisionResults results){
int total = 0;

    for (Spatial child : children.getArray()){
        total += child.collideWith(other, results);
    }
    return total;
}
2

Yeah I had the same issue, and came in here to tell you how to fix it but it seems like you already worked it out.

I meant raise an issue or make a pull request but well, I kinda sort of had other stuff to do and I guess I forgot…

3

Yes, there are a few things wrong with this optimization and it should probably be removed until it can be done properly. It had a ripple effect of at least one other bad change, too.

4

“at least one other bad change, too.”
err… wasn’t anything to do with MotionEvent.play() was it?
Regular MotionEvent.play() works, but when there is a listener overriding onWayPointReach it doesn’t seem to work for me at all. (I have a MotionEvent.pause() at each waypoint reached, and play() on user input.)
The same code used to work with JME3.0
Thanks, Tom H.

5

No the other change was part of this one.

“optimize a special case with the most inefficient approach possible”
then
“micro-optimize one aspect about that inefficient approach to be at least tolerable”

Specifically, rather than just doing a bounds intersection check they do an actual CollisionResults style collision check. But wait, that means we create a new collision results + collisions object every time! Oh, then let’s reuse a collision results object. (Though the internal CollisionResult objects are still created.)

It’s easy for me to say in hindsight but it’s just a bad change and probably should not have been accepted as it is.

6

Glad I’m not calling this code inside the update loop :wink:
I’ll look into my MotionEvent problem as a separate issue.
Thanks, Tom H.

7

FYI: I just commented out this optimization code in ‘master’ and left a big giant comment about a better way to do it.

The future fix may or may not work in your case as I’m not sure why the ray test was failing. Most people with the problem where trying to do bounding volume collisions.

8

Wait,

I have that somewhere in my code, a scenevisitor manually chacking for collisions.
I always thought I had a mistake somewhere, and settled for the quick hack, seems like this one is not on me :wink: