Tetris: layout de classes

Question

Eu escrevi um clone de tetris em funcionamento, mas ele tem um layout bastante confuso. Posso obter feedback sobre como reestruturar minhas aulas para melhorar minha codificação. Eu me concentro em tornar meu código o mais genérico possível, tentando torná -lo mais um motor para jogos apenas usando blocos.

Cada bloco é criado separadamente no jogo. Meu jogo tem 2 listas de bloqueios (listas vinculadas): StaticBlocks e Tetroid. O StaticBlocks é obviamente a lista de todos os blocos que não são de movimento, e o tetróide são os 4 blocos do tetróide atual.

1. No mundo principal, um mundo é criado.

2. Primeiro, um novo tetróide (4 blocos em uma lista de tetróide) é criado por (newtetrid)

3. A colisão é detectada pelas funções (*** colide), comparando cada um dos tetróide com todos os blocos de estática usando as funções (se *******).

4. Quando o tetróide para (atinge o fundo/blocos), ele é copiado (copytetrid) para os blocos de static e o tetróide é vazio, então os testes são feitos para linhas completas, blocos são destruídos/descartados etc, pesquisando blocos de static com (busca).

5. Um novo tetróide é criado.

(TranslateTeTroid) e (RotateTeTroid) executam operações em cada bloco na lista do tetróide um por um ( Eu acho que isso é uma prática ruim).

(DrawBlocklist) Acabai de passar por uma lista, executando a função draw () para cada bloco.

A rotação é controlada pela configuração do eixo de rotação em relação ao primeiro bloco no tetróide quando (newTeTroid) é chamado. Minha função de rotação (gire) para cada bloco a gira em torno do eixo, usando uma entrada +-1 para rotação esquerda/direita. Modos de rotação e estados são para blocos que giram de 2 ou 4 maneiras diferentes, definindo em que estado estão atualmente e se devem ser giradas para a esquerda ou direita. Não estou feliz com a forma como estes são definidos em "mundo", mas não sei onde colocá -los, mantendo minha função (girada) genérica para cada bloco.

Minhas aulas são as seguintes

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

Desculpe se isso é um pouco claro ou há muito tempo, estou apenas procurando uma ajuda para reestruturar.

Answer 1

• O que é solteiro responsabilidade do World classe? É apenas uma bolha contendo praticamente todo tipo de funcionalidade. Isso não é um bom design. Uma responsabilidade óbvia é "representar a grade na qual os blocos são colocados". Mas isso não tem nada a ver com a criação de tetróides ou manipulação de listas de blocos ou desenho. De fato, a maior parte disso provavelmente não precisa estar em uma aula. Eu esperaria o World objeto para conter o BlockList Você liga para o staticblocks para que possa definir a grade em que você está tocando.
• Por que você define o seu próprio Blocklist? Você disse que queria que seu código fosse genérico, então por que não permitir algum Contêiner a ser usado? Por que não posso usar um std::vector<Block> se eu quero? Ou a std::set<Block>, ou algum recipiente caseiro?
• Use nomes simples que não dupliquem informações ou se contradizem. TranslateTetroid não traduz um tetróide. Ele traduz todos os blocos em uma lista de bloqueio. Então deveria ser TranslateBlocks ou alguma coisa. Mas mesmo isso é redundante. Podemos ver na assinatura (é preciso um BlockList&) que funciona em blocos. Então apenas chame Translate.
• Tente evitar comentários no estilo C (/*...*/). C ++-estilo (//..) se comporta um pouco mais agradável, pois se você usar um comentar em estilo C um bloco inteiro de código, ele quebrará se esse bloco também contiver comentários no estilo C. (Como um exemplo simples, /*/**/*/ não vai funcionar, pois o compilador verá o primeiro */ como o fim do comentário, e assim o último */ não será considerado um comentário.
• O que há com todos os (sem nome) int parâmetros? É impossível ler seu código.
• Respeite os recursos e convenções da linguagem. A maneira de copiar um objeto é usar seu construtor de cópias. Então, em vez de um CopyTetroid função, dê BlockList um construtor de cópia. Então, se eu precisar copiar um, posso simplesmente fazer BlockList b1 = b0.
• Ao invés de void SetX(Y) e Y GetX() métodos, soltar o prefixo GET/Set redundante e simplesmente ter void X(Y) e Y X(). Sabemos que é um getter porque não leva parâmetros e retorna um valor. E sabemos que o outro é um setter, porque leva um parâmetro e retorna o vazio.
• BlockList Não é uma abstração muito boa. Você tem necessidades muito diferentes para "o tetróide atual" e "a lista de blocos estáticos atualmente na grade". Os blocos estáticos podem ser representados por uma sequência simples de blocos como você (embora uma sequência de linhas ou uma matriz 2D possa ser mais conveniente), mas o tetróide atualmente ativo precisa de informações adicionais, como o centro de rotação (que não pertence ao World).
  • Uma maneira simples de representar um tetróide e aliviar as rotações pode ser que os blocos de membros armazenem um deslocamento simples do centro de rotação. Isso facilita a computação das rotações e significa que os blocos de membros não precisam ser atualizados durante a tradução. Apenas o centro de rotação deve ser movido.
  • Na lista estática, nem é eficiente para os blocos conhecerem sua localização. Em vez disso, a grade deve mapear os locais para blocos (se eu perguntar à grade "Qual bloco existe na célula (5,8), ele deve ser capaz de devolver o bloco. Mas o bloco em si não precisa armazenar a coordenada. Se isso acontecer, pode se tornar uma dor de cabeça de manutenção. E se, devido a algum bug sutil, dois blocos acabarem com a mesma coordenada? Isso pode acontecer se os blocos armazenarem sua própria coordenada, mas não se a grade mantive uma lista de qual bloco estiver onde.)
  • Isso nos diz que precisamos de uma representação para um "bloco estático" e outro para um "bloco dinâmico" (ele precisa armazenar o deslocamento do centro do tetróide). De fato, o bloco "estático" pode ser fervido ao essencial: uma célula na grade contém um bloco, e esse bloco tem uma cor, ou não contém um bloco. Não há mais comportamento associado a esses blocos; portanto, talvez seja a célula na qual é colocado que deve ser modelado.
  • E precisamos de uma classe representando um tetróide móvel/dinâmico.
• Como grande parte da sua detecção de colisão é "preditiva", pois lida com "e se eu movesse o objeto por aqui", pode ser mais simples implementar funções de tradução/rotação não mutativas. Isso deve deixar o objeto original não modificado e uma cópia rotacionada/traduzida retornada.

Então, aqui está um primeiro passe no seu código, simplesmente renomeando, comentando e removendo o código sem alterar muito a estrutura.

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

Agora, vamos adicionar algumas das peças que faltavam:

Primeiro, precisaremos representar os blocos "dinâmicos", o tetróide que os possui e os blocos ou células estáticas em uma grade. (Também adicionaremos um método simples de "colidir" à classe mundial/grade)

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 precisaremos reimplementar as verificações especulativas de colisão. Dados os métodos de tradução/giro não mutativos, isso é simples: apenas criamos cópias rotadas/traduzidas e testamos as de colisão:

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

Eu pessoalmente abandonaria os blocos estáticos e lidaria com eles como linhas. Tendo um bloco estático, você está mantendo muito mais informações do que precisa.

Um mundo é feito de linhas, que é uma variedade de quadrados únicos. Os quadrados podem estar vazios ou uma cor (ou estendê -la se você tiver blocos especiais).

O mundo também possui um único bloco ativo, como você tem agora. A classe deve ter um método de girar e traduzir. O bloco obviamente precisará manter uma referência ao mundo para determinar se ele colidirá com os tijolos existentes ou a borda da placa.

Quando o bloco ativo sair do jogo, ele chamará algo como World.Update () que adicionará as peças do bloco ativo às linhas apropriadas, limpe todas as linhas completas, decidirá se você perdeu, etc. e finalmente criar um novo bloco ativo, se necessário.