# Code of game engine written in Java: what does it hide?

Have you ever wondered about game engines written in Java? jMonkeyEngine is such an engine, and a popular one\. In this article, we will look at it and check its source code for errors\. We may even find out why games are usually written in C\# and C\+\+ instead of Java\.

![1113_JMonkeyEngine/image1.png](https://import.viva64.com/docx/blog/1113_JMonkeyEngine/image1.png)

## About project

Here is some information about the project from its website:

jMonkeyEngine is a modern, developer\-friendly game engine written primarily in Java\. Its minimalist, code\-first approach makes it ideal for developers who need game engine support while retaining full control over their code and the capability to extend and customize the engine to fit their workflow\.

I've put together a brief summary and highlighted a few things I'd like to mention:

1\. The engine uses LWJGL \(a graphics library that provides access to the OpenGL and Vulkan APIs\) by default\. I'd like to note that the engine doesn't support DirectX due to the cross\-platform nature of Java and the Windows\-only focus of DX12\.

2\. It uses jBullet, a Java port of the Bullet library, to model physical space\. We'll analyze it as well \(the future me breaks in and ruins the intrigue by saying that there are some suspicious fragments, indeed\)\.

3\. The engine has an SDK \(they call it jmonkeyplatform\) based on the NetBeans IDE\. It provides a material editor, scene editor, filter editor, and a glsl\-shader lexer\. Of course, we'll have a look at it, too\.

![1113_JMonkeyEngine/image2.png](https://import.viva64.com/docx/blog/1113_JMonkeyEngine/image2.png)

4\. Apparently, developers don't use code analyzers\. I can rate the quality of the source code as good, but not without some strange fragments\. I'll tell you about them bellow\. Oh, and don't take my words about Java gamedev to heart, just kidding\.

## Looking for errors

<details>
   <summary>\\\(Almost\\\) important information</summary>

* To check the project, we'll use the PVS\-Studio static analyzer\. The article author is one of its developers\.
* You'll see code examples as you read\. I've shortened most of them so as not to overwhelm you\. I've also put an ellipsis to mark the abbreviated code: "\.\.\.\."\.
* At the time of the check, the latest revision was the [e584cb1](https://github.com/jMonkeyEngine/jmonkeyengine/tree/e584cb194c5862e5805ce318761691c7181e96a2) commit\. We checked it using the static analyzer\.
* You can find all checked sources and other source files on which all the judgments in the article are based — the links are permanent\.
* The article contains only the errors that seemed interesting to its author \(yep, imho\)\. If you'd like to see other errors, you can always download the analyzer and check the project yourself\.


</details>


### Just some weird stuff

Let's start our morning with an unreachable _return_\. Here is a fragment:

The [ListSort\.java\(877\)](https://github.com/jMonkeyEngine/jmonkeyengine/blob/b29c2f8c409577cff06100879c3f1829a0424cf3/jme3-core/src/main/java/com/jme3/util/ListSort.java#L877) file

```cpp
public void innerMergeHigh(Comparator<T> comp, 
                           T[] tempArray, T[] arr, int idxA) {
  lengthA--;
  if (lengthA == 0 || lengthB == 1) {
    return;
  }
  if (lengthB == 1) {
    return;
  }
  while (true) {
    ....
  }
}
```

The analyzer issues a warning:

[V6039](https://pvs-studio.com/en/docs/warnings/v6039/) There are two 'if' statements with identical conditional expressions\. The first 'if' statement contains method return\. This means that the second 'if' statement is senseless\. ListSort\.java\(874\), ListSort\.java\(877\)

Logically, a duplicate check should have a variable other than zero in its body\. Which one, exactly? We have all the fields of this class to choose from, and believe me, there are many\.

In the comments to the _ListSort_ class, there are links to the C code on which it's based\. The function is called _merge\_hi_ in its C form\. You can easily find it by its name:

```cpp
static Py_ssize_t merge_hi(....) {
  ....
  --na;
  if (na == 0)
    goto Succeed;
  if (nb == 1)
    goto CopyA;
  ....
}
```

I think it's just extra, copy\-pasted code rather than an error due to a missing check during porting\. Maybe Java not having a '_goto_' operator made a difference?

### Wonders of list handling

Shall we try to find an error in this method? 

The [FbxMesh\.java\(334\)](https://github.com/jMonkeyEngine/jmonkeyengine/blob/b29c2f8c409577cff06100879c3f1829a0424cf3/jme3-plugins/src/fbx/java/com/jme3/scene/plugins/fbx/objects/FbxMesh.java#L334) file

```cpp
private List<Geometry> createGeometries() throws IOException {
  Mesh mesh = new Mesh();
  mesh.setMode(Mode.Triangles);
  // Since each vertex should contain unique texcoord 
  // and normal we should unroll vertex indexing
  // So we don't use VertexBuffer.Type.Index for elements drawing
  // Moreover quads should be triangulated (this increases number of vertices)
  if(indices != null) {
    ....
  } else {
    // Stub for no vertex indexing (direct mapping)
    iCount = vCount = srcVertexCount;
    vertexMap = new ArrayList<>(vCount);     // <=
    indexMap = new ArrayList<>(vCount);
    reverseVertexMap = new ArrayList<>(vCount);
    for (int i = 0; i < vCount; ++i) {
      vertexMap.set(i, i);                   // <=
      indexMap.set(i, i); 
      List<Integer> l = new ArrayList<>(1);
      l.add(i);
      reverseVertexMap.add(l);
    }
  }
  ....
}
```

As a hint, have a look at the warning issued by the analyzer:

[V6025](https://pvs-studio.com/en/docs/warnings/v6025/) Index 'i' is out of bounds\. FbxMesh\.java\(336\)

The _else_ block always throws the _IndexOutOfBoundsException_ exception\. Let me explain why this is the case\. Once in the _else_ block, we create empty collections and try to replace their elements with _i_ \(just a counter that should have filled _vertexMap_ and _indexMap_ with values from 0 to vCount\) in the _for_ loop\. However, the [set method implementation](https://docs.oracle.com/javase/8/docs/api/java/util/List.html#set-int-E-) has made its own adjustments unnoticed by the developer\. It can replace only existing elements, not add new ones\. As a result, the code doesn't work as intended but throws an exception\.

This code should be called only if there are no vertex indices in the FBX model\. However, the format is proprietary, and there is no official documentation for it\. I've learned about it [here](https://banexdevblog.wordpress.com/2014/06/23/a-quick-tutorial-about-the-fbx-ascii-format/) and [there](https://stackoverflow.com/questions/19634369/read-texture-filename-from-fbx-with-fbx-sdk-c)\.

I'm wondering: is it possible to find an FBX model without specified polygon indices? For example, the OpenFBX library [throws an error](https://github.com/nem0/OpenFBX/blob/27df0a968042d7347b58751ae543dcb3843a8cfb/src/ofbx.cpp#L2356) if an object file has no indices\. I encourage you to discuss this matter in the comments\. I'll happily work out this dilemma with you\.

### Consequences of refactoring

The next warning is also related to reading the FBX format:

The [FbxTexture\.java\(101\)](https://github.com/jMonkeyEngine/jmonkeyengine/blob/b29c2f8c409577cff06100879c3f1829a0424cf3/jme3-plugins/src/fbx/java/com/jme3/scene/plugins/fbx/material/FbxTexture.java#L100) file

```cpp
@Override
public void fromElement(FbxElement element) {
  super.fromElement(element);
  if (getSubclassName().equals("")) {
    for (FbxElement e : element.children) {
      if (e.id.equals("Type")) {
        e.properties.get(0);
      } 
      /*else if (e.id.equals("FileName")) {
        filename = (String) e.properties.get(0);
      }*/
    }
  }
}
```

The analyzer warns us against calling the _get_ method without using its result:

[V6010](https://pvs-studio.com/en/docs/warnings/v6010/) The return value of function 'get' is required to be utilized\. FbxTexture\.java\(101\)

It's a weird loop: in the first _if_, we don't use the result, and the second is commented out\. Since the loop does nothing useful, we could just delete this code\. After analyzing the class changes, I can say that this fragment is the result of [refactoring](https://github.com/jMonkeyEngine/jmonkeyengine/commit/d6638e0edd126e2ee9e7d38f531fccc1e426a7b6#diff-b7992568dc9e1f8ca4c9a8715bdba2eb17af433246ffc39efccad8d8da18205dR102)\. Let's note the importance of static analysis for detecting such errors: it will certainly show you the fragments that need attention after code optimization\.

### "Find an issue" game

I invite you to find the issue in the following code:

The [SelectionPropertyEditor\.java\(61\)](https://github.com/jMonkeyEngine/sdk/blob/1aeb071b4368a36a9d7630e0f4be08205d6130ab/jme3-assetpack-support/src/com/jme3/gde/assetpack/browser/properties/SelectionPropertyEditor.java#L58) file

```cpp
public class SelectionPropertyEditor implements PropertyEditor {
  ....
  private String value;

  public SelectionPropertyEditor(String[] tags, String value) {
    this.tags = tags;
    this.value = value;
  }

  public void setValue(Object value) {
    if (value instanceof String) {
      value = (String) value;
    }
  }
  ....
}
```

In the setter method, the passed parameter is assigned to itself, and that's really weird\. I think the developer made a typo and forgot to add the _this_ keyword\. Of course, the analyzer notifies us:

[V6026](https://pvs-studio.com/en/docs/warnings/v6026/) This value is already assigned to the 'value' variable\. SelectionPropertyEditor\.java\(61\)

### Let's check JBullet

JBullet is a Java port of the [Bullet](https://en.wikipedia.org/wiki/Bullet_(software)) real\-time physics engine\. It's not developed directly by the jmonkey team, but a [fork](https://github.com/stephengold/jbullet) from one of the team members is used as a third\-party library in the main project, so it's worth a check as well\.

### How to lose one's way in broad daylight

Using a temporary variable to swap objects between each other seems like common practice, but not in this case:

The [HashedOverlappingPairCache\.java\(233\)](https://github.com/stephengold/jbullet/blob/8158d94fcb3c05ab765c1c18c7380cdfbdc443cf/src/com/bulletphysics/collision/broadphase/HashedOverlappingPairCache.java#L233) file

```cpp
@Override
public BroadphasePair findPair(BroadphaseProxy proxy0, BroadphaseProxy proxy1) {
  BulletStats.gFindPairs++;
  if (proxy0.getUid() > proxy1.getUid()) {
    BroadphaseProxy tmp = proxy0;
    proxy0 = proxy1;
    proxy1 = proxy0; // <=
  }
  ....
}
```

It seems like the developer has messed something up here\. Even the analyzer issued the following:

[V6026](https://pvs-studio.com/en/docs/warnings/v6026/) This value is already assigned to the 'proxy1' variable\. HashedOverlappingPairCache\.java\(233\)

After looking at the original library source code, I learned that the current error was made only in the Java port\. In the original code, a [function](https://github.com/bulletphysics/bullet3/blob/6bb8d1123d8a55d407b19fd3357c724d0f5c9d3c/src/Bullet3Collision/BroadPhaseCollision/b3OverlappingPairCache.cpp#L112) called _b3Swap_ is used for elements\. Let's take a look at it:

```cpp
template <typename T>
B3_FORCE_INLINE void b3Swap(T &a, T &b)
{
  T tmp = a;
  a = b;
  b = tmp;
}
```

It reminds you of something, doesn't it? :\)

Given the C\+\+ implementation we've already seen, we can easily find a way to fix the error:

```cpp
@Override
public BroadphasePair findPair(BroadphaseProxy proxy0, BroadphaseProxy proxy1) {
  BulletStats.gFindPairs++;
  if (proxy0.getUid() > proxy1.getUid()) {
    BroadphaseProxy tmp = proxy0;
    proxy0 = proxy1;
    proxy1 = tmp;
  }
  ....
}
```

### Reference \!\= value

Let's take a look at the code fragment that I find very strange:

The [CProfileNode\.java\(70\)](https://github.com/stephengold/jbullet/blob/master/src/com/bulletphysics/linearmath/CProfileNode.java#L70) file

```cpp
public CProfileNode getSubNode(String name) {
  // Try to find this sub node
  CProfileNode child = this.child;
  while (child != null) {
    if (child.name == name) {          // <=
      return child;
    }
    child = child.sibling;
  }

  // We didn't find it, so add it
  ....
  return node;
}
```

The analyzer has issued a warning:

[V6013](https://pvs-studio.com/en/docs/warnings/v6013/) Strings 'child\.name' and 'name' are compared by reference\. Possibly an equality comparison was intended\. CProfileNode\.java\(70\)

The developer here has made probably one of the most common and ugly errors in Java — comparing _String_ variables by reference rather than by value\.

![1113_JMonkeyEngine/image3.png](https://import.viva64.com/docx/blog/1113_JMonkeyEngine/image3.png)

The comparison may or may not work\. Using the [string pool](https://www.baeldung.com/java-string-pool) mechanism, Java tries to ensure that strings with the same internal content are the same objects, but it may not work :\) It seems quite possible that objects with different references are compared while their content is the same\. In this case, the comparison gives us _false_, even though we were expecting _true_\.

Copy\-paste\-oriented programming 

Let's look at the most common example of a copy\-paste error:

The [ConeTwistConstraint\.java\(403\)](https://github.com/stephengold/jbullet/blob/8158d94fcb3c05ab765c1c18c7380cdfbdc443cf/src/com/bulletphysics/dynamics/constraintsolver/ConeTwistConstraint.java#L403) file 

```cpp
public class ConeTwistConstraint extends TypedConstraint {
  ....
  private boolean solveTwistLimit;
  private boolean solveSwingLimit;
  ....

  public boolean getSolveTwistLimit() {
    return solveTwistLimit;
  }
  
  public boolean getSolveSwingLimit() {
    return solveTwistLimit; // <=
  } 
}
```

In this example, the programmer returns a field from a method\. However, based on the method signature, it would make sense to return a different field\. Most often, such errors occur due to inattention\. The developer copied and changed the method name but forgot to change the return field\. As a result, the analyzer issued a warning\. By the way, here it is:

[V6091](https://pvs-studio.com/en/docs/warnings/v6091/) Suspicious getter implementation\. The 'solveSwingLimit' field should probably be returned instead\. ConeTwistConstraint\.java\(407\), ConeTwistConstraint\.java\(74\)

## Conclusion

That's all for today\. I've collected all the errors, both in the article and in pull requests for the project developers \([click](https://github.com/stephengold/jbullet/pull/1), [click](https://github.com/jMonkeyEngine/sdk/pull/593), [click](https://github.com/jMonkeyEngine/jmonkeyengine/pull/2241)\)\. This is the second time I've been able to help the programming community by contributing to open\-source projects\. I'm very happy about it\.

Would you like to try static analysis? You can always introduce it into your project using the PVS\-Studio tool\. You can download the trial version [here](https://pvs-studio.com/en/pvs-studio/try-free/)\. We offer [free licensing](https://pvs-studio.com/en/order/open-source-license/) for open\-source projects\.