During the last few weeks I’ve been refactoring some horrible Lua code. This has been a ton of fun so far, and I learned many new things about Lua that I’d like to share.
Such Horrible Code
The code in question is that of a scripted Supreme Commander Forged Alliance Forever Map called Final Rush Pro v4. Essentially all the code resides in a single Lua file slightly over 2500 lines long. It is entirely procedural, uses global state all over, contains plenty of copy pasted code and, unsurprisingly, does not have a single test. What’s more is that at least some of the code must have been written by people not even at home with procedural programming, as there are several instances when massive if-else blocks are used rather than loops.
The high level approach I took was to identify cohesive sets of code in the huge file and move them out in dedicated files. These dedicated files would then have their dependencies explicitly defined and could be cleaned up one by one. This graph shows the lines of code of the Lua file that acts as entry point over time:
The first example can be seen in moving the “PrebuildTents” code into its own file. This code, coincidentally, nicely illustrates the copy pasting and insane use of if-else over loops. One huge issue that remains when simply moving the code like that is that it remains in global/static scope. In other words, it’s not possible to use the code in the file with two different sets of local values. I did some searching on how to idiosyncratically achieve polymorphism in Lua.
One of the first things I read through was the Object Orientated Programming pages of the Programming in Lua book. Following that approach, I created the very first version of a simple wrapper around a list of player armies. As you can see there, I wrote tests for that code (more on those tests below). I was not too happy with that approach as it does not provide nice encapsulation. After looking at the code of some of the more prominent Lua tools I came across, I decided to go with a closure based approach instead. Initially I would define a this local table, which would then get functions bound to it. I switched to returning a map at the end of the closure, which makes it more clear what the public functions are, and leaves one less local variable to worry about. (The closure is assigned to newInstance rather than just returned due to the way the import mechanism of the framework works, which is different than Lua’s native require.)
A downside of how the code in the files is organized is that you essentially need to read backwards when looking at how it is invoked. The public functions are listed at the very end of the file, with their dependencies defined before, and their dependencies defined before that. It would be nice to have the public functions more clearly visible at the top of the file, which is where you need to look for the constructor signature already.
Now the creating of cohesive sets of code is mostly done, the entry point file is down to 44 Lines of Code. It defaults some options coming from the framework/game, and then invokes a high level module that sets up the various aspects of the game, which totals 70 Lines of Code.
My next steps are further cleanup of individual sets of code, with a focus on minimizing dependencies and separating concerns. For this I’m using practices, principles and patterns which are by and large language agnostic, so I won’t get into them here. You can find the code of the new version of the map in the Final Rush Pro 5 repository on GitHub, including many small refactoring commits in the git history.
My first modifications to the code where with Notepad++ on Windows. While that editor provides syntax highlighting, there is no static code analysis or any of the essential things that require it, such as navigating to definitions. Hence I switched to my usual development environment, IntelliJ on Linux, using the IntelliJ Lua plugin.
While that switch to Linux made refactoring the code easier, it also prevent me from (manually) testing the code. This code, like many legacy balls of mud, binds very tightly to its framework, in this case the Supreme Commander game that only runs on Windows. While it’s often good to remove such binding, it’s not a trivial task, and not something I’d want to attempt without a fast feedback cycle.
The lack of fast feedback drove me to find a Lua testing tool to use. Several are listed on the lua-users wiki. After checking the project health of several tools, I decided to go with Busted, which I installed via LuaRocks. I then proceeded to create a wrapper for the list of players in the game (to replace code that was not only crappy but also incorrect) using Test Driven Development, resulting in a nice spec for the wrapper.
Unfortunately the same approach would not work for cleaning up most of the other code. The framework binding was just too high, and in a lot of cases, contrary to the typical scenario I’m used to (which are not games), perhaps simply the best that can be done. Hence I switched back to Windows.
On Windows I installed IntelliJ with the Lua plugin, TortoiseGit, and Busted. The latter was quite a hurdle, since my Windows administration skills are not exactly stellar. For Busted I needed to install Lua (ya really), LuaRocks and the MinGW compiler. Being able to run the tests in the IDE’s terminal was worth it though.
Version 5 of the map has now been released, see the release post for details on the new features.