Chip 8 Emulator

Question

I've recently started learning OpenGL and thought that a great way to start using it in its simplest form would be to create a Chip8 emulator using the keyboard callbacks and graphics that GLFW and OpenGL provide. As this question would be too large to request a review on my classes used with OpenGL and my Chip8 emulator, I'll keep it to just the Chip8.

I'm aware that not all op-codes have been implemented as I have only implemented the ones needed to get the 15 Chip8 games that I have copies of running. The code that I am most interested in having reviewed are the chip8.h and chip8.cpp files however I have included (the majority of) main.cpp for completeness.

An update that I would like to work on would be to switch from using a switch statement to using function pointers but would like a hard review of the current working code before I begin with that.

Thanks to this tutorial which I used to get started and for opcode 0xDXYN.

chip8.h

#ifndef CHIP8_H
#define CHIP8_H

#include <fstream>
#include <iostream>

class Chip8 {
public:
    Chip8(const char* file, int seed);
    ~Chip8();
    void PressKey(int k);
    void ReleaseKey(int k);
    void LoadProgram(const char* file, int seed);
    void Reset();
    void Cycle();
    bool getDraw();
    unsigned char* getGFX();

private:
    unsigned short opcode;
    unsigned char memory[4096];
    unsigned char V[16];
    unsigned short I;
    unsigned short pc;
    unsigned char gfx[64 * 32];
    unsigned char delay_timer;
    unsigned char sound_timer;
    unsigned short stack[16];
    unsigned char sp;
    unsigned char key[16];
    bool drawflag;
};

#endif

chip8.cpp

#include "chip8.h"

Chip8::Chip8(const char* file, int seed) {
    LoadProgram(file, seed);
}

Chip8::~Chip8() {

}

void Chip8::PressKey(int k) {
    key[k] = 1;
}

void Chip8::ReleaseKey(int k) {
    key[k] = 0;
}

void Chip8::LoadProgram(const char* file, int seed) {
    srand(seed);
    Reset();

    std::ifstream f(file,std::ios::binary);
    char c;
    int i = 512;

    while (!f.eof() && i < 4096) {
        f.get(c);
        std::cout << c;
        memory[i++] = c;
    }
}

void Chip8::Reset() {
    pc = 0x200;  // Program counter starts at 0x200
    opcode = 0;      // Reset current opcode    
    I = 0;      // Reset index register
    sp = 0;      // Reset stack pointer

    for (int i = 0; i < 64 * 32; i++) {
        gfx[i] = 0;
    }

    for (int i = 0; i < 16; i++) {
        stack[i] = 0;
        V[i] = 0;
        key[i] = 0;
    }

    for (int i = 0; i < 4096; i++) {
        memory[i] = 0;
    }

    unsigned char chip8_fontset[] = {
        0xF0, 0x90, 0x90, 0x90, 0xF0, // 0
        0x20, 0x60, 0x20, 0x20, 0x70, // 1
        0xF0, 0x10, 0xF0, 0x80, 0xF0, // 2
        0xF0, 0x10, 0xF0, 0x10, 0xF0, // 3
        0x90, 0x90, 0xF0, 0x10, 0x10, // 4
        0xF0, 0x80, 0xF0, 0x10, 0xF0, // 5
        0xF0, 0x80, 0xF0, 0x90, 0xF0, // 6
        0xF0, 0x10, 0x20, 0x40, 0x40, // 7
        0xF0, 0x90, 0xF0, 0x90, 0xF0, // 8
        0xF0, 0x90, 0xF0, 0x10, 0xF0, // 9
        0xF0, 0x90, 0xF0, 0x90, 0x90, // A
        0xE0, 0x90, 0xE0, 0x90, 0xE0, // B
        0xF0, 0x80, 0x80, 0x80, 0xF0, // C
        0xE0, 0x90, 0x90, 0x90, 0xE0, // D
        0xF0, 0x80, 0xF0, 0x80, 0xF0, // E
        0xF0, 0x80, 0xF0, 0x80, 0x80  // F
    };
    for (int i = 0; i < 80; i++) {
        memory[i] = chip8_fontset[i];
    }

    sound_timer = 0;
    delay_timer = 0;

    drawflag = false;
}

void Chip8::Cycle() {
    // Fetch opcode
    opcode = memory[pc] << 8 | memory[pc + 1];

    // Decode opcode
    switch (opcode & 0xF000)
    {
    case 0x0000: // ANNN
        switch (opcode & 0x0FFF) {
        case 0x00E0:
            for (int i = 0; i < 64 * 32; i++) {
                gfx[i] = 0;
            }
            drawflag = true;
            pc += 2;
            break;
        case 0x00EE:
            sp--;
            pc = stack[sp];
            pc += 2;
            break;
        default:
            std::cout << "Unknown opcode: 0x" << std::hex << opcode << std::endl;
        }
        break;
    case 0x1000: // 1NNN
        pc = opcode & 0x0FFF;
        break;
    case 0x2000: // 2NNN
        stack[sp] = pc;
        sp++;
        pc = opcode & 0x0FFF;
        break;
    case 0x3000: // 3XNN
        if (V[(opcode & 0x0F00) >> 8] == (opcode & 0x00FF)) {
            pc += 2;
        }
        pc += 2;
        break;
    case 0x4000: // 4XNN
        if (V[(opcode & 0x0F00) >> 8] != (opcode & 0x00FF)) {
            pc += 2;
        }
        pc += 2;
        break;
    case 0x5000: // 5XY0
        if (V[(opcode & 0x0F00) >> 8] == V[(opcode & 0x00F0) >> 4]) {
            pc += 2;
        }
        pc += 2;
        break;
    case 0x6000: // 6XNN
        V[(opcode & 0x0F00) >> 8] = opcode & 0x00FF;
        pc += 2;
        break;
    case 0x7000: // 7XNN
        V[15] = 0;
        if (V[(opcode & 0x0F00) >> 8] + opcode & 0x00FF <= 0x00FF) {
            V[(opcode & 0x0F00) >> 8] += opcode & 0x00FF;
        }
        else {
            V[(opcode & 0x0F00) >> 8] += opcode & 0x00FF;
            V[15] = 1;
        }
        pc += 2;
        break;
    case 0x8000: // 8XY#
        switch (opcode & 0x000F) {
        case 0x0000: // 8XY0
            V[(opcode & 0x0F00) >> 8] = V[(opcode & 0x00F0) >> 4];
            pc += 2;
            break;
        case 0x0001: // 8XY1
            V[(opcode & 0x0F00) >> 8] |= V[(opcode & 0x00F0) >> 4];
            pc += 2;
            break;
        case 0x0002: // 8XY2
            V[(opcode & 0x0F00) >> 8] &= V[(opcode & 0x00F0) >> 4];
            pc += 2;
            break;
        case 0x0003: // 8XY3
            V[(opcode & 0x0F00) >> 8] ^= V[(opcode & 0x00F0) >> 4];
            pc += 2;
            break;
        case 0x0004: // 8XY4
            V[15] = 0;
            if (V[(opcode & 0x0F00) >> 8] + V[(opcode & 0x00F0) >> 4] <= 0x00FF) {
                V[(opcode & 0x0F00) >> 8] += V[(opcode & 0x00F0) >> 4];
            }
            else {
                V[(opcode & 0x0F00) >> 8] += V[(opcode & 0x00F0) >> 4];
                V[15] = 1;
            }
            pc += 2;
            break;
        case 0x0005: // 8XY5
            V[15] = 0;
            if (V[(opcode & 0x00F0) >> 4] > V[(opcode & 0x0F00) >> 8]) {
                V[(opcode & 0x0F00) >> 8] -= V[(opcode & 0x00F0) >> 4];
            }
            else {
                V[(opcode & 0x0F00) >> 8] -= V[(opcode & 0x00F0) >> 4];
                V[15] = 1;
            }
            pc += 2;
            break;
        case 0x0006: // 8XY6
            V[15] = V[(opcode & 0x0F00) >> 8] & 0x1;
            V[(opcode & 0x0F00) >> 8] >>= 1;
            pc += 2;
            break;
        case 0x0007: // 8XY7
            V[15] = 0;
            if (V[(opcode & 0x00F0) >> 4] > V[(opcode & 0x0F00) >> 8]) {
                V[(opcode & 0x0F00) >> 8] = V[(opcode & 0x00F0) >> 4] - V[(opcode & 0x0F00) >> 8];
            }
            else {
                V[(opcode & 0x0F00) >> 8] = V[(opcode & 0x00F0) >> 4] - V[(opcode & 0x0F00) >> 8];
                V[15] = 1;
            }
            pc += 2;
            break;
        case 0x000E: // 8XYE
            V[15] = (V[(opcode & 0x0F00) >> 8] >> 7) & 0x1;
            V[(opcode & 0x0F00) >> 8] <<= 1;
            pc += 2;
            break;
        default:
            std::cout << "Unknown opcode: 0x" << std::hex << opcode << std::endl;
        }
        break;
    case 0x9000: // 9XY0
        if (V[(opcode & 0x0F00) >> 8] != V[(opcode & 0x00F0) >> 4]) {
            pc += 2;
        }
        pc += 2;
        break;
    case 0xA000: // ANNN
        I = opcode & 0x0FFF;
        pc += 2;
        break;
    case 0xC000: // CXNN
        V[(opcode & 0x0F00) >> 8] = (rand()%255) & (opcode & 0x00FF);
        pc += 2;
        break;
    case 0xD000: // DXYN
    {
        unsigned short x = V[(opcode & 0x0F00) >> 8];
        unsigned short y = V[(opcode & 0x00F0) >> 4];
        unsigned short height = opcode & 0x000F;
        unsigned short pixel;

        V[15] = 0;
        for (int yline = 0; yline < height; yline++)
        {
            pixel = memory[I + yline];
            for (int xline = 0; xline < 8; xline++)
            {
                if ((pixel & (0x80 >> xline)) != 0)
                {
                    if (gfx[(x + xline + ((y + yline) * 64))] == 1)
                        V[15] = 1;
                    gfx[x + xline + ((y + yline) * 64)] ^= 1;
                }
            }
        }
        drawflag = true;
        pc += 2;
    }
        break;
    case 0xE000: // EX##
        switch (opcode & 0x00FF) {
        case 0x009E: // EX9E
            if (key[V[(opcode & 0x0F00) >> 8]] == 1) {
                pc += 2;
            }
            pc += 2;
            break;
        case 0x00A1: // EXA1
            if (key[V[(opcode & 0x0F00) >> 8]] != 1) {
                pc += 2;
            }
            pc += 2;
            break;
        default:
            std::cout << "Unknown opcode: 0x" << std::hex << opcode << std::endl;
        }
        break;
    case 0xF000: // FXNN
        switch (opcode & 0x00FF) {
        case 0x000A: // FX0A
            for (int i = 0; i < 16; i++) {
                if (key[i]) {
                    V[(opcode & 0x0F00) >> 8] = i;
                    pc += 2;
                    break;
                }
            }
            break;
        case 0x0007: // FX07
            V[(opcode & 0x0F00) >> 8] = delay_timer;
            pc += 2;
            break;
        case 0x0015: // FX15
            delay_timer = V[(opcode & 0x0F00) >> 8];
            pc += 2;
            break;
        case 0x0018: // FX15
            sound_timer = V[(opcode & 0x0F00) >> 8];
            pc += 2;
            break;
        case 0x001E: // FX1E
            V[15] = 0;
            if (I + V[(opcode & 0x0F00) >> 8] < 0x0FFF) {
                I += V[(opcode & 0x0F00) >> 8];
            }
            else {
                I += V[(opcode & 0x0F00) >> 8];
                V[15] = 1;
            }
            pc += 2;
            break;
        case 0x0029: // FX29
            I = V[(opcode & 0x0F00) >> 8] * 0x5;
            pc += 2;
            break;
        case 0x0033: // FX33
            memory[I] = V[(opcode & 0x0F00) >> 8] / 100;
            memory[I + 1] = (V[(opcode & 0x0F00) >> 8] /10) % 10;
            memory[I + 2] = V[(opcode & 0x0F00) >> 8] % 10;
            pc += 2;
            break;
        case 0x0055: // FX55
            for (int i = 0; i <= ((opcode & 0x0F00) >> 8); i++) {
                memory[I + i] = V[i];
            }
            pc += 2;
            break;
        case 0x0065: // FX65
            for (int i = 0; i <= ((opcode & 0x0F00) >> 8); i++) {
                V[i] = memory[I + i];
            }
            pc += 2;
            break;
        default:
            std::cout << "Unknown opcode: 0x" << std::hex << opcode << std::endl;
        }
        break;
    default:
        std::cout << "Unknown opcode: 0x" << std::hex << opcode << std::endl;
    }

    // Update timers
    if (delay_timer > 0)
        delay_timer--;

    if (sound_timer > 0)
    {
        if (sound_timer == 1)
            std::cout << "Beep!" << std::endl;
        sound_timer--;
    }
}

bool Chip8::getDraw() {
    return drawflag;
}

unsigned char* Chip8::getGFX() {
    drawflag = false;
    return gfx;
}

main.cpp

// GLEW
#define GLEW_STATIC
#include <GL/glew.h>

// GLFW
#include <GLFW/glfw3.h>

// Other includes
#include "shader.h"
#include "display.h"
#include "mesh.h"
#include "chip8.h"
#include <iostream>

// Consts
const GLuint WIDTH = 300, HEIGHT = 150;

// Function prototypes
void key_callback(GLFWwindow* window, int key, int scancode, int action, int mode);
void mouse_callback(GLFWwindow* window, int button, int action, int modifier);

Chip8 chip8("./PONG.c8", 1);

int main()
{
    // Window Creation is here

    // Game loop
    while (!display.IsClosed())
    {
        chip8.Cycle();
        if (chip8.getDraw() == true) {
            unsigned char* gfx = chip8.getGFX();

            // Drawing code is here

            display.Update(true, true);
        }
        else {
            display.Update(false, true);
        }
    }

    return 0;
}

void key_callback(GLFWwindow* window, int key, int scancode, int action, int mode)
{
    if ((key == GLFW_KEY_ESCAPE || key == GLFW_KEY_ENTER) && action == GLFW_PRESS){
        glfwSetWindowShouldClose(window, GL_TRUE);
    }

    int k = -1;
    if (key == GLFW_KEY_1) {
        k = 0x1;
    }
    else if (key == GLFW_KEY_2) {
        k = 0x2;
    }
    else if (key == GLFW_KEY_3) {
        k = 0x3;
    }
    else if (key == GLFW_KEY_4) {
        k = 0xc;
    }
    else if (key == GLFW_KEY_Q) {
        k = 0x4;
    }
    else if (key == GLFW_KEY_W) {
        k = 0x5;
    }
    else if (key == GLFW_KEY_E) {
        k = 0x6;
    }
    else if (key == GLFW_KEY_R) {
        k = 0xd;
    }
    else if (key == GLFW_KEY_A) {
        k = 0x7;
    }
    else if (key == GLFW_KEY_S) {
        k = 0x8;
    }
    else if (key == GLFW_KEY_D) {
        k = 0x9;
    }
    else if (key == GLFW_KEY_F) {
        k = 0xe;
    }
    else if (key == GLFW_KEY_Z) {
        k = 0xa;
    }
    else if (key == GLFW_KEY_X) {
        k = 0x0;
    }
    else if (key == GLFW_KEY_C) {
        k = 0xb;
    }
    else if (key == GLFW_KEY_V) {
        k = 0xf;
    }
    if (k > -1) {
        if (action == GLFW_PRESS) {
            chip8.PressKey(k);
        }
        if (action == GLFW_RELEASE) {
            chip8.ReleaseKey(k);
        }
    }
}

void mouse_callback(GLFWwindow* window, int button, int action, int modifier)
{

}

Answer 1

A few suggestions (well, a lot — not that it looks bad at all) from looking at your code without running it yet:

You should, if the headers are available (they might not be on a platform like Arduino, but at least some of them will), take advantage of the following modern features:

• Use fixed-width integer types (in particular std::uint16_t instead of unsigned short and std::uint8_t instead of unsigned char). These are found in header cstdint or cintttypes. If those headers aren't available, the C versions are stdint.h and inttypes.h, and if those aren't available, you can use this implementation.
• Use std::array (header array) instead of plain C arrays, which, with basically no overhead, provide many useful methods, plus, don't decay to pointers, so you can have signatures like array<64 * 32, uint8_t> &getGFX().
• Consider using std::string or, even better, the no-overhead, soon-to-be standardized std::experimental::string_view/std::string_view in place of const char *. Header string for the former, experimental/string_view (C++11/14) or string_view (C++1z) for the latter.

• If you use standard library collections like array, string, string_view and so on, you can use ranged for loop syntax in place of C-style for loops. This is similar to Python's for item in items syntax in that it returns the items in order without having to create a variable for the index (thus avoiding visual noise and off-by-one errors).

  For a for loop that mutates the members of a collection, you would write for (auto &&item : items) {/* code */} to get a mutable reference to the collection member being iterated over called item on each iteration.[1]

  Some of your loops can be avoided by using modern features; for example, if memory were a array<uint8_t, 4096> (or any standard library collection), then setting it to all zeroes would be as easy as memory = {}

• Variables which are constants where the value is known at compile-time (i.e. they're initialized with a literal, or set to a constant that was initialized with a literal, etc) should be declared constexpr. Functions often can (and should) also be declared constexpr if they conform to certain restrictions, which tells the compiler to evaluate them at compile time if possible.

• Regarding your idea of refactoring your code to use function pointers, I'd recommend std::function (header functional) and/or C++11 lambda expressions instead. These offer a lot of additional functionality on top of C function pointers, but more importantly do away with the insane unreadable syntax.

General pointers:

• You should follow the C++ best practice known as "const correctness," which means that every variable and every function parameter that can be declared const should be declared const (and, if possible, preferring constexpr to const as well).

  For example, T foo should be declared constexpr T foo if possible, or, failing that, const T foo. For more complex examples see [2]

• Headers should be #include-ed in the file they're actually used, so fstream and iostream should be included in your cpp file, but not in your header file.

• Don't use the poorly-named std::endl with std::cout; it improves both readability and performance to simply do << '\n'.[3]

• Singleton classes are an antipattern that's a bit of a pet peeve for me. Instead, don't use a class, declare the "public" functions (and variables) in your header file, and declare/define the "private" variables (and functions) in your cpp file, wrapped in an anonymous namespace, like so:

// foo.h
int foo(int bar); // foo is accessible from every file foo.h is #include-ed in
// foo.cpp
#include "foo.h"
int foo(int bar) {return bar;}
namespace {
        int baz(int blah); // baz can only be accessed within foo.cpp
        // ...
        int baz(int blah) {return blah;}
}

• For consistency, use a switch statement in key_callback. (My personal style preference is in general to use switch statements instead of a long if-else chain all reading from the same variable).

---

[1] For a loop that only reads from without writing to a collection, there's:

for (auto item : items) {/* code */} to get a temporary copy of the collection member being iterated over (the copy is mutable but changing it doesn't affect the collection member it was copied from, for an immutable copy you could use const auto), and

for (const auto &item : items) {/* code */} to get an immutable reference to the current collection member.

Performance-wise the rule of thumb is that the latter is more efficient iterating over objects which require dynamic allocations (anything that uses new in the constructor) while the latter are more efficient iterating over collections made up of everything else, primitive types (integers, chars, floats/doubles, pointers), and structs/classes that don't change size.

In everything but the most performance-sensitive contexts it's probably millionths of a second in execution time either way, but I figure I should explain things thoroughly. Apologies if this is confusing; just pick one and you'll be fine.

auto is a modern C++ feature that infers the type of a declaration from context, which can be helpful for long and hard-to-remember type names, writing generic code with templates, or reducing the number of places in the code you need to change when you change a type declaration.

If item were an int, the above loops could also have been written int &&item, int item and const int &item.

Another helpful C++ type inference feature is decltype():

a_very_long_namespace::a_very_long_class<a_very_long_type> foo;
decltype(foo) bar;

[2] Prefer constexpr const T *foo to const T *const foo to T *foo

Prefer constexpr const T *const *foo to const T *const *const foo to T **foo

As you can see, constexpr makes the named part of the declaration const

For functions:

Prefer constexpr T func(const U foo, const V bar) to T func(U foo, V bar)

The one exception to const-correctness is that there's no point (unless forced to by the type system) in making function return values const, so a declaration like constexpr const T func() is going overboard. (constexpr refers to func() here and in the above example, not to T).

For methods:

Prefer constexpr T meth(const U foo, const V bar) const to T meth(U foo, V bar)

The const after the method signature guarantees that the method can't modify the object it was called on, nor does it return any kind of mutable reference to that object.

[3] endl appends a newline to and flushes an output buffer, but cout does this expensive operation automatically.