NameKeyGenerator and order of initialization of singleton classes are tightly coupled to CRC

[Caball009]

• fix(filesystem): Decouple File System from NameKeyGenerator and its CRC influence #1516 provided an incorrect workaround for a bug that had come to light after a prior unification effort without diving into the root cause.

The NameKeyGenerator is used in lots of places to provide a unique value for unique strings. It creates a hash for a given string, stores the hash internally and returns a monotonic / sequential ID associated with that hash.

TheScienceStore and TheUpgradeCenter rely on the NameKeyGenerator to provide a unique ID for the objects that they store. This way whenever a science or upgrade is used a client only has to send the ID to other clients and not the full name. As a result, sciences and upgrades must have the same ID for all clients or the game will mismatch eventually.

Given the sequential nature of the name key IDs, this means that the sciences and upgrades MUST have been hashed in the exact same order for all clients. There are two issues with this:

1. Reordering the sciences or upgrades internally in the ini files breaks the ID order.
2. Inserting new, even if unused, sciences or upgrades to the ini files breaks the ID order. In theory, it should be fine to put new items at the end, though that has other consequences.

Unfortunately, TheScienceStore and TheUpgradeCenter are far from the only classes to use the NameKeyGenerator, and more importantly, they're not initialized right after each other. This is what the internal name key ID values are currently during the initialization in GameEngine::init (Zero Hour):

index, ID before - after

 0, 0000 - 0001   TheNameKeyGenerator->init
 1, 0001 - 0097 TheScienceStore (!)
 2, 0097 - 0100   3 dummy strings
 3, 0100 - 0389 TheFunctionLexicon
 4, 0389 - 0606 TheModuleFactory
 5, 0606 - 0633 ThePlayerTemplateStore
 6, 0633 - 1062 TheFXListStore
 7, 1062 - 1425 TheWeaponStore
 8, 1425 - 1719 TheObjectCreationListStore
 9, 1719 - 1901 TheLocomotorStore
10, 1901 - 1912 TheDamageFXStore
11, 1912 - 1965 TheArmorStore
12, 1965 - 2265 TheThingFactory
13, 2265 - 2351 TheUpgradeCenter (!)
14, 2351 - 3841 TheGameClient
15, 3841 - 4287 TheGameLogic
16, 4287 - 4288 TheCrateSystem
17, 4288 - 4289 TheMapCache
18, 4289 - 4290   resetSubsystems

The three dummy strings (based on unmodded files):

Data\<registry language>\Language9x.ini
Data\Audio\Tracks\<registry language>\GLA_02.mp3
Data\Audio\Tracks\GLA_02.mp3

Originally these three strings were hashed here: string 1, string 2 and 3, but this was removed by us.
#1516 hardcoded these three strings to keep the ID in sync with retail: before and after. Only the increasing of the ID is important here, the strings themselves are irrelevant.

Hashing the three dummy strings ensured that the first hash for TheUpgradeCenter started at name key ID 2265 as retail expects from unmodded files. Unfortunately, hardcoding the IDs doesn't work for modded files. This became apparent after this replay started to mismatch after #1516 at 4:00: TEOD_0986_P11.zip mod files This mod expects the three strings to have IDs 265 - 267.

A superficial fix could just add three dummy strings after the initialization of TheScienceStore. The fix for retail compatibility will probably do just that. This doesn't address the fragile reliance on the name key ID order, though. If any code adds or removes a new hash between the initialization of TheScienceStore and TheUpgradeCenter, the ID values are different for the upgrades. We cannot hardcode name key IDs because they may be different for modded files. For compatibility with retail, with unmodded files, it's paramount that TheScienceStore starts with name key ID 1 and TheUpgradeCenter with name key ID 2265.

I checked with our first usable branch and those ID values are the same there. There's some discrepancy for TheGameClient, but that comes after the upgrades, so it makes no difference. I also tested this to bruteforce any issue:

assert(TheNameKeyGenerator->getID() == 1);

initSubsystem(TheScienceStore
initSubsystem(TheAudio // TheUpgradeCenter relies on TheAudio having been initialized
TheNameKeyGenerator->addDummyStringsUntil_ID_Equals(2265);
initSubsystem(TheUpgradeCenter
TheNameKeyGenerator->addDummyStringsUntil_ID_Equals(7000);

// initialize all other singleton classes

This passed the Golden Replay 1 and 300 / 300 other replays, which makes me confident that different name key IDs can only cause a CRC issue for the sciences and upgrades.

[xezon]

Very good research. Yes, filling up the 3 dummy strings after TheScienceStore looks like a good compromise for now.

[Caball009]

FYI: I noticed that the World Builder has its own logic for the initialization of the singleton classes / subsystems. I don't know if that's relevant here, and I've only looked at what Zero Hour does.

[xezon]

CRC is no concern for World Builder because it has no Multiplayer.

[Caball009]

I don't know how it stores sciences and upgrades for the scripts. I assume it does not use the name key generator for that, though, or we should already have noticed the discrepancy in name key ID values between the WB and the game.