Fixed those things.
Sure, I can do code reviews. I have looked at other active reviews before but
mostly ignored them because they were either trivial one-line fixes or part of
the code I wasn't familiar with at all yet.
Anything in particular I should look out for in code reviews?
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.)
Technically, in this branch a remaining savegame (when the deletion of the
replay file succeeded) would not be cleaned up (but this issue was there
before).
This is already fixed in the "robust file saving" branch though. When I added
the new cleanup routine there, I also fixed the other cleanup routines with it
(mainly catching file errors but also fixing this particular issue).
> + 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!",
I actually thought about this, but saw that in a Russian po-file all the
plurals are defined for strings without placeholder anyway. Should also have
checked whether it's actually displayed properly.
There was another older wrong use of this in this cc-file, I fixed this one,
too. (Or shouldn't I have? I figured there'd be new translations to do anyway.)
> + "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.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
