General Game Loop 3.0

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 sprites = new ArrayList<>();
protected final List 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 ti

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.

StackExchange Code Review Q#93085, answer score: 16