Hacker Newsnew | past | comments | ask | show | jobs | submitlogin
Emudore: a C64 emulator written from scratch using C++11 and SDL2 (github.com/marioballano)
112 points by okket on April 19, 2016 | hide | past | favorite | 18 comments


This is a cool project with nice, simple code. However, the submission's title (perhaps counter to the README) makes it seem like a good example of C++11, and I would caution against using it that way. From a cursory examination, and with all due respect to the author, this code is more classical C++ with a few uses of C++11 features than it is representative of how modern C++ (at the time of C++11's publication) should be written. There are a few areas where the code could be updated: adding tests, avoiding manual memory management, making it so objects cannot be created in invalid states that would cause crashes, const correctness, etc.

By way of explanation, I'll focus on just the issue of manual memory management. Simplifying a bit, consider this part of the C64 class's implementation:

    C64::C64()
    {
      cpu_  = new Cpu();
      mem_  = new Memory();
      cia1_ = new Cia1();
      ...

      cpu_->memory(mem_);
      cpu_->reset();
      ...

      cia1_->cpu(cpu_);
      ...

      mem_->cia1(cia1_);
      ...
    }

    C64::~C64()
    {
      delete cpu_;
      delete mem_;
      delete cia1_;
      ...
    }
The contract in this code is that a C64 owns the chips, the chips are born when the C64 is instantiated, and when the C64 dies, the chips all die along with it. There are two approaches for making this contract explicit, one is to simply not use pointers and references at all:

    Cpu cpu_;
    Memory memory_;
    Cia1 cia1_;
But this does mean we can't use polymorphism within the C64 class, we don't have nullability, and we can't replace the instances. The code doesn't leverage any of these, so that may be an appropriate choice. However, if we need any of those things, we can instead use the appropriate smart pointer:

    std::unique_ptr<Cpu> cpu_;
    std::unique_ptr<Memory> memory_;
    std::unique_ptr<Cia1> cia1_;
In either case, C64's destructor is no longer necessary. The other classes still take raw pointers (e.g., `Cpu*`) because they do not own the objects they receive and do not dictate lifetimes.

It's worth mentioning that many people would use `std::shared_ptr` here instead, but I think that's abuse that leads you towards circular references (memory leaks): the Cpu doesn't own its Memory, they just have their lifetimes managed exterior to them. This would actually come up with the existing design (if shared ownership were used), since the object dependency graph (which is perhaps another issue) has several circular dependencies.


On this topic, I'd like to get the advice of some HN C++ gurus.

In my own emulator (higan), I have a namespace for each emulated system, and each component is its own unique object. Example:

    namespace GameBoyAdvance {
      CPU cpu;
      PPU ppu;
      Cartridge cartridge;
      ...
    }
The obvious downside is that they aren't instantiable. You can only have one instance per process. Generally this isn't a problem for an emulator, but it's still something I don't like about my code.

The obvious "proper" way would be something like:

    class GameBoyAdvance {
      CPU cpu;
      PPU ppu;
      ...
    };
But there's a reason I don't do this: all of the chips need to communicate with one another. So PPU::scanline() may call CPU::raiseInterrupt(Interrupt::VerticalBlank) for example.

So you think something like:

    class GameBoyAdvance {
      CPU cpu{this};  //hooray C++11, this would be
      PPU ppu{this};  //much nastier in C++98
      ...
    };
And now your code goes from this:

    void PPU::scanline() {
      cpu.raiseInterrupt(Interrupt::VerticalBlank);
    }
To this:

    void PPU::scanline() {
      gameBoyAdvance->cpu.raiseInterrupt(Interrupt::VerticalBlank);
    }
Which aside from being a lot of extra typing (seriously, the chips in emulated hardware communicate all the time, all over the place), is also quite a lot slower.

The former can basically get inlined 100%. The compiler knows the exact address of the CPU object, because there's only one. The latter has to go through a pointer to get the address. Even though it's always a constant under actual emulation, the code basically has to account for the fact that you could change the pointer at any time. Even if you tried binding it as a reference, eg CPU(GameBoyAdvance&), the performance penalty remains.

Long ago, I wrote some #define wrappers so I could say "cpu().raiseInterrupt", and depending on compilation flags, it would use CPU cpu; or CPU* cpu; and the results were dramatic: at least a 15% loss of performance just by using the pointer version.

I'd really like the benefits of the latter design, but without the added red tape and performance penalties. But I can't think of any way to pull it off. I really can't afford the performance penalties, as I use 100% of top-end CPUs to try to emulate every hardware nuance possible already. Am I doing the right thing, or is there a better way?


It's a bit horrible and unsafe, but you can use offsetof and some nasty pointer casting to get from a member of a class back to the class. I've done this, and it compiles to extremely efficient code. You can even template a class on the value of offsetof, so you can use the same class as multiple members.

I'm on my phone, so no code examples (sorry). If you want one, reply and I'll write one when I get to a computer!


The GameBoyAdvance class could have a constructor which more directly connects together the pieces. The PPU doesn't care that it's inside a GameBoyAdvance: it has a connection to the CPU to raise an interrupt:

   // Don't throw away that simple old C++98 so fast.
   class GameBoyAdvance {
   private:
     CPU cpu;
     PPU ppu;
   public:
     GameBoyAdvance()
     : cpu(ppu), ppu(cpu) // CPU has ref to PPU and vice versa.
     {
     }
So now:

     void PPU::scanline() 
     {
       cpu.raiseInterrupt(Interrupt::VerticalBlank);
     }
Gee, exactly like before, with the namespace. Only now cpu is a member reference to a CPU, inside the PPU object.


Design Purists Hate Him. Or - How I learned to stop worrying and love singletons. Lose software bloat with this one weird tip. etc.

You know your software better than anyone and you clearly know enough about the design tradeoffs involved. Just try to contain your solution to as few classes as possible.


Slightly off-topic, but could you recommend any books/articles/talks/resources on modern C++? I learnt the language in the early 2000s and haven't used it in years. If I picked up a C++ project today I would write code exactly like the above.


Besides adrianN's suggestion, Tour of C++ from Bjarne.


Effective Modern C++.


You are nearly completely right.

The one good thing about the new/delete approach is that it keeps the master object small, since it's just a farm of pointers. No matter how you define a C64 instance, most of the storage comes from the heap.

I once ran into an issue where declaring a class on the stack was actually crashing the program (running in a small-ish embedded system). The quick and dirty fix was to new/delete the entire object. But when that is out in the open, you expose potential leaks; you need a smart pointer to manage that. Making some of the contents dynamic (so that the class is effectively a smart pointer) is another option.


There is really not a shortage of open source high quality C64 emulators. Does this one bring anything unique to the table?


From a user's perspective, it sounds like no:

"many of the more advanced games written in ML [machine language] do not yet play well due to unimplemented hardware features, writing an emulator is a tough task and after all my goal wasn't to write a full perfect emulator but to learn in the process of making a simple one"

The parent is getting downvotes, but I think it's a reasonable question: is this a program I should appreciate as an amibitious project and potential learning tool, or one I would actually want to use on a regular basis?


from the readme:

"I don't think anybody would ever dare to use this for an actual useful purpose, but just in case, the project is licensed under the Apache 2.0 license"

"Long story short: to learn a bit more about computer architecture, graphics, C++, etc.. while having some fun!"

"Due to some of the aforementioned facts, expect things to fail,"

It seems that the readme pretty much states that this is a WIP for learning, and shouldn't be used for anything.


Radare2 support is actually quite neat and useful. The monitor/debugger in vice is great, but reading and writing memory from external applications is something that I have been looking forward to.

Even without it, this kind of bare minimum emulation projects are always interesting to me, because the scope and size makes it more approachable.


If I were learning C++11 and/or SDL2, I'd be really interested. Or if I were versed in C++11 and wanted to learn more about how the C64 worked. Sometimes you're more in the mood to read/experiment with code than to read technical docs.


"This is my [emulator]. There are many like it, but this one is mine."


If you were asking if you should adopt it as a drop in replacement for your current emulator: "This project is much younger than it's predecessors I would assume it is less developed."

If you were asking why it should be displayed on this forum: "This project shows a high level of technical competency and I believe that is why we as a community are showing it support."


Speaking of C64, check out this entry which was submitted yesterday:

https://news.ycombinator.com/item?id=11521609 ('The 64' – A modern C64 console and handheld project)


Run it through emscripten, put it on the Archive for C64 games like they have for dos games?




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: