During a project it is important to stop and reflect on what you’ve implemented and see if you like it or not. In some cases this will lead to “refactoring” – the restructuring of architecture to improve upon it.
Overview
In Lesson 29 of this project, Events, I modified the interface of my IDependency by adding methods for `SetUp` and `TearDown`. There were some aspects of this that I liked, and it gave me an opportunity to introduce “default interface methods”, which I hadn’t really used before, so I thought I’d give it a chance.
Now that I’ve had some time to reflect, I’ve decided I don’t actually like what I did. Sorry to anyone following along, but this is also a teachable moment. In the past I may have merely mentioned that I “refactored” something and left it up to the reader to see what had changed, but one of my readers left some constructive criticism that I should do better. So I’m giving that a try – I will use this small lesson as an opportunity to describe what is changing and why so you can see my thought processes more clearly. Let me know if you would like me to keep making these kinds of posts, or if you would rather I keep moving on to more interesting things.
My initial complaint with the `SetUp` and `TearDown` was that it overall felt like a lot of extra boilerplate code for every system I added, especially given that most of the systems weren’t even using that functionality. Another critique would be that I broke the “Interface Segregation Principle” which states that “existing interfaces should not be augmented by the addition of new methods”. In my opinion, this principle is just a guiding light, not a hard and fast rule, especially while I am still prototyping, but I think it also started to break out of the “Single Responsibility Principle” – the main point of the IDependency was to replace the functionality of an Inversion of Control container, or in simpler words, to provide access to things I need without needing to know their concrete type. `SetUp` and `TearDown` are separate concerns.
What I really wanted with the `SetUp` and `TearDown` is a rough equivalent of things like a MonoBehaviour’s lifecycle methods such as `Start` and `OnDestroy`. In practice, I specifically needed a way to observe and unobserve events, and wanted a way to allow systems to perform their setup at a time that they would know all other systems had already been registered by the injector so that I didn’t need to worry about the order of their initialization.
A different option, and the one we will try in this lesson, is to create a system whose sole responsibility is to handle SetUp for “anything” that needs it. Likewise, I can create a system solely for TearDown. With that said, I do still have to register these two systems before any other systems, but I don’t feel too bad about it in this case, because I have confidence that these two systems will never need any other outside dependency. I did not feel the same about things like various game element systems that might want to observe events from each other.
Getting Started
Feel free to continue from where we left off, or download and use this project here. It has everything we did in the previous lesson ready to go.
IDependency
Open up IDependency.cs and delete the SetUp and TearDown methods. You may notice that this causes Unity to output a TON of scary looking compiler errors in the Console. Don’t worry – they are actually good in this case, because they are helping to make sure we don’t miss any steps during the refactor process. Our first goal is to make changes until all of those errors are resolved.
At the top of the list, I see an error that looks something like this:
Assets/Scripts/Actions/ActionInjector.cs(10,32): error CS1061: ‘ICheckSystem’ does not contain a definition for ‘SetUp’ and no accessible extension method ‘SetUp’ accepting a first argument of type ‘ICheckSystem’ could be found (are you missing a using directive or an assembly reference?)
There is a bunch of helpful information in this. Right off the bat it tells me exactly what script has an error, and even tells me what line it appears on (10), and even how far into the line (32)!
So the first thing I do is open that script. Line 10 looks like this:
ICheckSystem.Resolve().SetUp();
Basically, the compiler is telling me, “Hey I’m with you so far, I understand what an ICheckSystem is, and I know about the Resolve method, but what does SetUp mean for this type?”
Of course I expected it to fail, because the place where SetUp had previously been defined no longer exists. The solution to this error (and another error on line 15) is to delete from lines 8 to 16 – I want to remove the methods “SetUp” and “TearDown” such that the final result of the “ActionInjector” will now merely look like this:
public static class ActionInjector { public static void Inject() { ICheckSystem.Register(new CheckSystem()); } }
We will need to repeat this process several more times – we should delete the `SetUp` and `TearDown` methods from each of these files:
- AssetManagerInjector
- BoardInjector
- CombatInjector
- CombatActionsInjector
- AbilityScoreInjector
- HealthInjector
- DiceRollInjector
- DamageInjector
- SkillsInjector
- ComponentInjector
- DataInjector
- Injector
- SoloAdventureInjector
- EntityInjector
- FlowInjector
Ah, so nice. Deleting code often feels therapeuatic to me 🙂
There are still two remaining errors found in `AppFlow` because we also removed the `SetUp` and `TearDown` calls from the main `Injector`. For now, simply comment out those 2 lines (line 10 and 20).
At this point all of the errors should be resolved. If not, you should resolve any that remain before continuing.
SetUp System
Now we will create the system that is solely responsible for triggering the SetUp of any subscribed system that needs it. Create a new C# script at Scripts -> Dependency named SetUpSystem and add the following:
using System; using System.Collections.Generic; public interface ISetUpSystem : IDependency<ISetUpSystem> { void SetUp(); void Add(Action action); void Remove(Action action); } public class SetUpSystem : ISetUpSystem { List<Action> actions = new List<Action>(); public void SetUp() { foreach (var action in actions) action(); } public void Add(Action action) { actions.Add(action); } public void Remove(Action action) { actions.Remove(action); } }
The system is very simple – there are methods to “Add” and “Remove” actions that should be performed as part of “SetUp”. The “SetUp” method itself will invoke each of the actions that have been Added.
TearDown System
We will create another system that is nearly identical to the SetUp system – it just has the name TearDown instead of SetUp. Create a script named TearDownSystem in the same folder and add the following:
using System; using System.Collections.Generic; public interface ITearDownSystem : IDependency<ITearDownSystem> { void TearDown(); void Add(Action action); void Remove(Action action); } public class TearDownSystem : ITearDownSystem { List<Action> actions = new List<Action>(); public void TearDown() { foreach (var action in actions) action(); } public void Add(Action action) { actions.Add(action); } public void Remove(Action action) { actions.Remove(action); } }
Injector
Now open the main Injector script. Replace the Inject method with the following:
public static void Inject() { // These Must Be First // ------------------- ISetUpSystem.Register(new SetUpSystem()); ITearDownSystem.Register(new TearDownSystem()); // Order doesn't matter // -------------------- ActionInjector.Inject(); AssetManagerInjector.Inject(); BoardInjector.Inject(); CombatInjector.Inject(); ComponentInjector.Inject(); DataInjector.Inject(); DiceRollInjector.Inject(); EntityInjector.Inject(); FlowInjector.Inject(); IGameSystem.Register(new GameSystem()); IInputSystem.Register(new InputSystem()); SoloAdventureInjector.Inject(); }
Note that normally the order of injection shouldn’t matter, and so my systems had been added in alphabetical order (not including the prefix “I”), but this is a special case, and I want any other system to be able to refer to these two new systems right from their constructor. In order to help make sure I don’t forget later, I left some comments in the code about where order does and doesn’t matter.
App Flow
Open the AppFlow script again, and look at the two commented out lines. The first (line 10), we can replace with:
ISetUpSystem.Resolve().SetUp();
The second (line 20), can be replaced with:
ITearDownSystem.Resolve().TearDown();
Finishing Steps
For our last steps, I need to find any system that had used our old `SetUp` and `TearDown` structure. I am using Visual Studio as my IDE, so from the file menu I choose “Search -> Find in Files…” and then I told it to find instances of “SetUp”. I actually surprised myself by how many results were found – I was using “SetUp” (but with an Entity parameter) on many different classes. This is just another reason that it was probably a good idea to remove the interface method – it helps avoid possible naming conflicts in the future.
I didn’t feel like scrolling through the results, so I tried another search, this time with a more complete thing to find, “void SetUp()”. Much better. Not including the “Tests” I was able to identify that I only need to fix the “EntitySetSystem” and “EntityTableSystem”.
Open the “EntitySetSystem” and add the following constructor:
public EntitySetSystem() { ISetUpSystem.Resolve().Add(SetUp); ITearDownSystem.Resolve().Add(TearDown); }
Note that I could have used an “Action” that was any method with the same signature – the name of it doesn’t actually matter, so I could have had used methods named like “AddEvents” and “RemoveEvents” as long as they accept no parameters and have a return type of void.
Open the “EntityTableSystem” and add a constructor for it as well:
public EntityTableSystem() { ISetUpSystem.Resolve().Add(SetUp); ITearDownSystem.Resolve().Add(TearDown); }
Demo
Well… there isn’t anything “new” in this lesson, but it’s probably a good idea to play the demo and make sure nothing has broken. Despite the massive amount of code I deleted, everything should be functionally the same. If you want, you might want to add debug logs to make sure data gets deleted after combat entities are destroyed.
Unit Tests
As far as the game programming side of things go, our refactor is finished. However, it is important to run unit tests and make sure they all still work as well. Spoiler alert – they don’t. Tests for subjects that inherit from EntitySetSystem or EntityTableSystem will now fail because of a null-reference. The “SetUp” and “TearDown” systems won’t have been registered yet, so we would need to add that as part of the unit test’s SetUp. For example, open the AbilityScoreProviderTests and then add the following to the beginning of the SetUp method:
ISetUpSystem.Register(new SetUpSystem()); ITearDownSystem.Register(new TearDownSystem());
You would need to do the same for the following:
- BaseSkillSystemTests
- DamageSystemTests
- EntityTableSystemTests
- SoloHeroSystemTests
Once that is done, all of the tests should pass once again, woohoo!
Summary
Refactoring is a part of the development and maintenance of every large project I have been on. I decided to do some “out loud” as a learning opportunity in the hopes that it would be helpful. The refactoring itself removed the `SetUp` and `TearDown` methods from the `IDependency` interface and moved that functionality into its own system. The result was a large reduction in code without losing any functionality. Be sure to leave a comment and let me know if you enjoyed following along with this, or would rather I keep focused on the “interesting” stuff (new features).
If you got stuck along the way, feel free to download the finished project for this lesson here.
If you find value in my blog, you can support its continued development by becoming my patron. Visit my Patreon page here. Thanks!
I’m a big fan of this tutorial and I’ve been eagerly awaiting your next post. It’s been a while since your last update, and I was just wondering if you have anything new in the works.
No pressure at all, but I just wanted to let you know how much I enjoy your content and look forward to hearing from you soon.
I’m glad you’ve been enjoying the series. As a whole people have been pretty quiet on this project so I wondered how it was being received. I have been making progress on the side to try and get ahead again. So far I have worked on Ancestries, Backgrounds, and even have some pathfinding for larger units working now. I’m thinking about putting the project in an online repository so anyone interested can see where I am going before new posts are ready. Would you be interested in that, or do you prefer the guided tutorials?
Thanks for the update! I’m really happy to hear you’re making progress on the project.
While I appreciate the offer to see your work in an online repository, I’m personally more interested in the guided tutorials. I find them incredibly valuable as they offer a window into your thought process and design choices. Learning “how the sausage is made” is really insightful and helps me understand the project on a deeper level.
Of course, I completely understand if your circumstances have changed, and creating the full tutorials might not be feasible. Regardless of your decision, I’m still very much interested in seeing the project unfold and will eagerly await your next update.