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 public servers that support trunk games, since compatibility-breaking changes prevent players from upgrading to receive future bugfixes, and result in bug reports against obsolete versions. 2. Save compatibility architecture ---------------------------------- For save compatibility purposes, the actual version number (for example, 0.12.2 or 0.13-a0-2578-ged92a99) makes almost no difference. Instead, there are three version numbers (which we call “tags” for historic reasons) associated with each save and with each version of crawl: the character format, major version, and minor version. The character format tag is stored in the “character info” section of the save: the basic information such as species and level that is displayed in the save browser. If this section is changed in an incompatible way, the character format version would be incremented. However, Crawl would not even see the old saves; this would be rather bad (they might accidentally be overwritten, for example), so we have never made such a change: the format is still version 0. Appending new information to this section requires only incrementing the minor tag (see below), which is much safer. The major version tag is stored in each section of the save (each level, character, transiting monsters, and so on). Changing this version indicates that Crawl is no longer compatible with the old save: the save will be coloured red in the save browser and cannot be loaded. The major tag is never incremented within a release (e.g. between 0.12.0 and 0.12.1), only in trunk. We try to avoid doing this when we can, because it means trunk players will be unable to transfer their saves to newer versions to get bug fixes; it furthermore means that local players will have to keep the previous stable release installed until they have finished their games. The minor version tag is also stored in each section of the save, and is used to indicate changes that require special handling for loading old saves. The save-loading code if full of checks that say “only do this if the save’s minor tag is greater than N” or “initialize this new field if the minor tag is less than N”. Older versions of Crawl will refuse to load a save with a too-new minor tag (that is, we do not provide forward compatibility between versions), but newer versions will see the old tag and apply the appropriate fixups on load (which makes the save incompatible with the old version of crawl). The minor tag is reset to zero whenever the major tag changes. A major bump provides an opportunity to clean up the old save-compatibility code, since the old saves won’t be loadable anyway; and to remove or rearrange enumeration values that could not change without breaking compatibility. 3. Maintaining save compatibility --------------------------------- First, if you're adding a property to one of the actor, player or monster class, consider storing it in the props hash (declared in actor.h). Especially if the property is temporary or not applied to all monsters. Also, it won't create any save compatibility issues. 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 tag-version.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. 4. A few comments on tiles -------------------------- Whenever an old tile is removed without replacement, a new tile is added, or the order of tiles changes, saved games can look wonky when loaded. This happens because the tiles for floor and wall tiles get rolled once on level creation and are subsequently stored in the level files, so they don't change every time you reload the game. Any time a tile's number is changed, the actually displayed tile gets shifted a bit which can lead to floor looking like walls, staircases like shops and similar odd occurrences. Items and monsters that are cached out of sight may similarly be affected. This is not a big problem and in the development version, we don't even try to prevent this, but in the stable branches it should simply not happen. This means that we should never add new tiles for a minor release, unless said tiles replace the same amount of existing ones. If we expect that we might have to add new tiles in a stable branch later on, we should add placeholder tiles before the official release, that can later be replaced without affecting saved games. 5. A few comments on map caches and Lua markers ----------------------------------------------- Map and vault definitions are read from the relevant .des files and are stored in a binary format to prevent slow-down every time crawl starts. If new attributes or properties are added to vault definitions, save-compatibility needs to be ensured in much the same way as for normal saves. Until recently, the .des cache used a different version number to the main major/minor system. In theory, all that needs to be done now is to bump the minor version, which will cause the being-loaded .des cache file to be considered invalid, and thus rebuilt. When modifying a Lua marker, specifically when adding new options, etc, you'll need to likewise ensure save compatibility. The minor version is currently accessible by calling file.minor_version(reader). This will return a numeric value that has to be manually compared to the relevant minor tag. The current major version of the software is accessible by calling file.major_version(). There's no need to pass the reader in, as if the major version of a file mismatches, the game won't attempt to load it, and you will never get to a stage of being able to look for it. All Lua markers will need to be checked for minor-version checks when updating the major version. 6. Scheduling compatibility code for removal -------------------------------------------- In order to increase long-term maintainability, it is customary to wrap save compatibility code in #if blocks that check for the current value of TAG_MAJOR_VERSION: #if TAG_MAJOR_VERSION == 34 if (you.mutation[MUT_TELEPORT_CONTROL] == 1) you.mutation[MUT_TELEPORT_CONTROL] = 0; if (you.mutation[MUT_TRAMPLE_RESISTANCE] > 1) you.mutation[MUT_TRAMPLE_RESISTANCE] = 1; #endif Then when the major version is incremented and old saves are no longer supported, the compatibility code will not be compiled, and can be easily identified and removed for readability. For code that should run only for newer saves, the control structure itself, as well as any alternative code to handle older saves, should be protected by such #ifs; but the new code should be outside them. #if TAG_MAJOR_VERSION == 34 if (th.getMinorVersion() >= TAG_MINOR_LORC_TEMPERATURE) { #endif you.temperature = unmarshallFloat(th); // new save: keep this you.temperature_last = unmarshallFloat(th); #if TAG_MAJOR_VERSION == 34 } else { you.temperature = 0.0; you.temperature_last = 0.0; } #endif Then when the major version is incremented, the code will preprocess to: you.temperature = unmarshallFloat(th); you.temperature_last = unmarshallFloat(th); and the new behaviour will happen unconditionally. Other targets for #if TAG_MAJOR_VERSION checks include enumerators that are no longer used; and new enumerators that were added to the end of their enum for compatibility reasons but make more sense somewhere else in the list. In the latter case one would use a pair of #ifs, one for the current version and one for future versions: DNGN_ENTER_ABYSS, DNGN_EXIT_ABYSS, #if TAG_MAJOR_VERSION > 34 // new location DNGN_ABYSSAL_STAIR, #endif DNGN_STONE_ARCH, . . . DNGN_UNKNOWN_PORTAL, #if TAG_MAJOR_VERSION == 34 // old location DNGN_ABYSSAL_STAIR, DNGN_BADLY_SEALED_DOOR, #endif Note that the function tag_read_char() should generally *not* use the "#if TAG_MAJOR_VERSION" construct, as character chunks are intended to be visible (in the saved game browser) even across major save bumps. In that function, one would instead check the chunk's major version at runtime. Additionally, minor tag comparisons in tag_read_char() typically use numbers rather than tag_minor_version enumerators, so that they do not have to be rewritten on major version bumps. if (major > 34 || major == 34 && minor >= 29) crawl_state.map = unmarshallString2(th); A comment with the name of the enumerator would be apropos, but is not required: // TAG_MINOR_EXPLORE_MODE and TAG_MINOR_FIX_EXPLORE_MODE if (major == 34 && (minor >= 121 && minor < 130)) you.explore = unmarshallBoolean(th); 7. Git merges ------------- Whenever you do a merge or rebase of a branch that introduces new monsters, items, etc., it is very important to pay attention to enums. Unfortunately, git will sometimes automatically resolve the merge in exactly the way you didn't want, putting enumerators in their branch location rather than after the new trunk enumerators, all without giving any kind of conflict warning. If such a problem isn't caught before the servers are rebuilt, players will be left with buggy saves and the problems can be very hard to fix, with nasty kludges to try to guess the correct type of a monster. See for example the fixes in: 0.13-a0-2175-ga079a5c 0.14-a0-2325-gf687a29 To avoid situations like these, do not rely on conflict detection and resolution to merge enums correctly! Manually review all enum changes to make sure that nothing is inserted before an enumerator that already exists in trunk. You can look at the overall changes, with intervening history squashed, using a command like: ~$ git diff master..HEAD enum.h