5
\$\begingroup\$

This is a follow up question of Classic Tetris implementation for Windows console - Initial Version Review

Since the changes this small project of mine suffered after applying all the excellent advice given to me in previous thread were substantial, it seemed to be a sensible idea to open it for review once more and count on the community to decide if I have executed the proposed changes in the best possible way.

Now, in my first question, I presented the following point as one of the main doubts I had while coding:

  • Overall code logistics. What would be the best (advised) way of interrelating my class objects? Should I pass references around as member variables (the way I did with my Tetris class, it has pointers to screenBuffer and playingField objects) and make most of the game functionality internal to my objects or make them as independent of one another as possible, bringing all together in my program's main function by accessing each object when needed (essentially pulling some of the programs functionality out of my objects)?

Addressing the given point, I got the following piece of advice:

...make them separate. That way, because the objects don't overlap, you have more control at the upper level to do with them what you want. If you want to change how these objects interact, you can change the upper level without messing with the internal representation. If you need a new way for them to interact, you can just make a new method, and then you can keep the old way too, if you want, much easier.

So, After pulling all code responsible for interrelating objects out of my classes, I was left with four simple classes with no overlap (each of which in its own separate file) and seven or so helper methods responsible for bringing the objects together into the main execution flow of the program. I bundled all these helper methods into a single utility class (this class has internal references for each relevant object), so to make them available under the same namespace, it seemed to be the most organised thing to do.

Finally, my code:

Main.cpp:

#include <iostream>
using namespace std;

#include <Windows.h>
#include <thread>

#include "utils.h"
#include "tetris.h"
#include "playingField.h"
#include "screenBuffer.h"

int main(void)
{
    Tetris tetrisGame = Tetris();
    Screen screenBuffer = Screen(80, 30);
    PlayingField playingField = PlayingField();
    Utils gameUtils = Utils(playingField, tetrisGame, screenBuffer);

    while (!tetrisGame.gameOver) {

        // Timing
        this_thread::sleep_for(50ms);
        tetrisGame.speedCounter++;
        tetrisGame.forceDown = (tetrisGame.speed == tetrisGame.speedCounter);

        // Input
        gameUtils.processInput(tetrisGame.fallingPiece());

        // Logic
        gameUtils.computNextState();

        //Render Output
        gameUtils.draw(tetrisGame.fallingPiece());
    }

    CloseHandle(screenBuffer.hConsole);
    cout << "Game Over! Score:" << tetrisGame.score << endl;
    system("pause");

    return 0;
}

Tetromino.h

#pragma once

#include <iostream>

// Tetromino Class
//==============================================================

class Tetromino
{
public:
    int y;
    int x;
    int rotation;
    const std::wstring layout;

    Tetromino(std::wstring layout)
        : layout(layout), y(0), x(6), rotation(0)
    {}

};

PlayingField.h

#pragma once

// Playing Field Class
//==============================================================

class PlayingField
{
public:
    const int fieldWidth;
    const int fieldHeight;
    unsigned char *pField;

    PlayingField()
        : fieldWidth(12), fieldHeight(18), pField(nullptr)
    {
        // Creating play field buffer
        pField = new unsigned char[fieldHeight * fieldWidth];
        for (int x = 0; x < fieldWidth; x++)
            for (int y = 0; y < fieldHeight; y++)
                // 0 characters are spaces and 9 are borders
                pField[y * fieldWidth + x] = (x == 0 || x == fieldWidth - 1 || y == fieldHeight - 1) ? 9 : 0;
    }
};

ScreenBuffer.h

#pragma once

#include <Windows.h>

// Screen buffer class
//==============================================================

class Screen
{
public:
    const int screenWidth;
    const int screenHeight;
    wchar_t *screen;
    HANDLE hConsole;
    DWORD dwBytesWritten;

    Screen(const int screenWidth, const int screenHeight)
        : screenWidth(screenWidth), screenHeight(screenHeight)
    {
        screen = new wchar_t[screenWidth * screenHeight];
        for (int i = 0; i < screenWidth * screenHeight; i++) screen[i] = L' ';
        hConsole = CreateConsoleScreenBuffer(GENERIC_READ | GENERIC_WRITE, 0, NULL, CONSOLE_TEXTMODE_BUFFER, NULL);
        SetConsoleActiveScreenBuffer(hConsole);
        dwBytesWritten = 0;
    }
};

Tetris.h

#pragma once

#include <vector>
#include "tetromino.h"

// Game class
//==============================================================

class Tetris
{
public:
    Tetris();
    int score;
    int lines;
    int speed;
    bool key[4];
    bool gameOver;
    int nextPiece;
    bool forceDown;
    bool rotateHold;
    int pieceCount;
    int speedCounter;
    int currentPiece;
    std::vector<int> fullLines;
    std::vector<Tetromino>tetromino;
    Tetromino& fallingPiece();
    Tetromino& nextFallingPiece();
    void setUpNextPiece();
};

Tetris.cpp

#include <vector>
#include "Tetris.h"
#include "Tetromino.h"

Tetris::Tetris()
    : speed(20), score(0), lines(0), pieceCount(0), speedCounter(0), gameOver(false),
    forceDown(false), nextPiece(rand() % 7), currentPiece(rand() % 7), rotateHold(true)
{
    // Generate pieces
    tetromino.push_back(Tetromino(L"..X...X...X...X."));
    tetromino.push_back(Tetromino(L"..X..XX...X....."));
    tetromino.push_back(Tetromino(L".....XX..XX....."));
    tetromino.push_back(Tetromino(L"..X..XX..X......"));
    tetromino.push_back(Tetromino(L".X...XX...X....."));
    tetromino.push_back(Tetromino(L".X...X...XX....."));
    tetromino.push_back(Tetromino(L"..X...X..XX....."));
}

void Tetris::setUpNextPiece()
{
    currentPiece = nextPiece;
    nextPiece = rand() % 7;
    fallingPiece().rotation = 0;
    fallingPiece().y = 0;
    fallingPiece().x = 6;
    score += 25;
}

Tetromino& Tetris::fallingPiece()
{
    return tetromino[currentPiece];
}

Tetromino& Tetris::nextFallingPiece()
{
    return tetromino[nextPiece];
}

Utils.h

#pragma once

#include "tetris.h"
#include "tetromino.h"
#include "playingField.h"
#include "screenBuffer.h"

// Utils 
//==============================================================

class Utils
{
public:
    void computNextState();
    Utils(PlayingField&, Tetris&, Screen&);
    void draw(const Tetromino& tetromino);
    void processInput(Tetromino& tetromino);
    void checkLines(const Tetromino& tetromino);
    void lockPieceOnField(const Tetromino& tetromino);
    int rotate(const int x, const int y, const int rotation);
    bool doesPieceFit(const Tetromino& tetromino, const int rotation, const int x, const int y);

private:
    Tetris& game;
    Screen& screenBuffer;
    PlayingField& playingField;
};

Utils.cpp

#include <iostream>
using namespace std;

#include "Utils.h"
#include <thread>

#define XPADDING 34
#define YPADDING 5

Utils::Utils(PlayingField& playingField, Tetris& game, Screen& screenBuffer)
    : playingField(playingField), game(game), screenBuffer(screenBuffer)
{}

void Utils::computNextState()
{
    if (game.forceDown) {
        if (doesPieceFit(game.fallingPiece(), game.fallingPiece().rotation, game.fallingPiece().x, game.fallingPiece().y + 1)) {
            game.fallingPiece().y++;
        } else {
            lockPieceOnField(game.fallingPiece());
            game.pieceCount++;
            // Increse game speed every 10 tics
            if (game.pieceCount % 10 == 0)
                if (game.speed >= 10) game.speed--;
            checkLines(game.fallingPiece());
            game.setUpNextPiece();
            if (!game.fullLines.empty()) game.score += (1 << game.fullLines.size()) * 100;
            // Game over if it doesn't fit
            game.gameOver = !doesPieceFit(game.fallingPiece(), game.fallingPiece().rotation, game.fallingPiece().x, game.fallingPiece().y);
        }
        game.speedCounter = 0;
    }
}

int Utils::rotate(const int x, const int y, const int rotation)
{
    /*
    * Rotates piece layout
    * string based on given angle
    * 'rotation'
    */
    switch (rotation % 4) {
        case 0: return y * 4 + x;          // 0 degress
        case 1: return 12 + y - (x * 4);   // 90 degress
        case 2: return 15 - (y * 4) - x;   // 180 degress
        case 3: return 3 - y + (x * 4);    // 270 degress
    }

    return 0;
}

bool Utils::doesPieceFit(const Tetromino& tetromino, const int rotation, const int x, const int y)
{
    for (int px = 0; px < 4; px++)
        for (int py = 0; py < 4; py++) {
            int pi = rotate(px, py, rotation);
            int fi = (y + py) * playingField.fieldWidth + (x + px);
            if (x + px >= 0 && x + px < playingField.fieldWidth)
                if (y + py >= 0 && y + py < playingField.fieldHeight)
                    // if cell value != 0, it's occupied
                    if (tetromino.layout[pi] != L'.' && playingField.pField[fi] != 0)
                        return false;
        }
    return true;
}

void Utils::processInput(Tetromino& tetromino)
{
    bool key[4];
    // x27 = right arrow key
    // x25 = left arrow key
    // x28 = down arrow key
    for (int k = 0; k < 4; k++)
        key[k] = (0x8000 & GetAsyncKeyState((unsigned char) ("\x27\x25\x28Z"[k]))) != 0;

    // Handling input
    tetromino.x += (key[0] && doesPieceFit(tetromino, tetromino.rotation, tetromino.x + 1, tetromino.y)) ? 1 : 0;
    tetromino.x -= (key[1] && doesPieceFit(tetromino, tetromino.rotation, tetromino.x - 1, tetromino.y)) ? 1 : 0;
    tetromino.y += (key[2] && doesPieceFit(tetromino, tetromino.rotation, tetromino.x, tetromino.y + 1)) ? 1 : 0;

    if (key[3]) {
        tetromino.rotation += (game.rotateHold && doesPieceFit(tetromino, tetromino.rotation + 1, tetromino.x, tetromino.y)) ? 1 : 0;
        game.rotateHold = false;
    } else {
        game.rotateHold = true;
    }
}

void Utils::checkLines(const Tetromino& tetromino)
{
    for (int py = 0; py < 4; py++) {
        if (tetromino.y + py < playingField.fieldHeight - 1) {
            bool line = true;
            for (int px = 1; px < playingField.fieldWidth - 1; px++)
                // if any cell is empty, line isn't complete
                line &= (playingField.pField[(tetromino.y + py) * playingField.fieldWidth + px]) != 0;
            if (line) {
                // draw '=' symbols
                for (int px = 1; px < playingField.fieldWidth - 1; px++)
                    playingField.pField[(tetromino.y + py) * playingField.fieldWidth + px] = 8;
                game.fullLines.push_back(tetromino.y + py);
                game.lines++;
            }
        }
    }
}

void Utils::lockPieceOnField(const Tetromino& tetromino)
{
    for (int px = 0; px < 4; px++)
        for (int py = 0; py < 4; py++)
            if (tetromino.layout[rotate(px, py, tetromino.rotation)] != L'.')
                // 0 means empty spots in the playing field
                playingField.pField[(tetromino.y + py) * playingField.fieldWidth + (tetromino.x + px)] = 1;
}

void Utils::draw(const Tetromino& tetromino)
{
    // Draw playing field
    for (int x = 0; x < playingField.fieldWidth; x++)
        for (int y = 0; y < playingField.fieldHeight; y++)
            //mapping playing field (' ', 1,..., 9) to Screen characters (' ', A,...,#)
            screenBuffer.screen[(y + YPADDING) * screenBuffer.screenWidth + (x + XPADDING)] = L" ▒▒▒▒▒▒▒=▓"[playingField.pField[y * playingField.fieldWidth + x]];

    // Draw pieces
    for (int px = 0; px < 4; px++)
        for (int py = 0; py < 4; py++) {
            if (tetromino.layout[rotate(px, py, tetromino.rotation)] == L'X')
                // Drawing current piece ( n + ASCII code of character 'A') 0 . A, 1 - > B, ...
                screenBuffer.screen[(tetromino.y + py + YPADDING) * screenBuffer.screenWidth + (tetromino.x + px + XPADDING)] = 0x2592;
            if (game.nextFallingPiece().layout[rotate(px, py, game.nextFallingPiece().rotation)] == L'X')
                // Drawing next piece ( n + ASCII code of character 'A') 0 . A, 1 - > B, ...
                screenBuffer.screen[(YPADDING + 3 + py) * screenBuffer.screenWidth + (XPADDING / 2 + px + 3)] = 0x2592;
            else
                screenBuffer.screen[(YPADDING + 3 + py) * screenBuffer.screenWidth + (XPADDING / 2 + px + 3)] = ' ';

        }

    // Draw text
    swprintf_s(&screenBuffer.screen[YPADDING * screenBuffer.screenWidth + XPADDING / 4], 16, L"SCORE: %8d", game.score);
    swprintf_s(&screenBuffer.screen[(YPADDING + 1) * screenBuffer.screenWidth + XPADDING / 4], 16, L"LINES: %8d", game.lines);
    swprintf_s(&screenBuffer.screen[(YPADDING + 4) * screenBuffer.screenWidth + XPADDING / 4], 13, L"NEXT PIECE: ");

    if (!game.fullLines.empty()) {
        WriteConsoleOutputCharacter(screenBuffer.hConsole, screenBuffer.screen, screenBuffer.screenWidth * screenBuffer.screenHeight, {0,0}, &screenBuffer.dwBytesWritten);
        this_thread::sleep_for(400ms);
        for (auto &v : game.fullLines)
            for (int px = 1; px < playingField.fieldWidth - 1; px++) {
                for (int py = v; py > 0; py--)
                    // clear line, moving lines above one unit down
                    playingField.pField[py * playingField.fieldWidth + px] = playingField.pField[(py - 1) * playingField.fieldWidth + px];
                playingField.pField[px] = 0;
            }
        game.fullLines.clear();
    }

    // Display Frame
    WriteConsoleOutputCharacter(screenBuffer.hConsole, screenBuffer.screen, screenBuffer.screenWidth * screenBuffer.screenHeight, {0,0}, &screenBuffer.dwBytesWritten);
}
\$\endgroup\$
5
\$\begingroup\$

Game Play

  1. most Tetris games use the up-arrow to rotate the piece. Unless you have a very specific reason you need to do otherwise, I'd use up-arrow like everybody else.
  2. I'd consider making each individual block of a tetromino two character cells wide (and still only one high). At least in most western European fonts, characters are about twice as tall as they are wide, so your "squares" aren't very square. This is particularly misleading with a block that's 2x3 squares, but 3 squares wide is actually portrayed narrower than 2 squares tall.
  3. the cursor keys are extremely sensitive--to the point that it's often difficult to get a block to the desired column--you get back and forth and can't quite get it to stop in the right place.
  4. You haven't called srand anywhere, so every game has the exact same sequence of game pieces.

Early Exit

Right now, if the user decides to quite the game early (e.g., with ctrl+C) they're left with a console that does't really function normally. In my opinion, it would be better to handle this so the user gets a normally functioning console.

One way to do that would be to add a call to SetConsoleCtrlHandler, to set up a handler that will close the handle to the console when/if the user kills the application.

Structure

Right now, most of the game's top-level logic is actually hidden in Utils.cpp. Normally, I'd expect something named "utils.cpp" to contain things that are quite generic, with no relationship to the specific program at hand, beyond some extremely general thing it does (e.g., it does some sort of string processing, so our utilities include some string stuff).

Code vs. Comments

I'm not overly fond of code like this:

// 0 characters are spaces and 9 are borders
pField[y * fieldWidth + x] = (x == 0 || x == fieldWidth - 1 || y == fieldHeight - 1) ? 9 : 0;

I'd prefer something like this instead:

static const char space = '\x0';
static const char border = '\x9';

pField[y+fieldWidth+x] = (x==0 || x == fieldWidth-1 || y == fieldHeight-1) ? border : space;

Separation of Concerns

Right now, your PlayField manually allocates storage for the playing field. And it simulates 2D addressing in linear memory. And it knows about where the borders go in Tetris. And it doesn't do those very well--for example, it has a ctor that allocates memory with new, but there's no code to delete that memory anywhere, so the memory is leaked.

In my opinion, it would be better to use std::vector to manage the raw memory. Then write a simple wrapper to manage 2D addressing on top of that. Finally, add a layer to manage the Tetris border.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.