<< Newer Article #154 Older >>

Cleaning House

Every once in a while, you make a small change to the code, and then realize that you really should make a related change to keep things clean. Often, that second change cascades into a third, and a fourth, and then eventually you're making another big core change that everyone loves. All you can do at that point is keep going and hope you can put all the pieces back together again. Thus begins my story of Dr. Frankenstein's Latest Creation, or How I Ripped Apart the Core and Put it Back Together Again.

The change in question this week was making some sense of the ordering of initialization in mame.c. If you look at the code now (before any changes), it's pretty confusing. I even have a giant comment at the top of mame.c trying to explain all the steps and subsystems that are initialized and in what order. It's pretty nuts. And it doesn't have to be that complicated. So I flattened it all out. Now there is one init_machine() function which calls all the other init functions. Along the way, I documented why certain subsystems need to be initialized before other subsystems.

Then I looked at the exiting code with the thought of simplifying that as well.

Except that made even less sense. Only certain systems needed exit code, and the exit routines were not consistently called in the reverse order they were initialized. Aha, I can fix that, I thought. I added a new function add_exit_callback() which queues a function to be called at exit time. The idea is that each init routine can add its own exit callback if necessary. This nicely enforces the exit order to be the exact reverse of the init order (as it should be), and means that each init routine is free to decide whether or not it needs to be called at exit time.

I love consistency, so this appealed to me greatly. Along the way I also made sure that all the init routines were named similarly (subsystem_init()) and that they all returned error codes in a consistent fashion. I did this also the debugger system, and realized that with two debuggers in the source tree, there was no central place that defined the common interfaces. So I created debugger.h which defines the core debugger interfaces. It also added a new DEBUGGER_BREAK macro which you can insert into your code to break into the debugger (either old or new), and which nicely compiles to nothing on a non-debug build. Then I had the distinct pleasure of removing all direct accesses to debug_key_pressed which have accumulated over the years. And yes, I am serious that I found this immensely satisfying.

Along the way through this consistency kick, I noticed that the driver callbacks did not have any sort of consistency. We have DRIVER_INIT, which is called at init time. But was also have VIDEO_START, which is also called at init time. And then we have MACHINE_INIT, which is not called at init time. Furthermore, save state registrations should happen during init time, but more often than not, the most convenient place to put them is in the MACHINE_INIT callback, because that is shared among multiple game drivers.

To fix this required a bit of a re-think, and the addition of several more callbacks. I left DRIVER_INIT as-is. Then I added MACHINE_START, SOUND_START, and VIDEO_START callbacks, which are called right near the end of initialization (I'm still debating calling these MACHINE_INIT, SOUND_INIT and VIDEO_INIT instead, but they are not called at the same time as DRIVER_INIT, so this bothers me a bit). These three callbacks are specified in the MACHINE_DRIVER constructor, and are the ideal place to put save state registrations. The only one of the three that we currently have is VIDEO_START.

I also added three callbacks that are called at reset time. These are MACHINE_RESET, SOUND_RESET, and VIDEO_RESET. Note that MACHINE_RESET is the new name for what is currently MACHINE_INIT. A little search and replace magic made the change across existing files.

I originally thought about having MACHINE_STOP, SOUND_STOP, and VIDEO_STOP as well, but then realized that the corresponding _START routine can just call add_exit_callback to register a stop callback if it was necessary. Even better!

I could have just stopped there, but I noticed that a lot of the video initialization code in mame.c was pushed out to a separate function, and that in fact there was quite a lot of video code lurking in mame.c, as well as some additional bits in common.c. In fact, common.c is a giant pile of random crap that I've been dying to clean out (I already split out the ROM loading code previously into a new file romload.c). So I made a new file video.c and pushed all the video logic into there from mame.c and common.c. The resource management code in common.c really seemed fundamental to the core, so it got moved into mame.c. Most of the remaining common.c functionality is really generic stuff like coin counters and NVRAM that seemed more appropriate as a machine-level file, so I created machine/generic.c and put the rest of it there.

At this point, I was pretty much ready to do anything, so I took another to-do list item from my internal list and renamed driver.c to mamedriv.c. See, driver.c was a bad choice for two reasons. The first is that there is a file driver.h that defines a number of functions, which should be defined in driver.c, except that driver.c was already taken so we put them in mame.c. Now I get to fix that little annoyance. The second reason this is better is that derivative builds (MESS in particular) have their own driver lists. It is more logical to name the driver list source file based on the target you are building. In this case, mame.mak references mamedriv.c.

Once that was sorted out, I took a closer look at driver.h and discovered that there was a giant list of header files included there, most of which made sense, some of which were pretty useless. Since most every file in MAME includes driver.h, it's important to keep this list of headers to a reasonable minimum. While there I finally added state.h as a "standard" include header, and removed the explicit #include "state.h" calls that were floating around in the various source files.

Whew! I'm sure there are a few things I missed describing. I get a bit frenzied when I really start ripping through changes like this. But I'm much happier with the organization of the core and the initialization sequence. And I think I might even know where to put the call to render_init() now, which is the real reason I got started on this kick in the first place....