Add functions for reading/writing strings from/to datafile

Simplify the usage of datafile reader and writer by adding utility functions to read and write zero-terminated strings.

Validation for string data read from datafiles is improved.

Loading of external sounds in the editor is fixed,

src/engine/map.h

@@ -15,9 +15,10 @@ class IMap : public IInterface
 {
 	MACRO_INTERFACE("map", 0)
 public:
-	virtual void *GetData(int Index) = 0;
 	virtual int GetDataSize(int Index) const = 0;
+	virtual void *GetData(int Index) = 0;
 	virtual void *GetDataSwapped(int Index) = 0;
+	virtual bool GetDataString(int Index, char *pBuffer, size_t BufferSize) = 0;
 	virtual void UnloadData(int Index) = 0;
 	virtual int NumData() const = 0;
 

src/engine/shared/datafile.cpp

@@ -301,7 +301,7 @@ int CDataFileReader::GetDataSize(int Index) const
 	return Size;
 }
 
-void *CDataFileReader::GetDataImpl(int Index, int Swap)
+void *CDataFileReader::GetDataImpl(int Index, bool Swap)
 {
 	if(!m_pDataFile)
 	{
@@ -394,12 +394,27 @@ void *CDataFileReader::GetDataImpl(int Index, int Swap)
 
 void *CDataFileReader::GetData(int Index)
 {
-	return GetDataImpl(Index, 0);
+	return GetDataImpl(Index, false);
 }
 
 void *CDataFileReader::GetDataSwapped(int Index)
 {
-	return GetDataImpl(Index, 1);
+	return GetDataImpl(Index, true);
+}
+
+bool CDataFileReader::GetDataString(int Index, char *pBuffer, size_t BufferSize)
+{
+	const int DataSize = GetDataSize(Index);
+	const char *pData = static_cast<char *>(GetData(Index));
+	if(!DataSize || !pData)
+	{
+		pBuffer[0] = '\0';
+		UnloadData(Index);
+		return false;
+	}
+	str_copy(pBuffer, pData, minimum<size_t>(DataSize + 1, BufferSize));
+	UnloadData(Index);
+	return true;
 }
 
 void CDataFileReader::ReplaceData(int Index, char *pData, size_t Size)
@@ -745,6 +760,13 @@ int CDataFileWriter::AddDataSwapped(int Size, const void *pData)
 #endif
 }
 
+int CDataFileWriter::AddDataString(const char *pStr)
+{
+	if(pStr[0] == '\0')
+		return -1;
+	return AddData(str_length(pStr) + 1, pStr);
+}
+
 void CDataFileWriter::Finish()
 {
 	dbg_assert((bool)m_File, "file not open");

src/engine/shared/datafile.h

@@ -19,7 +19,7 @@ enum
 class CDataFileReader
 {
 	struct CDatafile *m_pDataFile;
-	void *GetDataImpl(int Index, int Swap);
+	void *GetDataImpl(int Index, bool Swap);
 	int GetFileDataSize(int Index) const;
 
 	int GetExternalItemType(int InternalType);
@@ -35,9 +35,10 @@ class CDataFileReader
 	bool IsOpen() const { return m_pDataFile != nullptr; }
 	IOHANDLE File() const;
 
+	int GetDataSize(int Index) const;
 	void *GetData(int Index);
 	void *GetDataSwapped(int Index); // makes sure that the data is 32bit LE ints when saved
-	int GetDataSize(int Index) const;
+	bool GetDataString(int Index, char *pBuffer, size_t BufferSize);
 	void ReplaceData(int Index, char *pData, size_t Size); // memory for data must have been allocated with malloc
 	void UnloadData(int Index);
 	int NumData() const;
@@ -129,6 +130,7 @@ class CDataFileWriter
 	bool Open(class IStorage *pStorage, const char *pFilename, int StorageType = IStorage::TYPE_SAVE);
 	int AddData(int Size, const void *pData, int CompressionLevel = Z_DEFAULT_COMPRESSION);
 	int AddDataSwapped(int Size, const void *pData);
+	int AddDataString(const char *pStr);
 	int AddItem(int Type, int ID, int Size, const void *pData);
 	void Finish();
 };

src/engine/shared/map.cpp

@@ -8,21 +8,26 @@
 
 CMap::CMap() = default;
 
-void *CMap::GetData(int Index)
+int CMap::GetDataSize(int Index) const
 {
-	return m_DataFile.GetData(Index);
+	return m_DataFile.GetDataSize(Index);
 }
 
-int CMap::GetDataSize(int Index) const
+void *CMap::GetData(int Index)
 {
-	return m_DataFile.GetDataSize(Index);
+	return m_DataFile.GetData(Index);
 }
 
 void *CMap::GetDataSwapped(int Index)
 {
 	return m_DataFile.GetDataSwapped(Index);
 }
 
+bool CMap::GetDataString(int Index, char *pBuffer, size_t BufferSize)
+{
+	return m_DataFile.GetDataString(Index, pBuffer, BufferSize);
+}
+
 void CMap::UnloadData(int Index)
 {
 	m_DataFile.UnloadData(Index);

src/engine/shared/map.h

@@ -17,9 +17,10 @@ class CMap : public IEngineMap
 
 	CDataFileReader *GetReader() { return &m_DataFile; }
 
-	void *GetData(int Index) override;
 	int GetDataSize(int Index) const override;
+	void *GetData(int Index) override;
 	void *GetDataSwapped(int Index) override;
+	bool GetDataString(int Index, char *pBuffer, size_t BufferSize) override;
 	void UnloadData(int Index) override;
 	int NumData() const override;
 

src/game/client/components/mapimages.cpp

@@ -99,35 +99,31 @@ void CMapImages::OnMapLoadImpl(class CLayers *pLayers, IMap *pMap)
 		}
 	}
 
-	int TextureLoadFlag = Graphics()->HasTextureArrays() ? IGraphics::TEXLOAD_TO_2D_ARRAY_TEXTURE : IGraphics::TEXLOAD_TO_3D_TEXTURE;
+	const int TextureLoadFlag = Graphics()->HasTextureArrays() ? IGraphics::TEXLOAD_TO_2D_ARRAY_TEXTURE : IGraphics::TEXLOAD_TO_3D_TEXTURE;
 
 	// load new textures
 	for(int i = 0; i < m_Count; i++)
 	{
-		int LoadFlag = (((m_aTextureUsedByTileOrQuadLayerFlag[i] & 1) != 0) ? TextureLoadFlag : 0) | (((m_aTextureUsedByTileOrQuadLayerFlag[i] & 2) != 0) ? 0 : (Graphics()->IsTileBufferingEnabled() ? IGraphics::TEXLOAD_NO_2D_TEXTURE : 0));
+		const int LoadFlag = (((m_aTextureUsedByTileOrQuadLayerFlag[i] & 1) != 0) ? TextureLoadFlag : 0) | (((m_aTextureUsedByTileOrQuadLayerFlag[i] & 2) != 0) ? 0 : (Graphics()->IsTileBufferingEnabled() ? IGraphics::TEXLOAD_NO_2D_TEXTURE : 0));
 		const CMapItemImage_v2 *pImg = (CMapItemImage_v2 *)pMap->GetItem(Start + i);
 		const int Format = pImg->m_Version < CMapItemImage_v2::CURRENT_VERSION ? CImageInfo::FORMAT_RGBA : pImg->m_Format;
+
+		char aName[IO_MAX_PATH_LENGTH];
+		if(!pMap->GetDataString(pImg->m_ImageName, aName, sizeof(aName)))
+			continue;
+
 		if(pImg->m_External)
 		{
 			char aPath[IO_MAX_PATH_LENGTH];
-			char *pName = (char *)pMap->GetData(pImg->m_ImageName);
-			str_format(aPath, sizeof(aPath), "mapres/%s.png", pName);
+			str_format(aPath, sizeof(aPath), "mapres/%s.png", aName);
 			m_aTextures[i] = Graphics()->LoadTexture(aPath, IStorage::TYPE_ALL, CImageInfo::FORMAT_AUTO, LoadFlag);
-			pMap->UnloadData(pImg->m_ImageName);
 		}
-		else if(Format != CImageInfo::FORMAT_RGBA)
-		{
-			m_aTextures[i] = Graphics()->InvalidTexture();
-			pMap->UnloadData(pImg->m_ImageName);
-		}
-		else
+		else if(Format == CImageInfo::FORMAT_RGBA)
 		{
 			void *pData = pMap->GetData(pImg->m_ImageData);
-			char *pName = (char *)pMap->GetData(pImg->m_ImageName);
-			char aTexName[128];
-			str_format(aTexName, sizeof(aTexName), "%s %s", "embedded:", pName);
+			char aTexName[IO_MAX_PATH_LENGTH];
+			str_format(aTexName, sizeof(aTexName), "embedded: %s", aName);
 			m_aTextures[i] = Graphics()->LoadTextureRaw(pImg->m_Width, pImg->m_Height, Format, pData, CImageInfo::FORMAT_RGBA, LoadFlag, aTexName);
-			pMap->UnloadData(pImg->m_ImageName);
 			pMap->UnloadData(pImg->m_ImageData);
 		}
 	}

src/game/client/components/mapsounds.cpp

@@ -30,14 +30,14 @@ void CMapSounds::OnMapLoad()
 	// load new samples
 	for(int i = 0; i < m_Count; i++)
 	{
-		m_aSounds[i] = 0;
-
 		CMapItemSound *pSound = (CMapItemSound *)pMap->GetItem(Start + i);
 		if(pSound->m_External)
 		{
+			char aName[IO_MAX_PATH_LENGTH];
+			if(!pMap->GetDataString(pSound->m_SoundName, aName, sizeof(aName)))
+				continue;
 			char aBuf[IO_MAX_PATH_LENGTH];
-			char *pName = (char *)pMap->GetData(pSound->m_SoundName);
-			str_format(aBuf, sizeof(aBuf), "mapres/%s.opus", pName);
+			str_format(aBuf, sizeof(aBuf), "mapres/%s.opus", aName);
 			m_aSounds[i] = Sound()->LoadOpus(aBuf);
 		}
 		else

src/game/editor/io.cpp

@@ -66,23 +66,10 @@ bool CEditorMap::Save(const char *pFileName)
 	{
 		CMapItemInfoSettings Item;
 		Item.m_Version = 1;
-
-		if(m_MapInfo.m_aAuthor[0])
-			Item.m_Author = Writer.AddData(str_length(m_MapInfo.m_aAuthor) + 1, m_MapInfo.m_aAuthor);
-		else
-			Item.m_Author = -1;
-		if(m_MapInfo.m_aVersion[0])
-			Item.m_MapVersion = Writer.AddData(str_length(m_MapInfo.m_aVersion) + 1, m_MapInfo.m_aVersion);
-		else
-			Item.m_MapVersion = -1;
-		if(m_MapInfo.m_aCredits[0])
-			Item.m_Credits = Writer.AddData(str_length(m_MapInfo.m_aCredits) + 1, m_MapInfo.m_aCredits);
-		else
-			Item.m_Credits = -1;
-		if(m_MapInfo.m_aLicense[0])
-			Item.m_License = Writer.AddData(str_length(m_MapInfo.m_aLicense) + 1, m_MapInfo.m_aLicense);
-		else
-			Item.m_License = -1;
+		Item.m_Author = Writer.AddDataString(m_MapInfo.m_aAuthor);
+		Item.m_MapVersion = Writer.AddDataString(m_MapInfo.m_aVersion);
+		Item.m_Credits = Writer.AddDataString(m_MapInfo.m_aCredits);
+		Item.m_License = Writer.AddDataString(m_MapInfo.m_aLicense);
 
 		Item.m_Settings = -1;
 		if(!m_vSettings.empty())
@@ -123,7 +110,7 @@ bool CEditorMap::Save(const char *pFileName)
 		Item.m_Width = pImg->m_Width;
 		Item.m_Height = pImg->m_Height;
 		Item.m_External = pImg->m_External;
-		Item.m_ImageName = Writer.AddData(str_length(pImg->m_aName) + 1, pImg->m_aName);
+		Item.m_ImageName = Writer.AddDataString(pImg->m_aName);
 		if(pImg->m_External)
 		{
 			Item.m_ImageData = -1;
@@ -162,7 +149,7 @@ bool CEditorMap::Save(const char *pFileName)
 		Item.m_Version = 1;
 
 		Item.m_External = 0;
-		Item.m_SoundName = Writer.AddData(str_length(pSound->m_aName) + 1, pSound->m_aName);
+		Item.m_SoundName = Writer.AddDataString(pSound->m_aName);
 		Item.m_SoundData = Writer.AddData(pSound->m_DataSize, pSound->m_pData);
 		Item.m_SoundDataSize = pSound->m_DataSize;
 
@@ -479,13 +466,13 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio
 					continue;
 
 				if(pItem->m_Author > -1)
-					str_copy(m_MapInfo.m_aAuthor, (char *)DataFile.GetData(pItem->m_Author));
+					DataFile.GetDataString(pItem->m_Author, m_MapInfo.m_aAuthor, sizeof(m_MapInfo.m_aAuthor));
 				if(pItem->m_MapVersion > -1)
-					str_copy(m_MapInfo.m_aVersion, (char *)DataFile.GetData(pItem->m_MapVersion));
+					DataFile.GetDataString(pItem->m_MapVersion, m_MapInfo.m_aVersion, sizeof(m_MapInfo.m_aVersion));
 				if(pItem->m_Credits > -1)
-					str_copy(m_MapInfo.m_aCredits, (char *)DataFile.GetData(pItem->m_Credits));
+					DataFile.GetDataString(pItem->m_Credits, m_MapInfo.m_aCredits, sizeof(m_MapInfo.m_aCredits));
 				if(pItem->m_License > -1)
-					str_copy(m_MapInfo.m_aLicense, (char *)DataFile.GetData(pItem->m_License));
+					DataFile.GetDataString(pItem->m_License, m_MapInfo.m_aLicense, sizeof(m_MapInfo.m_aLicense));
 
 				if(pItem->m_Version != 1 || ItemSize < (int)sizeof(CMapItemInfoSettings))
 					break;
@@ -512,17 +499,18 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio
 			for(int i = 0; i < Num; i++)
 			{
 				CMapItemImage_v2 *pItem = (CMapItemImage_v2 *)DataFile.GetItem(Start + i);
-				char *pName = (char *)DataFile.GetData(pItem->m_ImageName);
 
 				// copy base info
 				std::shared_ptr<CEditorImage> pImg = std::make_shared<CEditorImage>(m_pEditor);
 				pImg->m_External = pItem->m_External;
+				if(!DataFile.GetDataString(pItem->m_ImageName, pImg->m_aName, sizeof(pImg->m_aName)))
+					continue;
 
 				const int Format = pItem->m_Version < CMapItemImage_v2::CURRENT_VERSION ? CImageInfo::FORMAT_RGBA : pItem->m_Format;
 				if(pImg->m_External || (Format != CImageInfo::FORMAT_RGB && Format != CImageInfo::FORMAT_RGBA))
 				{
 					char aBuf[IO_MAX_PATH_LENGTH];
-					str_format(aBuf, sizeof(aBuf), "mapres/%s.png", pName);
+					str_format(aBuf, sizeof(aBuf), "mapres/%s.png", pImg->m_aName);
 
 					// load external
 					CEditorImage ImgInfo(m_pEditor);
@@ -554,10 +542,6 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio
 					pImg->m_Texture = m_pEditor->Graphics()->LoadTextureRaw(pImg->m_Width, pImg->m_Height, pImg->m_Format, pImg->m_pData, CImageInfo::FORMAT_AUTO, TextureLoadFlag);
 				}
 
-				// copy image name
-				if(pName)
-					str_copy(pImg->m_aName, pName);
-
 				// load auto mapper file
 				pImg->m_AutoMapper.Load(pImg->m_aName);
 
@@ -576,18 +560,19 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio
 			for(int i = 0; i < Num; i++)
 			{
 				CMapItemSound *pItem = (CMapItemSound *)DataFile.GetItem(Start + i);
-				char *pName = (char *)DataFile.GetData(pItem->m_SoundName);
 
 				// copy base info
 				std::shared_ptr<CEditorSound> pSound = std::make_shared<CEditorSound>(m_pEditor);
+				if(!DataFile.GetDataString(pItem->m_SoundName, pSound->m_aName, sizeof(pSound->m_aName)))
+					continue;
 
 				if(pItem->m_External)
 				{
 					char aBuf[IO_MAX_PATH_LENGTH];
-					str_format(aBuf, sizeof(aBuf), "mapres/%s.opus", pName);
+					str_format(aBuf, sizeof(aBuf), "mapres/%s.opus", pSound->m_aName);
 
 					// load external
-					if(m_pEditor->Storage()->ReadFile(pName, IStorage::TYPE_ALL, &pSound->m_pData, &pSound->m_DataSize))
+					if(m_pEditor->Storage()->ReadFile(aBuf, IStorage::TYPE_ALL, &pSound->m_pData, &pSound->m_DataSize))
 					{
 						pSound->m_SoundID = m_pEditor->Sound()->LoadOpusFromMem(pSound->m_pData, pSound->m_DataSize, true);
 					}
@@ -603,10 +588,6 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio
 					pSound->m_SoundID = m_pEditor->Sound()->LoadOpusFromMem(pSound->m_pData, pSound->m_DataSize, true);
 				}
 
-				// copy image name
-				if(pName)
-					str_copy(pSound->m_aName, pName);
-
 				m_vpSounds.push_back(pSound);
 
 				// unload image

src/test/datafile.cpp

@@ -58,3 +58,51 @@ TEST(Datafile, ExtendedType)
 		pStorage->RemoveFile(Info.m_aFilename, IStorage::TYPE_SAVE);
 	}
 }
+
+TEST(Datafile, StringData)
+{
+	auto pStorage = std::unique_ptr<IStorage>(CreateLocalStorage());
+	CTestInfo Info;
+
+	{
+		CDataFileWriter Writer;
+		Writer.Open(pStorage.get(), Info.m_aFilename);
+
+		EXPECT_EQ(Writer.AddDataString(""), -1); // Empty string is not added
+		EXPECT_EQ(Writer.AddDataString("Abc"), 0);
+		EXPECT_EQ(Writer.AddDataString("DDNet最好了"), 1);
+		EXPECT_EQ(Writer.AddDataString("aβい🐘"), 2);
+		EXPECT_EQ(Writer.AddData(3, "Abc"), 3); // Not zero-terminated
+		EXPECT_EQ(Writer.AddData(7, "foo\0bar"), 4); // Early zero-terminator
+		EXPECT_EQ(Writer.AddData(5, "xyz\xff\0"), 5); // Truncated UTF-8
+		EXPECT_EQ(Writer.AddData(4, "XYZ\xff"), 6); // Truncated UTF-8 and not zero-terminated
+
+		Writer.Finish();
+	}
+
+	{
+		CDataFileReader Reader;
+		Reader.Open(pStorage.get(), Info.m_aFilename, IStorage::TYPE_ALL);
+
+		char aBuf[256];
+		EXPECT_TRUE(Reader.GetDataString(0, aBuf, sizeof(aBuf)));
+		EXPECT_STREQ(aBuf, "Abc");
+		EXPECT_TRUE(Reader.GetDataString(1, aBuf, sizeof(aBuf)));
+		EXPECT_STREQ(aBuf, "DDNet最好了");
+		EXPECT_TRUE(Reader.GetDataString(2, aBuf, sizeof(aBuf)));
+		EXPECT_STREQ(aBuf, "aβい🐘");
+		EXPECT_TRUE(Reader.GetDataString(3, aBuf, sizeof(aBuf)));
+		EXPECT_STREQ(aBuf, "Abc");
+		EXPECT_TRUE(Reader.GetDataString(4, aBuf, sizeof(aBuf)));
+		EXPECT_STREQ(aBuf, "foo");
+		EXPECT_TRUE(Reader.GetDataString(5, aBuf, sizeof(aBuf)));
+		EXPECT_STREQ(aBuf, "xyz");
+		EXPECT_TRUE(Reader.GetDataString(6, aBuf, sizeof(aBuf)));
+		EXPECT_STREQ(aBuf, "XYZ");
+	}
+
+	if(!HasFailure())
+	{
+		pStorage->RemoveFile(Info.m_aFilename, IStorage::TYPE_SAVE);
+	}
+}