6
\$\begingroup\$

How can I improve this game?

#include <iostream>
#include <deque>
#include <vector>
#include <chrono>
#include <random>

#include <conio.h>
#include <Windows.h>
#include <tchar.h>

using Matrix = std::vector<std::vector<int>>;
namespace
{
    HANDLE hStdOut = GetStdHandle(STD_OUTPUT_HANDLE);
    CONSOLE_SCREEN_BUFFER_INFO csbi;
};

void clearScreen()
{
    static DWORD                count;
    static DWORD                cellCount;
    COORD                       homeCoords = { 0, 0 };

    if (!GetConsoleScreenBufferInfo(hStdOut, &csbi))
        std::cerr << "ERROR GetConsoleScreenBufferInfo - clearScreen : "
        << GetLastError() << std::endl;

    cellCount = csbi.dwSize.X *csbi.dwSize.Y;

    FillConsoleOutputCharacter(hStdOut, (TCHAR) ' ', cellCount, homeCoords, &count);

    FillConsoleOutputAttribute(hStdOut, csbi.wAttributes, cellCount, homeCoords, &count);

    SetConsoleCursorPosition(hStdOut, homeCoords);
}

namespace Tile
{
    enum
    {
        Empty,
        Wall,
        Food,
        Snake
    };
}

struct Point2D
{
    Point2D() = default;
    virtual ~Point2D() = default;
    Point2D(std::size_t row, std::size_t col)
    {
        this->row = row;
        this->col = col;
    }

    std::size_t row, col;
};

class NonCopyable
{
public:
    NonCopyable() = default;
    virtual ~NonCopyable() = default;

    NonCopyable(const NonCopyable &) = delete;
    NonCopyable(const NonCopyable &&) = delete;
    NonCopyable& operator = (const NonCopyable&) = delete;
};

template<typename T>
struct Random : public NonCopyable
{
    Random(float min, float max)
        : mUniformDistribution(min, max)
    {}

    T operator()()
    {
        return mUniformDistribution(mEngine);
    }

    std::default_random_engine mEngine{ std::random_device()() };

    template <typename U>
    static auto dist() -> typename std::enable_if<std::is_integral<U>::value, std::uniform_int_distribution<U>>::type;

    template <typename U>
    static auto dist() -> typename std::enable_if<std::is_floating_point<U>::value, std::uniform_real_distribution<U>>::type;

    decltype(dist<T>()) mUniformDistribution;
};

struct World : public NonCopyable
{
public:
    World() 
    {
        snake.emplace_back(Point2D(7, 7));
    }

    void performAI();

    bool moveSnake();

    std::size_t numberEatrn() const
    {
        return numEaten;
    }
private:
    Random<double> getRandom{ 0.0, 1.0 };

    bool randomChance(double time)
    {
        return getRandom() < time;
    }

    void placeFood();

    bool worldBoundry(const Point2D& pt) const;

    bool isCollide(const Point2D& head) const;

    Point2D getNextPosition(int dx, int dy);

    void draw(std::ostream& stream) const;

    friend std::ostream& operator<<(std::ostream& stream, const World& self)
    {
        self.draw(stream);
        return stream;
    }

    Matrix mField
    {
        {
            { 1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1 },
            { 1,2,0,0,0,0,0,0,0,0,0,0,0,0,0,2,1 },
            { 1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1 },
            { 1,0,0,0,1,0,0,0,0,0,0,0,1,0,0,0,1 },
            { 1,0,0,1,1,1,0,0,2,0,0,1,1,1,0,0,1 },
            { 1,0,0,0,1,0,0,0,0,0,0,0,1,0,0,0,1 },
            { 1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1 },
            { 1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1 },
            { 1,0,0,0,1,0,0,0,0,0,0,0,1,0,0,0,1 },
            { 1,0,0,0,1,0,0,0,0,0,0,0,1,0,0,0,1 },
            { 1,0,0,0,1,0,0,0,2,0,0,0,1,0,0,0,1 },
            { 1,0,0,0,1,0,0,0,0,0,0,0,1,0,0,0,1 },
            { 1,0,0,0,1,0,0,0,0,0,0,0,1,0,0,0,1 },
            { 1,2,0,0,0,0,0,0,0,0,0,0,0,0,0,2,1 },
            { 1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1 }
        }
    };

    std::deque<Point2D> snake; 
    std::minstd_rand rndEngine;

    std::size_t numRows = 17, numCols = 15;
    int dx = 1, dy = 0; 
    std::size_t numEaten = 0;
    const double mTurnRate = 0.2;
};

void World::placeFood()
{
    std::size_t row = rndEngine() % numRows;
    std::size_t col = rndEngine() % numCols;

    if (mField[row][col] == Tile::Empty)
    {
        mField[row][col] = Tile::Food;
    }
}

void World::draw(std::ostream& stream) const
{
    clearScreen();

    for (auto&& i : mField)
    {
        for (auto&& j : i)
        {
            switch (j)
            {
            case Tile::Wall:
                stream << '#';
                break;
            case Tile::Food:
                stream << '$';
                break;
            case Tile::Snake:
                stream << '*';
                break;
            default:
                stream << ' ';
                break;
            }
        }
        stream << '\n';
    }

    std::cout << "Food eaten: " << numEaten << '\n';
}

bool World::worldBoundry(const Point2D& point) const
{
    return point.col < numCols - 1 && point.row < numRows - 1;
}

bool World::isCollide(const Point2D& head) const
{
    return !worldBoundry(std::move(head)) ||
        mField[head.row][head.col] == Tile::Snake ||
        mField[head.row][head.col] == Tile::Wall;
}

Point2D World::getNextPosition(int dx, int dy)
{
    Point2D nextSpot = snake.front();

    nextSpot.col += dx;
    nextSpot.row += dy;

    return nextSpot;
}

void World::performAI()
{
    Point2D nextSpot = getNextPosition(dx, dy);

    if (isCollide(nextSpot) || randomChance(mTurnRate))
    {
        int leftDx = -dy;
        int leftDy = dx;

        int rightDx = dy;
        int rightDy = -dx;

        bool canMoveLeft = !isCollide(getNextPosition(leftDx, leftDy));
        bool canMoveRight = !isCollide(getNextPosition(rightDx, rightDy));

        bool turnLeft = false;
        double timeToTurn = 0.5;

        if (!canMoveLeft && !canMoveRight)
        {
            return;
        }
        else if (canMoveLeft && !canMoveRight)
        {
            turnLeft = true;
        }
        else if (!canMoveLeft && canMoveRight)
        {
            turnLeft = false;
        }
        else
        {
            turnLeft = randomChance(timeToTurn);
        }

        dx = turnLeft ? leftDx : rightDx;
        dy = turnLeft ? leftDy : rightDy;
    }
}

bool World::moveSnake() 
{
    Point2D nextHead = getNextPosition(dx, dy);

    if (isCollide(nextHead))
    {
        return false;
    }

    bool isFood = (mField[nextHead.row][nextHead.col] == Tile::Food);

    mField[nextHead.row][nextHead.col] = Tile::Snake;

    snake.push_front(nextHead);

    if (isFood)
    {
        placeFood();
        ++numEaten;
    }
    else
    {
        mField[snake.back().row][snake.back().col] = Tile::Empty;
        snake.pop_back();
    }

    return true;
}

class Game : public NonCopyable
{
public:
    void run();

private:
    void displayResult();

    World mWorld;
    const double mWaitTime = 0.1;
    const size_t mMaxFood = 7;
};

void Game::displayResult()
{
    if (mWorld.numberEatrn() == mMaxFood)
    {
        std::cout << "Yay!  The snake won!\n";
    }
    else
    {
        std::cout << "Oh no!  The snake crashed!\n";
    }
}

void Game::run()
{
    auto start = std::chrono::high_resolution_clock::now();

    while (mWorld.numberEatrn() < mMaxFood)
    {
        auto end = std::chrono::high_resolution_clock::now();

        double timeTakenInSeconds = (end - start).count()
            * (static_cast<double>(std::chrono::high_resolution_clock::period::num)
            / std::chrono::high_resolution_clock::period::den);

        if (timeTakenInSeconds > mWaitTime)
        {
            mWorld.performAI();
            if (!mWorld.moveSnake())
            {
                break;
            }
            std::cout << mWorld;
            start = std::chrono::high_resolution_clock::now();
        }
    }

    displayResult();
}

int main() 
{
    Game game;
    game.run();
}
\$\endgroup\$
2
  • \$\begingroup\$ With this early checkmark, I assume these improvements were already enough to (assumably) post a better follow-up question. I might look at more of the code later, but as I've mentioned, it will be easier to read as separate files. \$\endgroup\$
    – Jamal
    Jan 19, 2015 at 20:58
  • \$\begingroup\$ @Jamal .. i think you are reading my mind :) \$\endgroup\$
    – MORTAL
    Jan 20, 2015 at 12:09

2 Answers 2

3
\$\begingroup\$

My suggestions:

Change default constructor of Point2D

When you use

Point2D() = default;

and a user construct an object using:

Point2D p;

the members row and col are uninitialized.

Change it to:

Point2D() : row(0), col(0) {}

Initialize members in the initializer list

Change

Point2D(std::size_t row, std::size_t col)
{
   this->row = row;
   this->col = col;
}

to

Point2D(std::size_t r, std::size_t c) :  row(r), col(c) {}

Fix a spelling error in function name

I suspect this is a typo. Fix it anyway.

Change

  std::size_t numberEatrn() const

to

  std::size_t numberEaten() const

getRandom and randomChoice()

You have a private member variable named getRandom, which is initialized as:

     Random<double> getRandom{ 0.0, 1.0 };
  1. I think getRandom is a misleading choice of name for a variable. I would expect getRandom to be the name of function. I would suggest using a different name, such as random.

  2. It's used only in randomChoice(). If there is ever going to be one instance of game in your appliction, which seems like it is, I would suggest putting random as a static variable in randomChoice. It keeps the scope of the variable limited only to where it is needed.

    bool randomChance(double time)
    {
       static Random<double> random{ 0.0, 1.0 };
       return random() < time;
    }
    
  3. This is a oh-so-weak suggestion. I would change Random::operator()() to Random::get(). If you do that, the implementation of randomChoice would be:

    bool randomChance(double time)
    {
       static Random<double> random{ 0.0, 1.0 };
       return random.get() < time;
    }
    

    That feels better to me than

       return random() < time;
    

Replace a switch by using an array in World::draw

void World::draw(std::ostream& stream) const
{
   static char out[] = {" #$*"};

   clearScreen();

   for (auto&& i : mField)
   {
      for (auto&& j : i)
      {
         stream << out[j];
      }
      stream << '\n';
   }

   std::cout << "Food eaten: " << numEaten << '\n';
}
\$\endgroup\$
1
  • \$\begingroup\$ i thought that the key word default is going to initialize members data automatically. what the point of default if its act as empty body {} only \$\endgroup\$
    – MORTAL
    Jan 20, 2015 at 12:08
5
\$\begingroup\$
  • This should probably be split into separate files. Each class could have its own file, and main() should also have its own. Even a program of this length is hard to follow well in a single block, and the program may get even larger.

  • I would not have the using and namespace lines right next to each other. They are not at all related and may mislead someone.

  • The enum could instead be an enum class. Even if it's not needed right now, it may be useful in case it ever needs to be changed.

  • NonCopyable can be a struct.

  • displayResult() should be const.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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