The other day I was changing how the toy dispensers work in Toy Factory Fixer.
Instead of merely spitting out a steady stream of toys, I decided to create multiple waves of toys, separated by a number of turns.
I call these waves “Production Runs”, and I wanted to be able to set delays between them. So if a production run is scheduled to start in 5 turns, I want no toys to dispense for those 5 turns.
When a new turn starts, I need to take the next production run and decrement the number of turns left to start it.
Here’s the code:
void Dispenser::processNewTurn() { if (m_toys.size() == 0) { if (m_runs.size() > m_currentRun) { m_runs.at(m_currentRun + 1).turnToStart--; if (m_runs.at(m_currentRun + 1).turnToStart <= 0) { startRun(); } } } }
If I am currently dispensing toys, I don’t want to process anything, but otherwise, if there is a next run, I take the turnToStart value and decrease it by 1. When the count reaches 0, I can start that new run.
If I don’t have a next run, meaning that the current run is the last one, then one of my unit tests would crash, which is why I added the line that says:
if (m_runs.size() > m_currentRun)
But then a few other tests started to fail. I checked, and the block of code within the second if-statement never ran.
I printed out some logs, and sure enough, the size of m_runs was indeed greater than m_currentRun, which starts off at a value of -1, so of course it is smaller than a collection of 1 production run.
So what gives?
I had a vague recollection that there is a possible type conversion happening here.
I realized that the reason why the code wasn’t running in the second if block was because there was a difference between the type of integer returned by m_runs.size() and the type I am using for m_currentRun.
See, m_runs is a vector, and standard C++ collections have a function size() that returns a type size_t.
size_t is an unsigned integer.
m_currentRun is a signed integer.
Normally it is not a problem to mix and match integer types. Or maybe more accurately, it depends. I can create a for loop with a signed integer as an index and compare it to the size of the collection:
for(int i = 0; i < m_runs.size(); ++i)
And I expect that code should work.
But when comparing unsigned integers with signed integers using a binary operator such as > or <, C++ looks for a common type and automatically converts one value to it.
And in this case, the signed integer becomes an unsigned integer. In fact, it happens in the previous for loop case as well, but nothing goes wrong because i is always greater than or equal to 0, so whether it is signed or not, the value being compared doesn’t change.
In my processNewTurn() code above, however, m_currentRun is -1 to start, but it gets converted into an unsigned integer.
And -1 can’t be represented as an unsigned integer, so instead it now represents the largest possible integer.
Which means the expected logic is that m_runs.size() will always be less than that largest possible integer.
So a quick fix is to force the unsigned integer to be a signed integer instead:
if (m_runs.size() > m_currentRun)
becomes
if (static_cast<int>(m_runs.size()) > m_currentRun)
And now the code runs as I expected, with the comparison working as I expected.
And I am really glad that I not only did I have the experience to give me a vague recollection of running into this kind of issue before, but that I had tests that immediately told me that there was something wrong with my logic.
I found the issue and fixed it within minutes rather than continuing to write code ignorant of the problem and then wondering why the dispenser wasn’t working right potentially hours or even days later.
I know that a lot of game developers look at practices such as TDD as something that slows them down or that doesn’t work in games, but I don’t understand this point of view.
That fast feedback I described is key to being able to develop faster. I spend less time finding and fixing issues manually, and I am able to address issues while I am still thinking about them rather than needing to remember how the code worked at a later time.
It took me longer to write this blog post than it did to write the code, find the problem, and fix it, all because I found the bug immediately after I wrote it thanks to my unit tests that I just wrote revealing its existence.