feat: Packed asset bundle improvements (#1754)

* Removed some unneccessary indirection and added const-correctness

* improved packed asset bundle error messages

* rephrase the string_view initialization to satisfy microsoft

* change forward slashes to back slashes and let us never speak of this again

* make crc32b function static

* remove redundant 'static'

---------

Co-authored-by: jadebenn <9892985+jadebenn@users.noreply.github.com>

dCommon/dClient/AssetManager.cpp

@@ -6,6 +6,9 @@
 
 #include "zlib.h"
 
+constexpr uint32_t CRC32_INIT = 0xFFFFFFFF;
+constexpr auto NULL_TERMINATOR = std::string_view{"\0\0\0", 4};
+
 AssetManager::AssetManager(const std::filesystem::path& path) {
 	if (!std::filesystem::is_directory(path)) {
 		throw std::runtime_error("Attempted to load asset bundle (" + path.string() + ") however it is not a valid directory.");
@@ -18,12 +21,20 @@ AssetManager::AssetManager(const std::filesystem::path& path) {
 
 		m_RootPath = m_Path;
 		m_ResPath = (m_Path / "client" / "res");
-	} else if (std::filesystem::exists(m_Path / ".." / "versions") && std::filesystem::exists(m_Path / "res")) {
+	} else if (std::filesystem::exists(m_Path / "res" / "pack")) {
+		if (!std::filesystem::exists(m_Path / ".." / "versions")) {
+			throw std::runtime_error("No \"versions\" directory found in the parent directories of \"res\" - packed asset bundle cannot be loaded.");
+		}
+		
 		m_AssetBundleType = eAssetBundleType::Packed;
 
 		m_RootPath = (m_Path / "..");
 		m_ResPath = (m_Path / "res");
-	} else if (std::filesystem::exists(m_Path / "pack") && std::filesystem::exists(m_Path / ".." / ".." / "versions")) {
+	} else if (std::filesystem::exists(m_Path / "pack")) {
+		if (!std::filesystem::exists(m_Path / ".." / ".." / "versions")) {
+			throw std::runtime_error("No \"versions\" directory found in the parent directories of \"res\" - packed asset bundle cannot be loaded.");
+		}
+		
 		m_AssetBundleType = eAssetBundleType::Packed;
 
 		m_RootPath = (m_Path / ".." / "..");
@@ -48,6 +59,7 @@ AssetManager::AssetManager(const std::filesystem::path& path) {
 			break;
 		}
 		case eAssetBundleType::None:
+			[[fallthrough]];
 		case eAssetBundleType::Unpacked: {
 			break;
 		}
@@ -55,19 +67,10 @@ AssetManager::AssetManager(const std::filesystem::path& path) {
 }
 
 void AssetManager::LoadPackIndex() {
-	m_PackIndex = new PackIndex(m_RootPath);
+	m_PackIndex = PackIndex(m_RootPath);
 }
 
-std::filesystem::path AssetManager::GetResPath() {
+bool AssetManager::HasFile(std::string fixedName) const {
-	return m_ResPath;
-}
-
-eAssetBundleType AssetManager::GetAssetBundleType() {
-	return m_AssetBundleType;
-}
-
-bool AssetManager::HasFile(const char* name) {
-	auto fixedName = std::string(name);
 	std::transform(fixedName.begin(), fixedName.end(), fixedName.begin(), [](uint8_t c) { return std::tolower(c); });
 
 	// Special case for unpacked client have BrickModels in upper case
@@ -81,8 +84,7 @@ bool AssetManager::HasFile(const char* name) {
 	std::replace(fixedName.begin(), fixedName.end(), '/', '\\');
 	if (fixedName.rfind("client\\res\\", 0) != 0) fixedName = "client\\res\\" + fixedName;
 
-	uint32_t crc = crc32b(0xFFFFFFFF, reinterpret_cast<uint8_t*>(const_cast<char*>(fixedName.c_str())), fixedName.size());
+	const auto crc = crc32b(crc32b(CRC32_INIT, fixedName), NULL_TERMINATOR);
-	crc = crc32b(crc, reinterpret_cast<Bytef*>(const_cast<char*>("\0\0\0\0")), 4);
 
 	for (const auto& item : this->m_PackIndex->GetPackFileIndices()) {
 		if (item.m_Crc == crc) {
@@ -93,8 +95,7 @@ bool AssetManager::HasFile(const char* name) {
 	return false;
 }
 
-bool AssetManager::GetFile(const char* name, char** data, uint32_t* len) {
+bool AssetManager::GetFile(std::string fixedName, char** data, uint32_t* len) const {
-	auto fixedName = std::string(name);
 	std::transform(fixedName.begin(), fixedName.end(), fixedName.begin(), [](uint8_t c) { return std::tolower(c); });
 	std::replace(fixedName.begin(), fixedName.end(), '\\', '/'); // On the off chance someone has the wrong slashes, force forward slashes
 
@@ -129,8 +130,7 @@ bool AssetManager::GetFile(const char* name, char** data, uint32_t* len) {
 		fixedName = "client\\res\\" + fixedName;
 	}
 	int32_t packIndex = -1;
-	uint32_t crc = crc32b(0xFFFFFFFF, reinterpret_cast<uint8_t*>(const_cast<char*>(fixedName.c_str())), fixedName.size());
+	auto crc = crc32b(crc32b(CRC32_INIT, fixedName), NULL_TERMINATOR);
-	crc = crc32b(crc, reinterpret_cast<Bytef*>(const_cast<char*>("\0\0\0\0")), 4);
 
 	for (const auto& item : this->m_PackIndex->GetPackFileIndices()) {
 		if (item.m_Crc == crc) {
@@ -144,15 +144,13 @@ bool AssetManager::GetFile(const char* name, char** data, uint32_t* len) {
 		return false;
 	}
 
-	auto packs = this->m_PackIndex->GetPacks();
+	const auto& pack = this->m_PackIndex->GetPacks().at(packIndex);
-	auto* pack = packs.at(packIndex);
+	const bool success = pack.ReadFileFromPack(crc, data, len);
-
-	bool success = pack->ReadFileFromPack(crc, data, len);
 
 	return success;
 }
 
-AssetStream AssetManager::GetFile(const char* name) {
+AssetStream AssetManager::GetFile(const char* name) const {
 	char* buf; uint32_t len;
 
 	bool success = this->GetFile(name, &buf, &len);
@@ -160,23 +158,15 @@ AssetStream AssetManager::GetFile(const char* name) {
 	return AssetStream(buf, len, success);
 }
 
-uint32_t AssetManager::crc32b(uint32_t base, uint8_t* message, size_t l) {
+uint32_t AssetManager::crc32b(uint32_t crc, const std::string_view message) {
-	size_t i, j;
+	for (const auto byte : message) {
-	uint32_t crc, msb;
-
-	crc = base;
-	for (i = 0; i < l; i++) {
 		// xor next byte to upper bits of crc
-		crc ^= (static_cast<unsigned int>(message[i]) << 24);
+		crc ^= (static_cast<uint32_t>(std::bit_cast<uint8_t>(byte)) << 24);
-		for (j = 0; j < 8; j++) { // Do eight times.
+		for (size_t _ = 0; _ < 8; _++) { // Do eight times.
-			msb = crc >> 31;
+			const uint32_t msb = crc >> 31;
 			crc <<= 1;
 			crc ^= (0 - msb) & 0x04C11DB7;
 		}
 	}
 	return crc; // don't complement crc on output
 }
-
-AssetManager::~AssetManager() {
-	delete m_PackIndex;
-}

dCommon/dClient/AssetManager.h

@@ -61,23 +61,32 @@ struct AssetStream : std::istream {
 class AssetManager {
 public:
 	AssetManager(const std::filesystem::path& path);
-	~AssetManager();
 
-	std::filesystem::path GetResPath();
+	[[nodiscard]]
-	eAssetBundleType GetAssetBundleType();
+	const std::filesystem::path& GetResPath() const {
+		return m_ResPath;
+	}
+	
+	[[nodiscard]]
+	eAssetBundleType GetAssetBundleType() const {
+		return m_AssetBundleType;
+	}
 
-	bool HasFile(const char* name);
+	[[nodiscard]]
-	bool GetFile(const char* name, char** data, uint32_t* len);
+	bool HasFile(std::string name) const;
-	AssetStream GetFile(const char* name);
+
+	[[nodiscard]]
+	bool GetFile(std::string name, char** data, uint32_t* len) const;
+
+	[[nodiscard]]
+	AssetStream GetFile(const char* name) const;
 
 private:
 	void LoadPackIndex();
 
 	// Modified crc algorithm (mpeg2)
 	// Reference: https://stackoverflow.com/questions/54339800/how-to-modify-crc-32-to-crc-32-mpeg-2
-	inline uint32_t crc32b(uint32_t base, uint8_t* message, size_t l);
+	static inline uint32_t crc32b(uint32_t crc, std::string_view message);
-
-	bool m_SuccessfullyLoaded;
 
 	std::filesystem::path m_Path;
 	std::filesystem::path m_RootPath;
@@ -85,5 +94,5 @@ private:
 
 	eAssetBundleType m_AssetBundleType = eAssetBundleType::None;
 
-	PackIndex* m_PackIndex;
+	std::optional<PackIndex> m_PackIndex;
 };

dCommon/dClient/Pack.cpp

@@ -21,19 +21,20 @@ Pack::Pack(const std::filesystem::path& filePath) {
 
 	m_FileStream.seekg(recordCountPos, std::ios::beg);
 
-	BinaryIO::BinaryRead<uint32_t>(m_FileStream, m_RecordCount);
+	uint32_t recordCount = 0;
+	BinaryIO::BinaryRead<uint32_t>(m_FileStream, recordCount);
 
-	for (int i = 0; i < m_RecordCount; i++) {
+	m_Records.reserve(recordCount);
+	std::generate_n(std::back_inserter(m_Records), recordCount, [&] {
 		PackRecord record;
 		BinaryIO::BinaryRead<PackRecord>(m_FileStream, record);
-
+		return record;
-		m_Records.push_back(record);
+	});
-	}
 
 	m_FileStream.close();
 }
 
-bool Pack::HasFile(uint32_t crc) {
+bool Pack::HasFile(const uint32_t crc) const {
 	for (const auto& record : m_Records) {
 		if (record.m_Crc == crc) {
 			return true;
@@ -43,7 +44,7 @@ bool Pack::HasFile(uint32_t crc) {
 	return false;
 }
 
-bool Pack::ReadFileFromPack(uint32_t crc, char** data, uint32_t* len) {
+bool Pack::ReadFileFromPack(const uint32_t crc, char** data, uint32_t* len) const {
 	// Time for some wacky C file reading for speed reasons
 
 	PackRecord pkRecord{};

dCommon/dClient/Pack.h

@@ -24,16 +24,17 @@ struct PackRecord {
 class Pack {
 public:
 	Pack(const std::filesystem::path& filePath);
-	~Pack() = default;
 
-	bool HasFile(uint32_t crc);
+	[[nodiscard]]
-	bool ReadFileFromPack(uint32_t crc, char** data, uint32_t* len);
+	bool HasFile(uint32_t crc) const;
+
+	[[nodiscard]]
+	bool ReadFileFromPack(uint32_t crc, char** data, uint32_t* len) const;
 private:
 	std::ifstream m_FileStream;
 	std::filesystem::path m_FilePath;
 
 	char m_Version[7];
 
-	uint32_t m_RecordCount;
 	std::vector<PackRecord> m_Records;
 };

dCommon/dClient/PackIndex.cpp

@@ -6,38 +6,32 @@
 PackIndex::PackIndex(const std::filesystem::path& filePath) {
 	m_FileStream = std::ifstream(filePath / "versions" / "primary.pki", std::ios::in | std::ios::binary);
 
+	uint32_t packPathCount = 0;
 	BinaryIO::BinaryRead<uint32_t>(m_FileStream, m_Version);
-	BinaryIO::BinaryRead<uint32_t>(m_FileStream, m_PackPathCount);
+	BinaryIO::BinaryRead<uint32_t>(m_FileStream, packPathCount);
 	
-	m_PackPaths.resize(m_PackPathCount);
+	m_PackPaths.resize(packPathCount);
 	for (auto& item : m_PackPaths) {
 		BinaryIO::ReadString<uint32_t>(m_FileStream, item, BinaryIO::ReadType::String);
 	}
 
-	BinaryIO::BinaryRead<uint32_t>(m_FileStream, m_PackFileIndexCount);
+	uint32_t packFileIndexCount = 0;
+	BinaryIO::BinaryRead<uint32_t>(m_FileStream, packFileIndexCount);
 
-	for (int i = 0; i < m_PackFileIndexCount; i++) {
+	m_PackFileIndices.reserve(packFileIndexCount);
+	std::generate_n(std::back_inserter(m_PackFileIndices), packFileIndexCount, [&] {
 		PackFileIndex packFileIndex;
 		BinaryIO::BinaryRead<PackFileIndex>(m_FileStream, packFileIndex);
-
+		return packFileIndex;
-		m_PackFileIndices.push_back(packFileIndex);
+	});
-	}
 
 	LOG("Loaded pack catalog with %i pack files and %i files", m_PackPaths.size(), m_PackFileIndices.size());
 
+	m_Packs.reserve(m_PackPaths.size());
 	for (auto& item : m_PackPaths) {
 		std::replace(item.begin(), item.end(), '\\', '/');
-
+		m_Packs.emplace_back(filePath / item);
-		auto* pack = new Pack(filePath / item);
-
-		m_Packs.push_back(pack);
 	}
 
 	m_FileStream.close();
 }
-
-PackIndex::~PackIndex() {
-	for (const auto* item : m_Packs) {
-		delete item;
-	}
-}

dCommon/dClient/PackIndex.h

@@ -21,20 +21,23 @@ struct PackFileIndex {
 class PackIndex {
 public:
 	PackIndex(const std::filesystem::path& filePath);
-	~PackIndex();
 
-	const std::vector<std::string>& GetPackPaths() { return m_PackPaths; }
+	[[nodiscard]]
-	const std::vector<PackFileIndex>& GetPackFileIndices() { return m_PackFileIndices; }
+	const std::vector<std::string>& GetPackPaths() const { return m_PackPaths; }
-	const std::vector<Pack*>& GetPacks() { return m_Packs; }
+
+	[[nodiscard]]
+	const std::vector<PackFileIndex>& GetPackFileIndices() const { return m_PackFileIndices; }
+
+	[[nodiscard]]
+	const std::vector<Pack>& GetPacks() const { return m_Packs; }
 private:
 	std::ifstream m_FileStream;
 
 	uint32_t m_Version;
 
-	uint32_t m_PackPathCount;
 	std::vector<std::string> m_PackPaths;
-	uint32_t m_PackFileIndexCount;
+
 	std::vector<PackFileIndex> m_PackFileIndices;
 
-	std::vector<Pack*> m_Packs;
+	std::vector<Pack> m_Packs;
 };