Pregunta

He escrito un clon de Tetris de trabajo pero tiene un diseño bastante desordenado. ¿Podría por favor obtener información sobre cómo reestructurar mis clases de codificación para hacer mi mejor. I se centra en hacer que el código lo más genérica posible, tratando de hacerlo más un motor para juegos utilizando sólo bloques.

Cada bloque se crea por separado en el juego. Mi juego tiene 2 listas de bloques (listas enlazadas): StaticBlocks y Tetroid. StaticBlocks es, obviamente, la lista de todos los bloques sin movimiento, y tetroid son los 4 bloques de la tetroid actual.

  1. En principal se crea un mundo

    .

  2. En primer lugar un nuevo tetroid (4 bloques en una lista Tetroid) es creado por (NewTetroid)

  3. colisión es detectada por las funciones (*** chocan), mediante la comparación de cada uno de Tetroid con todos los StaticBlocks utilizando los (Si *****) funciones.

  4. Cuando las paradas tetroid (toca el fondo / bloques), se copia (CopyTetroid) a los StaticBlocks y Tetroid se hace vacío, entonces se hacen pruebas de líneas completas, los bloques son destruidos / caído etc mediante la búsqueda StaticBlocks con (Searchy).

  5. Una nueva tetroid se crea.

(TranslateTetroid) y (RotateTetroid) realizan operaciones en cada bloque en la lista Tetroid uno por uno ( Creo que esto es una mala práctica ).

(DrawBlockList) simplemente pasa a través de una lista, se ejecuta la función Draw () para cada bloque.

rotación se controla mediante el establecimiento de eje de rotación con respecto al primer bloque en Tetroid cuando (NewTetroid) se llama. Mi función de rotación (Rotar) para cada bloque gira alrededor del eje, mediante una entrada + -1 para la rotación izquierda / derecha. RotationModes y Estados son los bloques que giran en 2 o 4 maneras diferentes, definiendo qué estado se encuentran actualmente en, y si deben ser rotados izquierda o la derecha. No estoy contento de cómo éstas se definen en el "mundo", pero no sé dónde ponerlos, sin dejar de mantener mi función (Rotar) genérico para cada bloque .

Mis clases son las siguientes

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

Lo siento si esto es un poco confuso o largo aliento, sólo estoy buscando un poco de ayuda de reestructuración.

¿Fue útil?

Solución

  • ¿Cuál es el solo responsabilidad de la clase World? Es sólo una gota que contiene prácticamente todo tipo de funcionalidad. Eso no es un buen diseño. Una de las responsabilidades obvia es "representan la rejilla sobre la que se colocan los bloques". Pero eso no tiene nada que ver con la creación de tetroids o manipular las listas de bloqueo o dibujar. De hecho, la mayoría de que, probablemente, no tiene que estar en una clase en absoluto. Yo esperaría que el objeto World para contener la BlockList se llama StaticBlocks por lo que puede definir la cuadrícula en la que estás jugando.
  • ¿Por qué definir su propio Blocklist? Dijo que quería que su código sea genérico, ¿por qué no permites cualquier contenedor para ser utilizado? Por qué no puedo utilizar un std::vector<Block> si quiero? O una std::set<Block>, o algún recipiente hecho en casa?
  • Use nombres simples que no dupliquen información o se contradicen. TranslateTetroid no traduce un tetroid. Traduce todos los bloques en una lista de bloques. Por lo que debe TranslateBlocks o algo así. Pero incluso eso es redundante. Podemos ver en la firma (se necesita un BlockList&) que funciona en bloques. Por lo que sólo lo llaman Translate.
  • Trate de evitar los comentarios de estilo C (/*...*/). C ++ - estilo (//..) comporta un poco más agradable en el que si se utiliza un comentario al estilo de C a cabo todo un bloque de código, se va a romper si ese bloque también contenía los comentarios de estilo C. (Como un simple ejemplo, /*/**/*/ no funcionará, ya que el compilador ver la primera */ como el final del comentario, y así el último */ no será considerada un comentario.
  • ¿Qué pasa con todos los parámetros (sin nombre) int? Está haciendo su código imposible de leer.
  • características del lenguaje Respeto y convenciones. La manera de copiar un objeto está utilizando su constructor de copia. Así que en lugar de una función CopyTetroid, dar BlockList un constructor de copia. Entonces, si tengo que copiar uno, yo simplemente puedo hacer BlockList b1 = b0.
  • En lugar de void SetX(Y) y Y GetX() métodos, deje caer el redundante get / set prefijo y simplemente tienen void X(Y) y Y X(). Sabemos que es un captador, ya que no toma parámetros y devuelve un valor. Y sabemos que el otro es un colocador, ya que toma un parámetro y devuelve void.
  • BlockList no es una muy buena abstracción. Usted tiene necesidades muy diferentes para "el tetroid actual" y "la lista de bloques estáticos actualmente en la parrilla". Los bloques estáticos pueden ser representados por una simple secuencia de bloques como que tiene (aunque una secuencia de filas, o una matriz 2D, puede ser más conveniente), pero el tetroid actualmente activa necesita información adicional, tales como el centro de rotación (que no tiene cabida en la World).
    • Una forma sencilla de representar un tetroid, y para aliviar las rotaciones, podría ser que los bloques miembro almacenar un desplazamiento del centro de rotación simple. Eso hace que las rotaciones más fácil de calcular, y significa que los bloques miembros no tienen que ser actualizados en absoluto durante la traducción. Sólo el centro de rotación tiene que ser movido.
    • En la lista estática, ni siquiera es eficiente para bloques para conocer su ubicación. En lugar de ello, la red debe asignar ubicaciones a bloques (si pido la red "que existe en el bloque de (5,8) celular, debe ser capaz de devolver el bloque. Pero el bloque en sí no necesita almacenar la coordenada. Si lo hace, puede convertirse en un dolor de cabeza de mantenimiento. ¿Qué pasa si, debido a algún error sutil, dos bloques terminan con la misma coordenada? Eso puede suceder si almacenan bloquea su propia coordinar, pero no si la red contiene una lista de qué bloque es donde.)
    • esto nos dice que necesitamos una representación de un "bloque estático", y otro para un "bloque dinámico" (que necesita para almacenar el desplazamiento del centro de la tetroid). De hecho, el bloque "estática"k se puede reducir a lo esencial: Cualquiera de una celda de la cuadrícula contiene un bloque, y que el bloque tiene un color, o no contener un bloque. No hay ningún comportamiento adicional asociado con estos bloques, por lo que tal vez es la célula en la que se coloca que debe ser modelada en su lugar.
    • y necesitamos una clase que representa un tetroid móvil / dinámico.
  • Dado que una gran parte de su detección de colisiones es "predictivo" en que se trata de "lo que si me movía el objeto aquí", puede ser más sencillo de implementar no mutantes funciones de traslación / rotación. Estos deben dejar el objeto original sin modificar, y una copia girada / traducida devuelto.

Así que aquí es un primer paso en su código, cambiando simplemente el nombre, comentando y eliminación de código sin cambiar la estructura demasiado.

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

Ahora, vamos a añadir algunas de las piezas que faltan:

En primer lugar, tendremos que representan los bloques "dinámicas", el tetroid titulares de los mismos, y los bloques estáticos o células en una cuadrícula. (También añadiremos un método simple "Choque" a la clase de mundo / sistema)

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

y tendremos que volver a implementar los controles de colisión especulativas. Dada la no-mutante Traducir métodos / Rotar, es simple: basta con crear copias rotadas / traducidos, y poner a prueba los de colisión:

// 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))) { ... }

Otros consejos

Yo personalmente deshacerse de los bloques estáticos y tratar con ellos como filas. Tener un bloque estático que están manteniendo mucha más información de la que necesita.

A mundo está hecho de filas, que es una matriz de cuadrados individuales. Las plazas pueden ser vacía, o un color (o ampliarlo si tiene bloques especiales).

El mundo también es propietaria de un único bloque activo, ya que tienes ahora. La clase debe tener una rotación y traducir método. El bloque obviamente tendrá que mantener una referencia al mundo para determinar si chocará con ladrillos existentes o el borde de la placa.

Cuando el bloque activo fuera del juego, se llamará algo así como world.update () que se sumarán las piezas del bloque activo a las filas apropiadas y claras todas las filas completas, decidir si ha perdido, etc, y finalmente, crear un nuevo bloque activo si es necesario.

Licenciado bajo: CC-BY-SA con atribución
No afiliado a StackOverflow
scroll top