widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

SirVer has proposed merging lp:~widelands-dev/widelands/map_compatibility into lp:widelands.

Commit message:
Fixes loading of old maps.

- Do not load objective data in the editor. Objectives are now only used in savegames, so no need to try loading it into the editor.
- In the old days, we required that every zip file we write contains a directory that has the same names as the zip file. For example, each file in blub.wmf (the zip file) will start with "blub.wmf/".  Our current code expected the same thing. Problem is now, if your file is called "Blub Blah.wmf" and the base folder inside is called "BlubBlah.wmf", we are unable to load the map. This changes the code to actually find the common prefix of all files inside the Zip file, which is still a heuristic, but a better one.
- Removes support for multiline strings in profiles. We never use them since we moved scenarios to Lua, but it led to problems if users used "'" or '"' in their description strings for maps. 

Tested by loading all shipped maps and all maps uploaded to the homepage into the editor.

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/map_compatibility/+merge/302869
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/map_compatibility into lp:widelands.

=== modified file 'src/io/filesystem/zip_filesystem.cc'
--- src/io/filesystem/zip_filesystem.cc	2016-08-04 15:49:05 +0000
+++ src/io/filesystem/zip_filesystem.cc	2016-08-13 19:05:12 +0000
@@ -41,6 +41,29 @@
      basename_(fs_filename(zipfile.c_str())),
      write_handle_(nullptr),
      read_handle_(nullptr) {
+	std::string first_entry;
+	size_t longest_prefix = 0;
+	unz_file_info file_info;
+	char filename_inzip[256];
+	for (;;) {
+		unzGetCurrentFileInfo(read_handle(), &file_info, filename_inzip,
+		                      sizeof(filename_inzip), nullptr, 0, nullptr, 0);
+		if (first_entry.empty()) {
+			first_entry = filename_inzip;
+			longest_prefix = first_entry.size();
+		} else {
+			const std::string entry = filename_inzip;
+			size_t pos = 0;
+			while (pos < longest_prefix && pos < entry.size() && first_entry[pos] == entry[pos]) {
+				++pos;
+			}
+			longest_prefix = pos;
+		}
+
+		if (unzGoToNextFile(read_handle()) == UNZ_END_OF_LIST_OF_FILE)
+			break;
+	}
+	common_prefix_ = first_entry.substr(0, longest_prefix);
 }
 
 ZipFilesystem::ZipFile::~ZipFile() {
@@ -84,18 +107,7 @@
 }
 
 std::string ZipFilesystem::ZipFile::strip_basename(const std::string& filename) {
-	if (filename.compare(0, basename_.length(), basename_) == 0) {
-		// filename contains the name of the zip file as first element. This means
-		// this is an old zip file where all data were in a directory named as the
-		// file inside the zip file.
-		// return the filename without the first element
-		return filename.substr(basename_.length() + 1);
-	}
-
-	// seems to be a new zipfile without directory or a old zipfile was renamed
-	if (*filename.begin() == '/')
-		return filename.substr(1);
-	return filename;
+	return filename.substr(common_prefix_.size());
 }
 
 const zipFile& ZipFilesystem::ZipFile::write_handle() {

=== modified file 'src/io/filesystem/zip_filesystem.h'
--- src/io/filesystem/zip_filesystem.h	2016-08-04 15:49:05 +0000
+++ src/io/filesystem/zip_filesystem.h	2016-08-13 19:05:12 +0000
@@ -110,6 +110,11 @@
 		// E.g. "filename.zip"
 		std::string basename_;
 
+		// All files in our zipfile start with this prefix. We remember this to deal
+		// with legacy Widelands files that used to keep files in sub directories
+		// that had the same name than the containing zip file.
+		std::string common_prefix_;
+
 		// File handles for zipping and unzipping.
 		zipFile write_handle_;
 		unzFile read_handle_;

=== modified file 'src/map_io/map_objective_packet.cc'
--- src/map_io/map_objective_packet.cc	2016-08-04 15:49:05 +0000
+++ src/map_io/map_objective_packet.cc	2016-08-13 19:05:12 +0000
@@ -31,13 +31,7 @@
 
 constexpr int32_t kCurrentPacketVersion = 2;
 
-void MapObjectivePacket::read(FileSystem& fs,
-                              EditorGameBase& egbase,
-                              bool const skip,
-                              MapObjectLoader&) {
-	if (skip)
-		return;
-
+void read_objective_data(FileSystem& fs, EditorGameBase& egbase) {
 	Profile prof;
 	try {
 		prof.read("objective", nullptr, fs);
@@ -72,7 +66,7 @@
 	}
 }
 
-void MapObjectivePacket::write(FileSystem& fs, EditorGameBase& egbase, MapObjectSaver&) {
+void write_objective_data(FileSystem& fs, EditorGameBase& egbase) {
 	Profile prof;
 	prof.create_section("global").set_int("packet_version", kCurrentPacketVersion);
 

=== modified file 'src/map_io/map_objective_packet.h'
--- src/map_io/map_objective_packet.h	2014-09-10 16:57:31 +0000
+++ src/map_io/map_objective_packet.h	2016-08-13 19:05:12 +0000
@@ -22,6 +22,11 @@
 
 #include "map_io/map_data_packet.h"
 
-MAP_DATA_PACKET(MapObjectivePacket)
+namespace Widelands {
+
+void read_objective_data(FileSystem&, EditorGameBase&);
+void write_objective_data(FileSystem&, EditorGameBase&);
+
+}
 
 #endif  // end of include guard: WL_MAP_IO_MAP_OBJECTIVE_PACKET_H

=== modified file 'src/map_io/map_saver.cc'
--- src/map_io/map_saver.cc	2016-08-04 15:49:05 +0000
+++ src/map_io/map_saver.cc	2016-08-13 19:05:12 +0000
@@ -281,10 +281,7 @@
 	log("took %ums\n ", timer.ms_since_last_query());
 
 	log("Writing Objective Data ... ");
-	{
-		MapObjectivePacket p;
-		p.write(fs_, egbase_, *mos_);
-	}
+	write_objective_data(fs_, egbase_);
 	log("took %ums\n ", timer.ms_since_last_query());
 
 	log("Writing map images ... ");

=== modified file 'src/map_io/widelands_map_loader.cc'
--- src/map_io/widelands_map_loader.cc	2016-08-04 15:49:05 +0000
+++ src/map_io/widelands_map_loader.cc	2016-08-13 19:05:12 +0000
@@ -309,15 +309,16 @@
 			p.read(*fs_, egbase, is_game, *mol_);
 		}
 		log("took %ums\n ", timer.ms_since_last_query());
-	}  // load_type != MapLoader::LoadType::kEditor
 
-	//  Objectives
-	log("Reading Objective Data ... ");
-	{
-		MapObjectivePacket p;
-		p.read(*fs_, egbase, is_game, *mol_);
+		// Objectives. They are not needed in the Editor, since they are fully
+		// defined through Lua scripting. They are also not required for a game,
+		// since they will be only be set after it has started.
+		log("Reading Objective Data ... ");
+		if (!is_game) {
+			read_objective_data(*fs_, egbase);
+		}
+		log("took %ums\n ", timer.ms_since_last_query());
 	}
-	log("took %ums\n ", timer.ms_since_last_query());
 
 	log("Reading Scripting Data ... ");
 	{

=== modified file 'src/profile/profile.cc'
--- src/profile/profile.cc	2016-08-04 16:24:09 +0000
+++ src/profile/profile.cc	2016-08-13 19:05:12 +0000
@@ -637,15 +637,12 @@
 		char* p = nullptr;
 		Section* s = nullptr;
 
-		bool reading_multiline = 0;
 		std::string data;
 		char* key = nullptr;
 		bool translate_line = false;
 		while (char* line = fr.read_line()) {
 			++linenr;
-
-			if (!reading_multiline)
-				p = line;
+			p = line;
 
 			p = skipwhite(p);
 			if (!p[0] || p[0] == '#')
@@ -661,60 +658,27 @@
 			} else {
 				char* tail = nullptr;
 				translate_line = false;
-				if (reading_multiline) {
-					// Note: comments are killed by walking backwards into the string
-					rtrim(p);
-					while (*line != '\'' && *line != '"') {
-						if (*line == 0)
-							throw wexception("runaway multiline string");
-						if (*line == '_')
-							translate_line = true;
-						++line;
-					}
-
-					// skip " or '
-					++line;
-
-					for (char* eot = line + strlen(line) - 1; *eot != '"' && *eot != '\''; --eot)
-						*eot = 0;
-					// NOTE: we leave the last '"' and do not remove them
-					tail = line;
-				} else {
-					tail = strchr(p, '=');
-					if (!tail)
-						throw wexception("invalid syntax: %s", line);
-					*tail++ = '\0';
-					key = p;
-					if (*tail == '_') {
-						tail += 1;  // skip =_, which is only used for translations
-						translate_line = true;
-					}
-					tail = skipwhite(tail);
-					killcomments(tail);
-					rtrim(tail);
-					rtrim(p);
-
-					// first, check for multiline string
-					if ((tail[0] == '\'' || tail[0] == '"') && (tail[1] == '\'' || tail[1] == '"')) {
-						reading_multiline = true;
-						tail += 2;
-					}
-
-					// then remove surrounding '' or ""
-					if (tail[0] == '\'' || tail[0] == '"')
-						++tail;
+				tail = strchr(p, '=');
+				if (!tail)
+					throw wexception("invalid syntax: %s", line);
+				*tail++ = '\0';
+				key = p;
+				if (*tail == '_') {
+					tail += 1;  // skip =_, which is only used for translations
+					translate_line = true;
 				}
+				tail = skipwhite(tail);
+				killcomments(tail);
+				rtrim(tail);
+				rtrim(p);
+
+				// then remove surrounding '' or ""
+				if (tail[0] == '\'' || tail[0] == '"')
+					++tail;
 				if (tail) {
 					char* const eot = tail + strlen(tail) - 1;
 					if (*eot == '\'' || *eot == '"') {
 						*eot = '\0';
-						if (*tail) {
-							char* const eot2 = tail + strlen(tail) - 1;
-							if (*eot2 == '\'' || *eot2 == '"') {
-								reading_multiline = false;
-								*eot2 = '\0';
-							}
-						}
 					}
 
 					// ready to insert
@@ -730,7 +694,7 @@
 					} else {
 						data += tail;
 					}
-					if (s && !reading_multiline) {
+					if (s) {
 						s->create_val_duplicate(key, data.c_str());
 						data.clear();
 					}

=== added directory 'test/maps/Aquila.wmf'
=== added directory 'test/maps/Aquila.wmf/binary'
=== added file 'test/maps/Aquila.wmf/binary/bob'
Binary files test/maps/Aquila.wmf/binary/bob	1970-01-01 00:00:00 +0000 and test/maps/Aquila.wmf/binary/bob	2016-08-13 19:05:12 +0000 differ
=== added file 'test/maps/Aquila.wmf/binary/bob_data'
Binary files test/maps/Aquila.wmf/binary/bob_data	1970-01-01 00:00:00 +0000 and test/maps/Aquila.wmf/binary/bob_data	2016-08-13 19:05:12 +0000 differ
=== added file 'test/maps/Aquila.wmf/binary/building'
Binary files test/maps/Aquila.wmf/binary/building	1970-01-01 00:00:00 +0000 and test/maps/Aquila.wmf/binary/building	2016-08-13 19:05:12 +0000 differ
=== added file 'test/maps/Aquila.wmf/binary/exploration'
Binary files test/maps/Aquila.wmf/binary/exploration	1970-01-01 00:00:00 +0000 and test/maps/Aquila.wmf/binary/exploration	2016-08-13 19:05:12 +0000 differ
=== added file 'test/maps/Aquila.wmf/binary/flag'
Binary files test/maps/Aquila.wmf/binary/flag	1970-01-01 00:00:00 +0000 and test/maps/Aquila.wmf/binary/flag	2016-08-13 19:05:12 +0000 differ
=== added file 'test/maps/Aquila.wmf/binary/heights'
Binary files test/maps/Aquila.wmf/binary/heights	1970-01-01 00:00:00 +0000 and test/maps/Aquila.wmf/binary/heights	2016-08-13 19:05:12 +0000 differ
=== added file 'test/maps/Aquila.wmf/binary/mapobjects'
Binary files test/maps/Aquila.wmf/binary/mapobjects	1970-01-01 00:00:00 +0000 and test/maps/Aquila.wmf/binary/mapobjects	2016-08-13 19:05:12 +0000 differ
=== added file 'test/maps/Aquila.wmf/binary/node_ownership'
Binary files test/maps/Aquila.wmf/binary/node_ownership	1970-01-01 00:00:00 +0000 and test/maps/Aquila.wmf/binary/node_ownership	2016-08-13 19:05:12 +0000 differ
=== added file 'test/maps/Aquila.wmf/binary/resource'
Binary files test/maps/Aquila.wmf/binary/resource	1970-01-01 00:00:00 +0000 and test/maps/Aquila.wmf/binary/resource	2016-08-13 19:05:12 +0000 differ
=== added file 'test/maps/Aquila.wmf/binary/road'
Binary files test/maps/Aquila.wmf/binary/road	1970-01-01 00:00:00 +0000 and test/maps/Aquila.wmf/binary/road	2016-08-13 19:05:12 +0000 differ
=== added file 'test/maps/Aquila.wmf/binary/terrain'
Binary files test/maps/Aquila.wmf/binary/terrain	1970-01-01 00:00:00 +0000 and test/maps/Aquila.wmf/binary/terrain	2016-08-13 19:05:12 +0000 differ
=== added file 'test/maps/Aquila.wmf/binary/ware'
Binary files test/maps/Aquila.wmf/binary/ware	1970-01-01 00:00:00 +0000 and test/maps/Aquila.wmf/binary/ware	2016-08-13 19:05:12 +0000 differ
=== added file 'test/maps/Aquila.wmf/elemental'
--- test/maps/Aquila.wmf/elemental	1970-01-01 00:00:00 +0000
+++ test/maps/Aquila.wmf/elemental	2016-08-13 19:05:12 +0000
@@ -0,0 +1,11 @@
+# Automatically created by Widelands svn4982 (inoffical)
+
+[global]
+packet_version="1"
+map_w="128"
+map_h="128"
+nr_players="4"
+world="desert"
+name="Aquila"
+author="Templer"
+descr="Aquila, das gelobte Land, es ist uns verheißen. Wo mag es sich befinden? Niemand kann etwas sagen.  Jüngst widerfuhr Oma Kaygur ein Wachtraum. Seither deutet sie vage in Richtung Norden, wann immer von Aquila gesprochen wird. Anführer Tigiquas lachte nur breit. Er gab nie etwas auf Aberglaube. Wird unser neuer Anführer den Aufbruch befehlen? Das Volk schaut auf Euch, My Lord."

=== added file 'test/maps/Aquila.wmf/extra_data'
--- test/maps/Aquila.wmf/extra_data	1970-01-01 00:00:00 +0000
+++ test/maps/Aquila.wmf/extra_data	2016-08-13 19:05:12 +0000
@@ -0,0 +1,4 @@
+# Automatically created by Widelands svn4982 (inoffical)
+
+[global]
+packet_version="1"

=== added file 'test/maps/Aquila.wmf/objective'
--- test/maps/Aquila.wmf/objective	1970-01-01 00:00:00 +0000
+++ test/maps/Aquila.wmf/objective	2016-08-13 19:05:12 +0000
@@ -0,0 +1,4 @@
+# Automatically created by Widelands svn4982 (inoffical)
+
+[global]
+packet_version="1"

=== added directory 'test/maps/Aquila.wmf/player'
=== added file 'test/maps/Aquila.wmf/player_names'
--- test/maps/Aquila.wmf/player_names	1970-01-01 00:00:00 +0000
+++ test/maps/Aquila.wmf/player_names	2016-08-13 19:05:12 +0000
@@ -0,0 +1,24 @@
+# Automatically created by Widelands svn4982 (inoffical)
+
+[global]
+packet_version="1"
+
+[player_1]
+name="Lord Sandkeks"
+tribe="empire"
+ai=
+
+[player_2]
+name="Fürst Lapsus"
+tribe="barbarians"
+ai=
+
+[player_3]
+name="Satan Kurl"
+tribe="empire"
+ai=
+
+[player_4]
+name="Baron Heidschnuck"
+tribe="atlanteans"
+ai=

=== added file 'test/maps/Aquila.wmf/player_position'
--- test/maps/Aquila.wmf/player_position	1970-01-01 00:00:00 +0000
+++ test/maps/Aquila.wmf/player_position	2016-08-13 19:05:12 +0000
@@ -0,0 +1,8 @@
+# Automatically created by Widelands svn4982 (inoffical)
+
+[global]
+packet_version="2"
+player_1="115 107"
+player_2="84 32"
+player_3="20 25"
+player_4="72 90"

=== added directory 'test/maps/Aquila.wmf/scripting'
=== added file 'test/maps/Aquila.wmf/scripting/editor_init.lua'
--- test/maps/Aquila.wmf/scripting/editor_init.lua	1970-01-01 00:00:00 +0000
+++ test/maps/Aquila.wmf/scripting/editor_init.lua	2016-08-13 19:05:12 +0000
@@ -0,0 +1,3 @@
+-- No logic required. Just try to load this old map in the Editor.
+print("# All Tests passed.")
+wl.ui.MapView():close()

=== added file 'test/maps/Aquila.wmf/scripting/editor_test_loading.lua'
--- test/maps/Aquila.wmf/scripting/editor_test_loading.lua	1970-01-01 00:00:00 +0000
+++ test/maps/Aquila.wmf/scripting/editor_test_loading.lua	2016-08-13 19:05:12 +0000
@@ -0,0 +1,2 @@
+-- All code in editor_test.lua. This file is just here so that the test runner
+-- picks up this map as a test case.

=== added file 'test/maps/Aquila.wmf/variable'
--- test/maps/Aquila.wmf/variable	1970-01-01 00:00:00 +0000
+++ test/maps/Aquila.wmf/variable	2016-08-13 19:05:12 +0000
@@ -0,0 +1,4 @@
+# Automatically created by Widelands svn4982 (inoffical)
+
+[global]
+packet_version="1"