Problem
I just made this Java game loop and I want someone’s opinion on it. It’s based off what I’ve heard and read. Let me know if I went about it correctly and if there’s any big problems with it.
public void run() {
long nanosecond = 1000000000;
int targetFps = 60;
int targetUpdates = 20;
long update = nanosecond / targetUpdates;
long draw = nanosecond / targetFps;
long lastUpdate = System.nanoTime();
long lastDraw = System.nanoTime();
long lastTime = System.nanoTime();
int fps = 0;
int updates = 0;
while (running) {
long now = System.nanoTime();
while ((now - lastUpdate) > update) {
update();
lastUpdate = now;
updates++;
}
while ((now - lastDraw) > draw) {
draw();
lastDraw = now;
fps++;
}
if ((now - lastTime) >= nanosecond) {
lastTime = System.nanoTime();
System.out.println("Fps: "+ fps + " | Updates: " + updates);
fps = 0;
updates = 0;
}
}
}
Solution
Problem statement
Nothing to do with the code, but when asking a question here, it is helpful to give us a statement of what the code does. Reading your code, this is what I think it is intended to do.
This code loops until code elsewhere marks the object field
running
asfalse
. Every so many nanoseconds, it redraws the screen. Every so many nanoseconds, it updates something (I don’t actually know what). The code tracks how many updates and redraws it does and every second reports that information. After reporting, it clears the numbers for next time.
I wish there was more detail, but this at least describes what the code itself does. You could improve it with more information about what happens when a draw
or update
occurs.
The title might be “Game loop that tracks frame speed and update frequency”.
Constants
long nanosecond = 1000000000;
int targetFps = 60;
int targetUpdates = 20;
long update = nanosecond / targetUpdates;
long draw = nanosecond / targetFps;
These values seem to be constant for the run of the program. Consider defining them outside the method as constants.
public final long NANOSECONDS_PER_SECOND = 1000000000;
public final int FPS_TARGET = 60;
public final int UPDATE_TARGET = 20;
public final long UPDATE_INTERVAL = NANOSECONDS_PER_SECOND / UPDATE_TARGET;
public final long DRAW_INTERVAL = NANOSECONDS_PER_SECOND / FPS_TARGET;
I also changed the names. Both to comply with naming standards for constants and because I didn’t like the originals. Naming is hard, so these names may still be imperfect, but I find them easier to follow.
I don’t like nanosecond
as it doesn’t describe what the variable does. It certainly does not represent a nanosecond. The name second
would actually be more accurate, but it’s really the ratio.
I prefer to put TARGET
last in the name.
I find both update
and draw
unclear. They aren’t updates and draws. They are the intervals at which those things happen.
Functionality
long lastUpdate = System.nanoTime();
long lastDraw = System.nanoTime();
long lastTime = System.nanoTime();
I’ll explain more about why later, but I’d actually write this as
long nextUpdate = System.nanoTime() + UPDATE_INTERVAL;
long nextDraw = System.nanoTime() + DRAW_INTERVAL;
long nextReset = System.nanoTime() + NANOSECONDS_PER_SECOND;
This is a functional change. Rather than tracking the most recent whatever, track the next one.
I changed Time
to Reset
because that’s what you do when the time period is up.
More naming
int fps = 0;
int updates = 0;
I’d prefer
int frameCount = 0;
int updateCount = 0;
The latter is just because I like naming counts as such. I find it easier to keep track of what I’m doing. I’d expect updates
to be a collection representing the updates that occurred.
But for fps
, that’s not what it is. FPS means Frames Per Second in my lexicon. So what you would be counting here is frames, not “fps”.
Back to functionality
long now = System.nanoTime();
while ((now - lastUpdate) > update) {
update();
lastUpdate = now;
updates++;
}
while ((now - lastDraw) > draw) {
draw();
lastDraw = now;
fps++;
}
if ((now - lastTime) >= nanosecond) {
lastTime = System.nanoTime();
System.out.println("Fps: "+ fps + " | Updates: " + updates);
fps = 0;
updates = 0;
}
I might change this to
long now = System.nanoTime();
while (now > nextUpdate) {
update();
nextUpdate = now + UPDATE_INTERVAL;
updateCount++;
}
while (now > nextDraw) {
draw();
nextDraw = now + DRAW_INTERVAL;
frameCount++;
}
if (now >= nextReset) {
nextReset = now + NANOSECONDS_PER_SECOND;
System.out.println("Fps: "+ frameCount + " | Updates: " + updateCount);
frameCount = 0;
updateCount = 0;
}
Now, rather than subtracting the last time from the current time and comparing it to the desired interval, we just compare the already calculated next time to the current time. So we compare more often and therefore more accurately.
Also consider
long now = System.nanoTime();
while (now > nextUpdate) {
update();
nextUpdate += UPDATE_INTERVAL;
updateCount++;
}
while (now > nextDraw) {
draw();
nextDraw += DRAW_INTERVAL;
frameCount++;
}
if (now >= nextReset) {
nextReset += NANOSECONDS_PER_SECOND;
double interval = (double) (now - lastReset);
double fps = frameCount / interval;
double updatesPerSecond = updateCount / interval;
System.out.println("Fps: "+ fps + " | Updates: " + updatesPerSecond);
frameCount = 0;
updateCount = 0;
lastReset = now;
}
What this does, if something is a bit late, the next one will still occur at the scheduled time or shortly after. So instead of lateness being propagated, we keep close to schedule. We’re always a little late because we refuse to be early. But we don’t become increasingly late.
Dividing the counts by the actual time difference makes the numbers more accurate. Closer to representing the actual FPS and update frequency. The original basically assumes that it will always be one second even though that’s hardly ever actually true.
Also requires
long lastReset = System.nanoTime();
long nextUpdate = lastReset + UPDATE_INTERVAL;
long nextDraw = lastReset + DRAW_INTERVAL;
long nextReset = lastReset + NANOSECONDS_PER_SECOND;
We need to define lastReset
for use later.
We do not need to use it to set the other variables, but since we have it we might as well.