Cataclysm Dark Days Ahead, Análisis estático y panecillos

Cuadro 10

Lo más probable es que, por el título del artículo, ya haya adivinado que el foco está en los errores en el código fuente. Pero esto no es lo único que se discutirá en este artículo. Si además de C ++ y errores en el código de otra persona, te atraen los juegos inusuales y te interesa saber qué son estos "bagels" y qué comen con ellos, ¡bienvenido a cat!

En mi búsqueda de juegos inusuales, me topé con el juego Cataclysm Dark Days Ahead, que difiere de otros gráficos inusuales: se implementa utilizando caracteres ASCII multicolores sobre un fondo negro.

Lo que llama la atención en este juego y su suerte es cuánto se implementa todo en ellos. Específicamente, en Cataclysm, por ejemplo, incluso para crear un personaje, quiero buscar guías, ya que hay docenas de diferentes parámetros, características y tramas iniciales, sin mencionar las variaciones de eventos en el juego en sí.

Este es un juego de código abierto, y también escrito en C ++. Por lo tanto, era imposible pasar por alto y no ejecutar este proyecto a través del analizador estático PVS-Studio, en cuyo desarrollo ahora participo activamente. El proyecto en sí me sorprendió con la alta calidad del código, sin embargo, todavía contiene algunos defectos y discutiré varios de ellos en este artículo.

Hasta la fecha, se han probado muchos juegos con PVS-Studio. Por ejemplo, puede leer nuestro otro artículo, " Análisis estático en la industria de los videojuegos: los 10 principales errores de software ".

La lógica


Ejemplo 1:

El siguiente ejemplo es un error de copia típico.

V501 Hay subexpresiones idénticas a la izquierda y a la derecha de '||' operador: rng (2, 7) <abs (z) || rng (2, 7) <abs (z) overmap.cpp 1503

bool overmap::generate_sub( const int z ) { .... if( rng( 2, 7 ) < abs( z ) || rng( 2, 7 ) < abs( z ) ) { .... } .... } 

Aquí la misma condición se verifica dos veces. Lo más probable es que la expresión se haya copiado y se olvidó de cambiar algo en ella. Me resulta difícil decir si este error es significativo, pero la verificación no funciona según lo previsto.

Una advertencia similar:
  • V501 Hay subexpresiones idénticas 'one_in (100000 / to_turns <int> (dur))' a la izquierda y a la derecha del operador '&&'. player_hardcoded_effects.cpp 547

Cuadro 9

Ejemplo 2

V728 Se puede simplificar una verificación excesiva. El '(A y B) || (! A &&! B) 'expresión es equivalente a la expresión' bool (A) == bool (B) '. Inventory_ui.cpp 199

 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 .... } .... } 

No hay error en la condición, pero es innecesariamente complicado. Merecería la pena compadecerse de aquellos que tienen que desmontar esta condición, y es más fácil escribir if (left_fav == right_fav) .

Una advertencia similar:

  • V728 Se puede simplificar una verificación excesiva. El '(A &&! B) || (! A && B) 'expresión es equivalente a la expresión' bool (A)! = Bool (B) '. iuse_actor.cpp 2653

Retiro I


Resultó ser un descubrimiento para mí que los juegos que hoy se llaman "bagels" son seguidores bastante leves del viejo género de los juegos roguelike. Todo comenzó con el juego de culto Rogue de 1980, que se convirtió en un modelo a seguir e inspiró a muchos estudiantes y programadores a crear sus propios juegos. Creo que la comunidad de juego de roles de la junta de DnD también ha aportado mucho y sus variaciones.

Cuadro 8

Microoptimización


Ejemplo 3

El siguiente grupo de advertencias del analizador no indica un error, sino la posibilidad de microoptimización del código del programa.

V801 Disminución del rendimiento. Es mejor redefinir el argumento de la segunda función como referencia. Considere reemplazar 'const ... type' con 'const ... & type'. map.cpp 4644

 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; } 

Aquí itdpe_id oculta std :: string . Dado que el argumento aún se pasa constante, lo que no permitirá que se cambie, sería más rápido simplemente pasar una referencia variable a la función y no desperdiciar recursos en la copia. Y aunque, muy probablemente, la línea allí será muy pequeña, pero la copia constante sin razón aparente es innecesaria. Además, esta función se llama desde diferentes lugares, muchos de los cuales, a su vez, también obtienen el tipo del exterior y lo copian.

Advertencias similares:

  • V801 Disminución del rendimiento. Es mejor redefinir el argumento de la tercera función como referencia. Considere reemplazar 'const ... evt_filter' con 'const ... & evt_filter'. input.cpp 691
  • V801 Disminución del rendimiento. Es mejor redefinir el argumento de la quinta función como referencia. Considere reemplazar 'const ... color' con 'const ... & color'. salida.h 207
  • En total, el analizador generó 32 de tales advertencias.

Ejemplo 4

V813 Disminución del rendimiento. El argumento 'str' probablemente debería representarse como una referencia constante. catacharset.cpp 256

 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; } 

En este caso, el argumento, aunque no es constante, no cambia en el cuerpo de la función. Por lo tanto, para la optimización, sería bueno pasarlo a través de un enlace constante y no forzar al compilador a crear copias locales.

Esta advertencia tampoco fue única, hubo 26 casos en total.

Cuadro 7

Advertencias similares:

  • V813 Disminución del rendimiento. El argumento 'mensaje' probablemente debería representarse como una referencia constante. json.cpp 1452
  • V813 Disminución del rendimiento. El argumento 's' probablemente debería representarse como una referencia constante. catacharset.cpp 218
  • Y así sucesivamente ...

Retiro II


Algunos de los juegos clásicos de roguelike todavía se están desarrollando. Si va a los repositorios GitHub Cataclysm DDA o NetHack, puede ver que los cambios se realizan activamente todos los días. NetHack es generalmente el juego más antiguo que aún se está desarrollando: fue lanzado en julio de 1987 y la última versión data de 2018.

Sin embargo, uno de los famosos juegos posteriores de este género es Dwarf Fortress, desarrollado desde 2002 y lanzado por primera vez en 2006. "Perder es divertido" es el lema del juego, que refleja con precisión su esencia, ya que es imposible ganarlo. Este juego en 2007 obtuvo el título de mejor juego roguelike del año como resultado de la votación, que se celebra anualmente en el sitio web de ASCII GAMES.

Cuadro 6

Por cierto, aquellos que estén interesados ​​en este juego pueden estar interesados ​​en las siguientes noticias. Dwarf Fortress se lanzará en Steam con gráficos mejorados de 32 bits. Con una imagen actualizada en la que están trabajando dos moderadores experimentados, la versión premium de Dwarf Fortress recibirá pistas de música adicionales y soporte para Steam Workshop. Pero, en todo caso, los propietarios de la versión paga de Dwarf Fortress podrán cambiar los gráficos actualizados al formulario anterior en ASCII. Más detalles

Operador de asignación de anulación


Ejemplos 5, 6:

También hubo un par interesante de advertencias similares.

V690 La clase 'JsonObject' implementa un constructor de copia, pero carece del operador '='. Es peligroso usar tal clase. json.h 647

 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() { .... } .... } 

Esta clase tiene un constructor y destructor de copia, sin embargo, no sobrecarga el operador de asignación. El problema aquí es que un operador de asignación generado automáticamente solo puede asignar un puntero a JsonIn . Como resultado, ambos objetos de la clase JsonObject apuntan al mismo JsonIn . No se sabe si tal situación podría surgir en algún lugar ahora, pero, en cualquier caso, este es un rastrillo que alguien pisoteará tarde o temprano.

Un problema similar está presente en la siguiente clase.

V690 La clase 'JsonArray' implementa un constructor de copia, pero carece del operador '='. Es peligroso usar tal clase. json.h 820

 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() { .... } } 

Puede leer más sobre el peligro de una falta de sobrecarga de un operador de asignación para una clase compleja en el artículo " La ley de los dos grandes " (o en la traducción de este artículo " C ++: La ley de los dos grandes ").

Ejemplos 7, 8:

Otro ejemplo relacionado con el operador de asignación sobrecargado, pero esta vez estamos hablando de su implementación específica.

V794 El operador de asignación debe estar protegido del caso de 'this == & other'. mattack_common.h 49

 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; } 

El problema es que esta implementación no está protegida de la asignación del objeto a sí mismo, lo cual es una práctica insegura. Es decir, si se pasa una referencia a * this a este operador, puede producirse una pérdida de memoria.

Un ejemplo similar de una sobrecarga de operador de asignación errónea con un efecto secundario interesante:

V794 El operador de asignación debe estar protegido del caso de 'this == & rhs'. player_activity.cpp 38

 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; } 

En este caso, al igual que no se verifica la asignación del objeto a sí mismo. Pero además, el vector se llena. Si intenta asignarse el objeto a usted por una sobrecarga, entonces en el campo de objetivos obtenemos un vector duplicado, algunos de cuyos elementos están dañados. Sin embargo, hay un claro antes de la transformación que borrará el vector del objeto y los datos se perderán.

Cuadro 16

Retiro III


En 2008, los bagels incluso adquirieron una definición formal, que recibió el nombre épico "Interpretación de Berlín". Según esta definición, las características principales de tales juegos son:

  • Un mundo generado aleatoriamente que aumenta el valor de repetición;
  • Permadeath: si tu personaje muere, muere para siempre y todos los objetos se pierden;
  • Paso a paso: los cambios ocurren solo junto con la acción del jugador, hasta que se realiza la acción - el tiempo se detiene;
  • Supervivencia: los recursos son extremadamente limitados.

Bueno y lo más importante: los bagels están destinados principalmente a explorar y descubrir el mundo, buscando nuevas formas de usar objetos y pasar por mazmorras.

La situación habitual en Cataclysm DDA: congelado y hambriento de muerte, te atormenta la sed y, de hecho, tienes seis tentáculos en lugar de piernas.

Cuadro 15

Detalles importantes


Ejemplo 9

V1028 Posible desbordamiento. Considere convertir los operandos del operador 'inicio + más grande' al tipo 'size_t', no el resultado. worldfactory.cpp 638

 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 ) ) { .... } .... } .... } 

Parece que el programador quería evitar el desbordamiento. Pero traer el resultado de la suma en este caso no tiene sentido, ya que se producirá un desbordamiento cuando se agreguen los números, y la expansión de tipo se realizará en el resultado sin sentido. Para evitar esta situación, solo necesita convertir uno de los argumentos a un tipo más grande: (static_cast <tamaño_t> (inicio) + más grande) .

Ejemplo 10

V530 Se requiere utilizar el valor de retorno de la función 'tamaño'. worldfactory.cpp 1340

 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; } 

Para tales casos, hay un pequeño truco. Si la variable no se usa, en lugar de intentar llamar a cualquier método, simplemente puede escribir (void) world_name para suprimir la advertencia del compilador.

Ejemplo 11

V812 Disminución del rendimiento. Uso ineficaz de la función 'contar'. Posiblemente puede ser reemplazado por la llamada a la función 'buscar'. player.cpp 9600

 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; } ) { .... } .... } 

A juzgar por el hecho de que el resultado del conteo se compara con cero, la idea es entender si hay al menos un elemento requerido entre la actividad . Pero count se ve obligado a pasar por todo el contenedor, ya que cuenta todas las ocurrencias del elemento. En esta situación, será más rápido usar find , que se detiene después de encontrar la primera coincidencia.

Ejemplo 12

El siguiente error se detecta fácilmente si conoce una sutileza.

V739 EOF no debe compararse con un valor del tipo 'char'. El 'ch' debe ser del tipo 'int'. json.cpp 762

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

Cuadro 3

Este es uno de esos errores que pueden ser difíciles de notar si no sabe que EOF se define como -1. En consecuencia, si intenta compararlo con una variable de tipo con signo char , la condición casi siempre es falsa . La única excepción es si el código de caracteres es 0xFF (255). Al comparar, dicho símbolo se convertirá en -1 y la condición será verdadera.

Ejemplo 13

El próximo pequeño error puede algún día volverse crítico. No es de extrañar que esté en la lista de CWE como CWE-834 . Y había, por cierto, cinco de ellos.

V663 Infinite loop es posible. La condición 'cin.eof ()' es insuficiente para romper el bucle. Considere agregar la llamada a la función 'cin.fail ()' a la expresión condicional. action.cpp 46

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

Como se indica en la advertencia, verificar para llegar al final del archivo mientras se lee no es suficiente, también debe verificar el error de lectura cin.fail () . Cambie el código para una lectura más segura:

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

keymap_txt.clear () es necesario para eliminar el estado de error (indicador) de la secuencia en caso de un error de lectura del archivo; de lo contrario, el texto no puede leerse más. keymap_txt.ignore con los parámetros numeric_limits <streamsize> :: max () y un carácter de control de avance de línea le permite omitir el resto de la línea.

Hay una manera mucho más simple de dejar de leer:

 while( !keymap_txt ) { .... } 

Cuando se usa en un contexto de lógica, se convierte en un valor equivalente a verdadero hasta que se alcanza EOF .

Retiro IV


Ahora los juegos más populares son aquellos que combinan los signos de los juegos de roguelike y otros géneros: plataformas, estrategias, etc. Tales juegos se han llamado roguelike-like o roguelite. Dichos juegos incluyen títulos tan famosos como Don't Starve, The Binding of Isaac, FTL: Faster Than Light, Darkest Dungeon e incluso Diablo.

Aunque a veces la diferencia entre roguelike y roguelite es tan pequeña que no está claro a qué género pertenece el juego. Alguien cree que Dwarf Fortress ya no es un roguelike, pero para alguien, Diablo es un bagel clásico.

Imagen 1

Conclusión


Aunque el proyecto en su conjunto es un ejemplo de código de alta calidad, y no fue posible encontrar muchos errores graves, esto no significa que el uso del análisis estático sea redundante. El punto no está en las comprobaciones únicas que hacemos para popularizar la metodología del análisis de código estático, sino en el uso regular del analizador. Luego, se pueden identificar muchos errores en una etapa temprana y, por lo tanto, reducir el costo de corregirlos. Ejemplo de cálculos.

Imagen 2

Se está trabajando activamente en el juego considerado, y hay una comunidad activa de modders. Además, está portado a muchas plataformas, incluidas iOS y Android. Entonces, si estás interesado en este juego, ¡te recomiendo que lo pruebes!

Source: https://habr.com/ru/post/449472/


All Articles