Cataclysm Dark Days Ahead, statische Analyse und Bagels

Bild 10

Aus dem Titel des Artikels haben Sie höchstwahrscheinlich bereits erraten, dass der Fokus auf Fehlern im Quellcode liegt. Dies ist jedoch nicht das einzige, was in diesem Artikel behandelt wird. Wenn Sie neben C ++ und Fehlern im Code eines anderen von ungewöhnlichen Spielen angezogen werden und interessiert sind, was diese „Bagels“ sind und was sie mit ihnen essen, willkommen bei cat!

Auf meiner Suche nach ungewöhnlichen Spielen bin ich auf ein Cataclysm Dark Days Ahead-Spiel gestoßen, das sich von anderen ungewöhnlichen Grafiken unterscheidet: Es wird mit mehrfarbigen ASCII-Zeichen auf schwarzem Hintergrund implementiert.

Was in diesem Spiel und seiner Art auffällt, ist, wie viel alles in ihnen implementiert ist. Insbesondere in Cataclysm zum Beispiel, um einen Charakter zu erstellen, möchte ich nach Anleitungen suchen, da es Dutzende verschiedener Parameter, Funktionen und anfängliche Handlungen gibt, ganz zu schweigen von den Variationen der Ereignisse im Spiel selbst.

Dies ist ein Open Source-Spiel, das auch in C ++ geschrieben wurde. Es war also unmöglich, dieses Projekt über den statischen Analysator PVS-Studio zu passieren und nicht auszuführen, an dessen Entwicklung ich jetzt aktiv beteiligt bin. Das Projekt selbst hat mich mit der hohen Qualität des Codes überrascht, es enthält jedoch immer noch einige Mängel, und ich werde in diesem Artikel einige davon diskutieren.

Bisher wurden viele Spiele mit PVS-Studio getestet. Sie können beispielsweise unseren anderen Artikel „ Statische Analyse in der Videospielbranche: Top 10 Softwarefehler “ lesen.

Logik


Beispiel 1:

Das folgende Beispiel ist ein typischer Kopierfehler.

V501 Links und rechts vom '||' befinden sich identische Unterausdrücke. Operator: 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 ) ) { .... } .... } 

Hier wird der gleiche Zustand zweimal überprüft. Höchstwahrscheinlich wurde der Ausdruck kopiert und vergessen, etwas daran zu ändern. Es fällt mir schwer zu sagen, ob dieser Fehler signifikant ist, aber die Prüfung funktioniert nicht wie beabsichtigt.

Eine ähnliche Warnung:
  • V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke 'one_in (100000 / to_turns <int> (dur))'. player_hardcoded_effects.cpp 547

Bild 9

Beispiel 2:

V728 Eine übermäßige Überprüfung kann vereinfacht werden. Die '(A && B) || (! A &&! B) 'Ausdruck entspricht dem Ausdruck' bool (A) == bool (B) '. inventar_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 .... } .... } 

Es gibt keinen Fehler in der Bedingung, aber es ist unnötig kompliziert. Es lohnt sich, Mitleid mit denen zu haben, die diese Bedingung zerlegen müssen, und es ist einfacher zu schreiben, wenn (left_fav == right_fav) .

Eine ähnliche Warnung:

  • V728 Eine übermäßige Überprüfung kann vereinfacht werden. Das '(A &&! B) || (! A && B) 'Ausdruck entspricht dem Ausdruck' bool (A)! = Bool (B) '. iuse_actor.cpp 2653

Rückzug ich


Es stellte sich für mich als Entdeckung heraus, dass die Spiele, die heute als „Bagels“ bezeichnet werden, nur ziemlich leichte Anhänger des alten Genres der schurkenhaften Spiele sind. Alles begann mit dem Kult-Rogue-Spiel von 1980, das zum Vorbild wurde und viele Studenten und Programmierer dazu inspirierte, ihre eigenen Spiele zu entwickeln. Ich denke, dass auch die DnD-Board-Rollenspiel-Community und ihre Variationen viel gebracht haben.

Bild 8

Mikrooptimierung


Beispiel 3:

Die nächste Gruppe von Analysatorwarnungen zeigt keinen Fehler an, sondern die Möglichkeit einer Mikrooptimierung des Programmcodes.

V801 Leistungsminderung. Es ist besser, das zweite Funktionsargument als Referenz neu zu definieren. Ersetzen Sie 'const ... type' durch '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; } 

Hier versteckt itdpe_id std :: string . Da das Argument immer noch konstant übergeben wird, wodurch es nicht geändert werden kann, wäre es schneller, nur einen variablen Verweis auf die Funktion zu übergeben und keine Ressourcen beim Kopieren zu verschwenden. Und obwohl die Zeile dort höchstwahrscheinlich sehr klein sein wird, ist ein ständiges Kopieren ohne ersichtlichen Grund nicht erforderlich. Darüber hinaus wird diese Funktion von verschiedenen Stellen aus aufgerufen, von denen viele wiederum auch von außen typisiert und kopiert werden.

Ähnliche Warnungen:

  • V801 Leistungsminderung. Es ist besser, das dritte Funktionsargument als Referenz neu zu definieren. Ersetzen Sie 'const ... evt_filter' durch 'const ... & evt_filter'. input.cpp 691
  • V801 Leistungsminderung. Es ist besser, das fünfte Funktionsargument als Referenz neu zu definieren. Ersetzen Sie 'const ... color' durch 'const ... & color'. output.h 207
  • Insgesamt erzeugte der Analysator 32 solcher Warnungen.

Beispiel 4:

V813 Leistungsminderung. Das Argument 'str' sollte wahrscheinlich als konstante Referenz wiedergegeben werden. 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; } 

In diesem Fall ändert sich das Argument, obwohl es nicht konstant ist, nicht im Hauptteil der Funktion. Zur Optimierung wäre es daher hilfreich, eine konstante Verknüpfung zu verwenden und den Compiler nicht zu zwingen, lokale Kopien zu erstellen.

Diese Warnung war auch nicht einzeln, es gab insgesamt 26 solcher Fälle.

Bild 7

Ähnliche Warnungen:

  • V813 Leistungsminderung. Das Argument 'message' sollte wahrscheinlich als konstante Referenz gerendert werden. json.cpp 1452
  • V813 Leistungsminderung. Das Argument 's' sollte wahrscheinlich als konstante Referenz wiedergegeben werden. catacharset.cpp 218
  • Usw...

Rückzug II


Einige der klassischen Roguelike-Spiele werden noch entwickelt. Wenn Sie zu den GitHub Cataclysm DDA- oder NetHack-Repositorys gehen, können Sie sehen, dass täglich Änderungen aktiv vorgenommen werden. NetHack ist im Allgemeinen das älteste Spiel, das noch entwickelt wird: Es wurde im Juli 1987 veröffentlicht und die neueste Version stammt aus dem Jahr 2018.

Eines der bekanntesten, jedoch späteren Spiele dieses Genres ist Dwarf Fortress, das seit 2002 entwickelt und erstmals 2006 veröffentlicht wurde. "Verlieren macht Spaß" lautet das Motto des Spiels, das seine Essenz genau widerspiegelt, da es unmöglich ist, es zu gewinnen. Dieses Spiel im Jahr 2007 wurde aufgrund der Abstimmung, die jährlich auf der ASCII GAMES-Website stattfindet, zum besten schurkenhaften Spiel des Jahres gekürt.

Bild 6

Übrigens könnten diejenigen, die an diesem Spiel interessiert sind, an den folgenden Neuigkeiten interessiert sein. Dwarf Fortress wird auf Steam mit verbesserten 32-Bit-Grafiken veröffentlicht. Mit einem aktualisierten Bild, an dem zwei erfahrene Spielmoderatoren arbeiten, erhält die Premium-Version von Dwarf Fortress zusätzliche Musiktitel und Unterstützung für Steam Workshop. Wenn überhaupt, können die Besitzer der kostenpflichtigen Version von Dwarf Fortress die aktualisierten Grafiken auf das vorherige Formular in ASCII ändern. Weitere Details .

Zuweisungsoperator überschreiben


Beispiele 5, 6:

Es gab auch ein interessantes Paar ähnlicher Warnungen.

V690 Die Klasse 'JsonObject' implementiert einen Kopierkonstruktor, es fehlt jedoch der Operator '='. Es ist gefährlich, eine solche Klasse zu benutzen. 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() { .... } .... } 

Diese Klasse verfügt über einen Kopierkonstruktor und einen Destruktor, überlastet jedoch den Zuweisungsoperator nicht. Das Problem hierbei ist, dass ein automatisch generierter Zuweisungsoperator nur einen Zeiger auf JsonIn zuweisen kann . Infolgedessen verweisen beide Objekte der JsonObject- Klasse auf dasselbe JsonIn . Es ist nicht bekannt, ob eine solche Situation jetzt irgendwo auftreten könnte, aber auf jeden Fall ist dies ein Rechen, auf den früher oder später jemand treten wird.

Ein ähnliches Problem tritt in der folgenden Klasse auf.

V690 Die Klasse 'JsonArray' implementiert einen Kopierkonstruktor, es fehlt jedoch der Operator '='. Es ist gefährlich, eine solche Klasse zu benutzen. 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() { .... } } 

Weitere Informationen zur Gefahr einer fehlenden Überlastung eines Zuweisungsoperators für eine komplexe Klasse finden Sie im Artikel " Das Gesetz der großen Zwei " (oder in der Übersetzung dieses Artikels " C ++: Das Gesetz der großen Zwei ").

Beispiele 7, 8:

Ein weiteres Beispiel bezog sich auf den überladenen Zuweisungsoperator, aber dieses Mal sprechen wir über seine spezifische Implementierung.

V794 Der Zuweisungsoperator sollte vor dem Fall 'this == & other' geschützt werden. 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; } 

Das Problem ist, dass diese Implementierung nicht davor geschützt ist, das Objekt sich selbst zuzuweisen, was eine unsichere Vorgehensweise ist. Das heißt, wenn ein Verweis auf * dies an diesen Operator übergeben wird, kann ein Speicherverlust auftreten.

Ein ähnliches Beispiel für eine fehlerhafte Überlastung des Zuweisungsoperators mit einem interessanten Nebeneffekt:

V794 Der Zuweisungsoperator sollte vor dem Fall 'this == & rhs' geschützt werden. 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; } 

In diesem Fall wird die Zuordnung des Objekts zu sich selbst nicht überprüft. Zusätzlich füllt sich der Vektor. Wenn Sie versuchen, sich das Objekt durch eine solche Überladung zuzuweisen, erhalten wir im Zielfeld einen doppelten Vektor, dessen Elemente teilweise beschädigt sind. Vor der Transformation ist jedoch klar , dass der Vektor des Objekts gelöscht wird und Daten verloren gehen.

Bild 16

Rückzug III


Im Jahr 2008 erhielten Bagels sogar eine formale Definition, die den epischen Namen „Berlin Interpretation“ erhielt. Nach dieser Definition sind die Hauptmerkmale solcher Spiele:

  • Eine zufällig generierte Welt, die den Wiederholungswert erhöht.
  • Permadeath: Wenn dein Charakter stirbt, stirbt er für immer und alle Gegenstände sind verloren.
  • Schritt für Schritt: Änderungen erfolgen nur zusammen mit der Aktion des Spielers, bis die Aktion ausgeführt wird - die Zeit stoppt;
  • Überleben: Die Ressourcen sind äußerst begrenzt.

Gut und vor allem: Bagels zielen in erster Linie darauf ab, die Welt zu erkunden und zu entdecken, nach neuen Wegen zu suchen, um Objekte zu benutzen und durch Dungeons zu gehen.

Die übliche Situation in Cataclysm DDA: Gefroren und todhungrig, werden Sie von Durst gequält, und tatsächlich haben Sie sechs Tentakel anstelle von Beinen.

Bild 15

Wichtige Details


Beispiel 9:

V1028 Möglicher Überlauf. Ziehen Sie in Betracht, Operanden des Operators 'start + large' in den Typ 'size_t' umzuwandeln, nicht in das Ergebnis. 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 ) ) { .... } .... } .... } 

Es sieht so aus, als wollte der Programmierer einen Überlauf vermeiden. In diesem Fall ist es jedoch sinnlos, das Ergebnis der Addition zu bringen, da beim Hinzufügen der Zahlen ein Überlauf auftritt und eine Typerweiterung für das bedeutungslose Ergebnis durchgeführt wird. Um diese Situation zu vermeiden, müssen Sie nur eines der Argumente in einen größeren Typ umwandeln : (static_cast <Größe_t> (Start) + größer) .

Beispiel 10:

V530 Der Rückgabewert der Funktion 'Größe' muss verwendet werden. 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; } 

Für solche Fälle gibt es einen kleinen Trick. Wenn die Variable nicht verwendet wird, können Sie, anstatt zu versuchen, eine Methode aufzurufen, einfach (void) world_name schreiben, um die Compiler-Warnung zu unterdrücken.

Beispiel 11:

V812 Leistungsminderung. Ineffektive Verwendung der Zählfunktion. Es kann möglicherweise durch den Aufruf der Funktion 'find' ersetzt werden. 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; } ) { .... } .... } 

Gemessen an der Tatsache, dass das Ergebnis der Zählung mit Null verglichen wird, besteht die Idee darin zu verstehen, ob es mindestens ein erforderliches Element in der Aktivität gibt . Die Zählung muss jedoch den gesamten Container durchlaufen, da alle Vorkommen des Elements gezählt werden. In dieser Situation ist es schneller, find zu verwenden, das stoppt, nachdem das erste Match gefunden wurde.

Beispiel 12:

Der folgende Fehler wird leicht erkannt, wenn Sie eine Subtilität kennen.

V739 EOF sollte nicht mit einem Wert vom Typ 'char' verglichen werden. Das 'ch' sollte vom Typ 'int' sein. json.cpp 762

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

Bild 3

Dies ist einer dieser Fehler, die schwer zu bemerken sein können, wenn Sie nicht wissen, dass EOF als -1 definiert ist. Wenn Sie versuchen, es mit einer Variablen vom Typ signiertes Zeichen zu vergleichen, ist die Bedingung dementsprechend fast immer falsch . Die einzige Ausnahme ist, wenn der Zeichencode 0xFF (255) ist. Beim Vergleich wird ein solches Symbol zu -1 und die Bedingung ist wahr.

Beispiel 13:

Der nächste kleine Fehler könnte eines Tages kritisch werden. Kein Wunder, dass es auf der CWE-Liste als CWE-834 steht . Und es waren übrigens fünf von ihnen.

V663 Endlosschleife ist möglich. Die Bedingung 'cin.eof ()' reicht nicht aus, um die Schleife zu verlassen. Fügen Sie dem bedingten Ausdruck möglicherweise den Funktionsaufruf 'cin.fail ()' hinzu. action.cpp 46

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

Wie in der Warnung angegeben, reicht es nicht aus, beim Lesen zu prüfen, ob das Ende der Datei erreicht ist. Sie müssen auch nach cin.fail () -Lesefehlern suchen . Ändern Sie den Code für ein sichereres Lesen:

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

keymap_txt.clear () wird benötigt, um den Fehlerstatus (Flag) im Falle eines Lesefehlers aus der Datei aus dem Stream zu entfernen, andernfalls kann der Text nicht weiter gelesen werden. Mit keymap_txt.ignore mit den Parametern numeric_limits <streamsize> :: max () und einem Zeilenvorschubsteuerzeichen können Sie den Rest der Zeile überspringen.

Es gibt einen viel einfacheren Weg, mit dem Lesen aufzuhören:

 while( !keymap_txt ) { .... } 

Wenn es in einem logischen Kontext verwendet wird, konvertiert es sich in einen Wert, der true entspricht, bis EOF erreicht ist.

Rückzug IV


Jetzt sind die beliebtesten Spiele diejenigen, die die Zeichen von Roguelike-Spielen und anderen Genres kombinieren: Plattformer, Strategien usw. Solche Spiele werden inzwischen als Roguelike-like oder Roguelite bezeichnet. Zu solchen Spielen gehören so berühmte Titel wie Don't Starve, Die Bindung von Isaac, FTL: Schneller als Licht, Darkest Dungeon und sogar Diablo.

Obwohl der Unterschied zwischen Roguelike und Roguelite manchmal so gering ist, dass nicht klar ist, zu welchem ​​Genre das Spiel gehört. Jemand glaubt, dass die Zwergenfestung nicht mehr wie ein Schurke ist, aber für jemanden ist Diablo ein klassischer Bagel.

Bild 1

Fazit


Obwohl das gesamte Projekt ein Beispiel für hochwertigen Code ist und es nicht möglich war, viele schwerwiegende Fehler zu finden, bedeutet dies nicht, dass die Verwendung der statischen Analyse für ihn überflüssig ist. Es geht nicht um einmalige Überprüfungen, die wir durchführen, um die Methodik der statischen Code-Analyse bekannt zu machen, sondern um die regelmäßige Verwendung des Analysators. Dann können viele Fehler frühzeitig erkannt werden und reduzieren somit die Kosten für deren Korrektur. Beispiel für Berechnungen.

Bild 2

An dem betrachteten Spiel wird aktiv gearbeitet, und es gibt eine aktive Community von Moddern. Darüber hinaus ist es auf viele Plattformen portiert, einschließlich iOS und Android. Wenn Sie an diesem Spiel interessiert sind, empfehle ich Ihnen, es zu versuchen!

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


All Articles