Game loop, is this one good?

Newbie & Debugging Questions
#1

Hi, what do you think about this game loop? The code is taken from a youtube tutorial.
I’ve read a lot of articles about game loop and timesteps (including the thread on this forum: http://www.java-gaming.org/index.php?topic=24220.0), but I don’t understand when the cpu is highly used.
I think this code is good, but i don’t know if the cpu can be stressed using it (i don’t see any Thread.sleep() in the code).
Can you say me if there are important problems?

package engine.test.game;

import java.awt.Canvas;
import java.awt.Dimension;
import java.awt.Graphics;
import java.awt.image.BufferStrategy;
import java.awt.image.BufferedImage;
import java.awt.image.DataBufferInt;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.swing.JFrame;

public class Game extends Canvas implements Runnable
{
    private static final long serialVersionUID = 1L;
    
    public static final int WIDTH = 100;
    public static final int HEIGHT = 60;
    public static final int SCALE = 6;
    public static final String TITLE = "Game";
    
    private Thread thread;
    private JFrame frame;
    private boolean running = false;
    
    private Screen screen;
    
    private BufferedImage image = new BufferedImage(WIDTH, HEIGHT, BufferedImage.TYPE_INT_RGB);
    private int[] pixels = ((DataBufferInt)image.getRaster().getDataBuffer()).getData();
    
    public Game()
    {
        Dimension size = new Dimension(WIDTH * SCALE, HEIGHT * SCALE);
        setPreferredSize(size);
        
        screen = new Screen(WIDTH, HEIGHT);
        
        frame = new JFrame();
    }
    
    public synchronized void start()
    {
        running = true;
        
        thread = new Thread(this, "Game");
        thread.start();
    }
    
    public synchronized void stop()
    {
        running = false;
        
        try {
            thread.join();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
    
    public void run()
    {
        long now = 0;
        long timer = System.currentTimeMillis();
        long startTime = System.nanoTime();
        long lastTime = System.nanoTime();
        long stopTime = System.nanoTime() + 5000000000L;
        double ns = 1000000000 / 60;
        double delta = 0;
        int updates = 0;
        int frames = 0;
        
        // execute for a fixed time, i don't need to execute it more than some minutes
        while (now <= stopTime) {
            now = System.nanoTime();
            delta += (now - lastTime) / ns;
            lastTime = now;
            while (delta >= 1) {
                update();
                updates++;
                delta--;
            }
            render();
            frames++;
            
            if (System.currentTimeMillis() - timer > 1000) {
                timer += 1000;
                System.out.println(updates + " upd, " + frames + " fps");
                frame.setTitle(TITLE + " | " + updates + " upd, " + frames + " fps");
                updates = 0;
                frames = 0;
            }
        }
        
        long endTime = System.nanoTime();
        System.out.println("Duration: " + ((endTime - startTime)/1000000) + "ms");
        
        stop();
    }
    
    public void update()
    {
        
    }
    
    public void render()
    {
        BufferStrategy bs = getBufferStrategy();
        if (null == bs) {
            createBufferStrategy(3);
            
            return;
        }
        
        screen.clear();
        screen.render();
        
        for (int i = 0, l = pixels.length; i < l; i++) {
            pixels[i] = screen.pixels[i];
        }
        
        Graphics g = bs.getDrawGraphics();
        
        g.drawImage(image, 0, 0, getWidth(), getHeight(), null);
        g.dispose();
        bs.show();
    }
    
    public static void main(String[] args)
    {
        Game game = new Game();
        game.frame.setResizable(false);
        game.frame.setTitle(TITLE);
        game.frame.add(game);
        game.frame.pack();
        game.frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        game.frame.setVisible(true);
        game.frame.setLocationRelativeTo(null);
        
        game.start();
    }
}

Thanks in advance and sorry for my bad english (correct me if you note mistake).

#2

It is okay. IMO, you need to clean up the scope of these variables:

long now = 0;
        long timer = System.currentTimeMillis();
        long startTime = System.nanoTime();
        long lastTime = System.nanoTime();
        long stopTime = System.nanoTime() + 5000000000L;
        double ns = 1000000000 / 60;
        double delta = 0;
        int updates = 0;
        int frames = 0;

They are in a really confusing position and it is bad because none of those matter within the actual loop other than lastTime. StartTime and stopTime are getting re-instantiated every pass when they don’t need to be…

Then there’s this…

now = System.nanoTime();

there seems to be alot of redundant variables, but other than that it is a good game loop.

#3

Yes, most of the variables are not meaningful, have been added only for debug. The loop can be simplified to:

public void run()
    {
        long now = 0;
        long timer = System.currentTimeMillis();
        long lastTime = System.nanoTime();
        double ns = 1000000000 / 60;
        double delta = 0;
        
        while (running) {
            now = System.nanoTime();
            delta += (now - lastTime) / ns;
            lastTime = now;
            while (delta >= 1) {
                update();
                delta--;
            }
            render();
        }
        stop();
    }

So, since the loop have no problem, I have another question: does it make sense to call render() all those times?
If the position of the entities are updated 60 times per second why should I call render more than 60 times? (doesn’t it show entities in the same positions?)

Thanks again.

#4

@Nausica: Your post doesn’t make any sense. Nearly all those variables are exactly where they should be. The only one I’d consider moving is [icode]ns[/icode], and that would be made a static constant.

@raxell: Why don’t you just follow the article you linked to?

Firstly, having a loop go for that amount of time is just weird. And if my rushed math is right, that loop will stop after 5 seconds.

Secondly, there is nothing to control the speed of the loop. It’s just going to go as fast as it can, which is pointless if your screen runs at 60Hz, and makes any delta-timing bugs far more severe. Not to mention that people will wonder why a small game is taking up so much CPU time.

Finally, why multithreading?

#5

This isn’t an answer, more of a suggestion. I can see your currently using java2D the default graphics built in with java and such. As I learned two days ago, Java2D sin’t normally used for professional games in java. Instead, I realized to use libraries which saved me hours of confusing coding. libraries include slick2D, LWJGL, LibGDX, JMonkey, and more. This is meant if you are a beginner in game dev, or you don’t know about libraries like I did

#6

Half the variables declared would be better confined in a method somewhere because they will only be called once, and many variables are in fact redundant.

#7

Which ones exactly, are called more than once?

#8

That was just a test, the final version will not stop after 5 seconds.

I forgot to say that this is not a game to be distributed. I’m creating a simulator of a football match that will run on a server (so the user will only see the result of the match, or if he will see the match will be on a browser using javascript).
I only need graphics to see if the movements of the player (so the math and physics) are as expected.
The speed of the game shouldn’t be an issue.

Doesn’t it run on a single thread?

#9

? Never said anything about the variables that are being called more than once. OP’s original code had many redundant variables that were only called once and never used, some were used for the same thing, the delta loop could have been it’s own method, etc. OP cleaned up his code right after my post so the point of this is…?

#10

I apologise for the following flame-like post, but I think a few things need to be cleared up. Don’t take it personally.

(Emphasis mine)

Let’s have a look:


        long now = 0; // Used to keep track of the current time. Used multiple times each frame.
        long timer = System.currentTimeMillis(); // Used to work out when to calculate the FPS. Checked each frame.
        long startTime = System.nanoTime(); // Stores the start of the loop. Checked each frame.
        long lastTime = System.nanoTime(); // Stores the time since the last frame. Used each frame.
        long stopTime = System.nanoTime() + 5000000000L; // Stores when the loop will end. Checked each frame.
        double ns = 1000000000 / 60; // The amount of nanoseconds per frame. Should be a constant.
        double delta = 0; // Stores the delta. Used each frame.
        int updates = 0; // Counts the updates per second. Changed each update, used each second.
        int frames = 0; // Counts the frames per second. Changed each frame, used each second.

I see no redundant variables (apart from ns, which I’ve previously mentioned).

So, you’re saying that you should separate parts of a loop method into a loop method… ???

I see no change to the code, unless you were referring to the removal of debugging variables.