15
\$\begingroup\$

I designed a "Ticker" based on how I imagine Minecraft's tick system works. My ticker calls a method tickRate times a second and provides a Delta Time value (deltaTime) to the method being called.

The issue I have with my code is that I find the update method (which handles timing) to be ugly but I'm not sure what I can do.

My ticker class is designed to be instantiated and used whenever, it does not have to be used right away (which is why I don't set any time related information in the constructor). This is why there is a check for nextTick and lastTick being 0 inside update, as to ensure that the delta time does not go crazy on the first tick. The issue with this is that it looks a bit ugly, however I'm not sure what I could do. I could move the reset code into the reset method, however that would now mean that the reset method is time dependant, and must be only be called when you intend to use the ticker.

Ticker.java

import java.util.HashSet;
import java.util.Set;

public class Ticker {
    private long lastTick;
    private long nextTick;
    private int tickRate;

    private Set<TickListener> tickListeners = new HashSet<>();

    public Ticker(int tickRate) {
        this.tickRate = tickRate;
    }

    public void addTickListener(TickListener listener) {
        tickListeners.add(listener);
    }

    public void removeTickListener(TickListener listener) {
        tickListeners.remove(listener);
    }

    public void setTickRate(int tickRate) {
        this.tickRate = tickRate;
    }

    public int getTickRate() {
        return tickRate;
    }

    public void reset() {
        lastTick = 0;
        nextTick = 0;
    }

    public boolean update() {
        long currentTime = System.currentTimeMillis();

        if (currentTime >= nextTick) {
            long targetTimeDelta = 1000L / tickRate;

            if (lastTick == 0 || nextTick == 0) {
                lastTick = currentTime - targetTimeDelta;
                nextTick = currentTime;
            }

            float deltaTime = (float) (currentTime - lastTick) / targetTimeDelta;

            for (TickListener listener : tickListeners) {
                listener.onTick(deltaTime);
            }

            lastTick = currentTime;
            nextTick = currentTime + targetTimeDelta;

            return true;
        }

        return false;
    }
}

TickListener.java

public interface TickListener {
    void onTick(float deltaTime);
}

Example usage

Ticker ticker = new Ticker(20); // 20 ticks per second

ticker.addTickListener(new TickListener() {

    @Override
    public void onTick(float deltaTime) {
        System.out.println(String.format("Ticked with deltaTime %f", deltaTime));
    }
});

while (true) {
    ticker.update();
}
\$\endgroup\$
  • \$\begingroup\$ Is TickListener a custom class? If so, I'm sure reviewers would appreciate having it so they could more easily understand your code. :) \$\endgroup\$ – Ethan Bierlein Nov 26 '15 at 1:11
  • \$\begingroup\$ TickListener is simply an interface with a method called onTick. I'll add it to the OP now. \$\endgroup\$ – Jack Wilsdon Nov 26 '15 at 1:36
  • \$\begingroup\$ You may want to consider stackoverflow.com/questions/351565/… \$\endgroup\$ – WW. Nov 26 '15 at 5:05
  • 1
    \$\begingroup\$ I would prefer to see you abstract System.currentTimeMillis() to some kind of external interface (for example, a Clock interface) - this would allow you to move backward and forward through time in unit tests without having to actually wait that many milliseconds? \$\endgroup\$ – Dan Pantry Nov 26 '15 at 9:03
6
\$\begingroup\$

This is one of the cleanest code I have come across in a while. Congrats to your lovely, well-written code!

Just some points:

  1. I don't like seeing this:

    public class Ticker {
        private long lastTick;
        // ...
        }
    }
    

    It is more readable (IMO) when you put spaces after the beginning and before the end:

    public class Ticker {
    
        private long lastTick;
        // ...
        }
    
    }
    

    I don't see a reason why not to do so, as the rest of your code is very well spaced.

  2. This method:

    public boolean update() {
        long currentTime = System.currentTimeMillis();
    
        if (currentTime >= nextTick) {
            long targetTimeDelta = 1000L / tickRate;
    
            if (lastTick == 0 || nextTick == 0) {
                lastTick = currentTime - targetTimeDelta;
                nextTick = currentTime;
            }
    
            float deltaTime = (float) (currentTime - lastTick) / targetTimeDelta;
    
            for (TickListener listener : tickListeners) {
                listener.onTick(deltaTime);
            }
    
            lastTick = currentTime;
            nextTick = currentTime + targetTimeDelta;
    
            return true;
        }
    
        return false;
    }
    

    Just my opinion, but I feel uncomfortable with that many spaces in code. The only place I would use spaces would be:

    • Between Node declarations in JavaFX, or JWhatever declarations in Swing.
    • Where there is an obvious change in what the code will do.

    So it would be more like:

    public boolean update() {
        long currentTime = System.currentTimeMillis();
        if (currentTime >= nextTick) {
            long targetTimeDelta = 1000L / tickRate;
            if (lastTick == 0 || nextTick == 0) {
                lastTick = currentTime - targetTimeDelta;
                nextTick = currentTime;
            }
    
            float deltaTime = (float) (currentTime - lastTick) / targetTimeDelta;
            for (TickListener listener : tickListeners) {
                listener.onTick(deltaTime);
            }
    
            lastTick = currentTime;
            nextTick = currentTime + targetTimeDelta;
            return true;
        }
        return false;
    }
    
\$\endgroup\$
  • 2
    \$\begingroup\$ For #2: You say to remove white space that shows that blocks of statements do related but not identical things… why? \$\endgroup\$ – Nic Hartley Nov 26 '15 at 4:41
  • 8
    \$\begingroup\$ I prefer the original layout. It's a matter of taste, but it is certainly easier for me to read. \$\endgroup\$ – user5151179 Nov 26 '15 at 6:20
6
\$\begingroup\$

Your code is good, but if you care about any chance of multiple ticks per frame, you may want to use a while loop to keep ticking until you're up to speed. This is because System.currentTimeMillis() and System.nanoTime() are 'timers', rather than 'countdowns' - they only measure time.

So, it's entirely possible something will go wrong or otherwise be slow and only one update will be called per 3 or 4 seconds, for example. This is where you could use a while loop:

public boolean update() {
    long currentTime = System.currentTimeMillis();
    long targetDelta = 1000L / tickRate;
    ....

    boolean flag = false;
    while(currentTime >= lastTick + targetDelta) {
        for(TickListener listener : tickListeners) {
            listener.onTick(deltaTime);
        }
        lastTick += targetDelta;
        flag = true;
    }
    return flag;
}

I might also suggest using nanoTime() instead of currentTimeMIllis(), but this is up to you.

\$\endgroup\$
  • \$\begingroup\$ I designed the class to be non-blocking, so that you actually call the update method in a while loop. I should have added an example to my original post. \$\endgroup\$ – Jack Wilsdon Nov 26 '15 at 16:58
-1
\$\begingroup\$

An Example how I do it in my Projects when I need it, is that I use a new Thread and let it have handle the needed Stuff.

TickHandler.java

package me.donjoe.foo;

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

public class TickHandler extends Thread {

    private int tickRate = 60;
    private List<TickHandler> q = new ArrayList<>();

    @Override
    public void run() {
        try {
            // Devide 1000 Millisecond (1 Second) through the tickrate
            // to get the time every tick has
            long timeout = 1000L / this.tickRate;
            // Buffer-Time which carries over the Time that the last tick took too long.
            // Used to make the Ticks in-sync again.
            long buffer = 0L;
            while (true) {
                // Check if Interrupted. If so, stop this Thread
                if (this.isInterrupted()) throw new InterruptedException();
                // Start Time
                long startTime = System.currentTimeMillis();

                // TODO: Do some Stuff here
                // Handle the TickHandler / Callbacks
                for (TickHandler handler : this.q) handler.handle();
                // Maybe clear the Q, depends on how you like to work with it
                // this.q.clear();

                // Time to wait in this Tick
                long toWait = (timeout - buffer) - (System.currentTimeMillis() - start);

                // When the Time is negative, the Tick took too long to execute.
                // Set the buffer to the time it took too long and let is balance out.
                if (toWait < 0) buffer = -toWait;
                else {
                    this.wait(toWait);
                    // Clear the buffer. No time to wait on the next one
                    buffer = 0L;
                }
            }
        } catch (InterruptedException e) {
            // The Thread got stopped. Do stuff here
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ The issue with this now is thread safety. If I do anything outside of the ticker thread, you can get race conditions which I'd rather avoid. \$\endgroup\$ – Jack Wilsdon Nov 26 '15 at 18:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.