Use temporary file when saving editor map

Write the map to a temporary file first. When the map was saved to the temporary file successfully, first delete the existing map file having the real filename, then rename the temporary file to the real filename.

If deleting or renaming fails, show an error message popup and log an error message to the console.

The implementation is consistent with the way temporary files are utilized by Microsoft Word, so this should work on Windows.

See: https://support.microsoft.com/en-us/topic/description-of-how-word-creates-temporary-files-66b112fb-d2c0-8f40-a0be-70a367cc4c85

Different from ddnet#4482, this first deletes the old map file before renaming the temporary file. Although it appears that renaming a file would also override the target file, it could be that this does not work on all systems. Additionally, this adds descriptive error messages in the cases of failure.

Closes ddnet#4476.

Co-authored-by: Dennis Felsing <[email protected]>

src/game/editor/editor.cpp

@@ -7448,10 +7448,27 @@ void CEditor::HandleWriterFinishJobs()
  std::shared_ptr<CDataFileWriterFinishJob> pJob = m_WriterFinishJobs.front();
  if(pJob->Status() != IJob::STATE_DONE)
  return;
+ m_WriterFinishJobs.pop_front();
 
- char aBuf[IO_MAX_PATH_LENGTH + 32];
- str_format(aBuf, sizeof(aBuf), "saving '%s' done", pJob->GetFileName());
- Console()->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "editor", aBuf);
+ char aBuf[2 * IO_MAX_PATH_LENGTH + 128];
+ if(Storage()->FileExists(pJob->GetRealFileName(), IStorage::TYPE_SAVE) && !Storage()->RemoveFile(pJob->GetRealFileName(), IStorage::TYPE_SAVE))
+ {
+ str_format(aBuf, sizeof(aBuf), "Saving failed: Could not remove old map file '%s'.", pJob->GetRealFileName());
+ ShowFileDialogError("%s", aBuf);
+ Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "editor/save", aBuf);
+ return;
+ }
+
+ if(!Storage()->RenameFile(pJob->GetTempFileName(), pJob->GetRealFileName(), IStorage::TYPE_SAVE))
+ {
+ str_format(aBuf, sizeof(aBuf), "Saving failed: Could not move temporary map file '%s' to '%s'.", pJob->GetTempFileName(), pJob->GetRealFileName());
+ ShowFileDialogError("%s", aBuf);
+ Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "editor/save", aBuf);
+ return;
+ }
+
+ str_format(aBuf, sizeof(aBuf), "saving '%s' done", pJob->GetRealFileName());
+ Console()->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "editor/save", aBuf);
 
  // send rcon.. if we can
  if(Client()->RconAuthed())
@@ -7466,13 +7483,11 @@ void CEditor::HandleWriterFinishJobs()
  if(!mem_comp(ServerAddr.ip, aIpv4Localhost, sizeof(aIpv4Localhost)) || !mem_comp(ServerAddr.ip, aIpv6Localhost, sizeof(aIpv6Localhost)))
  {
  char aMapName[128];
- IStorage::StripPathAndExtension(pJob->GetFileName(), aMapName, sizeof(aMapName));
+ IStorage::StripPathAndExtension(pJob->GetRealFileName(), aMapName, sizeof(aMapName));
  if(!str_comp(aMapName, CurrentServerInfo.m_aMap))
  Client()->Rcon("reload");
  }
  }
-
- m_WriterFinishJobs.pop_front();
 }
 
 void CEditor::OnUpdate()

src/game/editor/editor.h

@@ -769,7 +769,8 @@ class CLayerGame : public CLayerTiles
 
 class CDataFileWriterFinishJob : public IJob
 {
- char m_aFileName[IO_MAX_PATH_LENGTH];
+ char m_aRealFileName[IO_MAX_PATH_LENGTH];
+ char m_aTempFileName[IO_MAX_PATH_LENGTH];
  CDataFileWriter m_Writer;
 
  void Run() override
@@ -778,13 +779,15 @@ class CDataFileWriterFinishJob : public IJob
  }
 
 public:
- CDataFileWriterFinishJob(const char *pFileName, CDataFileWriter &&Writer) :
+ CDataFileWriterFinishJob(const char *pRealFileName, const char *pTempFileName, CDataFileWriter &&Writer) :
  m_Writer(std::move(Writer))
  {
- str_copy(m_aFileName, pFileName);
+ str_copy(m_aRealFileName, pRealFileName);
+ str_copy(m_aTempFileName, pTempFileName);
  }
 
- const char *GetFileName() const { return m_aFileName; }
+ const char *GetRealFileName() const { return m_aRealFileName; }
+ const char *GetTempFileName() const { return m_aTempFileName; }
 };
 
 class CEditor : public IEditor

src/game/editor/io.cpp

@@ -34,21 +34,23 @@ struct CSoundSource_DEPRECATED
 bool CEditor::Save(const char *pFilename)
 {
  // Check if file with this name is already being saved at the moment
- if(std::any_of(std::begin(m_WriterFinishJobs), std::end(m_WriterFinishJobs), [pFilename](const std::shared_ptr<CDataFileWriterFinishJob> &Job) { return str_comp(pFilename, Job->GetFileName()) == 0; }))
+ if(std::any_of(std::begin(m_WriterFinishJobs), std::end(m_WriterFinishJobs), [pFilename](const std::shared_ptr<CDataFileWriterFinishJob> &Job) { return str_comp(pFilename, Job->GetRealFileName()) == 0; }))
  return false;
 
  return m_Map.Save(pFilename);
 }
 
 bool CEditorMap::Save(const char *pFileName)
 {
+ char aFileNameTmp[IO_MAX_PATH_LENGTH];
+ str_format(aFileNameTmp, sizeof(aFileNameTmp), "%s.%d.tmp", pFileName, pid());
  char aBuf[IO_MAX_PATH_LENGTH + 64];
- str_format(aBuf, sizeof(aBuf), "saving to '%s'...", pFileName);
+ str_format(aBuf, sizeof(aBuf), "saving to '%s'...", aFileNameTmp);
  m_pEditor->Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "editor", aBuf);
  CDataFileWriter Writer;
- if(!Writer.Open(m_pEditor->Storage(), pFileName))
+ if(!Writer.Open(m_pEditor->Storage(), aFileNameTmp))
  {
- str_format(aBuf, sizeof(aBuf), "failed to open file '%s'...", pFileName);
+ str_format(aBuf, sizeof(aBuf), "failed to open file '%s'...", aFileNameTmp);
  m_pEditor->Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "editor", aBuf);
  return false;
  }
@@ -420,7 +422,7 @@ bool CEditorMap::Save(const char *pFileName)
  }
 
  // finish the data file
- std::shared_ptr<CDataFileWriterFinishJob> pWriterFinishJob = std::make_shared<CDataFileWriterFinishJob>(pFileName, std::move(Writer));
+ std::shared_ptr<CDataFileWriterFinishJob> pWriterFinishJob = std::make_shared<CDataFileWriterFinishJob>(pFileName, aFileNameTmp, std::move(Writer));
  m_pEditor->Engine()->AddJob(pWriterFinishJob);
  m_pEditor->m_WriterFinishJobs.push_back(pWriterFinishJob);