Broken Saved Game Parsing

[DanRStevens]

We may have broken saved game parsing. I received the following errors when trying to run the OP2MapImager on my Outpost 2 folder:

Render initialized (May take up to 45 seconds): SGAME6.OP2
All instances of version tag in .map and .op2 files should be greater than 4112Error reading from file

Render initialized (May take up to 45 seconds): SGAME1.OP2
All instances of version tag in .map and .op2 files should be greater than 4112std::bad_alloc

Render initialized (May take up to 45 seconds): SGAME0.OP2
All instances of version tag in .map and .op2 files should be greater than 4112Error reading from file

This might happen if reading has been offset by an incorrectly sized field. Or maybe the saved games have a different version number than maps. We did change the version number code.

[Brett208]

I looked at this today. I believe it is caused by:

---------------------------
    4   tag - must match tag at first DWORD of .map file else error (see Header Section)
---------------------------
        Note: extra data can be read here, depending on the parameters to the load routine.
        .map files shouldn't load any data here, probably only saved games (which do seem to load extra data here)

For a saved game, we are not accounting for the extra data between the 2 version tag reads.

streamReader.Read<uint32_t>(map.terrainTypes);
ReadVersionTag(streamReader);
// TODO: Account for saved game data
ReadVersionTag(streamReader);
// TODO: Check if tile group data exists on a saved game.
ReadTileGroups(streamReader, map);

-Brett

[DanRStevens]

I think you're right. Thanks for figuring that out.

Perhaps we should write a method to read the unknown counts, and advance the stream by the calculated size of that data. We probably don't need to worry about storing the data at this point.

---------------------------
    4   tag - must match tag at first DWORD of .map file else error (see Header Section)
---------------------------
        Note: extra data can be read here, depending on the parameters to the load routine.
        .map files shouldn't load any data here, probably only saved games (which do seem to load extra data here)
    4   numUnits
    4   **TODO** figure out what this is
    4   **TODO** figure out what this is
    4   **TODO** figure out what this is
    4   sizeofUnit - 0x78 (120 bytes) if sizeofUnit != 0x78 && numUnits != 0 then Error
- - - - - - - - - - - - - -
        Note: Reading continues here in a subroutine at 0x00446CF0
    4   numObjects1 (number of 512 byte objects)
    4   numObjects2 (number of 4 byte objects)
        Note: if (numObjects1 == 0) return (successfully)
    512*numObjects  objectArray **TODO** find out what this object is
        Note: if (numObjects2 == 0) return (successfully)
    4*numObjects2   objectArray **TODO** find out what this object is
        Note: End of subroutine read
- - - - - - - - - - - - - -
    4   unitID - **TODO** find out significance
    4   unitID - **TODO** find out significance
    0x1DF88 unitRecord[1023] - 1023 unit records of 120 bytes each
        Note: End of "optional" data (not really optional for saved games files)
---------------------------
    4   tag - must match tag at first DWORD of .map file else error (see Header Section)
        Note: End of .map load.
---------------------------

---

Edit: We should also perhaps take a look at the messages at the end of the error lines reported previously:

Error reading from file
std::bad_alloc
Error reading from file

Those might indicate something wrong beyond what we've already diagnosed. They appear to be from exceptions thrown sometime after the cerr warning was printed. The recent change to throw instead of write to cerr will silence these additional errors. We should still look into the root cause though.

[Brett208]

@DanRStevens, OP2MapImager is imaging saved games fine for me now and I'm not seeing any errors or output. Could you elaborate a little more on what I need to do to produce the following:

Error reading from file
std::bad_alloc
Error reading from file

I suspect that this might have been caused by trying to parse the Saved Game specific data into the TileGroups, which is now prevented? Wanted to ask though instead of assuming.

Thanks, Brett

[DanRStevens]

Those error messages were appended to the ends of the original cerr error messages:

All instances of version tag in .map and .op2 files should be greater than 4112Error reading from file

They appear to be exception output. Now that we throw exceptions earlier, due to the file version tag check, we abort earlier. Hence there is no chance for that second error to be hit and cause output. However, we never actually investigated the cause of that second error. It might be relevant. In particular, we got different error messages for different files, so that is odd. Additionally, they are not the errors we might have expected for failing to account for the saved game data.

To test that, we may need to check out the original version of the code at the base of this branch and work from there.

[DanRStevens]

With the current code, the error is now:

./OP2MapImager ../opu-git-svn/GameDownload/Outpost2/trunk/SGAME0.OP2
Render initialized (May take up to 45 seconds): SGAME0.OP2
All instances of version tag in .map and .op2 files should be greater than 4112.
Found version tag is 275.

This is more clear, though still an error.

[Brett208]

I am testing 4 different saved games. They all read and render fine with OP2MapImager for me without throwing an error. How did you create the saved game? If you want to send it to me, I'll step debug through it and see where it diverges from our documentation.

Thanks, Brett

[DanRStevens]

I tried using the saved game with the garage storage view bug: https://forum.outpost2.net/index.php/topic,5816.msg86570.html#msg86570

The error is:

./OP2MapImager ../opu-git-svn/GameDownload/Outpost2/trunk/SGAME6.OP2 
Render initialized (May take up to 45 seconds): SGAME6.OP2
All instances of version tag in .map and .op2 files should be greater than 4112.
Found version tag is 285.

---

I grepped for "Error reading from file" (the previous error). It comes from: https://github.com/OutpostUniverse/OP2Utility/blob/67d36bda3e4548dad1db4e0a3e579b4b512ed0bf/src/Stream/FileReader.cpp#L33-L39

That may indicate an attempt to read past the end of the file. This is actually surprising. Considering where the additional saved game data is located, it should have simply read an extra version tag (misinterpreting saved game data), warned about the discrepancy in the version, and then returned, without error. At least that's what I believe the previous code should have done.

Edit: I just realized, it probably tried to load the tile group information at the end, which Outpost2.exe doesn't. That may account for the difference. So, exactly as you suggested a few posts back.

[Brett208]

OK, thanks for the quick response. A few notes below:

I attempted to image the saved game and got the same error as @DanRStevens.

I wonder if the error is in the saved game itself or in the MapReader code. The saved game may contain the bug that blows up the game when loading the garage???

The current codebase stops reading the saved game after checking the last version tag. I modified the code base to stop reading after reading the second version tag. This allowed the saved game to form a proper map render.

Our error is likely happening after the second version tag and before the third version tag.

The savedGameDataSection2 contents are below after reading before hitting the second version tag:

?savedGameData
    unitCount: 279
    unknown1: 334
    unknown2: 394
    unknown3: 339
    sizeOfUnit: 120
    objectCount1: 176
    objectCount2: 94
    objects1: { size=176 }
    objects2: { size=94 }
    unitID1: 31
    unitID2: 95
    unitRecord: { size=2047 }

?savedGameData.objects2
{ size=94 }
    [capacity]: 94
    [allocator]: allocator
    [0]: 37
    [1]: 72
    [2]: 141
    [3]: 145
    [4]: 48
    [5]: 24
    [6]: 146
    [7]: 84
    [8]: 38
    [9]: 47
    [10]: 76
    [11]: 51
    [12]: 28
    [13]: 104
    [14]: 73
    [15]: 95
    [16]: 50
    [17]: 34
    [18]: 115
    [19]: 94
    [20]: 139
    [21]: 98
    [22]: 144
    [23]: 165
    [24]: 49
    [25]: 17
    [26]: 142
    [27]: 102
    [28]: 60
    [29]: 27
    [30]: 153
    [31]: 6
    [32]: 89
    [33]: 20
    [34]: 4
    [35]: 151
    [36]: 124
    [37]: 66
    [38]: 143
    [39]: 113
    [40]: 33
    [41]: 11
    [42]: 13
    [43]: 130
    [44]: 128
    [45]: 74
    [46]: 111
    [47]: 23
    [48]: 8
    [49]: 53
    [50]: 71
    [51]: 59
    [52]: 42
    [53]: 80
    [54]: 86
    [55]: 155
    [56]: 133
    [57]: 35
    [58]: 30
    [59]: 61
    [60]: 65
    [61]: 127
    [62]: 117
    [63]: 12
    [64]: 122
    [65]: 19
    [66]: 46
    [67]: 29
    [68]: 87
    [69]: 67
    [70]: 137
    [71]: 55
    [72]: 92
    [73]: 154
    [74]: 45
    [75]: 75
    [76]: 100
    [77]: 9
    [78]: 68
    [79]: 121
    [80]: 156
    [81]: 114
    [82]: 138
    [83]: 15
    [84]: 152
    [85]: 118
    [86]: 168
    [87]: 169
    [88]: 170
    [89]: 171
    [90]: 172
    [91]: 173
    [92]: 174
    [93]: 175
    [Raw View]: {...}

I used HxD to find the int 4113 in the saved game. It was at the following offsets:

    0x1E025
    0x32C1F
    0x42519
    0x86D47

The difference between the 3rd and 4th offset is 0x4482E.

Sorry, nothing conclusive. This might be hard to diagnose due to our lack of knowledge of what the fields in SavedGameDataSection2 actually represent.

-Brett

[DanRStevens]

Ok, thank you very much for looking into this. That's some good debug info, and nicely presented.

I don't see anything obvious at present. I'll compare the code to the file format notes, though I suspect the notes might not tell the whole story. In particular, I'm uncertain where the DLL specific saved data section is. That seems to be missing from the file format notes. That may be why the last version tag doesn't match up in the expected location.

Perhaps an OllyDbg session is in order to investigate this one.

---

I've noticed saved game processing now works for some saved games files. Progress! :+1:

[DanRStevens]

Ok, I've got some more info. It seems there is an extra optional data block for saved games after the unit array and before the final version tag. There's also additional data loaded after the final version tag for saved games. That additional data includes the level DLL data.

Details:

---------------------------
    4   tag - must match tag at first DWORD of .map file else error (see Header Section)
---------------------------
    Note: Extra data is read here for Saved Games. Map files do not contain this section of data.
    4   numUnits
    4   lastUsedUnitIndex
    4   nextFreeUnitSlotIndex
    4   firstFreeUnitSlotIndex
    4   sizeofUnit - 0x78 (120 bytes) if sizeofUnit != 0x78 && numUnits != 0 then Error
- - - - - - - - - - - - - -
    Note: Reading continues here in a subroutine at 0x00446CF0
    4   numObjects1 (number of 512 byte objects)
    4   numObjects2 (number of 4 byte objects)
    Note: if (numObjects1 == 0) return (successfully)
    512*numObjects  objectArray **TODO** find out what this object is
    Note: if (numObjects2 == 0) return (successfully)
    4*numObjects2   objectArray **TODO** find out what this object is
    Note: End of subroutine read
- - - - - - - - - - - - - -
    4   unitID - nextUnitIndex
    4   unitID - prevUnitIndex
    UnitArray (pre and post unit limit patch) sizeof(unitRecord) = 120 bytes:
        0x1DF88 UnitRecord[1023] unitArray - Original (pre unit limit patch)
        0x3BF88 UnitRecord[2047] unitArray - Updated (post unit limit patch)
- - - - - - - - - - - - - -
    Optional: This section is only read if (firstFreeUnitSlotIndex != nextFreeUnitSlotIndex)
    0x2000 Unit*[2048] unitFreeList
- - - - - - - - - - - - - -
    Note: End of Saved Game specific data section. Map and Saved Game data continue from here.
---------------------------
    4   tag - must match tag at first DWORD of .map file else error (see Header Section)
    Note: End of .map load. (Outpost2.exe does not load .map data beyond this point)
---------------------------
    Additional data: This is found only in saved game files
        MessageLog
        LevelDLL
        Player[]
        Research
        DayNight
        UnitHotKeyGroups
        DetailPane
        RandomNumberGenerator
        Lava
        Blight
---------------------------
    Extra data: This is found in .map files, but is not loaded or processed by Outpost2.exe.
    Each tile group is tagged with a name (eg. "LAVA_LEFT", "midd_lava", "RIGHT_LAVA", etc.)

I believe if we add support for optionally loaded the 0x2000 sized data block, that should allow the version tags to match up, and eliminate the error we are still seeing for some saved games.

[Brett208]

Thanks for doing this research. Everything was easy to follow.

Should the following line:

0x2000 Unit*[2048] unitFreeList

Read:

0x2000 UnitID*[2048] unitFreeList

Thanks, Brett

[DanRStevens]

Ahh, right. For the file format, that should be UnitIndex[2048] unitFreeList. Outpost 2 does some funny union stuff where it converts pointer values into indexes when saving, and then converts the indexes back to pointers after loading. It overwrites the same memory while doing this. That may be why the type was a little bit off. The file format uses indexes, the in memory format uses pointers.