Use std::vector<char> consistently for file operations

This made it possible to get rid of two GameSave constructors.

Also clean up Client::LoadSaveFile, Client::ReadFile, and Client::WriteFile in the process, and remove unused SaveRenderer::Render

src/PowderToySDL.cpp

@@ -916,15 +916,15 @@ int main(int argc, char * argv[])
 			{
 				try
 				{
-					std::vector<unsigned char> gameSaveData = Client::Ref().ReadFile(arguments["open"]);
-					if (!gameSaveData.size())
+					std::vector<char> gameSaveData;
+					if (!Client::Ref().ReadFile(gameSaveData, arguments["open"]))
 					{
 						new ErrorMessage("Error", "Could not read file");
 					}
 					else
 					{
 						SaveFile * newFile = new SaveFile(arguments["open"]);
-						GameSave * newSave = new GameSave(gameSaveData);
+						GameSave * newSave = new GameSave(std::move(gameSaveData));
 						newFile->SetGameSave(newSave);
 						gameController->LoadSaveFile(newFile);
 						delete newFile;
@@ -976,10 +976,10 @@ int main(int argc, char * argv[])
 				SaveInfo * newSave = Client::Ref().GetSave(saveId, 0);
 				if (!newSave)
 					throw std::runtime_error("Could not load save info");
-				std::vector<unsigned char> saveData = Client::Ref().GetSaveData(saveId, 0);
+				auto saveData = Client::Ref().GetSaveData(saveId, 0);
 				if (!saveData.size())
 					throw std::runtime_error(("Could not load save\n" + Client::Ref().GetLastError()).ToUtf8());
-				GameSave * newGameSave = new GameSave(saveData);
+				GameSave * newGameSave = new GameSave(std::move(saveData));
 				newSave->SetGameSave(newGameSave);
 
 				gameController->LoadSave(newSave);

src/client/Client.cpp

@@ -406,86 +406,6 @@ bool Client::DoInstallation()
 #endif
 }
 
-bool Client::WriteFile(std::vector<unsigned char> fileData, ByteString filename)
-{
-	bool saveError = false;
-	try
-	{
-		std::ofstream fileStream;
-		fileStream.open(filename, std::ios::binary);
-		if(fileStream.is_open())
-		{
-			fileStream.write((char*)&fileData[0], fileData.size());
-			fileStream.close();
-		}
-		else
-			saveError = true;
-	}
-	catch (std::exception & e)
-	{
-		std::cerr << "WriteFile:" << e.what() << std::endl;
-		saveError = true;
-	}
-	return saveError;
-}
-
-bool Client::WriteFile(std::vector<char> fileData, ByteString filename)
-{
-	bool saveError = false;
-	try
-	{
-		std::ofstream fileStream;
-		fileStream.open(filename, std::ios::binary);
-		if(fileStream.is_open())
-		{
-			fileStream.write(&fileData[0], fileData.size());
-			fileStream.close();
-		}
-		else
-			saveError = true;
-	}
-	catch (std::exception & e)
-	{
-		std::cerr << "WriteFile:" << e.what() << std::endl;
-		saveError = true;
-	}
-	return saveError;
-}
-
-std::vector<unsigned char> Client::ReadFile(ByteString filename)
-{
-	try
-	{
-		std::ifstream fileStream;
-		fileStream.open(filename, std::ios::binary);
-		if(fileStream.is_open())
-		{
-			fileStream.seekg(0, std::ios::end);
-			size_t fileSize = fileStream.tellg();
-			fileStream.seekg(0);
-
-			unsigned char * tempData = new unsigned char[fileSize];
-			fileStream.read((char *)tempData, fileSize);
-			fileStream.close();
-
-			std::vector<unsigned char> fileData;
-			fileData.insert(fileData.end(), tempData, tempData+fileSize);
-			delete[] tempData;
-
-			return fileData;
-		}
-		else
-		{
-			return std::vector<unsigned char>();
-		}
-	}
-	catch(std::exception & e)
-	{
-		std::cerr << "Readfile: " << e.what() << std::endl;
-		throw;
-	}
-}
-
 void Client::SetMessageOfTheDay(String message)
 {
 	messageOfTheDay = message;
@@ -1042,7 +962,7 @@ RequestStatus Client::ExecVote(int saveID, int direction)
 	return ret;
 }
 
-std::vector<unsigned char> Client::GetSaveData(int saveID, int saveDate)
+std::vector<char> Client::GetSaveData(int saveID, int saveDate)
 {
 	lastError = "";
 	int dataStatus;
@@ -1059,9 +979,9 @@ std::vector<unsigned char> Client::GetSaveData(int saveID, int saveDate)
 	ParseServerReturn(data, dataStatus, false);
 	if (data.size() && dataStatus == 200)
 	{
-		return std::vector<unsigned char>(data.begin(), data.end());
+		return std::vector<char>(data.begin(), data.end());
 	}
-	return std::vector<unsigned char>();
+	return {};
 }
 
 LoginStatus Client::Login(ByteString username, ByteString password, User & user)
@@ -1329,21 +1249,39 @@ SaveInfo * Client::GetSave(int saveID, int saveDate)
 
 SaveFile * Client::LoadSaveFile(ByteString filename)
 {
-	if (!Platform::FileExists(filename))
-		return nullptr;
-	SaveFile * file = new SaveFile(filename);
-	try
+	ByteString err;
+	SaveFile *file = nullptr;
+	if (Platform::FileExists(filename))
 	{
-		GameSave * tempSave = new GameSave(ReadFile(filename));
-		file->SetGameSave(tempSave);
+		file = new SaveFile(filename);
+		try
+		{
+			std::vector<char> data;
+			if (ReadFile(data, filename))
+			{
+				file->SetGameSave(new GameSave(std::move(data)));
+			}
+			else
+			{
+				err = "failed to open";
+			}
+		}
+		catch (const ParseException &e)
+		{
+			err = e.what();
+		}
 	}
-	catch (const ParseException &e)
+	else
 	{
+		err = "does not exist";
+	}
+	if (err.size())
+	{
+		std::cerr << "Client: " << filename << ": " << err << std::endl;
+		file->SetLoadingError(err.FromUtf8());
 #ifdef LUACONSOLE
-		luacon_ci->SetLastError(ByteString(e.what()).FromUtf8());
+		luacon_ci->SetLastError(err.FromUtf8());
 #endif
-		std::cerr << "Client: Invalid save file '" << filename << "': " << e.what() << std::endl;
-		file->SetLoadingError(ByteString(e.what()).FromUtf8());
 	}
 	return file;
 }
@@ -1855,3 +1793,30 @@ void Client::SetPrefUnicode(ByteString prop, String value)
 {
 	SetPref(prop, value.ToUtf8());
 }
+
+bool Client::ReadFile(std::vector<char> &fileData, ByteString filename)
+{
+	std::ifstream f(filename, std::ios::binary);
+	if (f) f.seekg(0, std::ios::end);
+	if (f) fileData.resize(f.tellg());
+	if (f) f.seekg(0);
+	if (f) f.read(&fileData[0], fileData.size());
+	if (!f)
+	{
+		std::cerr << "ReadFile: " << filename << ": " << strerror(errno) << std::endl;
+		return false;
+	}
+	return true;
+}
+
+bool Client::WriteFile(std::vector<char> fileData, ByteString filename)
+{
+	std::ofstream f(filename, std::ios::binary);
+	if (f) f.write(&fileData[0], fileData.size());
+	if (!f)
+	{
+		std::cerr << "WriteFile: " << filename << ": " << strerror(errno) << std::endl;
+		return false;
+	}
+	return true;
+}

src/client/Client.h

@@ -104,8 +104,6 @@ public:
 
 	bool DoInstallation();
 
-	std::vector<unsigned char> ReadFile(ByteString filename);
-
 	void AddServerNotification(std::pair<String, ByteString> notification);
 	std::vector<std::pair<String, ByteString> > GetServerNotifications();
 
@@ -115,7 +113,7 @@ public:
 	void Initialise(ByteString proxyString, bool disableNetwork);
 	bool IsFirstRun();
 
-	bool WriteFile(std::vector<unsigned char> fileData, ByteString filename);
+	bool ReadFile(std::vector<char> &fileData, ByteString filename);
 	bool WriteFile(std::vector<char> fileData, ByteString filename);
 
 	void AddListener(ClientListener * listener);
@@ -136,7 +134,7 @@ public:
 
 	RequestStatus AddComment(int saveID, String comment);
 
-	std::vector<unsigned char> GetSaveData(int saveID, int saveDate);
+	std::vector<char> GetSaveData(int saveID, int saveDate);
 
 	LoginStatus Login(ByteString username, ByteString password, User & user);
 	std::vector<SaveInfo*> * SearchSaves(int start, int count, String query, ByteString sort, ByteString category, int & resultCount);

src/client/GameSave.cpp

@@ -99,55 +99,6 @@ GameSave::GameSave(std::vector<char> data)
 	Collapse();
 }
 
-GameSave::GameSave(std::vector<unsigned char> data)
-{
-	blockWidth = 0;
-	blockHeight = 0;
-
-	InitData();
-	InitVars();
-	expanded = false;
-	hasOriginalData = true;
-	originalData = std::vector<char>(data.begin(), data.end());
-	try
-	{
-		Expand();
-	}
-	catch(ParseException & e)
-	{
-		std::cout << e.what() << std::endl;
-		dealloc();	//Free any allocated memory
-		throw;
-	}
-	Collapse();
-}
-
-GameSave::GameSave(char * data, int dataSize)
-{
-	blockWidth = 0;
-	blockHeight = 0;
-
-	InitData();
-	InitVars();
-	expanded = false;
-	hasOriginalData = true;
-	originalData = std::vector<char>(data, data+dataSize);
-#ifdef DEBUG
-	std::cout << "Creating Expanded save from data" << std::endl;
-#endif
-	try
-	{
-		Expand();
-	}
-	catch(ParseException & e)
-	{
-		std::cout << e.what() << std::endl;
-		dealloc();	//Free any allocated memory
-		throw;
-	}
-	//Collapse();
-}
-
 // Called on every new GameSave, including the copy constructor
 void GameSave::InitData()
 {

src/client/GameSave.h

@@ -117,9 +117,7 @@ public:
 	GameSave();
 	GameSave(const GameSave & save);
 	GameSave(int width, int height);
-	GameSave(char * data, int dataSize);
 	GameSave(std::vector<char> data);
-	GameSave(std::vector<unsigned char> data);
 	~GameSave();
 	void setSize(int width, int height);
 	char * Serialise(unsigned int & dataSize);

src/gui/filebrowser/FileBrowserActivity.cpp

@@ -47,10 +47,10 @@ class LoadFilesTask: public Task
 			SaveFile * saveFile = new SaveFile(directory + *iter);
 			try
 			{
-				std::vector<unsigned char> data = Client::Ref().ReadFile(directory + *iter);
-				if (data.empty())
+				std::vector<char> data;
+				if (!Client::Ref().ReadFile(data, directory + *iter))
 					continue;
-				GameSave * tempSave = new GameSave(data);
+				GameSave * tempSave = new GameSave(std::move(data));
 				saveFile->SetGameSave(tempSave);
 				saveFiles.push_back(saveFile);
 

src/gui/game/BitmapBrush.cpp

@@ -5,7 +5,7 @@
 
 #include "common/tpt-minmax.h"
 
-BitmapBrush::BitmapBrush(std::vector<unsigned char> newBitmap, ui::Point rectSize_):
+BitmapBrush::BitmapBrush(unsigned char *newBitmap, ui::Point rectSize_):
 	Brush(ui::Point(0, 0)),
 	origSize(0, 0)
 {

src/gui/game/BitmapBrush.h

@@ -17,7 +17,7 @@ protected:
 	ui::Point origSize;
 	unsigned char * origBitmap;
 public:
-	BitmapBrush(std::vector<unsigned char> newBitmap, ui::Point rectSize);
+	BitmapBrush(unsigned char *newBitmap, ui::Point rectSize);
 	void GenerateBitmap() override;
 	virtual ~BitmapBrush();
 };

src/gui/game/GameController.cpp

@@ -1210,7 +1210,7 @@ void GameController::OpenLocalSaveWindow(bool asCurrent)
 			std::vector<char> saveData = gameSave->Serialise();
 			if (saveData.size() == 0)
 				new ErrorMessage("Error", "Unable to serialize game data.");
-			else if (Client::Ref().WriteFile(saveData, gameModel->GetSaveFile()->GetName()))
+			else if (!Client::Ref().WriteFile(saveData, gameModel->GetSaveFile()->GetName()))
 				new ErrorMessage("Error", "Unable to write save file.");
 			else
 				gameModel->SetInfoTip("Saved Successfully");

src/gui/game/GameModel.cpp

@@ -481,8 +481,8 @@ void GameModel::BuildBrushList()
 	std::vector<ByteString> brushFiles = Platform::DirectorySearch(BRUSH_DIR, "", { ".ptb" });
 	for (size_t i = 0; i < brushFiles.size(); i++)
 	{
-		std::vector<unsigned char> brushData = Client::Ref().ReadFile(BRUSH_DIR + ByteString(PATH_SEP) + brushFiles[i]);
-		if(!brushData.size())
+		std::vector<char> brushData;
+		if (!Client::Ref().ReadFile(brushData, BRUSH_DIR + ByteString(PATH_SEP) + brushFiles[i]))
 		{
 			std::cout << "Brushes: Skipping " << brushFiles[i] << ". Could not open" << std::endl;
 			continue;
@@ -493,7 +493,7 @@ void GameModel::BuildBrushList()
 			std::cout << "Brushes: Skipping " << brushFiles[i] << ". Invalid bitmap size" << std::endl;
 			continue;
 		}
-		brushList.push_back(new BitmapBrush(brushData, ui::Point(dimension, dimension)));
+		brushList.push_back(new BitmapBrush(reinterpret_cast<unsigned char *>(&brushData[0]), ui::Point(dimension, dimension)));
 	}
 
 	if (hasStoredRadius && (size_t)currentBrush < brushList.size())

src/gui/preview/PreviewModel.cpp

@@ -301,7 +301,7 @@ void PreviewModel::Update()
 		if (status == 200 && ret.size())
 		{
 			delete saveData;
-			saveData = new std::vector<unsigned char>(ret.begin(), ret.end());
+			saveData = new std::vector<char>(ret.begin(), ret.end());
 			if (saveInfo && saveData)
 				OnSaveReady();
 		}

src/gui/preview/PreviewModel.h

@@ -19,7 +19,7 @@ class PreviewModel
 	bool canOpen;
 	std::vector<PreviewView*> observers;
 	SaveInfo * saveInfo;
-	std::vector<unsigned char> * saveData;
+	std::vector<char> * saveData;
 	std::vector<SaveComment*> * saveComments;
 	void notifySaveChanged();
 	void notifySaveCommentsChanged();

src/gui/save/LocalSaveActivity.cpp

@@ -115,7 +115,7 @@ void LocalSaveActivity::saveWrite(ByteString finalFilename)
 	std::vector<char> saveData = gameSave->Serialise();
 	if (saveData.size() == 0)
 		new ErrorMessage("Error", "Unable to serialize game data.");
-	else if (Client::Ref().WriteFile(saveData, finalFilename))
+	else if (!Client::Ref().WriteFile(saveData, finalFilename))
 		new ErrorMessage("Error", "Unable to write save file.");
 	else
 	{

src/simulation/SaveRenderer.cpp

@@ -165,27 +165,6 @@ VideoBuffer * SaveRenderer::Render(GameSave * save, bool decorations, bool fire,
 	return tempThumb;
 }
 
-VideoBuffer * SaveRenderer::Render(unsigned char * saveData, int dataSize, bool decorations, bool fire)
-{
-	std::lock_guard<std::mutex> g(renderMutex);
-
-	GameSave * tempSave;
-	try {
-		tempSave = new GameSave((char*)saveData, dataSize);
-	} catch (std::exception & e) {
-
-		//Todo: make this look a little less shit
-		VideoBuffer * buffer = new VideoBuffer(64, 64);
-		buffer->BlendCharacter(32, 32, 'x', 255, 255, 255, 255);
-
-		return buffer;
-	}
-	VideoBuffer * thumb = Render(tempSave, decorations, fire);
-	delete tempSave;
-
-	return thumb;
-}
-
 SaveRenderer::~SaveRenderer()
 {
 }