# Разбор ошибок в игровом движке Stride

Stride – это мощный, бесплатный и активно развивающийся игровой движок, реализованный на C\#\. Он вполне может стать альтернативой Unity, но насколько качественный исходный код Stride? Узнаем это с помощью статического анализатора PVS\-Studio\.

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

## Введение

Следствием активного развития проекта является не только внедрение нового функционала и возможностей, но и возникновение новых ошибок\. В этой статье мы разберём самые интересные из них\. К слову, разбор ошибок в проекте — отличный способ оценить его качество\. Однако проверка крупных проектов отнюдь не простая задача, поэтому найти ошибки в игровом движке нам поможет статический анализатор PVS\-Studio\.

Проверка проводилась на релизе Stride Game Engine 4\.1\.0\.1734\. Исходный код доступен на [GitHub](https://github.com/stride3d/stride/releases/tag/releases%2F4.1.0.1734)\.

## Разбор ошибок Stride Game Engine

### Использование анонимной функции для отмены подписки на событие

**Issue 1**

```cpp
private void RemoveEditor(....)
{
  ....
  if (....)
  {
    multiEditor.OpenedAssets.CollectionChanged -= (_, e) => 
      MultiEditorOpenAssetsChanged(multiEditor, e);
    ....
  }
  ....
}
```

Срабатывание PVS\-Studio: [V3084](https://pvs-studio.ru/ru/docs/warnings/v3084/)\. Anonymous function is used to unsubscribe from 'CollectionChanged' event\. No handlers will be unsubscribed, as a separate delegate instance is created for each anonymous function declaration\. AssetEditorsManager\.cs 444

В приведённом примере выполняется отписка от события _CollectionChanged_\. Однако это не сработает, т\. к\. в качестве ссылки на объект обработчика используется анонимная функция\. При её объявлении создается новый экземпляр делегата, отличный от ранее подписанного на _CollectionChanged_\. Даже если тела функций идентичны, они всё равно будут представлять разные объекты\. Таким образом, отписка от события с помощью анонимной функции не произойдёт и значение _CollectionChanged_ никак не изменится\.

### Риск null\-разыменования при обращении к свойству

**Issue 2**

```cpp
private void SetIsVisible(bool isVisible)
{
  if (isVisible)
    PreviewService.OnShowPreview(); // <=
  else
    PreviewService.OnHidePreview(); // <=
}
```

Срабатывание PVS\-Studio: [V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/)\. Possible null dereference\. Consider inspecting 'PreviewService'\. PreviewViewModel\.cs 109

В данном случае анализатор предупреждает о возможном разыменовании нулевой ссылки\. Обратите внимание на метод _SetIsVisible_\. Здесь отсутствуют проверки _PreviewService_ на _null_\. На их необходимость указывает реализация этого свойства:

```cpp
private IAssetPreviewService PreviewService
{
  get
  {
    if (previewService != null)
      return previewService;

    previewService = ServiceProvider.TryGet<IAssetPreviewService>();
    if (previewService == null)
      return null;                   

    previewService.PreviewAssetUpdated += PreviewAssetUpdated;
      return previewService;
  }
}
```

Из реализации видно, что _PreviewService_ действительно может вернуть _null_\. Следовательно, угроза возникновения ошибки вполне реальна и требует соответствующей обработки\. Возможным решением проблемы может стать добавление условия:

```cpp
private void SetIsVisible(bool isVisible)
{
  if (PreviewService != null)       // <=
  {
    if (isVisible)
      PreviewService.OnShowPreview(); 
    else
      PreviewService.OnHidePreview(); 
  }
}
```

### Лишнее присваивание значения

**Issue 3**

```cpp
public TexAtlas(...., TexImage atlas) : base(....)
{
  ....
  Name = atlas.Name;
  ....
  Name = "";
}
```

Срабатывание PVS\-Studio: [V3008](https://pvs-studio.ru/ru/docs/warnings/v3008/)\. The 'Name' variable is assigned values twice successively\. Perhaps this is a mistake\. TexAtlas\.cs 48, 43

В конструкторе свойству _Name_ два раза присваивается значение\. Предположительно, разработчик хотел протестировать функционал класса при пустом _Name_\. Для этого он добавил второе присваивание, но позднее забыл его удалить\. Теперь, независимо от входных параметров конструктора, этому свойству всегда будет присваиваться пустая строка\. Это неизбежно приведёт к нарушению в логике программы, использующей значение этого свойства\.

### Потерялась проверка на null

**Issue 4**

```cpp
public override void EndScreenDeviceChange(....)
{
  ....
  window.FullscreenIsBorderlessWindow = FullscreenIsBorderlessWindow; // <=
  if (....)
  {
    ....
    if (window != null)
    {
      window.ClientSize = new Size2(clientWidth, clientHeight);
    }
    ....
  }
  else
  {
    ....
    if (window != null)
    {
      window.ClientSize = new Size2(clientWidth, clientHeight);
      window.Location = savedFormLocation;
      ....
      window.BringToFront();
    }

  }
  ....
}
```

Срабатывание PVS\-Studio: [V3095](https://pvs-studio.ru/ru/docs/warnings/v3095/)\. The 'window' object was used before it was verified against null\. GameWindowSDL\.cs 64, 70\.

В начале метода _EndScreenDeviceChange_ объект _window_ используется без предварительной проверки на _null_, о чём и предупреждает анализатор\. На актуальность этого предупреждения указывает последующий код: все обращения к переменной выполняются внутри блока условного оператора, реализующего такую проверку\.

Предупреждения анализатора о потенциальных ошибках, приводящих к исключению типа _NullReferenceException,_ встречаются при проверке C\# проектов намного чаще, чем предупреждения других типов\. Наверное, стоит уделять этой проблеме отдельное внимание при написании кода\. Давайте рассмотрим ещё одно срабатывание такого же типа_\._

**Issue 5**

```cpp
private async Task Update()
{
  while (....)
  {
    ....
    if (....)
    {
      ....
      materialFilterRenderFeature.MaterialFilter =
        (materialFilterRenderFeature != null && ....)? ....
      ....
    }
  }
}
```

Срабатывание PVS\-Studio: [V3095](https://pvs-studio.ru/ru/docs/warnings/v3095/)\. The 'materialFilterRenderFeature' object was used before it was verified against null\. EditorGameRenderModeService\.cs 106, 107\.

В отличие от предыдущего случая, в этом примере есть проверка на _null_, но реализована она несколько\.\.\. странно\. Выполняется обращение к свойству _MaterialFilter_ объекта _materialFilterRenderFeature_\. Далее этому свойству присваивается результат выполнения тернарного оператора\. Условное выражение этого оператора содержит проверку _materialFilterRenderFeature_ на _null_\. Однако в ней нет никакой необходимости\. В случае если _materialFilterRenderFeature_ равен _null_, выполнение программы благополучно завершится исключением _NullReferenceException_ ещё при обращении к свойству _MaterialFilter_ :\)\.

### Когда проверка на null приводит к NullReferenceException

**Issue 6**

```cpp
public Palette GlobalPalette
{
  get {....}
  set
  {
    SetTagValue("GlobalPalette", (value != null) ? null : value.Data);
  }
}
```

Срабатывание PVS\-Studio: [V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/)\. Possible null dereference\. Consider inspecting 'value'\. MetadataModels\.cs 132

Методу _SetTagValue_ в качестве второго аргумента передаётся тернарное выражение\. Это выражение проверяет наличие значения _null_ у переменной _value_\. Если _value_ равно _null_, в качестве аргумента методу передаётся _value\.Data\._ Обращение к свойству этой переменной неизбежно приведёт к вызову исключения _NullReferenceException_\. Предположительно, разработчик перепутал операнды местами\. Таким образом, корректная реализация тернарного оператора может выглядеть так:

```cpp
(value != null) ? value.Data : null
```

### Одинаковое условие, но разное предназначение

**Issue 7**

```cpp
internal IObjectNode SetTarget(....)
{
  if (....)
  {
    var targetValue = targetNode.Retrieve();

    if (targetValue != null && !type.IsInstanceOfType(targetValue))
      throw new InvalidOperationException(@"The type of the retrieved" +
                                           "node content does not match" +
                                           "the type of this reference");

    if (targetValue != null && !type.IsInstanceOfType(targetValue))
      throw new InvalidOperationException("TargetNode type does not " +
                                          "match the reference type.");

    ....
  }
  ....
}
```

Срабатывание PVS\-Studio: [V3021](https://pvs-studio.ru/ru/docs/warnings/v3021/)\. 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\.ObjectReference\.cs 134

Анализатор обнаружил два похожих оператора _if_ с идентичными условиями\. В теле обоих условных операторов вызывается исключение, тем самым прерывая выполнение программы\. На первый взгляд, здесь нет ничего страшного, просто немного лишнего кода\. Однако обратите внимание на сообщения, передаваемые в исключения, — они разные\. Можно предположить, что разработчик хотел реализовать две различные проверки\. Возможно, он скопировал одну из них и поменял сообщение, но забыл изменить условное выражение\. Таким образом, одна из возможных ошибок осталась необработанной и всё ещё представляет угрозу для корректной работы программы\.

Исходя из сообщения второго исключения, можно предположить, что корректная реализация соответствующего _if_ должна выглядеть так:

```cpp
if (targetNode!= null && !type.IsInstanceOfType(targetNode))
  throw new InvalidOperationException("TargetNode type does not " +
                                      "match the reference type.");
```

Рассмотрим аналогичный случай, найденный анализатором в этом игровом движке\.

**Issue 8**

```cpp
private GraphViewModel(....) : base(serviceProvider)
{
  ....
  if (rootPresenters == null) 
    throw new ArgumentNullException(nameof(rootPresenters));
  ....
  if (rootPresenters == null) 
    throw new ArgumentNullException(nameof(rootNode));
  ....
}
```

Срабатывание PVS\-Studio: [V3021](https://pvs-studio.ru/ru/docs/warnings/v3021/)\. 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\. GraphViewModel\.cs 40

Перед нами снова два оператора _if_ с одинаковыми условиями\. Как и в предыдущем случае, на ошибку указывают аргументы, передаваемые в конструктор исключения\. В этот раз они являются наименованиями проверяемых переменных\. Очевидно, что ошибка допущена во втором условном операторе\. Его корректная реализация, скорее всего, должна иметь следующий вид:

```cpp
if (rootNode == null)
  throw new ArgumentNullException(nameof(rootNode));
```

### Copy paste ошибки или какой case лишний?

**Issue 9**

```cpp
public override void AddPoint(....)
{
  ....
  switch (Component)
  {
    case VectorComponent.X:
      value.X = realPoint.Y;
      break;

    case VectorComponent.Y:
      value.Y = realPoint.Y;
      break;

    case VectorComponent.Z:
      value.Z = realPoint.Y; // <=
      break;

    case VectorComponent.W:
      value.Z = realPoint.Y; // <=
      break;

    default:
      throw new NotSupportedException();
  }
  ....
}
```

Срабатывание PVS\-Studio: [V3139](https://pvs-studio.ru/ru/docs/warnings/v3139/)\. Two or more case\-branches perform the same actions\.  RotationComponentCurveViewModel\.cs 53

Перед вами _switch_, операторы case которого очень похожи друг на друга\. Метки каждого из них соответствуют шаблону: _VectorComponent\.\{0\}_, где вместо _\{0\}_ может быть любое из следующих свойств: _X_, _Y_, _Z_ или _W_\. В теле каждого оператора _case_ выполняется присваивание вида _value\.\{0\} \= realPoint\.Y_\. А теперь обратите внимание на последний из них\. Он имеет метку _VectorComponent\.W_, а в теле производится операция _value\.Z \= realPoint\.Y_\. Она идентична присваиванию в предыдущем _case_\. Согласно выявленному нами шаблону, вместо _value\.Z_ в данном выражении должно быть _value\.W_\. 

Таким образом, корректная реализация последнего _case_ может выглядеть так:

```cpp
case VectorComponent.W:
  value.W = realPoint.Y; 
  break;
```

### Не стоит жалеть кода на обработку NullReferenceException

**Issue 10**

```cpp
protected override Size ArrangeOverride(....)
{
  ....
  if (....)
  {
    for (....)
    {
      var child = itemsControl.ItemContainerGenerator
                              .ContainerFromIndex(i) as TreeViewItem;
      ....
      if (child != null)
      {
        child.Arrange(....);
        currentY += child.ActualHeight;
      }
      ....
    }
  }
  else
  {
    for (....)
    {
      var child = itemsControl.ItemContainerGenerator
                              .ContainerFromIndex(i) as UIElement;
            
      child?.Arrange(....);
      currentY += child.DesiredSize.Height; // <=
    }
  }
  ....
}
```

Срабатывание PVS\-Studio: [V3125](https://pvs-studio.ru/ru/docs/warnings/v3125/)\. The 'child' object was used after it was verified against null\. VirtualizingTreePanel\.cs 430, 429

Обратите внимание на блоки _then_ и _else_ условного оператора\. Они чем\-то похожи, не так ли? В обоих выполняется цикл, внутри которого переменная _child_ инициализируется с помощью оператора _as_\. Но есть одно существенное отличие – в первом цикле все обращения к переменной _child_ защищены от _NullReferenceException_ соответствующей проверкой, а вот во втором случае – не совсем\. Можно предположить, что разработчик намеренно не стал добавлять явную проверку, обработав первое обращение к _child_ оператором _null\-conditional_:

```cpp
child?.Arrange(....);
```

При этом он не обратил внимания на второе обращение:

```cpp
currentY += child.DesiredSize.Height;
```

Результатом этой операции все также может стать выбрасывание исключения _NullReferenceException_\.

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

Потенциально опасные фрагменты кода, найденные в Stride Game Engine, оказались вполне интересными и определённо заслуживают внимания\. Но насколько же оказались верны срабатывания анализатора? Мне кажется, точно ответить на этот вопрос могут только разработчики игрового движка, проведя собственное исследование с помощью PVS\-Studio\. 

В любом случае отсутствие потенциально опасного кода лучше, чем его наличие\. Статический анализатор позволяет быстро обнаружить такой код, тем самым помогая свести количество ошибок в проекте к минимуму, не тратя огромное количество времени и сил на их поиск\.

Вы легко можете в этом убедиться, скачав [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/try-free/) и опробовав его на своих проектах в течение пробного периода\.

Для наилучшего пользовательского опыта рекомендую ознакомиться с основами использования анализатора\. Это можно сделать, прочитав соответствующие разделы в [документации](https://pvs-studio.ru/ru/docs/)\.

На этом статья завершается\. Надеюсь, она была вам интересна :\)\.

Чистого вам кода и успешных проектов\! До встречи в следующих статьях\!