# Cataclysm Dark Days Ahead, статический анализ и рогалики

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

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

В своем поиске необычных игр я наткнулась на игру Cataclysm Dark Days Ahead, отличающуюся от других необычной графикой: она реализована с помощью разноцветных символов ASCII на черном фоне\.

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

Это игра с открытым исходным кодом, и, к тому же, написана на С\+\+\. Так что невозможно было пройти мимо и не прогнать этот проект через статический анализатор PVS\-Studio, в разработке которого я сейчас принимаю активное участие\. Сам проект удивил высоким качеством кода, однако, в нем все равно находятся некоторые недоработки и несколько из них я рассмотрю в этой статье\.

К настоящему моменту с помощью PVS\-Studio было проверено уже достаточно много игр\. Например, вы можете ознакомиться с другой нашей статьей "[Статический анализ в видеоигровой индустрии: топ\-10 программных ошибок](https://habr.com/ru/company/pvs-studio/blog/354808/)"\.

## Логика

**Пример 1:**

[V728](https://pvs-studio.ru/ru/docs/warnings/v728/) An excessive check can be simplified\. The '\(A && B\) \|\| \(\!A && \!B\)' expression is equivalent to the 'bool\(A\) \=\= bool\(B\)' expression\. inventory\_ui\.cpp 199

```cpp
bool inventory_selector_preset::sort_compare( .... ) const
{
  ....
  const bool left_fav  = g->u.inv.assigned.count( lhs.location->invlet );
  const bool right_fav = g->u.inv.assigned.count( rhs.location->invlet );
  if( ( left_fav && right_fav ) || ( !left_fav && !right_fav ) ) {
    return ....
  } 
  ....
}
```

Ошибки в условии нет, но оно излишне усложнено\. Стоило бы сжалиться над тем, кому придется разбирать это условие, и написать проще _if\( left\_fav \=\= right\_fav \)_ \.

Аналогичное предупреждение:

* V728 An excessive check can be simplified\. The '\(A && \!B\) \|\| \(\!A && B\)' expression is equivalent to the 'bool\(A\) \!\= bool\(B\)' expression\. iuse\_actor\.cpp 2653

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

## Отступление I

Для меня оказалось открытием, что игры, которые на сегодняшний день называют "рогаликами" \- это лишь достаточно лайтовые последователи старого жанра roguelike игр\. Началось все с культовой игры Rogue 1980 года, ставшей образцом для подражания и вдохновившей множество студентов и программистов на создание собственных игр\. Полагаю, многое также привнесло сообщество настольной ролевой игры DnD и её вариаций\.

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

## Микрооптимизации

**Пример 2:**

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

[V801](https://pvs-studio.ru/ru/docs/warnings/v801/) Decreased performance\. It is better to redefine the second function argument as a reference\. Consider replacing 'const \.\. type' with 'const \.\. &type'\. map\.cpp 4644

```cpp
template <typename Stack>
std::list<item> use_amount_stack( Stack stack, const itype_id type )
{
  std::list<item> ret;
  for( auto a = stack.begin(); a != stack.end() && quantity > 0; ) {
      if( a->use_amount( type, ret ) ) {
          a = stack.erase( a );
      } else {
          ++a;
      }
  }
  return ret;
}
```

Здесь за _itype\_id_ скрывается _std::string_\. Так как аргумент все равно передается константным, что не позволит его изменить, быстрее было бы просто передать в функцию ссылку на переменную и не тратить ресурсы на копирование\. И хотя, скорее всего, строка там будет совсем небольшая, но постоянное копирование без видимой на то причины излишне\. Тем более, что эта функция вызывается из разных мест, многие из которых, в свою очередь, также получают _type_ извне и копируют его\.

Аналогичные предупреждения:

* V801 Decreased performance\. It is better to redefine the third function argument as a reference\. Consider replacing 'const \.\. evt\_filter' with 'const \.\. &evt\_filter'\. input\.cpp 691
* V801 Decreased performance\. It is better to redefine the fifth function argument as a reference\. Consider replacing 'const \.\. color' with 'const \.\. &color'\. output\.h 207
* В целом, анализатор выдал 32 таких предупреждения\.

**Пример 3:**

[V813](https://pvs-studio.ru/ru/docs/warnings/v813/) Decreased performance\. The 'str' argument should probably be rendered as a constant reference\. catacharset\.cpp 256

```cpp
std::string base64_encode( std::string str )
{
  if( str.length() > 0 && str[0] == '#' ) {
    return str;
  }
  int input_length = str.length();
  std::string encoded_data( output_length, '\0' );
  ....
  for( int i = 0, j = 0; i < input_length; ) {
    ....
  }
  for( int i = 0; i < mod_table[input_length % 3]; i++ ) {
    encoded_data[output_length - 1 - i] = '=';
  }
  return "#" + encoded_data;
}
```

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

Это предупреждение также было не единичным, всего таких случаев нашлось 26\.

![0628_ASCII_Cataclysm_ru/image4.png](https://import.viva64.com/docx/blog/0628_ASCII_Cataclysm_ru/image4.png)

Аналогичные предупреждения:

* V813 Decreased performance\. The 'message' argument should probably be rendered as a constant reference\. json\.cpp 1452
* V813 Decreased performance\. The 's' argument should probably be rendered as a constant reference\. catacharset\.cpp 218
* И так далее\.\.\.

## Отступление II

Некоторые из классических roguelike игр до сих пор активно развиваются\. Если зайти в репозитории GitHub Cataclysm DDA или NetHack, то можно увидеть, что изменения активно вносятся каждый день\. NetHack вообще является самой старой игрой, разработка которой идет до сих пор: её релиз произошел в июле 1987 года, а последняя версия датируется 2018 годом\.

Одной из известных, однако, более поздних игр этого жанра является Dwarf Fortress, разрабатываемая с 2002 года и впервые выпущенная в 2006 году\. "Losing is fun" \("Проигрывать весело"\) \- девиз игры, в точности отражающий её суть, так как победить в ней невозможно\. Эта игра в 2007 году заслужила звание лучшей roguelike игры года в результате голосования, которое ежегодно проводится на сайте ASCII GAMES\.

![0628_ASCII_Cataclysm_ru/image5.png](https://import.viva64.com/docx/blog/0628_ASCII_Cataclysm_ru/image5.png)

Кстати, тем, кто интересуется этой игрой, возможно будет интересна следующая новость\. Dwarf Fortress выйдет в Steam с улучшенной 32\-битной графикой\. С обновлённой картинкой, над которой работают два опытных модера игры, премиум\-версия Dwarf Fortress получит дополнительные музыкальные треки и поддержку Steam Workshop\. Но если что, владельцы платной версии Dwarf Fortress смогут поменять обновлённую графику на прежний вид в ASCII\. [Подробнее](https://www.patreon.com/posts/25343688)\.

## Переопределение оператора присваивания

**Примеры 4, 5:**

Также нашлась интересная пара сходных предупреждений\.

[V690](https://pvs-studio.ru/ru/docs/warnings/v690/) The 'JsonObject' class implements a copy constructor, but lacks the '\=' operator\. It is dangerous to use such a class\. json\.h 647

```cpp
class JsonObject
{
  private:
  ....
  JsonIn *jsin;
  ....

  public:
  JsonObject( JsonIn &jsin );
  JsonObject( const JsonObject &jsobj );
  JsonObject() : positions(), start( 0 ), end( 0 ), jsin( NULL ) {}
  ~JsonObject() {
    finish();
  }
  void finish(); // moves the stream to the end of the object
  ....
  void JsonObject::finish()
  {
    ....
  }
  ....
}
```

Этот класс обладает конструктором копирования и деструктором, однако, для него отсутствует перегрузка оператора присваивания\. Проблема здесь состоит в том, что автоматически сгенерированный оператор присваивания может лишь присвоить указатель к _JsonIn_\. В результате оба объекта класса _JsonObject_ указывают на один тот же _JsonIn_\. Неизвестно, может ли где\-то сейчас возникнуть такая ситуация, но, в любом случае, это \- грабли, на которые рано или поздно кто\-то наступит\.

Аналогичная проблема присутствует в следующем классе\.

[V690](https://pvs-studio.ru/ru/docs/warnings/v690/) The 'JsonArray' class implements a copy constructor, but lacks the '\=' operator\. It is dangerous to use such a class\. json\.h 820

```cpp
class JsonArray
{
  private:
  ....
  JsonIn *jsin;
  ....

  public:
  JsonArray( JsonIn &jsin );
  JsonArray( const JsonArray &jsarr );
  JsonArray() : positions(), ...., jsin( NULL ) {};
  ~JsonArray() {
    finish();
  }

  void finish(); // move the stream position to the end of the array
  void JsonArray::finish()
  {
    ....
  }
}
```

Более подробно об опасности нехватки перегрузки оператора присваивания для сложного класса можно почитать в статье "[The Law of The Big Two](https://www.artima.com/cppsource/bigtwo.html)" \(или в переводе этой статьи "[CИ\+\+: Закон Большой Двойки](https://habr.com/ru/post/31346/)"\)\.

**Примеры 6, 7:**

Еще один пример, связанный с перегруженным оператором присваивания, но на этот раз речь о его конкретной реализации\.

[V794](https://pvs-studio.ru/ru/docs/warnings/v794/) The assignment operator should be protected from the case of 'this \=\= &other'\. mattack\_common\.h 49

```cpp
class StringRef {
  public:
    ....
  private:
    friend struct StringRefTestAccess;
    char const* m_start;
    size_type m_size;
    char* m_data = nullptr;
    ....
auto operator = ( StringRef const &other ) noexcept -> StringRef& {
  delete[] m_data;
  m_data = nullptr;
  m_start = other.m_start;
  m_size = other.m_size;
  return *this;
}
```

Проблема в том, что данная реализация не защищена от присвоения объекта самому себе, что является небезопасной практикой\. То есть если этому оператору будет передана ссылка на _\*this_, может произойти утечка памяти\.

Схожий пример ошибочной перегрузки оператора присваивания с интересным побочным эффектом:

[V794](https://pvs-studio.ru/ru/docs/warnings/v794/) The assignment operator should be protected from the case of 'this \=\= &rhs'\. player\_activity\.cpp 38

```cpp
player_activity &player_activity::operator=( const player_activity &rhs )
{
  type = rhs.type;
  ....
  targets.clear();
  targets.reserve( rhs.targets.size() );

  std::transform( rhs.targets.begin(),
                  rhs.targets.end(),
                  std::back_inserter( targets ),
                  []( const item_location & e ) {
                    return e.clone();
                  } );

  return *this;
}
```

В этом случае точно так же отсутствует проверка на присваивание объекта самому себе\. Но в дополнение заполняется вектор\. Если попытаться такой перегрузкой присвоить объект самому себе, то в поле _targets_ получим удвоенный вектор, часть элементов которого испорчена\. Однако здесь перед _transform_ присутствует _clear_, что очистит вектор объекта и данные будут потеряны\.

![0628_ASCII_Cataclysm_ru/image6.png](https://import.viva64.com/docx/blog/0628_ASCII_Cataclysm_ru/image6.png)

## Отступление III

В 2008 году рогалики даже обзавелись формальным определением, которое получило эпичное название "Берлинская интерпретация"\. Согласно этому определению, основными чертами таких игр являются:

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

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

Обычная ситуация в Cataclysm DDA: промерзли и голодны до смерти, вас мучает жажда, да и вообще у вас вместо ног шесть тентаклей\.

![0628_ASCII_Cataclysm_ru/image7.png](https://import.viva64.com/docx/blog/0628_ASCII_Cataclysm_ru/image7.png)

## Немаловажные детали

**Пример 8:**

[V1028](https://pvs-studio.ru/ru/docs/warnings/v1028/) Possible overflow\. Consider casting operands of the 'start \+ larger' operator to the 'size\_t' type, not the result\. worldfactory\.cpp 638

```cpp
void worldfactory::draw_mod_list( int &start, .... )
{
  ....
  int larger = ....;
  unsigned int iNum = ....;  
  ....
  for( .... )
  {
    if(   iNum >= static_cast<size_t>( start )
       && iNum < static_cast<size_t>( start + larger ) )
    {
      ....
    }
    ....
  }
....
}
```

Похоже, программист хотел избежать переполнения\. Но приведение результата сложения в таком случае бессмысленно, так как переполнение возникнет уже при сложении чисел, и расширение типов произведется над бессмысленным результатом\. Для того, чтобы избежать этой ситуации, необходимо привести лишь один из аргументов к большему типу: _\(static\_cast<size\_t\> \(start\) \+ larger\)_\.

**Пример 9:**

[V530](https://pvs-studio.ru/ru/docs/warnings/v530/) The return value of function 'size' is required to be utilized\. worldfactory\.cpp 1340

```cpp
bool worldfactory::world_need_lua_build( std::string world_name )
{
#ifndef LUA
....
#endif
    // Prevent unused var error when LUA and RELEASE enabled.
    world_name.size();
    return false;
}
```

Для таких случаев существует небольшая хитрость\. Если переменная оказывается неиспользованной, вместо того, чтобы пытаться вызвать какой\-либо метод, можно просто написать _\(void\)world\_name_ для подавления предупреждения компилятора\.

**Пример 10:**

[V812](https://pvs-studio.ru/ru/docs/warnings/v812/) Decreased performance\. Ineffective use of the 'count' function\. It can possibly be replaced by the call to the 'find' function\. player\.cpp 9600

```cpp
bool player::read( int inventory_position, const bool continuous )
{
  ....
  player_activity activity;

  if(   !continuous
     || !std::all_of( learners.begin(),
                      learners.end(), 
                      [&]( std::pair<npc *, std::string> elem )
                      {
                        return std::count( activity.values.begin(),
                                           activity.values.end(), 
                                           elem.first->getID() ) != 0;
                      } )
  {
    ....
  }
  ....
}
```

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

**Пример 11:**

Следующая ошибка легко обнаруживается, если знать об одной тонкости\.

[V739](https://pvs-studio.ru/ru/docs/warnings/v739/) EOF should not be compared with a value of the 'char' type\. The 'ch' should be of the 'int' type\. json\.cpp 762

```cpp
void JsonIn::skip_separator()
{
  signed char ch;
  ....
  if (ch == ',') {
    if( ate_separator ) {
      ....
    }
    ....
  } else if (ch == EOF) {
  ....
}
```

![0628_ASCII_Cataclysm_ru/image8.png](https://import.viva64.com/docx/blog/0628_ASCII_Cataclysm_ru/image8.png)

Это одна из тех ошибок, которые бывает сложно заметить, если не знать, что _EOF_ определен как \-1\. Соответственно, если пытаться сравнивать его с переменной типа _signed char_, условие почти всегда оказывается _false_\. Единственное исключение, это если кодом символа будет 0xFF \(255\)\. При сравнении такой символ превратится в \-1 и условие окажется верным\.

**Пример 12:**

Следующая небольшая ошибка однажды может стать критической\. Не зря она есть в списке CWE как [CWE\-834](https://cwe.mitre.org/data/definitions/834.html)\. А их, кстати, было целых пять\.

[V663](https://pvs-studio.ru/ru/docs/warnings/v663/) Infinite loop is possible\. The 'cin\.eof\(\)' condition is insufficient to break from the loop\. Consider adding the 'cin\.fail\(\)' function call to the conditional expression\. action\.cpp 46

```cpp

void parse_keymap( std::istream &keymap_txt, .... )
  {
    while( !keymap_txt.eof() ) {
    ....
  }
}
```

Как сказано в предупреждении, проверки на достижение конца файла при чтении недостаточно, необходимо также проводить проверку на ошибку считывания _cin\.fail\(\)_\. Изменим код для более безопасного считывания:

```cpp
while( !keymap_txt.eof() )
{
  if(keymap_txt.fail())
  {
    keymap_txt.clear();
    keymap_txt.ignore(numeric_limits<streamsize>::max(),'\n');
    break;
  }
  ....
}
```

_keymap\_txt\.clear\(\)_ нужен для того, чтобы при ошибке считывания из файла убрать из потока состояние \(флажок\) ошибки, иначе дальше считать текст будет нельзя\. _keymap\_txt\.ignore_ с параметрами _numeric\_limits<streamsize\>::max\(\)_ и управляющим символом перевода строки позволяет пропустить оставшуюся часть строки\. 

Есть и гораздо более простой способ остановки считывания:

```cpp
while( !keymap_txt )
{
  ....
}
```

Если использовать поток в контексте логики, он конвертирует себя в значение эквивалентное _true_, пока не будет достигнут _EOF_\.

## Отступление IV

Сейчас наибольшую популярность имеют игры, которые сочетают в себе признаки roguelike игр и других жанров: платформеров, стратегий и др\. Такие игры стали называть roguelike\-like или roguelite\. К таким играм относят такие известные тайтлы, как Don't Starve, The Binding of Isaac, FTL:Faster Than Light, Darkest Dungeon и даже Diablo\.

Хотя временами разница между roguelike и roguelite столь мала, что не понятно, к какому жанру отнести игру\. Кто\-то считает, что Dwarf Fortress уже не roguelike, а для кого\-то и Diablo \- классический рогалик\.

![0628_ASCII_Cataclysm_ru/image9.png](https://import.viva64.com/docx/blog/0628_ASCII_Cataclysm_ru/image9.png)

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

Хоть проект в целом и является примером качественного кода, и не удалось найти много серьёзных ошибок, это не значит, что использование статического анализа для него является избыточным\. Суть не в разовых проверках, которые делаем с целью популяризации методологии статического анализа кода, а в регулярном использовании анализатора\. Тогда многие ошибки можно выявить на самом раннем этапе и, следовательно, сократить стоимость их исправления\. [Пример](https://pvs-studio.ru/ru/blog/posts/0606/) расчётов\.

![0628_ASCII_Cataclysm_ru/image10.png](https://import.viva64.com/docx/blog/0628_ASCII_Cataclysm_ru/image10.png)

Над рассмотренной игрой и сейчас ведется активная работа, и существует активное сообщество моддеров\. Причем она портирована на множество платформ, в том числе iOS и Android\. Так что, если вас заинтересовала эта игра, рекомендую попробовать\!