General Game Loop 2.0

Question

Follow up from General Game Loop

I have two interfaces. Entity for ticks. Drawable for rendering.

Drawable.java

import java.awt.Graphics;

public interface Drawable
{
    public void draw (Graphics g);
}

Entity.java

public interface Entity
{
    public void tick ();
    public void second ();
}

I have a class to initialize the window that everything will be displayed to.

Window.java

import java.awt.Dimension;

import javax.swing.JFrame;

public class Window
{
    public Window (int width, int height, String title, Game game) {
        game.setPreferredSize(new Dimension (width, height));
        game.setMaximumSize(new Dimension (width, height));
        game.setMinimumSize(new Dimension (width, height));

        JFrame frame = new JFrame (title);
        frame.add (game);
        frame.pack ();
        frame.setDefaultCloseOperation (JFrame.EXIT_ON_CLOSE);
        frame.setResizable (false);
        frame.setLocationRelativeTo (null);
        frame.setVisible (true);
    }
}

Then there is the main component.

Game.java

import java.util.ArrayList;
import java.util.List;

import Logging.*;

import java.awt.Canvas;
import java.awt.Color;
import java.awt.Graphics;
import java.awt.image.BufferStrategy;

public class Game extends Canvas implements Runnable {

    /**
     * Run, Tick, Render, Entities and Drawables
     */
    private boolean isRunning = true;
    private Thread thread;

    private static final long NANOSECOND        = 1000000000;
    private static final double OPTIMAL_TICKS   = 50.0;
    private static final double OPTIMAL_TIME    = NANOSECOND / OPTIMAL_TICKS;

    private long lastLoopTime = System.nanoTime ();
    private long currentTime;
    private double deltaTime;
    private long secondTimer = System.currentTimeMillis ();

    private final List<Entity> entities = new ArrayList<> ();
    private final List<Drawable> drawables = new ArrayList<> ();

    /**
     * Window Constants
     */
    private static final int WIDTH      = 800;
    private static final int HEIGHT     = 600;
    private static final String TITLE   = "Test Game";

    /**
     * Logging
     */
    private final FPSViewer fpsViewer = new FPSViewer ();

    // there are more here, stripped for this review

    /**
     * Data Contaienrs
     */
    // there are more here, stripped for this review

    /**
     * Main Function, instantiating Game 
     */
    public static void main() {
        new Game ();
    }

    /**
     * Game constructor, initailized by main ()
     */
    public Game () {
        // fps tracking
        entities.add (fpsViewer);
        drawables.add (fpsViewer);

        Window window = new Window (WIDTH, HEIGHT, TITLE, this);

        thread = new Thread (this);
        thread.start ();
    }

    /**
     * Game Loop
     */
    public void run () {
        while (isRunning) {
            // get delta time
            currentTime = System.nanoTime ();
            deltaTime += (currentTime - lastLoopTime) / OPTIMAL_TIME;
            lastLoopTime = currentTime;

            while (deltaTime >= 1) {
                update ();
                deltaTime--;
            }

            // update the game
            render ();

            // reset if a second has passed
            if (System.currentTimeMillis () - secondTimer > 1000) {
                updatePerSecond ();
                secondTimer += 1000;
            }
        }
    }

    /**
     * Game Updates for all entities
     */
    private void update () {
        for (Entity e : entities) {
            e.tick ();
        }        
    }

    private void updatePerSecond () {
        for (Entity e : entities) {
            e.second ();
        }
    }

    /**
     * Renders all graphics for all drawables
     */
    private void render () {
        BufferStrategy bufferstrategy = getBufferStrategy ();

        if (bufferstrategy == null) {
            createBufferStrategy (3);
            return;
        }

        Graphics g = bufferstrategy.getDrawGraphics();

        g.setColor (Color.white);
        g.fillRect (0, 0, getWidth (), getHeight ());

        for (Drawable d : drawables) {
            d.draw (g);
        }

        g.dispose ();
        bufferstrategy.show ();
    }
}

And finally, to test that everything is working.

FPSViewer.java

import java.awt.Color;
import java.awt.Graphics;

public class FPSViewer implements Drawable, Entity {
    // executions and renderings per second
    private int ticks;
    private int frames;

    private int averageTicks;
    private int averageFrames;

    /**
     * From interface Drawable
     */
    public void draw (Graphics g) {
        frames++;
        g.setColor(Color.black);
        g.drawString("FPS: " + averageFrames + " Ticks: " + averageTicks, 680, 16);
    }

    /**
     * From interface Entitiy
     */
    public void tick () {
        ticks ++;
    }

    public void second () {
        averageFrames = frames / 60;
        averageTicks = ticks;
        frames = 0;
        ticks = 0;
    }
}

How will the actual game work?

• The game will consist mostly of 2D Buttons.
• Mouse and keyboard input.
• 99% of all events will directly be by the player. If the player goes away, the game stops, in most scenarios. (This made me remove the pause part from last iteration.)
• Turn-based combat later on.

Now to the feedback wanted: Everything! Be merciless!

Update

• private static final long NANOSECOND = 1000000000 to private static final long NANOS_IN_SECOND = 1000000000L
• FPSViewer renamed to FPSTracker removed division by 60 for frames.

Answer 1

Your code is pleasant to read, and easy enough to follow. That's a great start. There are a number of issues, mostly on the mild side, but I am concerned about the timing in the game loop.

Simple things

It was pointed out to me recently, that you can't trust System.currentTimeMillis()... if the system clock changes, you could skip forward, or backward in time. The effects here would be trivial, so it's not a huge deal... but just saying.

Nitpick: private static final long NANOSECOND = 1000000000; should have a long designator on the constant: 1000000000L. If it helps, too, in Java you can now have an underscore separator: 1_000_000_000L as well. Also, NANOSECOND is an odd name... should it be NANOS_IN_SECOND?

The Drawable, Entity, and Window all look fine.

FPSViewer

FPSViewer looks odd. Two things:

• Why the / 60? What is the meaning of that? From what I can tell, frames is the count of draw() calls in a second. Why divide it by 60?
• worse, dividing by 60 is an integer operation. You are doing massive truncation in this process.

Game Loop

You have three periodic events that are happening:

1. each second there is a special 'event'
2. each frame-period there is a 'draw'
3. each other time there is a 'tick'

Your loop is complicated, it uses nano-seconds for one timer, milliseconds for another, and it seems the timer for the 'tick' loop is broken....

Let's break your code down:

public void run () {

Fine, you're a runnable, in your own thread (which should have a name, by the way).

    while (isRunning) {

OK, we loop until told not to.

        // get delta time
        currentTime = System.nanoTime ();
        deltaTime += (currentTime - lastLoopTime) / OPTIMAL_TIME;
        lastLoopTime = currentTime;

OK, get the current time, and preserve it for the next loop in lastLoopTime, but what about the line in the middle: deltaTime += (currentTime - lastLoopTime) / OPTIMAL_TIME;?

deltaTime is a double value that does not actually measure a change in time (bad name), it measures the number of ticks to put in before the next 'draw'. The apparent target is 50.0 ticks per second, so, you see whether a full 'tick' worth of time has happened and you tick() as many times as needed, before the draw.

        while (deltaTime >= 1) {
            update ();
            deltaTime--;
        }

Once the ticks are done, you render/draw the components

        // update the game
        render ();

and then you check for the expiry of a second.

        // reset if a second has passed
        if (System.currentTimeMillis () - secondTimer > 1000) {
            updatePerSecond ();
            secondTimer += 1000;
        }
    }
}

To summarize:

• you have a tight loop (CPU at 100%, no idle time)
• you render on every loop
• you tick about 50-times per second
• you have a 1-second routine about once each second.

There is no need to keep most of that information in the class level. Keeping it in the method should be fine.

The loop I would use is as follows:

public void run () {

    long nextTick = System.nanoTime();
    long nextSecond = nextTick;

    final long rateLimit = NANOS_PER_SECOND / 200; // most 200 frames per second....

    while (isRunning) {

        long loopStart = System.nanoTime();

        // update the game
        render ();

        final long now = System.nanoTime();

        if (now - nextTick >= 0) {

            update();

            // catch up with as many ticks as needed (skip them if necessary).
            do {
                nextTick += NANOS_PER_TICK;
            } while (now - nextTick >= 0);

        }

        while (now - nextSecond >= 0) {

            updatePerSecond();

            // catch up with as many ticks as needed (no skipping!).
            nextSecond += NANOS_PER_SECOND;
        }

        final long delayms =  ((loopStart + rateLimit) - System.nanoTime()) / NANOS_PER_MILLISECOND;
        if (delayms > 0) {
            // more than a millisecond wait, do it....
            try {
                Thread.sleep(delayms);
            } catch (InterruptedException ie) {
                // ignore.
            }
        }

    }
}

Update: Alternate game loop

    long nextTick = System.nanoTime();
    long nextSecond = nextTick;
    long nextRender = nextTick;

    while (isRunning) {

        final long now = System.nanoTime();

        if (now - nextRender >= 0) {
            // update the game
            render ();
            do {
                nextRender += NANOS_PER_RENDER;
            } while (now - nextRender >= 0);
        }

        if (now - nextTick >= 0) {
            update();
            // catch up with as many ticks as needed (skip them if necessary).
            do {
                nextTick += NANOS_PER_TICK;
            } while (now - nextTick >= 0);
        }

        if (now - nextSecond >= 0) {
            updatePerSecond();
            // catch up with as many ticks as needed (no skipping!).
            nextSecond += NANOS_PER_SECOND;
        }

        // calculate the delay to the next event.....
        final long workTime = System.nanoTime();
        final long minDelay = Math.min(nextSecond - workTime,
                 Math.min(nextTick - workTime, nextRender - workTime));

        if (minDelay > 0) {
            //next event is no due yet. Yield the system....
            // to some point after the next delay... (millisecond precision)
            long milliDelay = (minDelay + 1_000_000) / 1_000_000L;
            try {
                Thread.sleep(milliDelay);
            } catch (InterruptedException ie) {
                // ignore.
            }
        }
    }
}