- ¿Cuál es el único responsabilidad de la clase
World? Es solo un blob que contiene prácticamente todo tipo de funcionalidad. Eso no es un buen diseño. Una responsabilidad obvia es "representar la cuadrícula sobre la cual se colocan los bloques". Pero eso no tiene nada que ver con la creación de tetroides o la manipulación de listas de bloques o dibujos. De hecho, la mayor parte de eso probablemente no necesita estar en una clase en absoluto. Esperaría que el objeto World contenga el BlockList que llamas StaticBlocks para que pueda definir la grilla en la que estás jugando.
- ¿Por qué define su propio
Blocklist? Dijiste que querías que tu código fuera genérico, ¿por qué no permites que se use cualquier contenedor? ¿Por qué no puedo usar un std::vector<Block> si quiero? ¿O un std::set<Block>, o algún recipiente hecho en casa?
- Use nombres simples que no dupliquen la información ni se contradigan.
TranslateTetroid no traduce un tetroide. Traduce todos los bloques en una lista de bloqueo. Entonces debería ser TranslateBlocks o algo así. Pero incluso eso es redundante. Podemos ver desde la firma (lleva un BlockList&) que funciona en bloques. Así que solo llámalo Translate.
- Intenta evitar los comentarios al estilo C (
/*...*/). C++ - style (//..) se comporta un poco mejor si utiliza un comentario al estilo C de todo un bloque de código, se romperá si ese bloque también contiene comentarios al 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 (sin nombre)
int parámetros? Está haciendo que su código sea imposible de leer.
- Respete las características y convenciones del idioma. La forma de copiar un objeto es usar su constructor de copia. Entonces, en lugar de una función
CopyTetroid, proporcione BlockList un constructor de copia. copie uno, simplemente puedo hacer BlockList b1 = b0.
- En lugar de los métodos
void SetX(Y) y Y GetX(), elimine el preconfigurado Get/Set redundante arreglar y simplemente tener void X(Y) y Y X(). Sabemos que es un getter porque no toma parámetros y devuelve un valor. Y sabemos que el otro es un setter porque toma un parámetro y devuelve el vacío.
BlockList no es una muy buena abstracción. Tiene necesidades muy diferentes para "el tetroide actual" y "la lista de bloques estáticos actualmente en la red". Los bloques estáticos se pueden representar mediante una secuencia simple de bloques como usted (aunque una secuencia de filas, o una matriz 2D, puede ser más conveniente), pero el tetroide actualmente activo necesita información adicional, como el centro de rotación (que no pertenece al 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 sean más fáciles de computar, y significa que los bloques miembros no necesitan actualizarse durante la traducción. Solo el centro de rotación debe moverse.
- En la lista estática, ni siquiera es eficiente que los bloques conozcan su ubicación.En cambio, la cuadrícula debe asignar ubicaciones a bloques (si le pregunto a la cuadrícula "qué bloque existe en la celda
(5,8), debería poder 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 los bloques almacenan sus propias coordenadas, pero no si la cuadrícula contiene una lista de qué bloque es el que está.)
- 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" se puede reducir a lo esencial: o una celda en la cuadrícula contiene un bloque, y ese bloque tiene un color, o no contiene un bloque. No hay ningún otro comportamiento asociado con estos bloques, por lo que tal vez sea la celda en la que se coloca eso sh debería ser modelado en su lugar.
- y necesitamos una clase que represente un tetroide móvil/dinámico.
- Dado que gran parte de su detección de colisión es "predictiva" en cuanto a "qué pasaría si moviera el objeto por aquí", puede ser más sencillo implementar funciones de traducción/rotación no mutadas. Estos deberían dejar el objeto original sin modificar, y una copia girada/traducida devuelta.
Así que aquí hay un primer paso en el código, simplemente cambiar el nombre, comentar y eliminar el código sin cambiar demasiado la estructura.
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. (vamos a añadir 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))) { ... }
Eso es absolutamente increíble, exactamente lo que quería! ¡Muchas gracias! – Ash