It’s been fairly cathartic to go back and clean some of this code up - almost feels like some unfinished business. Still not sure if I want to continue working on it, but I thought I’d at least cull some of the garbage in here to unmuddy the water a bit.
Let’s take away the easy stuff. All those classes that aren’t used or do nothing I’d want to take forward.
First up, the GUI subsytem. It looks like I was building out a GUI system - it wasn’t hooked up to anything except an empty “editor game state” (or the code was commented out). The GUI system can handle bitmap font display, text labels and ‘windows’. I looked at the code and decided to remove it. The whole thing. If I need a GUI in the future (probably), there will be better solutions than this half finished thing - dear ImGui is one such system that I could pretty much drop into place when I need it.
That’s quite a lot of code to remove. Past-me would have kept it around because of that reason - current me thinks YAGNI and points out that it’s still in the git hitsory if I need it (I won’t).
The Entity subsystem had the
DTIClass (from Game Programming Gems 2) and
PropertySet stuff to provide a runtime type information and metaclass system. It was probably here for automating script binding and data loading. Well it’s not used, so it’s going.
I could pretty much delete this stuff straight up without affecting anything. Gone.
I was building some rudimentary node-based path navigation system that’s not hooked up or used. By the looks of it, it wasn’t going to work anyway. Gone.
I had an exception handling system in here that seemed to track the callstack of methods to the site of the exception. Well none were thrown anyway and this looks like it is just going to be problematic. Gone.
I thought I’d tackle the old nemesis, the
MMOPtr combo. The
IMMO system is basically a simple reference counted pointer system that tracks all ‘managed’ objects in a global list (eg: if you inherit the
IMMO base). The reference counter is incremented/decremented by the scoping of the
MMOPtr object. Each frame, we ‘clean’ up the dead list by iterating it and deleting any objects.
There’s a couple of sets of problems with this system. The first is that the
IMMO system itself has a few mechanical flaws:
- global static list of objects
- cleaning all dead on each frame
- no custom allocation strategies (heap or gtfo)
- and more fundamentally, an
IMMOcannot be a stack object
The ‘easy’ route to fixing this would be a blanket find/replace of the IMMO system with something like
However, the second set of problems is common to all smart pointer style systems (including
- easy to create circular reference groups of objects
- easy to disregard object ownership and lifetime concerns
That second point is a huge one. You’ll recognise it when you see public method signatures that pass around
std::shared_ptr for everything. What you’re essentially saying here is “I don’t know who owns this anymore” and as a result, an object’s lifetime becomes less deterministic.
In languages such as C# this is fine as the garbage collector is sophisticated and the memory is managed - C# was designed around this very concept. In C++, and in games specifically, this can become a problem.
The use of the
IMMO system in Manta-X is a huge abuse of this type of pointer. Almost every method signature has an
MMOPtr reference which means that every object in the game has this problem. Therefore a blanket replacement of the
IMMO system with
std::shared_ptr may address some of the mechanical problems with
IMMO, it does not solve the architectural issues of the codebase. Instead, removing IMMO means going through each system, one at a time and making decisions about ownership and lifetime and changing it to reflect this.
So that’s what I did.
Over the course of 10 or so commits, I went through and extricated the IMMO system from the codebase. Here’s the order, from first to last:
- Image Loading
- Texture System
- Model System
- Ship ‘class’ system
- Particle system (I discovered that this whole system was total junk)
- GameData (analogous to UE4’s GameInstance)
- Entity System
- IMMO deleted
What was very clear was that the use of IMMO in the majority of these systems was replaced by holding onto a
std::unique_ptr and doling out a raw pointer to anything that needed it. In the future, this may get replaced in some systems with a handle, but right here, right now, this works.
In removing IMMO, I ended up poking around almost everything in the project. As a result, I’ve decided that the next goal for me is to remove everything in the code that isn’t been exercised by the current running state. So if it’s not part of a live codepath, it’s going.
After this cleanup, cloc is 4168 (down from 5990) and compile times:
Time Elapsed 00:00:56.54
That’s down from
Until next time.