Another easy one:
At this moment there are 145 open issues at GitHub: https://github.com/jMonkeyEngine/jmonkeyengine/issues
Some of them look like easy targets for people wishing to contribute:
Adding https://github.com/jMonkeyEngine/sdk/issues/193 there.
The library has to be added into the gradle dependencies.
When you look into the gradle file you see how it’s handled.
The only difficulty is ensuring to not make the sdk run on lwjgl3 (or both, I don’t know what it would pick then), so it has to be something optional I think
Yes! Fixing that SDK issue would be a major contribution.
And since pull-request integration is getting attention this week, let me point out that reviewing a PR also counts as a contribution in my book.
The Engine currently has 10 open pull requests: https://github.com/jMonkeyEngine/jmonkeyengine/pulls?q=is%3Aopen+is%3Apr
Only a few active people are authorized to integrate pull requests. But if you can read this thread, it’s likely you can contribute by reviewing a PR.
I’m particularly concerned about PR 1049, which has been awaiting review since March. It addresses an important bug. It’s a bit complicated, but not outrageously so. Would someone who understands collision detection take a close look, please?
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:
https://github.com/jMonkeyEngine/jmonkeyengine/pull/1049/files#diff-d941a2fa3d691e342c2a72202deedacaL108 Here (for instance), a collision is added to results, but the caller https://github.com/jMonkeyEngine/jmonkeyengine/pull/1049/files#diff-d7bd87d72acc390f85a5bd834b05dbbeL280 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
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 https://github.com/jMonkeyEngine/jmonkeyengine/pull/1049/files#diff-d941a2fa3d691e342c2a72202deedacaL127, there no multi collisions are possible, but https://github.com/jMonkeyEngine/jmonkeyengine/pull/1049/files#diff-d941a2fa3d691e342c2a72202deedacaR119 fixes the aspect of 1 (only add if within distance), and you can also see
See https://github.com/jMonkeyEngine/jmonkeyengine/pull/1049/files#diff-d941a2fa3d691e342c2a72202deedacaR127 The old behavior was to return “0 collisions”, if the first collision was already out of range. I am uncertain about that
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)
Requesting a quick review of PR 1181.
This looks to be an easy fix: issue 1197
Continuous integration at TravisCI appears to be broken again. Would someone please take a look?
As we discussed in pm, i took this opportunity to merge the pr that moved the CI to github actions. It should build successfully now.
Here’s another easy issue to address: https://github.com/jMonkeyEngine/jmonkeyengine/issues/1210
I have a holiday next week, I’ll try to squeeze in some work on the GitHub issues.
Maybe a bit off topic, but it might be related to improvements/work on the engine… I notice that since some time now, the asset pipeline is pointing towards gltf. This importer is maintained and is the best option to load/import models into jme. The importer however is using by default the PBRLighting material. The fragment shader used by this material
PBRLighting.frag is however incompatible with GLSL 1.10 which is the default of OpenGL2.0 which is also the default of jme…
(Or at least it is incompatible on my machine, bumping the version of OpenGL to 3.2 removes this issue)
I don’t know what you guys think of it, but I think we should either bump the default version of jme to OpenGL3.2 or change the behaviour of the gltf importer to use the Lighting material by default instead of the PBR one. As of now a new user will extend
SimpleApplication load a model and will be presented with an error.
Uncaught exception thrown in Thread[jME3 Main,5,main] RendererException: compile error in: ShaderSource[name=Common/MatDefs/Light/PBRLighting.frag, defines, type=Fragment, language=GLSL110] ERROR: 0:193: GLSL 110 does not allow sub- or super-matrix constructors ERROR: 0:194: Use of undeclared identifier 'wToLocalRot' ERROR: 0:194: Use of undeclared identifier 'wToLocalRot' ERROR: 0:207: Use of undeclared identifier 'wToLocalRot' ERROR: 0:208: Use of undeclared identifier 'rayLs' ERROR: 0:211: Use of undeclared identifier 'wToLocalRot' ERROR: 0:215: Use of undeclared identifier 'rayLs' ERROR: 0:216: Use of undeclared identifier 'rayLs' ERROR: 0:217: Use of undeclared identifier 'firstPlaneIntersect' ERROR: 0:217: Use of undeclared identifier 'secondPlaneIntersect' ERROR: 0:218: Use of undeclared identifier 'furthestPlane' ERROR: 0:218: Use of undeclared identifier 'furthestPlane' ERROR: 0:218: Use of undeclared identifier 'furthestPlane' ERROR: 0:220: Attempt to use 'distance' as a variable ERROR: 0:221: Use of undeclared identifier 'intersectPositionWs'
edit: or leave everything as is and fix the fragment shader. I didn’t look at it yet, but if I remember correctly newer versions of GLSL extract the upper-left part of matrices. This way you can create a mat3 from a mat4 for example. I’m not saying this is the error in this particular case, I just remembered that I made this mistake before
The engine uses by default the compatibility profile, it is sometimes not be in pair with the core profile supported by the same driver, and a source of many issues. My personal opinion is that the engine should use by default a more recent core profile, 3.2 would be probably the most logical choice, since it is old enough to be supported by pretty much every driver (2009) and contains also all the most important features.
Important discussion, but definitely off-topic.
Also note that this particular issue mostly only happens with Mac OS X and thus most people don’t see this issue with PBR
Just putting out there that for those who are interested in contributing, there is a project on github that gives you a guage of what you may be able to achieve; especially if it’s your first time getting your feet wet.
Here’s an interesting and important issue involving filters and MSAA, just opened yesterday:
Also ready for review: PR 1268
Looking for someone who understands LWJGL3.