Arklon, I was just looking at your second last point, and I think I know what's going on.
The function in your second last point tries to return a recycled object if one exists, otherwise it allocates new memory for the object. When it allocates memory, it allocates a chunk from a pre-reserved range, where each chunk holds 16 objects. It then initializes the next pointers for those 16 new objects. The problem is the memory allocation isn't checked. When the reserved range runs out, the allocation can fail. If you create a lot of units, and they all need those objects to compute their paths, then the reserved range is probably being exhausted.
If the code used the unchecked return value, it would crash almost right away, since address 0 is never writable to help catch null pointer errors. Instead, the code uses the address it requested, assuming it would be allocated. Hence, it writes to heap memory, which is a valid write address, and corrupts it.
A quick fix would be to increase the size of the reserved memory range. See address 0x00446B06 for the size being reserved.
I see. I still would like to know why this occurs when I add units to FightGroups, yet it doesn't when I just issue them move commands instead. And yeah, when I said DestWaypoints I was referring to the large structure with all the various pathfinding data (in DataStructure PathFinder.txt it's the Waypoint struct, or in my test project it's in OP2Types\OP2Pathfinder.h as DestWaypoints). Although, are you actually referring to the char direction[64] part? Because that'd be interesting, since I thought to the unit class would point to the DestWaypoints struct it's stored in.
Increasing the size of the block would only prolong the inevitable, it'd just take more units to trigger the overflow. We'd have to make it allocate the objects on the heap, find and patch the recycling code to deallocate the objects, and find out what causes objects to sometimes fail to recycle.
And, of course, I would like to know how to intentionally emulate the bug by getting pointers to the tilesets and overwriting their bitmap and palette data with DestWaypoints data, since just fixing the bug and leaving it at that would do an injustice to such an amazing bug.
I have a feeling that might involve having to decipher the functionality of the pathfinder functions (to see what's being written to the DestWaypoints object) which were too much of a mess even for you, though. I was looking at the pathfinding code myself a while back and I pretty much "nope"-d out of it. I guess if you decipher what all the fields in the struct mean you can probably guesstimate which fields are likely getting written to and in what way, but ideally I'd like to just find some pathfinder function which I can pass a pointer into the tileset bitmap/palette data as the input for DestWaypoints pointer along with some waypoints/etc. just to keep it simple.
On a side note, I was looking at one of your older posts on your work on 1.3.5, and you mentioned that you'd like to strip the relocation table from Outpost2.exe in case it might interfere with any baked-in code patches. I feel differently about it, actually. I think it's fortunate that older compilers tended to compile exe's with relocation tables by default (even though they're pretty unnecessary for exe's unlike dll's), as was the case with OP2. Because that information exists, you can, for example, parse it to find every (direct) pointer to an internal object.
It's a very convenient means of resizing/replacing statically-allocated objects and arrays like the graphics object or the unitTypeInfo array, so additional sprites/etc. or unit types can be added. For some of these things, trying to do it by patching each reference by hand would be pretty sick - for example, the graphics object has 226 references! Fortunately, the locations of all 226 are conveniently contained in the relocation table. It also has the locations of all direct function pointers (such as in vftables), but that does not include relative pointers (mainly CALL REL32 which is a regular function call) nor RVA pointers, as relocation info for those types of pointers only exist in unlinked .objs.
If you're concerned with the relocation table messing with code patches (you move or delete a direct pointer reference), you can patch the reloc table to point to the new offset of the pointer or set the relocation type to 0 (IMAGE_REL_BASED_ABSOLUTE) which makes it get ignored, or I guess set the RELOCS_STRIPPED or whatever flag in the file header since that would still keep the physical data.
Here is a link to a dump of the relocation table:
link. It includes the actual pointers that get relocated, which dumpbin doesn't show. IBR stands for IMAGE_BASE_RELOCATION, which is the header of each relocation block, and REL is a relocation entry. Each block contains relocations for (by default) a 4096-byte range, for example 0x00401000-0x00402000. x86 PE relocations are always type 3 (IMAGE_REL_BASED_HIGHLOW), but sometimes a dummy type 0 relocation is added to the end of a block to pad the next block to 4 bytes (since relocation entries are WORDs).