The Contribution Mega-Thread

Development
22

For 1049 I can recommend reviewing file-by-file, because except of BresenhamTerrainPicker, everyone should be able to review the files. Most stuff is Unit Test related (BaseTest/BaseAWTTest, build.gradle/TerrainTestCollision), and TerrainQuad is only adding a setter and moving some logic into the picker.

For Bresenham there were two issues and unfortunately I did not do multiple PRs or at least commits, but:

  1. Fix and Improve TerrainPicking by MeFisto94 · Pull Request #1049 · jMonkeyEngine/jmonkeyengine · GitHub Here (for instance), a collision is added to results, but the caller Fix and Improve TerrainPicking by MeFisto94 · Pull Request #1049 · jMonkeyEngine/jmonkeyengine · GitHub doesn’t remove the collision, if it was out of range. It simply sets the result of collideWith to 0. This is “legal”, but doesn’t work, because no one will be able to remove the collision from the results again, because:
    When colliding against a whole scene, jme performs collideWith recursively and passes the CollisionResults around, which means the results could already contain some other (legit) collisions

  2. The old code assumed, that there is only one intersection possible, which is wrong if you think about mountains.
    It’s questionable if there is a use for a multi collision raycast there, but the engine also does it for other geometries, so why not give that power to the user.
    For performance reasons, you can disable multi-collisions, which improves performance, because unlike regular raycasts, terrain can walk “the grid”, starting from the start point, it will move along each quad and do collisions there. This means the first collision will always be the closest.
    See Fix and Improve TerrainPicking by MeFisto94 · Pull Request #1049 · jMonkeyEngine/jmonkeyengine · GitHub, there no multi collisions are possible, but Fix and Improve TerrainPicking by MeFisto94 · Pull Request #1049 · jMonkeyEngine/jmonkeyengine · GitHub fixes the aspect of 1 (only add if within distance), and you can also see multipleCollisions

  3. See Fix and Improve TerrainPicking by MeFisto94 · Pull Request #1049 · jMonkeyEngine/jmonkeyengine · GitHub The old behavior was to return “0 collisions”, if the first collision was already out of range. I am uncertain about that

  4. All in All it would be good if everyone would try to use that code within their projects and see if behavior changes. That’s also why I updated the TerrainTest, there I didn’t notice anything odd anymore, especially with MultiCollisions (toggle that in the code, I did not bind it to a key)

2 Likes