Facet-MUD-Project / facetjs

An implementation of the Facet MUD Project in JavaScript
MIT License
1 stars 0 forks source link

Game.gameLoop is not deterministic #21

Open tarkatronic opened 4 years ago

tarkatronic commented 4 years ago

Describe the bug Currently, the game loop is not deterministic -- its behavior depends on outside variables; specifically, the game state. This presents a number of challenges, such as making testing a bit more difficult than it probably should be. For example, in testing, you have to call game.shutdown() before game.gameLoop(), otherwise it will begin actually looping and hang your tests indefinitely.

To Reproduce Steps to reproduce the behavior:

let game = Game.getInstance();
game.gameLoop();

...watch it loop and hang until interrupted.

let game = Game.getInstance();
game.shutdown();
game.gameLoop();

...watch it loop a single time and then exit.

Expected behavior The game loop should be fully deterministic. Given the same inputs, it should produce the same outputs and behavior.

Additional context Also outlined in this report is that this method produces side effects. Specifically, calling setTimeout(). I'm not really sure there is a way around that. But it sticks out to me in this context, as this bug is calling out the game loop as not being a "pure" function.

tarkatronic commented 4 years ago

While talking with @DaftPun, a possibility has come up for how this might work. I'm not sure how well this will play with JS async stuff, but it seems worth trying out. Basically, instead of using setTimeout(...) and the game loop itself always setting this timeout, we could do something like:

/* server.ts */
async runServer() {
    // ...
    while (game.state == GameState.RUNNING) {
        await game.gameLoop();
    }
}

/* game.ts */
async gameLoop() {
    // ...
    // Wait 250ms, adapted from https://stackoverflow.com/a/39914235/1971587
    await new Promise(r => setTimeout(r, 250));
    // NOTE: We should also gather start and end time for the loop, then wait for the greater of that or 250ms.
}

My concern is, this may end up just blocking the event loop and chewing up all the CPU. That is not what we want. But if not... this just might work.