save_compatibility.txt

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 occurences.
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 mis-matches, 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