From 1e6bb9e867330779428e2ff8e18f328ecdde7afe Mon Sep 17 00:00:00 2001 From: Stefan O'Rear Date: Mon, 9 Nov 2009 17:03:50 -0800 Subject: Check in the save compatibility mailing --- crawl-ref/docs/develop/save_compatibility.txt | 108 ++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 crawl-ref/docs/develop/save_compatibility.txt (limited to 'crawl-ref/docs/develop') diff --git a/crawl-ref/docs/develop/save_compatibility.txt b/crawl-ref/docs/develop/save_compatibility.txt new file mode 100644 index 0000000000..577d8d0d14 --- /dev/null +++ b/crawl-ref/docs/develop/save_compatibility.txt @@ -0,0 +1,108 @@ +Date: Mon, 9 Nov 2009 14:17:28 -0800 +From: Stefan O'Rear +To: crawl-ref-discuss@lists.sourceforge.net +Message-ID: <20091109221728.GA7668@localhost.localdomain> +Subject: [Crawl-ref-discuss] Savefile compatibility: how and why + +Robert asked me to write something for the list on the subject of +savefile compatibility, so here goes. + +1. WHY + +Having a savefile compatibility procedure is crucial in the stable +branches, as it allows applying bug fix releases in mid-game. It's +also important in the development trees, although we currently give +it far less attention than it deserves. The reason for this is that +properly testing many features, such as the recent portal refactors +that happened to make leaving Zot impossible, require a long game. +If it is impossible to upgrade in the middle of a game, then all bugs +for late-game features will be reported against old versions, need- +lessly complicating the bug fixing process. + +The situation is exacerbated by the public beta server (Marc Thoben's +crawl.develz.org), since it can only be updated site-wide; it cannot +be updated between games as there are almost always saved games on +the server, so it is simply updated (with loss of saves) rarely. And +rarely is bad. + +2. HOW + +If you are changing any of the code in tags.cc (which implements load +and save for savefiles), you probably need to be concerned about save +compatibility. Any code which uses the marshall* and unmarshall* +functions is, likewise, probably unsafe to simply change. Changing +the values of existing enums (for instance, adding a new option +before existing options) is not generally safe, as saves store the +numeric values of enum variables. + +By way of example, suppose you wanted to add a new field to the player +structure, you.xyzzy. Now you want to save this field, and without +breaking saves. To do this, find the functions tag_read_you and +tag_construct_you in tags.cc. Unfortunately, you can't just add +marshall and unmarshall calls, since if you get an old save, the wrong +value will be unmarshalled, synchronization is lost and things +fall apart. So first, add a new option to the tag_minor_version enum +in tags.h. Make sure the new option is at the end, and that you +update TAG_MINOR_VERSION to correspond to the new option. Now, any +savefile created by the new code, or any later savefile, will have a +minor version >= your new version number. So, make the marshall and +unmarshall code conditional on minorVersion, like so: + + if (minorVersion >= TAG_MINOR_JIYVA) + you.second_god_name = unmarshallString(th); + +... + + marshallString(th, you.second_god_name); + +Is this clear? If you want to change the representation of an existing +field, or delete an old field, use a check in the other direction for +saves created by *older* versions of Crawl; for instance, take this +code, which reads in either a Subversion or a Git revision number, +depending on version: + + if (minorVersion < TAG_MINOR_GITREV) + { + std::string rev_str = unmarshallString(th); + int rev_int = unmarshallLong(th); + + UNUSED(rev_str); + UNUSED(rev_int); + } + + if (minorVersion >= TAG_MINOR_GITREV) + { + std::string rev_str = unmarshallString(th); + UNUSED(rev_str); + } + +If you want to change an enum without breaking savefile compatibility, +the cardinal rule is that the numeric values of all existing constants +must not change. If you are adding an option, add it at the end; if +you are removing a value, leave a placeholder of some kind. Be sure +to test your code well, as there are sometimes obscure requirements on +what needs to be done for a placeholder. Case in point: if a +MUT_UNUSED_n placeholder exists in the mutation_type enumeration, but +no mutation definition exists in mutation_defs, it will work fine until +somebody plays with a Vampire, as generating the list of fully active +mutations requires looking at mutation definitions for all mutations. + +In extreme cases, it may not be possible to preserve load compatibility. +If this is the case, and please do this sparingly, you should increase +TAG_MAJOR_VERSION. This will ensure that old save files are correctly +rejected, instead of causing a crash. + +Historically TAG_MAJOR_VERSION has always been equal to the submajor +version of Crawl itself; however, this buys us very little, as users +do not need to identify Crawl save versions by contents under any +normal circumstance, and has cost us dearly in time spent debugging +startup crashes. Therefore we should not do it; TAG_MAJOR_VERSION +should be incremented on all incompatible changes. + +When TAG_MAJOR_VERSION is incremented, all existing TAG_MINOR_ checks +are no longer necessary and can be removed, along with deleting the +values of TAG_MINOR_ and restarting minor numbering from 1. + +Thank you for listening, I hope this was useful. + +Stefan -- cgit v1.2.3-54-g00ecf