# Что скрывается в коде игрового движка, написанного на Java?

Вы когда\-нибудь интересовались игровыми движками, написанными на Java? В этой статье мы рассмотрим и проверим на наличие ошибок в исходном коде один из популярных игровых движков — jMonkeyEngine\. Возможно, мы даже узнаем, почему игры пишутся на C\# и C\+\+, а не на Java\.

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

## О проекте

Для описания проекта приведу данные с их сайта:

jMonkeyEngine — это современный и удобный для разработчиков игровой движок, написанный в основном на Java\. Его минималистичный подход "сначала код" делает его идеальным для разработчиков, которым нужна поддержка игрового движка и сохранность полного контроля над своим кодом с возможностью расширения и адаптации движка к своему рабочему процессу\.

Я собрал краткую характеристику и выделю несколько моментов, которые хочу упомянуть:

1\. Использует по умолчанию LWJGL \(графическая библиотека, предоставляющая доступ к API OpenGL и Vulkan\)\. Отмечу, что использование DirectX движком не подразумевается по причине кроссплатформенности Java и ориентированности DX12 только на Windows\.

2\. Использует для моделирования физического пространства jBullet — порт библиотеки Bullet на Java\. Мы дополнительно его проанализируем \(я из будущего ворвусь и без интриги скажу, что там действительно есть подозрительные места\)\.

3\. Движок имеет SDK \(именуют его jmonkeyplatform\), основанный на IDE NetBeans\. Он включает в себя редактор материалов, сцен, фильтров, лексер glsl\-шейдеров\. Его мы тоже обязательно проверим\.

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

4\. Разработчики, судя по всему, не используют анализаторы кода\. Я могу оценить качество исходного кода как хорошее, но не без странных мест, о которых сейчас и расскажу\. А про то, что игры не пишутся на Java, я пошутил :\)

## Ищем ошибки

<details>
   <summary>Важная \\\(почти\\\) информация</summary>

* Для проверки проекта мы будем использовать статический анализатор PVS\-Studio, разработчиком которого автор статьи и является\.
* Во время чтения вы встретите примеры кода\. Большинство из них сокращено, чтобы не перегружать читателя\. Пометкой сокращённого кода является многоточие "\.\.\.\."\.
* На момент проверки проекта последней ревизией был коммит [e584cb1](https://github.com/jMonkeyEngine/jmonkeyengine/tree/e584cb194c5862e5805ce318761691c7181e96a2), её мы и будем проверять статическим анализатором\.
* Все исходники, которые проверялись, и все суждения, основанные на других исходных файлах, снабжены постоянной ссылкой, по которой их можно найти\.
* В статье приведены только те ошибки, что показались интересными именно автору \(да, вкусовщина\)\. Если кто\-то хочет посмотреть и на остальные, то всегда можно скачать анализатор и проверить проект самостоятельно\.


</details>


### Просто странности

Начнём утро с недостижимого _return_\. Представляю вам фрагмент:

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

```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) {
    ....
  }
}
```

Получили срабатывание:

[V6039](https://pvs-studio.ru/ru/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\)

С точки зрения логики дублирующая проверка должна была бы иметь в своём теле другую сравниваемую с нулём переменную\. Какую конкретно? На выбор у нас есть все поля этого класса, и поверьте мне — их много\.

В комментариях к классу _ListSort_ есть ссылки на С\-шный код, на котором он основан\. Сама функция в С\-виде называется _merge\_hi_, её несложно найти по названию:

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

Я склоняюсь к тому, что это лишний, "закопипащенный" код, нежели какая\-либо ошибка, связанная с потерей проверки при портировании\. Может быть, сыграло роль, что в Java нет оператора 'goto'?

### Чудеса работ со списком

Попробуем найти ошибку в этом методе вместе? 

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

```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);
    }
  }
  ....
}
```

В качестве подсказки предлагаю вам посмотреть сообщение анализатора:

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

Блок _else_ всегда приведёт к исключению _IndexOutOfBoundsException_\. Сейчас объясню, почему так происходит\. Попадая в блок _else_, мы создаём пустые коллекции и в цикле _for_ пытаемся заменить их элементы на _i_ \(просто счётчик который должен бы был заполнить _vertexMap _и_ indexMap_ значениями от 0 до vCount\)\. Но [реализация метода set](https://docs.oracle.com/javase/8/docs/api/java/util/List.html#set-int-E-) незаметно для разработчика внесла свои коррективы и может заменять только уже существующие элементы, а не добавлять новые\. Поэтому этот код с нами дружить не будет, а будет выкидывать исключение\.

Это место должно вызываться только в том случае, если индексы вершин в FBX модели отсутствуют, но формат является проприетарным, и официальной документации на него нет\. Лично я черпал свои знания отсюда: [тык](https://banexdevblog.wordpress.com/2014/06/23/a-quick-tutorial-about-the-fbx-ascii-format/), [тык](https://stackoverflow.com/questions/19634369/read-texture-filename-from-fbx-with-fbx-sdk-c)\.

Интересно, можно ли найти модель FBX без указания индексов полигонов? Например, библиотека OpenFBX [выкидывает ошибку](https://github.com/nem0/OpenFBX/blob/27df0a968042d7347b58751ae543dcb3843a8cfb/src/ofbx.cpp#L2356), если у объектного файла индексы отсутствуют\. Предлагаю обсудить этот вопрос в комментариях\. Охотно готов с вами разобраться в этой дилемме\.

### Последствие рефакторинга

Следующее срабатывание тоже связано с чтением FBX формата:

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

```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);
      }*/
    }
  }
}
```

Анализатор ругается на вызов метода _get_ без использования его результата:

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

Странный цикл: в первом _if_ мы не используем результат, а второй вообще закомментирован\. Так как цикл в принципе не выполняет никаких полезных действий, то этот код стоит просто удалить\. Проанализировав изменения класса, могу сказать, что это место появилось [вследствие рефакторингов](https://github.com/jMonkeyEngine/jmonkeyengine/commit/d6638e0edd126e2ee9e7d38f531fccc1e426a7b6#diff-b7992568dc9e1f8ca4c9a8715bdba2eb17af433246ffc39efccad8d8da18205dR102)\. Подметим актуальность статического анализа для поиска такого рода ошибок — он обязательно даст указание на места, требующие внимания после оптимизации кода\.

### Игра "найди проблему"

Предлагаю вам найти проблему вот тут:

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

```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;
    }
  }
  ....
}
```

Здесь передаваемый параметр в сеттер\-методе присваивается самому себе, и это действительно странно\. Я думаю, разработчик опечатался и забыл добавить ключевое слово _this_\. И, конечно же, анализатор нам об этом сообщит:

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

### Проверяем JBullet

JBullet — порт физического движка реального времени [Bullet](https://ru.wikipedia.org/wiki/Bullet_Physics_Library) на Java\. Он не является прямой разработкой команды jmonkey, но [форк](https://github.com/stephengold/jbullet) одного из разработчиков их команды используется как сторонняя библиотека в основном проекте, поэтому её тоже стоит проверить\.

### Как заблудиться в трёх соснах

Использование временной переменной для смены объектов между собой кажется обычной практикой, но не в этом случае:

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

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

Кажется, как будто разработчик здесь что\-то напутал, и даже анализатор встал в стойку и сказал, что:

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

Изучив исходники оригинальной библиотеки, я узнал, что текущая ошибка — порождение исключительно java\-порта\. В оригинале для элементов используется [функция](https://github.com/bulletphysics/bullet3/blob/6bb8d1123d8a55d407b19fd3357c724d0f5c9d3c/src/Bullet3Collision/BroadPhaseCollision/b3OverlappingPairCache.cpp#L112) с названием _b3Swap_\. Я хочу, чтобы вы посмотрели, как она выглядит:

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

Что\-то напоминает, не правда ли? :\)

Учитывая реализацию на C\+\+, которую мы уже увидели, определить исправление нам будет совсем не сложно:

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

### Ссылка \!\= значение

Представлю вам фрагмент кода, который мне кажется очень странным:

Файл [CProfileNode\.java\(70\)](https://github.com/stephengold/jbullet/blob/master/src/com/bulletphysics/linearmath/CProfileNode.java#L70)

```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;
}
```

Срабатывание диагностики:

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

Разработчик здесь допустил, наверное, одну из самых популярных по неприятности ошибку в Java — сравнение _String_ переменных по ссылке, а не по значению\.

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

Само сравнение может как работать, так и не работать\. Суть в том, что Java при помощи механизма [пула строк](https://www.baeldung.com/java-string-pool) будет стараться сделать так, чтобы строки с одинаковым внутренним содержимым являлись одинаковыми объектами, но не факт, что получится :\) Вполне возможен сценарий, что будут сравниваться объекты, которые имеют разные ссылки, при этом их содержимое одинаково\. И в таком случае сравнение даст нам отрицательный результат, хотя мы ожидали истину\.

### Copy\-Paste\-ориентированное программирование

Давайте взглянем на самый частый пример Copy\-Paste ошибок:

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

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

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

В этом примере программист возвращает поле из метода, хотя по его сигнатуре было бы логично вернуть другое\. Чаще всего такие ошибки появляются из\-за невнимательности\. Программист скопировал, поменял название метода, забыл поменять возвращаемое поле, и в итоге мы видим срабатывание анализатора\. И да, кстати, вот оно:

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

## Заключение

На этом на сегодня всё\. Все ошибки я собрал как в статье, так и в pull request'ах для разработчиков этого проекта \([тык](https://github.com/stephengold/jbullet/pull/1), [тык](https://github.com/jMonkeyEngine/sdk/pull/593), [тык](https://github.com/jMonkeyEngine/jmonkeyengine/pull/2241)\)\. Уже второй раз мне удаётся помочь сообществу программистов, внося посильный вклад в Open Source проекты, и я этому очень рад\.

А вы хотите попробовать статический анализ? Вы всегда можете внедрить его в свой проект, используя инструмент PVS\-Studio\. Скачать триальную версию можно [здесь](https://pvs-studio.ru/ru/pvs-studio/try-free/)\. Для открытых проектов существует вариант [бесплатного лицензирования](https://pvs-studio.ru/ru/order/open-source-license/)\.