widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

Review: Approve

Code LGTM, still needs testing.

I have also added a reply to one of your comments.

Diff comments:

> 
> === modified file 'src/wui/load_or_save_game.cc'
> --- src/wui/load_or_save_game.cc	2018-10-26 07:09:29 +0000
> +++ src/wui/load_or_save_game.cc	2018-11-14 12:37:16 +0000
> @@ -269,12 +276,69 @@
>  		table_.focus();
>  	}
>  	if (do_delete) {
> +		// Failed deletions aren't a serious problem, we just catch the errors
> +		// and keep track to notify the player.
> +		std::set<uint32_t> failed_selections;
> +		bool failed;
>  		for (const uint32_t index : selections) {
> +			failed = false;
>  			const std::string& deleteme = get_filename(index);
> -			g_fs->fs_unlink(deleteme);
> -			if (filetype_ == FileType::kReplay) {
> -				g_fs->fs_unlink(deleteme + kSavegameExtension);
> -			}
> +			try {
> +				g_fs->fs_unlink(deleteme);
> +			} catch (const FileError& e) {
> +				log("player-requested file deletion failed: %s", e.what());
> +				failed = true;
> +			}
> +			if (filetype_ == FileType::kReplay) {
> +				try {
> +					g_fs->fs_unlink(deleteme + kSavegameExtension);
> +					// If at least one of the two relevant files of a replay are
> +					// successfully deleted then count it as success.
> +					// (From the player perspective the replay is gone.)
> +					failed = false;
> +					// If it was a multiplayer replay, also delete the synchstream. Do
> +					// it here, so it's only attempted if replay deletion was successful.
> +					if (g_fs->file_exists(deleteme + kSyncstreamExtension)) {
> +						g_fs->fs_unlink(deleteme + kSyncstreamExtension);
> +					}
> +				} catch (const FileError& e) {
> +					log("player-requested file deletion failed: %s", e.what());
> +				}
> +			}
> +			if (failed) {
> +				failed_selections.insert(index);
> +			}
> +		}
> +		if (!failed_selections.empty()) {
> +			// Notify the player.
> +			std::string caption = ngettext("Error Deleting File!",

Those are sins of the past ;)

> +			   "Error Deleting Files!", failed_selections.size());
> +			if (filetype_ == FileType::kReplay) {
> +				if (selections.size() == 1) {
> +					header = _("The replay could not be deleted.");
> +				} else {
> +					header =
> +					   (boost::format(ngettext("%s replay could not be deleted.",
> +					   "%s replays could not be deleted.", failed_selections.size()))
> +					   % failed_selections.size()).str();
> +				}
> +			} else {
> +				if (selections.size() == 1) {
> +					header = _("The game could not be deleted.");
> +				} else {
> +					header =
> +					   (boost::format(ngettext("%s game could not be deleted.",
> +					   "%s games could not be deleted.", failed_selections.size()))
> +					   % failed_selections.size()).str();
> +				}
> +			}
> +			std::string message = (boost::format("%s\n%s") % header
> +			   % filename_list_string(failed_selections)).str();
> +			UI::WLMessageBox msgBox(
> +			   parent_->get_parent()->get_parent(),
> +				 caption, message,
> +			   UI::WLMessageBox::MBoxType::kOk);
> +			msgBox.run<UI::Panel::Returncodes>();
>  		}
>  		fill_table();
>  


-- 
https://code.launchpad.net/~widelands-dev/widelands/handling-various-fs-errors/+merge/358767
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/handling-various-fs-errors.