2048 console game implementation

Question

So I was asked to do a quick 2048 game with C++ but using only stuff like arrays, structures, functions, pointers without using vectors, lambda and so on. The restrictions are here because it's just a study exercise (or homework I should say).

I might comment out the following code if needed. This is what I've came up with so far:

#include <iostream>
#include <ctime>
#include <iomanip>
#include <windows.h>
#include <conio.h>

using namespace std;

enum colors 
{
    BLACK, BLUE, GREEN, CYAN, RED, PURPLE, YELLOW, GREY,
    LIGHTGREY, LIGHTBLUE, LIGHTGREEN, LIGHTCYAN, LIGHTRED,
    LIGHTPURPLE, LIGHTYELLOW, WHITE
};

void setConsoleColor(int textColor, int bgColor) 
{
    SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), (textColor + (bgColor * 16)));
}

int randN(int start, int end)
{
    return rand() % (end - start) + start;
}

int randN(int n1, int n2, int percent)
{
    if ((rand() % 100) < percent)
        return n1;
    else
        return n2;
}

enum Direction
{
    Left = 75,
    Right = 77,
    Up = 72,
    Down = 80
};

struct Cell
{
    int x;
    int y;
    int value;

    Cell() 
    { 
        x = -1;
        y = -1;
        value = -1;
    }

    Cell(int x, int y, int value)
    {
        this->x = x;
        this->y = y;
        this->value = value;
    }

    void setValue(int value)
    {
        this->value = value;
    }

    void display()
    {
        cout << setw(4) << value;
    }

    bool isEmpty()
    {
        return value == 0;
    }

    bool isEqualTo(Cell cell)
    {
        return value == cell.value;
    }
};

struct Game
{
    Cell *cells;
    Cell *prevCells;
    int size;
    int score = 0;
    int highscore;
    int moves = 0;

    bool isMoved()
    {
        for (int i = 0; i < size * size; i++)
        {
            if (!cells[i].isEqualTo(prevCells[i]))
            {
                moves += 1;
                return true;
            }
        }
        return false;
    }

    void Doubled(int points)
    {
        score += points;
    }

    bool isCellsFull()
    {
        for (int i = 0; i < size * size; i++)
        {
            if (cells[i].isEmpty())
                return false;
        }
        return true;
    }

    Cell &findCell(int x, int y)
    {
        for (int i = 0; i < size * size; i++)
        {
            if (cells[i].x == x && cells[i].y == y)
                return cells[i];
        }
        return Cell();
    }

    Cell &randEmptyCell()
    {
        if (isCellsFull())
            return Cell();
        int i;
        do
        {
            i = randN(0, size * size);
        } while (!cells[i].isEmpty());
        return cells[i];
    }

    void create(int size)
    {
        this->size = size;
        cells = new Cell[size * size];
        prevCells = new Cell[size * size];

        for (int x = 0, c = 0; x < size; x++)
        {
            for (int y = 0; y < size; y++, c++)
            {
                cells[c] = Cell(x, y, 0);
            }
        }
    }

    void display()
    {
        for (int i = 0, c = 0; i < size + 1; i++)
        {
            for (int j = 0; j < size; j++)
                cout << " ----";
            cout << endl;
            if (i == size)
                break;
            for (int j = 0; j < size + 1; j++, c++)
            {
                cout << "|";
                if (j == size)
                    break;
                if (cells[c].isEmpty())
                    cout << setw(4) << " ";
                else
                {
                    setConsoleColor(LIGHTGREEN, BLUE);
                    cells[c].display();
                    setConsoleColor(GREY, BLACK);
                }
            }
            cout << endl;
        }
        cout << "Score: " << score << "  Highscore: " << highscore << endl;
        cout << "Moves: " << moves << endl;
    }

    void handleKey()
    {
        copy(cells, cells + size * size, prevCells);
        _getch();
        int ch = _getch();
        switch (ch)
        {
        case Left:
            moveCells(Left);
            break;
        case Right:
            moveCells(Right);
            break;
        case Up:
            moveCells(Up);
            break;
        case Down:
            moveCells(Down);
            break;
        }
    }

    void moveCells(Direction dir)
    {
        int start = dir == Left || dir == Up ? 0 : size - 1;
        int end = start == 0 ? size : -1;

        for (int x = 0; x < size; x++)
        {
            for (int y = start; y != end; (start < end ? y++ : y--))
            {
                Cell &c1 = dir == Left || dir == Right ? findCell(x, y) : findCell(y, x);
                if (c1.isEmpty())
                    continue;
                for (int k = y + (start == 0 ? -1 : 1); k != (end == -1 ? size : -1); (start < end ? k-- : k++))
                {
                    Cell &c2 = dir == Left || dir == Right ? findCell(x, k) : findCell(k, x);
                    Cell &c3 = dir == Left ?  findCell(x, k + 1) :
                               dir == Right ? findCell(x, k - 1) :
                               dir == Up ?    findCell(k + 1, x) :
                                              findCell(k - 1, x);
                    if (c1.isEqualTo(c2))
                    {
                        c2.setValue(c2.value * 2);
                        c1.setValue(0);
                        Doubled(c2.value);
                    }
                    else if (!c2.isEmpty() && c3.isEmpty())
                    {
                        c3.setValue(c1.value);
                        c1.setValue(0);
                    }
                    else if (k == start && c2.isEmpty())
                    {
                        c2.setValue(c1.value);
                        c1.setValue(0);
                    }
                }
            }
        }
    }

    void spawnCells(int amount)
    {
        for (int i = 0; i < amount; i++)
        {
            randEmptyCell().setValue(randN(2, 4, 85));
        }
    }

    bool isOver()
    {
        if (!isCellsFull())
            return false;
        for (int i = 0; i < size; i++)
        {
            for (int j = 0; j < size; j++)
            {
                Cell &center = findCell(i, j);
                Cell &right = findCell(i, j + 1);
                Cell &bottom = findCell(i + 1, j);

                if (center.isEqualTo(right) || center.isEqualTo(bottom))
                    return false;
            }
            return true;
        }
    }

    bool isWin()
    {
        for (int i = 0; i < size * size; i++)
        {
            if (cells[i].value == 2048)
                return true;
        }
        return false;
    }
};

void main()
{
    setlocale(LC_ALL, "rus");

    srand(time(0));

    bool playAgain = true;
    int highscore = 0;

    do
    {
        Game game;
        game.create(4);
        game.spawnCells(2);
        game.highscore = highscore;
        game.display();

        do
        {
            game.handleKey();
            if (game.isMoved())
                game.spawnCells(1);
            if (game.score > highscore)
                game.highscore = game.score;
            system("cls");
            game.display();
        } while (!game.isWin() && !game.isOver());

        if (game.isOver())
        {
            setConsoleColor(LIGHTRED, BLACK);
            cout << "You won!" << endl;
            setConsoleColor(GREY, BLACK);
        }
        else if (game.isWin())
        {
            setConsoleColor(LIGHTGREEN, BLACK);
            cout << "You lost!" << endl;
            setConsoleColor(GREY, BLACK);
        }

        cout << "Play again? y/n\n";
        char ch;
        do
        {
            cin >> ch;
            if (ch != 'y' && ch != 'n')
                cout << "Incorrect!\n";
        } while (ch != 'y' && ch != 'n');

        switch (ch)
        {
        case 'y':
            highscore = game.score;
            system("cls");
            break;
        case 'n':
            playAgain = false;
            break;
        }
    } while (playAgain);
}

What do you think needs to be changed/simplified? Is there a way to increase readability? Also I would appreciate some general tips for writing console games like that.

Answer 1

Broken Code

This code:

Cell &findCell(int x, int y)
{
    for (int i = 0; i < size * size; i++)
    {
        if (cells[i].x == x && cells[i].y == y)
            return cells[i];
    }
    return Cell();
}

...has a serious problem--the return Cell(); is attempting to return a reference to a temporary object that will no longer be valid by the time the calling code receives it. At a guess, you must be using an older compiler--the current versions of both VC++ and MinGW both reject this with an error.

Unfortunately, fixing this may not be trivial--you pretty much need to redesign the code to some degree. Available options include:

1. Creating a "not present" object (e.g., with static lifetime, probably as a global), and return a reference to that when the searched-for object can't be found.
2. throw an exception if the object can't be found.
3. return a pointer instead of a reference, allowing you to return a null pointer if not found.
4. Possibly return an Optional<T> (but you can't do an optional reference, so if you really want to go this route, you'd probably need something like an optional<reference_wrapper<Cell>>).

For at least one other example, randEmptyCell has the same problem.

Two Step Initialization

Your Game class has a create member function, which looks like a design smell to me. To create a Game you should just...create a Game. Creating a Game object, then calling create on that object to really create an actual game is two-step initialization, which you usually want to avoid. Generally speaking, if you need to do something to create an object, that should be done in the constructor.

Unnecessary Switch

It looks to me like this switch statement:

    switch (ch)
    {
    case Left:
        moveCells(Left);
        break;
    case Right:
        moveCells(Right);
        break;
    case Up:
        moveCells(Up);
        break;
    case Down:
        moveCells(Down);
        break;
    }
}

...is unnecessarily long and kind of pointless. For the most part, it's equivalent to just: moveCells(ch);, though you may need to assure that ch is one of Up, Down, Left or Right first. One possibility (that still uses a switch statement) would be:

switch (ch) {
case Up:
case Down:
case Left:
case Right:
    moveCells(ch);
}

This accomplishes the job rather more easily.

srand/rand must go

Though using them is somewhat more work, I'd prefer to use the random number generators in random instead of srand/rand. They're generally of considerably higher quality, and they already provide code to handle creating random numbers within a range. I've spent a while working on creating a wrapper to make it somewhat easier to use these, and come up with the following:

#pragma once
#include <array>
#include <random>
#include <algorithm>

class generator {
    template <class Rand> 
    class Seed {
        class seeder {
            std::array < std::random_device::result_type, Rand::state_size > rand_data;
        public:
            seeder() {
                std::random_device rd;
                std::generate(rand_data.begin(), rand_data.end(), std::ref(rd));
            }

            typename std::array < std::random_device::result_type, Rand::state_size >::iterator begin() { return rand_data.begin(); }
            typename std::array < std::random_device::result_type, Rand::state_size >::iterator end() { return rand_data.end(); }
        } seed;

        std::seed_seq s;

    public:
        Seed() : s(seed.begin(), seed.end()) { }

        template <class I>
        auto generate(I a, I b) { return s.generate(std::forward<I>(a), std::forward<I>(b)); }
    };

    using Rand = std::mt19937_64;
    Seed<Rand> seed;
    Rand rng;
    std::uniform_int_distribution<int> uni;
public:
    generator(int high) : rng(seed), uni(0, high) {}
    generator(int low, int high) : rng(seed), uni(low, high) { }
    int operator()() { return uni(rng); }
};

I realize this is a bunch of code that you probably don't want to deal with right now. To keep it simple to use, it's written as a header, so you basically just do something like:

#include "rand.h"

int main() { 
    generator g(100);    // create a generator for numbers from 0 to 100

    // generate and print out some numbers:
    for (int i = 0; i < 100; i++) {
        std::cout << g() << "\t";
        if (i % 10 == 0)
            std::cout << "\n";
    }
}

So, even though it's more code than I'd like, using it is pretty easy (even easier than srand()/rand()), and you get much better random number generation. Oh, one other minor point: as it stands now, this depends on a C++ 17 feature that deduce the template parameter from the value passed to the constructor. For compilers that don't implement that yet, change generator g(100); to generator<int> g(100);.