Tetris: la mise en page des classes

Question

J'ai écrit un clone de tetris de travail, mais il a une disposition assez désordonnée. Pourrais-je s'il vous plaît obtenir des commentaires sur la façon de restructurer mes classes pour faire mon codage mieux. Je vise à rendre mon code aussi générique que possible, en essayant de le rendre plus un moteur pour les jeux utilisant uniquement des blocs.

Chaque bloc est créé dans le jeu séparement. Mon jeu a 2 Blocklists (listes liées): StaticBlocks et Tetroid. StaticBlocks est évidemment la liste de tous les blocs non mobiles, et Tetroid sont les 4 blocs de la Tetroid actuelle.

1. Dans un monde principal est créé.

2. Tout d'abord une nouvelle Tetroid (4 blocs dans une liste Tetroid) est créé par (NewTetroid)

3. collision est détectée par les fonctions (*** Collide), en comparant chacune des Tetroid avec tous les StaticBlocks en utilisant les fonctions (Si *****).

4. Quand les arrêts Tetroid (touche le fond / blocs), il est copié (CopyTetroid) aux StaticBlocks et Tetroid est fait vide, des tests sont effectués pour des lignes complètes, les blocs sont détruits / chuté etc en recherchant StaticBlocks avec (SearchY).

5. Un nouveau Tetroid est créé.

(TranslateTetroid) et (RotateTetroid) effectuent des opérations sur chaque bloc dans la liste Tetroid un par un ( Je pense que cela est une mauvaise pratique ).

(DrawBlockList) va juste une liste, en cours d'exécution de la fonction draw () pour chaque bloc.

La rotation est commandée par réglage de l'axe de rotation par rapport au premier bloc dans Tetroid lorsque (NewTetroid) est appelée. Ma fonction de rotation (Rotation) pour chaque bloc qu'il tourne autour de l'axe, à l'aide d'une entrée + -1 pour la rotation à gauche / droite. RotationModes et les Etats sont des blocs qui tournent en 2 ou 4 façons différentes, définir quel état ils sont actuellement, et si elles doivent être mis en rotation à gauche ou à droite. Je ne suis pas satisfait de la façon dont ceux-ci sont définis dans « Monde », mais je ne sais pas où les mettre, tout en gardant toujours la fonction de mon (Rotation) générique pour chaque bloc .

Mes classes sont les suivantes

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

Désolé si cela est un peu claire ou longue haleine, je suis à la recherche d'une restructuration de l'aide.

Answer 1

• Quel est le à la responsabilité de la classe World? Il est juste un blob contenant pratiquement toutes sortes de fonctionnalités. Ce n'est pas une bonne conception. Une responsabilité évidente est « représente la grille sur laquelle les blocs sont placés ». Mais cela n'a rien à voir avec la création tetroids ou la manipulation des listes de bloc ou un dessin. En fait, la plupart de cela ne probablement pas besoin d'être dans une classe du tout. Je me attends à l'objet World pour contenir le BlockList que vous appelez StaticBlocks il peut donc définir la grille sur laquelle vous jouez.
• Pourquoi définissez-vous votre propre Blocklist? Vous avez dit que vous vouliez que votre code soit générique, alors pourquoi ne pas permettre tout conteneur à utiliser? Pourquoi ne puis-je utiliser un std::vector<Block> si je veux? Ou un std::set<Block>, ou un conteneur infusé à la maison?
• Utiliser des noms simples qui ne font pas double emploi ou des informations se contredisent. TranslateTetroid ne se traduit pas Tetroid. Il se traduit par tous les blocs dans une liste de blocage. Il doit donc être TranslateBlocks ou quelque chose. Mais même cela est redondant. On peut voir de la signature (il faut BlockList&) que cela fonctionne sur des blocs. Il suffit donc de l'appeler Translate.
• Essayez d'éviter les commentaires de style C (/*...*/). C ++ - style (//..) se comporte un peu mieux en ce que si vous utilisez un commentaire de style C sur un bloc entier de code, il va briser si ce bloc contenait aussi des commentaires de style C. (A titre d'exemple simple, /*/**/*/ ne fonctionnera pas, comme le compilateur verra la première */ comme la fin du commentaire, et donc la dernière */ ne sera pas considéré comme un commentaire.
• Qu'est-ce avec tous les (sans nom) paramètres de int? Il est fait votre code impossible à lire.
• caractéristiques linguistiques de respect et conventions. La façon de copier un objet utilise son constructeur de copie. Ainsi, plutôt que d'une fonction de CopyTetroid, donner BlockList un constructeur de copie. Ensuite, si je dois copier un, je peux simplement faire BlockList b1 = b0.
• plutôt que des méthodes de void SetX(Y) et Y GetX(), déposez redondant Get / Set préfixe et ont simplement void X(Y) et Y X(). Nous savons qu'il est un getter car il ne prend aucun paramètre et renvoie une valeur. Et nous savons que l'autre est un compositeur, car il faut un paramètre et retourne void.
• BlockList est pas une très bonne abstraction. Vous avez des besoins très différents pour « l'Tetroid actuelle » et « la liste des blocs statiques actuellement sur la grille ». Les blocs statiques peuvent être représentés par une simple séquence de blocs que vous avez (même si une séquence de lignes, ou un tableau 2D, peut être plus commode), mais la Tetroid active a besoin d'informations supplémentaires, telles que le centre de rotation (qui ne fait pas partie du World).
  • Une façon simple de représenter une Tetroid, et pour faciliter la rotation, peut-être d'avoir les blocs membres stockent simple décalage par rapport au centre de rotation. Cela fait des rotations plus facile à calculer, et signifie que les blocs membres ne doivent pas être mis à jour à tout lors de la traduction. Juste le centre de rotation doit être déplacé.
  • Dans la liste statique, il est même pas efficace pour les blocs de connaître leur emplacement. Au lieu de cela, la grille devrait cartographier les endroits à blocs (si je demande à la grille « qui bloquent existe dans (5,8) cellulaire, il devrait être en mesure de retourner le bloc. Mais le bloc lui-même n'a pas besoin de stocker les coordonnées. Dans le cas contraire, il peut devenir un casse-tête d'entretien. Et si, à cause d'un bug subtil, deux blocs se retrouvent avec la même coordonnée? Cela peut arriver si les blocs stockent leurs propres coordonnées, mais pas si la grille contient une liste dont le bloc est là.)
  • cela nous dit que nous avons besoin d'une représentation pour un « bloc statique », et un autre pour un « bloc dynamique » (dont il a besoin pour stocker le décalage du centre du Tetroid). En fait, le bloc « statique »k peut être cuit à l'essentiel: soit une cellule de la grille contient un bloc, et en ce que le bloc a une couleur, ou qu'il ne contient pas un bloc. Il n'y a pas d'autres comportements associés à ces blocs, donc peut-être est la cellule dans laquelle il est placé devant être modélisé à la place.
  • et nous avons besoin d'une classe représentant un Tetroid mobile / dynamique.
• Comme beaucoup de votre détection de collision est « prédictive » en ce qu'il traite de « si je me suis déplacé l'objet ici », il peut être plus simple à mettre en œuvre des fonctions de translation / rotation non-mutation. Ceux-ci devraient quitter l'objet original non modifié, et une copie pivotée / translated retourné.

Alors, voici une première passe sur votre code, renommer simplement, en commentant et en supprimant le code sans changer la structure trop.

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

Maintenant, nous allons ajouter quelques-unes des pièces manquantes:

Tout d'abord, nous devons représenter les blocs « dynamiques », les Tetroid les posséder, et les blocs statiques ou des cellules dans une grille. (Nous allons également ajouter une méthode simple « heurtait » à la classe mondiale / réseau)

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

et nous aurons besoin de re-mettre en œuvre les contrôles de collision spéculatifs. Compte tenu de la non-mutationniste méthodes Traduire / rotation, c'est simple: Nous venons de créer des copies pivotés / traduits et tester ceux de collision:

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

Personnellement, je fossé les blocs statiques et de les traiter comme des lignes. Avoir un bloc statique vous garder beaucoup plus d'informations que vous avez besoin.

Un monde est composé de lignes, ce qui est une matrice de carrés simples. Les places peuvent être soit vide, soit une couleur (ou l'étendre si vous avez des blocs spéciaux).

Le monde possède également un seul bloc actif, que vous avez maintenant. La classe doit avoir une rotation et la méthode traduire. Le bloc devra évidemment conserver une référence au monde afin de déterminer si elle se heurtera avec des briques existantes ou le bord de la carte.

Lorsque le bloc actif est hors jeu, il appellera quelque chose comme world.update () qui ajoutera les morceaux du bloc actif aux lignes appropriées, claires toutes les lignes complètes, décider si vous avez perdu, etc, et enfin créer un nouveau bloc actif si nécessaire.