From 665e90c169ca292decdc33611e304e946d6ab432 Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Thu, 17 Oct 2024 21:56:41 -0300 Subject: [PATCH 1/3] 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. --- src/io/fileloader.hpp | 39 ++++++++++++++------ src/io/filestream.cpp | 26 ++++++++----- src/io/functions/iologindata_load_player.cpp | 12 +----- src/io/functions/iologindata_save_player.cpp | 4 +- src/io/ioprey.cpp | 5 ++- 5 files changed, 51 insertions(+), 35 deletions(-) diff --git a/src/io/fileloader.hpp b/src/io/fileloader.hpp index 2069487202f..863837464ae 100644 --- a/src/io/fileloader.hpp +++ b/src/io/fileloader.hpp @@ -22,9 +22,9 @@ namespace OTB { Node &operator=(const Node &) = delete; std::list 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 bool read(T &ret) { + static_assert(std::is_trivially_copyable_v, "Type T must be trivially copyable"); + if (size() < sizeof(T)) { return false; } - memcpy(&ret, p, sizeof(T)); + std::span charSpan { p, sizeof(T) }; + auto byteSpan = std::as_bytes(charSpan); + + std::array tempBuffer; + std::ranges::copy(byteSpan, tempBuffer.begin()); + + ret = std::bit_cast(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 tempBuffer(strLen); + std::span 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 void write(T add) { - char* addr = reinterpret_cast(&add); - std::copy(addr, addr + sizeof(T), std::back_inserter(buffer)); + static_assert(std::is_trivially_copyable_v, "Type T must be trivially copyable"); + + auto byteArray = std::bit_cast>(add); + std::span 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(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 index 71b2dd4aa2f..687c2501df9 100644 --- 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,7 +30,8 @@ void FileStream::skip(uint32_t len) { uint32_t FileStream::size() const { std::size_t size = m_data.size(); if (size > std::numeric_limits::max()) { - throw std::overflow_error("File size exceeds uint32_t range"); + g_logger().error("File size exceeds uint32_t range"); + return {}; } return static_cast(size); @@ -37,27 +39,31 @@ uint32_t FileStream::size() const { template bool FileStream::read(T &ret, bool escape) { + static_assert(std::is_trivially_copyable_v, "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 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 sourceSpan(m_data.data() + m_pos, size); + std::ranges::copy(sourceSpan, array.begin()); m_pos += size; } + ret = std::bit_cast(array); return true; } @@ -65,7 +71,8 @@ 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 index cb24c092ebb..5b2621c1cbf 100644 --- 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, DBResu query << "SELECT * FROM `player_prey` WHERE `player_id` = " << player->getGUID(); if ((result = db.storeQuery(query.str()))) { do { - auto selectedRaceId = result->getNumber("raceid"); - if (selectedRaceId == 0) { - continue; - } auto slot = std::make_unique(static_cast(result->getNumber("slot"))); auto state = static_cast(result->getNumber("state")); if (slot->id == PreySlot_Two && state == PreyDataState_Locked) { @@ -749,7 +745,7 @@ void IOLoginDataLoad::loadPlayerPreyClass(std::shared_ptr player, DBResu } else { slot->state = state; } - slot->selectedRaceId = selectedRaceId; + slot->selectedRaceId = result->getNumber("raceid"); slot->option = static_cast(result->getNumber("option")); slot->bonus = static_cast(result->getNumber("bonus_type")); slot->bonusRarity = static_cast(result->getNumber("bonus_rarity")); @@ -785,10 +781,6 @@ void IOLoginDataLoad::loadPlayerTaskHuntingClass(std::shared_ptr player, query << "SELECT * FROM `player_taskhunt` WHERE `player_id` = " << player->getGUID(); if ((result = db.storeQuery(query.str()))) { do { - auto selectedRaceId = result->getNumber("raceid"); - if (selectedRaceId == 0) { - continue; - } auto slot = std::make_unique(static_cast(result->getNumber("slot"))); auto state = static_cast(result->getNumber("state")); if (slot->id == PreySlot_Two && state == PreyTaskDataState_Locked) { @@ -800,7 +792,7 @@ void IOLoginDataLoad::loadPlayerTaskHuntingClass(std::shared_ptr player, } else { slot->state = state; } - slot->selectedRaceId = selectedRaceId; + slot->selectedRaceId = result->getNumber("raceid"); slot->upgrade = result->getNumber("upgrade"); slot->rarity = static_cast(result->getNumber("rarity")); slot->currentKills = result->getNumber("kills"); diff --git a/src/io/functions/iologindata_save_player.cpp b/src/io/functions/iologindata_save_player.cpp index f320a67ca22..ec8ab71fd07 100644 --- 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) { << 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(raceId); }); @@ -659,7 +659,7 @@ bool IOLoginDataSave::savePlayerTaskHuntingClass(std::shared_ptr 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(raceId); }); diff --git a/src/io/ioprey.cpp b/src/io/ioprey.cpp index 8dcbfe10e46..b21f442bf67 100644 --- a/src/io/ioprey.cpp +++ b/src/io/ioprey.cpp @@ -65,8 +65,9 @@ void PreySlot::reloadMonsterGrid(std::vector 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 bestiary = g_game().getBestiaryList(); + const std::map &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, 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; } From 2735b8ce1500c5effa33307f99d85e071a8c3246 Mon Sep 17 00:00:00 2001 From: Marco Date: Thu, 17 Oct 2024 22:03:29 -0300 Subject: [PATCH 2/3] refactor: improve readability and optimize toPosition function (#2928) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactors the string.toPosition function to improve readability and optimize the code. The main changes include: • Renaming variables to more descriptive names. • Optimized pattern matching by reusing patterns. • Returning nil when no pattern is matched, improving error handling. These changes make the code clearer and more efficient without altering its functionality. --- data-otservbr-global/lib/quests/soul_war.lua | 18 ------------------ data/libs/functions/string.lua | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/data-otservbr-global/lib/quests/soul_war.lua b/data-otservbr-global/lib/quests/soul_war.lua index 668c67f92b9..f5b2d75f5ba 100644 --- a/data-otservbr-global/lib/quests/soul_war.lua +++ b/data-otservbr-global/lib/quests/soul_war.lua @@ -1569,21 +1569,3 @@ function Creature:applyZoneEffect(var, combat, zoneName) return true end - -function string.toPosition(str) - local patterns = { - -- table format - "{%s*x%s*=%s*(%d+)%s*,%s*y%s*=%s*(%d+)%s*,%s*z%s*=%s*(%d+)%s*}", - -- Position format - "Position%s*%((%d+)%s*,%s*(%d+)%s*,%s*(%d+)%s*%)", - -- x, y, z format - "(%d+)%s*,%s*(%d+)%s*,%s*(%d+)", - } - - for _, pattern in ipairs(patterns) do - local x, y, z = string.match(str, pattern) - if x and y and z then - return Position(tonumber(x), tonumber(y), tonumber(z)) - end - end -end diff --git a/data/libs/functions/string.lua b/data/libs/functions/string.lua index 9d746b82e3a..42b7a8bab4b 100644 --- a/data/libs/functions/string.lua +++ b/data/libs/functions/string.lua @@ -129,3 +129,19 @@ end string.capitalize = function(str) return str:gsub("%f[%a].", string.upper) end + +function string.toPosition(inputString) + local positionPatterns = { + "{%s*x%s*=%s*(%d+)%s*,%s*y%s*=%s*(%d+)%s*,%s*z%s*=%s*(%d+)%s*}", + "Position%s*%((%d+)%s*,%s*(%d+)%s*,%s*(%d+)%s*%)", + "(%d+)%s*,%s*(%d+)%s*,%s*(%d+)", + } + + for _, pattern in ipairs(positionPatterns) do + local posX, posY, posZ = string.match(inputString, pattern) + if posX and posY and posZ then + return Position(tonumber(posX), tonumber(posY), tonumber(posZ)) + end + end + return nil +end From 1af76e2e04951fe048da5d319c5c3e57a1dc5d6e Mon Sep 17 00:00:00 2001 From: Marco Date: Thu, 17 Oct 2024 22:06:04 -0300 Subject: [PATCH 3/3] refactor: optimize time formatting function for better performance (#2904) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactors the getTimeInWords function to improve performance and code readability. The new implementation adopts a more efficient approach for calculating and formatting time in days, hours, minutes, and seconds, reducing redundancy and simplifying the logical flow. • Optimized the function for better execution speed. • Simplified the conditions and formatting structure. • Maintained the same functionality with clearer and more efficient code. --- .../movements_boss_entrance.lua | 2 +- .../actions_portal_brain_head.lua | 2 +- data/events/scripts/player.lua | 2 +- data/libs/functions/boss_lever.lua | 2 +- data/libs/functions/functions.lua | 62 ------------------- data/libs/functions/game.lua | 34 ++++++++++ data/libs/systems/concoctions.lua | 6 +- .../scripts/eventcallbacks/player/on_look.lua | 2 +- 8 files changed, 42 insertions(+), 70 deletions(-) diff --git a/data-otservbr-global/scripts/quests/dangerous_depth/movements_boss_entrance.lua b/data-otservbr-global/scripts/quests/dangerous_depth/movements_boss_entrance.lua index 20300f3ac5f..a33dccd762d 100644 --- a/data-otservbr-global/scripts/quests/dangerous_depth/movements_boss_entrance.lua +++ b/data-otservbr-global/scripts/quests/dangerous_depth/movements_boss_entrance.lua @@ -37,7 +37,7 @@ function bossEntrance.onStepIn(creature, item, position, fromPosition, toPositio if timeLeft > 0 then player:teleportTo(fromPosition, true) player:getPosition():sendMagicEffect(CONST_ME_TELEPORT) - player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "You have to wait " .. getTimeInWords(timeLeft) .. " to face " .. bossName .. " again!") + player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "You have to wait " .. Game.getTimeInWords(timeLeft) .. " to face " .. bossName .. " again!") player:getPosition():sendMagicEffect(CONST_ME_POFF) return true end diff --git a/data-otservbr-global/scripts/quests/feaster_of_souls/actions_portal_brain_head.lua b/data-otservbr-global/scripts/quests/feaster_of_souls/actions_portal_brain_head.lua index aebc0b3ca7a..badaffad5a2 100644 --- a/data-otservbr-global/scripts/quests/feaster_of_souls/actions_portal_brain_head.lua +++ b/data-otservbr-global/scripts/quests/feaster_of_souls/actions_portal_brain_head.lua @@ -124,7 +124,7 @@ function teleportBoss.onStepIn(creature, item, position, fromPosition) if timeLeft > 0 then player:teleportTo(config.exitPosition, true) player:getPosition():sendMagicEffect(CONST_ME_TELEPORT) - player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "You have to wait " .. getTimeInWords(timeLeft) .. " to face " .. config.bossName .. " again!") + player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "You have to wait " .. Game.getTimeInWords(timeLeft) .. " to face " .. config.bossName .. " again!") player:getPosition():sendMagicEffect(CONST_ME_POFF) return false end diff --git a/data/events/scripts/player.lua b/data/events/scripts/player.lua index ec1a92f3528..eee28b632d6 100644 --- a/data/events/scripts/player.lua +++ b/data/events/scripts/player.lua @@ -216,7 +216,7 @@ function Player:onLookInBattleList(creature, distance) if master and table.contains(summons, creature:getName():lower()) then local familiarSummonTime = master:kv():get("familiar-summon-time") or 0 description = description .. " (Master: " .. master:getName() .. "). \z - It will disappear in " .. getTimeInWords(familiarSummonTime - os.time()) + It will disappear in " .. Game.getTimeInWords(familiarSummonTime - os.time()) end end if self:getGroup():getAccess() then diff --git a/data/libs/functions/boss_lever.lua b/data/libs/functions/boss_lever.lua index 9cd577ee911..b1141619ce2 100644 --- a/data/libs/functions/boss_lever.lua +++ b/data/libs/functions/boss_lever.lua @@ -191,7 +191,7 @@ function BossLever:onUse(player) local currentTime = os.time() if lastEncounter and currentTime < lastEncounter then local timeLeft = lastEncounter - currentTime - local timeMessage = getTimeInWords(timeLeft) .. " to face " .. self.name .. " again!" + local timeMessage = Game.getTimeInWords(timeLeft) .. " to face " .. self.name .. " again!" local message = "You have to wait " .. timeMessage if currentPlayer ~= player then diff --git a/data/libs/functions/functions.lua b/data/libs/functions/functions.lua index c2349ce7fd6..2327a1cf937 100644 --- a/data/libs/functions/functions.lua +++ b/data/libs/functions/functions.lua @@ -65,43 +65,6 @@ function getTitle(uid) return false end -function getTimeInWords(secsParam) - local secs = tonumber(secsParam) - local days = math.floor(secs / (24 * 3600)) - secs = secs - (days * 24 * 3600) - local hours, minutes, seconds = getHours(secs), getMinutes(secs), getSeconds(secs) - local timeStr = "" - - if days > 0 then - timeStr = days .. (days > 1 and " days" or " day") - end - - if hours > 0 then - if timeStr ~= "" then - timeStr = timeStr .. ", " - end - - timeStr = timeStr .. hours .. (hours > 1 and " hours" or " hour") - end - - if minutes > 0 then - if timeStr ~= "" then - timeStr = timeStr .. ", " - end - - timeStr = timeStr .. minutes .. (minutes > 1 and " minutes" or " minute") - end - - if seconds > 0 then - if timeStr ~= "" then - timeStr = timeStr .. " and " - end - - timeStr = timeStr .. seconds .. (seconds > 1 and " seconds" or " second") - end - return timeStr -end - function getLootRandom(modifier) local multi = (configManager.getNumber(configKeys.RATE_LOOT) * SCHEDULE_LOOT_RATE) * (modifier or 1) return math.random(0, MAX_LOOTCHANCE) * 100 / math.max(1, multi) @@ -949,31 +912,6 @@ function SetInfluenced(monsterType, monster, player, influencedLevel) monster:setForgeStack(influencedLevel) end -function getHours(seconds) - return math.floor((seconds / 60) / 60) -end - -function getMinutes(seconds) - return math.floor(seconds / 60) % 60 -end - -function getSeconds(seconds) - return seconds % 60 -end - -function getTime(seconds) - local hours, minutes = getHours(seconds), getMinutes(seconds) - if minutes > 59 then - minutes = minutes - hours * 60 - end - - if minutes < 10 then - minutes = "0" .. minutes - end - - return hours .. ":" .. minutes .. "h" -end - function ReloadDataEvent(cid) local player = Player(cid) if not player then diff --git a/data/libs/functions/game.lua b/data/libs/functions/game.lua index e4d40bef318..a7c7f7617ce 100644 --- a/data/libs/functions/game.lua +++ b/data/libs/functions/game.lua @@ -133,3 +133,37 @@ function Game.setStorageValue(key, value) globalStorageTable[key] = value end + +function Game.getTimeInWords(seconds) + local days = math.floor(seconds / (24 * 3600)) + seconds = seconds % (24 * 3600) + local hours = math.floor(seconds / 3600) + seconds = seconds % 3600 + local minutes = math.floor(seconds / 60) + seconds = seconds % 60 + + local timeParts = {} + + if days > 0 then + table.insert(timeParts, days .. (days > 1 and " days" or " day")) + end + + if hours > 0 then + table.insert(timeParts, hours .. (hours > 1 and " hours" or " hour")) + end + + if minutes > 0 then + table.insert(timeParts, minutes .. (minutes > 1 and " minutes" or " minute")) + end + + if seconds > 0 or #timeParts == 0 then + table.insert(timeParts, seconds .. (seconds > 1 and " seconds" or " second")) + end + + local timeStr = table.concat(timeParts, ", ") + local lastComma = timeStr:find(", [%a%d]+$") + if lastComma then + timeStr = timeStr:sub(1, lastComma - 1) .. " and" .. timeStr:sub(lastComma + 1) + end + return timeStr +end diff --git a/data/libs/systems/concoctions.lua b/data/libs/systems/concoctions.lua index 72547a8a1ab..ee856e16482 100644 --- a/data/libs/systems/concoctions.lua +++ b/data/libs/systems/concoctions.lua @@ -158,7 +158,7 @@ function Concoction:init(player, sendMessage) return end eventPlayer:sendTextMessage(MESSAGE_EVENT_ADVANCE, "Your concoction " .. name .. " is still active for another " .. duration .. ".") - end, 500, player:getId(), self.name, getTimeInWords(self:timeLeft(player))) + end, 500, player:getId(), self.name, Game.getTimeInWords(self:timeLeft(player))) end end @@ -180,7 +180,7 @@ function Concoction:activate(player, item) local cooldown = self:cooldown() if self:lastActivatedAt(player) + cooldown > os.time() then local cooldownLeft = self:lastActivatedAt(player) + cooldown - os.time() - player:sendTextMessage(MESSAGE_FAILURE, "You must wait " .. getTimeInWords(cooldownLeft) .. " before using " .. item:getName() .. " again.") + player:sendTextMessage(MESSAGE_FAILURE, "You must wait " .. Game.getTimeInWords(cooldownLeft) .. " before using " .. item:getName() .. " again.") return true end self:timeLeft(player, self:totalDuration()) @@ -191,7 +191,7 @@ function Concoction:activate(player, item) self.config.callback(player, self.config) else self:addCondition(player) - player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "You have activated " .. item:getName() .. ". It will last for " .. getTimeInWords(self:totalDuration()) .. consumptionString .. ".") + player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "You have activated " .. item:getName() .. ". It will last for " .. Game.getTimeInWords(self:totalDuration()) .. consumptionString .. ".") if self:tickType() == ConcoctionTickType.Online then addEvent(tick, updateInterval * 1000, self.id, player:getId(), updateInterval) end diff --git a/data/scripts/eventcallbacks/player/on_look.lua b/data/scripts/eventcallbacks/player/on_look.lua index 68824cb9b44..e0a1ab86788 100644 --- a/data/scripts/eventcallbacks/player/on_look.lua +++ b/data/scripts/eventcallbacks/player/on_look.lua @@ -44,7 +44,7 @@ local function handleCreatureDescription(inspectedThing, lookDistance) local monsterMaster = inspectedThing:getMaster() if monsterMaster and table.contains({ "sorcerer familiar", "knight familiar", "druid familiar", "paladin familiar" }, inspectedThing:getName():lower()) then local summonTimeRemaining = monsterMaster:kv():get("familiar-summon-time") or 0 - descriptionText = string.format("%s (Master: %s). It will disappear in %s", descriptionText, monsterMaster:getName(), getTimeInWords(summonTimeRemaining - os.time())) + descriptionText = string.format("%s (Master: %s). It will disappear in %s", descriptionText, monsterMaster:getName(), Game.getTimeInWords(summonTimeRemaining - os.time())) end end