CHIP-8 delay timer in C++

Question

I'm writing a CHIP-8 emulator in C++, which has two timers. If these timers are non-zero, they are decremented by 1 at 60Hz. I'm still getting the hang of C++, so I wanted to see if I'm doing this the Right Way.

Here's the header file:

#include <chrono>
struct TimedRegister
{
 unsigned char value;
 std::chrono::time_point<std::chrono::high_resolution_clock> last_write;
 TimedRegister();
 void set(const unsigned char new_value);
 void decrement();
};

And this is the the implementation:

#include "chip8.hpp"
#include <chrono>
TimedRegister::TimedRegister()
 : value(0)
{}
void
TimedRegister::set(const unsigned char new_value)
{
 value = new_value;
 last_write = std::chrono::high_resolution_clock::now();
}
void
TimedRegister::decrement()
{
 if (value == 0)
 return;
 const auto now = std::chrono::high_resolution_clock::now();
 if (std::chrono::duration_cast<std::chrono::milliseconds>(now - last_write)
 .count() >= 1000 / 60) {
 --value;
 last_write = now;
 }
}

The decrement function is called from the main emulator loop like this:

while (!chip8.halted) {
 chip8.delay_timer.decrement();
}

They seem to work fine, but I'm not sure if I'm getting away with using a heavy-handed, brute-force approach.

• 12Me21: This is somewhat unrelated, but ideally you should be updating the timers while refreshing the screen, like the original interpreter does.

Answer 1

When doing emulation you should disconnect from the real world and only provide a abstract view to a world.

In other words don't use a real time clock instead simulate the timer by linking it to the emulated clockspeed. For example if you pick 8 MHz you would have 133 333 instructions executed per decrement of each timer.

This will allow you to pause and fast forward when debugging programs.

• supercat: For Chip8, the timer should be cued off display refresh. If memory serves, vintage Chip8 implementations run roughly 1800 cycles (900 two-cycle instructions) between frames, but some operations like draw sprite wait for the next refresh.

Answer 2

Since you keep a note of last_write, you don't need to keep updating the actual value.

Instead, implement a read() overload that computes the value as seen by the emulated system:

unsigned char TimedRegister::read()
{
 if (!value)
 return 0;
 auto now = std::chrono::high_resolution_clock::now();
 auto elapsed = std::chrono::duration_cast<Chip8::Tick>(now - last_write);
 if (elapsed.count() > value)
 // assign to value, so we don't look at clock next time
 return value = 0;
 return value - elapsed.count();
}

I'd consider moving away from std::chrono::high_resolution_clock for this - you really want your own emulated-system clock, which you can advance under program control. That will make both unit-testing and debugging easier.

Answer 3

Some minor observations:

• Make the code easier to read
  Long lines including std::chrono::high_resolution_clock and std::chrono::milliseconds might be a good choice for applying the using directive:
  using high_res_clock = std::chrono::high_resolution_clock;
  You can then use high_res_clock instead of the unwieldy old term.

• Use compiler warnings
  Always enable as many warning flags as possible. That way the compiler can help you eliminate possible errors. In this case you should initialize last_write just like you do with value.

• class VS. struct
  I'm unsure how this ties into the rest of your code but I was taught to use struct for PODs and class if you have methods. Just remember that in classes everything is private by default unlike in structs where it's the opposite.