Starcraft AI blog

CombatCommander::updateIdleSquad() does this:

    for (const auto unit : _combatUnits)
    {
        // if it hasn't been assigned to a squad yet, put it in the low priority idle squad
        if (_squadData.canAssignUnitToSquad(unit, idleSquad))
        {
            idleSquad.addUnit(unit);
        }
    }

The code is correct as it stands. It cannot assign 1 unit to 2 squads. But I consider it a latent bug: Only the lowest-priority squad can be updated this way. It would be easy and natural to make changes to squad priorities and cause an error, or to copy and paste this code under the assumption that it would be correct in a different case. You would get an irritating bug with no obvious cause. In the spirit of defensive programming, I recommend changing it to this:

    for (const auto unit : _combatUnits)
    {
        // if it hasn't been assigned to a squad yet, put it in the low priority idle squad
        if (_squadData.canAssignUnitToSquad(unit, idleSquad))
        {
            _squadData.assignUnitToSquad(unit, idleSquad);
        }
    }

assignUnitToSquad() ensures that the unit is removed from any previous squad before it is added to the new squad. Any squad other than the lowest-priority squad might steal a unit from another squad, and you can’t steal it without taking it away. Assigning 1 unit to 2 squads would cause problems, and Steamhammer checks later and will throw.

At a minimum, I recommend commenting the code so you don’t copy-paste it heedlessly when adding a new squad type.