Недавно в сети появилась новость о выходе новой версии проекта fheroes2. В нашей компании многие сотрудники являются поклонниками серии игр «Герои Меча и Магии», и естественно, мы не могли пройти мимо и в процессе знакомства с проектом проверили его с помощью анализатора PVS-Studio. 
Знакомство с проектом
Free Heroes of Might and Magic II — это реализация игрового движка Heroes of Might and Magic II с открытым исходным кодом.Для того, чтобы играть в обновленную версию, вам понадобится оригинальная игра Heroes of Might and Magic II или хотя бы ее демо-версия, которую можно скачать с помощью исходного кода, распространяемого вместе с ней.
сценарий (в зависимости от операционной системы необходимо выбрать соответствующую версию).
После успешной сборки проекта я решил немного поностальгировать и запустить игру.
Для удобства я немного отредактировал файл fheroes2.cfg, задав следующие параметры:
Я также установил свое разрешение в параметре видео режим .heroes speed = 10 ai speed = 10 battle speed = 10
После всех манипуляций я запустил игру и увидел знакомый главный экран: 
Если вы не установили разрешение экрана или не хотите заморачиваться с конфигом, вы можете развернуть игру в полноэкранный режим с помощью клавиши f4. Далее я зашёл в стандартную игру.
Так как я скачал демо-версию, то доступна только одна карта - Broken Alliance. 
Очень удобно, что окна с картой, героями и настройками можно перемещать в нужные части экрана.
ИИ, у которого, судя по отзывам, были проблемы в предыдущих версиях игры, теперь довольно быстро осваивает карту и неплохо сражается.
В общем, я еще немного побегал и мне очень понравилось.
Последней доступной версией проекта на момент написания статьи была 0.8.4, она улучшила производительность игры на малопроизводительных устройствах, добавила большое количество геймплейных и косметических нововведений, о которых вы можете прочитать Здесь .
Также мое внимание привлекла надпись: «Устранено более сотни ошибок по сравнению с предыдущей версией».
Ну, судя по всему, разработчики серьезно следят за качеством исходного кода и, как видно из страницы проекта на GitHub, уже на постоянной основе используют статический анализатор Sonar Cxx, а иногда и проверяют проект анализатором Cppcheck. Мне кажется, если астрологи объявят неделю статического анализа, а разработчики добавят PVS-Studio в список своих утилит, ошибок станет еще меньше.
Давайте убедимся в этом, рассмотрев несколько проблемных фрагментов кода, которые я нашел с его помощью.
Кроме того, открытые проекты могут использовать анализатор PVS-Studio. бесплатно .
Микрооптимизации
Для разнообразия начнем не с ошибок, а с фрагментов кода, которые можно немного оптимизировать.Немного потому, что реальную оптимизацию нужно делать с помощью профайлеров.
Статические анализаторы не имеют данных о том, как часто выполняется тот или иной код, а потому не способны показать, где находится реальное узкое место.
Поэтому, кстати, набор предупреждений PVS-Studio, связанных с увеличением скорости работы, мы называем «микрооптимизациями».
Мы не ожидаем, что описанное здесь поможет каким-либо существенным образом ускорить игру.
Просто хотел обратить внимание на этот комплекс диагностики, который обычно не освещается в наших серийные статьи о проверке проектов с открытым исходным кодом и поэтому остается в тени.
Предупреждение №1 В823 Снижение производительности.
Объект может быть создан на месте в контейнере «список».
Рассмотрите возможность замены методов: «push_back» -> «emplace_back».
инструменты.
cpp 231 std::list<std::string> StringSplit( const std::string & str, .
) { std::list<std::string> list; size_t pos1 = 0; size_t pos2 = std::string::npos; while ( pos1 < str.size() && std::string::npos != (pos2 = str.find(sep, pos1))) { list.push_back( str.substr( pos1, pos2 - pos1 ) ); pos1 = pos2 + sep.size(); } .
}
Анализатор подсказывает, что в этом случае эффективнее будет использовать метод emplace_back .
В общем, простая замена метода отталкивать на emplace_back не даст повышения производительности, если аргументом является значение r. Однако в нашем случае класс станд::строка есть конструктор, создающий объект из двух итераторов (см.
И именно это позволит нам исключить ненужный вызов конструктора перемещения в сочетании с emplace_back : std::list<std::string> StringSplit( const std::string & str, .
) { std::list<std::string> list; size_t pos1 = 0; size_t pos2 = std::string::npos; while ( pos1 < str.size() && std::string::npos != (pos2 = str.find(sep, pos1))) { list.emplace_back(str.begin() + pos1, str.begin() + pos2); pos1 = pos2 + sep.size(); } .
}
Это полезно для повышения производительности, поскольку операция выполняется в цикле.
Хочу отметить, что это предупреждение важно, поскольку в ходе проверки было обнаружено более сотни подобных сообщений.
Вот некоторые из них:
- В823 Снижение производительности.
Объект может быть создан на месте в контейнере «loop_sounds».
Рассмотрите возможность замены методов: «push_back» -> «emplace_back».
агг.
cpp 461
- В823 Снижение производительности.
Объект может быть создан на месте в контейнереprojectileOffset. Рассмотрите возможность замены методов: «push_back» -> «emplace_back».
bin_info.cpp 183
- В823 Снижение производительности.
Объект может быть создан на месте в контейнере «действия».
Рассмотрите возможность замены методов: «push_back» -> «emplace_back».
ai_normal_battle.cpp 264
- В823 Снижение производительности.
Объект может быть создан на месте в контейнере «действия».
Рассмотрите возможность замены методов: «push_back» -> «emplace_back».
ai_normal_battle.cpp 288
- В823 Снижение производительности.
Объект может быть создан на месте в контейнере «действия».
Рассмотрите возможность замены методов: «push_back» -> «emplace_back».
ai_normal_battle.cpp 433
- и так далее
Функция strlen вызывалась несколько раз внутри тела цикла.
инструменты.
cpp 216 void StringReplace( std::string & dst,
const char * pred,
const std::string & src )
{
size_t pos = std::string::npos;
while ( std::string::npos != ( pos = dst.find( pred ) ) )
{
dst.replace( pos, std::strlen( pred ), src );
}
}
В этом случае функция стрлен вызывается на каждой итерации цикла с размером строки пред не меняется.
Код можно упростить самым тривиальным образом, вынеся расчет длины строки за пределы цикла и обозначив ее как константу.
void StringReplace( std::string & dst,
const char * pred,
const std::string & src )
{
size_t pos = std::string::npos;
const size_t predSize = std::strlen( pred);
while ( std::string::npos != ( pos = dst.find( pred ) ) )
{
dst.replace( pos, predSize, src );
}
}
Предупреждение №3 В827 Максимальный размер вектора optionAreas известен во время компиляции.
Рассмотрите возможность его предварительного выделения, вызвав optionAreas.reserve(6) Battle_dialogs.cpp 217 void Battle::DialogBattleSettings( .
)
{
std::vector optionAreas;
optionAreas.push_back( fheroes2::Rect( pos_rt.x + 36,
pos_rt.y + 47,
panelWidth,
panelHeight ) );
optionAreas.push_back( fheroes2::Rect( pos_rt.x + 128,
pos_rt.y + 47,
panelWidth,
panelHeight ) );
optionAreas.push_back( fheroes2::Rect( pos_rt.x + 220,
pos_rt.y + 47,
panelWidth,
panelHeight ) );
optionAreas.push_back( fheroes2::Rect( pos_rt.x + 36,
pos_rt.y + 157,
panelWidth,
panelHeight ) );
optionAreas.push_back( fheroes2::Rect( pos_rt.x + 128,
pos_rt.y + 157,
panelWidth,
panelHeight ) );
optionAreas.push_back( fheroes2::Rect( pos_rt.x + 220,
pos_rt.y + 157,
panelWidth,
panelHeight ) );
}
Анализатор обнаружил станд::вектор , максимальный размер которого известен на этапе компиляции.
Перед заполнением контейнера было бы гораздо эффективнее вызвать: optionAreas.reserve(6);
В этом случае вызовы отталкивать не приведет к перераспределению внутреннего буфера в векторе и перемещению элементов в новую область памяти.
Или, альтернативно, этот код можно переписать, используя станд::массив .
Предупреждения N4.0, 4.1.4.7
- В809 Проверка того, что значение указателя не равно NULL, не требуется.
Проверку if (armyBar) можно убрать.
Kingdom_overview.cpp 62
- В809 Проверка того, что значение указателя не равно NULL, не требуется.
Проверку if (artifactsBar) можно удалить.
Kingdom_overview.cpp 64
- В809 Проверка того, что значение указателя не равно NULL, не требуется.
Проверку if (seckillsBar) можно удалить.
Kingdom_overview.cpp 66
- В809 Проверка того, что значение указателя не равно NULL, не требуется.
Проверку if (primskillsBar) можно удалить.
Kingdom_overview.cpp 68
- В809 Проверка того, что значение указателя не равно NULL, не требуется.
Проверку if (armyBarGuard) можно убрать.
Kingdom_overview.cpp 279
- В809 Проверка того, что значение указателя не равно NULL, не требуется.
Проверку if (armyBarGuest) можно убрать.
Kingdom_overview.cpp 281
- В809 Проверка того, что значение указателя не равно NULL, не требуется.
Проверку if (dwellingsBar) можно удалить.
Kingdom_overview.cpp 283
void Clear( void )
{
if ( armyBar )
delete armyBar;
if ( artifactsBar )
delete artifactsBar;
if ( secskillsBar )
delete secskillsBar;
if ( primskillsBar )
delete primskillsBar;
}
void Clear( void )
{
if ( armyBarGuard )
delete armyBarGuard;
if ( armyBarGuest )
delete armyBarGuest;
if ( dwellingsBar )
delete dwellingsBar;
}
В этом случае можно провести рефакторинг кода, убрав из функций все проверки на нулевые указатели, поскольку оператор удалить справляется с этим правильно.
Прироста производительности это может и не дать (компилятор сам уберет проверки), но упростит код и сделает его более читабельным.
Общий анализ
Предупреждение N5 По этому фрагменту кода анализатор выдал 2 предупреждения:- V654 Условие 'i < originalPalette.size()' of loop is always false. battle_interface.cpp 3689
- V621 Рассмотрите возможность проверки оператора for. Возможно, цикл выполнится неправильно или не выполнится вообще.
Battle_interface.cpp 3689
void Battle::Interface::RedrawActionBloodLustSpell( Unit & target )
{
std::vector > originalPalette;
if ( target.Modes( SP_STONE ) )
{
originalPalette.push_back( PAL::GetPalette( PAL::GRAY ) );
}
else if ( target.Modes( CAP_MIRRORIMAGE ) )
{
originalPalette.push_back( PAL::GetPalette( PAL::MIRROR_IMAGE ) );
}
if ( !originalPalette.empty() )
{
for ( size_t i = 1; i < originalPalette.size(); ++i )
{
originalPalette[0] = PAL::CombinePalettes( originalPalette[0],
originalPalette[i] );
}
fheroes2::ApplyPalette( unitSprite, originalPalette[0] );
}
.
}
Как мы видим, здесь программист допустил ошибку в алгоритме.
По мере выполнения функции вектор оригиналПалитра увеличивается в размере на единицу или остается пустым.
В если расположен над циклом, мы войдём только тогда, когда оригиналPalette.size() равен единице.
Соответственно, переменная я никогда не будет меньше размера вектора, и из-за этого мы получим недостижимый кусок кода.
Предупреждение N6 В547 Выражение «palette.empty()» всегда истинно.
image_tool.cpp 32 const std::vector PALPAlette()
{
std::vector palette;
if (palette.empty()) //<=
{
palette.resize( 256 * 3 );
for ( size_t i = 0; i < palette.size(); ++i )
{
palette[i] = kb_pal[i] << 2;
}
}
return palette;
}
В этом случае анализатор видит, что мы заведомо создаем пустой вектор, эта проверка избыточна, а значит, можно упростить код, удалив его.
Предупреждение N7 В668 Нет смысла проверять указатель listlog на нулевое значение, так как память была выделена с помощью оператора new. Исключение будет сгенерировано в случае ошибки выделения памяти.
Battle_interface.cpp 986 Battle::Interface::Interface(.
) { .
listlog = new StatusListBox(); .
if ( listlog ) { .
} .
}
Анализатор обнаружил ситуацию, когда значение указателя, возвращаемого оператором новый , сравнивается с нулем.
Как правило, это означает, что программа, если она не сможет выделить память, будет вести себя не так, как ожидает программист. Если оператор новый не смог выделить память, то согласно языковому стандарту С++, выдается исключение станд::bad_alloc() Это означает, что данная проверка является избыточной.
Также были обнаружены еще два подобных обнаружения:
- В668 Нет смысла проверять указатель «elem» на нуль, так как память была выделена с помощью оператора «new».
Исключение будет сгенерировано в случае ошибки выделения памяти.
Battle_arena.cpp 1079
- В668 Нет смысла проверять указатель image на ноль, так как память была выделена с помощью оператора new. Исключение будет сгенерировано в случае ошибки выделения памяти.
Battle_arena.cpp 1095
void Battle::Interface::MouseLeftClickBoardAction( .
)
{
.
themes = GetSwordCursorDirection( Board::GetDirection( index,
_currentUnit->GetHeadIndex()));
.
if ( _currentUnit )
{
.
}
.
}
Указатель _currentUnit сначала разыменовывается, а затем проверяется на наличие НУЛЕВОЙ .
Это может означать одну из двух очевидных вещей: произойдет неопределенное поведение, если указатель действительно равен нулю, или указатель никогда не может быть нулевым, и программа всегда работает правильно.
Если подразумевается первый вариант, то проверку следует производить перед разыменованием.
Во втором варианте можно опустить лишнюю проверку.
Заключение
Как по мне, проект сейчас очень близок к оригинальной версии игры.Код проекта довольно качественный, что неудивительно, поскольку разработчики используют несколько статических анализаторов.
Однако нет предела совершенству, а еще меньшего количества ошибок можно добиться, добавив к используемым решениям PVS-Studio, который, как уже говорилось ранее, бесплатно для проектов с открытым исходным кодом.
В заключение хотелось бы сказать спасибо разработчикам, движок действительно классный.
Если вы ищете хороший и интересный проект с открытым исходным кодом, в котором можно поучаствовать, то fheroes2 — это то, что вам нужно.
Если вы хотите поделиться этой статьей с англоязычной аудиторией, воспользуйтесь ссылкой для перевода: Владислав Столяров.
Бесплатные Heroes of Might and Magic II: проект с открытым исходным кодом, частью которого вы захотите стать .
Теги: #игры #Разработка игр #с открытым кодом #C++ #Герои Меча и Магии #Герои Меча и Магии #pvs-studio #статический анализ #статический анализатор
-
Бамбук
19 Oct, 24 -
Фарадей, Майкл
19 Oct, 24 -
Intel Core I7-8086K (Часть 4)
19 Oct, 24 -
Диаграммы И Графики: Понимание Тафте
19 Oct, 24 -
Гимнастика Для Ума. Пара Головоломок
19 Oct, 24 -
Безопасность Правительственных Сайтов
19 Oct, 24
