[Vinifera Bug] Chain-save corruption bug

[Rampastring]

First Check

• I added a very descriptive title to this issue.
• I used the GitHub search to find a similar issue and didn't find it.
• I have attached as much information as possible (screenshots, debug and exception logs, etc).

Description

Saved games often get corrupted when you chain-save, meaning create new saved games after loading a session.

(Create a save, load the save, and then create another save, load the new save, save more and so on)

This does not happen similarly in vanilla TS.

Chained saves are commonly used especially when people play difficult or long missions that are hard to beat in just one go, or with only 1 saved game. This bug makes such missions a poor experience as when losing to some AI attack in the late-game, the user's only valid saved game might be from half an hour or more prior.

Required Code (optional)

No response

Steps To Reproduce

This seems to be a common issue, giving several ways for reproducing this.

Method 1:

1. Start any multiplayer scenario, settings don't matter (can even be 0 AI players)
2. Deploy your MCV, build a Power Plant and a Barracks / Hand of Nod
3. Build a Light Infantry
4. Make the infantry force-fire at the ground
5. Without ever telling the infantry to stop, create a save and load it
6. Create another save and load it -> the game will give a swizzler error

Method 2:

1. Start any multiplayer scenario, pick Nod as your faction. Other settings don't matter.
2. Deploy your MCV, build a Power Plant and a Hand of Nod
3. Build a Rocket Infantry
4. Create a save and load it
5. Find a tree, force-fire at it and destroy it
6. Create another save and load it -> the game will give a swizzler error

Method 3:

1. Start the Firestorm Nod campaign
2. Save right during the intro
3. Load the save you made
4. Save again
5. Load the new save you just made
6. Keep repeating steps 4-5, iow. continue making new saves and loading them, without doing anything special in the game. The game will create a corrupted save typically after 2-10 cycles (sometimes even 2, often 4 is enough, but can take more), at which point loading will fail.

This has been preproduced with both the TS Client as well as Vanilla TS.

No interaction with units or other game mechanics is necessary for causing the save corruption issue. It also happens on other campaigns and missions, but for myself, based on quick testing, it seemed to happen fastest with the first Firestorm Nod mission.

Expected Behaviour

Chaining saved games should not lead to saved games getting corrupted.

Actual Behaviour

Chaining saved games makes them corrupted at some point.

Version

~b789478

Additional Context

I suspect this is a bug that was overlooked in the original extensions PR #896, because while people tested saves, chained saves most likely weren't tested enough. We first bumped into this while testing missions in DTA, in a "real-world" scenario.

From Bittah Commander on Mod Haven:

I tested chained saves with PR #898 and it also crashed
#889 didn't allow me to save and older ones can't be downloaded anymore

https://discord.com/channels/338055131666972683/826473223276986409/1058110660216639539

[Rampastring]

I looked a bit more into this.

The bug is caused by Vinifera creating duplicate class extension instances when you load a game. The old class extensions are loaded and new ones made on load, so if a save originally had, for example, 3 infantry and 3 infantry extensions, after loading there's 3 infantry and 6 infantry extensions in the game.

The reason why this happens is that _On_Load_Clear_Scenario_Intercept, hooked in to the game's Load_All function at 0x005D6BEC, is supposed to enable loading mode (Vinifera_PerformingLoad), which prevents Vinifera from creating new extensions on load.

However, another hook, _Load_Game_Check_Return_Value, currently makes it so that the original game's Load_All function at 0x005D6BE0 is never even called, which prevents the _On_Load_Clear_Scenario_Intercept hook from ever being executed. Instead, Vinifera executes its own version of Load_All, called Vinifera_Get_All.

Due to this Vinifera_PerformingLoad is set to false while loading and Vinifera creates duplicate class extension instances.

Simply adding Vinifera_PerformingLoad = true; to the beginning of the Vinifera_Get_All function fixes the bug.

[CCHyper]

Thanks for putting the time into hunting this down @Rampastring.

This has been fixed upstream now, so closing the issue.