widelands-dev team mailing list archive
-
widelands-dev team
-
Mailing list archive
-
Message #15188
[Merge] lp:~widelands-dev/widelands/robust-saving into lp:widelands
Arty has proposed merging lp:~widelands-dev/widelands/robust-saving into lp:widelands with lp:~widelands-dev/widelands/bug-1753230-working-with-tempfiles as a prerequisite.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/robust-saving/+merge/358220
--
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/robust-saving into lp:widelands.
=== modified file 'src/editor/CMakeLists.txt'
--- src/editor/CMakeLists.txt 2018-07-19 16:43:14 +0000
+++ src/editor/CMakeLists.txt 2018-11-02 11:15:15 +0000
@@ -99,6 +99,7 @@
logic
logic_constants
logic_filesystem_constants
+ logic_generic_save_handler
logic_tribe_basic_info
logic_widelands_geometry
map_io
=== modified file 'src/editor/ui_menus/main_menu_save_map.cc'
--- src/editor/ui_menus/main_menu_save_map.cc 2018-11-02 11:15:14 +0000
+++ src/editor/ui_menus/main_menu_save_map.cc 2018-11-02 11:15:15 +0000
@@ -29,6 +29,7 @@
#include "base/i18n.h"
#include "base/macros.h"
+#include "base/time_string.h"
#include "base/wexception.h"
#include "editor/editorinteractive.h"
#include "editor/ui_menus/main_menu_map_options.h"
@@ -37,12 +38,13 @@
#include "io/filesystem/layered_filesystem.h"
#include "io/filesystem/zip_filesystem.h"
#include "logic/filesystem_constants.h"
+#include "logic/generic_save_handler.h"
#include "map_io/map_saver.h"
#include "map_io/widelands_map_loader.h"
#include "ui_basic/messagebox.h"
#include "wui/mapdetails.h"
#include "wui/maptable.h"
-
+#include <iostream>
inline EditorInteractive& MainMenuSaveMap::eia() {
return dynamic_cast<EditorInteractive&>(*get_parent());
}
@@ -119,7 +121,8 @@
* Called when the ok button was pressed or a file in list was double clicked.
*/
void MainMenuSaveMap::clicked_ok() {
- assert(ok_.enabled());
+ if (!ok_.enabled())
+ return;
std::string filename = editbox_->text();
std::string complete_filename;
@@ -243,15 +246,12 @@
/**
* Save the map in the current directory with
- * the current filename
+ * the given filename
*
* returns true if dialog should close, false if it
* should stay open
*/
bool MainMenuSaveMap::save_map(std::string filename, bool binary) {
- // Make sure that the current directory exists and is writeable.
- g_fs->ensure_directory_exists(curdir_);
-
// Trim it for preceding/trailing whitespaces in user input
boost::trim(filename);
@@ -272,18 +272,6 @@
if (mbox.run<UI::Panel::Returncodes>() == UI::Panel::Returncodes::kBack)
return false;
}
-
- // Try deleting file (if it exists). If it fails, give a message and let the player choose a new name.
- try {
- g_fs->fs_unlink(complete_filename);
- } catch (const std::exception& e) {
- const std::string s =
- (boost::format(_("File ‘%s.tmp’ could not be deleted.")) % FileSystem::fs_filename(filename.c_str())).str()
- + " " + _("Try saving under a different name!");
- UI::WLMessageBox mbox(&eia(), _("Error Saving Map!"), s, UI::WLMessageBox::MBoxType::kOk);
- mbox.run<UI::Panel::Returncodes>();
- return false;
- }
Widelands::EditorGameBase& egbase = eia().egbase();
Widelands::Map* map = egbase.mutable_map();
@@ -301,23 +289,34 @@
} else {
map->delete_tag("artifacts");
}
-
- // Try saving.
- try {
- std::unique_ptr<FileSystem> fs(
- g_fs->create_sub_file_system(complete_filename, binary ? FileSystem::ZIP : FileSystem::DIR));
- Widelands::MapSaver* wms = new Widelands::MapSaver(*fs, egbase);
- wms->save();
- delete wms;
- fs.reset();
- } catch (const std::exception& e) {
- std::string s = _("Error Saving Map!\nSaved map file may be corrupt!\n\nReason "
- "given:\n");
- s += e.what();
- UI::WLMessageBox mbox(&eia(), _("Error Saving Map!"), s, UI::WLMessageBox::MBoxType::kOk);
- mbox.run<UI::Panel::Returncodes>();
+
+ // Try saving the map.
+ GenericSaveHandler gsh(
+ [&egbase](FileSystem& fs) {
+ Widelands::MapSaver wms(fs, egbase);
+ wms.save();
+ },
+ complete_filename,
+ binary ? FileSystem::ZIP : FileSystem::DIR
+ );
+ uint32_t error = gsh.save();
+ if (error == GenericSaveHandler::kSuccess ||
+ error == GenericSaveHandler::kDeletingBackupFailed) {
+ // No need to bother the player if only the temporary backup couldn't be deleted.
+ // Automatic cleanup will try to deal with it later.
+ egbase.get_ibase()->log_message(_("Map saved"));
+ return true;
}
- die();
+ std::string msg = gsh.localized_formatted_result_message();
+ UI::WLMessageBox mbox(this, _("Error Saving Map!"), msg, UI::WLMessageBox::MBoxType::kOk);
+ mbox.run<UI::Panel::Returncodes>();
+
+ // If only the backup failed (likely just because of a file lock),
+ // then leave the dialog open for the player to try with a new filename.
+ if (error == GenericSaveHandler::kBackupFailed)
+ return false;
+
+ // In the other error cases close the dialog.
return true;
}
=== modified file 'src/io/filesystem/disk_filesystem.cc'
--- src/io/filesystem/disk_filesystem.cc 2018-04-07 16:59:00 +0000
+++ src/io/filesystem/disk_filesystem.cc 2018-11-02 11:15:15 +0000
@@ -167,15 +167,13 @@
}
/**
- * Create a sub filesystem out of this filesystem
+ * Make a sub filesystem out of this filesystem
*/
FileSystem* RealFSImpl::make_sub_file_system(const std::string& path) {
FileSystemPath fspath(canonicalize_name(path));
if (!fspath.exists_) {
- throw wexception("RealFSImpl: unable to create sub filesystem, path does not exist for '%s'"
- " in directory '%s'",
- fspath.c_str(), directory_.c_str());
+ throw FileNotFoundError("RealFSImpl::make_sub_file_system", fspath);
}
if (fspath.is_directory_)
@@ -190,10 +188,7 @@
FileSystem* RealFSImpl::create_sub_file_system(const std::string& path, Type const fs) {
FileSystemPath fspath(canonicalize_name(path));
if (fspath.exists_)
- throw wexception(
- "path '%s'' already exists in directory '%s', can not create a filesystem from it",
- path.c_str(), directory_.c_str());
-
+ throw FileError("RealFSImpl::create_sub_file_system", fspath, "path already exists, cannot create new filesystem from it");
if (fs == FileSystem::DIR) {
ensure_directory_exists(path);
return new RealFSImpl(fspath);
@@ -221,20 +216,22 @@
void RealFSImpl::unlink_file(const std::string& file) {
FileSystemPath fspath(canonicalize_name(file));
if (!fspath.exists_) {
- throw wexception(
- "RealFSImpl: unable to unlink file, path does not exist for '%s' in directory '%s'",
- fspath.c_str(), directory_.c_str());
+ throw FileNotFoundError("RealFSImpl::unlink_file", fspath);
}
if (fspath.is_directory_) {
- throw wexception(
- "RealFSImpl: unable to unlink file, path '%s' in directory '%s' is a directory",
- fspath.c_str(), directory_.c_str());
+ throw FileTypeError("RealFSImpl::unlink_file", fspath, "path is a directory");
}
-
#ifndef _WIN32
- unlink(fspath.c_str());
+ if (unlink(fspath.c_str()) == -1)
+ throw FileError("RealFSImpl::unlink_file", fspath, strerror(errno));
#else
- DeleteFile(fspath.c_str());
+ // Note: We could possibly replace this with _unlink()
+ // which would then also work with errno instead of GetLastError(),
+ // but I am not sure if this is available (and works properly)
+ // on all windows platforms or only with Visual Studio.
+ if (!DeleteFile(fspath.c_str()))
+ throw FileError("RealFSImpl::unlink_file", fspath, std::string("file error (Windows error code ")+std::to_string(GetLastError())+")");
+ // TODO: generate proper system message from GetLastError() via FormatMessage
#endif
}
@@ -244,14 +241,10 @@
void RealFSImpl::unlink_directory(const std::string& file) {
FileSystemPath fspath(canonicalize_name(file));
if (!fspath.exists_) {
- throw wexception("RealFSImpl: unable to unlink directory, path does not exist for '%s'"
- " in directory '%s'",
- fspath.c_str(), directory_.c_str());
+ throw FileNotFoundError("RealFSImpl::unlink_directory", fspath);
}
if (!fspath.is_directory_) {
- throw wexception("RealFSImpl: unable to unlink directory, path '%s' in directory '%s'"
- " is not a directory",
- fspath.c_str(), directory_.c_str());
+ throw FileTypeError("RealFSImpl::unlink_directory", fspath, "path is not a directory");
}
FilenameSet files = list_directory(file);
@@ -268,14 +261,18 @@
unlink_file(*pname);
}
-// NOTE: this might fail if this directory contains CVS dir,
-// so no error checking here
+// NOTE: this might fail if this directory contains CVS dir
#ifndef _WIN32
- rmdir(fspath.c_str());
+ if (rmdir(fspath.c_str()) == -1)
+ throw FileError("RealFSImpl::unlink_directory", fspath, strerror(errno));
#else
+ // Note: We could possibly replace this with _rmdir()
+ // which would then also work with errno instead of GetLastError(),
+ // but I am not sure if this is available (and works properly)
+ // on all windows platforms or only with Visual Studio.
if (!RemoveDirectory(fspath.c_str()))
- throw wexception(
- "'%s' could not be deleted in directory '%s'.", fspath.c_str(), directory_.c_str());
+ throw FileError("RealFSImpl::unlink_directory", fspath, std::string("file error (Windows error code ")+std::to_string(GetLastError())+")");
+ // TODO: generate proper system message from GetLastError() via FormatMessage
#endif
}
@@ -292,25 +289,20 @@
boost::replace_all(clean_dirname, "\\", "/");
#endif
- try {
- std::string::size_type it = 0;
- while (it < clean_dirname.size()) {
- it = clean_dirname.find('/', it);
-
- FileSystemPath fspath(canonicalize_name(clean_dirname.substr(0, it)));
- if (fspath.exists_ && !fspath.is_directory_)
- throw wexception("'%s' in directory '%s' exists and is not a directory",
- clean_dirname.substr(0, it).c_str(), directory_.c_str());
- if (!fspath.exists_)
- make_directory(clean_dirname.substr(0, it));
-
- if (it == std::string::npos)
- break;
- ++it;
- }
- } catch (const std::exception& e) {
- throw wexception("RealFSImpl::ensure_directory_exists(%s) in directory '%s': %s",
- clean_dirname.c_str(), directory_.c_str(), e.what());
+ std::string::size_type it = 0;
+ while (it < clean_dirname.size()) {
+ it = clean_dirname.find('/', it);
+
+ FileSystemPath fspath(canonicalize_name(clean_dirname.substr(0, it)));
+ if (fspath.exists_ && !fspath.is_directory_)
+ throw FileTypeError("RealFSImpl::ensure_directory_exists",
+ fspath, "path is not a directory");
+ if (!fspath.exists_)
+ make_directory(clean_dirname.substr(0, it));
+
+ if (it == std::string::npos)
+ break;
+ ++it;
}
}
@@ -318,33 +310,37 @@
* Create this directory, throw an error if it already exists or
* if a file is in the way or if the creation fails.
*
- * Pleas note, this function does not honor parents,
+ * Please note, this function does not honor parents,
* make_directory("onedir/otherdir/onemoredir") will fail
* if either onedir or otherdir is missing
*/
void RealFSImpl::make_directory(const std::string& dirname) {
FileSystemPath fspath(canonicalize_name(dirname));
if (fspath.exists_)
- throw wexception("a file/directory with the name '%s' already exists in directory '%s'",
- dirname.c_str(), directory_.c_str());
-
- if
-#ifdef _WIN32
- (!CreateDirectory(fspath.c_str(), NULL))
+ throw FileError("RealFSImpl::make_directory", fspath, "a file or directory with that name already exists");
+
+#ifndef _WIN32
+ if (!mkdir(fspath.c_str(), 0x1FF) == -1)
+ throw FileError("RealFSImpl::make_directory", fspath, strerror(errno));
#else
- (mkdir(fspath.c_str(), 0x1FF) == -1)
+ // Note: We could possibly replace this with _mkdir()
+ // which would then also work with errno instead of GetLastError(),
+ // but I am not sure if this is available (and works properly)
+ // on all windows platforms or only with Visual Studio.
+ if (!CreateDirectory(fspath.c_str(), NULL))
+ throw FileError("RealFSImpl::make_directory", fspath, std::string("file error (Windows error code ")+std::to_string(GetLastError())+")");
+ // TODO: generate proper system message from GetLastError() via FormatMessage
#endif
- throw DirectoryCannotCreateError("RealFSImpl::make_directory", dirname, strerror(errno));
}
/**
- * Read the given file into alloced memory; called by FileRead::open.
+ * Read the given file into allocated memory; called by FileRead::open.
* Throws an exception if the file couldn't be opened.
*/
void* RealFSImpl::load(const std::string& fname, size_t& length) {
const std::string fullname = canonicalize_name(fname);
if (is_directory(fullname)) {
- throw FileError("RealFSImpl::load", fullname);
+ throw FileTypeError("RealFSImpl::load", fullname, "path is a directory");
}
FILE* file = nullptr;
@@ -353,7 +349,7 @@
try {
file = fopen(fullname.c_str(), "rb");
if (!file)
- throw FileError("RealFSImpl::load", fullname);
+ throw FileError("RealFSImpl::load", fullname, "could not open file for reading");
// determine the size of the file (rather quirky, but it doesn't require
// potentially unportable functions)
@@ -371,6 +367,9 @@
// allocate a buffer and read the entire file into it
data = malloc(size + 1); // TODO(unknown): memory leak!
+ if (!data)
+ throw wexception("RealFSImpl::load: memory allocation failed for reading file %s (%s) with size %" PRIuS "",
+ fname.c_str(), fullname.c_str(), size);
int result = fread(data, size, 1, file);
if (size && (result != 1)) {
throw wexception("RealFSImpl::load: read failed for %s (%s) with size %" PRIuS "",
@@ -383,10 +382,10 @@
length = size;
} catch (...) {
- if (file) {
+ if (file)
fclose(file);
- }
- free(data);
+ if (data)
+ free(data);
throw;
}
@@ -410,7 +409,7 @@
FILE* const f = fopen(fullname.c_str(), append ? "a" : "wb");
if (!f)
- throw wexception("could not open %s (%s) for writing", fname.c_str(), fullname.c_str());
+ throw FileError("RealFSImpl::write", fullname, "could not open file for writing");
size_t const c = fwrite(data, length, 1, f);
fclose(f);
@@ -424,8 +423,7 @@
const std::string fullname1 = canonicalize_name(old_name);
const std::string fullname2 = canonicalize_name(new_name);
if (rename(fullname1.c_str(), fullname2.c_str()) != 0)
- throw wexception("DiskFileSystem: unable to rename %s to %s: %s", fullname1.c_str(),
- fullname2.c_str(), strerror(errno));
+ throw FileError("RealFSImpl::fs_rename", fullname1, std::string("unable to rename file to ") + fullname2 + ", " + strerror(errno));
}
/*****************************************************************************
@@ -439,7 +437,7 @@
struct RealFSStreamRead : public StreamRead {
explicit RealFSStreamRead(const std::string& fname) : file_(fopen(fname.c_str(), "rb")) {
if (!file_)
- throw wexception("could not open %s for reading", fname.c_str());
+ throw FileError("RealFSStreamRead::RealFSStreamRead", fname, "could not open file for reading");
}
~RealFSStreamRead() override {
@@ -477,7 +475,7 @@
explicit RealFSStreamWrite(const std::string& fname) : filename_(fname) {
file_ = fopen(fname.c_str(), "wb");
if (!file_)
- throw wexception("could not open %s for writing", fname.c_str());
+ throw FileError("RealFSStreamWrite::RealFSStreamWrite", fname, "could not open file for writing");
}
~RealFSStreamWrite() override {
=== modified file 'src/io/filesystem/filesystem.cc'
--- src/io/filesystem/filesystem.cc 2018-07-12 04:41:20 +0000
+++ src/io/filesystem/filesystem.cc 2018-11-02 11:15:15 +0000
@@ -306,7 +306,7 @@
std::list<std::string>::iterator i;
#ifdef _WIN32
- // remove all slashes with backslashes so following can work.
+ // replace all slashes with backslashes so following can work.
for (uint32_t j = 0; j < path.size(); ++j) {
if (path[j] == '/')
path[j] = '\\';
@@ -463,7 +463,7 @@
fs.ensure_directory_exists(".widelands");
fs.fs_unlink(".widelands");
return true;
- } catch (...) {
+ } catch (const FileError& e) {
log("Directory %s is not writeable - next try\n", path);
}
=== modified file 'src/io/filesystem/zip_filesystem.cc'
--- src/io/filesystem/zip_filesystem.cc 2018-11-02 11:15:14 +0000
+++ src/io/filesystem/zip_filesystem.cc 2018-11-02 11:15:15 +0000
@@ -268,12 +268,14 @@
if (!file_exists(path)) {
throw wexception(
"ZipFilesystem::make_sub_file_system: The path '%s' does not exist in zip file '%s'.",
- path.c_str(), zip_file_->path().c_str());
+ (basedir_in_zip_file_.empty() ? path : basedir_in_zip_file_ + "/" + path).c_str(),
+ zip_file_->path().c_str());
}
if (!is_directory(path)) {
- throw wexception("ZipFilesystem::make_sub_file_system: "
- "The path '%s' needs to be a directory in zip file '%s'.",
- path.c_str(), zip_file_->path().c_str());
+ throw wexception(
+ "ZipFilesystem::make_sub_file_system: The path '%s' needs to be a directory in zip file '%s'.",
+ (basedir_in_zip_file_.empty()?path:basedir_in_zip_file_+"/"+path).c_str(),
+ zip_file_->path().c_str());
}
std::string localpath = path;
@@ -291,7 +293,10 @@
// see Filesystem::create
FileSystem* ZipFilesystem::create_sub_file_system(const std::string& path, Type const type) {
if (file_exists(path)) {
- throw wexception("ZipFilesystem::create_sub_file_system: Sub file system already exists.");
+ throw wexception(
+ "ZipFilesystem::create_sub_file_system: Path '%s' already exists in zip file %s.",
+ (basedir_in_zip_file_.empty() ? path : basedir_in_zip_file_ + "/" + path).c_str(),
+ zip_file_->path().c_str());
}
if (type != FileSystem::DIR)
@@ -438,8 +443,9 @@
"ZipFilesystem::write", complete_filename,
(boost::format("in path '%s'', Error") % zip_file_->path() % strerror(errno)).str());
default:
- throw FileError("ZipFilesystem::write", complete_filename,
- (boost::format("in path '%s'") % zip_file_->path()).str());
+ throw FileError(
+ "ZipFilesystem::write", complete_filename,
+ (boost::format("in path '%s'") % zip_file_->path()).str());
}
zipCloseFileInZip(zip_file_->write_handle());
=== modified file 'src/logic/CMakeLists.txt'
--- src/logic/CMakeLists.txt 2018-08-24 07:12:20 +0000
+++ src/logic/CMakeLists.txt 2018-11-02 11:15:15 +0000
@@ -7,7 +7,6 @@
base_i18n
)
-
wl_library(logic_widelands_geometry
SRCS
widelands_geometry.cc
@@ -81,6 +80,7 @@
filesystem_constants.h
filesystem_constants.cc
)
+
wl_library(logic_tribe_basic_info
SRCS
map_objects/tribes/tribe_basic_info.cc
@@ -93,6 +93,14 @@
)
+wl_library(logic_generic_save_handler
+ SRCS
+ generic_save_handler.h
+ generic_save_handler.cc
+ DEPENDS
+ io_filesystem
+)
+
wl_library(logic
SRCS
ai_dna_handler.cc
@@ -281,6 +289,7 @@
logic_filesystem_constants
logic_game_controller
logic_game_settings
+ logic_generic_save_handler
logic_tribe_basic_info
logic_widelands_geometry
map_io
=== modified file 'src/logic/editor_game_base.cc'
--- src/logic/editor_game_base.cc 2018-11-02 11:15:14 +0000
+++ src/logic/editor_game_base.cc 2018-11-02 11:15:15 +0000
@@ -120,7 +120,7 @@
int suffix;
for (suffix = 0; suffix <= 9; suffix++)
{
- complete_filename = filename + "-" + std::to_string(suffix) + ".tmp";
+ complete_filename = filename + "-" + std::to_string(suffix) + kTempFileExtension;
if (!g_fs->file_exists(complete_filename))
break;
}
=== modified file 'src/logic/filesystem_constants.h'
--- src/logic/filesystem_constants.h 2018-11-02 11:15:14 +0000
+++ src/logic/filesystem_constants.h 2018-11-02 11:15:15 +0000
@@ -45,6 +45,11 @@
// We delete (accidentally remaining) temp files older than a week
constexpr double kTempFilesKeepAroundTime = 7 * 24 * 60 * 60;
+/// Filesystem names for for temporary backup when overwriting files during saving
+const std::string kTempBackupExtension = ".tmp";
+// We delete (accidentally remaining) temp backup files older than a day
+constexpr double kTempBackupsKeepAroundTime = 24 * 60 * 60;
+
/// Filesystem names and timeouts for replays
const std::string kReplayDir = "replays";
const std::string kReplayExtension = ".wrpl";
=== added file 'src/logic/generic_save_handler.cc'
--- src/logic/generic_save_handler.cc 1970-01-01 00:00:00 +0000
+++ src/logic/generic_save_handler.cc 2018-11-02 11:15:15 +0000
@@ -0,0 +1,253 @@
+/*
+ * Copyright (C) 2002-2018 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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ */
+
+#include "logic/generic_save_handler.h"
+
+#include <boost/format.hpp>
+#include <string>
+
+#include "base/i18n.h"
+#include "base/log.h"
+#include "base/time_string.h"
+#include "base/wexception.h"
+#include "io/filesystem/filesystem.h"
+#include "io/filesystem/filesystem_exceptions.h"
+#include "io/filesystem/layered_filesystem.h"
+#include "logic/filesystem_constants.h"
+
+void GenericSaveHandler::clear_errors() {
+ error_ = kSuccess;
+ for (uint32_t errbit = 0; errbit < maxErrors; errbit++) {
+ error_msg_[errbit].clear();
+ }
+ backup_filename_.clear();
+ return;
+}
+
+void GenericSaveHandler::make_backup() {
+ std::string backup_filename_base = dir_ + g_fs->file_separator() + timestring() + "_" + filename_;
+ backup_filename_ = backup_filename_base + kTempBackupExtension;
+
+ // If a file with that name already exists, then try a few name modifications.
+ if (g_fs->file_exists(backup_filename_))
+ {
+ int suffix;
+ for (suffix = 0; suffix <= 9; suffix++)
+ {
+ backup_filename_ = backup_filename_base + "-" + std::to_string(suffix) + kTempBackupExtension;
+ if (!g_fs->file_exists(backup_filename_))
+ break;
+ }
+ if (suffix > 9) {
+ error_ |= kBackupFailed;
+ error_msg_[bitBackupFailed] = ( boost::format("GenericSaveHandler::make_backup: %s: "
+ "for all considered filenames a file already existed (last filename tried was %s)\n")
+ % complete_filename_.c_str() % backup_filename_ ).str();
+ log("%s", error_msg_[bitBackupFailed].c_str());
+ return;
+ }
+ }
+
+ // Try to rename file.
+ try {
+ g_fs->fs_rename(complete_filename_, backup_filename_);
+ } catch (const FileError& e) {
+ error_ |= kBackupFailed;
+ error_msg_[bitBackupFailed] = ( boost::format("GenericSaveHandler::make_backup: file %s "
+ "could not be renamed: %s\n") % complete_filename_.c_str() % backup_filename_ ).str();
+ log("%s", error_msg_[bitBackupFailed].c_str());
+ return;
+ }
+
+ return;
+}
+
+void GenericSaveHandler::save_file() {
+ // Write data to file/dir.
+ try {
+ std::unique_ptr<FileSystem> fs(g_fs->create_sub_file_system(complete_filename_, type_));
+ do_save_(*fs);
+ } catch (const std::exception& e) {
+ error_ |= kSavingDataFailed;
+ error_msg_[bitSavingDataFailed] = ( boost::format("GenericSaveHandler::save_file: "
+ "data could not be written to file %s: %s\n") % complete_filename_.c_str() % e.what() ).str();
+ log("%s", error_msg_[bitSavingDataFailed].c_str());
+ }
+
+ if (error_ & kSavingDataFailed) {
+ // Delete remnants of the failed save attempt.
+ if (g_fs->file_exists(complete_filename_)) {
+ try {
+ g_fs->fs_unlink(complete_filename_);
+ } catch (const FileError& e) {
+ error_ |= kCorruptFileLeft;
+ error_msg_[bitCorruptFileLeft] = ( boost::format("GenericSaveHandler::save_file: "
+ "possibly corrupt file %s could not be deleted: %s\n") % complete_filename_.c_str() % e.what() ).str();
+ log("%s", error_msg_[bitCorruptFileLeft].c_str());
+ }
+ }
+ }
+ return;
+}
+
+uint32_t GenericSaveHandler::save() {
+ try { // everything additionally in one big try block to catch any unexpected errors
+ clear_errors();
+
+ // Make sure that the current directory exists and is writeable.
+ try {
+ g_fs->ensure_directory_exists(dir_);
+ } catch (const FileError& e) {
+ error_ |= kCreatingDirFailed;
+ error_msg_[bitCreatingDirFailed] = ( boost::format("GenericSaveHandler::save: "
+ "directory %s could not be created: %s\n") % dir_.c_str() % e.what() ).str();
+ log("%s", error_msg_[bitCreatingDirFailed].c_str());
+ return error_;
+ }
+
+ // Make a backup if file already exists.
+ if (g_fs->file_exists(complete_filename_)) {
+ make_backup();
+ }
+ if (error_) {
+ return error_;
+ }
+
+ // Write data to file/dir.
+ save_file();
+
+ // Restore or delete backup if one was made.
+ if (!backup_filename_.empty())
+ {
+ if (!error_) {
+ // Delete backup.
+ try {
+ g_fs->fs_unlink(backup_filename_);
+ } catch (const FileError& e) {
+ error_ |= kDeletingBackupFailed;
+ error_msg_[bitDeletingBackupFailed] = ( boost::format("GenericSaveHandler::save: "
+ "backup file %s could not be deleted: %s\n") % backup_filename_.c_str() % e.what() ).str();
+ log("%s", error_msg_[bitDeletingBackupFailed].c_str());
+ }
+
+ } else {
+ if (error_ & kCorruptFileLeft) {
+ error_ |= kRestoringBackupFailed;
+ error_msg_[bitRestoringBackupFailed] = ( boost::format("GenericSaveHandler::save: "
+ "file %s could not be restored from backup %s: file still exists\n")
+ % complete_filename_.c_str() % backup_filename_.c_str() ).str();
+ log("%s", error_msg_[bitRestoringBackupFailed].c_str());
+ }
+ else {
+ // Restore backup.
+ try {
+ g_fs->fs_rename(backup_filename_, complete_filename_);
+ } catch (const FileError& e) {
+ error_ |= kRestoringBackupFailed;
+ error_msg_[bitRestoringBackupFailed] = ( boost::format("GenericSaveHandler::save: "
+ "file %s could not be restored from backup %s: %s\n") % backup_filename_.c_str()
+ % backup_filename_.c_str() % e.what() ).str();
+ log("%s", error_msg_[bitRestoringBackupFailed].c_str());
+ }
+ }
+ }
+ }
+
+ } catch(const std::exception& e) {
+ error_ |= kUnexpectedError;
+ error_msg_[bitUnexpectedError] = ( boost::format("GenericSaveHandler::save: "
+ "unknown error: %s\n") % e.what() ).str();
+ log("%s", error_msg_[bitUnexpectedError].c_str());
+ }
+
+ return error_;
+}
+
+std::string GenericSaveHandler::error_message(uint32_t error_mask) {
+ error_mask &= error_;
+ std::string err_msg;
+ for (uint32_t errind = 0; errind < maxErrors; errind++) {
+ if ((error_mask >> errind) & 1) {
+ err_msg += error_msg_[errind];
+ }
+ }
+ return err_msg;
+}
+
+std::string GenericSaveHandler::localized_formatted_result_message() {
+ std::string msg;
+
+ if (error_ == kSuccess || error_ == kDeletingBackupFailed) {
+ // no need to mention a failed backup deletion
+ return _("File successfully saved!");
+ }
+
+ if (error_ == kCreatingDirFailed) {
+ return
+ ( boost::format(_("Directory ‘%s’ could not be created!"))
+ % dir_ ).str();
+ }
+
+ if (error_ == kBackupFailed) {
+ return
+ ( boost::format(_("File ‘%s’ could not be removed!"))
+ % complete_filename_.c_str() ).str() + "\n"
+ + _("Try saving under a different name!");
+ }
+
+ // from here on multiple errors might have occurred
+ if (error_ & kSavingDataFailed) {
+ msg =
+ ( boost::format(_("Error writing data to file ‘%s’!"))
+ % complete_filename_.c_str() ).str();
+ }
+
+ if (error_ & kCorruptFileLeft) {
+ if (!msg.empty()) msg += "\n";
+ msg += ( boost::format(_("Saved file may be corrupt!")) ).str();
+ }
+
+ if (error_ & kRestoringBackupFailed) {
+ if (!msg.empty()) msg += "\n";
+ msg +=
+ ( boost::format(_("File ‘%s’ could not be restored!"))
+ % complete_filename_.c_str() ).str() + "\n" +
+ ( boost::format(_("Backup file ‘%s’ will be available for some time."))
+ % backup_filename_.c_str() ).str();
+ }
+
+ if ( !backup_filename_.empty() &&
+ (error_ & kSavingDataFailed) &&
+ !(error_ & kCorruptFileLeft) &&
+ !(error_ & kRestoringBackupFailed) ) {
+ if (!msg.empty()) msg += "\n";
+ msg +=
+ ( boost::format(_("File ‘%s’ was restored from backup."))
+ % complete_filename_.c_str() ).str();
+ }
+
+ if (error_ & kUnexpectedError) {
+ if (!msg.empty()) msg += "\n";
+ msg +=
+ ( boost::format(_("An unexpected error occurred:" )) ).str()
+ + "\n" + error_msg_[bitUnexpectedError];
+ }
+
+ return msg;
+}
=== added file 'src/logic/generic_save_handler.h'
--- src/logic/generic_save_handler.h 1970-01-01 00:00:00 +0000
+++ src/logic/generic_save_handler.h 2018-11-02 11:15:15 +0000
@@ -0,0 +1,125 @@
+/*
+ * Copyright (C) 2002-2018 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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ */
+
+#ifndef WL_LOGIC_GENERIC_SAVE_HANDLER_H
+#define WL_LOGIC_GENERIC_SAVE_HANDLER_H
+
+#include <functional>
+#include <string>
+
+#include <stdint.h>
+
+#include "io/filesystem/filesystem.h"
+
+// just some constants for convenience
+namespace {
+ enum ErrorBitIndex : uint32_t {
+ bitCreatingDirFailed = 0,
+ bitBackupFailed,
+ bitSavingDataFailed,
+ bitCorruptFileLeft,
+ bitDeletingBackupFailed,
+ bitRestoringBackupFailed,
+ bitUnexpectedError,
+ maxErrors
+ };
+}
+
+/**
+ * Class that provides handling all errors for generic file saving.
+ * It stores error codes and error messages.
+ * It can also generate an overview message aimed at a human user.
+ *
+ * The saving routine (what actually writes data to a file system) must be provided.
+ *
+ * TODO(Arty) Possibly make it more generic, allowing to provide options whether
+ * to overwrite, whether to make backups, etc.
+ * Maybe allow to provide a naming scheme, etc.
+ */
+class GenericSaveHandler {
+public:
+ // error constants; usable as bit masks
+ static constexpr uint32_t kSuccess = 0;
+ static constexpr uint32_t kCreatingDirFailed = uint32_t(1) << bitCreatingDirFailed;
+ static constexpr uint32_t kBackupFailed = uint32_t(1) << bitBackupFailed;
+ static constexpr uint32_t kSavingDataFailed = uint32_t(1) << bitSavingDataFailed;
+ static constexpr uint32_t kCorruptFileLeft = uint32_t(1) << bitCorruptFileLeft;
+ static constexpr uint32_t kDeletingBackupFailed = uint32_t(1) << bitDeletingBackupFailed;
+ static constexpr uint32_t kRestoringBackupFailed = uint32_t(1) << bitRestoringBackupFailed;
+ static constexpr uint32_t kUnexpectedError = uint32_t(1) << bitUnexpectedError;
+ static constexpr uint32_t kAllErrors = (1 << maxErrors) - 1;
+
+ explicit GenericSaveHandler(
+ std::function<void(FileSystem&)> do_save, // function which actually saves data to the filesystem
+ std::string complete_filename,
+ FileSystem::Type type)
+ : do_save_(do_save),
+ complete_filename_(complete_filename),
+ dir_(FileSystem::fs_dirname(complete_filename.c_str())),
+ filename_(FileSystem::fs_filename(complete_filename.c_str())),
+ type_(type),
+ error_(kSuccess) {};
+
+ /**
+ * Tries to save a file.
+ * If the file already exists, it will be overwritten but a temporary backup is made
+ * which will be restored if saving fails and deleted otherwise.
+ * Catches ALL errors.
+ * Error messages for all errors are written to the log but also stored.
+ * Stores and returns an error code (bit mask of all occurred errors).
+ */
+ uint32_t save();
+
+ // returns the stored error code (from the last saving operation)
+ uint32_t error() { return error_; };
+
+ // Returns the combination of error_messages (of occurred errors) specified by a bit mask.
+ std::string error_message(uint32_t error_mask = kAllErrors);
+
+ // Generates a localized formatted message describing
+ // the result of the saving attempt.
+ // Aimed to be sufficiently informative for a human user.
+ std::string localized_formatted_result_message();
+
+private:
+ std::function<void(FileSystem&)> do_save_;
+ std::string complete_filename_;
+ std::string dir_;
+ std::string filename_;
+ FileSystem::Type type_;
+
+ // Backup filename is automatically generated when saving but is also
+ // stored for generating messages containing backup-related things.
+ std::string backup_filename_;
+ uint32_t error_;
+ std::string error_msg_[maxErrors];
+
+ void clear_errors();
+
+ // Finds a suitable backup filename and tries to rename a file.
+ // Stores an errorcode and error message (if applicable).
+ void make_backup();
+
+ // Saves a file. Assumes file doesn't exist.
+ // Stores an errorcode and error message (if applicable).
+ void save_file();
+};
+
+
+#endif // end of include guard: WL_LOGIC_GENERIC_SAVE_HANDLER_H
=== modified file 'src/logic/save_handler.cc'
--- src/logic/save_handler.cc 2018-08-26 18:22:27 +0000
+++ src/logic/save_handler.cc 2018-11-02 11:15:15 +0000
@@ -31,15 +31,14 @@
#include "base/time_string.h"
#include "base/wexception.h"
#include "game_io/game_saver.h"
+#include "io/filesystem/filesystem_exceptions.h"
#include "io/filesystem/filesystem.h"
#include "logic/filesystem_constants.h"
#include "logic/game.h"
#include "logic/game_controller.h"
+#include "logic/generic_save_handler.h"
#include "wui/interactive_base.h"
-// The actual work of saving is done by the GameSaver
-using Widelands::GameSaver;
-
SaveHandler::SaveHandler()
: next_save_realtime_(0),
initialized_(false),
@@ -52,34 +51,59 @@
number_of_rolls_(5) {
}
-void SaveHandler::roll_save_files(const std::string& filename) {
-
- int32_t rolls = number_of_rolls_;
- log("Autosave: Rolling savefiles (count): %d\n", rolls);
- rolls--;
- std::string filename_previous =
- create_file_name(kSaveDir, (boost::format("%s_%02d") % filename % rolls).str());
- if (rolls > 0 && g_fs->file_exists(filename_previous)) {
- g_fs->fs_unlink(filename_previous); // Delete last of the rolling files
- log("Autosave: Deleted %s\n", filename_previous.c_str());
- }
+bool SaveHandler::roll_save_files(const std::string& filename, std::string* const error) {
+ int32_t rolls = 0;
+ std::string filename_previous;
+
+ // only roll the smallest necessary number of files
+ while (rolls < number_of_rolls_) {
+ filename_previous =
+ create_file_name(kSaveDir, (boost::format("%s_%02d") % filename % rolls).str());
+ if (!g_fs->file_exists(filename_previous))
+ break;
+ rolls++;
+ }
+ if (rolls < number_of_rolls_) {
+ // There is a file missing in the sequence; no need to delete any file.
+ log("Autosave: Rolling savefiles (count): %d of %d\n", rolls, number_of_rolls_);
+ }
+ else {
+ log("Autosave: Rolling savefiles (count): %d\n", rolls);
+ rolls--;
+ filename_previous =
+ create_file_name(kSaveDir, (boost::format("%s_%02d") % filename % rolls).str());
+ if (rolls > 0) {
+ try {
+ g_fs->fs_unlink(filename_previous); // Delete last of the rolling files
+ log("Autosave: Deleted %s\n", filename_previous.c_str());
+ } catch (const FileError& e) {
+ log("Autosave: Unable to delete file %s: %s\n", filename_previous.c_str(), e.what());
+ if (error) {
+ *error =
+ (boost::format("Autosave: Unable to delete file %s: %s\n")
+ % filename_previous.c_str() % e.what()).str();
+ }
+ return false;
+ }
+ }
+ }
+
rolls--;
while (rolls >= 0) {
const std::string filename_next =
create_file_name(kSaveDir, (boost::format("%s_%02d") % filename % rolls).str());
- if (g_fs->file_exists(filename_next)) {
- try {
- g_fs->fs_rename(
- filename_next, filename_previous); // e.g. wl_autosave_08 -> wl_autosave_09
- log("Autosave: Rolled %s to %s\n", filename_next.c_str(), filename_previous.c_str());
- } catch (const std::exception& e) {
- log("Autosave: Unable to rename file %s to %s: %s\n", filename_previous.c_str(),
- filename_next.c_str(), e.what());
- }
+ try {
+ g_fs->fs_rename(filename_next, filename_previous); // e.g. wl_autosave_08 -> wl_autosave_09
+ log("Autosave: Rolled %s to %s\n", filename_next.c_str(), filename_previous.c_str());
+ } catch (const FileError& e) {
+ log("Autosave: Unable to roll file %s to %s: %s\n", filename_previous.c_str(),
+ filename_next.c_str(), e.what());
+ return false;
}
filename_previous = filename_next;
rolls--;
}
+ return true;
}
/**
@@ -106,43 +130,6 @@
}
/**
- * If saving fails restore the backup file.
- *
- * @return true when save was a success.
- */
-bool SaveHandler::save_and_handle_error(Widelands::Game& game,
- const std::string& complete_filename,
- const std::string& backup_filename) {
- std::string error;
- bool result = save_game(game, complete_filename, &error);
- if (!result) {
- log("Autosave: ERROR! - %s\n", error.c_str());
- game.get_ibase()->log_message(_("Saving failed!"));
-
- // if backup file was created, move it back
- if (backup_filename.length() > 0) {
- if (g_fs->file_exists(complete_filename)) {
- g_fs->fs_unlink(complete_filename);
- }
- g_fs->fs_rename(backup_filename, complete_filename);
- }
- // Wait 30 seconds until next save try
- next_save_realtime_ = SDL_GetTicks() + 30000;
- } else {
- // if backup file was created, time to remove it
- if (backup_filename.length() > 0 && g_fs->file_exists(backup_filename)) {
- g_fs->fs_unlink(backup_filename);
- }
-
- // Count save interval from end of save.
- // This prevents us from going into endless autosave cycles if the save should take longer
- // than the autosave interval.
- next_save_realtime_ = SDL_GetTicks() + autosave_interval_in_ms_;
- }
- return result;
-}
-
-/**
* Check if autosave is needed and allowed or save was requested by user.
*/
void SaveHandler::think(Widelands::Game& game) {
@@ -155,6 +142,9 @@
// Are we saving now?
if (saving_next_tick_ || save_requested_) {
+ saving_next_tick_ = false;
+ bool save_success = true;
+ std::string error;
std::string filename = autosave_filename_;
if (save_requested_) {
// Requested by user
@@ -166,59 +156,34 @@
save_filename_ = "";
} else {
// Autosave ...
- roll_save_files(filename);
- filename = (boost::format("%s_00") % autosave_filename_).str();
- log("Autosave: saving as %s\n", filename.c_str());
- }
-
- // Saving now
- std::string complete_filename = create_file_name(kSaveDir, filename);
- std::string backup_filename;
-
- // always overwrite a file
- if (g_fs->file_exists(complete_filename)) {
- filename += "2";
- backup_filename = create_file_name(kSaveDir, filename);
- if (g_fs->file_exists(backup_filename)) {
- g_fs->fs_unlink(backup_filename);
- }
- if (save_requested_) {
- // Exceptions (e.g. no file permissions) will be handled in UI
- g_fs->fs_rename(complete_filename, backup_filename);
- } else {
- // We're autosaving, try to handle file permissions issues
- try {
- g_fs->fs_rename(complete_filename, backup_filename);
- } catch (const std::exception& e) {
- log("Autosave: Unable to rename file %s to %s: %s\n", complete_filename.c_str(),
- backup_filename.c_str(), e.what());
- // See if we can find a file that doesn't exist and save to that
- int current_autosave = 0;
- do {
- filename = create_file_name(
- kSaveDir,
- (boost::format("%s_0%d") % autosave_filename_ % (++current_autosave)).str());
- } while (current_autosave < 9 && g_fs->file_exists(filename));
- complete_filename = filename;
- log("Autosave: saving as %s instead\n", complete_filename.c_str());
- try {
- g_fs->fs_rename(complete_filename, backup_filename);
- } catch (const std::exception&) {
- log("Autosave failed, skipping this interval\n");
- saving_next_tick_ = check_next_tick(game, realtime);
- return;
- }
- }
- }
- }
-
- if (!save_and_handle_error(game, complete_filename, backup_filename)) {
+ save_success = roll_save_files(filename, &error);
+ if (save_success) {
+ filename = (boost::format("%s_00") % autosave_filename_).str();
+ log("Autosave: saving as %s\n", filename.c_str());
+ }
+ }
+
+ if (save_success) {
+ // Saving now (always overwrite file)
+ std::string complete_filename = create_file_name(kSaveDir, filename);
+ save_success = save_game(game, complete_filename, &error);
+ }
+ if (!save_success) {
+ log("Autosave: ERROR! - %s\n", error.c_str());
+ game.get_ibase()->log_message(_("Saving failed!"));
+
+ // Wait 30 seconds until next save try
+ next_save_realtime_ = SDL_GetTicks() + 30000;
return;
}
+ // Count save interval from end of save.
+ // This prevents us from going into endless autosave cycles if the save should take longer
+ // than the autosave interval.
+ next_save_realtime_ = SDL_GetTicks() + autosave_interval_in_ms_;
+
log("Autosave: save took %d ms\n", SDL_GetTicks() - realtime);
game.get_ibase()->log_message(_("Game saved"));
- saving_next_tick_ = false;
} else {
saving_next_tick_ = check_next_tick(game, realtime);
}
@@ -262,10 +227,13 @@
return complete_filename;
}
+
/*
* Save the game using the GameSaver.
*
- * Will copy text of exceptions to error string.
+ * Overwrites file if it exists.
+ *
+ * Will copy text of errors to error string.
*
* returns true if saved, false in case some error occured.
*/
@@ -274,22 +242,26 @@
std::string* const error) {
ScopedTimer save_timer("SaveHandler::save_game() took %ums");
- // Make sure that the base directory exists
- g_fs->ensure_directory_exists(kSaveDir);
-
- // Make a filesystem out of this
- std::unique_ptr<FileSystem> fs;
- fs.reset(g_fs->create_sub_file_system(complete_filename, fs_type_));
-
- bool result = true;
- GameSaver gs(*fs, game);
- try {
- gs.save();
- } catch (const std::exception& e) {
- if (error)
- *error = e.what();
- result = false;
- }
-
- return result;
+ // save game via the GenericSaveHandler
+ GenericSaveHandler gsh(
+ [&game](FileSystem& fs) {
+ Widelands::GameSaver gs(fs, game);
+ gs.save();
+ },
+ complete_filename,
+ fs_type_
+ );
+ gsh.save();
+
+ // Ignore it if only the temporary backup wasn't deleted
+ // but save was successfull otherwise
+ if (gsh.error() == GenericSaveHandler::kSuccess ||
+ gsh.error() == GenericSaveHandler::kDeletingBackupFailed) {
+ return true;
+ }
+
+ if (error) {
+ *error = gsh.error_message();
+ }
+ return false;
}
=== modified file 'src/logic/save_handler.h'
--- src/logic/save_handler.h 2018-04-07 16:59:00 +0000
+++ src/logic/save_handler.h 2018-11-02 11:15:15 +0000
@@ -42,6 +42,8 @@
void think(Widelands::Game&);
std::string create_file_name(const std::string& dir, const std::string& filename) const;
+
+ // saves the game, overwrites file, handles errors
bool save_game(Widelands::Game&, const std::string& filename, std::string* error = nullptr);
const std::string get_cur_filename() {
@@ -82,11 +84,8 @@
int32_t number_of_rolls_; // For rolling file update
void initialize(uint32_t gametime);
- void roll_save_files(const std::string& filename);
+ bool roll_save_files(const std::string& filename, std::string* error);
bool check_next_tick(Widelands::Game& game, uint32_t realtime);
- bool save_and_handle_error(Widelands::Game& game,
- const std::string& complete_filename,
- const std::string& backup_filename);
};
#endif // end of include guard: WL_LOGIC_SAVE_HANDLER_H
=== modified file 'src/wlapplication.cc'
--- src/wlapplication.cc 2018-11-02 11:15:14 +0000
+++ src/wlapplication.cc 2018-11-02 11:15:15 +0000
@@ -58,6 +58,7 @@
#include "graphic/text_constants.h"
#include "helper.h"
#include "io/filesystem/disk_filesystem.h"
+#include "io/filesystem/filesystem_exceptions.h"
#include "io/filesystem/layered_filesystem.h"
#include "logic/ai_dna_handler.h"
#include "logic/filesystem_constants.h"
@@ -215,7 +216,7 @@
// Returns true if 'filename' was autogenerated, i.e. if 'extract_creation_day' can return a date
// and it is old enough to be deleted.
bool is_autogenerated_and_expired(const std::string& filename,
- const double keep_time = kReplayKeepAroundTime) {
+ const double keep_time) {
tm tfile;
if (!extract_creation_day(filename, &tfile)) {
return false;
@@ -340,6 +341,7 @@
cleanup_replays();
cleanup_ai_files();
cleanup_temp_files();
+ cleanup_temp_backups();
Section& config = g_options.pull_section("global");
@@ -1464,22 +1466,16 @@
void WLApplication::cleanup_replays() {
for (const std::string& filename :
filter(g_fs->list_directory(kReplayDir), [](const std::string& fn) {
- return boost::ends_with(
- fn, (boost::format("%s%s") % kReplayExtension % kSyncstreamExtension).str());
- })) {
- if (is_autogenerated_and_expired(filename)) {
- log("Delete syncstream %s\n", filename.c_str());
- g_fs->fs_unlink(filename);
- }
- }
-
- for (const std::string& filename :
- filter(g_fs->list_directory(kReplayDir),
- [](const std::string& fn) { return boost::ends_with(fn, kReplayExtension); })) {
- if (is_autogenerated_and_expired(filename)) {
+ return boost::ends_with(fn, kReplayExtension) ||
+ boost::ends_with(fn, kReplayExtension + kSavegameExtension) ||
+ boost::ends_with(fn, kReplayExtension + kSyncstreamExtension); })) {
+ if (is_autogenerated_and_expired(filename, kReplayKeepAroundTime)) {
log("Deleting replay %s\n", filename.c_str());
- g_fs->fs_unlink(filename);
- g_fs->fs_unlink(filename + kSavegameExtension);
+ try {
+ g_fs->fs_unlink(filename);
+ } catch (const FileError& e) {
+ log("WLApplication::cleanup_replays: File %s couldn't be deleted.\n", filename.c_str());
+ }
}
}
}
@@ -1494,7 +1490,11 @@
})) {
if (is_autogenerated_and_expired(filename, kAIFilesKeepAroundTime)) {
log("Deleting generated ai file: %s\n", filename.c_str());
- g_fs->fs_unlink(filename);
+ try {
+ g_fs->fs_unlink(filename);
+ } catch (const FileError& e) {
+ log("WLApplication::cleanup_ai_files: File %s couldn't be deleted.\n", filename.c_str());
+ }
}
}
}
@@ -1509,9 +1509,51 @@
})) {
if (is_autogenerated_and_expired(filename, kTempFilesKeepAroundTime)) {
log("Deleting old temp file: %s\n", filename.c_str());
- g_fs->fs_unlink(filename);
- }
- }
+ try {
+ g_fs->fs_unlink(filename);
+ } catch (const FileError& e) {
+ log("WLApplication::cleanup_temp_files: File %s couldn't be deleted.\n", filename.c_str());
+ }
+ }
+ }
+}
+
+/**
+ * Recursively delete temporary backup files in a given directory
+ */
+void WLApplication::cleanup_temp_backups(std::string dir) {
+ for (const std::string& filename :
+ filter(g_fs->list_directory(dir), [](const std::string& fn) {
+ return boost::ends_with(fn, kTempBackupExtension);
+ })) {
+ if (is_autogenerated_and_expired(filename, kTempBackupsKeepAroundTime)) {
+ log("Deleting old temp backup file: %s\n", filename.c_str());
+ try {
+ g_fs->fs_unlink(filename);
+ } catch (const FileError& e) {
+ log("WLApplication::cleanup_temp_backups: File %s couldn't be deleted.\n", filename.c_str());
+ }
+ }
+ }
+ // recursively delete in subdirs
+ for (const std::string& dirname :
+ filter(g_fs->list_directory(dir), [](const std::string& fn) {
+ return g_fs->is_directory(fn) &&
+ // avoid searching within savegames/maps/backups that were created as directories instead of zipfiles
+ !boost::ends_with(fn, kSavegameExtension) &&
+ !boost::ends_with(fn, kWidelandsMapExtension) &&
+ !boost::ends_with(fn, kTempBackupExtension);
+ })) {
+ cleanup_temp_backups(dirname);
+ }
+}
+
+/**
+ * Delete old temporary backup files that might still lurk around (game crashes etc.)
+ */
+void WLApplication::cleanup_temp_backups() {
+ cleanup_temp_backups(kSaveDir);
+ cleanup_temp_backups(kMapsDir);
}
bool WLApplication::redirect_output(std::string path) {
=== modified file 'src/wlapplication.h'
--- src/wlapplication.h 2018-11-02 11:15:14 +0000
+++ src/wlapplication.h 2018-11-02 11:15:15 +0000
@@ -213,10 +213,10 @@
void setup_homedir();
void cleanup_replays();
-
void cleanup_ai_files();
-
void cleanup_temp_files();
+ void cleanup_temp_backups(std::string dir);
+ void cleanup_temp_backups();
bool redirect_output(std::string path = "");
=== modified file 'src/wui/game_main_menu_save_game.cc'
--- src/wui/game_main_menu_save_game.cc 2018-10-26 07:09:29 +0000
+++ src/wui/game_main_menu_save_game.cc 2018-11-02 11:15:15 +0000
@@ -21,6 +21,7 @@
#include <memory>
+#include <boost/algorithm/string.hpp>
#include <boost/format.hpp>
#include "base/i18n.h"
@@ -32,6 +33,7 @@
#include "logic/filesystem_constants.h"
#include "logic/game.h"
#include "logic/game_controller.h"
+#include "logic/generic_save_handler.h"
#include "logic/playersmanager.h"
#include "ui_basic/messagebox.h"
#include "wui/interactive_gamebase.h"
@@ -153,63 +155,16 @@
}
}
-static void dosave(InteractiveGameBase& igbase, const std::string& complete_filename) {
- Widelands::Game& game = igbase.game();
-
- std::string error;
- if (!game.save_handler().save_game(game, complete_filename, &error)) {
- std::string s = _("Game Saving Error!\nSaved game file may be corrupt!\n\n"
- "Reason given:\n");
- s += error;
- UI::WLMessageBox mbox(&igbase, _("Save Game Error!"), s, UI::WLMessageBox::MBoxType::kOk);
- mbox.run<UI::Panel::Returncodes>();
- }
- game.save_handler().set_current_filename(complete_filename);
-}
-
-struct SaveWarnMessageBox : public UI::WLMessageBox {
- SaveWarnMessageBox(GameMainMenuSaveGame& parent, const std::string& filename)
- : UI::WLMessageBox(&parent,
- _("Save Game Error!"),
- (boost::format(_("A file with the name ‘%s’ already exists. Overwrite?")) %
- FileSystem::fs_filename(filename.c_str()))
- .str(),
- MBoxType::kOkCancel),
- filename_(filename) {
- }
-
- GameMainMenuSaveGame& menu_save_game() {
- return dynamic_cast<GameMainMenuSaveGame&>(*get_parent());
- }
-
- void clicked_ok() override {
- g_fs->fs_unlink(filename_);
- dosave(menu_save_game().igbase(), filename_);
- menu_save_game().die();
- }
-
- void clicked_back() override {
- die();
- }
-
-private:
- std::string const filename_;
-};
-
void GameMainMenuSaveGame::ok() {
- if (filename_editbox_.text().empty()) {
+ if (!ok_.enabled()) {
return;
}
- std::string const complete_filename =
- igbase().game().save_handler().create_file_name(curdir_, filename_editbox_.text());
-
- // Check if file exists. If it does, show a warning.
- if (g_fs->file_exists(complete_filename)) {
- new SaveWarnMessageBox(*this, complete_filename);
+ std::string filename = filename_editbox_.text();
+ if (save_game(filename, !g_options.pull_section("global").get_bool("nozip", false))) {
+ die();
} else {
- dosave(igbase(), complete_filename);
- die();
+ load_or_save_.table_.focus();
}
}
@@ -244,3 +199,67 @@
}
igbase().game().game_controller()->set_paused(paused);
}
+
+/**
+ * Save the game in the Savegame directory with
+ * the given filename
+ *
+ * returns true if dialog should close, false if it
+ * should stay open
+ */
+bool GameMainMenuSaveGame::save_game(std::string filename, bool binary) {
+ // Trim it for preceding/trailing whitespaces in user input
+ boost::trim(filename);
+
+ // OK, first check if the extension matches (ignoring case).
+ if (!boost::iends_with(filename, kSavegameExtension))
+ filename += kSavegameExtension;
+
+ // Append directory name.
+ const std::string complete_filename = curdir_ + g_fs->file_separator() + filename;
+
+ // Check if file exists. If so, show a warning.
+ if (g_fs->file_exists(complete_filename)) {
+ const std::string s =
+ (boost::format(_("A file with the name ‘%s’ already exists. Overwrite?")) %
+ FileSystem::fs_filename(filename.c_str()))
+ .str();
+ UI::WLMessageBox mbox(this, _("Error Saving Game!"), s, UI::WLMessageBox::MBoxType::kOkCancel);
+ if (mbox.run<UI::Panel::Returncodes>() == UI::Panel::Returncodes::kBack)
+ return false;
+ }
+
+ // Try saving the game.
+ Widelands::Game& game = igbase().game();
+ GenericSaveHandler gsh(
+ [&game](FileSystem& fs) {
+ Widelands::GameSaver gs(fs, game);
+ gs.save();
+ },
+ complete_filename,
+ binary ? FileSystem::ZIP : FileSystem::DIR
+ );
+ uint32_t error = gsh.save();
+ if (error == GenericSaveHandler::kSuccess ||
+ error == GenericSaveHandler::kDeletingBackupFailed) {
+ // No need to bother the player if only the temporary backup couldn't be deleted.
+ // Automatic cleanup will try to deal with it later.
+ game.save_handler().set_current_filename(complete_filename);
+ igbase().log_message(_("Game saved"));
+ return true;
+ }
+
+ // Show player an error message.
+ std::string msg = gsh.localized_formatted_result_message();
+ UI::WLMessageBox mbox(this, _("Error Saving Game!"), msg, UI::WLMessageBox::MBoxType::kOk);
+ mbox.run<UI::Panel::Returncodes>();
+
+ // If only the backup failed (likely just because of a file lock),
+ // then leave the dialog open for the player to try with a new filename.
+ if (error == GenericSaveHandler::kBackupFailed)
+ return false;
+
+ // In the other error cases close the dialog.
+ igbase().log_message(_("Saving failed!"));
+ return true;
+}
=== modified file 'src/wui/game_main_menu_save_game.h'
--- src/wui/game_main_menu_save_game.h 2018-05-23 04:40:43 +0000
+++ src/wui/game_main_menu_save_game.h 2018-11-02 11:15:15 +0000
@@ -59,6 +59,8 @@
/// Called when the OK button is clicked or the Return key pressed in the edit box.
void ok();
+
+ bool save_game(std::string filename, bool binary);
/// Pause/unpause the game
void pause_game(bool paused);
Follow ups
