The savegame version check is already redundant with the engine and mod version check.
If we would not use one common version tracking system (engine + mod version), then we would have to add these component versions all over the place.
Details
- Reviewers
wraitii - Commits
- rP20729: Delete Savegame format version, refs rP10454.
Notice the meaninglessness and the beauty of removing that from the code.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
The savegame version is the version of the savegame format, so it's not identical to the engine or mod version.
See the second sentence of the summary:
If we would not use one common version tracking system (engine + mod version), then we would have to add these component versions all over the place.
For instance we could add a replay format version, a simulation component version, a gamesetup version, a session verison, a selection panels version, a user config format version (that is bumped each time we add a hotkey).
Each time we do a change in the serialized part of the simulation, we change the savegame format but not bump the version number.
A file format (or network protocol) versioning system is only relevant if there can be more than one application. But pyrogenesis savegames are only loaded by pyrogenesis.
(It's common practice to add it to network protocols because one assumes that a protocol is used by different client implementations. Someone implementing a network client for 0AD that can work with many protocols of 0AD is highly unlikely too but still more likely than someone implementing a 0AD savegame loader application and keep compatibility with different savegame versions within the same release)
Like how likely is it that someone develops a new savegame version that can be used with an old 0ad version, or updates 0ad support of savegames of old versions?
Yesterday 2017-12-28 on irc:
22:51 < elexis> Philip: we were wondering if we could remove the savegame version or if that had any use case we didn't discover
22:52 < elexis> it was added in https://code.wildfiregames.com/rP10454
22:53 < elexis> (it still saves the engine version (release number) and mods)
22:54 < elexis> (SAVED_GAME_VERSION_MAJOR in SavedGame.cpp)
22:58 < Philip> I think version numbers are always useful
22:58 < Philip> or, rather, lack of version numbers often becomes a huge pain in the future
22:58 < Philip> and including them has very little cost, so they should almost always be included
23:00 < elexis> but how is the savegame different from any other file format, maps, simulation components, gui files
23:01 < elexis> if we only consider releases, the savegame format is covered by the engine version
23:02 < elexis> two savegames of different svn revisions are incompatible despite the same savegame version number
23:03 < elexis> can you mention an example where it would be a pain to not have it?
23:04 < elexis> *construct
23:07 < Philip> Oh, I didn't know someone had added engine_version
23:07 < Philip> Maybe that's sufficient (except for really old saves that didn't have engine_version, but that's unlikely to matter)
23:08 < elexis> mostly because the savegame version format never changed since
23:08 < elexis> and we needed an engine version for replays and such too
23:08 < Philip> Saved games are different from game/mod content files (maps, scripts, etc) because we expect users to upgrade the game and still have all their old saved games and have the game respond sensibly when they try to open them
23:09 < Philip> (either by saying it's incompatible, or ideally by making it work somehow)
23:09 < elexis> backwards compatibility is something we can't really achieve without adding tons of ugly code
23:09 < Philip> whereas the game engine should never be incompatible with the game content files, unless the installer has really messed up
23:10 < elexis> yes
23:12 < elexis> thanks for your feedback!
23:13 < Philip> I think replays and saved games have similar compatibility requirements (since people will save them for a long time, share them, etc)
23:16 < elexis> the engine version was added in https://code.wildfiregames.com/rP16906 in preparation for the replay compatibility filter
It seems to me we can perfectly handle incompatibilities by knowing that we broke save games in some given version of the engine, so I'd say this fits requirements.
I assume this doesn't break loading existing saved games.
Thanks for your guys feedback.
source/ps/SavedGame.cpp | ||
---|---|---|
39 ↗ | (On Diff #4668) | I leave this TODO in here for now. I guess it wants to tell us that the engine version check should be in C++ and the JS GUI should only display an error message based on that function, rather than having an own logic to check for the engine version in JS. |