General Game Loop 3.0

Question

Follow up from General Game Loop 2.0

It has been quite a while. Some major changes involve:

• Removed dependancy on Swing. The more I read up on Swing, the more I understood it was meant for handling forms. Directly "Swing is a GUI widget toolkit for Java". As games generally depend a on GUI but aren't solely based on it I figured I would do that writing myself.
• Mouse and Keyboard input have been implemented.
• Event handling is implemented, reflection is used to call the right method or active object's method.
• Scenes have been implemented, see they as the "current room". Think page in an install wizard.

Scenes are responsible for all active sprites and objects on the screen. You can ignore all Z-classes as of now, they might be in a future question.

The idea is if the core class can't perform an event, it will give it to the scene, if the scene can't handle the event it will be passed on to the active container. If that one can't handle it then and only then will the event be trashed.

package scene;

import java.awt.Graphics;
import java.awt.Image;
import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
import java.util.List;

import zlibrary.ZBackground;
import zlibrary.ZDrawable;
import zlibrary.ZObject;
import core.Event;
import core.EventAdder;

public abstract class Scene
{
    private ZBackground background;
    protected final List<ZDrawable> sprites = new ArrayList<>();
    protected final List<ZObject> objects = new ArrayList<>();
    protected EventAdder eventAdder;

    public Scene (Image image, EventAdder eventAdder) {
        background = new ZBackground (image, 0, 0);
        this.eventAdder = eventAdder;
    }

    /**
     * Rendering, called from render in Game.java
     * @param g - 
     */
    public void render (Graphics g) {
        background.render(g);
        for (ZDrawable sprite : sprites) {
            sprite.render(g);
        }
    }

    /**
     * Updating, called from tick in Game.java
     */
    public void tick () {
        for (ZObject o : objects) {
            o.tick ();
        }
    }

    /**
     * Iterates through all objects existing in current scene and checks if that button was pressed by key input
     * @param keyCode - which key was pressed
     */
    public void keyPressed(int keyCode) {
        for (ZObject o : objects) {
            if (o.isKeyHotkey (keyCode)) {
                o.pressed ();
            }
        }
    }

    /**
     * Iterates through all objects existing in current scene and checks if that button was pressed by mouse input
     * @param x - mouse position in x when left button was pressed
     * @param y - mouse position in y when left button was pressed 
     */
    public void buttonPressed(int x, int y) {
        for (ZObject o: objects) {
            if (o.isMouseInside (x, y)) {
                o.pressed ();
            }
        }
    }

    /**
     * Removes this scene, called from switchScene in Game.java when switching scene
     */
    public void remove () {
        sprites.clear();
        objects.clear();
    }

    /**
     * Event handler for current scene; uses reflection to invoke method based on event
     * @param event - which event was triggered from Game.java
     */
    public void eventHandler (Event event) {
        System.out.println(this.getClass().getName() + ": Event: " + event.toString());     
        try {
            this.getClass().getMethod(event.getMethod(), String.class).invoke(this, event.getArgument());
        } catch (IllegalAccessException | IllegalArgumentException
                | InvocationTargetException | SecurityException
                | NoSuchMethodException e) {
            e.printStackTrace();
            System.exit(-1);
        }
    }

    /**
     * Overridden toString
     * @return the name of the scene
     */
    public String toString () {
        return this.getClass().getName();
    }
}

For the main class.

package core;

import java.awt.Canvas;
import java.awt.Dimension;
import java.awt.Frame;
import java.awt.Graphics;
import java.awt.Image;
import java.awt.event.KeyAdapter;
import java.awt.event.KeyEvent;
import java.awt.event.MouseAdapter;
import java.awt.event.MouseEvent;
import java.awt.event.WindowAdapter;
import java.awt.event.WindowEvent;
import java.awt.image.BufferStrategy;
import java.lang.reflect.InvocationTargetException;

import scene.Scene;

class Game extends Canvas implements Runnable {     
    // Run, Tick, Render, Entities and Drawables
    private static final long serialVersionUID      = 7629246777343825696L;
    private boolean isRunning;
    private Thread thread;
    private long now                                = System.nanoTime ();
    private long nextTick                           = now;
    private long nextRender                         = now;

    // Window Settings, Event Queue and Current Scene
    private Frame frame;
    private final EventQueue eventQueue             = new EventQueue ();
    private Scene currentScene;

    /**
     * Constructor, initialized by main ()
     */
    private Game () {
        // set up the window
        initWindow ();

        // off we go
        thread = new Thread(this);
        thread.start ();
    }

    /**
     * Game Loop, executed in thread
     */
    public void run () {        
        // off we go
        isRunning = true;

        // set up title screen
        eventQueue.getEventAdder().add(new Event ("switchScene,TitleScreen"));

        while (isRunning) {
            now = System.nanoTime ();

            // get all events and execute them
            Event currentEvent;
            while ((currentEvent = eventQueue.get()) != null ) {
                eventHandler (currentEvent);
            }

            // call tick for all entities
            if (now - nextTick >= 0) {
                update();
                do {
                    nextTick += Constants.NANOS_PER_TICK;
                } while (now - nextTick >= 0);
            }

            // call draw for all drawables
            if (now - nextRender >= 0) {
                render ();
                do {
                    nextRender += Constants.NANOS_PER_RENDER;
                } while (now - nextRender >= 0);
            }

            // yield time to other processes
            final long workTime = System.nanoTime();
            final long minDelay = Math.min(nextTick - workTime, nextRender - workTime);

            if (minDelay > 0) {
                sleep ((long)((minDelay + Constants.NANOS_PER_MILLISECOND) / Constants.NANOS_PER_MILLISECOND));
            }
        }
    }

    /**
     * Switches from one scene to another, removes the old one as well
     * @param scene - which scene to switch to
     */
    public void switchScene (String scene) {
        if (currentScene != null) {
            currentScene.remove ();
        }
        try {
            Class<?> sceneToLoad = Class.forName("scene."+scene);
            System.out.println("--"+sceneToLoad.toString());

            Class[] arguments = new Class[2];
            arguments[0] = Image.class;
            arguments[1] = EventAdder.class;

            currentScene = (Scene) sceneToLoad.getConstructor(arguments).newInstance(null, eventQueue.getEventAdder());
        } catch (ClassNotFoundException | InstantiationException | IllegalAccessException |
                 IllegalArgumentException | InvocationTargetException | NoSuchMethodException |
                 SecurityException e) {
            e.printStackTrace();
            eventQueue.getEventAdder().add(new Event ("exitGame,"+Constants.ERROR_NO_SUCH_SCENE_EXISTS));
        }
        System.out.println("Switched scene to: " + currentScene.toString());
    }

    /**
     * Exits game, saves the events.
     * @param code - code to exit with
     */
    public void exitGame (String code) {
        System.exit(Integer.parseInt(code));
    }

    /**
     * Event handler for Game; uses reflection to invoke method based on event
     * @param event - which event was triggered
     */
    public void eventHandler (Event event) {
        System.out.println("Event: " + event.toString());
        try {
            this.getClass().getMethod(event.getMethod(), String.class).invoke(this, event.getArgument());
        } catch (IllegalAccessException | IllegalArgumentException
                | InvocationTargetException | SecurityException e) {
            e.printStackTrace();
            eventQueue.getEventAdder().add(new Event ("exitGame,"+Constants.ERROR_INVALID_EVENT));
        } catch (NoSuchMethodException e) {
            currentScene.eventHandler(event);
        }
    }

    /**
     * Game Updates for current scene
     */
    private void update () {
        currentScene.tick ();
    }

    /**
     * Renders current scene
     */
    private void render () {
        BufferStrategy bufferstrategy = getBufferStrategy ();

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

        Graphics g = bufferstrategy.getDrawGraphics();
        g.clearRect(0, 0, Constants.WIDTH, Constants.HEIGHT);

        currentScene.render(g);

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

    /**
     * Helper for Game, initializes the frame, adds window, key and mouse listener
     */
    private void initWindow () {
        setPreferredSize (new Dimension (Constants.WIDTH, Constants.HEIGHT));
        setMaximumSize (new Dimension (Constants.WIDTH, Constants.HEIGHT));
        setMinimumSize (new Dimension (Constants.WIDTH, Constants.HEIGHT));

        addKeyListener(new KeyAdapter () {
            public void keyReleased (KeyEvent key) {
                currentScene.keyPressed(key.getKeyCode());
            }
        });

        addMouseListener(new MouseAdapter () {
            public void mousePressed(MouseEvent mb) {
                if (mb.getButton() == MouseEvent.BUTTON1) {
                    currentScene.buttonPressed(mb.getX(), mb.getY());
                }
            }
        });

        frame = new Frame (Constants.TITLE);
        frame.add (this);
        frame.setResizable (false);
        frame.pack ();
        frame.setLocationRelativeTo (null);
        frame.setVisible (true);

        frame.addWindowListener (new WindowAdapter () {
            public void windowClosing(WindowEvent we){
                eventQueue.getEventAdder().add(new Event ("exitGame,"+0));
            }
        });

        frame.addKeyListener(new KeyAdapter () {
            public void keyReleased (KeyEvent key) {
                currentScene.keyPressed(key.getKeyCode());
            }
        });
    }

    /**
     * Helper for Game, lets the current thread sleep
     * @param delayMS - time in milliseconds to sleep
     */
    private void sleep (long delayMS) {
        try {
            Thread.sleep(delayMS);
        } catch (InterruptedException ie) {}
    }

    /**
     * Main Function, instantiating Game 
     * @param args - system input, none used
     */
    public static void main(String[] args) {
        new Game();
    }
}

Some thoughts I have, and some direct problems I know exist.

/**
 * Helper for Game, initializes the frame, adds window, key and mouse listener
 */
private void initWindow () {
    ...
}

Currently has double sets of keyListener and windowsListener. Both the frame and the window itself currently needs them. The ideal solution would be to only need for one of them.

Messy Reflection

My idea was to get it as dynamic as possible, to never have to touch the core files after a certain point in development. However, they didn't become as dynamic as I wanted, a lot of type casting and error checking is needed here and there for it work.

Split Game.java

I've been thinking about moving everything directly related to the Window and the Frame out of the Game.java class and instead let Game have a Window class, maybe even split up Window and Frame.

Small addition: The package names are a bit temporary as well, do not worry about them not following the naming convention. They will later on when I have the structure more set in stone.

Answer 1

First of all, it's a lot easier to review code if I can easily compile and run it on my PC. When reviewers can run your code, they can also test their theories, and often tell you more than theories, and include actual code that you can use. So if you want interesting answers, make it easier to review your code, ideally provide a GitHub link.

The idea is if the core class can't perform an event, it will give it to the scene, if the scene can't handle the event it will be passed on to the active container. If that one can't handle it then and only then will the event be trashed.

Sounds like the Chain of responsibility pattern. That's what you should be doing instead of the messy reflection stuff. This is one of the biggest problems in your code.

The other big problem is that Game is doing too much:

• manages a thread
• manages the event queue
• configures a frame

As you suspected, you need to split this up.

Simple improvements

Some simple improvements are possible:

• Make everything final that you can. Scene.background is never reassigned, so it can be final.

• Convert member variables to local variables when possible. For example isRunning, thread, frame.

Common bad practices

Some known bad practices that static analysis tools would point out about your code:

• Printing to console: consider using a logger instead

• Exiting in the middle of the code: find a more graceful way to shut down your program.

Strange code

The way you process the event queue is more complicated than it needs to be:

Event currentEvent;
while ((currentEvent = eventQueue.get()) != null) {
    eventHandler(currentEvent);
}

The natural way to process a queue would be more like this:

while (!queue.isEmpty()) {
    eventHandler(queue.poll());
}

---

I'm wondering why you write this kind of conditions:

if (now - nextTick >= 0) {

Instead of the more natural:

if (now >= nextTick) {

And why do this kind of loop:

do {
    nextTick += Constants.NANOS_PER_TICK;
} while (now - nextTick >= 0);

Instead of the faster math:

nextTick += (1 + (now - nextTick) / Constants.NANOS_PER_TICK) * Constants.NANOS_PER_TICK;

Other less obvious bad practices and code smell

A class/interface called Constants smells. Is that really the best place to put WIDTH, HEIGHT, TITLE? I seriously doubt it. I suggest to delete that class and move the constants out to more appropriate places.

Keep in mind that there's no need to keep all constants in the same place. They should be in the place where they are needed and make the most sense.

---

The Game constructor immediately starts executing a thread. This is a bit odd. Take a hint from the JDK's Thread class: it doesn't starts executing itself immediately upon construction, it's a separate action.

---

About Scene.keyPressed, can you associate multiple objects to the same hot key? If yes, the method is fine. If not, then instead of iterating over the objects it would be better to use a map.

---

remove is not a great name for a method to remove self. A method named "remove" usually takes an object to remove as parameter. Maybe cleanup would be better.

Answer 2

In your render method you have a magic number, and I don't know what it does:

private void render () {
    BufferStrategy bufferstrategy = getBufferStrategy ();

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

    Graphics g = bufferstrategy.getDrawGraphics();
    g.clearRect(0, 0, Constants.WIDTH, Constants.HEIGHT);

    currentScene.render(g);

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

Mainly this line of code:

createBufferStrategy(3);

I assume that 3 is a type of BufferStrategy that you want, but I read it and don't know what this means. You are creating a BufferStrategy with a value of 3; 3 what?

This would be a good spot for a brief comment saying what this means.