Skip to content

Commit

Permalink
#5231: Remove check if output path is writeable since it doesn't tell…
Browse files Browse the repository at this point in the history
… anything about whether we can move the file by fs::rename.

This simplifies the backup saving code a bit, and removes the references to wxutil namespace
  • Loading branch information
codereader committed May 24, 2020
1 parent 7e07d33 commit fc015cb
Showing 1 changed file with 26 additions and 37 deletions.
63 changes: 26 additions & 37 deletions radiant/map/MapResource.cpp
Expand Up @@ -9,15 +9,13 @@
#include "iarchive.h"
#include "igroupnode.h"
#include "ifilesystem.h"
#include "imainframe.h"
#include "iregistry.h"
#include "imapinfofile.h"

#include "map/Map.h"
#include "map/RootNode.h"
#include "mapfile.h"
#include "gamelib.h"
#include "wxutil/dialog/MessageBox.h"
#include "debugging/debugging.h"
#include "os/path.h"
#include "os/file.h"
Expand Down Expand Up @@ -181,34 +179,35 @@ bool MapResource::saveBackup()
fs::path auxFile = fullpath;
auxFile.replace_extension(_infoFileExt);

if (os::fileIsWritable(fullpath.string()))
{
fs::path backup = fullpath;
backup.replace_extension(".bak");
fs::path backup = fullpath;
backup.replace_extension(".bak");

// replace_extension() doesn't accept something like ".darkradiant.bak", so roll our own
fs::path auxFileBackup = auxFile.string() + ".bak";

bool errorOccurred = false;
// replace_extension() doesn't accept something like ".darkradiant.bak", so roll our own
fs::path auxFileBackup = auxFile.string() + ".bak";

try
{
// remove backup
if (fs::exists(backup))
{
fs::remove(backup);
}
bool errorOccurred = false;

// rename current to backup
fs::rename(fullpath, backup);
}
catch (fs::filesystem_error& ex)
try
{
// remove backup
if (fs::exists(backup))
{
rWarning() << "Error while creating backups: " << ex.what() <<
", the file is possibly opened by the game." << std::endl;
errorOccurred = true;
fs::remove(backup);
}

// rename current to backup
fs::rename(fullpath, backup);
}
catch (fs::filesystem_error& ex)
{
rWarning() << "Error while creating backups: " << ex.what() <<
", the file is possibly opened by the game." << std::endl;
errorOccurred = true;
}

// Handle the .darkradiant file only if the above succeeded
if (!errorOccurred)
{
try
{
// remove aux file backup
Expand All @@ -224,25 +223,15 @@ bool MapResource::saveBackup()
fs::rename(auxFile, auxFileBackup);
}
}
catch (fs::filesystem_error& ex)
catch (fs::filesystem_error & ex)
{
rWarning() << "Error while creating backups: " << ex.what() <<
rWarning() << "Error while creating backups: " << ex.what() <<
", the file is possibly opened by the game." << std::endl;
errorOccurred = true;
}

return !errorOccurred;
}
else
{
rError() << "map path is not writeable: " << fullpath.string() << std::endl;

// File is write-protected
wxutil::Messagebox::ShowError(
fmt::format(_("File is write-protected: {0}"), fullpath.string()));

return false;
}
return !errorOccurred;
}

return false;
Expand Down

0 comments on commit fc015cb

Please sign in to comment.