Check in the save compatibility mailing

1 files changed, 108 insertions, 0 deletions

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 <stefanor@cox.net>
+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