Crash when removing droids from group about warzone2100

[KJeff01]

@ManManson Would you be up to taking this? The code this centers around may be near stuff you are planning on changing/refactoring at some point.

[ManManson]

Yes, I'll take a look.

[ManManson]

The problem lies within the loadSaveDroid function. The exact underlying scenario that triggers the error is as follows:

1. The savegame files from Beta 8 (after let me win cheat and returing to base from Beta 7) are valid, i.e. there are two commanders, one of which has the associated group with ID=1 (the one which was back at the base during the offworld mission, let's call it A). The second one, which returned from the Beta 7 (let's call it B), doesn't have any group (that's another issue, but not really relevant at the moment).
2. The first one to be constructed during loadSaveDroid function is B, which will first create an empty group with ID=0 for itself in the reallyBuildDroid function.
3. Later on, in the same loadSaveDroid function, there's the code to handle aigroup droid attribute. This code triggers the creation of another group, this time with ID=1, and assigns itself to it as a commander. The group with ID=0 gets destroyed.
4. Sometime later A will be created in the loadSaveDroid -> reallyBuildDroid function call sequence. Again, it will create a group with ID=0, which will later be destroyed after processing aigroup reference, which, in this particular case, is set to 1 (as was defined in the droid.json file from the savegame).
5. Now, the code handling the aigroup attribute will issue a grpFind(1) call. This will find an existing group with ID=1 created by B. From now on, everything is broken.
6. A tries to add itself to this ID=1 group, effectively increasing its refCount by an additional 1 (though, the psCommander still points to B).
7. Sometime later the save fails to load (not sure about the exact reason, though), leading to calling the freeAllDroids function.
8. A is destroyed first, this will reset the B's group type from GT_COMMAND to GT_NORMAL, but the group is still there, because the refcount is still positive.
9. B gets destroyed, attempts to remove itself from its group
10. This triggers the code branch in DROID_GROUP::remove that will try to remove it from group's psList. Commanders aren't added to this list, so std::find returns an end() iterator. And we try to call psList.erase(psList.end()), which is forbidden and wreaks havok by corrupting psList:s memory.
11. DROID_GROUP::~DROID_GROUP tries to call dtor for psList and raises SIGSEGV.

The code that allocates and assigns the groups to droids and commanders should be fixed to eliminate such behavior. I'll try to fix that.

[ManManson]

The main issue is that the commander from the offworld mission loses its group (e.g. the associated DROID_GROUP is destroyed and psDroid->psGroup is set to nullptr in droidRemove() function) upon transitioning from Beta 7 to Beta 8 as described in the issue repro.

The issue can be worked around either by preserving an empty group for the commander in such case, so that its psGroup is still set, or by modifying the loadSaveDroid() behavior to check for other group IDs present in the current droid.json file to avoid clashes between group IDs.

EDIT: the latter seems to be more preferrable, since this solution would work irrelevant of whether the group is set or not, which can also protect from some programming errors when groups are lost, like this particular case, but not limited to.