hash collision in the mapping.json

[cengelha]

Describe the bug
It was noticed that there is a hash collision in the mapping.json (see zencq/NomNom#165 for more).

In this case one is used for account data and one for actual saves. But as everything is put together in the same list, it is not possible to use the correct mapping without adding extra handling.

Expected behavior
Splitting the mapping for account data and saves into its own lists within the mapping.json would solve this very easily.

Result could look like this:

    "libMBIN_version": "4.52.0.2",
    "Mapping_account": [...],
    "Mapping_save": [...],

[monkeyman192]

I'm ok with a change like this. But the authors of all tools which use this data (which should hopefully be pretty much any save editor) will need to be made aware of this change (ideally by way of issues on github linking this issue).
If you want to raise issues on all relevant repositories so that authors may be notified of the potential changes so feedback may be gathered, that would be very useful.
Because changing the format would be a backward incompatible change we'd need to either create a new file with a different name which tool authors can opt into using and then deprecate the old format and remove once we are happy all the authors are no longer using it (keeping in mind that the tools may support older versions of the game which would mean they would need to support both the old and new versions depending on game version which may add some complexity)...

[Lenni009]

You could also put it all into one file, so you have

{
    "libMBIN_version": "4.52.0.2",
    "Mapping_account": [...],
    "Mapping_save": [...],
    "Mapping": [...]
}

I realise this is not exactly beautiful though.

[cengelha]

What I did in the meantime as a workaround (but may be the way to go to achieve this in a less impactful way) is splitting Mapping before the UserSettingsData. As UserSettingsData and all its members are added last, this works fine.

Only issue with the current file is that generic keys like >MX/Value are not added again below UserSettingsData if they were already in it due to the HashSet.

Could be solved with two separate HashSets and then concat them as lists. Even if some tools don't utilize that, it shouldn't be a problem if a handful entries are twice in the file.

var save_mapping = new HashSet<Tuple<string, string>>();
UpdateHashes(typeof(libMBIN.NMS.GameComponents.GcPlayerStateData), save_mapping);
// ...

// Also add the GcUserSettingsData class
var account_mapping = new HashSet<Tuple<string, string>>();
account_mapping.Add(new Tuple<string, string>(HashName("UserSettingsData"), "UserSettingsData"));
UpdateHashes(typeof(libMBIN.NMS.GameComponents.GcUserSettingsData), account_mapping);

main_data["libMBIN_version"] = libMBIN.Version.AssemblyVersion.ToString();
main_data["Mapping"] = save_mapping.ToList().Concat(account_mapping.ToList());

[monkeyman192]

I'm happy for you to open a PR with a suggested fix so that people may see the result in the generated artefacts of the PR, then people can give better comments on the format of the file.

[cengelha]

Just wanted to wait for feedback on the suggestion :) Created a PR with the code example above, keeping it in one list.

[cmkushnir]

Hmm, personally i probably would have had accountdata_mapping.json for accountdata.hg, and save_mapping.json for save*.hg. Although pc gog and steam seem to use a similar format for both accountdata.hg and save*.hg i'm not sure i would assume this would be true, or continue to be true, for other platforms e.g. game pass seems to store info in radically different set of files.