Тетрис: расположение классов

• Какова ответственность единого класса мира ? Это просто блок, содержащий практически все виды функциональности. Это не очень хороший дизайн. Одна из очевидных обязанностей - "представлять сетку, на которую помещаются блоки". Но это не имеет никакого отношения к созданию тетроидов, манипулированию списками блоков или отрисовке. На самом деле, большая часть этого, наверное, совсем не обязательно должна быть в классе. Я бы ожидал, что объект World будет содержать BlockList, который вы вызываете StaticBlocks, чтобы он мог определять сетку, на которой вы играете.
• Почему вы определяете свой собственный Blocklist? Вы сказали, что хотите, чтобы ваш код был общим, так почему бы не разрешить использовать любой контейнер ? Почему я не могу использовать std::vector, если хочу? Или std::set, или какой-нибудь домашний контейнер?
• Используйте простые имена, которые не дублируют информацию и не противоречат сами себе. TranslateTetroid не переводит тетройд. Он переводит все блоки в блок-листе. Так что это должно быть TranslateBlocks или что-то в этом роде. Но даже это излишне. Из подписи (это занимает BlockList&) видно, что он работает на блоках. Так что просто назовите его Translate.
• Попробуйте избежать комментариев в стиле Си (/*...*/). Стиль Си++ (//...) ведет себя несколько лучше, так как если использовать комментарий в стиле Си из всего блока кода, то он разобьется, если в этом блоке также будут содержаться комментарии в стиле Си. (В качестве простого примера /*/**/*/ не сработает, так как компилятор увидит первый */ в качестве конца комментария, и поэтому последний */ не будет считаться комментарием.
• Что со всеми (неназванными) int параметрами? Это делает ваш код недоступным для чтения.
• Уважайте особенности языка и условности. Способ копирования объекта - использование его конструктора копирования. Поэтому вместо функции CopyTetroid дайте BlockList конструктор копирования. Затем, если мне нужно скопировать один, я могу просто сделать BlockList b1 = b0.
• Вместо void SetX(Y) и Y GetX() методов, сбросить лишний префикс Get/Set и просто сделать void X(Y) и Y X(). Мы знаем, что это геттер, потому что он не принимает параметров и возвращает значение. И мы знаем, что другой - сеттер, потому что он принимает параметр и возвращает void.
• BlockList - не очень хорошая абстракция. У вас очень разные потребности для "текущего тетроида" и "списка статических блоков, находящихся в данный момент на сетке". Статические блоки могут быть представлены простой последовательностью блоков, как у вас есть (хотя последовательность рядов, или 2D массив, может быть более удобным), но активный в данный момент тетройд нуждается в дополнительной информации, такой как центр вращения (который не принадлежит в World).
  • Простой способ представления тетроида и облегчения вращений может заключаться в том, чтобы блоки-члены сохраняли простое смещение от центра вращения. Это облегчает расчеты вращений и означает, что блоки-члены не должны обновляться во время перевода. Нужно просто переместить центр вращения.
  • В статическом списке блоки даже не знают своего местоположения. Вместо этого сетка должна отображать расположение блоков (если я спрошу сетку "какой блок существует в ячейке (5,8), то она должна быть в состоянии вернуть блок. но в самом блоке нет необходимости хранить координаты. Если он будет хранить, то это может стать головной болью при обслуживании. Что, если из-за какой-то тонкой ошибки два блока окажутся с одной и той же координатой? Это может произойти, если блоки будут хранить свою собственную координату, но не в том случае, если в сетке есть список того, какой блок где)
  • это говорит нам о том, что нам нужно одно представление для "статического блока", а другое - для "динамического блока" (он должен хранить смещение от центра тетроида). Фактически, "статический" блок можно сварить до необходимого: Либо ячейка в сетке содержит блок, и этот блок имеет цвет, либо он не содержит блока. Дальнейшего поведения, связанного с этими блоками, нет, так что, возможно, вместо этого следует смоделировать ячейку, в которую он помещен.
  • и нам нужен класс, представляющий подвижный/динамический тетройд.
• так как многие из обнаружений столкновений являются "предсказывающими" в том смысле, что они имеют дело с "а что, если я переместил объект сюда", то может быть проще реализовать неперемещающиеся функции трансляции/вращения. Они должны оставлять оригинальный объект неизменным, а вращающуюся/переведенную копию возвращать.

Итак, вот первый проход по вашему коду, просто переименовывая, комментируя и удаляя код, не изменяя слишком много структуры.

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

Теперь, давайте добавим некоторые недостающие фрагменты:

Сначала нам нужно будет представить "динамические" блоки, тетройды, владеющие ими, и статические блоки или ячейки в сетке. (Мы также добавим простой метод "Collides" в класс world/grid)

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

и нам нужно будет заново провести спекулятивные проверки на столкновение. Учитывая не мутирующие методы Translate/Rotate, это просто: Мы просто создаем вращающиеся/переводимые копии и проверяем их на столкновение:

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