4. Optimierung und Tipps anhand von Snake

Optimierung und Tipps anhand von Snake

  • R
    Mod

    Zero07 schrieb:

    gotoxy(27, height);
cout << "Punkte: " << points;

    void Snake::increasePoints() {
    points += 100;

    gotoxy(35, height);
    cout << points;
}

    Die beiden Zahlen sind jetzt auf die aktuellen Fenstergrößen angepasst und nicht dynamisch-->verbesserungswürdig
    100 einfach nur, da sich die Punktzahl bei jedem "Essen" um 100 erhöht.
    224 ist doch die Zahl die beim Drücken einer Sondertaste(in dem Fall die Pfeiltasten) zurückgegeben wird oder nicht?!

    du weisst es, ich weiss es in diesem fall, aber generell hat es schon einen grund keine magic numbers zu haben, denn man kann sich streiten ab wann man nicht mehr weiss was eine magic zahl soll und da code meistens waechst, z.b. koenntest du ja noch einbauen dass neben den koeder noch ab und zu besonders punktreiche 'fliegen' auftauchen, fuer kurze zeit; waere es nicht intuitiv durch den code zu gehen und nach stellen zu suchen die eventuel eine zahl verwenden die man aendern will.

    Wie meinst du das mit dem 2d vector für die "direction"?

    void Snake::moveHead() {
    head+=currentDirection;
/*
    switch(getCurrentDirection()) {
    case LEFT:
        head.X--;
        break;
    case UP:
        head.Y--;
        break;
    case RIGHT:
        head.X++;
        break;
    case DOWN:
        head.Y++;
        break;
    }
*/
}

    entsprechend

    void Snake::updateCurrentDirection() {
    if(_getch() == 224) {
        if(currentDirection == LEFT || currentDirection == RIGHT) {
            switch(_getch()) {
            case DOWN:
                currentDirection = COORD(0,-1);
                break;
            case UP:
                currentDirection = COORD(0,1);
                break;
            }
        }
        else {
            switch(_getch()) {
            case LEFT:
                currentDirection = COORD(-1,0);
                break;
            case RIGHT:
                currentDirection = COORD(1,0);
                break;
            }
        }
    }    
}

    wieso ist eigentlich COORD gross geschrieben? ist das ein macro oder sowas? deine "Snake" klasse folgt anderen regeln.

    0
  • D

    Was du mit der 224 machst, ist schon richtig, aber wär halt nett wenn du es dranschreibst, oder gar irgendwo Sondertastencode = 224 oder so schreibst und dann auf (_getch() == Sondertastencode) prüfst.

    Direction könnte eine Mini-Klasse mit x und y als Member sein und mit schöner Operatorüberladung (ruhig auch als freie Funktionen). Dazu noch eine Enum mit vier Elementen und du könntest dann bei dir sowas wie position += Direction::Up; sagen.

    Edit: Mist, zu lahm. 😉

    0
  • R
    Mod

    Dobi schrieb:

    ...sowas wie position += Direction::Up; sagen

    stimmt, sowas ist besser also meine magic numbers, also sowas wie

    case DOWN:
   currentDirection = Direction::Up;
   break;
    0

  • Bzgl. Namensgebung würde ich statt "breadth" doch eher "width" verwenden (ist zwar inhaltlich nicht falsch, aber ich kenne keine API, die diesen Ausdruck verwendet 😉

    0
  • D

    rapso schrieb:

    Dobi schrieb:

    ...sowas wie position += Direction::Up; sagen

    stimmt, sowas ist besser also meine magic numbers, also sowas wie

    case DOWN:
   currentDirection = Direction::Up;
   break;

    Dazu vielleicht noch etwas, um das Geswitche los zu werden:

    std::map<Datentyp_den_getch_returned, Direction> keyToDirection;

    Da dann die view Zuordnungen rein und man kann sowas machen:

    currentDirection = keyToDirection[_getch()];

    Dass bei unbekannten Tasten die Map wächst (Direction sollte ja eh nen default ctor mit 0,0 haben.) kann man entweder ignorieren oder halt vorher mit

    auto key = _getch();
if (keyToDirection.find(key) != std::end(keyToDirection))

    nachschaun.

    0
  • D

    @Zero07: Falls du Lust drauf hast: Zufällig hab ich vor Kurzem auch ne Snake/Tron-Variante geschrieben, hauptsächlich um mal SFML kennen zu lernen.
    http://daiw.de/share/Dron.zip Source liegt bei.

    0
  • Z

    rapso schrieb:

    wieso ist eigentlich COORD gross geschrieben? ist das ein macro oder sowas? deine "Snake" klasse folgt anderen regeln.

    Das ist doch so in windows.h definiert.

    Dobi schrieb:

    Dazu vielleicht noch etwas, um das Geswitche los zu werden:

    std::map<Datentyp_den_getch_returned, Direction> keyToDirection;

    Die Idee an sich ist gut, aber macht es in dem Fall Sinn? Angenommen die "currentDirection" ist rechts, dann kommt als nachfolgende Richung ja nur hoch oder runter in Frage. Diese Überprüfung ist doch mit einem switch in dem Fal einfacher oder irre ich mich da?! 😕

    Dobi schrieb:

    @Zero07: Falls du Lust drauf hast: Zufällig hab ich vor Kurzem auch ne Snake/Tron-Variante geschrieben, hauptsächlich um mal SFML kennen zu lernen.
    http://daiw.de/share/Dron.zip Source liegt bei.

    Ich schau es mir bei Gelegenheit mal an 😉

    0
  • D

    Zero07 schrieb:

    Dobi schrieb:

    Dazu vielleicht noch etwas, um das Geswitche los zu werden:

    std::map<Datentyp_den_getch_returned, Direction> keyToDirection;

    Die Idee an sich ist gut, aber macht es in dem Fall Sinn? Angenommen die "currentDirection" ist rechts, dann kommt als nachfolgende Richung ja nur hoch oder runter in Frage. Diese Überprüfung ist doch mit einem switch in dem Fal einfacher oder irre ich mich da?! 😕

    Wenn man schon nach rechts fährt, ist rechts irgendwie schon ne gültige Richtung für den nächsten Zeitschritt. 😉
    Wenn jemand versucht, rückwärts zu fahren, kann man das entweder erlauben (und ihn sterben lassen) oder die Eingabe verwerfen, was ja dank schön überladenen Operatoren in der Vektorklasse einfach geht:

    if (nextDirection - currentDirection == Direction::none)
    nextDirection = currentDirection;

    (So hab ichs in meiner Dron\src\Dron\PlayerControllers\KeyController.cpp Zeile 52 gemacht.)
    Was man von beidem dann nimmt, hängt halt davon ab, welches Gameplay man wünscht.
    Ich mag es, solche Ablauflogik in maps zu schieben anstatt sie hart mit switchs oder ifs im code zu haben, weil ich persönlich den Code dann übersichtlicher finde. Ein weiter (und nicht nur subjektiver ;)) Vorteil ist vielleicht auch der, dass wenn du später mal von 2d auf 3d gehen (oder nur Diagonalen hinzufügen) willst, du nichts weiter tun musst, als nur neue keys+directions in die map zu schieben. Das könnte ja sogar von außen passieren (configurierbare Eingabetasten, Joypad, sonstwas). Der Rest funktioniert dann automatisch. 🙂

    0

  • Vorweg: ich finde nicht dass das zu viel Code für so ein simples Spiel ist - so simpel ist Snake nämlich auch wieder nicht. Kommt natürlich immer drauf an mit was man es vergleicht, aber verglichen mit z.B. Pong oder Zahlen-Raten ist Snake hoch-komplex 😃
    (Und so viel Code ist es auch wieder nicht, sind ja bloss ein paar Zeilen)

    Also mich stört noch irgendwie die main Funktion. Die muss noch viel zu viel über das Spiel wissen, um es richtig "bedienen" zu können.

    Die einfachste Variante wäre den ganzen Loop einfach mit in das Spiel zu packen.

    Im derzeitigen Zustand macht es nichtmal Sinn dass main ein eigenes Snake Objekt instanziert. Es gibt ja sowieso nur eine Snake Klasse, und es werden auch keine Parameter gesetzt bevor das Spiel losgeht.
    In dem Fall würde es also reichen der Snake Klasse eine statische play Funktion zu verpassen, die dann 1:1 den Code deiner derzeitigen main enthält. Bis auf das sowieso fragwürdige system("pause") .

    Ein paar andere Dinge - und bitte ignorier' diese einfach wenn es nicht die Art von Tip ist die du suchst. Einige davon sind auch für ein Projekt/Spiel dieser Grössenordnung vollkommen irrelevant, werden aber bei grösseren Projekten schnell ... naja sagen wir mal nicht ganz uninteressant.

    * Du hast ein paar Funktionen in deiner Snake Klasse die eigentlich nichts mit Snake zu tun haben. Was man auch schön daran sieht, dass sie keinerlei Member der Snake-Klasse verwenden, keine privaten Memberfunktionen aufrufen etc.
    Ein klarer Fall sind z.B. gotoxy und print .
    Dann ist da noch printBorder . printBorder greift zwar auf height und breadth zu, aber die könnte man genau so gut als Parameter übergeben.
    Ich würde daraus freie Funktionen machen. u.U. in einem anonymen Namespace definiert, in dem .cpp File wo die Klasse Snake implementiert wird.

    • getFieldSize_x und getFieldSize_y machen mMn. keinen Sinn, ich würde daraus gleich Konstanten machen. Oder ganz auf die zwei verschiedenen Werte (1x mit Rand und 1x ohne) verzichten - du brauchst die Werte ohne Rand an genau einer Stelle, da kannst du mMn. genau so gut das "-2" direkt hinschreiben. Oder auch die Spezialbehandlung für den Rand ganz rausnehmen - du prüfst ja sowieso vorher ob das Feld frei ist, und SO viel Rand gibt es auch nicht dass da ein ernsthaftes Performance-Problem entstehen würde.

    * Du verwendest die gleiche Benamsung für Parameter, lokale Variablen und Membervariablen. Gleiche Benamsung für Parameter und lokale Variablen ist mMn. vollkommen OK, aber bei Membervariablen finde ich es sehr irritierend. Viele Projekte verwenden hier als Prefix einfach "m_". Es gibt noch andere Konventionen, aber soweit ich beurteilen kann ist "m_" die am stärksten verbreitete, und ich verwende es auch selbst.

    * Dein Snake Spiel macht im Moment zumindest zwei Dinge, die mMn. nichts mit dem eigentlichen Spiel zu tun haben: es kümmert sich darum wie und wo der Input herkommt, und wie der Output ausgegeben wird (wie die Grafik angezeigt wird). Ersteres ist einfach zu ändern, und auch in grösseren Projekten interessant. De Grafikausgabe ist eine andere Sachen. In grösseren Projekten ist die auch oft eng mit dem Gamecode verwoben, einfach aus Performance-Gründen. Wenn man es sich allerdings leisten kann hier eine bessere Trennung zu machen, halte ich es für eine gute Idee es auch zu tun.

    * Wurde schon erwähnt, aber egal: die Benamsung deiner Funktionen/Variablen/... ist nicht optimal. Den "Stilbruch" bei getFieldSize_x und getFieldSize_y hat rapso schon erwähnt. "collision" finde ich auch nicht optimal. "isHeadCollision" oder "hasSnakeCrashed" fände ich sprechender.

    * Die verwendeten Typen. Wieso short points ? Ich würde dem Score nen ganzen int spendieren. Bzw. wenn es keine Scores < 0 geben kann dann nen unsigned int . Weil's wurscht ist, du hast davon genau eine Variable, die paar Bytes kannst du dir leisten.
    Auf der anderen Seite - wieso short field[][] ? Du hast so wenig verschiedene Zustände, da reicht ein char/unsigned char . COORD finde ich auch komisch. OK, es ist die von Windows "vorgegebene" Struktur um Positionen innerhalb der Konsole zu speichern. Ich würde trotzdem entweder einfach POINT verwenden, oder gleich eine eigene Struktur definieren.

    0

  • Und weil's mir gerade aufgefallen ist ... die "food" Geschichte.

    Zero07 schrieb:

    void Snake::setRandomFood() { 
     bool freePos = false; 

     while(!freePos) { 
         food.X = rand() % (getFieldSize_x())+1; 
         food.Y = rand() % (getFieldSize_y())+1; 

         if(field[food.Y][food.X] != 1) { 
             freePos = true; 
         } 

     } 
     gotoxy(food.X, food.Y); 
     cout << iconFood; 

     field[food.Y][food.X] = 2; 
}

    Die freePos Variable ist etwas, was ich gar nicht mag: eine Variable die nur dazu dient einen Loop zu beenden. In diesem Fall schnell zu sehen. Mach das in einer komplizierteren Schleife die über 20+ Zeilen geht, und man sieht es garantiert nicht mehr auf den ersten Blick.

    Lässt sich genau so gut so schreiben:

    for (;;) { 
         food.X = rand() % (getFieldSize_x())+1; 
         food.Y = rand() % (getFieldSize_y())+1; 

         if(field[food.Y][food.X] != 1)
             break; // break ist nicht böse
     }

    Und in diesem Fall, da die Abbruchbedingung die letzte Anweisung in der Schleife ist, sogar noch einfacher mit do-while:

    do { 
         food.X = rand() % (getFieldSize_x())+1; 
         food.Y = rand() % (getFieldSize_y())+1; 
     }
     while (field[food.Y][food.X] == 1); // bzw. gleich != 0   -- mit == 1 bekommst du z.B. ein Problem sobald du mehr als nur ein "food" erlaubst

    Dann die Ausgabe des "Food Icons" in setRandomFood - das ist duplizierter Code, genau dafür hast du ja die print Funktion geschrieben.

    Und wozu ist die "food" Membervariable gut? Ich finde die bloss verwirrend. Du verwendest die an genau zwei Stellen, nämlich einmal in setRandomFood und dann nochmal in initializeField . Wobei noch dazu eine unnötige Abhängigkeit entsteht, nämlich dass setRandomFood vor initializeField aufgerufen wird.
    Alles in allem würde ich das eher so schreiben:

    void Snake::setRandomFood() { 
     COORD pos; // das Member "food" brauchen wir nimmer
     do
         pos = getRandomPosition(); // neue Funktion
     while (!isTileFree(pos)); // noch eine neue Funktion

     print(pos, iconFood);
     tileAt(pos) = 2; // nächste neue Funktion
}

short Snake::tileAt(COORD const& pos) const {
    return field[pos.x][pos.y];
}

short& Snake::tileAt(COORD const& pos) {
    return field[pos.x][pos.y];
}

COORD Snake::getRandomPosition() const {
    COORD const pos = { rand() % (breadth - 2) + 1, rand() % (height - 2) + 1 };
    return pos;

    // bzw. C++11:
    return { rand() % (breadth - 2) + 1, rand() % (height - 2) + 1 };
}

bool Snake::isTileFree(COORD const& pos) const {
    return tileAt(pos) == 0;
}

void Snake::initializeField() { 
    COORD pos;
    for (pos.x = 0; pos.x < height; pos.x++) { 
        for (pos.y = 0; pos.y < breadth; pos.y++) { 
            bool const isBorder = pos.x == 0
                || pos.x == height - 1
                || pos.y == 0
                || pos.y == breadth - 1;

            tileAt(pos) = isBorder ? -1 : 0;
        } 
    } 
    setRandomFood();
}
    0

  • Bzw. so wäre es vielleicht noch "schöner"...:

    COORD Snake::getFreePosition() const { 
    for (;;) {
        COORD const pos = getRandomPosition();
        if (isTileFree(pos))
            return pos;
    }
}

void Snake::setRandomFood() { 
    COORD const pos = getFreePosition();
    tileAt(pos) = 2;
    print(pos, iconFood); // ist zwar genaugenommen egal, aber jedes mal
                          // wenn ich print() vor dem Update des Feldes sehe kommt es mir falsch platziert vor.
}
    0
Anmelden zum Antworten