Notes from Noah’s Ark Game Refactoring

Recently, I mostly finished a fairly large scale refactoring of code for the Noah’s Ark game.  I’m pretty pleased with how it turned out – the code seems simpler, and I think I’ll be able to more rapidly add features.

Part of the refactoring was motivated by some perceived complexity introduced by some architectural ideas I was trying out – namely, “entities”, “components”, and “systems” – that I’d roughly borrowed from other game engine designs that I’ve read about through the years.

Needless to say, those ideas didn’t seem to be working very well for the game I’m working on.  From various things I’ve learned over the last year, it seemed like there were better solutions to things I was dealing with that would allow replacing a lot of complexity with simplicity.

Will the game’s architecture ultimately return to code similar to what it looked like before the refactoring?  Maybe.  But for now, I think the code is in a much better state than it used to be.  Miscellaneous notes from the refactoring are below.

Making main() Meaningful

Originally, I had a very small amount of code in main():

int exitCode = NoahArkGame::RunGame();
return exitCode;

The bulk of the game logic was contained in a NoahArkGame class.  Part of this was a holdover from when I was using Haaf’s Game Engine before switching over to SFML, but even then, it was largely unnecessary.

So I eliminated this NoahArkGame class and basically moved its logic directly into main().  Why?  Well, using this class (along with some others – “state” and “system” classes) resulted in more indirection that was making things a bit harder to understand, force-fitting certain things into a structure that might not actually be appropriate.  Putting the core game logic in main() meant I could more clearly see the connections necessary between different parts of the code and would make it easier to modify as I discovered new things.

Another interesting idea is that many people often describe a program has having a “main loop.”  Well, if you’re someone new to a code base, where would you expect to find the code for the “main loop”?  Without introducing new constructs that someone would have to learn, the first obvious answer that comes to mind is main().  Anything else is introducing a layer of indirection that has to be jumped through.  I should note that I may not 100% agree with this philosophy, but at the very least, there’s nothing inherently bad about putting a lot of code for the Noah’s Ark game directly in main() for now.  Don’t be afraid of main().

One final note – putting logic directly in main() does have the downside of making it harder to unit test (not that I was planning to do so with the NoahArkGame class either).  But a lot of the pieces orchestrated by the main loop (i.e. actual user input, rendering to screen, etc.) are inherently harder to unit test anyway, so trying to “unit test” the could be a near futile effort anyway.  Just a valid trade-off to consider; there may be more effective ways to test the main loop than unit testing too.

Cleaning Up Tile Map Code

I had a ton of code/classes related to dealing with “tile maps” in the game.  Some of this was intentionally duplicated as I was refactoring smaller parts of the game, but even accounting for that, it was still a messy sea of complexity.

I ultimately eliminated several classes – OverworldMap, ScrollableOverworldMap, TileMapBuilder, TileMapObjectLayer, TileMapTileLayer, ITileMapData, ITileMapLayer, OverworldMapSpecification, SurroundingTileMapLoader, TileMapLayers, TileMapLoader, TiledMapJsonFile – that didn’t really have the best abstractions for useful and intuitive “real-world objects” (in the context of the problems of the game) and thus made the code as a whole more difficult to understand.  That left what seems like a more intuitive collection of classes.

A ton of code was also moved/removed from the TileMap class – it either wasn’t needed anymore or seemed better to place elsewhere for now (to avoid making the TileMap class harder to understand).   I should note that the name of the TileMap class may not best fit the abstraction that it represents, but it isn’t quite clear to me yet exactly what the best class name would be.

Loading All Assets At Startup

I was previously loading assets (i.e. tile map/image files) from disk on-demand, only when needed.  That resulted in some jitteryness/slowdowns during gameplay that required additional complexity to solve.

I did some calculations, and it seemed reasonable to fit basically all of my game assets in memory at once (i.e. 50-60 MB of total usage; my “personal limit” is to keep the game under 100 MB of usage).  Furthermore, most of the assets would be needed in memory basically for the entirety of gameplay anyway, so there isn’t going to be much of a reason to try and free up memory for them.

Therefore, I changed things to just load all assets into memory at startup.  This has the downside of taking about 6-8 seconds (it was 12-14 seconds before I added a tiny bit of multi-threading for tile map loading).  But I’m not too concerned about this since I think I can ultimately have most of this go on in the background during an intro sequence that will be added later.  The good news is that the rest of the code generally doesn’t have to worry about asset loading times.

Making Member Variables Public

Another significant change I made was to make most member variables public instead of private.  This is a common “no-no” in encapsulation/object-oriented guidelines, but I’ve been questioning it lately.

First off, nothing is truly “private” – all there really is is data in the computer’s memory.  Various programming languages do somewhat good jobs at trying to protect these private variables from external access, but this is often just a compilation error.  In C++, you can still easily access private members using pointer offsets.  And some languages don’t even properly support “private” member variables – everything is public, and you have to jump through extraordinary hoops to emulate privacy.

So privacy either doesn’t exist in some programming languages and can be subverted using [often bad] techniques in others.

What are the supposed benefits of private members?  Primarily, the intent seems to be to encapsulate/hide implementation details.  There’s certainly some value in that, but just because a piece of data can be thought of as a member of a class, that doesn’t necessarily mean it is an “implementation detail.”  Sometimes, a data member can intuitively be thought of as part of the abstraction presented by a class.  In these cases, it seems to make more sense for that data member to be public – users expect that data to be in the class by virtue of the abstraction it is trying to present.  Trying to hide this and provide public getter/setter methods just results in more code to read/write/maintain and often makes a class harder to use.

Furthermore, most of the “problems” I tend to see in code aren’t actually caused by lack of “private” members – they’re more fundamental design problems.  As code has to get more complex to solve more complex problems, the impact of a public/private member seems to get tinier in the context of these larger design issues.  And if we’re trusting our developers to solve these much more complicated problems, shouldn’t we be trusting them to make responsible decisions when it comes to data access?  In short, the usage of a data member is more of the issue rather than its public/private accessibility.  Good developers should be able to notice code that is potentially accessing a private implementation detail in a negative way and avoid that code entirely.  Rather than treating a symptom of the problem, I’m wondering if perhaps we should focus more on addressing what seems to be the root cause of these problems – mentorship and growth for programmers.  We want programmers to become better people, and not just so that they can craft simple yet exceptional software.

All of that said, I think there is still probably a case for using private members and having public member functions in public API design – you don’t want to cause frustration or unnecessary work for your users if you need to change some implementation detail or data representation.  But for “internal” code that you don’t intend to heavily reuse throughout tons of projects, I question the value of private members.  It’s your code, and you can change it however you see fit.  And if you do need to change something, the compiler (at least in languages like C++) can easily point you to all of the places that need to be changed.

Not Using Classes For Everything

Last but not least, I deleted some classes that were just serving as cruft and making it harder to use/modify code.  In particular, the GraphicsSystem and CollisionSystem classes were eliminated, with the code now existing as free-floating functions in separate files.

I still find [at least what I consider] “object-oriented programming” to be valuable in certain places.  Primarily, allowing a human to work intuitively with “objects” he/she is familiar with is powerful for solving certain kinds of problems.  Classes can also provide a nice home for functionality that can make it easier to find code in a large code base.

However, object-oriented programming certainly is not a silver bullet.  Not every concept is necessarily best represented as an object.  Too many classes can make a code base harder to navigate.  And object-oriented programming can result in too much indirection and structure that could make code harder to understand or change (which can be costly if an object-oriented design is flawed).

Might the free-floating functions eventually end up in a class?  Perhaps.  If I can find a good home for them eventually, that’s something I’d kind of like to do.  But I don’t yet know what that home would be, and right now, free-floating functions makes it easier for me to shape the code into a proper design later.

 

Anyway, that sums up a lot of the major changes I recently made for refactoring code for the Noah’s Ark game (and why).  It’s entirely possible that something I’ve mentioned above is horribly wrong or stupid.  Time will tell, but I think the code is better now than it was before.

Happy coding.

Leave a Reply