Если вы занимаетесь разработкой ПО в сфере видеоигровой индустрии и задаётесь вопросом о том, что ещё можно сделать, чтобы повысить качество продукта \ упростить процесс разработки, и при этом не используете статический анализ - самое время начать. Сомневаетесь? Что ж, я попробую вас в этом убедить. Если же вам просто интересно посмотреть на ошибки, которые допускают разработчики из сферы видеоигровой индустрии, то, опять же, вы попали по адресу - специально для вас отобраны наиболее интересные.
Несмотря на то, что разработка видеоигр включает в себя большое количество этапов, одним из основных остаётся программирование. Даже если вы не пишете тысячи строк кода, так или иначе приходится использовать различные инструменты, от качества которых зависят в том числе удобство работы и конечный результат. Если же вы являетесь разработчиком таких инструментов (например, игровых движков), вам и так должно быть всё понятно.
Почему статический анализ полезен в сфере разработки программного обеспечения в целом?
Есть несколько основных причин:
Но так или иначе, это всё теория, а вам, наверняка, интересны более практические примеры. Что же, они представлены ниже.
Если вы дочитали до этого момента, что-то рассказывать вам о том, что такое Unreal Engine или что за компания такая - Epic Games - не стоит, и, если эти ребята не пользуются у вас авторитетом - напишите мне, кто тогда пользуется.
Команда PVS-Studio несколько раз сотрудничала с командой Epic Games, помогая им внедрять и использовать статический анализ в проекте Unreal Engine и исправлять ошибки \ ложные срабатывания, обнаруженные анализатором. Уверен, это был интересный и полезный опыт для обеих сторон.
Одним из следствий стало добавление в Unreal Engine специального флага, который позволил удобно интегрировать статический анализ в сборочную систему Unreal Engine проектов.
Ключевая мысль проста - ребята заботятся о качестве своего кода и используют для этого разные подходы, одним из которых стало внедрение статического анализа.
Джон Кармак, один из самых известных разработчиков в сфере видеоигровой индустрии, в своё время признал активное использование методики статического анализа одним из главных своих достижений как программиста: "The most important thing I have done as a programmer in recent years is to aggressively pursue static code analysis." В следующий раз, когда кто-то из ваших знакомых скажет, что статический анализ – инструмент для новичков, покажите ему эту цитату. Своё знакомство со статическим анализом Кармак описал в соответствующей статье, с которой я настоятельно рекомендую ознакомиться: как для мотивации, так и для общего развития.
Пожалуй, один из лучших способов показать пользу статического анализа – демонстрация его возможностей на практике. Этим, например, занимается команда PVS-Studio, проверяя open source проекты.
В выигрыше остаются все:
В общем – чем не доказательство эффективности и пользы?
Пока кто-то размышляет о том, стоит ли внедрять статический анализ в свой процесс разработки, некоторые команды уже вовсю его используют и извлекают пользу! Например, Rocksteady, Epic Games, ZeniMax Media, Oculus, Codemasters, Wargaming и другие (источник).
Сразу стоит оговориться, что это не какой-то мега-глобальный 'топ', а те ошибки, найденные анализатором PVS-Studio в играх и игровых движках, которые показались мне наиболее интересными.
Как обычно, для большего интереса предлагаю вам сначала самостоятельно попробовать найти ошибку в приведённом фрагменте кода, а лишь затем читать предупреждение анализатора и описание проблемы.
Десятое место
Источник: Ищем аномалии в X-Ray Engine
На десятом месте расположилась ошибка из игрового движка X-Ray Engine, использовавшегося в играх S.T.A.L.K.E.R. Те, кто играл в игры из этой серии, наверняка помнят много забавных (и не очень) багов, возникавших во время игрового процесса. Особенно это касается S.T.A.L.K.E.R.: Clear Sky, в которую без патчей играть было вообще нереально (до сих пор помню баг, 'убивавший' все сохранения). Как показал анализ игрового движка - баги в коде действительно есть, и их достаточно много. Ниже представлен один из них.
BOOL CActor::net_Spawn(CSE_Abstract* DC)
{
....
m_States.empty();
....
}Предупреждение PVS-Studio: V530 The return value of function 'empty' is required to be utilized.
Проблема проста - не используют возвращаемое логическое значение метода empty, указывающее, пуст ли контейнер. Судя по тому, что в приведённом выражении кроме вызова метода ничего нет, можно предположить, что разработчик хотел очистить контейнер, но ошибся, вызвав метод empty вместо clear.
Кто-то может сказать, что это слишком простая ошибка для Топ-10. Но в этом и её прелесть! Несмотря на то, что вот так, взглядом со стороны, ошибка кажется достаточно простой и очевидной, подобные 'простые' ошибки не перестают появляться (и обнаруживаться) в различных проектах.
Девятое место
Источник: Долгожданная проверка CryEngine V
Продолжаем раскрывать тему ошибок в игровых движках. На девятом месте - код из CryEngine V. В играх на этом движке я не замечал столько багов, сколько в играх на X-Ray Engine, но 'вскрытие' показало, что подозрительных фрагментов кода движок содержит немало.
void CCryDXGLDeviceContext::
OMGetBlendState(...., FLOAT BlendFactor[4], ....)
{
CCryDXGLBlendState::ToInterface(ppBlendState, m_spBlendState);
if ((*ppBlendState) != NULL)
(*ppBlendState)->AddRef();
BlendFactor[0] = m_auBlendFactor[0];
BlendFactor[1] = m_auBlendFactor[1];
BlendFactor[2] = m_auBlendFactor[2];
BlendFactor[2] = m_auBlendFactor[3];
*pSampleMask = m_uSampleMask;
}Предупреждение PVS-Studio: V519 The 'BlendFactor[2]' variable is assigned values twice successively. Perhaps this is a mistake.
Как неоднократно упоминалось в наших статьях, никто не застрахован от опечаток. На практике также неоднократно подтверждалось, что статический анализ отлично справляется с обнаружением copy-paste и опечаток. Переписывали значения из массива m_auBlendFactor в BlendFactor, но случайно опечатались и два раза написали BlendFactor[2]. Из-за этой путаницы в BlendFactor[2] будет записано значение из m_auBlendFactor[3], а в BlendFactor[3] значение не поменяется.
Восьмое место
Источник: Единорог в космосе: проверяем исходный код 'Space Engineers'
Немного изменим курс и перейдём от C++ к C#. Перед нами - код из проекта Space Engineers - игры-'песочницы' о строительстве и обслуживании космических структур. Сам я в игру не играл, но как сказал один из комментаторов статьи: "Я не сильно удивлён результатами :)". Что ж, интересные ошибки действительно удалось найти, ниже - две из них.
public void Init(string cueName)
{
....
if (m_arcade.Hash == MyStringHash.NullOrEmpty &&
m_realistic.Hash == MyStringHash.NullOrEmpty)
MySandboxGame.Log.WriteLine(string.Format(
"Could not find any sound for '{0}'", cueName));
else
{
if (m_arcade.IsNull)
string.Format(
"Could not find arcade sound for '{0}'", cueName);
if (m_realistic.IsNull)
string.Format(
"Could not find realistic sound for '{0}'", cueName);
}
}Предупреждения PVS-Studio:
Как видите, ошибки игнорирования возвращаемых значений методов встречаются не только в коде на C++, но и на C#. Метод String.Format составляет результирующую строку на основе строки формата и объектов для форматирования, после чего возвращает результат как возвращаемое значение метода. В коде выше в else-ветви есть два вызова string.Format, но их возвращаемые значения не используются. Скорее всего, необходимо было залогировать эти сообщения по аналогии с тем, как это сделано в then-ветви оператора if, используя метод MySandboxGame.Log.WriteLine.
Седьмое место
Источник: Проверка проекта Quake III Arena GPL
Я уже говорил, что статический анализ хорошо обнаруживает опечатки? Пример ниже - ещё одно тому подтверждение.
void Terrain_AddMovePoint(....) {
....
x = ( v[ 0 ] - p->origin[ 0 ] ) / p->scale_x;
y = ( v[ 1 ] - p->origin[ 1 ] ) / p->scale_x;
....
}Предупреждение PVS-Studio: V537 Consider reviewing the correctness of 'scale_x' item's usage.
Происходит запись в переменные x и y, но в обоих присвоениях в выражении вычисления значения используется подвыражение p->scale_x. Выглядит очень подозрительно и похоже на то, что во втором случае необходимо заменить выражение p->scale_x на p->scale_y.
Шестое место
Источник: Проверяем исходный C#-код Unity
Компания Unity Technologies не так давно открыла исходный код собственного игрового движка - Unity. Мы не могли пройти мимо такого события и при анализе нашли немало интересных мест. Ниже - одно из них.
public override bool IsValid()
{
....
return base.IsValid()
&& (pageSize >= 1 || pageSize <= 1000)
&& totalFilters <= 10;
}Предупреждение PVS-Studio: V3063 A part of conditional expression is always true if it is evaluated: pageSize <= 1000.
Имеем неверную проверку диапазона для pageSize. Скорее всего планировалось проверить, что значение pageSize лежит в пределах [1; 1000]. Однако допущена досадная опечатка: вместо оператора '&&' программист случайно написал '||'. Получается, что подвыражение на самом деле ничего не проверяет.
Пятое место
Источник: Анализируем ошибки в открытых компонентах Unity3D
На соседнем месте расположилась интересная ошибка из компонентов Unity3D. Статья-источник была написана более чем за год до того, как был открыт исходный код движка Unity. Тем не менее, уже тогда можно было найти что-то интересное.
public static CrawledMemorySnapshot Unpack(....)
{
....
var result = new CrawledMemorySnapshot
{
....
staticFields = packedSnapshot.typeDescriptions
.Where(t =>
t.staticFieldBytes != null &
t.staticFieldBytes.Length > 0)
.Select(t => UnpackStaticFields(t))
.ToArray()
....
};
....
}Предупреждение PVS-Studio: V3080 Possible null dereference. Consider inspecting 't.staticFieldBytes'.
Обратите внимание на лямбда-выражение, переданное в качестве аргумента методу Where. Судя по коду, коллекция typeDescriptions может содержать такие элементы, у которых член staticFieldBytes может иметь значение null. Именно поэтому, прежде чем обратиться к свойству Length, выполняется проверка, что staticFieldBytes != null. Вот только были перепутаны операторы, и вместо '&&' используется '&'. Это значит, что независимо от результата вычисления левого выражения (true \ false) правое выражение также будет вычисляться, из-за чего, если staticFieldBytes == null, при обращении к свойству Length будет сгенерировано исключение типа NullReferenceException. При использовании '&&' такой ситуации не возникло бы, так как, если staticFieldBytes == null, правая часть выражения не вычислялась бы.
Несмотря на то, что Unity – единственный движок, дважды попавший в данный "Топ-10", энтузиастам это не мешает создавать на нём много замечательных игр. В том числе – про борьбу с 'багами'.
Четвертое место
Источник: Анализ исходного кода движка Godot
Иногда встречаются интересные ошибки, связанные с пропущенными ключевыми словами. Пример: создаётся объект исключения, но никак не используется, так как разработчик забывает указать ключевое слово throw. Встречаются такие ошибки и в C# проектах, и в C++ проектах. Пропущенное ключевое слово встретилось и в игровом движке Godot Engine.
Variant Variant::get(const Variant& p_index, bool *r_valid) const
{
....
if (ie.type == InputEvent::ACTION)
{
if (str =="action")
{
valid=true;
return ie.action.action;
}
else if (str == "pressed")
{
valid=true;
ie.action.pressed;
}
}
....
}Предупреждение PVS-Studio: V607 Ownerless expression 'ie.action.pressed'.
В приведённом фрагменте кода видно, что в зависимости от значений ie.type и str хотели вернуть определённое значение типа Variant. Вот только один раз выражение возврата написали нормально - return ie.action.action;, а во второй раз забыли оператор return, из-за чего нужное значение не будет возвращено, а метод продолжит своё выполнение.
Третье место
Источник: PVS-Studio: анализируем код Doom 3
Вот мы и подобрались к "Топ-3". Замыкает его небольшой фрагмент исходного кода из игры Doom 3. Как я говорил выше, то, что со стороны ошибка выглядит просто и может быть непонятно, как можно так ошибиться, ещё ничего не значит - на практике каких типов ошибок только не встретишь...
void Sys_GetCurrentMemoryStatus( sysMemoryStats_t &stats ) {
....
memset( &statex, sizeof( statex ), 0 );
....
}Предупреждение PVS-Studio: V575 The 'memset' function processes '0' elements. Inspect the third argument.
Чтобы лучше понять проблему, стоит вспомнить сигнатуру функции memset:
void* memset(void* dest, int ch, size_t count);Сопоставив сигнатуру функции с её вызовом, можно заметить, что последние два аргумента перепутаны местами. Как следствие - какой-то блок памяти, который требовалось обнулить, так и останется неизменённым.
Второе место
Второе место заняла ошибка, найденная в C# коде игрового движка Xenko.
Источник: Ищем ошибки в игровом движке Xenko
private static ImageDescription
CreateDescription(TextureDimension dimension,
int width, int height, int depth, ....) { .... }
public static Image New3D(int width, int height, int depth, ....)
{
return new Image(CreateDescription(TextureDimension.Texture3D,
width, width, depth,
mipMapCount, format, 1),
dataPointer, 0, null, false);
}Предупреждение PVS-Studio: V3065 Parameter 'height' is not utilized inside method's body.
Ошибку допустили, передавая аргументы методу CreateDescription. Если посмотреть на его сигнатуру, можно увидеть, что второй, третий и четвёртый параметры имеют названия width, height и depth соответственно. При вызове же передаются аргументы width, width и depth. Подозрительно? Вот и анализатор тоже счёл это место достаточно подозрительным, чтобы выдать предупреждение.
Первое место
Источник: Долгожданная проверка Unreal Engine 4
Возглавляет наш сегодняшний "Топ-10" ошибка из игрового движка Unreal Engine. Как и в случае со статьёй "Toп 10 ошибок в C++ проектах за 2017 год", увидев приведённую ниже ошибку, я сразу понял, кто должен возглавить список самых интересных ошибок.
bool VertInfluencedByActiveBone(
FParticleEmitterInstance* Owner,
USkeletalMeshComponent* InSkelMeshComponent,
int32 InVertexIndex,
int32* OutBoneIndex = NULL);
void UParticleModuleLocationSkelVertSurface::Spawn(....)
{
....
int32 BoneIndex1, BoneIndex2, BoneIndex3;
BoneIndex1 = BoneIndex2 = BoneIndex3 = INDEX_NONE;
if(!VertInfluencedByActiveBone(
Owner, SourceComponent, VertIndex[0], &BoneIndex1) &&
!VertInfluencedByActiveBone(
Owner, SourceComponent, VertIndex[1], &BoneIndex2) &&
!VertInfluencedByActiveBone(
Owner, SourceComponent, VertIndex[2]) &BoneIndex3)
{
....
}Предупреждение PVS-Studio: V564 The '&' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '&&' operator.
Не удивлюсь, если, сопоставляя предупреждение анализатора и приведённый код, вы задались вопросом: "А где здесь, собственно, использование '&' вместо '&&'?". Но, если обратить внимание, что последний параметр функции VertInfluencedByActiveBone имеет значение по умолчанию, и упростить условное выражение оператора if, всё станет более понятно:
if (!foo(....) && !foo(....) && !foo(....) & arg)Обратите особое внимание на последнее подвыражение:
!VertInfluencedByActiveBone(Owner, SourceComponent, VertIndex[2])
&BoneIndex3Параметр со значением по умолчанию сыграл злую шутку - если бы значения по умолчанию не было, данный код бы просто не скомпилировался. Но так как оно есть, всё успешно компилируется, а ошибка не менее успешно маскируется. Вот это подозрительное место и заметил анализатор - инфиксная операция '&', в которой левый операнд имеет тип bool, а правый - int32.
Надеюсь, мне удалось показать, что статический анализ будет полезен при разработке игр и игровых движков и является ещё одним ответом на вопрос о повышении качества кода (и конечного продукта, как следствие). Если вы занимаетесь разработкой в сфере видеоигровой индустрии, то просто обязаны рассказать коллегам о статическом анализе и показать эту статью. Думаете с чего начать? Начните с PVS-Studio.