# Flax Engine\. Знакомство с игровым движком и анализ его исходного кода "Как будто у Unreal и Unity родился ребёнок" — такое трогательное описание дали этому движку в GameDev\-сообществе\. Эта фраза не только мило звучит, но и точно передаёт его суть, ведь движок действительно задумывался как нечто среднее между Unity Engine и Unreal Engine\. ![1153_FlaxEngine_ru/image1.png](https://import.viva64.com/docx/blog/1153_FlaxEngine_ru/image1.png) ## Введение Привет вам, дорогие читатели\! Я хочу познакомить вас с ещё одной интересной находкой с бескрайних просторов GitHub\. Flax Engine — это полноценный мультиплатформенный коммерческий игровой движок от польских разработчиков\. В этой статье мы кратко рассмотрим основные особенности движка, а после разберём наиболее интересные проблемы, найденные в его исходном коде с помощью статического анализатора кода PVS\-Studio\. Это хороший способ немного прокачать своё умение замечать баги и опечатки в C\# и C\+\+ коде\. ## PVS\-Studio? Что это? PVS\-Studio — это статический анализатор кода, предназначенный для поиска потенциальных ошибок и уязвимостей в С\#, C, C\+\+ и Java коде без его фактического выполнения\. С помощью PVS\-Studio можно проанализировать как отдельные файлы, так и проект целиком\. Результатом такого анализа является отчёт с предупреждениями, с которым можно удобно работать через специальную панель в ваших любимых IDE: ![1153_FlaxEngine_ru/image2.png](https://import.viva64.com/docx/blog/1153_FlaxEngine_ru/image2.png) При желании вы можете получить больше информации об анализаторе, посетив официальный [сайт](https://pvs-studio.ru/ru/), а сейчас давайте вернемся к движку\! ## О Flax Engine Итак, уважаемые читатели, знакомьтесь, это Flax Engine: ![1153_FlaxEngine_ru/image3.png](https://import.viva64.com/docx/blog/1153_FlaxEngine_ru/image3.png) Что же выделяет Flax среди других движков? * Поддержка скриптинга как на С\+\+, так и на C\#\. Также имеется возможность визуального программирования; * Поддержка \.NET 8 и C\# 12, что означает возможность не только использовать новейшие функции языка, но и получить прирост в производительности за счёт значительных улучшений среды выполнения\. Для более подробного ознакомления с этими нововведениями можете обратиться к нашей [статье](https://pvs-studio.ru/ru/blog/posts/csharp/1080/)\. Также хочется отметить, что на момент написания статьи, в Unity поддержка этих версий отсутствует; * Если вам показалось, что данный движок больше ориентирован на C\# разработку, нежели на C\+\+, то вы ошибаетесь\. Напротив, ядро движка реализовано на С\+\+, что позволяет соответствующим разработчикам использовать его API напрямую\. Это даёт преимущество в производительности, а также возможность использовать некоторые дополнительные API; * Наличие весьма солидного для Open Source движка набора инструментов, со списком которых можно ознакомиться на [отдельной странице](https://flaxengine.com/features/) движка; * Абсолютно весь функционал движка можно использовать бесплатно, если доход от ваших проектов не превышает $250000 за квартал\. При преодолении этого порога стоимость движка будет равна 4% прибыли от ваших проектов, реализованных на его основе\. Теперь, когда мы немного познакомились с движком, почему бы не узнать, что думает анализатор [PVS\-Studio](https://pvs-studio.ru/ru/) о его исходном коде? Далее мы рассмотрим потенциальные проблемы в C\# и C\+\+ коде самой актуальной на момент написания статьи версии движка — [1\.8\.6512\.2](https://github.com/FlaxEngine/FlaxEngine/releases/tag/1.8.6512.2)\. Готовы? Тогда поехали\! ## Разбор ошибок в C\# коде Flax Engine ### Избыточные проверки условий **Case 1** ```cpp public override string TypeDescription { get { // Translate asset type name var typeName = TypeName; string[] typeNamespaces = typeName.Split('.'); if ( typeNamespaces.Length != 0 && typeNamespaces.Length != 0) // <= { typeName = Utilities.Utils .GetPropertyNameUI( typeNamespaces[typeNamespaces.Length - 1]); } return typeName; } } ``` Предупреждение PVS\-Studio: [V3001](https://pvs-studio.ru/ru/docs/warnings/v3001/) There are identical sub\-expressions 'typeNamespaces\.Length \!\= 0' to the left and to the right of the '&&' operator\. AssetItem\.cs: [83](https://github.com/FlaxEngine/FlaxEngine/blob/009e1aaebba30f4ee7198a1e8212de91f59386bf/Source/Editor/Content/Items/AssetItem.cs#L83)\. Анализатор обнаружил два идентичных выражения, связанных оператором _&&_\. Несомненно, одно из них было указано по ошибке\. Но является ли это просто избыточным кодом или все же признаком более серьезной проблемы? Например, вместо второго повторяющегося неравенства могла предполагаться следующая проверка: ```cpp typeNamespaces[typeNamespaces.Length - 1].Length != 0 ``` Вот ещё один аналогичный случай, найденный в проекте\. **Case 2** ```cpp public override bool OnMouseDown(Float2 location, MouseButton button) { .... if (_rightMouseDown || (_middleMouseDown && _middleMouseDown)) // <= { // Start navigating StartMouseCapture(); Focus(); return true; } .... } ``` Предупреждение PVS\-Studio: [V3001](https://pvs-studio.ru/ru/docs/warnings/v3001/) There are identical sub\-expressions '\_middleMouseDown' to the left and to the right of the '&&' operator\. VisjectSurface\.Input\.cs: [495](https://github.com/FlaxEngine/FlaxEngine/blob/009e1aaebba30f4ee7198a1e8212de91f59386bf/Source/Editor/Surface/VisjectSurface.Input.cs#L495)\. ### Ошибка из\-за невнимательного copy\-paste ```cpp partial class Window { .... public void Show() .... public void Hide() .... } public class ContextMenuBase : ContainerControl { private Window _window; .... public void Show() // <= { _window.Show(); } public void Hide() // <= { _window.Show(); } public void Minimize() { _window.Minimize(); } } ``` Предупреждение PVS\-Studio: [V3013](https://pvs-studio.ru/ru/docs/warnings/v3013/) It is odd that the body of 'Show' function is fully equivalent to the body of 'Hide' function \(70, line 78\)\. WindowRootControl\.cs: [70](https://github.com/FlaxEngine/FlaxEngine/blob/009e1aaebba30f4ee7198a1e8212de91f59386bf/Source/Engine/UI/GUI/WindowRootControl.cs#L70), [78](https://github.com/FlaxEngine/FlaxEngine/blob/009e1aaebba30f4ee7198a1e8212de91f59386bf/Source/Engine/UI/GUI/WindowRootControl.cs#L78)\. Типичная ошибка из\-за невнимательности при использовании Copy\-Paste\. В методе _Hide_ вызывается метод _\_window\.Show _вместо_ \_window\.Hide_\. ### Выход за границы массива ```cpp public Matrix2x2(float[] values) { .... if (values.Length != 4) throw new ArgumentOutOfRangeException(....); M11 = values[0]; M12 = values[1]; M21 = values[3]; M22 = values[4]; // <= } ``` Предупреждение PVS\-Studio: [V3106](https://pvs-studio.ru/ru/docs/warnings/v3106/) Possibly index is out of bound\. The '4' index is pointing beyond 'values' bound\. Matrix2x2\.cs: [98](https://github.com/FlaxEngine/FlaxEngine/blob/009e1aaebba30f4ee7198a1e8212de91f59386bf/Source/Engine/Core/Math/Matrix2x2.cs#L98)\. Из условия видно, что в массиве _values_ может быть только строго четыре элемента\. Так как индексация элементов начинается с 0, индекс самого последнего элемента массива будет равен 3\. Однако в последнем присваивании выполняется обращение по индексу 4, что непременно приведёт к исключению\. ### Недостижимый код **Case 1** ```cpp public void SetMemberValue(object instance, object value) { .... if (type.IsEnum) value = Convert.ChangeType(value, Enum.GetUnderlyingType(type.Type)); else if (type.Type == typeof(byte)) value = Convert.ToByte(value); else if (type.Type == typeof(sbyte)) value = Convert.ToSByte(value); else if (type.Type == typeof(short)) value = Convert.ToInt16(value); else if (type.Type == typeof(int)) // <= value = Convert.ToInt32(value); else if (type.Type == typeof(long)) value = Convert.ToInt64(value); else if (type.Type == typeof(int)) // <= value = Convert.ToUInt16(value); .... } ``` Предупреждение PVS\-Studio: [V3003](https://pvs-studio.ru/ru/docs/warnings/v3003/)\. The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 78, 82\. MemberComparison\.cs: [78](https://github.com/FlaxEngine/FlaxEngine/blob/009e1aaebba30f4ee7198a1e8212de91f59386bf/Source/Editor/Utilities/MemberComparison.cs#L78), [82](https://github.com/FlaxEngine/FlaxEngine/blob/009e1aaebba30f4ee7198a1e8212de91f59386bf/Source/Editor/Utilities/MemberComparison.cs#L82)\. Анализатор обнаружил два одинаковых условия внутри связанных _else if_\. В случае, если _type\.Type_ будет иметь значение _int_, код выполнится только в теле первого из них, а код в теле второго будет недостижим\. Так как в теле второго рассматриваемого _else if_ выполняется конвертация к типу _UInt16_, логичным решением была бы замена соответствующего условия на следующее: ```cpp type.Type == typeof(ushort) ``` **Case 2** ```cpp private void UpdateDragPositioning(....) { if (....) _dragOverMode = DragItemPositioning.Above; else if (....) _dragOverMode = DragItemPositioning.Below; else _dragOverMode = DragItemPositioning.At; // Update DraggedOverNode var tree = ParentTree; if (_dragOverMode == DragItemPositioning.None) // <= { if (tree != null && tree.DraggedOverNode == this) tree.DraggedOverNode = null; } else if (tree != null) tree.DraggedOverNode = this; } ``` Предупреждение PVS\-Studio: [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression '\_dragOverMode \=\= DragItemPositioning\.None' is always false\. TreeNode\.cs: [566](https://github.com/FlaxEngine/FlaxEngine/blob/009e1aaebba30f4ee7198a1e8212de91f59386bf/Source/Editor/GUI/Tree/TreeNode.cs#L566)\. Анализатор обнаружил всегда ложное _if_\-условие, из\-за чего код внутри _then_\-ветви никогда не будет выполнен\. Обратите внимание, что в результате выполнения первой условной конструкции поле _\_dragOverMode_ всегда получает новое значение, отличное от _DragItemPositioning\.None_\. Из\-за этого следующий _if_ становится бессмысленным\. Не исключено, что это было сделано намеренно, а лишний код просто забыли убрать\. Но если это все\-таки ошибка, одним из вариантов её исправления может быть перенос первой условной конструкции из начала метода в его конец\. Таким образом, исправленный вариант мог бы выглядеть так: ```cpp private void UpdateDragPositioning(....) { // Update DraggedOverNode var tree = ParentTree; if (_dragOverMode == DragItemPositioning.None) .... if (....) _dragOverMode = DragItemPositioning.Above; else if (....) _dragOverMode = DragItemPositioning.Below; else _dragOverMode = DragItemPositioning.At; } ``` Это решение не должно нарушить логику метода, но наверняка проверить это могут только разработчики движка\. ### Проблема с синхронизацией потоков из\-за оптимизаций компилятора ```cpp protected Window _window; .... public DialogResult ShowDialog(Window parentWindow) { // Show window Show(parentWindow); // Set wait flag Interlocked.Increment(ref _isWaitingForDialog); // Wait for the closing do { Thread.Sleep(1); } while (_window); // <= // Clear wait flag Interlocked.Decrement(ref _isWaitingForDialog); return _result; } ``` Предупреждение PVS\-Studio: [V3032](https://pvs-studio.ru/ru/docs/warnings/v3032/) Waiting on this expression is unreliable, as compiler may optimize some of the variables\. Use volatile variable\(s\) or synchronization primitives to avoid this\. Dialog\.cs: [108](https://github.com/FlaxEngine/FlaxEngine/blob/009e1aaebba30f4ee7198a1e8212de91f59386bf/Source/Editor/GUI/Dialogs/Dialog.cs#L108)\. Этот код содержит потенциальную проблему, которую невозможно отловить при работе с **Debug** конфигурацией\. В чём же секрет неуловимости этого зверя? Дело в том, что возникнуть эта проблема может только при сборке Release версии вследствие оптимизации кода компилятором\. Помимо этого, её возникновение зависит и от других факторов, таких как используемая версия \.NET и количество процессоров в системе\. Суть проблемы заключается в бесконечном зацикливании _while_ из\-за возможного кеширования компилятором значения поля _\_window_\. Это может произойти из\-за того, что значение поля _\_window_ никак не меняется внутри самого цикла, а изменение в других потоках не ожидается компилятором, так как поле было объявлено без ключевого слова _volatile_\. Подробнее об этом можно прочитать в [MSDN](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/volatile?redirectedfrom=MSDN)\. ## Разбор ошибок в C\+\+ коде Flax Engine ### Потенциальное разыменование nullptr **Case 1** ```cpp void Append(const T* data, int32 length) { .... auto prev = Base::_data; .... Base::_data = (T*)Allocator::Allocate(Base::_length * sizeof(T)); Platform::MemoryCopy(Base::_data, prev, prevLength * sizeof(T)); // <= .... if (_isAllocated && prev) .... } ``` Предупреждение PVS\-Studio: [V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'prev' pointer was utilized before it was verified against nullptr\. Check lines: 'PlatformBase\.h: [178](https://github.com/FlaxEngine/FlaxEngine/blob/009e1aaebba30f4ee7198a1e8212de91f59386bf/Source/Engine/Platform/Base/PlatformBase.h#L178)', 'DataContainer\.h: [339](https://github.com/FlaxEngine/FlaxEngine/blob/009e1aaebba30f4ee7198a1e8212de91f59386bf/Source/Engine/Core/Types/DataContainer.h#L339)', 'DataContainer\.h: [342](https://github.com/FlaxEngine/FlaxEngine/blob/009e1aaebba30f4ee7198a1e8212de91f59386bf/Source/Engine/Core/Types/DataContainer.h#L342)'\. DataContainer\.h [339](https://github.com/FlaxEngine/FlaxEngine/blob/009e1aaebba30f4ee7198a1e8212de91f59386bf/Source/Engine/Core/Types/DataContainer.h#L339)\. Анализатор указывает на использование _prev_ в качестве второго аргумента метода _MemoryCopy_\. Потенциальная проблема заключается в том, что _prev_ может иметь значение _nullptr,_ на что указывает соответствующая проверка\. Но действительно ли передача _nullptr_ в _MemoryCopy_ опасна? Вдруг этот случай обрабатывается внутри самого метода? Чтобы ответить на эти вопросы, взглянем на реализацию _MemoryCopy_: ```cpp FORCE_INLINE static void MemoryCopy(void* dst, const void* src, uint64 size) { memcpy(dst, src, static_cast(size)); } ``` Теперь видно, что значение второго параметра напрямую передаётся в функцию [_memcpy_](https://en.cppreference.com/w/cpp/string/byte/memcpy) без какой\-либо предварительной проверки на неравенство _nullptr_, что создаёт риск возникновения [неопределённого поведения](https://pvs-studio.ru/ru/blog/terms/0066/#:~:text=Неопределённое%20поведение%20(англ.%20undefined%20behaviour),результат%20операции%20Б%20не%20определён)\. Вдумчивый читатель может не согласиться с этим, возразив: "Здесь же дополнительно передаётся _size_ \(количество байт, которое необходимо скопировать\), а значит, если этот параметр равен 0, то и копирование выполнено не будет"\. Увы, но это не совсем так\. Документация на функцию _memcpy_ ясно даёт понять, что в неё нельзя передавать невалидные указатели\. Даже если количество копируемых данных равно нулю: "If either dest or src is an invalid or null pointer, the behavior is undefined, even if count is zero" **Case 2** ```cpp void Variant::SetAsset(Asset* asset) { if (Type.Type != VariantType::Asset) SetType(VariantType(VariantType::Asset)); if (AsAsset) { asset->OnUnloaded.Unbind(this); asset->RemoveReference(); // <= } AsAsset = asset; if (asset) { asset->AddReference(); asset->OnUnloaded.Bind(this); } } ``` Предупреждение PVS\-Studio: [V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'asset' pointer was utilized before it was verified against nullptr\. Check lines: 2706, 2709\. Variant\.cpp: [2706](https://github.com/FlaxEngine/FlaxEngine/blob/009e1aaebba30f4ee7198a1e8212de91f59386bf/Source/Engine/Core/Types/Variant.cpp#L2706), [2709](https://github.com/FlaxEngine/FlaxEngine/blob/009e1aaebba30f4ee7198a1e8212de91f59386bf/Source/Engine/Core/Types/Variant.cpp#L2709)\. Весьма странный код\. Кажется, что в теле первого оператора _if_ должно обрабатываться старое значение _AsAsset_ перед его заменой на новое, на что указывает условие этого _if_\. Также на ошибку в этом месте указывает условие второго _if,_ в котором выполняется проверка параметра _asset_ на неравенство _nullptr_\. Если ожидается, что _asset_ может быть равен _nullptr_, значит его разыменование внутри первого _if_ может привести к [неопределённому поведению](https://pvs-studio.ru/ru/blog/terms/0066/#:~:text=Неопределённое%20поведение%20(англ.%20undefined%20behaviour),результат%20операции%20Б%20не%20определён)\. Наиболее логичным исправлением в данном случае является замена _asset_ на _AsAsset _внутри первого _if_: ```cpp void Variant::SetAsset(Asset* asset) { .... if (AsAsset) { AsAsset ->OnUnloaded.Unbind(this); AsAsset ->RemoveReference(); // <= } AsAsset = asset; if (asset) { asset->AddReference(); asset->OnUnloaded.Bind(this); } } ``` ### Потенциальная ошибка при побитовом сдвиге ```cpp void StringUtils::ConvertUTF162UTF8(....) { Array unicode; .... const uint32 uni = unicode[j]; const uint32 count = uni <= 0x7FU ? 1 : uni <= 0x7FFU ? 2 : uni <= 0xFFFFU ? 3 : uni <= 0x1FFFFFU ? 4 : uni <= 0x3FFFFFFU ? 5 : uni <= 0x7FFFFFFFU ? 6 : 7; // <= to[i++] = (char)(count <= 1 ? (byte)uni : ((byte(0xFFU) << (8 - count)) | byte(uni >> (6 * (count - 1))))); // <= .... } ``` Предупреждение PVS\-Studio: [V610](https://pvs-studio.ru/ru/docs/warnings/v610/) Undefined behavior\. Check the shift operator '\>\>'\. The right operand \('\(6 \* \(count \- 1\)\)' \= \[6\.\.36\]\) is greater than or equal to the length in bits of the promoted left operand\. StringUtilsBase\.cpp\. [253](https://github.com/FlaxEngine/FlaxEngine/blob/009e1aaebba30f4ee7198a1e8212de91f59386bf/Source/Engine/Platform/Base/StringUtilsBase.cpp#L253)\. Анализатор обращает внимание на возможность получить неожиданный результат при битовом смещении в выражении _uni \>\> \(6 \* \(count \- 1\)\)_\. Произойти это может из\-за того, что _uni_ имеет тип _int32_\. Смещение этого значения на 32 бита и более вправо может привести к [неопределённому поведению](https://pvs-studio.ru/ru/blog/terms/0066/#:~:text=Неопределённое%20поведение%20(англ.%20undefined%20behaviour),результат%20операции%20Б%20не%20определён)\. Анализатор вычислил, что наибольший возможный шаг при побитовом сдвиге переменной _uni_ равен 36\. Как он это определил? Обратите внимание на тернарный оператор, используемый для инициализации переменной _count_: ```cpp const uint32 count = uni <= 0x7FU ? 1 : uni <= 0x7FFU ? 2 : uni <= 0xFFFFU ? 3 : uni <= 0x1FFFFFU ? 4 : uni <= 0x3FFFFFFU ? 5 : uni <= 0x7FFFFFFFU ? 6 : 7; ``` Видно, что максимальное значение, которое может быть присвоено переменной, равно 7\. А теперь подставим это значение в выражение, представляющее собой шаг сдвига: ```cpp 6 * (count - 1) = 6 * (7 – 1) = 36 ``` Что же, в этот раз анализатор оказался прав\. Дело закрыто\! ## Заключение На этом статья завершается, надеюсь, она вам понравилась :\) А ведь это далеко не первый игровой движок, исходный код которого мы проверяли\. На случай, если вам захочется ознакомиться с другими нашими похожими статьями, оставлю ссылки на них ниже\. Статьи с разбором проблем в C\+\+ коде: * [От винта\! Смотрим движок War Thunder и говорим с его создателями](https://pvs-studio.ru/ru/blog/posts/cpp/1098/) * [Проверка игрового движка qdEngine, часть первая: топ 10 предупреждений PVS\-Studio](https://pvs-studio.ru/ru/blog/posts/cpp/1119/) * [Проверка игрового движка qdEngine, часть вторая: упрощение C\+\+ кода](https://pvs-studio.ru/ru/blog/posts/cpp/1121/) * [Проверка игрового движка qdEngine, часть третья: дополнительная десятка багов](https://pvs-studio.ru/ru/blog/posts/cpp/1123/) * [Герои Кода и Магии: анализ игрового движка VCMI](https://pvs-studio.ru/ru/blog/posts/cpp/1058/) Статьи с разбором проблем в C\# коде: * [Повторная проверка Unity статическим анализатором PVS\-Studio](https://pvs-studio.ru/ru/blog/posts/csharp/0932/) * [Разбор ошибок в игровом движке Stride](https://pvs-studio.ru/ru/blog/posts/csharp/0994/) * [Игра с null: проверка MonoGame статическим анализатором PVS\-Studio](https://pvs-studio.ru/ru/blog/posts/csharp/0915/) Надеюсь, вас также заинтересовал инструмент, который использовался для поиска описанных выше проблем в обширной кодовой базе движка\. Пользуясь моментом, хочу предложить познакомиться и с ним, запросив пробную лицензию на нашем [сайте](https://pvs-studio.ru/ru/pvs-studio/try-free/)\. После этого вы сможете бесплатно опробовать весь функционал анализатора PVS\-Studio в течение пробного периода\. До встречи в следующих статьях\!