- クラスの唯一の責任は何ですか？
Worldこれは、事実上あらゆる種類の機能を含む単なるブロブです。それは良いデザインではありません。明らかな責任の 1 つは、「ブロックが配置されるグリッドを表す」ことです。しかし、それは tetroid の作成や、ブロック リストや描画の操作とは関係ありません。実際、そのほとんどはおそらくクラスにある必要はまったくありません。あなたがプレイしているグリッドを定義できるように、Worldオブジェクトにはあなたが呼び出す StaticBlocks が含まれていると思います。BlockList
- なぜ独自の を定義するの
Blocklistですか? コードをジェネリックにしたいとおっしゃいましたが、コンテナの使用を許可しないのはなぜですか? std::vector<Block>したいのになぜ使えないのですか？それともstd::set<Block>自家製の容器ですか？
- 情報が重複したり矛盾したりしない単純な名前を使用してください。
TranslateTetroidテトロイドを変換しません。ブロックリスト内のすべてのブロックを翻訳します。だから、それTranslateBlocksか何かである必要があります。しかし、それでも冗長です。BlockList&シグネチャ ( が必要です) から、ブロックで機能することがわかります。だからそれを呼び出すだけTranslateです。
- C スタイルのコメント (
/*...*/) は避けるようにしてください。C++ スタイル ( //..) は、コードのブロック全体を C スタイルのコメント アウトで使用すると、そのブロックに C スタイルのコメントが含まれていると壊れてしまうという点で、少しはうまく動作します。(単純な例/*/**/*/として、コンパイラは最初*/のコメントをコメントの終わりとして認識し、最後の*/コメントはコメントと見なさないため、機能しません。
- すべての (名前のない)
intパラメータとは何ですか? コードが読めなくなります。
- 言語の特徴と慣習を尊重します。オブジェクトをコピーする方法は、そのコピー コンストラクターを使用することです。
CopyTetroidしたがって、関数ではなくBlockList、コピー コンストラクターを指定します。次に、1 つをコピーする必要がある場合は、単純にBlockList b1 = b0.
void SetX(Y)andメソッドではなくY GetX()、冗長な Get/Set プレフィックスを削除し、単純にvoid X(Y)and を使用しY X()ます。パラメータを取らず、値を返すため、getter であることがわかります。もう 1 つはパラメーターを取り、void を返すため、セッターであることがわかります。BlockListあまり良い抽象化ではありません。「現在のテトロイド」と「現在グリッド上にある静的ブロックのリスト」のニーズは大きく異なります。静的ブロックは、あなたが持っているようにブロックの単純なシーケンスで表すことができます (ただし、一連の行または 2D 配列の方が便利かもしれません) が、現在アクティブなテトロイドには、回転の中心などの追加情報が必要です (これはには属さないWorld)。
- テトロイドを表し、回転を容易にする簡単な方法は、メンバー ブロックに回転の中心からの単純なオフセットを格納させることです。これにより、回転の計算が容易になり、移動中にメンバー ブロックをまったく更新する必要がなくなります。回転の中心だけを移動する必要があります。
- 静的リストでは、ブロックがその位置を知ることさえ効率的ではありません。代わりに、グリッドは位置をブロックにマップする必要があります (グリッドに「どのブロックが cell に存在するかを尋ねると
(5,8)、ブロックを返すことができるはずです。しかし、ブロック自体は座標を格納する必要はありません。格納できる場合は、メンテナンスの頭痛の種になります. いくつかの微妙なバグが原因で、2 つのブロックが同じ座標になってしまったらどうなるでしょうか? これは、ブロックが独自の座標を保存している場合に発生する可能性がありますが、グリッドがどのブロックがどこにあるかのリストを保持している場合はそうではありません.)
- これは、「静的ブロック」には 1 つの表現が必要であり、「動的ブロック」には別の表現が必要であることを示しています (テトロイドの中心からのオフセットを格納する必要があります)。実際、「静的」ブロックは要点に要約できます。グリッド内のセルにブロックが含まれていて、そのブロックに色があるか、ブロックが含まれていないかのいずれかです。これらのブロックに関連するこれ以上の動作はありません。そのため、代わりにモデル化する必要があるのは、ブロックが配置されているセルである可能性があります。
- そして、可動/動的テトロイドを表すクラスが必要です。
- 衝突検出の多くは「オブジェクトをここに移動したらどうなるか」を扱うという点で「予測的」であるため、変化しない平行移動/回転関数を実装する方が簡単な場合があります。これらは元のオブジェクトを変更せずに残し、回転/変換されたコピーを返す必要があります。
したがって、これがコードの最初のパスです。構造をあまり変更せずに、コードの名前を変更し、コメントを付け、コードを削除するだけです。
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);
}
それでは、不足している部分をいくつか追加しましょう。
まず、「動的」ブロック、それらを所有するテトロイド、グリッド内の静的ブロックまたはセルを表す必要があります。(単純な「衝突」メソッドもワールド/グリッド クラスに追加します)
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))) { ... }