widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

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

Commit message:
create_parent_directory() uses the code from FileSystem now instead of rolling its own.

Also cleaned up mapdata.h and moved implementation into the new mapdata.cc.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1422147 in widelands: "Parent directory can't be navigated while saveloading"
  https://bugs.launchpad.net/widelands/+bug/1422147

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/parent_directory/+merge/285276

Should fix the attached bug, though I have no windows system for testing. Mac OS X still works unaffacted, so Linux should to.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/parent_directory into lp:widelands.

=== modified file 'src/io/filesystem/filesystem.h'
--- src/io/filesystem/filesystem.h	2014-12-03 07:15:40 +0000
+++ src/io/filesystem/filesystem.h	2016-02-06 19:02:37 +0000
@@ -122,6 +122,9 @@
 
 	///Given a filename, return the name with any path stripped off.
 	static const char * fs_filename(const char * n);
+
+	// Everything before the final separator (/ or \) in 'full_path'. The
+	// returned value is either the empty string or ends with a separator.
 	static std::string fs_dirname(const std::string& full_path);
 
 	///Given a filename (without any path), return the extension, if any.

=== modified file 'src/wui/CMakeLists.txt'
--- src/wui/CMakeLists.txt	2016-01-18 19:35:25 +0000
+++ src/wui/CMakeLists.txt	2016-02-06 19:02:37 +0000
@@ -58,6 +58,7 @@
   SRCS
     mapdetails.cc
     mapdetails.h
+    mapdata.cc
     mapdata.h
     maptable.cc
     maptable.h

=== added file 'src/wui/mapdata.cc'
--- src/wui/mapdata.cc	1970-01-01 00:00:00 +0000
+++ src/wui/mapdata.cc	2016-02-06 19:02:37 +0000
@@ -0,0 +1,140 @@
+/*
+ * Copyright (C) 2006-2016 by the Widelands Development Team
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ */
+
+#include "io/filesystem/filesystem.h"
+#include "wui/mapdata.h"
+
+MapData::MapData() : authors(""), nrplayers(0), width(0), height(0),
+		maptype(MapData::MapType::kNormal), displaytype(MapData::DisplayType::kMapnamesLocalized) {}
+
+		MapData::MapData(const Widelands::Map& map, const std::string& init_filename,
+			  const MapData::MapType& init_maptype,
+			  const MapData::DisplayType& init_displaytype) :
+		MapData() {
+		i18n::Textdomain td("maps");
+		filename = init_filename;
+		name = map.get_name();
+		localized_name = name.empty() ? "" : _(name);
+		authors = MapAuthorData(map.get_author());
+		description = map.get_description().empty() ? "" : _(map.get_description());
+		hint = map.get_hint().empty() ? "" : _(map.get_hint());
+		nrplayers = map.get_nrplayers();
+		width = map.get_width();
+		height = map.get_height();
+		suggested_teams = map.get_suggested_teams();
+		tags = map.get_tags();
+		maptype = init_maptype;
+		displaytype = init_displaytype;
+
+		if (maptype == MapData::MapType::kScenario) {
+			tags.insert("scenario");
+		}
+	}
+
+MapData::MapData(const std::string& init_filename, const std::string& init_localized_name) :
+		MapData() {
+		filename = init_filename;
+		name = init_localized_name;
+		localized_name = init_localized_name;
+		maptype = MapData::MapType::kDirectory;
+	}
+
+bool MapData::compare_names(const MapData& other) {
+	// The parent directory gets special treatment.
+	if (localized_name == parent_name() && maptype == MapData::MapType::kDirectory) {
+		return true;
+	} else if (other.localized_name == parent_name() &&
+	           other.maptype == MapData::MapType::kDirectory) {
+		return false;
+	}
+
+	std::string this_name;
+	std::string other_name;
+	switch (displaytype) {
+	case MapData::DisplayType::kFilenames:
+		this_name = filename;
+		other_name = other.filename;
+		break;
+
+	case MapData::DisplayType::kMapnames:
+		this_name = name;
+		other_name = other.name;
+		break;
+
+	case MapData::DisplayType::kMapnamesLocalized:
+		this_name = localized_name;
+		other_name = other.localized_name;
+		break;
+	}
+
+	// If there is no width, we have a directory - we want them first.
+	if (!width && !other.width) {
+		return this_name < other_name;
+	} else if (!width && other.width) {
+		return true;
+	} else if (width && !other.width) {
+		return false;
+	}
+	return this_name < other_name;
+}
+
+bool MapData::compare_players(const MapData& other) {
+	if (nrplayers == other.nrplayers) {
+		return compare_names(other);
+	}
+	return nrplayers < other.nrplayers;
+}
+
+bool MapData::compare_size(const MapData& other) {
+	if (width == other.width && height == other.height) {
+		return compare_names(other);
+	}
+	if (width != other.width) {
+		return width < other.width;
+	}
+	return height < other.height;
+}
+
+// static
+MapData MapData::create_parent_dir(const std::string& current_dir) {
+	std::string filename = FileSystem::fs_dirname(current_dir);
+	if (!filename.empty()) {
+		// fs_dirname always returns a directory with a separator at the end.
+		filename.pop_back();
+	}
+	return MapData(filename, parent_name());
+}
+
+// static
+std::string MapData::parent_name() {
+	/** TRANSLATORS: Parent directory/folder */
+	return (boost::format("\\<%s\\>") % _("parent")).str();
+}
+
+// static
+MapData MapData::create_directory(const std::string& directory) {
+	std::string localized_name;
+	if (boost::equals(directory, "maps/MP_Scenarios")) {
+		/** TRANSLATORS: Directory name for MP Scenarios in map selection */
+		localized_name = _("Multiplayer Scenarios");
+	} else {
+		localized_name = FileSystem::fs_filename(directory.c_str());
+	}
+	return MapData(directory, localized_name);
+}

=== modified file 'src/wui/mapdata.h'
--- src/wui/mapdata.h	2016-01-26 09:10:52 +0000
+++ src/wui/mapdata.h	2016-02-06 19:02:37 +0000
@@ -58,8 +58,6 @@
  * Data about a map that we're interested in.
  */
 struct MapData {
-	using Tags = std::set<std::string>;
-
 	enum class MapType {
 		kNormal,
 		kDirectory,
@@ -75,70 +73,29 @@
 
 
 	/// For incomplete data
-	MapData() : authors(""), nrplayers(0), width(0), height(0),
-		maptype(MapData::MapType::kNormal), displaytype(MapData::DisplayType::kMapnamesLocalized) {}
+	MapData();
 
 	/// For normal maps and scenarios
 	MapData(const Widelands::Map& map, const std::string& init_filename,
 			  const MapData::MapType& init_maptype,
-			  const MapData::DisplayType& init_displaytype) :
-		MapData() {
-		i18n::Textdomain td("maps");
-		filename = init_filename;
-		name = map.get_name();
-		localized_name = name.empty() ? "" : _(name);
-		authors = MapAuthorData(map.get_author());
-		description = map.get_description().empty() ? "" : _(map.get_description());
-		hint = map.get_hint().empty() ? "" : _(map.get_hint());
-		nrplayers = map.get_nrplayers();
-		width = map.get_width();
-		height = map.get_height();
-		suggested_teams = map.get_suggested_teams();
-		tags = map.get_tags();
-		maptype = init_maptype;
-		displaytype = init_displaytype;
-
-		if (maptype == MapData::MapType::kScenario) {
-			tags.insert("scenario");
-		}
-	}
+			  const MapData::DisplayType& init_displaytype);
 
 	/// For directories
-	MapData(const std::string& init_filename, const std::string& init_localized_name) :
-		MapData() {
-		filename = init_filename;
-		name = init_localized_name;
-		localized_name = init_localized_name;
-		maptype = MapData::MapType::kDirectory;
-	}
+	MapData(const std::string& init_filename, const std::string& init_localized_name);
 
 	/// The localized name of the parent directory
-	static const std::string parent_name() {
-		/** TRANSLATORS: Parent directory/folder */
-		return (boost::format("\\<%s\\>") % _("parent")).str();
-	}
+	static std::string parent_name();
 
 	/// Get the ".." directory
-	static MapData create_parent_dir(const std::string& current_dir) {
-#ifndef _WIN32
-		const std::string filename = current_dir.substr(0, current_dir.rfind('/'));
-#else
-		const std::string filename = current_dir.substr(0, current_dir.rfind('\\'));
-#endif
-		return MapData(filename, parent_name());
-	}
+	static MapData create_parent_dir(const std::string& current_dir);
 
 	/// Create a subdirectory
-	static MapData create_directory(const std::string& directory) {
-		std::string localized_name;
-		if (boost::equals(directory, "maps/MP_Scenarios")) {
-			/** TRANSLATORS: Directory name for MP Scenarios in map selection */
-			localized_name = _("Multiplayer Scenarios");
-		} else {
-			localized_name = FileSystem::fs_filename(directory.c_str());
-		}
-		return MapData(directory, localized_name);
-	}
+	static MapData create_directory(const std::string& directory);
+
+	// Sorting functions to order by different categories.
+	bool compare_names(const MapData& other);
+	bool compare_players(const MapData& other);
+	bool compare_size(const MapData& other);
 
 	std::string filename;
 	std::string name;
@@ -150,64 +107,9 @@
 	uint32_t width;
 	uint32_t height;
 	std::vector<Widelands::Map::SuggestedTeamLineup> suggested_teams;
-	Tags tags;
+	std::set<std::string> tags;
 	MapData::MapType maptype;
 	MapData::DisplayType displaytype;
-
-	bool compare_names(const MapData& other) {
-		// The parent directory gets special treatment.
-		if (localized_name == parent_name() && maptype == MapData::MapType::kDirectory) {
-			return true;
-		} else if (other.localized_name == parent_name() && other.maptype == MapData::MapType::kDirectory)  {
-			return false;
-		}
-
-		std::string this_name;
-		std::string other_name;
-		switch (displaytype) {
-		case MapData::DisplayType::kFilenames:
-			this_name = filename;
-			other_name = other.filename;
-			break;
-
-		case MapData::DisplayType::kMapnames:
-			this_name = name;
-			other_name = other.name;
-			break;
-
-		case MapData::DisplayType::kMapnamesLocalized:
-			this_name = localized_name;
-			other_name = other.localized_name;
-			break;
-		}
-
-		// If there is no width, we have a directory - we want them first.
-		if (!width && !other.width) {
-			return this_name < other_name;
-		} else if (!width && other.width) {
-			return true;
-		} else if (width && !other.width) {
-			return false;
-		}
-		return this_name < other_name;
-	}
-
-	bool compare_players(const MapData& other) {
-		if (nrplayers == other.nrplayers) {
-			return compare_names(other);
-		}
-		return nrplayers < other.nrplayers;
-	}
-
-	bool compare_size(const MapData& other) {
-		if (width == other.width && height == other.height) {
-			return compare_names(other);
-		}
-		if (width != other.width) {
-			return width < other.width;
-		}
-		return height < other.height;
-	}
 };
 
 #endif  // end of include guard: WL_WUI_MAPDATA_H