improve: change filestream/fileloader to std::ranges::copy (#2984)

Revert accidental prey loading change from: #2980 Refactored FileStream and FileLoader to utilize `std::ranges::copy` and other modern C++ features, replacing `memcpy` and `reinterpret_cast` for better readability and maintainability.
opentibiabr · Oct 18, 2024 · 665e90c · 665e90c
1 parent f05fb5d
commit 665e90c
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 35 deletions.
diff --git a/src/io/fileloader.hpp b/src/io/fileloader.hpp
@@ -22,9 +22,9 @@ namespace OTB {
 		Node &operator=(const Node &) = delete;
 
 		std::list<Node> children;
-		mio::mmap_source::const_iterator propsBegin;
-		mio::mmap_source::const_iterator propsEnd;
-		uint8_t type;
+		mio::mmap_source::const_iterator propsBegin {};
+		mio::mmap_source::const_iterator propsEnd {};
+		uint8_t type {};
 		enum NodeChar : uint8_t {
 			ESCAPE = 0xFD,
 			START = 0xFE,
@@ -67,12 +67,22 @@ class PropStream {
 
 	template <typename T>
 	bool read(T &ret) {
+		static_assert(std::is_trivially_copyable_v<T>, "Type T must be trivially copyable");
+
 		if (size() < sizeof(T)) {
 			return false;
 		}
 
-		memcpy(&ret, p, sizeof(T));
+		std::span<const char> charSpan { p, sizeof(T) };
+		auto byteSpan = std::as_bytes(charSpan);
+
+		std::array<std::byte, sizeof(T)> tempBuffer;
+		std::ranges::copy(byteSpan, tempBuffer.begin());
+
+		ret = std::bit_cast<T>(tempBuffer);
+
 		p += sizeof(T);
+
 		return true;
 	}
 
@@ -86,12 +96,14 @@ class PropStream {
 			return false;
 		}
 
-		char* str = new char[strLen + 1];
-		memcpy(str, p, strLen);
-		str[strLen] = 0;
-		ret.assign(str, strLen);
-		delete[] str;
+		std::vector<char> tempBuffer(strLen);
+		std::span<const char> sourceSpan(p, strLen);
+		std::ranges::copy(sourceSpan, tempBuffer.begin());
+
+		ret.assign(tempBuffer.begin(), tempBuffer.end());
+
 		p += strLen;
+
 		return true;
 	}
 
@@ -128,8 +140,11 @@ class PropWriteStream {
 
 	template <typename T>
 	void write(T add) {
-		char* addr = reinterpret_cast<char*>(&add);
-		std::copy(addr, addr + sizeof(T), std::back_inserter(buffer));
+		static_assert(std::is_trivially_copyable_v<T>, "Type T must be trivially copyable");
+
+		auto byteArray = std::bit_cast<std::array<char, sizeof(T)>>(add);
+		std::span<const char> charSpan(byteArray);
+		std::ranges::copy(charSpan, std::back_inserter(buffer));
 	}
 
 	void writeString(const std::string &str) {
@@ -140,7 +155,7 @@ class PropWriteStream {
 		}
 
 		write(static_cast<uint16_t>(strLength));
-		std::copy(str.begin(), str.end(), std::back_inserter(buffer));
+		std::ranges::copy(str, std::back_inserter(buffer));
 	}
 
 private:

diff --git a/src/io/filestream.cpp b/src/io/filestream.cpp
@@ -17,7 +17,8 @@ uint32_t FileStream::tell() const {
 
 void FileStream::seek(uint32_t pos) {
 	if (pos > m_data.size()) {
-		throw std::ios_base::failure("Seek failed");
+		g_logger().error("Seek failed");
+		return;
 	}
 	m_pos = pos;
 }
@@ -29,43 +30,49 @@ void FileStream::skip(uint32_t len) {
 uint32_t FileStream::size() const {
 	std::size_t size = m_data.size();
 	if (size > std::numeric_limits<uint32_t>::max()) {
-		throw std::overflow_error("File size exceeds uint32_t range");
+		g_logger().error("File size exceeds uint32_t range");
+		return {};
 	}
 
 	return static_cast<uint32_t>(size);
 }
 
 template <typename T>
 bool FileStream::read(T &ret, bool escape) {
+	static_assert(std::is_trivially_copyable_v<T>, "Type T must be trivially copyable");
+
 	const auto size = sizeof(T);
 
 	if (m_pos + size > m_data.size()) {
-		throw std::ios_base::failure("Read failed");
+		g_logger().error("Read failed");
+		return false;
 	}
 
 	std::array<uint8_t, sizeof(T)> array;
 	if (escape) {
-		for (int_fast8_t i = -1; ++i < size;) {
+		for (int_fast8_t i = 0; i < size; ++i) {
 			if (m_data[m_pos] == OTB::Node::ESCAPE) {
 				++m_pos;
 			}
 			array[i] = m_data[m_pos];
 			++m_pos;
 		}
-		memcpy(&ret, array.data(), size);
 	} else {
-		memcpy(&ret, &m_data[m_pos], size);
+		std::span<const uint8_t> sourceSpan(m_data.data() + m_pos, size);
+		std::ranges::copy(sourceSpan, array.begin());
 		m_pos += size;
 	}
 
+	ret = std::bit_cast<T>(array);
 	return true;
 }
 
 uint8_t FileStream::getU8() {
 	uint8_t v = 0;
 
 	if (m_pos + 1 > m_data.size()) {
-		throw std::ios_base::failure("Failed to getU8");
+		g_logger().error("Failed to getU8");
+		return {};
 	}
 
 	// Fast Escape Val
@@ -101,13 +108,14 @@ std::string FileStream::getString() {
 	std::string str;
 	if (const uint16_t len = getU16(); len > 0 && len < 8192) {
 		if (m_pos + len > m_data.size()) {
-			throw std::ios_base::failure("[FileStream::getString] - Read failed");
+			g_logger().error("[FileStream::getString] - Read failed");
+			return {};
 		}
 
 		str = { (char*)&m_data[m_pos], len };
 		m_pos += len;
 	} else if (len != 0) {
-		throw std::ios_base::failure("[FileStream::getString] - Read failed because string is too big");
+		g_logger().error("[FileStream::getString] - Read failed because string is too big");
 	}
 	return str;
 }

diff --git a/src/io/functions/iologindata_load_player.cpp b/src/io/functions/iologindata_load_player.cpp
@@ -734,10 +734,6 @@ void IOLoginDataLoad::loadPlayerPreyClass(std::shared_ptr<Player> player, DBResu
 		query << "SELECT * FROM `player_prey` WHERE `player_id` = " << player->getGUID();
 		if ((result = db.storeQuery(query.str()))) {
 			do {
-				auto selectedRaceId = result->getNumber<uint16_t>("raceid");
-				if (selectedRaceId == 0) {
-					continue;
-				}
 				auto slot = std::make_unique<PreySlot>(static_cast<PreySlot_t>(result->getNumber<uint16_t>("slot")));
 				auto state = static_cast<PreyDataState_t>(result->getNumber<uint16_t>("state"));
 				if (slot->id == PreySlot_Two && state == PreyDataState_Locked) {
@@ -749,7 +745,7 @@ void IOLoginDataLoad::loadPlayerPreyClass(std::shared_ptr<Player> player, DBResu
 				} else {
 					slot->state = state;
 				}
-				slot->selectedRaceId = selectedRaceId;
+				slot->selectedRaceId = result->getNumber<uint16_t>("raceid");
 				slot->option = static_cast<PreyOption_t>(result->getNumber<uint16_t>("option"));
 				slot->bonus = static_cast<PreyBonus_t>(result->getNumber<uint16_t>("bonus_type"));
 				slot->bonusRarity = static_cast<uint8_t>(result->getNumber<uint16_t>("bonus_rarity"));
@@ -785,10 +781,6 @@ void IOLoginDataLoad::loadPlayerTaskHuntingClass(std::shared_ptr<Player> player,
 		query << "SELECT * FROM `player_taskhunt` WHERE `player_id` = " << player->getGUID();
 		if ((result = db.storeQuery(query.str()))) {
 			do {
-				auto selectedRaceId = result->getNumber<uint16_t>("raceid");
-				if (selectedRaceId == 0) {
-					continue;
-				}
 				auto slot = std::make_unique<TaskHuntingSlot>(static_cast<PreySlot_t>(result->getNumber<uint16_t>("slot")));
 				auto state = static_cast<PreyTaskDataState_t>(result->getNumber<uint16_t>("state"));
 				if (slot->id == PreySlot_Two && state == PreyTaskDataState_Locked) {
@@ -800,7 +792,7 @@ void IOLoginDataLoad::loadPlayerTaskHuntingClass(std::shared_ptr<Player> player,
 				} else {
 					slot->state = state;
 				}
-				slot->selectedRaceId = selectedRaceId;
+				slot->selectedRaceId = result->getNumber<uint16_t>("raceid");
 				slot->upgrade = result->getNumber<bool>("upgrade");
 				slot->rarity = static_cast<uint8_t>(result->getNumber<uint16_t>("rarity"));
 				slot->currentKills = result->getNumber<uint16_t>("kills");

diff --git a/src/io/functions/iologindata_save_player.cpp b/src/io/functions/iologindata_save_player.cpp
@@ -606,7 +606,7 @@ bool IOLoginDataSave::savePlayerPreyClass(std::shared_ptr<Player> player) {
 					  << slot->freeRerollTimeStamp << ", ";
 
 				PropWriteStream propPreyStream;
-				std::ranges::for_each(slot->raceIdList.begin(), slot->raceIdList.end(), [&propPreyStream](uint16_t raceId) {
+				std::ranges::for_each(slot->raceIdList, [&propPreyStream](uint16_t raceId) {
 					propPreyStream.write<uint16_t>(raceId);
 				});
 
@@ -659,7 +659,7 @@ bool IOLoginDataSave::savePlayerTaskHuntingClass(std::shared_ptr<Player> player)
 				query << slot->freeRerollTimeStamp << ", ";
 
 				PropWriteStream propTaskHuntingStream;
-				std::ranges::for_each(slot->raceIdList.begin(), slot->raceIdList.end(), [&propTaskHuntingStream](uint16_t raceId) {
+				std::ranges::for_each(slot->raceIdList, [&propTaskHuntingStream](uint16_t raceId) {
 					propTaskHuntingStream.write<uint16_t>(raceId);
 				});
 

diff --git a/src/io/ioprey.cpp b/src/io/ioprey.cpp
@@ -65,8 +65,9 @@ void PreySlot::reloadMonsterGrid(std::vector<uint16_t> blackList, uint32_t level
 	// Disabling prey system if the server have less then 36 registered monsters on bestiary because:
 	// - Impossible to generate random lists without duplications on slots.
 	// - Stress the server with unnecessary loops.
-	std::map<uint16_t, std::string> bestiary = g_game().getBestiaryList();
+	const std::map<uint16_t, std::string> &bestiary = g_game().getBestiaryList();
 	if (bestiary.size() < 36) {
+		g_logger().error("[PreySlot::reloadMonsterGrid] - Bestiary size is less than 36, disabling prey system.");
 		return;
 	}
 
@@ -338,7 +339,7 @@ void IOPrey::parsePreyAction(std::shared_ptr<Player> player, PreySlot_t slotId,
 		} else if (player->getPreyWithMonster(raceId)) {
 			player->sendMessageDialog("This creature is already selected on another slot.");
 			return;
-		} else if (!mtype->info.isPreyable) {
+		} else if (mtype && !mtype->info.isPreyable) {
 			player->sendMessageDialog("This creature can't be select on prey. Please choose another one.");
 			return;
 		}