CitiesSkylinesMods / TMPE

Cities: Skylines Traffic Manager: President Edition
https://steamcommunity.com/sharedfiles/filedetails/?id=1637663252
MIT License
563 stars 85 forks source link

save options in xml format #1704

Open kianzarrin opened 1 year ago

kianzarrin commented 1 year ago

fixes #562

saved game options are serialized as xml and then converted to byte[] data.

TMPE.zip

kvakvs commented 1 year ago

I see legacy save is deleted - so this will upgrade legacy savefiles to XML and no going back? Do we ever need users to be able to go back? Probably its safe to say they don't need to load saves in old TM:PE

krzychu124 commented 1 year ago

I see legacy save is deleted - so this will upgrade legacy savefiles to XML and no going back? Do we ever need users to be able to go back? Probably its safe to say they don't need to load saves in old TM:PE

IMO, it's not the best idea but not so bad either. Depends on how it will be released. It can be done similar like how deprecation in software is done. As an example, first something is marked as deprecated and then removed at some point in the future, The important note is that deprecated thing still works so in our case we should still save data the old way additionally just to be able to switch back in case of problems. Not too long, maybe just for one version, but still. Remember, we have Test and Stable, recently they were released in sync (because of the game update required compatible version of mod) but as of migration it might be better to test things first to at least see if everything works in more random conditions, but also... can be released again like recently - in sync, so basically no way of going back.

Upcoming game update (Dec 13, 2022) doesn't break anything in the mod (as opposed to previous patches) so theoretically user could use previous version but if I release both Test and Stable with the same version they won't have a chance to do so 😂 Anyways, I'll try to retest this today again and we will see.

kianzarrin commented 1 year ago

@krzychu124 @kvakvs to make it forward compatible I need to first save legacy and then save xml and then place it in the byte array. loading needs to skip over the legacy part.

its not hard but could be a bit confusing and annoying to read.

should I go ahead and do it?

kvakvs commented 1 year ago

@krzychu124 @kvakvs to make it forward compatible I need to first save legacy and then save xml and then place it in the byte array. loading needs to skip over the legacy part. its not hard but could be a bit confusing and annoying to read. should I go ahead and do it?

Wait for Krzychu's opinion, i think we do not need dual save format. Is that a realistic scenario when users from Epic games/where older game version is running, try and load new XML-only savegames?

kianzarrin commented 1 year ago

Is that a realistic scenario when users from Epic games/where older game version is running, try and load new XML-only savegames?

Legacy load is supported.

kvakvs commented 1 year ago

Is that a realistic scenario when users from Epic games/where older game version is running, try and load new XML-only savegames?

Legacy load is supported.

Legacy save is deleted, i wonder if that is a very rare scenario when it is necessary and we can ignore it.

kianzarrin commented 1 year ago

oh now I understand what you mean! I think we can expect ppl to run latest TMPE after the release. specially if they want to run new saves.

krzychu124 commented 1 year ago

So.. we partially need #1709 because of changes you've made in #1702, now the UI is not updated when value changes. Previously UI components were loading settings updating UI state but also setting the Options values which probably is not the case currently

On Hot-reload order of calls (read settings, init Settings screen) is different -> first load settings, then generate SettingsUI

kianzarrin commented 1 year ago

Convert to draft because I have not tested my fix.

kianzarrin commented 1 year ago

@krzychu124 do I have to use IObserver/IObservable? or the current approach is fine?