Design priorities (functionality, clarity, and performance, in that order)

Civilization 5 is a very complex game. From the intricacies of its rules to complexities of its procedural generation of maps, the game is filled with an enormous amount of intricately-balanced systems all working in harmony to create the experience. While performance is always important, even just getting the game to work properly was quite a challenge.

This informed the priorities I had for development. First and foremost I wanted Civ Clone to work, to do some acceptable percentage of what Civilization 5 is capable of, no matter how I could manage it. Following closely behind that was clarity: I wanted to make it as straightforward as possible to figure out what was going on, what was and wasn't working, and what needed to be done. The simulation was so complex that very careful organization was required to keep track of everything without getting overwhelmed.

In any game, performance is a huge priority. But since I'd already bitten off an incredibly large project for myself, I opted to address performance as a more distant third priority. When it made it difficult or impossible to play the game I addressed it as best I could, but overall I felt it was more important to make the simulation complete than to make it run smoothly, especially since the machine I was developing on can't even run the well-optimized Civilization 5 particularly smoothly.

Thus the performance optimization is probably the least developed part of the entire game, besides the AI. I did spend a decent chunk of time making map generation and rendering more efficient, but there was much work in that area left unfinished. If I were to continue development I'd likely skew more towards optimization and bugfixing now that most of the features are in place.

One thing I did not prioritize in the slightest was brevity. Names are long, classes and interfaces are numerous, methods are filled with white space and intermediate variables to improve comprehension, and even relatively simple calculations are sometimes given their own methods or classes to reduce the profile of methods and the responsibilities of classes. As might be indicated by the amount of documentation for this project, I prefer to over-explain and leave a programmer informed but bored than under-explain and leave them confused and frustrated.

"Canon" classes

From the outset, it was clear that the design would involve numerous relationships between various objects: Cities owning cells, civs owning cities, units owned by civilizations and occupying cells, and so on. Having struggled with state synchronization in the past, I wanted to make sure that, for every relationship between objects, there was exactly one class and exactly one object that keeps track of it. That way there would be no strange bugs where two objects disagreed about the world around them. This was especially important because I needed to make a variety of queries. For instance, I might need to know what Units a HexCell had, but I also might need to know the HexCell a Unit occupies. Delegating the relationship to either one of those classes would make life more difficult for the other.

Since there were many classes of this type, and they'd be defining important relationships between objects, I wanted to develop a naming convention that immediately identified these classes as maintaining relational information, and also as entities whose methods had important side effects. The convention I used was to suffix all classes of this type with the word "Canon." Thus a class like CityPossessionCanon is the sole authority on which civilizations own which cities. UnitPositionCanon is the only class that determines where units are on the map. SocialPolicyCanon handles all locking/unlocking of social policies by civilizations.

There are some substantial consequences from this decision. It defines important properties of important game entities in places other than their implementing object. This makes it more difficult to determine what sorts of relationships one object might possess with others, since those relationships aren't managed directly by the object itself. It also means Canon classes need to listen for object destruction so they can remove records that no longer apply. Having to reference specific -Canon classes increases the number of dependencies for consuming classes. For instance, if a class wants needs a unit's location and owner, it requires IUnitPositionCanon and IUnitPossessionCanon as dependencies.

Overall I've found the -Canon construction a useful one because it reduces the potential for bugs and makes certain queries much simpler. Whether the naming convention itself is useful isn't clear, and it might need to be changed if the nomenclature proves confusing to others. "Canon" isn't exactly a word that passes through normal conversation frequently.

"Logic" classes

During development, I found myself creating a large number of classes whose primary function was to compute some useful or important value for which the calculation was complex (most of the time, at least). Since the -Canon construction was already in place, I thought it useful to distinguish between classes that managed important state and those that did not. I figured such a convention would make it easier to track down bugs related to inconsistent state: if a unit was in the wrong place, one would know immediately it wasn't a calculation class that put it there.

I decided to adopt a naming convention to distinguish these calculation classes from the rest of the codebase. I decided to suffix the class names with the word "Logic" (UnitFortificationLogic, YieldGenerationLogic, GridPartitionLogic) to indicate they were performing calculations rather than maintaining state.

Months after the decision had been made and dozens of -Logic classes had been created, I realized that "Logic" is an incredibly generic term that applies to essentially all computational operations. Calling something a -Logic class doesn't inherently provide any information. But by the time I realized that the convention was already stuck in my head and it seemed unhelpful to change it. So while I like the general idea behind this naming convention, a much better suffix should've been chosen.

"Responder" classes

For reasons of decoupling and dependency management, I found myself creating a large number of signals that fired on various important game events (units enter and leaving hexes, cities changing owner, improvements being pillaged). As the number of consumers for these signals grew, I began to notice certain classes whose sole purpose was to respond to a number of these signals and perform important tasks. I felt that these classes were important to keep in mind because they represented behavior that corresponded with methods in other sections of the codebase but had no direct ties to them. In other words: If I want to know the entirety of what constructing a city does to the simulation, how do I figure that out?

I started to use another naming convention to flag a class whose sole purpose was to react to signals firing. I began to suffix them with the word "Responder" (CityConquestResponder, CivDiscoveryResponder, FreeGoldenAgeResponder). This concerns only classes that consume events: classes that produce them aren't tagged with this suffix.

Of the naming conventions I used, this one was applied the least consistently and was perhaps the least helpful. I don't know if the design of these -Responder classes is inherently bad since I don't know enough about Reactive Design to have a solid intuition on its use. Probably classes of this type make some sense, but they should be focused around very specific actions taken (FreeGoldenAgeResponder) rather than around a collection of signals from a single source (UnitResponder). Even more unclear is whether the naming convention (regardless of the suffix used) is useful. Reactive code felt noteworthy because it was a new design paradigm I was experimenting with, but that doesn't mean it ought to be treated differently from other classes in a production environment.

Configuration via ScriptableObjects

Games typically involve a lot of data, not just models and textures and audio files, but also tons of configuration information, especially for system-heavy ones. Games like Civilization 5 define their important game-mechanical entities through non-coding files, sometimes with hundreds of entries, that describe everything from their name and icon to the backswing point of their animations. Which is ideal: data-driven design is a good practice in data-heavy applications.

The de-facto standard for such data in the gaming industry is to use XML files: They're easy, every engine can parse them, you can define arbitrary data structures, and you can even build schema files to check for correctness and detect errors in the data.

Unity presents its own internal solution to data-driven design: The ScriptableObject. These are persistent chunks of data, implemented as classes, that can store information outside of scenes and the normal game object hierarchy yet still be referenced from them and accessed as regular objects and classes. Plus they work with Unity's inspector system, which allows drag-and-drop interfaces, data validation, and custom containers for more complex data. This makes them convenient to use in many contexts, but the fact that they're internal means they can't be modified (at least not easily) after the game has been built. That lack of post-build access would be very troublesome for adding mods to the game, which rely on visible data and non-engine scripts to add new behaviors and mechanics to the game.

Despite the fact that Civ 5 uses XML files, I opted to stick with ScriptableObjects for the sake of simplicity. They're easier to connect to code, they have built-in GUIs, don't require custom schema files for validation, and ultimately did what I needed them to do. The use of ScriptableObjects for data storage and configuration means that the game is essentially unmoddable, but I considered that an acceptable loss given the design goals of the project.

I did make a small modification to the standard workflow for ScriptableObjects, though. By default, Unity allows serialization (and thus design-time modification) only of public fields and properties. This seemed dangerous to me for things like unit definitions, because it meant my code could modify properties that should never be changed during runtime. So instead of passing concrete ScriptableObjects into my codebase, I passed interfaces with read-only properties into the codebase which I then bound to concrete ScriptableObject implementations. To allow this data to be manipulable at design time but not at runtime, I had to manually back these properties with private fields that I flagged with the [SerializeField] attribute to force Unity to keep track of them and add them to the inspector. New versions of Unity and .Net might allow easier serialization of "public get; private set" properties like this, but the versions I began with did not, so this pattern established itself and became the standard.

Signals and custom events

During the development of Civ Clone, I began experimenting with a development method called Reactive Programming, a paradigm that involves events and messages more than code order for execution. I found this intriguing enough that I installed UniRx, a reactive framework, to the project, and made substantial use of it throughout development.

Because of the structure of the rest of the codebase, my addition of reactive elements took a peculiar form. I ended up creating a number of classed suffixed with the word Signals (CitySignals, UnitSignals, CivilizationSignals) that contained within them all of the signals that might fire on behalf of a particular game entity (cities, units, civilizations). These signals were ISubjects<>, which consumers could both listen to for new events and send events of their own. UniRx only supported ISubjects<> of a single type argument, so signals requiring multiple arguments mostly made use of Tuples<> of varying size.

This implementation has consequences. For starters, it's not possible to listen to an event coming from an instance of an object: subscriptions are to events of a particular type on every instance of that object in the runtime. This is often quite useful, since many of the classes that consume these events are keeping track of behaviors for every instance anyways.

The implementation also takes no action to prevent erroneous event production. Since every consumer has the ability to produce events on every signal they've been given, it's entirely possible for completely unrelated classes to pass new events into the pipeline that should not exist. I opted to solve this problem essentially by using the honor system, trusting myself not to fire events at the wrong time. Since the events are all intuitively-named and are almost always fired from a single point in the codebase, it seemed fairly straightforward to avoid firing off false events. A larger project with more contributors might have need of isolating consumers from producers more thoroughly, though.

Interfaces for dependencies used by a single class

During development, I frequently found myself performing incredibly complex operations within a single class. For reasons of organization and to prevent God Objects, I ended up splitting the components of certain problems into sub-pieces that were handled by dedicated helper classes (MapGeneration is the most conspicuous case, where almost every class is a helper class of MapGenerator). This ended up generating a large number of Helper classes in use by only a single concrete implementation.

Despite this fact, I ended up hiding almost every single Helper class behind an interface, even if the class was relatively straightforward and was in use nowhere else. I did this to assist with unit testing, that I could test algorithms at a higher level to make sure the helper classes were being called correctly, and also in the line-by-line details of those classes.

I also used this basic pattern for helper classes in subsystems largely uncovered by unit tests (again, see MapGenerator). I did this partly to keep a consistent convention for dependency injection and nomenclature, partially out of habit, and partially to facilitate unit testing in the future. Ultimately the architecture of the untested sections of the codebase should've been reworked to facilitate unit testing, at which point this pattern might be applied more purposefully.

Dependency injection conventions

I'm making very heavy use of a plugin called Zenject to do dependency injection. One of the conventions it has is the notion of an Installer: A place where classes are instantiated and bindings made within particular Containers. Then, any class in that Container can pull in one of the bound dependencies by declaring it as an argument to its constructor or to some sort of Inject() method with the [Inject] attribute on it.

There are a large number of classes suffixed with the word "Installer", often one for every subsystem. These are the classes that set up dependency injection, binding implementations to interfaces, and ensuring single instances of classes that should not have duplicate objects. While different subsystems have different Installers, currently they all funnel into the same Container: One defined for the entire Unity Scene in this single-Scene application. I could've created separate sub-Containers for each of the modules and carefully excluded certain dependencies from certain sections of the codebase, but I opted not to. I figured that the best way of ensuring my codebase didn't become a rat's nest of dependencies was to try and limit the number of dependencies per class and maintain Single Responsibility as much as I could. Thus it was expected problematically-intertwined classes could be identified manually, which seemed acceptable given the scope of the project and the fact that it isn't (and doesn't need to be) turned into a set of modular deployables. It was most convenient to be able to pull in dependencies from different namespaces and decide, based on the diversity of namespaces and number of dependencies, whether something was too interconnected or not.

Animator as a finite state machine for the UI

Unfortunately, neither the version of Unity nor the version of .Net I began work with had a normal FSM built into them. Finding a .Net library or plugin with one isn't particularly difficult, but working with those that I found for the UI seemed incredibly tedious and fraught with potential for mistakes. I wanted the entire UI for the simulation (and also the Map Editor and menu navigating before play time) to be contained within a set of nested FSMs, with each new screen or set of panels represented by a state. I wanted to be able to manage this FSM, as much as I could, through a GUI so that I could visualize how everything was connecting and make changes relatively easily.

Built-in Unity at the start of development had only a single system that could suffice for this: the Animator component, which needs a FSM to switch and blend between animations and provides a GUI for connecting states. While not designed for UI, using the Animator as a FSM for managing multi-screen UI seems to have been (at least at the time of development) a common strategy among Unity developers, so I adopted this strategy as well.

That decision has a number of consequences. For starters, states in Unity's Animator can have custom components attached to them so long as they implement the StateMachineBehaviour class. But the implementation of these behaviors, and the controller that runs the Animator, means that Zenject's standard dependency injection pipeline doesn't work on them properly. I ended up having to manually add all of the UI Animator's StateMachineBehaviours to Zenject's injection queue to pass dependencies to them. And that turned out to be just about the only way to pass objects from the runtime into the Animator: Unity's inspector doesn't let you drop objects in a scene into an Animation Controller's behaviors, because technically the controllers aren't part of the scene in the same way that ScriptableObjects aren't.

This meant that, in order to pass a specific set of dependencies (like a set of panels to activate when the state was entered) I had to have custom dependency signatures, which required creating an entirely new class. So even though many of the UI states differ only slightly in their behavior, every single one of them has its own class. This ended up creating a bit of namespace pollution that I mitigated in part by placing the states in their own UI.StateMachine namespace. But it undoubtedly violates DRY by repeating many of the same actions over and over. Thankfully most of those actions were fairly simple.

The convention for switching between states was not ideal, either. The main way to pass data into an Animator at runtime is to use a group of Set_() methods (SetTrigger(), SetFloat(), SetBool(), SetInteger()). Strings cannot be passed in. How, then, does one turn user input into state changes within the Animator?

The solution I used was to create a number of Trigger parameters (booleans then set themselves after they're consumed, so they can't cause multiple state changes) with names like "_____ Requested" (Play Mode Requested, Return Requested, Save Cell Editing Requested). When I wanted to flag a state change, I used Animator.SetTrigger(), passing in the parameter's name, and then created simple transitions between the various states that looked for these triggers.

This plan worked fairly well except for one issue: the number of triggers. The Animator Controller I use has 41 triggers, many of which are only used for a single transition between a specific panel and one of its sub-panels. And the names of those triggers have to be synchronized both inside and outside the Animator, another potential source of bugs.

Overall, I think the FSM-based UI I used was acceptable for the project at hand. But its construction was tedious and the potential for bugs was higher than I would've liked. The better solution, I think, would be to use a more rigorous UI tool than the one Unity offers internally, possibly something like WPF. Or I could find/build something that permits the construction of a GUI-based FSM within the scene hierarchy with a more general transition system so I can pass requests into it more cleanly.

Unit tests over inline-comments

I think communicating what a piece of code does is a crucial component of software development. However, I'm leery of relying on comment-heavy code. It's very easy for the source code to change in a way the documentation doesn't reflect, which could lead to a codebase full of lies. Thus I've been looking for ways of communicating the purpose of a piece of code without resorting to big blocks of comments at the top of every class and in front of every method. Part of this process involves simply writing cleaner code: descriptive names, Single Responsibility classes, clean formatting conventions to make blocks of related code visually cohesive, etc.

But the notion of "self-documenting code" seems like a dangerous trap to fall into, especially for abstract concepts for which good names are difficult to find. So in addition to trying to maintain a clean codebase, I tried to use unit tests as a form of self-checking documentation. Thus a class's behavior should (ideally) be described by its set of unit tests.

A quick perusal of the codebase will reveal, however, that this ideal is not always maintained. There are some classes whose behavior cannot be feasibly unit-tested, and so goes without such tests. These classes almost certainly needed more documentation during the development process, but many were changing so quickly that updating documentation every single time a change was made would've vastly decreased productivity. As a sort of compromise, I've added an extensive amount of documentation about the various components of the codebase after the bulk of development has ceased and before I opted to present the project to the public. Ideally it'll give a sufficient understanding of the architecture that its decisions can be understood and its code can be improved without the grinding slog that often accompanies uncommented code.

That being said, figuring out best practices for documentation is, for me, still a work in progress. Design goals need to be better-documented, as do architectural decisions. And if I'm going to use test coverage as a form of living documentation, I'll need to do more work to add not just unit testing, integration testing and other, more cohesive test patterns.