Tetris: Layout delle classi

Question

Ho scritto un clone di Tetris di lavoro, ma ha un layout piuttosto disordinato. Potrei si prega di ottenere un feedback su come ristrutturare le mie classi di fare il mio meglio codifica. Ho concentra sul rendere il mio codice più generico possibile, cercando di renderlo più un motore per i giochi solo con i blocchi.

Ogni blocco è creato separatamente nel gioco. Il mio gioco ha 2 liste di blocco (liste collegate): StaticBlocks e Tetroid. StaticBlocks è ovviamente l'elenco di tutti i blocchi senza movimento, e tetroid sono i 4 blocchi del tetroid corrente.

1. In main si crea

   un mondo.

2. In primo luogo una nuova tetroid (4 blocchi in un elenco Tetroid) è stato creato da (NewTetroid)

3. collisione viene rilevato dalle funzioni (*** Collide), confrontando ciascun Tetroid con tutti i StaticBlocks base alle (Se *****) funzioni.

4. Quando gli arresti tetroid (tocca il fondo / blocchi), esso viene copiato (CopyTetroid) alle StaticBlocks e Tetroid è fatta vuoto, le prove sono fatte per linee complete, blocchi vengono distrutti / scesa ecc ricercando StaticBlocks con (SearchY).

5. Un nuovo tetroid viene creata.

(TranslateTetroid) e (RotateTetroid) eseguono operazioni su ogni blocco nella lista Tetroid uno per uno ( Penso che questa sia una cattiva pratica ).

(DrawBlockList) va solo attraverso un elenco, in esecuzione la funzione draw () per ogni blocco.

rotazione viene controllata modificando asse di rotazione rispetto al primo blocco Tetroid quando (NewTetroid) viene chiamato. Mia funzione di rotazione (Ruota) per ciascun blocco ruota attorno all'asse, utilizzando un ingresso + -1 per rotazione sinistra / destra. RotationModes e membri sono per blocchi che ruotano in 2 o 4 modi diversi, definendo che stato si trovano attualmente, e se devono essere ruotati a sinistra oa destra. Io non sono contento di come questi sono definiti nel "mondo", ma non so dove metterli, pur mantenendo ancora la mia funzione (Ruota) generico per ogni blocco .

Le mie lezioni sono i seguenti

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

Scusate se questo è un po 'poco chiare o lungo senza fiato, sto solo cercando un aiuto di ristrutturazione.

Answer 1

• Qual è il solo responsabilità della classe World? E 'solo un blob che contiene praticamente ogni tipo di funzionalità. Questo non è un buon design. Una responsabilità evidente è "rappresentano la griglia su cui sono posizionati i blocchi". Ma questo non ha niente a che fare con la creazione di tetroids o la manipolazione di liste di blocco o un disegno. In realtà, la maggior parte di che probabilmente non ha bisogno di essere in una classe a tutti. Mi aspetto l'oggetto World per contenere la BlockList si chiama StaticBlocks in modo che possa definire la griglia su cui si sta giocando.
• Perché si fa a definire il proprio Blocklist? Hai detto che volevi il codice per essere generico, quindi perché non permettono qualsiasi contenitore da utilizzare? Perché non è possibile utilizzare un std::vector<Block> se voglio? O un std::set<Block>, o qualche contenitore fatta in casa?
• Utilizzare nomi semplici che non duplicano le informazioni o contraddire se stessi. TranslateTetroid non si traduce un tetroid. Traduce tutti i blocchi in un blocklist. Così dovrebbe essere TranslateBlocks o qualcosa del genere. Ma anche questo è ridondante. Possiamo vedere dalla firma (ci vuole un BlockList&) che funziona su blocchi. Così lo chiamano semplicemente Translate.
• Cercare di evitare commenti in stile C (/*...*/). C ++ - stile (//..) si comporta un po 'più bello nel senso che se si utilizza un commento C-stile fuori un intero blocco di codice, si romperà se quel blocco conteneva anche i commenti in stile C. (Come semplice esempio, /*/**/*/ non funziona, come il compilatore vedere il primo */ come la fine del commento, e quindi l'ultima */ non sarà considerato un commento.
• Che cosa è con tutti i (senza nome) Parametri int? Sta facendo il codice impossibile da leggere.
• caratteristiche del linguaggio rispetto e convegni. Il modo per copiare un oggetto sta utilizzando il proprio costruttore di copia. Quindi, piuttosto che una funzione CopyTetroid, dare BlockList un costruttore di copia. Poi se ho bisogno di copiare uno, posso semplicemente fare BlockList b1 = b0.
• Piuttosto che void SetX(Y) e Y GetX() metodi, far cadere il ridondante Get / Set prefisso e semplicemente void X(Y) e Y X(). Sappiamo che è un getter, perché non accetta parametri e restituisce un valore. E sappiamo che l'altro è un setter, perché ci vuole un parametro e restituisce void.
• BlockList non è una buona astrazione. Lei ha esigenze molto diverse per "la tetroid corrente" e "l'elenco dei blocchi statici attualmente sulla griglia di partenza". I blocchi statici possono essere rappresentati da una semplice sequenza di blocchi si dispone (anche se una sequenza di righe, o una matrice 2D, può essere più conveniente), ma il tetroid attualmente attivo richiede informazioni aggiuntive, come ad esempio il centro di rotazione (che non appartiene alla World).
  • Un modo semplice per rappresentare un tetroid, e per facilitare rotazioni, potrebbe essere quella di avere i blocchi utente memorizzare un offset dal centro di rotazione semplice. Ciò rende più facile da calcolare rotazioni, e significa che i blocchi utente non devono essere aggiornati affatto durante la traduzione. Proprio il centro di rotazione deve essere spostata.
  • Nella lista statica, non è nemmeno efficace per i blocchi di conoscere la loro posizione. Al contrario, la griglia dovrebbe mappare posizioni a blocchi (se chiedo la griglia ", che esiste in blocco (5,8) cellulare, dovrebbe essere in grado di restituire il blocco. Ma il blocco stesso non ha bisogno di memorizzare la coordinata. Se lo fa, può diventare un mal di testa di manutenzione. che cosa succede se, a causa di qualche bug sottile, due blocchi finiscono con la stessa coordinata? Questo può accadere se i blocchi memorizzano la loro coordinate, ma non se la griglia contiene un elenco di quale blocco è dove.)
  • questo ci dice che abbiamo bisogno di una rappresentazione di un "blocco statico", e un altro per un "blocco dinamico" (di cui ha bisogno per memorizzare l'offset dal centro del tetroid). Infatti, il blocco "statica"k può essere bollito fino all'essenziale: O una cella della griglia contiene un blocco, e quel blocco ha un colore, o non contiene un blocco. Non sono disponibili ulteriori comportamento associato a questi blocchi, quindi forse è la cella in cui è posto che dovrebbe essere modellato invece.
  • e abbiamo bisogno di una classe che rappresenta un tetroid mobile / dinamico.
• Dal momento che un sacco di rilevamento delle collisioni è "predittiva", in quanto si tratta di "che cosa succede se ho spostato l'oggetto qui", può essere più semplice da implementare non mutanti funzioni traduzione / rotazione. Questi dovrebbero lasciare l'oggetto originale non modificata, e / copia tradotta ruotata restituito.

Quindi, ecco un primo passaggio sul vostro codice, è sufficiente rinominare, commentando e la rimozione di codice senza modificare la struttura troppo.

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

Ora, aggiungiamo dei pezzi mancanti:

In primo luogo, abbiamo bisogno di rappresentare i blocchi "dinamici", il tetroid possederle, ei blocchi statici o cellule in una griglia. (Aggiungeremo anche un metodo semplice "Collides" al / classe mondiale griglia)

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

e avremo bisogno di re-implementare i controlli di collisione speculativi. Data la non-mutante Tradurre metodi / Ruota, che è semplice: Abbiamo appena creiamo ruotati / copie tradotte, e testare quelli per collisione:

// 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

Vorrei fosso personalmente i blocchi statici e trattare con loro come righe. Avere un blocco statico si stanno mantenendo molte più informazioni di cui avete bisogno.

Un mondo è fatto di righe, che è una matrice di singole piazze. Le piazze possono essere sia vuota, o un colore (o estendere, se si dispone di blocchi speciali).

Il mondo possiede anche un unico blocco attivo, come hai ora. La classe deve avere una rotazione e metodo di tradurre. Il blocco avrà ovviamente bisogno di mantenere un riferimento al mondo per determinare se si scontrerà con mattoni esistenti o il bordo della scheda.

Quando il blocco attivo va fuori gioco, chiamerà qualcosa come world.update () cui si aggiungeranno i pezzi del blocco attivo alle righe appropriate, chiare tutte le righe piene, decidere se hai perso, ecc, e infine, creare un nuovo blocco attivo, se necessario.