widelands-dev team mailing list archive

[meitis <baschar-en-tour@xxxxxx>]

meitis has proposed merging lp:~meitis/widelands/bug1375915 into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1375915 in widelands: "scripting files get deleted when a map is resaved"
  https://bugs.launchpad.net/widelands/+bug/1375915

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

=== modified file 'src/editor/ui_menus/editor_main_menu_save_map.cc'
--- src/editor/ui_menus/editor_main_menu_save_map.cc	2015-08-06 17:14:34 +0000
+++ src/editor/ui_menus/editor_main_menu_save_map.cc	2015-09-08 21:29:10 +0000
@@ -374,26 +374,66 @@
 	complete_filename            += "/";
 	complete_filename            += filename;
 
+	std::string tmpName = "";
+
 	//  Check if file exists. If so, show a warning.
 	if (g_fs->file_exists(complete_filename)) {
 		std::string s =
 			(boost::format(_("A file with the name ‘%s’ already exists. Overwrite?"))
 				% FileSystem::fs_filename(filename.c_str())).str();
-		UI::WLMessageBox mbox
-			(&eia(), _("Error Saving Map!"), s, UI::WLMessageBox::MBoxType::kOkCancel);
-		if (mbox.run<UI::Panel::Returncodes>() == UI::Panel::Returncodes::kBack)
+		{
+			UI::WLMessageBox mbox
+				(&eia(), _("Error Saving Map!"), s, UI::WLMessageBox::MBoxType::kOkCancel);
+			if (mbox.run<UI::Panel::Returncodes>() == UI::Panel::Returncodes::kBack)
+				return false;
+		}
+
+		// save to a tmp file/dir first, rename later
+		// (important to keep script files in the script directory)
+		tmpName = complete_filename + ".tmp";
+		if (g_fs->file_exists(tmpName)) {
+			s = (boost::format(_
+					("A file with the name ‘%s.tmp’ already exists. You have to remove it manually."))
+						% FileSystem::fs_filename(filename.c_str())).str();
+			UI::WLMessageBox mbox
+				(&eia(), _("Error Saving Map!"), s, UI::WLMessageBox::MBoxType::kOk);
+			mbox.run<UI::Panel::Returncodes>();
 			return false;
+		}
 
-		g_fs->fs_unlink(complete_filename);
+		// for discussion: if the directory to write to already exists
+		// keep the script files of that direcoty?
+		// though decision, I would vote: no
+		// I vote for: keep the script files of the source directory
+		// (which may be none if its a new map)
+		// compromise might be: keep target scripts if a new map, keep source scripts if source exists
+		/*
+		std::unique_ptr<FileSystem> fs_tmp
+			(g_fs->create_sub_file_system(complete_filename, FileSystem::DIR));
+		eia().egbase.map().swap_filesystem(fs_tmp);
+		*/
 	}
 
 	std::unique_ptr<FileSystem> fs
-			(g_fs->create_sub_file_system(complete_filename, binary ? FileSystem::ZIP : FileSystem::DIR));
+		(g_fs->create_sub_file_system(tmpName.empty() ? complete_filename : tmpName,
+			binary ? FileSystem::ZIP : FileSystem::DIR));
 	Widelands::MapSaver wms(*fs, eia().egbase());
 	try {
 		wms.save();
 		eia().set_need_save(false);
+
+		// if saved to a tmp file earlier, rename now
+		if (!tmpName.empty()) {
+			g_fs->fs_unlink(complete_filename);
+			g_fs->fs_rename(tmpName, complete_filename);
+			fs.reset(g_fs->make_sub_file_system(complete_filename));
+		}
+
+		eia().egbase().map().swap_filesystem(fs);
 	} catch (const std::exception & e) {
+		if (!tmpName.empty()) {
+			g_fs->fs_unlink(tmpName);
+		}
 		std::string s =
 			_
 			("Error Saving Map!\nSaved map file may be corrupt!\n\nReason "

=== modified file 'src/logic/map.cc'
--- src/logic/map.cc	2015-07-29 07:26:48 +0000
+++ src/logic/map.cc	2015-09-08 21:29:10 +0000
@@ -308,6 +308,8 @@
 	objectives_.clear();
 
 	m_port_spaces.clear();
+
+	//filesystem_.reset(nullptr);
 }
 
 /*
@@ -474,6 +476,11 @@
 	return m_scenario_closeables[p - 1];
 }
 
+void Map::swap_filesystem(std::unique_ptr<FileSystem>& fs)
+{
+	filesystem_.swap(fs);
+}
+
 FileSystem* Map::filesystem() const {
 	return filesystem_.get();
 }

=== modified file 'src/logic/map.h'
--- src/logic/map.h	2015-07-27 10:57:01 +0000
+++ src/logic/map.h	2015-09-08 21:29:10 +0000
@@ -206,6 +206,8 @@
 	// Allows access to the filesystem of the map to access auxiliary files.
 	// This can be nullptr if this file is new.
 	FileSystem* filesystem() const;
+	// swap the filesystem after load / save
+	void swap_filesystem(std::unique_ptr<FileSystem>& fs);
 
 	// informational functions
 	const std::string& get_filename()    const {return m_filename;}