Tetris: Layout der Klassen

Question

Ich habe eine Arbeits tetris Klon geschrieben, aber es hat ein ziemlich chaotisch-Layout. Kann ich bitte Feedback bekommen, wie meine Klassen neu zu strukturieren, um meine besser zu machen Codierung. I konzentriert sich auf meinen Code so allgemein wie möglich zu machen, versuchen, es für die Spiele ein Motor nur Blöcke mit zu machen.

Jeder Block wird separat im Spiel erstellt. Mein Spiel hat 2 Blocklist (verkettete Listen): StaticBlocks und Tetroid. StaticBlocks ist offensichtlich die Liste aller unbewegten Blöcke und Tetroid sind die vier Blöcke des aktuellen Tetroid.

1. Im Haupt einer Welt geschaffen wird.

2. Zuerst wird eine neue Tetroid (4 Blöcke in einer Liste Tetroid) erstellt von (NewTetroid)

3. Kollision durch die (*** Collide) Funktionen ermittelt wird, die von jedem des Tetroid mit all den StaticBlocks Vergleich der (If *****) Funktionen.

4. Wenn die Tetroid Anschläge (trifft auf die untere / Blöcke) wird kopiert (CopyTetroid) an die StaticBlocks und Tetroid wird leer gemacht, dann Tests für komplette Anlagen vorgenommen werden, werden Blöcke zerstört / gelöscht usw. durch StaticBlocks Suche mit (Searchy).

5. Eine neue Tetroid erstellt wird.

(TranslateTetroid) und (RotateTetroid) führen Operationen an jedem Block in der Tetroid Liste eins nach dem anderen ( Ich denke, diese schlechte Praxis ).

(DrawBlockList) geht nur durch eine Liste für jeden Block die Draw () Funktion ausgeführt wird.

Rotation wird gesteuert durch die Drehung Stellachse relativ zu dem ersten Block in Tetroid wenn (NewTetroid) aufgerufen wird. Meine Drehfunktion (Drehen) für jeden Block dreht es sich um die Achse, unter Verwendung einer Eingabe + -1 für links / rechts Rotation. RotationModes und Staaten sind für die Blöcke, die in 2 oder 4 verschiedene Möglichkeiten drehen, zu definieren, was Zustand sie sich gerade befinden, und ob sie gedreht links oder rechts sein. Ich bin nicht glücklich darüber, wie diese in „Welt“ definiert ist, aber ich weiß nicht, wo sie zu setzen, während immer noch meine (Drehen) Funktion generic für jeden Block zu halten .

sind meine Klassen folgt als

class World
{
    public:
    /* Constructor/Destructor */
    World();
    ~World();

    /* Blocks Operations */
    void AppendBlock(int, int, BlockList&);
    void RemoveBlock(Block*, BlockList&);;

    /* Tetroid Operations */
    void NewTetroid(int, int, int, BlockList&);
    void TranslateTetroid(int, int, BlockList&);
    void RotateTetroid(int, BlockList&);
    void CopyTetroid(BlockList&, BlockList&);

    /* Draw */
    void DrawBlockList(BlockList&);
    void DrawWalls();

    /* Collisions */
    bool TranslateCollide(int, int, BlockList&, BlockList&);
    bool RotateCollide(int, BlockList&, BlockList&);
    bool OverlapCollide(BlockList&, BlockList&); // For end of game

    /* Game Mechanics */
    bool CompleteLine(BlockList&); // Test all line
    bool CompleteLine(int, BlockList&); // Test specific line
    void ColourLine(int, BlockList&);
    void DestroyLine(int, BlockList&);
    void DropLine(int, BlockList&); // Drops all blocks above line

    int rotationAxisX;
    int rotationAxisY;
    int rotationState; // Which rotation it is currently in
    int rotationModes; // How many diff rotations possible

    private:
    int wallX1;
    int wallX2;
    int wallY1;
    int wallY2;
};

class BlockList
{
    public:
    BlockList();
    ~BlockList();

    Block* GetFirst();
    Block* GetLast();

    /* List Operations */
    void Append(int, int);
    int  Remove(Block*);
    int  SearchY(int);

    private:
    Block *first;
    Block *last;
};

class Block
{
    public:
    Block(int, int);
    ~Block();

    int GetX();
    int GetY();

    void SetColour(int, int, int);

    void Translate(int, int);
    void Rotate(int, int, int);

    /* Return values simulating the operation (for collision purposes) */
    int IfTranslateX(int);
    int IfTranslateY(int);
    int IfRotateX(int, int, int);
    int IfRotateY(int, int, int);

    void Draw();

    Block *next;

    private:
    int pX; // position x
    int pY; // position y
    int colourR;
    int colourG;
    int colourB;
};

Sorry, wenn dies ist etwas unklar oder langatmig, ich bin nur die Suche nach etwas Hilfe Umstrukturierung.

Answer 1

• Was ist die Single Verantwortung der World Klasse? Es ist nur ein Klecks praktisch jede Art von Funktionalität enthält. Das ist nicht ein gutes Design. Eine nahe liegende Aufgabe ist es „stellen die Gitter, auf die Blöcke gelegt werden“. Aber das hat nichts mit dem zu tun tetroids Erstellen oder Blocklisten zu manipulieren oder zu zeichnen. In der Tat, die meisten, dass wahrscheinlich nicht in einer Klasse überhaupt sein müssen. Ich würde das World Objekt erwartet, dass die BlockList Sie StaticBlocks nennen enthalten, so dass es das Gitter, auf das definieren können Sie spielen.
• Warum Sie Ihre eigenen Blocklist definieren? Sie sagten, Sie Ihren Code wollte generisch sein, also warum nicht zulassen, dass jeder Behälter verwendet werden? Warum kann ich keine std::vector<Block> verwenden, wenn ich will? Oder ein std::set<Block> oder ein selbst gebrautes Container?
• Verwenden Sie einfache Namen, die keine doppelten Informationen oder widersprechen sich. TranslateTetroid übersetzt nicht ein Tetroid. Er übersetzt alle Blöcke in einem Blockliste. So sollte es TranslateBlocks oder etwas sein. Aber auch das ist überflüssig. Wir können von der Unterschrift sehen (es dauert eine BlockList&), die es auf Blöcken arbeitet. Also nur nennen es Translate.
• Versuchen C-artige Kommentare (/*...*/) zu vermeiden. C ++ - Stil (//..) verhält sich ein bisschen schöner, dass, wenn Sie einen C-Stil Kommentar aus einem gesamten Block von Code verwenden, es brechen werden, wenn dieser Block auch C-artige Kommentare enthält. (Als ein einfaches Beispiel, /*/**/*/ wird nicht funktionieren, da der Compiler den ersten */ als das Ende des Kommentars sehen, und so die letzten */ wird kein Kommentar berücksichtigt werden.
• Was ist mit all den (unbenannt) int Parameter? Es macht Ihren Code unmöglich zu lesen.
• Respect Sprache verfügt und Konventionen. Die Art und Weise, ein Objekt zu kopieren ist mit seiner Copykonstruktor. Anstatt also eine CopyTetroid Funktion, gibt BlockList eine Kopie Konstruktor. Dann, wenn ich kopieren muß, kann ich einfach BlockList b1 = b0 tun.
• Anstatt void SetX(Y) und Y GetX() Methoden, den redundanten Get / Set-Präfix fallen und einfach void X(Y) und Y X() haben. Wir wissen, es ist ein Getter, weil es keine Parameter und gibt einen Wert zurück. Und wir wissen, der andere ein Setter ist, weil es einen Parameter und gibt void zurück nimmt.
• BlockList ist nicht eine sehr gute Abstraktion. Sie haben sehr unterschiedliche Bedürfnisse für „den aktuellen Tetroid“ und „die Liste der statischen Blöcke zur Zeit auf dem Gitter“. Die statischen Blöcke können durch eine einfache Folge von Blöcken dargestellt werden, wie Sie haben (obwohl eine Folge von Zeilen oder eine 2D-Array kann bequemer sein), aber die aktuell aktive Tetroid zusätzliche Informationen benötigt, wie beispielsweise den Drehpunkt (die gehört nicht in den World).
  • Ein einfacher Weg, um eine Tetroid darzustellen, und Drehungen zu erleichtern, die Mitglieds Blöcke eine einfache vom Rotationszentrum versetzt speichern haben sein könnte. Das macht Drehungen einfacher zu berechnen, und bedeutet, dass die Mitgliedblöcke gar nicht während der Übersetzung aktualisiert werden. Nur die Dreh muss verschoben werden.
  • In der statischen Liste, ist es nicht einmal effizient für die Blöcke ihre Lage zu kennen. Stattdessen sollten die Gitterplätze auf Blöcke Karte (wenn ich das Raster „fragen, den Block in Zelle (5,8) vorhanden ist, sollte es in der Lage sein, den Block zurückzukehren. Aber der Block selbst brauchte nicht die Koordinate zu speichern. Ist dies der Fall, ist es kann Kopfschmerzen Wartung werden. Was passiert, wenn, aufgrund eines subtilen Bug, zwei Blöcke mit dem gleichen Ende koordinieren? das speichern, wenn Blöcke passieren können ihre eigenen koordinieren, aber nicht, wenn das Gitter einen Listenblock, von denen hält sich wo befindet.)
  • Dies sagt uns, dass wir eine Darstellung für einen „statischen Block“ benötigen, und eine andere für einen „dynamischen Block“ (es muss die von der Mitte des Tetroid Offset speichern). In der Tat, bloc die „statischen“k kann auf das Wesentliche gekocht werden: Entweder eine Zelle im Gitter einen Block enthält, und dass der Block eine Farbe aufweist, oder es enthält keinen Block. Es gibt keine weitere Verhalten mit diesen Blöcken zugeordnet ist, so vielleicht ist es die Zelle, in die er gestellt wird, dass sollte stattdessen modelliert werden.
  • und wir brauchen eine Klasse eine bewegliche / dynamische Tetroid darstellt.
• Da viele Ihrer Kollisionserkennung ist „predictive“, dass sie beschäftigt sich mit „was ist, wenn ich das Objekt hier bewegt“, kann es einfacher sein, Funktionen zu implementieren, nicht-Mutieren Übersetzung / Rotation. Diese sollten das ursprüngliche Objekt unverändert lassen, und eine gedreht / übersetzte Kopie zurückgegeben.

Also hier ist ein erster Durchlauf auf dem Code, einfach umbenannt, kommentiert und Code zu entfernen, ohne die Struktur zu viel zu ändern.

class World
{
public:
    // Constructor/Destructor
    // the constructor should bring the object into a useful state. 
    // For that, it needs to know the dimensions of the grid it is creating, does it not?
    World(int width, int height);
    ~World();

    // none of thes have anything to do with the world
    ///* Blocks Operations */
    //void AppendBlock(int, int, BlockList&);
    //void RemoveBlock(Block*, BlockList&);;

    // Tetroid Operations
    // What's wrong with using BlockList's constructor for, well, constructing BlockLists? Why do you need NewTetroid?
    //void NewTetroid(int, int, int, BlockList&);

    // none of these belong in the World class. They deal with BlockLists, not the entire world.
    //void TranslateTetroid(int, int, BlockList&);
    //void RotateTetroid(int, BlockList&);
    //void CopyTetroid(BlockList&, BlockList&);

    // Drawing isn't the responsibility of the world
    ///* Draw */
    //void DrawBlockList(BlockList&);
    //void DrawWalls();

    // these are generic functions used to test for collisions between any two blocklists. So don't place them in the grid/world class.
    ///* Collisions */
    //bool TranslateCollide(int, int, BlockList&, BlockList&);
    //bool RotateCollide(int, BlockList&, BlockList&);
    //bool OverlapCollide(BlockList&, BlockList&); // For end of game

    // given that these functions take the blocklist on which they're operating as an argument, why do they need to be members of this, or any, class?
    // Game Mechanics 
    bool AnyCompleteLines(BlockList&); // Renamed. I assume that it returns true if *any* line is complete?
    bool IsLineComplete(int line, BlockList&); // Renamed. Avoid ambiguous names like "CompleteLine". is that a command? (complete this line) or a question (is this line complete)?
    void ColourLine(int line, BlockList&); // how is the line supposed to be coloured? Which colour?
    void DestroyLine(int line, BlockList&); 
    void DropLine(int, BlockList&); // Drops all blocks above line

    // bad terminology. The objects are rotated about the Z axis. The x/y coordinates around which it is rotated are not axes, just a point.
    int rotationAxisX;
    int rotationAxisY;
    // what's this for? How many rotation states exist? what are they?
    int rotationState; // Which rotation it is currently in
    // same as above. What is this, what is it for?
    int rotationModes; // How many diff rotations possible

private:
    int wallX1;
    int wallX2;
    int wallY1;
    int wallY2;
};

// The language already has perfectly well defined containers. No need to reinvent the wheel
//class BlockList
//{
//public:
//  BlockList();
//  ~BlockList();
//
//  Block* GetFirst();
//  Block* GetLast();
//
//  /* List Operations */
//  void Append(int, int);
//  int  Remove(Block*);
//  int  SearchY(int);
//
//private:
//  Block *first;
//  Block *last;
//};

struct Colour {
    int r, g, b;
};

class Block
{
public:
    Block(int x, int y);
    ~Block();

    int X();
    int Y();

    void Colour(const Colour& col);

    void Translate(int down, int left); // add parameter names so we know the direction in which it is being translated
    // what were the three original parameters for? Surely we just need to know how many 90-degree rotations in a fixed direction (clockwise, for example) are desired?
    void Rotate(int cwSteps); 

    // If rotate/translate is non-mutating and instead create new objects, we don't need these predictive collision functions.x ½
    //// Return values simulating the operation (for collision purposes) 
    //int IfTranslateX(int);
    //int IfTranslateY(int);
    //int IfRotateX(int, int, int);
    //int IfRotateY(int, int, int);

    // the object shouldn't know how to draw itself. That's building an awful lot of complexity into the class
    //void Draw();

    //Block *next; // is there a next? How come? What does it mean? In which context? 

private:
    int x; // position x
    int y; // position y
    Colour col;
    //int colourR;
    //int colourG;
    //int colourB;
};

// Because the argument block is passed by value it is implicitly copied, so we can modify that and return it
Block Translate(Block bl, int down, int left) {
    return bl.Translate(down, left);
}
Block Rotate(Block bl, cwSteps) {
    return bl.Rotate(cwSteps);
}

Lassen Sie uns nun einige der fehlenden Teile hinzuzufügen:

Als erstes werden wir die „dynamischen“ Blöcke stellen müssen, die Tetroid sie, und die statischen Blöcke oder Zellen in einem Raster zu besitzen.

(Wir werden auch ein einfaches „Collides“ Verfahren zur Welt / grid-Klasse hinzufügen)

class Grid
{
public:
    // Constructor/Destructor
    Grid(int width, int height);
    ~Grid();

    // perhaps these should be moved out into a separate "game mechanics" object
    bool AnyCompleteLines();
    bool IsLineComplete(int line);
    void ColourLine(int line, Colour col);Which colour?
    void DestroyLine(int line); 
    void DropLine(int);

    int findFirstInColumn(int x, int y); // Starting from cell (x,y), find the first non-empty cell directly below it. This corresponds to the SearchY function in the old BlockList class
    // To find the contents of cell (x,y) we can do cells[x + width*y]. Write a wrapper for this:
    Cell& operator()(int x, int y) { return cells[x + width*y]; }
    bool Collides(Tetroid& tet); // test if a tetroid collides with the blocks currently in the grid

private:
    // we can compute the wall positions on demand from the grid dimensions
    int leftWallX() { return 0; }
    int rightWallX() { return width; }
    int topWallY() { return 0; }
    int bottomWallY { return height; }

    int width;
    int height;

    // let this contain all the cells in the grid. 
    std::vector<Cell> cells; 

};

// represents a cell in the game board grid
class Cell {
public:
    bool hasBlock();
    Colour Colour();
};

struct Colour {
    int r, g, b;
};

class Block
{
public:
    Block(int x, int y, Colour col);
    ~Block();

    int X();
    int Y();
void X(int);
void Y(int);

    void Colour(const Colour& col);

private:
    int x; // x-offset from center
    int y; // y-offset from center
    Colour col; // this could be moved to the Tetroid class, if you assume that tetroids are always single-coloured
};

class Tetroid { // since you want this generalized for more than just Tetris, perhaps this is a bad name
public:
    template <typename BlockIter>
    Tetroid(BlockIter first, BlockIter last); // given a range of blocks, as represented by an iterator pair, store the blocks in the tetroid

    void Translate(int down, int left) { 
        centerX += left; 
        centerY += down;
    }
    void Rotate(int cwSteps) {
        typedef std::vector<Block>::iterator iter;
        for (iter cur = blocks.begin(); cur != blocks.end(); ++cur){
            // rotate the block (*cur) cwSteps times 90 degrees clockwise.
                    // a naive (but inefficient, especially for large rotations) solution could be this:
        // while there is clockwise rotation left to perform
        for (; cwSteps > 0; --cwSteps){
            int x = -cur->Y(); // assuming the Y axis points downwards, the new X offset is simply the old Y offset negated
            int y = cur->X(); // and the new Y offset is the old X offset unmodified
            cur->X(x);
            cur->Y(y);
        }
        // if there is any counter-clockwise rotation to perform (if cwSteps was negative)
        for (; cwSteps < 0; --cwSteps){
            int x = cur->Y();
            int y = -cur->X();
            cur->X(x);
            cur->Y(y);
        }
        }
    }

private:
    int centerX, centerY;
    std::vector<Block> blocks;
};

Tetroid Translate(Tetroid tet, int down, int left) {
    return tet.Translate(down, left);
}
Tetroid Rotate(Tetroid tet, cwSteps) {
    return tet.Rotate(cwSteps);
}

und wir werden die spekulativen Kollisionsprüfungen neu implementieren müssen. Aufgrund der nicht-Mutieren Übersetzen / Rotate Methoden, die einfach: Wir gedreht / übersetzt Kopien nur erstellen und testen Sie die für Kollision:

// test if a tetroid t would collide with the grid g if it was translated (x,y) units
if (g.Collides(Translate(t, x, y))) { ... }

// test if a tetroid t would collide with the grid g if it was rotated x times clockwise
if (g.Collides(Rotate(t, x))) { ... }

Answer 2

Ich persönlich würde die statischen Blöcke und mit ihnen umgehen als Zeilen Graben. einen statischen Block mit Ihnen viel mehr Informationen halten, als Sie benötigen.

ist eine Welt der Zeilen gemacht, das ein Array von einzelnen Plätzen. Die Quadrate können entweder leer sein oder eine Farbe (oder es verlängern, wenn Sie spezielle Blöcke haben).

Die Welt besitzt auch einen einzigen aktiven Block, wie Sie jetzt haben. Die Klasse soll ein Drehen hat und Verfahren umzusetzen. Der Block wird natürlich muß einen Verweis auf die Welt zu erhalten, um festzustellen, ob es mit dem bestehenden Ziegeln oder der Kante der Platte kollidiert.

Wenn der aktive Block des Spiels geht, wird es so etwas wie world.update () aufrufen, die die Teile des aktiven Blocks in die entsprechenden Zeilen hinzufügen wird, die alle voll Zeilen löschen, entscheiden, ob Sie verloren haben, etc, und schließlich erstellen Sie einen neuen aktiven Block, wenn nötig.