New persistence strategy

[Elesbaan70]

Resolves #1502

This PR provides a new way to save data so that changes can be made to the model without breaking old savegames.

The complete task list for this project is at #1502.

Also see this wiki page: https://github.com/CitiesSkylinesMods/TMPE/wiki/Loading-and-Saving-Data

[kvakvs]

General question: Why XML, it might look like its a good fit, because named nodes, but that's where convenience ends. XML is hard to read for human, it is large, and it is not fast to write/parse and requires compression to be viable.

[Elesbaan70]

"Hard to read for human" is relative. It's certainly more readable than the binary format we were using before. Its readability has already been a valuable tool for me in working on #1392.

On your other points, the original choice was Json, but we were unable to find a trustworthy and full-featured Json serializer that was compatible with the particular combination of Unity and Mono versions we have to work with. Json.net is not compatible, and the Unity port of Json.net is abandoned and also nonfunctional.

The main advantage of XML is that we can use a known format and proven, stable code. If we rolled our own, we would be taking on a whole new level of maintenance responsibility.

One way we could reduce size is by using very short names, but this would pretty much destroy readability.

I'm open to other suggestions.......

[kvakvs]

Keep XML, the work is done and i do not have strong objections to the format. XML seems to be the stuff used ubiquitously in business C# projects. How big serialized data for a large map can be? There are some limitations.

[kvakvs]

Is save/load easily testable? Something like:

1. Create an object with randomized fields in allowed range (property based testing or a similar approach by coding random field values manually)
2. Serialize
3. Deserialize
4. Compare
5. For future versions try and keep an old serialized sample (one per old version) which could be tested that it still reads without errors

[Elesbaan70]

I will look into this. Probably best implemented in the unit test project.

[kvakvs]

Loading save file with features blocked, will it skip loading the incompatible sections?
When you save the game will the game lose that skipped data?

[Elesbaan70]

There is no attempt to preserve unsupported data through a load and re-save. That data is completely skipped.

I would oppose adding this as a requirement. Putting it into practice in a reliable way is far more complicated than it might appear on the surface.

When saving XML data, the save code has the option of marking its data as requiring a specific feature identified in an enum. It gets added to a featuresRequired collection.

When featuresRequired comes back non-empty, the framework transfers the required feature with the highest numeric value (which would normally mean the newest feature) to the featuresForbidden collection, clears featuresRequired, and repeats the save.

The above step is repeated until the save code either (1) is no longer adding items to featuresRequired, or (2) indicates that it is no longer able to save the data. In the latter case, this means that some older versions will simply lose this data, due to either a fundamental incompatibility or an intentional choice on our part.

Outside of dev and alpha builds, we will normally choose not to write old data, since we do not support older versions. We could decide to keep writing old data for some period of time, in case an emergency rollback is necessary.

[kvakvs]

A lot of new code has no xmldoc comments. Is this some copy pasted pattern code?
Not everyone knows the meaning of this new added stuff, me included, a lot of magical new interfaces and fields and properties, no idea what they all do.

[Elesbaan70]

Yes, I will do this. (It's not copy/pasted code, it's all new.)

I just felt a little urgency to get people's eyes on this, because @krzychu124 is waiting on it for Airports DLC enhancements.

btw, I will probably be cleaning up this framework a little more and publishing it separately as an open-source library, which I'll then use in other mod work. Whether TMPE switches over to that or continues to use its own version, is a decision that can be made when the time comes.

[kvakvs]

Can add tests to your checklist (at least run manual test once):

• Old save in new serialization code (should read)
• New save in old serialization code (should do what? ignore new and load old, or have no old and load blank settings)
• New save in new serialization code

[Elesbaan70]

Can add tests to your checklist (at least run manual test once):

Here's what I tested:

1. Transition from binary to XML:
   1. Reading old data:
      1. Load old data in new version. No XML found, so it reads the binary data.
   2. Saving new data with backward compatibility:
      1. Save new (XML) and old (binary) data to same file (default behavior until we decide we don't need to write the binary data anymore).
      2. Load in old version. It completely ignores the XML data.
      3. Load in new version. It sees the XML data and ignores the binary data.
2. Versioning in XML (currently only tested in Displaced lane support in traffic light simulation #1392 code):
   1. Reading old data:
      1. Load old data into new version. It detects the absence of the new feature, and reads the old way.
   2. Saving new data with backward compatibility:
      1. Save XML elements with and without the new feature.
      2. Load in old version. It skips elements with the new feature and reads elements without.
      3. Load in new version. It reads elements with the new feature and skips elements without.

[Elesbaan70]

@kvakvs If you weren't already aware, the original issue that prompted this was, as indicated in #1502, the inability to revise the save format without breaking older builds. While we don't support old versions, this still creates a problem for anyone (including ourselves) who wants to run test builds.

But since we don't support old versions, the intention is that once a feature is released to stable, the legacy code that writes data the old way gets removed. (The code to read the old way has to stay pretty much forever, though I imagine we'll identify exceptions on a case-by-case basis.)

[Elesbaan70]

I am converting this to draft, since TrafficManager.Util.Record was not addressed.