A little bit of savegame code cleanup; and a small format change to make

life easier (or rather, possible) for dump_savegame.  Should not break
saves (let me know if they do).

 - Fixed dump_savegame bug reading TAG_LEVEL.  Handle lua map_markers
   (by skipping over them) -- requires format change and minor version bump.

 - Consolidated YOU_MINOR_VERSION, LEVEL_MINOR_VERSION, GHOST_MINOR_VERSION
   into a single TAG_MINOR_VERSION, because otherwise versions can't be passed
   into data structures being deserialized (because they may be contained in
   both you and ghost, for example).

 - Clean out old code that pretends to restore other major versions,
   and some duplicate code that pretends level loading and general
   tagged file loading are different.  (Left ghosts alone because they
   really do do something different, slightly)




git-svn-id: https://crawl-ref.svn.sourceforge.net/svnroot/crawl-ref/trunk@4420 c06c8d41-db1a-0410-9941-cceddc491573

8 files changed, 140 insertions, 153 deletions

diff --git a/crawl-ref/source/externs.h b/crawl-ref/source/externs.h
index 6ebfaff787..c157fb7744 100644
--- a/crawl-ref/source/externs.h
+++ b/crawl-ref/source/externs.h
@@ -1331,7 +1331,7 @@ public:
     void clear();
 
     void write(writer &) const;
-    void read(reader &);
+    void read(reader &, int minorVersion);
 
 private:
     typedef std::multimap<coord_def, map_marker *> dgn_marker_map;
diff --git a/crawl-ref/source/files.cc b/crawl-ref/source/files.cc
index c1ffa33500..41ceddfe99 100644
--- a/crawl-ref/source/files.cc
+++ b/crawl-ref/source/files.cc
@@ -107,16 +107,22 @@
 #endif
 #endif
 
-void save_level(int level_saved, level_area_type lt,
-                branch_type where_were_you);
+static void save_level(int level_saved, level_area_type lt,
+                       branch_type where_were_you);
 
-#define GHOST_MINOR_VERSION 1
-#define LEVEL_MINOR_VERSION 1
+static bool _get_and_validate_version(FILE *restoreFile, char& major, char& minor,
+                                      std::string* reason=0);
 
-// 1: starting version
-// 2: append piety_hysteresis to TAG_YOU
-// 3: add quiver info.
-#define YOU_MINOR_VERSION   3
+
+static bool determine_ghost_version( FILE *ghostFile,
+                                     char &majorVersion, char &minorVersion );
+
+static void restore_ghost_version( FILE *ghostFile, char major, char minor );
+
+static void restore_tagged_file( FILE *restoreFile, int fileType,
+                                 char minorVersion );
+
+static void load_ghost();
 
 const short GHOST_SIGNATURE = short( 0xDC55 );
 
@@ -136,29 +142,6 @@ static void redraw_all(void)
         REDRAW_LINE_1_MASK | REDRAW_LINE_2_MASK | REDRAW_LINE_3_MASK;
 }
 
-static bool determine_version( FILE *restoreFile,
-                               char &majorVersion, char &minorVersion );
-
-static void restore_version( FILE *restoreFile,
-                             char majorVersion, char minorVersion );
-
-static bool determine_level_version( FILE *levelFile,
-                                     char &majorVersion, char &minorVersion );
-
-static void restore_level_version( FILE *levelFile,
-                                   char majorVersion, char minorVersion );
-
-static bool determine_ghost_version( FILE *ghostFile,
-                                     char &majorVersion, char &minorVersion );
-
-static void restore_ghost_version( FILE *ghostFile,
-                                   char majorVersion, char minorVersion );
-
-static void restore_tagged_file( FILE *restoreFile, int fileType,
-                                 char minorVersion );
-
-static void load_ghost();
-
 static std::string uid_as_string()
 {
 #ifdef MULTIUSER
@@ -207,11 +190,10 @@ player_save_info read_character_info(const std::string &savefile)
     if (!charf)
         return fromfile;
 
-    char majorVersion = 0;
-    char minorVersion = 0;
+    char majorVersion;
+    char minorVersion;
 
-    if (determine_version(charf, majorVersion, minorVersion)
-        && majorVersion == SAVE_MAJOR_VERSION)
+    if (_get_and_validate_version(charf, majorVersion, minorVersion))
     {
         // backup before we clobber "you"
         const player backup(you);
@@ -647,7 +629,7 @@ std::string make_filename( const char *prefix, int level, branch_type where,
                                  isGhost );
 }
 
-static void write_version( FILE *dataFile, int majorVersion, int minorVersion,
+static void _write_version( FILE *dataFile, int majorVersion, int minorVersion,
                            bool extended_version )
 {
     // write version
@@ -675,21 +657,23 @@ static void write_version( FILE *dataFile, int majorVersion, int minorVersion,
     }
 }
 
-static void write_tagged_file( FILE *dataFile, char majorVersion,
-                               char minorVersion, int fileType,
-                               bool extended_version = false )
+static void _write_tagged_file( FILE *outf, int fileType,
+                                bool extended_version = false )
 {
     // find all relevant tags
     char tags[NUM_TAGS];
     tag_set_expected(tags, fileType);
 
-    write_version( dataFile, majorVersion, minorVersion, extended_version );
+    _write_version( outf, TAG_MAJOR_VERSION, TAG_MINOR_VERSION,
+                    extended_version );
 
     // all other tags
     for (int i = 1; i < NUM_TAGS; i++)
     {
         if (tags[i] == 1)
-            tag_write((tag_type)i, dataFile);
+        {
+            tag_write((tag_type)i, outf);
+        }
     }
 }
 
@@ -701,14 +685,10 @@ bool travel_load_map( branch_type branch, int absdepth )
     if (!levelFile)
         return false;
 
-    // BEGIN -- must load the old level : pre-load tasks
-
-    // LOAD various tags
     char majorVersion;
     char minorVersion;
 
-    if (!determine_level_version( levelFile, majorVersion, minorVersion )
-            || majorVersion != SAVE_MAJOR_VERSION)
+    if (!_get_and_validate_version( levelFile, majorVersion, minorVersion ))
     {
         fclose(levelFile);
         return false;
@@ -1051,15 +1031,17 @@ bool load( dungeon_feature_type stair_taken, load_mode_type load_mode,
         // BEGIN -- must load the old level : pre-load tasks
 
         // LOAD various tags
-        char majorVersion = 0;
-        char minorVersion = 0;
+        char majorVersion;
+        char minorVersion;
 
-        if (!determine_level_version( levelFile, majorVersion, minorVersion ))
+        std::string reason;
+        if (!_get_and_validate_version( levelFile, majorVersion, minorVersion, &reason ))
         {
-            end(-1, false, "\nLevel file appears to be invalid.\n");
+            end(-1, false, "\nLevel file is invalid.  %s\n",
+                reason.c_str());
         }
 
-        restore_level_version( levelFile, majorVersion, minorVersion );
+        restore_tagged_file(levelFile, TAGTYPE_LEVEL, minorVersion);
 
         // sanity check - EOF
         if (!feof( levelFile ))
@@ -1232,8 +1214,7 @@ void save_level(int level_saved, level_area_type old_ltype,
     // nail all items to the ground
     fix_item_coordinates();
 
-    write_tagged_file( saveFile, SAVE_MAJOR_VERSION,
-                       LEVEL_MINOR_VERSION, TAGTYPE_LEVEL );
+    _write_tagged_file( saveFile, TAGTYPE_LEVEL );
 
     fclose(saveFile);
 
@@ -1314,8 +1295,7 @@ void save_game(bool leave_game, const char *farewellmsg)
     if (!charf)
         end(-1, true, "Unable to open \"%s\" for writing!\n", charFile.c_str());
 
-    write_tagged_file( charf, SAVE_MAJOR_VERSION,
-                       YOU_MINOR_VERSION, TAGTYPE_PLAYER );
+    _write_tagged_file( charf, TAGTYPE_PLAYER );
 
     fclose(charf);
     DO_CHMOD_PRIVATE(charFile.c_str());
@@ -1386,8 +1366,7 @@ void load_ghost(void)
         return;
     }
 
-    if (majorVersion != SAVE_MAJOR_VERSION
-        || minorVersion != GHOST_MINOR_VERSION)
+    if (majorVersion != TAG_MAJOR_VERSION || minorVersion > TAG_MINOR_VERSION)
     {
 
         fclose(gfile);
@@ -1439,13 +1418,13 @@ void restore_game(void)
     if (!charf )
         end(-1, true, "Unable to open %s for reading!\n", charFile.c_str() );
 
-    char majorVersion = 0;
-    char minorVersion = 0;
-
-    if (!determine_version(charf, majorVersion, minorVersion))
-        end(-1, false, "\nSavefile appears to be invalid.\n");
+    char majorVersion;
+    char minorVersion;
+    std::string reason;
+    if (!_get_and_validate_version(charf, majorVersion, minorVersion, &reason))
+        end(-1, false, "\nSave file is invalid.  %s\n", reason.c_str());
 
-    restore_version(charf, majorVersion, minorVersion);
+    restore_tagged_file(charf, TAGTYPE_PLAYER, minorVersion);
 
     // sanity check - EOF
     if (!feof(charf))
@@ -1602,46 +1581,39 @@ bool apply_to_all_dungeons(bool (*applicator)())
     return success;
 }
 
-static bool determine_version( FILE *restoreFile,
-                               char &majorVersion, char &minorVersion )
+static bool _get_and_validate_version(FILE *restoreFile, char &major, char &minor,
+                                      std::string* reason)
 {
+    std::string dummy;
+    if (reason == 0) reason = &dummy;
+
     // read first two bytes.
     char buf[2];
     if (read2(restoreFile, buf, 2) != 2)
+    {
+        major = minor = -1;
+        *reason = "File is corrupt.";
         return false;               // empty file?
+    }
 
-    // otherwise, read version and validate.
-    majorVersion = buf[0];
-    minorVersion = buf[1];
-
-    if (majorVersion == SAVE_MAJOR_VERSION)
-        return true;
-
-    return false;   // if it's not 0, no idea
-}
+    major = buf[0];
+    minor = buf[1];
 
-static void restore_version( FILE *restoreFile,
-                             char majorVersion, char minorVersion )
-{
-    // assuming the following check can be removed once we can read all
-    // savefile versions.
-    if (majorVersion != SAVE_MAJOR_VERSION)
+    if (major != TAG_MAJOR_VERSION)
     {
-        end(-1, false, "\nSorry, this release cannot read a v%d.%d savefile.\n",
-            majorVersion, minorVersion);
+        *reason = make_stringf("Major version mismatch: %d (want %d).", major, TAG_MAJOR_VERSION);
+        return false;
     }
 
-    switch(majorVersion)
+    if (minor >  TAG_MINOR_VERSION)
     {
-        case SAVE_MAJOR_VERSION:
-            restore_tagged_file(restoreFile, TAGTYPE_PLAYER, minorVersion);
-            break;
-        default:
-            break;
+        *reason = make_stringf("Minor version mismatch: %d (want <= %d).", minor, TAG_MINOR_VERSION);
+        return false;
     }
+
+    return true;
 }
 
-// generic v4 restore function
 static void restore_tagged_file( FILE *restoreFile, int fileType,
                                  char minorVersion )
 {
@@ -1665,46 +1637,6 @@ static void restore_tagged_file( FILE *restoreFile, int fileType,
             tag_missing(i, minorVersion);
 }
 
-static bool determine_level_version( FILE *levelFile,
-                                     char &majorVersion, char &minorVersion )
-{
-    // read first two bytes.
-    char buf[2];
-    if (read2(levelFile, buf, 2) != 2)
-        return false;               // empty file?
-
-    // otherwise, read version and validate.
-    majorVersion = buf[0];
-    minorVersion = buf[1];
-
-    if (majorVersion == SAVE_MAJOR_VERSION)
-        return true;
-
-    return false;   // if its not SAVE_MAJOR_VERSION, no idea
-}
-
-static void restore_level_version( FILE *levelFile,
-                                   char majorVersion, char minorVersion )
-{
-    // assuming the following check can be removed once we can read all
-    // savefile versions.
-    if (majorVersion != SAVE_MAJOR_VERSION)
-    {
-        end(-1, false,
-            "\nSorry, this release cannot read a v%d.%d level file.\n",
-            majorVersion, minorVersion);
-    }
-
-    switch(majorVersion)
-    {
-        case SAVE_MAJOR_VERSION:
-            restore_tagged_file(levelFile, TAGTYPE_LEVEL, minorVersion);
-            break;
-        default:
-            break;
-    }
-}
-
 static bool determine_ghost_version( FILE *ghostFile,
                                      char &majorVersion, char &minorVersion )
 {
@@ -1722,15 +1654,15 @@ static bool determine_ghost_version( FILE *ghostFile,
     if (unmarshallShort(inf) != GHOST_SIGNATURE)
         return (false);
 
-    if (majorVersion == SAVE_MAJOR_VERSION
-        && minorVersion <= GHOST_MINOR_VERSION)
+    if (majorVersion == TAG_MAJOR_VERSION
+        && minorVersion <= TAG_MINOR_VERSION)
     {
         // Discard three more 32-bit words of padding.
         inf.read(NULL, 3*4);
         return !feof(ghostFile);
     }
 
-    return false;   // if its not SAVE_MAJOR_VERSION, no idea!
+    return false;   // if its not TAG_MAJOR_VERSION, no idea!
 }
 
 static void restore_ghost_version( FILE *ghostFile,
@@ -1738,7 +1670,7 @@ static void restore_ghost_version( FILE *ghostFile,
 {
     switch(majorVersion)
     {
-    case SAVE_MAJOR_VERSION:
+    case TAG_MAJOR_VERSION:
         restore_tagged_file(ghostFile, TAGTYPE_GHOST, minorVersion);
         break;
     default:
@@ -1776,9 +1708,7 @@ void save_ghost( bool force )
         return;
     }
 
-    write_tagged_file( gfile, SAVE_MAJOR_VERSION,
-                       GHOST_MINOR_VERSION, TAGTYPE_GHOST,
-                       true );
+    _write_tagged_file( gfile, TAGTYPE_GHOST, true );
 
     lk_close(gfile, "wb", cha_fil);
 
@@ -1789,7 +1719,7 @@ void save_ghost( bool force )
     DO_CHMOD_PRIVATE(cha_fil.c_str());
 }                               // end save_ghost()
 
-
+// XXX: remove?
 void generate_random_demon()
 {
     int rdem = 0;
diff --git a/crawl-ref/source/mapmark.cc b/crawl-ref/source/mapmark.cc
index 5c7a008d49..3ecbe9afd1 100644
--- a/crawl-ref/source/mapmark.cc
+++ b/crawl-ref/source/mapmark.cc
@@ -733,22 +733,46 @@ void map_markers::clear()
     markers.clear();
 }
 
+static const long MARKERS_COOKY = 0x17742C32;
 void map_markers::write(writer &outf) const
 {
-    // how many markers
+    marshallLong(outf, MARKERS_COOKY);
+
+    std::vector<unsigned char> buf;
+
     marshallShort(outf, markers.size());
     for (dgn_marker_map::const_iterator i = markers.begin();
          i != markers.end(); ++i)
     {
-        i->second->write(outf);
+        buf.clear();
+        writer tmp_outf(&buf);
+        i->second->write(tmp_outf);
+
+        // Write the marker data, prefixed by a size
+        marshallLong(outf, buf.size());
+        outf.write(&buf[0], buf.size());
     }
 }
 
-void map_markers::read(reader &inf)
+void map_markers::read(reader &inf, int minorVersion)
 {
     clear();
+
+    if (minorVersion >= TAG_MINOR_MAPMARK)
+    {
+        const long cooky = unmarshallLong(inf);
+        ASSERT(cooky == MARKERS_COOKY);
+    }
+
     const int nmarkers = unmarshallShort(inf);
     for (int i = 0; i < nmarkers; ++i)
+    {
+        // used by tools
+        if (minorVersion >= TAG_MINOR_MAPMARK)
+            unmarshallLong(inf);
         if (map_marker *mark = map_marker::read_marker(inf))
+        {
             add(mark);
+        }
+    }
 }
diff --git a/crawl-ref/source/python/crawl/tags.py b/crawl-ref/source/python/crawl/tags.py
index 4ba9c39c28..7548603d32 100644
--- a/crawl-ref/source/python/crawl/tags.py
+++ b/crawl-ref/source/python/crawl/tags.py
@@ -12,6 +12,9 @@ import binfile
 # Some constants and things
 # ----------------------------------------------------------------------
 
+TAG_MAJOR_VERSION = 5
+TAG_MINOR_VERSION = 4
+
 NUM_MONSTER_SPELL_SLOTS = 6
 NUM_MONSTER_SLOTS = 10
 MONS_PLAYER_GHOST = 400
@@ -212,7 +215,11 @@ class Quiver(object):
 def Coord(f): return f.stream('HH')
 
 class MapMarker(object):
-    def __init__(self, f):
+    def __init__(self, f, minor):
+        if minor >= 4:
+            expected_size = f.stream1('I')
+
+        num_read = -f.file.tell()
         self.mark_type = mark_type = f.stream1('H')
         if mark_type == 0:      # map_feature_marker
             self.read_base(f)
@@ -233,6 +240,11 @@ class MapMarker(object):
         else:
             assert "Unknown map marker type %d" % mark_type
 
+        num_read += f.file.tell()
+
+        if minor >= 4:
+            assert num_read == expected_size
+
     def read_base(self, f):
         self.coord = Coord(f)
 
@@ -246,6 +258,10 @@ class TaggedFile(object):
         f.byteorder = '>'
         self.f = f
         self.major, self.minor = f.stream('bb')
+        print '  version %d.%d' % (self.major, self.minor)
+        if (self.major != TAG_MAJOR_VERSION or
+            self.minor > TAG_MINOR_VERSION):
+            print "  WARNING: Cannot handle this version!"
         self.tags = dict( self._gen_tags() )
 
     def _gen_tags(self):
@@ -258,9 +274,9 @@ class TaggedFile(object):
             tag_name = TAGS_NAMES[tag_id]
             try:  constructor = TAG_TO_CLASS[tag_name]
             except KeyError:
-                print "  Found %s (currently unsupported)" % tag_name
+                print "  Skipping %s" % tag_name
             else:
-                print "  Found %s (parsing)" % tag_name
+                print "  Parsing %s" % tag_name
                 sub_reader = binfile.reader(StringIO.StringIO(data))
                 sub_reader.byteorder = '>'
                 data = constructor(sub_reader, self.minor)
@@ -414,15 +430,18 @@ class TagLEVEL(TagBase):
         self.grid = [ f.stream('BHHHHH')
                       for i in xrange(self.gx)
                       for j in xrange(self.gy) ]
-
         expected = self.gx * self.gy
         self.grid_colours = list(gen_run_length_decode(f, 'B', expected))
 
+        self.cloud_no = f.stream1('H')
         self.clouds = stream_array(f, 'H', 'BBBHBH', limit=1000)
         self.shops = stream_array(f, 'B', 'BBBBBBBB', limit=15)
         self.sanctuary = Coord(f)
         self.sanctuary_time = f.stream1('B')
-        self.markers = [ MapMarker(f) for x in xrange(f.stream1('H')) ]
+        if minor >= 4:
+            cooky = f.stream1('I')
+            assert cooky == 0x17742C32
+        self.markers = [ MapMarker(f, minor) for x in xrange(f.stream1('H')) ]
         assert_end(f.file)
 
         
diff --git a/crawl-ref/source/tags.cc b/crawl-ref/source/tags.cc
index 5333899f1a..7cad040f15 100644
--- a/crawl-ref/source/tags.cc
+++ b/crawl-ref/source/tags.cc
@@ -1326,7 +1326,8 @@ static void tag_read_you(reader &th, char minorVersion)
 
     // old: quiver info.  Discard it.
     count_c = unmarshallByte(th);
-    if (minorVersion >= 3) ASSERT(count_c == 0);
+    if (minorVersion >= TAG_MINOR_QUIVER)
+        ASSERT(count_c == 0);
     for (j = 0; j < count_c; ++j)
     {
         unmarshallByte(th);
@@ -1384,10 +1385,10 @@ static void tag_read_you(reader &th, char minorVersion)
     for (i = 0; i < count_c; i++)
          you.beheld_by.push_back(unmarshallByte(th));
 
-    if (minorVersion >= 2)
+    if (minorVersion >= TAG_MINOR_PIETY)
         you.piety_hysteresis = unmarshallByte(th);
 
-    if (minorVersion >= 3)
+    if (minorVersion >= TAG_MINOR_QUIVER)
         you.m_quiver->load(th);
 }
 
@@ -1986,7 +1987,7 @@ static void tag_read_level( reader &th, char minorVersion )
     unmarshallCoord(th, env.sanctuary_pos);
     env.sanctuary_time = unmarshallByte(th);
 
-    env.markers.read(th);
+    env.markers.read(th, minorVersion);
 }
 
 static void tag_read_level_items(reader &th, char minorVersion)
diff --git a/crawl-ref/source/tags.h b/crawl-ref/source/tags.h
index 385f25cfa2..ff10c242e3 100644
--- a/crawl-ref/source/tags.h
+++ b/crawl-ref/source/tags.h
@@ -41,6 +41,21 @@ enum tag_file_type   // file types supported by tag system
     TAGTYPE_PLAYER_NAME         // Used only to read the player name
 };
 
+enum tag_major_version
+{
+    TAG_MAJOR_START = 5,
+    TAG_MAJOR_VERSION = 5
+};
+
+enum tag_minor_version
+{
+    TAG_MINOR_PIETY = 2,        // Added piety_hysteresis
+    TAG_MINOR_QUIVER = 3,       // Added quiver
+    TAG_MINOR_MAPMARK = 4,      // Added sizes to map markers
+    TAG_MINOR_VERSION = 4       // Current version
+};
+
+
 /* ***********************************************************************
  * writer API
  * *********************************************************************** */
diff --git a/crawl-ref/source/util/dump_savegame b/crawl-ref/source/util/dump_savegame
index 9069c38ac4..a29c36c733 100755
--- a/crawl-ref/source/util/dump_savegame
+++ b/crawl-ref/source/util/dump_savegame
@@ -32,7 +32,7 @@ def process_zip(opts, fn):
                 save = crawl.tags.TaggedFile(StringIO(zip.read(n)))
             except Exception, e:
                 print "  Failed (not a tag file?)"
-                continue
+                raise
 
             process_file(opts, save)
 
diff --git a/crawl-ref/source/version.h b/crawl-ref/source/version.h
index fba3edbf2b..c34cc1eb8d 100644
--- a/crawl-ref/source/version.h
+++ b/crawl-ref/source/version.h
@@ -58,6 +58,4 @@
 #define VERSION_DETAIL BUILD_DATE
 #endif
 
-#define SAVE_MAJOR_VERSION 5
-
 #endif