Terminal based game: Part 2

Question

Follow up to: Terminal based game

Finished up the framework.

Still sticking with the X-Term based version.
As I want a very simple framework to use for teaching (not this part initially).

But my next question is the snake game implemented using this class.

#ifndef THORSANVIL_GAMEENGINE_GAME_H
#define THORSANVIL_GAMEENGINE_GAME_H

#include <iostream>
#include <signal.h>
#include <unistd.h>
#include <termios.h>
#include <sys/select.h>


static volatile sig_atomic_t done = 0;
extern "C" void signalHandler(int signal)
{
    if (!done) {
        done = signal;
    }
}

namespace ThorsAnvil::GameEngine
{

bool installHandler(int signal)
{
    struct sigaction    action;
    sigemptyset(&action.sa_mask);
    action.sa_flags     = 0;
    action.sa_handler   = signalHandler;

    int result = sigaction(signal, &action, nullptr);
    return result == 0;
}


class Game
{
    using Time = std::chrono::time_point<std::chrono::high_resolution_clock>;
    using Duration = std::chrono::duration<double, std::micro>;
    using Step = std::chrono::duration<double, std::milli>;

    static constexpr char clearScreen[]     = "\033[2J";
    static constexpr char moveToZeroZero[]  = "\033[0;0H";
    static constexpr char hideCursor[]      = "\033[?25l";
    static constexpr char showCursor[]      = "\033[?25h";

    bool        gameOver;
    termios     originalConfig;
    Time        nextDrawTime;
    Time        nextStepTime;
    Duration    durationDrawTime;
    Duration    durationStepTime;
    Duration    timeNeeded;
    int         sleep;

    int sleepTime()
    {
        Time     now            = std::chrono::high_resolution_clock::now();
        Duration sleepForDraw   = nextDrawTime - now - timeNeeded;
        Duration sleepForStep   = nextStepTime - now;
        Duration smallestSleep  = sleepForDraw < sleepForStep ? sleepForDraw : sleepForStep;
        int      microSeconds   = smallestSleep.count();
        return microSeconds > 0 ? microSeconds : 1;
    }
    void draw()
    {
        Time lastDrawTime    = std::chrono::high_resolution_clock::now();
        std::cout << moveToZeroZero;
        drawFrame();
        std::cout << "Sleep: " << sleep << " \n";

        durationDrawTime    = std::chrono::high_resolution_clock::now() - lastDrawTime;
        timeNeeded          = durationDrawTime + durationStepTime;
        nextDrawTime        = lastDrawTime + std::chrono::milliseconds(redrawRateMilliSeconds());
    }
    void input()
    {
        if (done) {
            gameOver = true;
            return;
        }

        fd_set  input;
        FD_ZERO(&input);
        FD_SET(STDIN_FILENO, &input);

        sleep = sleepTime();
        timeval timeout{sleep / 1'000'000, sleep % 1'000'000};
        if (select(STDIN_FILENO + 1, &input, nullptr, nullptr, &timeout) > 0) {
            char key = std::cin.get();

            handleInput(key);
        }
    }
    void logic()
    {
        int timeUp              = (nextStepTime - std::chrono::high_resolution_clock::now()).count();

        if (timeUp <= 0) {
            Time lastStepTime   = std::chrono::high_resolution_clock::now();
            handleLogic();
            durationStepTime    = std::chrono::high_resolution_clock::now() - lastStepTime;
            timeNeeded          = durationDrawTime + durationStepTime;
            nextStepTime        = lastStepTime + std::chrono::milliseconds(gameStepTimeMilliSeconds());
        }
    }
    protected:
        virtual void drawFrame()  = 0;
        virtual int  gameStepTimeMilliSeconds() {return 500;}
        virtual int  redrawRateMilliSeconds()   {return gameStepTimeMilliSeconds();}
        virtual void handleInput(char k)
        {
            if (k == 'Q') {
                gameOver = true;
            }
        }
        virtual void handleLogic()  {}

        void setGameOver()
        {
            gameOver = true;
        }
        void newGame()
        {
            gameOver = false;
        }

    public:
        Game()
            : gameOver(false)
            , sleep(0)
        {
            if (!installHandler(SIGINT) || !installHandler(SIGHUP) || !installHandler(SIGTERM)) {
                throw std::runtime_error("Fail: Installing signal handlers");
            }

            termios     config;
            if (tcgetattr(STDIN_FILENO, &originalConfig) != 0 || tcgetattr(STDIN_FILENO, &config) != 0) {
                throw std::runtime_error("Fail: Getting keyboard state");
            }
            config.c_lflag &= ~(ICANON | ECHO);
            config.c_cc[VMIN] = 1; /* Blocking input */
            config.c_cc[VTIME] = 0;
            if (tcsetattr(STDIN_FILENO, TCSANOW, &config) != 0) {
                throw std::runtime_error("Fail: Setting keyboard state");
            }
        }
        virtual ~Game()
        {
            tcsetattr(STDIN_FILENO, TCSAFLUSH, &originalConfig);
        }
        void run()
        {
            std::cout << clearScreen
                      << hideCursor;


            while (!gameOver) {

                draw();

                input();
                logic();
            }

            std::cout << showCursor;
        }
};

}
#endif

Not 100% confident in the chrono stuff. Any feedback on how to do that better really appreciated.

Also: I am not sure about if I should have two timers. One for display refresh gameStepTimeMilliSeconds() and one for step refresh gameStepTimeMilliSeconds(). Their interactions seems to be minor but anybody with experience in this area would love to get input on that.

Note: The linked Snake game has a draw time (unoptimized) for a brute force draw of the screen of around 950 micro seconds. So potentially we could do 1000 frames a second. This weekend I will explore the timings of an optimized draw (i.e. only updating the characters that would change).

Answer 1

Missing #includes

My compiler complains about a lot of undefined functions and types, because you forgot to add all the necessary #include statements.

Since this is a header only, create a .cpp file that only includes your header, and try to compile it to an object file.

Use std::chrono::steady_clock

Unfortunately, it is not well-defined what kind of clock std::chrono::high_resolution_clock actually is. It is best to avoid it. Instead, use std::chrono::steady_clock; it is guaranteed to be steady (ie, doesn't have jumps because of NTP updates, daylight savings time changes or leap seconds), and likely has the same resolution as std::chrono::high_resolution_clock anyway.

Avoid specifying time resolution unless really necessary

You are dealing with explicit milliseconds and microseconds way too early. Try to keep durations in unspecified std::chrono::duration variables for as long as possible. You should only convert it to a concrete value at the last possible moment, which is right before calling select().

I recommend using this pattern:

using Clock = std::chrono::steady_clock;
using Time = Clock::time_point;
using Duration = Clock::duration;
using Rep = Duration::rep;
…
Duration sleepTime()
{
    auto now = Clock::now();
    auto sleepForDraw   = nextDrawTime - now - timeNeeded;
    auto sleepForStep   = nextStepTime - now;
    return std::min(sleepForDraw, sleepForStep);
}

void input() {
    …
    auto sleep_us = std::max(Rep{1}, 
        std::chrono::duration_cast<std::chrono::microseconds>(sleepTime()).count());
    timeval timeout{sleep / 1'000'000, sleep % 1'000'000};
    …
}

Note how much simpler sleepTime() is now. Also, why was there a Step type to begin with? You are not using it anywhere.

Issues with select()

It is possible for select() to return 1 even if there is no character available to read from std::cin.

Another issue that is more likely to occur is that std::cin is allowed to buffer its input. Consider that I press two keys in very short succession; they might both get read into std::cin's underlying stream buffer at the same time. So at first select() returns 1, and you call std::cin.get() which will return the first key. But when you call select() again, since both keys have already been read into a buffer, it will wait for a third key to be pressed.

There is no way you can safely mix select() with std::cin(). The best you can do without an external library is to make the POSIX filedescriptor 0 non-blocking, and then to read() characters from it.

Move setup and cleanup to the constructor and destructor

In run() you hide the cursor and clear the screen before doing the actual run loop, and then afterwards you show the cursor again. These things should be done in the constructor and destructor instead. Consider what happens if an exception is thrown while the game is running.

If you really want to keep it inside run(), then use an RAII object to manage the cursor visibility state.