fix(mismatch): Fix mismatch due to custom upgrades in custom maps not properly resetting after match

[helmutbuhler]

Fixes: #1436

The UpgradeCenter class doesn't reset itself properly and as such can cause mismatches in a second match when the upgrades were modified due to custom scripts in a first match.

This PR fixes it by reloading the corresponding INI files during reset. Not exactly elegant, but it works!

[xezon]

I suggest take a look at INI_LOAD_CREATE_OVERRIDES to see if this problem can be properly fixed by using INI overrides. I suspect that there are more systems that currently do not handle overrides correctly.

map.ini is loaded with this special flag for this very purpose.

	sprintf(fullFledgeFilename, "%s\\map.ini", filename);
	if (TheFileSystem->doesFileExist(fullFledgeFilename)) {
		DEBUG_LOG(("Loading map.ini"));
		INI ini;
		ini.load( AsciiString(fullFledgeFilename), INI_LOAD_CREATE_OVERRIDES, NULL );
	}

[helmutbuhler]

Yes, I noticed that too, but I'm not sure what the purpose of this is. Is it just to save some loading time between games? If so, I'd suggest to remove this whole override idea and simply reload the ini state. That should simplify the code quite a bit and eliminate the possibility for mismatches due to oversights.

[xezon]

I expect that Overrides are crucial functionality because we also want to be able to override INI definitions in part, so for example just override the GLA Worker Object to do something unique in a map. Sure we could reload the entire INI stack as soon a definition needs removing, but I expect it is better to just be able to unload INI overrides on runtime.

[helmutbuhler]

Can't you already override part of the INI definitions in a map?
I don't understand, why do you think it's better?

[xezon]

I think I would need to study this more to make a better statement.

[Caball009]

fwiw, I created a draft PR to unify all relevant INI parsing with overrides: #2083

[xezon]

Yes, I noticed that too, but I'm not sure what the purpose of this is. Is it just to save some loading time between games? If so, I'd suggest to remove this whole override idea and simply reload the ini state. That should simplify the code quite a bit and eliminate the possibility for mismatches due to oversights.

Maybe yes.

Perhaps we can remove everything related to INI_LOAD_CREATE_OVERRIDES and INI_LOAD_MULTIFILE, and then load map ini files with INI_LOAD_OVERWRITE. When that happens, on next level load (back to menu), all INI related Systems would need to be fully destroyed and then reinitialized (see ini.loadFileDirectory amd initSubsystem). For the system it certainly is magnitutes more work than just dropping the Override, but as you say for maintenance it is much easier because Overrides would need no attention at all.

We would need to review if any INI parse functions refer to other systems that are dependent on INI state. If that is NOT the case (but it likely is), then we could just purge and reload all the systems that the map.ini file did actually touch. Otherwise, we would need to recreate and reload all systems beginning from the highest in the list up to the end. So if GameData.ini needed reloading, then almost all systems would need to be recreated, because it is very high up in the load order.

We need this kind of functionality anyway, if we would like to be able to load Mods on game client runtime. This would be great, because then one could join a Shockwave Mod or Contra Mod right from the lobby without restarting the game, as long as the Project files are present for the game to see. But that is for another day and has more complexity than just INI.

BUT, if the Override stuff was properly added everywhere then it would be the superior implementation, because it means the least work on runtime. Perhaps we first look into how we can make the Override code very simple to integrate to all INI related classes. If we can do, then it could live on as EA had intended. Ideally it requires just a few hooks into a class and then it would be good to go.

BUT, there is another BUT, the Override implementation is a bit weird in the way that if there was a second Override after the first Override, then the first Override would be copied into the second Override. This would happen if we were to allow multiple INI files in Custom Maps. To be more efficient, this would need changing, to specify the desired Override depth, before loading multiple INI files. Otherwise this can end up with long Override chains when there are many INI files touching the same systems. This problem would not exist when just doing Overwrites, instead of Overrides.

[xezon]

After thinking about this some more, I think here is what we can do:

Build a hybrid of Overwrite and Override, where we make backups of all initial loaded INI systems, and keep them in memory, then overwrite the used INI system when loading into a map, and then restoring the INI systems from the backup before the next level load. This way, the pristine INI state is restored from memory on each level load and the map can overwrite it any way it wants. This strategy is not intrusive into the various INI systems and only requires some manager class that deals with the recovery of INI systems from the backup. Ideally, all initial INI load stuff also exists in this manager class so that all these responsibilities are closely tied together.

BUT, there also is a problem with this approach, if one of the INI systems holds pointers to other INI systems. Then it cannot be copied easily. This would need checking first.