- shrughes
- Oct 11, 2008
-
(call/cc call/cc)
|
BlockAnimator.h:
code:#ifndef __BLOCK_ANIMATOR__
#define __BLOCK_ANIMATOR__
// NOTE: Other projects' header files should not be included after
// yours! One reason is, you'd rather have name conflicts happen in
// _your_ header files, instead of seeing them cryptically in others'.
// There are probably other good reasons -- I'm parroting the Google
// C++ Style Guide. An exception is the .cpp file's corresponding .h
// file, which should go first, to make sure that the .h file includes
// or declares all of its dependencies.
#include "Animator.h"
#include <SDL.h>
class BlockAnimator : public Animator {
int z;
const int centerx;
const int centery;
int x;
int y;
int x2;
int y2;
const int block;
const Uint64 ticktime;
// Uint64 last_tick = SDL_GetTicks(); // <- NOTE: using member
// initializers not inside a constructor for stateful behavior
// like here is insane.
Uint64 last_tick;
// ^ NOTE: Some of these fields can (and should) be marked
// const because they are constant for the lifetime of the
// object, and that makes them more readable. (Some could
// also be static, since they're global constants. Also, I
// would generally move all these const fields up to the top
// (as a default choice), except maaybe for ticktime. It's
// generally nice to say to the reader, ">HERE< is the
// stateful part of the object and <THERE< is the const part
// of the object" if there's not an overriding reason to do
// different.
public:
// NOTE: It's not important immediately but I think you'll
// find that declaring and implementing the destructor next to
// the constructors is what you end up preferring in the long
// run.
// NOTE: (x,y) pairs can go in a point type? I don't know yet.
// NOTE: I'm guessing (at first glance) block should be an
// enum type. enum class Block { Line, Z, S, blah blah blah
// };
BlockAnimator(int x, int y, int x2, int y2, int block);
bool nextFrame(void);
void render(Renderer &r);
~BlockAnimator();
};
#endif
BlockAnimator.cpp:
code:#include "BlockAnimator.h"
#include "Renderer.h"
#include <iostream>
// vvv NOTE: The parameter names of the constructor should not be the
// same as the field names. (Your compiler should have given you a
// warning!) I would use postfix underscores on private field names,
// myself. If you don't, then you should use a prefix underscore
// (followed by a lowercase character) or a postfix underscore on the
// parameter names, to prevent a name conflict.
// BlockAnimator::BlockAnimator(int x, int y, int x2, int y2, int block) {
// centery = ((y / 32) - 2) * 32;
// this->x = x - centerx;
// this->y = y - centery;
// this->x2 = x2 - centerx;
// this->y2 = y2 - centery;
// this->block = block;
// };
// ^ NOTE: You should have used a member initializer list to
// initialize the members. (This also lets you fields that are
// constant for the lifetime of the object as const -- see
// BlockAnimator.h.) Here is a replacement implementation:
BlockAnimator::BlockAnimator(int _x, int _y, int _x2, int _y2, int _block)
: z(256),
centerx(4 * 32 + 64),
centery((_y / 32 - 2) * 32),
x(_x - centerx), // NOTE: [1]
y(_y - centery),
x2(_x2 - centerx),
y2(_y2 - centery),
block(_block),
ticktime(10),
last_tick(SDL_GetTicks()) { }
// NOTE [1]: It's okay to use centerx here because centerx is
// initialized before x. centerx is initialized before x because the
// field is declared before x. The member initializer list is also
// written that way. Your compiler should emit a warning if its order
// is inconsistent with field order i.e. initialization order!
bool BlockAnimator::nextFrame(void) {
Uint64 tick = SDL_GetTicks();
if (tick - last_tick >= ticktime) {
last_tick = tick;
// NOTE: What's with all the "this" keyword usage?
// Just use x and x2, etc. If you want to make it
// clear they're members, name them with a postfix
// underscore, as x_ and x2_, etc.
if ((this->x + centerx <= 0) && (this->x2 + centerx <= 0) ||
(this->x + centerx >= 800) && (this->x2 + centerx >= 800) ||
(this->y + centery <= 0) && (this->y2 + centery <= 0) ||
(this->y + centery >= 768) && (this->y2 + centery >= 768) || z <= 100) {
return false;
}
else
{
this->x = this->x * 256 / z;
this->y = this->y * 256 / z;
this->x2 = this->x2 * 256 / z;
this->y2 = this->y2 * 256 / z;
z = z - 2;
}
return true;
}
return true;
};
void BlockAnimator::render(Renderer &r) {
r.renderBlock(x + centerx, y + centery, x2 - x, y2 - y, block - 1);
}
BlockAnimator::~BlockAnimator() {
}
GameLogic.h:
code:#ifndef __GAMELOGIC__
#define __GAMELOGIC__
#include "PlayField.h"
#include "Tetri.h"
#include "Animator.h"
class Animator;
class GameLogic {
private:
// v NOTE: Why are there all these initializers on these
// fields, when some of them get reinitialized in the actual
// constructor? lockTimeOut ends up getting the value 800, so
// why is it assigned 1600 here? You should initialize all
// the fields of an object in the same place, not half of them
// in the header and half in the cpp file.
Uint64 lockTimeOut = 1600;
Uint64 dropTimeout = 800;
Uint64 lastDrop = 0;
Uint64 score = 0;
Uint64 lines_completed = 0;
int lines_to_level = 10;
Uint64 level = 0;
std::vector<int> blockbag;
std::vector<Animator *> *alist;
// v NOTE: It's so loving easy to typo nextPiece into
// nnextPiece.
Tetri *nnextPiece = NULL;
Tetri *nextPiece = NULL;
public:
GameLogic(std::vector<Animator *> &animlist);
bool adjustFit(PlayField &board, Tetri &block);
bool kickFit(PlayField &board, Tetri &block);
bool checkFit(PlayField &p, Tetri &block);
bool addBlock(PlayField &p, Tetri &block);
void moveBlockLeft(PlayField &p, Tetri &block);
void moveBlockRight(PlayField &p, Tetri &block);
void moveBlockDown(PlayField &p, Tetri &block);
void RotateBlockCW(PlayField &p, Tetri &block);
void RotateBlockCCW(PlayField &p, Tetri &block);
void GravityBlockDown(PlayField &p, Tetri &block);
Uint64 getScore(void);
Uint64 getLevel(void);
Uint64 getLines(void);
Tetri *nextBlock(void);
Tetri *drawPiece(void);
Tetri *getNextPiece(void);
int clearLines(PlayField &p);
void reset(void);
~GameLogic(void);
};
#endif
GameLogic.cpp:
code:#include "GameLogic.h"
#include "Animator.h"
#include "BlockAnimator.h"
#include <cstdio>
#include <cstdlib>
#include <ctime>
GameLogic::GameLogic(std::vector<Animator *> &animlist) {
lockTimeOut = 800;
dropTimeout = 440;
lastDrop = 0;
// v NOTE: There is no need to use .reserve(7) right before
// assigning a 7-element initializer_list on the subsequent
// line.
blockbag.reserve(7);
blockbag = { 1, 2, 3, 4, 5, 6, 7 };
// v NOTE: This is _not_ the place to globally seed the random
// number generator. That would be in main.
std::srand(std::time(NULL));
// v NOTE: Why not pass a pointer to vector as animlist? Then
// the caller sees that they're passing a pointer, instead of
// wondering whether it gets passed by value or by reference
// or copied in. Probably there is a design problem about
// this too, I haven't looked at it yet.
alist = &animlist;
nnextPiece = NULL;
nextPiece = drawPiece();
}
int GameLogic::clearLines(PlayField &p) {
int lcount = 0;
int cleared = 0;
for (int y = 0; y < 22; y++) {
for (int x = 0; x < 10; x++) {
if (p.getBoardAt(x, y) != 0) {
lcount++;
}
}
if (lcount == 10) {
cleared++;
for (int x = 0; x < 10; x++) {
alist->push_back(new BlockAnimator(x * 32 + 64, y * 32, x * 32 + 64 + 31, y * 32 + 31, p.getBoardAt(x, y)));
p.setBoardAt(x, y, 0);
}
for (int yy = y; yy > 0; yy--) {
for (int xx = 0; xx < 10; xx++) {
int tv = p.getBoardAt(xx, yy - 1);
p.setBoardAt(xx, yy, tv);
}
}
}
lcount = 0;
}
if (cleared > 0) {
lines_completed = lines_completed + cleared;
Uint64 line_score = (100 * cleared * cleared);
if (level > 0) {
line_score = line_score * level;
}
score = score + line_score;
lines_to_level = lines_to_level - cleared;
if (lines_to_level <= 0) {
level++;
lines_to_level = 10 + (level * 2);
// v NOTE: Just use dropTimeout =
// std::max<Uint64>(dropTimeout - 40, 80);
//
// But what if dropTimeout (a Uint64) was less
// than 40 somehow! (Because of future
// changes to the logic somewhere.) Then
// it'll wrap around to a super-high unsigned
// value, and not get clamped to 80 properly!
// This is one of many examples why you should
// use signed types by default.
dropTimeout = dropTimeout - 40;
if (dropTimeout < 80) {
dropTimeout = 80;
}
}
std::cout << "Score: " << score << std::endl;
std::cout << "Level: " << level << std::endl;
std::cout << "Lines: " << lines_completed << std::endl;
std::cout << "Lines to Level: " << lines_to_level << std::endl;
std::cout << "droptimeout: " << dropTimeout << std::endl;
}
// NOTE: You _could_ just have a static const char *const
// arr[] = { "Single!", "Double!", "Triple!", "TETRIS!" };
// std::cout << arr[cleared - 1] << std::endl;
switch (cleared) {
case 1:
std::cout << "Single!" << std::endl;
break;
case 2:
std::cout << "Double!" << std::endl;
break;
case 3:
std::cout << "Triple!" << std::endl;
break;
case 4:
std::cout << "TETRIS!" << std::endl;
break;
}
return cleared;
}
// v NOTE: One does not simply return a pointer to a freshly allocated
// object on the heap. If you were writing C, the right way to return a fresh
// heap pointer would be:
//
// void drawPiece(Tetri **ptr_out)
//
// maybe with a name saying "create" or "alloc" or something. That
// way, the caller has to explicitly put the pointer in a variable,
// that they'll see and remember to free. They can't accidentally
// pass the result directly to a function that doesn't free the
// pointer.
//
// In C++, you'd still want to treat raw pointers the same way, but
// instead here this function should return a std::unique_ptr<Tetri>.
Tetri *GameLogic::drawPiece(void) {
// v NOTE: npiece isn't used until the switch statement below,
// so why is it used here?
Tetri *npiece;
int r = std::rand() % blockbag.size();
int piece = blockbag[r];
blockbag.erase(blockbag.begin() + r);
if (blockbag.size() == 0) {
blockbag = { 1, 2, 3, 4, 5, 6, 7 };
}
switch (piece) {
// NOTE: It would be perfectly fine to have
// case 1: return new Iblock();
// case 2: return new Lblock();
// ...
// (ignoring that you'd really want to return
// std::unique_ptr<Iblock>(new Iblock())) -- or
// std::make_unique<IBlock>() in C++14.
case 1:
npiece = new Iblock();
break;
case 2:
npiece = new Lblock();
break;
case 3:
npiece = new Jblock();
break;
case 4:
npiece = new Oblock();
break;
case 5:
npiece = new Sblock();
break;
case 6:
npiece = new Tblock();
break;
case 7:
npiece = new Zblock();
break;
default:
// NOTE: Returning NULL in an impossible case is nuts.
// A likely explanation is a memory error causing
// corruption getting you to this state, so you should
// just crash.
npiece = NULL;
break;
}
return npiece;
}
Tetri *GameLogic::nextBlock(void) {
// v NOTE: There is no reason for rpiece to be assigned NULL
// immediately before being assigned the value of nextPiece.
//
// Also, same comment about implicitly passing ownership of
// pointers around applies -- this should return a
// std::unique_ptr<Tetri>.
Tetri *rpiece = NULL;
rpiece = nextPiece;
nnextPiece = drawPiece();
nextPiece = nnextPiece;
return rpiece;
}
Tetri *GameLogic::getNextPiece(void) {
// NOTE: Hopefully the caller of this function _doesn't_
// except ownership of the pointer.
// NOTE: Hopefully nothing happens that results in nextPiece
// being deleted while the caller is using its return value.
// This is a fragile interface...
return nextPiece;
}
bool GameLogic::adjustFit(PlayField &board, Tetri &block) {
int original_x = block.get_x();
int original_y = block.get_y();
if (checkFit(board, block)) {
return true;
}
else {
// v NOTE: foo == false should be written !foo.
while (((block.xOfLeftMostBlock() + block.get_x()) < 0) && (checkFit(board, block) == false)) {
block.set_x(block.get_x() + 1);
}
while (((block.xOfRightMostBlock() + block.get_x()) > 9) && (checkFit(board, block) == false)) {
block.set_x(block.get_x() - 1);
}
}
if (!checkFit(board, block)) {
block.set_y(original_y);
block.set_x(original_x);
return false;
}
return true;
};
bool GameLogic::kickFit(PlayField &board, Tetri &block) {
int original_x = block.get_x();
int original_y = block.get_y();
if (!checkFit(board, block)) {
block.set_y(original_y);
block.set_x(original_x + 1);
if (!checkFit(board, block)) {
block.set_x(original_x - 1);
if (!checkFit(board, block)) {
block.set_x(original_x);
return false;
}
return true;
}
block.set_x(original_x);
}
return false;
}
bool GameLogic::checkFit(PlayField &p, Tetri &block) {
// vvv NOTE: This looks sketchy, hopefully I get back to this.
// (This can be a const int *?)
int *frame = block.getFrame();
// vvv NOTE: This function is complicated enough that these
// variables should be const.
int pos_x = block.get_x();
int pos_y = block.get_y();
// NOTE: Unless I'm mistaken, block can be passed as a const
// reference (a const Tetri &block) if its methods are
// properly declared const. PlayField can too, presumably, if
// its methods are properly declared as const.
for (int i_y = 0; i_y < 4; i_y++) {
for (int i_x = 0; i_x < 4; i_x++) {
if (frame[i_x + i_y * 4] != 0) {
if (!((p.checkInBounds(i_x + pos_x, i_y + pos_y)) && p.boardEmptyAt(i_x + pos_x, i_y + pos_y))) {
return false;
}
}
}
}
return true;
};
bool GameLogic::addBlock(PlayField &p, Tetri &block) {
int *frame = block.getFrame();
int pos_x = block.get_x();
int pos_y = block.get_y();
for (int i_y = 0; i_y < 4; i_y++) {
for (int i_x = 0; i_x < 4; i_x++) {
if (frame[i_x + i_y * 4] != 0) {
if ((p.checkInBounds(i_x + pos_x, i_y + pos_y)) && p.boardEmptyAt(i_x + pos_x, i_y + pos_y)) {
p.setBoardAt(i_x + pos_x, i_y + pos_y, frame[i_x + i_y * 4]);
}
else
{
return false;
}
// ^ NOTE: Just use `} else {` srsly.
}
}
}
return true;
};
void GameLogic::moveBlockLeft(PlayField &p, Tetri &block) {
int t_x = block.get_x();
int t_y = block.get_y();
block.set_x(t_x - 1);
if (!adjustFit(p, block)) {
block.set_x(t_x);
block.set_y(t_y);
}
};
void GameLogic::moveBlockRight(PlayField &p, Tetri &block) {
int t_x = block.get_x();
int t_y = block.get_y();
block.set_x(t_x + 1);
if (!adjustFit(p, block)) {
block.set_x(t_x);
block.set_y(t_y);
}
};
void GameLogic::moveBlockDown(PlayField &p, Tetri &block) {
int t_x = block.get_x();
int t_y = block.get_y();
block.set_y(t_y + 1);
if (!adjustFit(p, block)) {
block.set_x(t_x);
block.set_y(t_y);
}
};
void GameLogic::RotateBlockCW(PlayField &p, Tetri &block) {
int t_x = block.get_x();
int t_y = block.get_y();
block.rotateCW();
if (!adjustFit(p, block)) {
block.rotateCCW();
block.set_x(t_x);
block.set_y(t_y);
}
};
void GameLogic::RotateBlockCCW(PlayField &p, Tetri &block) {
int t_x = block.get_x();
int t_y = block.get_y();
block.rotateCCW();
if (!adjustFit(p, block)) {
block.rotateCW();
block.set_x(t_x);
block.set_y(t_y);
}
};
void GameLogic::GravityBlockDown(PlayField &p, Tetri &block) {
if (&block == NULL || &p == NULL) {
return;
}
int t_x = block.get_x();
int t_y = block.get_y();
if (SDL_GetTicks() - lastDrop >= dropTimeout) {
block.set_y(t_y + 1);
lastDrop = SDL_GetTicks();
if (!adjustFit(p, block)) {
block.set_y(t_y);
block.set_x(t_x);
if (block.getLockTimer() == 0) {
block.setLockTimer(SDL_GetTicks());
}
else
{
if (SDL_GetTicks() - block.getLockTimer() >= lockTimeOut) {
block.setLocked(true);
}
}
}
else
{
block.setLockTimer(0);
}
}
}
Uint64 GameLogic::getScore(void) {
return score;
}
Uint64 GameLogic::getLevel(void) {
return level;
}
Uint64 GameLogic::getLines(void) {
return lines_completed;
}
GameLogic::~GameLogic(void) {
if (nnextPiece != NULL) {
delete nnextPiece;
}
}
void GameLogic::reset(void) {
// NOTE: This is poo poo duplication of the constructor code.
// And: if you're resetting the game, can you not just
// construct a brand new GameLogic object?
lastDrop = 0;
score = 0;
lines_completed = 0;
lines_to_level = 10;
level = 0;
lockTimeOut = 800;
dropTimeout = 440;
// vvv NOTE: delete already checks for a NULL pointer and does
// nothing if it's NULL.
if (nnextPiece != NULL) {
delete nnextPiece;
}
nnextPiece = NULL;
// vvv NOTE: Are we just memory-leaking nextPiece here?
// (Questions like this would be less prevalent with a
// std::unique_ptr, of course.)
nextPiece = drawPiece();
}
playfield.h:
code:#ifndef __PLAYFIELD__
#define __PLAYFIELD__
// NOTE: No need for the iostream header here: it's only used in the
// .cpp file.
#include <iostream>
#include <vector>
class PlayField {
private:
// vvv NOTE: No reason this can't just be a std::vector<std::vector<int>>.
// vvv NOTE: Or an int[22][10] for that matter, as long as the
// size is unconfigurable...
std::vector<int> board;
public:
PlayField();
bool setBoardAt(int x, int y, int val);
// NOTE: I marked the method getBoardAt const as an example.
int getBoardAt(int x, int y) const;
bool checkInBounds(int x, int y);
bool boardEmptyAt(int x, int y);
void drawBoard(void);
void clearBoard(void);
};
#endif
playfield.cpp:
code:#include "PlayField.h"
// NOTE: So much duplication of playing field constants (like 22 and
// 10 and 220) here and elsewhere.
bool PlayField::checkInBounds(int x, int y) {
if ((x >= 0) && (x < 10) && (y >= 0) && (y < 22)) {
return true;
}
return false;
};
bool PlayField::boardEmptyAt(int x, int y) {
if (board[x + y * 10] == 0) {
return true;
}
return false;
};
PlayField::PlayField() {
// NOTE: board = std::vector<int>(220);
// or board = std::vector<int>(220, 0); (same thing)
// Actually, use a member initializer list for the constructor of course:
// PlayField::PlayField() : board(220, 0) { }
board.reserve(220);
for (int i = 0; i < 220; i++) {
board.push_back(0);
}
};
bool PlayField::setBoardAt(int x, int y, int val) {
int index = x + y * 10;
// NOTE: What's wrong with index being 0?
if (index < 220 && index > 0) {
board[index] = val;
return true;
}
return false;
};
// NOTE: This method could be const. I marked it const as an example.
int PlayField::getBoardAt(int x, int y) const {
int index = x + y * 10;
// NOTE: What's wrong with index being 0?
if (index < 220 && index > 0) {
return board[index];
}
return 0;
}
void PlayField::drawBoard(void) {
for (int y = 0; y < 22; y++) {
for (int x = 0; x < 10; x++) {
std::cout << board[x + y * 10] << " ";
}
std::cout << std::endl;
}
};
void PlayField::clearBoard(void) {
// NOTE: std::fill(board.begin(), board.end(), 0);
for (int y = 0; y < 22; y++) {
for (int x = 0; x < 10; x++) {
board[x + y * 10] = 0;
}
}
}
This is as far as I got. Though I noticed more memory management shenanigans elsewhere.
|