Dependency from CollisionShapeFactory to Terrain

Hello,



I always try to keep the used libs at a minimum. I know we are talking here about ~160kb but anyways.



Currently creating a mesh based collisionshape needs the terrain library, but a simple change in the order of checking the types would eliminate this dependency.



Here is the patch:



[patch]

This patch file was generated by NetBeans IDE

It uses platform neutral UTF-8 encoding and n newlines.

— Base (BASE)

+++ Locally Modified (Based On LOCAL)

@@ -161,17 +161,17 @@

  • @return A MeshCollisionShape or a CompoundCollisionShape with MeshCollisionShapes as children if the supplied spatial is a Node. A HeightieldCollisionShape if a TerrainQuad was supplied.

    */

    public static CollisionShape createMeshShape(Spatial spatial) {
  •    if (spatial instanceof TerrainQuad) {<br />
    
  •    if (spatial instanceof Geometry) {<br />
    
  •        return createSingleMeshShape((Geometry) spatial, spatial);<br />
    
  •    } else if (spatial instanceof Node) {<br />
    
  •        return createMeshCompoundShape((Node) spatial);<br />
    
  •    }else if (spatial instanceof TerrainQuad) {<br />
    

TerrainQuad terrain = (TerrainQuad) spatial;

return new HeightfieldCollisionShape(terrain.getHeightMap(), terrain.getLocalScale());

} else if (spatial instanceof TerrainPatch) {

TerrainPatch terrain = (TerrainPatch) spatial;

return new HeightfieldCollisionShape(terrain.getHeightMap(), terrain.getLocalScale());

  •    } else if (spatial instanceof Geometry) {<br />
    
  •        return createSingleMeshShape((Geometry) spatial, spatial);<br />
    
  •    } else if (spatial instanceof Node) {<br />
    
  •        return createMeshCompoundShape((Node) spatial);<br />
    
  •    } else {<br />
    
  •    }  else {<br />
    

throw new IllegalArgumentException("Supplied spatial must either be Node or Geometry!");

}

}

[/patch]

Actually the patch above does not work since a Terrain extends Node :frowning: damn…

Hm, the TerrainTestCollision still works, so the patch might work, and i have to read the function of ‘instanceof’ again

Yea it is a little bit tightly coupled there, at least right now. We might be able to add a hint to Spatial, or a getCustomCollisionShape() method. Then custom spatials could use their own collision shapes or borrow from others. Something other than terrain might want to use that heightfield class. Thoughts @normen?

I’d think that just making a core “Heightfield” marker interface with the float[] getHeightMap method and let terrainquad implement that would decouple everything?

I was thinking more that if Spatial had a way to supply any collision shape supported by Bullet (Heightfield being one of about 7) then it could be useful to separate it out that way. But I’m unsure of the demand for these custom collision shapes and if it is worth implementing. These collision shapes would of course have to be abstracted out of the bullet project. So maybe for now a simple heightfield interface will be sufficient.

Yeah, any integration either yields some inter-dependency or some silly interface for generic heightmap data or something right in core :frowning:

Well I did a quickie HeightfieldCollision interface that Terrain implements. It could in theory be used by anything else that cares to supply a square heightfield. The benefit also being that CollisionShapeFactory isn’t restricted to just TerrainQuad then, but possibly any other terrain implementing the same HeightfieldCollision interface. I wouldn’t suggest using it based on the 160kb terrain dependency, but it does soften the CollisionShapeFactory.