From 37b5112b601dd3c1bc1816cbe9cbc6593cf78c65 Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Mon, 23 Sep 2024 14:10:44 -0300 Subject: [PATCH] fix: qodana reviews --- .github/workflows/analysis-qodana.yml | 46 ++----------------- src/config/configmanager.cpp | 4 +- .../players/livestream/livestream.cpp | 31 ++++++------- .../players/livestream/livestream.hpp | 17 ++++--- src/creatures/players/player.cpp | 4 +- .../creatures/player/player_functions.cpp | 27 ++++++----- .../functions/items/imbuement_functions.cpp | 2 +- src/server/network/protocol/protocolgame.cpp | 22 +++++---- src/server/network/protocol/protocolgame.hpp | 8 +--- src/server/network/protocol/protocollogin.cpp | 2 +- src/utils/tools.cpp | 4 ++ src/utils/tools.hpp | 2 + 12 files changed, 67 insertions(+), 102 deletions(-) diff --git a/.github/workflows/analysis-qodana.yml b/.github/workflows/analysis-qodana.yml index a137111df3f..ca04f992bff 100644 --- a/.github/workflows/analysis-qodana.yml +++ b/.github/workflows/analysis-qodana.yml @@ -19,50 +19,14 @@ jobs: - uses: actions/checkout@v4 with: ref: ${{ github.event.pull_request.head.sha }} - fetch-depth: 0 - - # - name: Install Linux Dependencies - # run: > - # sudo apt-get update && sudo apt-get install ccache - # linux-headers-$(uname -r) - # - name: CCache - # uses: hendrikmuhs/ccache-action@main - # with: - # max-size: "1G" - # key: ccache-qodana - - # - name: Restore artifacts and install vcpkg - # id: vcpkg-step - # run: | - # vcpkgCommitId=$(grep '.builtin-baseline' vcpkg.json | awk -F: '{print $2}' | tr -d '," ') - # echo "vcpkg commit ID: $vcpkgCommitId" - # echo "VCPKG_GIT_COMMIT_ID=$vcpkgCommitId" >> $GITHUB_ENV - # - name: Get vcpkg commit id from vcpkg.json - # uses: lukka/run-vcpkg@main - # with: - # vcpkgGitURL: "https://github.com/microsoft/vcpkg.git" - # vcpkgGitCommitId: ${{ env.VCPKG_GIT_COMMIT_ID }} - - # - name: Get latest CMake and ninja - # uses: lukka/get-cmake@main - - # - name: Run CMake - # uses: lukka/run-cmake@main - # with: - # configurePreset: linux-debug - - # - name: Qodana Scan - # run: | - # docker run \ - # -v $(pwd):/data/project/ \ - # -v $(pwd):$(pwd) \ - # -e QODANA_TOKEN="${{ secrets.QODANA_TOKEN }}" \ - # jetbrains/qodana-clang:2024.1-eap \ - # --compile-commands ./build/linux-debug/compile_commands.json + fetch-depth: 2 - name: 'Qodana Scan' uses: JetBrains/qodana-action@v2024.1 with: - args: --compile-commands,./build/linux-debug/compile_commands.json,--baseline,qodana-base.sarif.json + args: > + --compile-commands ./build/linux-debug/compile_commands.json + --baseline qodana-base.sarif.json + --changes ${GITHUB_BASE_REF}..${GITHUB_HEAD_REF} env: QODANA_TOKEN: ${{ secrets.QODANA_TOKEN }} diff --git a/src/config/configmanager.cpp b/src/config/configmanager.cpp index b7e69f4165d..93c3ca9266b 100644 --- a/src/config/configmanager.cpp +++ b/src/config/configmanager.cpp @@ -20,12 +20,12 @@ #endif namespace ConfigDefaultValues { - constexpr auto LIVESTREAM_EXPERIENCE_MULTIPLIER = 1.15f; + constexpr auto LIVESTREAM_EXPERIENCE_MULTIPLIER = 1.15F; constexpr auto LIVESTREAM_MAX_VIEWERS = 10; constexpr auto LIVESTREAM_MAXIMUM_VIEWERS_PER_IP = 10; constexpr auto LIVESTREAM_PREMIUM_MAX_VIEWERS = 20; constexpr auto LIVESTREAM_CASTER_MIN_LEVEL = 200; -} +} // namespace ConfigDefaultValues ConfigManager &ConfigManager::getInstance() { return inject(); diff --git a/src/creatures/players/livestream/livestream.cpp b/src/creatures/players/livestream/livestream.cpp index 206b158c129..ffe763690a2 100644 --- a/src/creatures/players/livestream/livestream.cpp +++ b/src/creatures/players/livestream/livestream.cpp @@ -85,10 +85,12 @@ const std::map &Livestream::getLivestreamBans() const { void Livestream::setBanViewer(const std::vector &bans) { // Remove banned viewers not in the new bans list - std::erase_if(m_bans, [&bans](const auto &ban) { + auto removedCount = std::erase_if(m_bans, [&bans](const auto &ban) { return std::find(bans.begin(), bans.end(), ban.first) == bans.end(); }); + g_logger().debug("[{}] removed {} banned viewers", __METHOD_NAME__, removedCount); + // Add new bans to the list and disconnect clients for (const auto &ban : bans) { for (const auto &[client, clientInfo] : m_viewers) { @@ -631,7 +633,7 @@ void Livestream::sendToChannel(const std::shared_ptr &creature, SpeakC } } -void Livestream::sendShop(std::shared_ptr npc) const { +void Livestream::sendShop(const std::shared_ptr &npc) const { if (m_owner) { m_owner->sendShop(npc); } @@ -1427,7 +1429,7 @@ void Livestream::addViewer(const ProtocolGame_ptr &client, bool spy) { sendChannelEvent(CHANNEL_CAST, guestString, CHANNELEVENT_JOIN); if (m_viewers.size() > m_livestreamCasterLiveRecord) { - m_livestreamCasterLiveRecord = m_viewers.size(); + m_livestreamCasterLiveRecord = static_cast(m_viewers.size()); if (m_owner->player) { m_owner->player->kv()->scoped("livestream-system")->set("live-record", static_cast(m_livestreamCasterLiveRecord)); } @@ -1455,7 +1457,7 @@ void Livestream::removeViewer(const ProtocolGame_ptr &client, bool spy) { } void Livestream::handle(const ProtocolGame_ptr &client, const std::string &text, uint16_t channelId) { - if (!m_owner) { + if (m_owner == nullptr) { return; } @@ -1468,9 +1470,11 @@ void Livestream::handle(const ProtocolGame_ptr &client, const std::string &text, if (client->m_livestreamMessageCooldownTime + MESSAGE_COOLDOWN_MS < now) { client->m_livestreamMessageCooldownTime = now; client->m_livestreamMessageCount = 0; - } else if (client->m_livestreamMessageCount++ >= MESSAGE_LIMIT) { - client->sendTextMessage(TextMessage(MESSAGE_STATUS, fmt::format("Please wait {} seconds to send another message.", ((client->m_livestreamMessageCooldownTime + MESSAGE_COOLDOWN_MS - now) / SECONDS_IN_MS) + 1))); - return; + } else { + if (client->m_livestreamMessageCount++ >= MESSAGE_LIMIT) { + client->sendTextMessage(TextMessage(MESSAGE_STATUS, fmt::format("Please wait {} seconds to send another message.", ((client->m_livestreamMessageCooldownTime + MESSAGE_COOLDOWN_MS - now) / SECONDS_IN_MS) + 1))); + return; + } } bool isCastChannel = channelId == CHANNEL_CAST; @@ -1526,10 +1530,6 @@ void Livestream::changeViewerName(const ProtocolGame_ptr &client, const std::vec updateViewerName(client, sit, newName, isCastChannel); } -bool Livestream::isNameLengthValid(const std::string &name) const { - return name.length() > MIN_NAME_LENGTH && name.length() < MAX_NAME_LENGTH; -} - void Livestream::updateViewerName(const ProtocolGame_ptr &client, std::map::iterator sit, const std::string &newName, bool isCastChannel) { // If the viewer is a cast channel, notify others about the name change if (isCastChannel) { @@ -1550,12 +1550,9 @@ void Livestream::updateViewerName(const ProtocolGame_ptr &client, std::map::iterator sit, const std::string &text, bool isCastChannel) { diff --git a/src/creatures/players/livestream/livestream.hpp b/src/creatures/players/livestream/livestream.hpp index 08fd09421f2..d509bb66795 100644 --- a/src/creatures/players/livestream/livestream.hpp +++ b/src/creatures/players/livestream/livestream.hpp @@ -43,7 +43,7 @@ class Livestream { void clear(bool clearAll); bool checkPassword(const std::string &_password); - void handle(const ProtocolGame_ptr &client, const std::string &text, uint16_t channelId); + void handle(const std::shared_ptr &client, const std::string &text, uint16_t channelId); size_t getLivestreamViewerCount(); std::vector getLivestreamViewers() const; std::vector> getLivestreamViewersByIP(uint32_t ip) const; @@ -62,7 +62,7 @@ class Livestream { void setLivestreamBroadcasting(bool value); std::string getLivestreamBroadcastTimeString() const; void addViewer(const std::shared_ptr &client, bool spy = false); - void removeViewer(const ProtocolGame_ptr &client, bool spy = false); + void removeViewer(const std::shared_ptr &client, bool spy = false); int64_t getLivestreamBroadcastTime() const; void setLivestreamBroadcastingTime(int64_t time); uint32_t getLivestreamLiveRecord() const; @@ -123,7 +123,7 @@ class Livestream { void sendTextWindow(uint32_t windowTextId, const std::shared_ptr &item, uint16_t maxlen, bool canWrite) const; void sendTextWindow(uint32_t windowTextId, uint32_t itemId, const std::string &text) const; void sendToChannel(const std::shared_ptr &creature, SpeakClasses type, const std::string &text, uint16_t channelId); - void sendShop(std::shared_ptr npc) const; + void sendShop(const std::shared_ptr &npc) const; void sendSaleItemList(const std::vector &shopVector, const std::map &inventoryMap); void sendCloseShop() const; void sendTradeItemRequest(const std::string &traderName, const std::shared_ptr &item, bool ack) const; @@ -257,13 +257,12 @@ class Livestream { bool isLivestreamViewer() const; private: - void processCommand(const ProtocolGame_ptr &client, const std::string &text, std::map::iterator sit, bool isCastChannel); - void showViewers(const ProtocolGame_ptr &client); - void changeViewerName(const ProtocolGame_ptr &client, const std::vector &CommandParam, std::map::iterator sit, bool isCastChannel); + void processCommand(const std::shared_ptr &client, const std::string &text, std::map, ViewerInfo>::iterator sit, bool isCastChannel); + void showViewers(const std::shared_ptr &client); + void changeViewerName(const std::shared_ptr &client, const std::vector &CommandParam, std::map, ViewerInfo>::iterator sit, bool isCastChannel); bool isNameAvailable(const std::string &name) const; - void handleChatMessage(const ProtocolGame_ptr &client, std::map::iterator sit, const std::string &text, bool isCastChannel); - bool isNameLengthValid(const std::string &name) const; - void updateViewerName(const ProtocolGame_ptr &client, std::map::iterator sit, const std::string &newName, bool isCastChannel); + void handleChatMessage(const std::shared_ptr &client, std::map, ViewerInfo>::iterator sit, const std::string &text, bool isCastChannel); + void updateViewerName(const std::shared_ptr &client, std::map, ViewerInfo>::iterator sit, const std::string &newName, bool isCastChannel); friend class Player; std::map, ViewerInfo> m_viewers; diff --git a/src/creatures/players/player.cpp b/src/creatures/players/player.cpp index 1a182c22281..6b97e14bbc3 100644 --- a/src/creatures/players/player.cpp +++ b/src/creatures/players/player.cpp @@ -1460,7 +1460,7 @@ void Player::onApplyImbuement(Imbuement* imbuement, std::shared_ptr item, return; } - const auto items = imbuement->getItems(); + const auto &items = imbuement->getItems(); for (auto &[key, value] : items) { const ItemType &itemType = Item::items[key]; if (static_self_cast()->getItemTypeCount(key) + this->getStashItemCount(itemType.id) < value) { @@ -1750,7 +1750,7 @@ void Player::onCreatureAppear(std::shared_ptr creature, bool isLogin) auto livestreamLiveRecord = kv()->scoped("livestream-system")->get("live-record"); if (livestreamLiveRecord) { g_logger().trace("Loading cast live record: {}", livestreamLiveRecord->getNumber()); - client->setLivestreamLiveRecord(livestreamLiveRecord->getNumber()); + client->setLivestreamLiveRecord(static_cast(livestreamLiveRecord->getNumber())); } #endif diff --git a/src/lua/functions/creatures/player/player_functions.cpp b/src/lua/functions/creatures/player/player_functions.cpp index 3929e4fc698..333d8d5e365 100644 --- a/src/lua/functions/creatures/player/player_functions.cpp +++ b/src/lua/functions/creatures/player/player_functions.cpp @@ -4471,7 +4471,7 @@ int PlayerFunctions::luaPlayerGetLivestreamViewersCount(lua_State* L) { return 1; } - lua_pushnumber(L, player->client->getLivestreamViewerCount()); + lua_pushnumber(L, static_cast(player->client->getLivestreamViewerCount())); return 1; } @@ -4491,32 +4491,31 @@ int PlayerFunctions::luaPlayerGetLivestreamViewers(lua_State* L) { createCastTable(L, "names"); auto viewers = player->client->getLivestreamViewers(); - - auto it = viewers.begin(); - for (std::size_t i = 1; it != viewers.end(); ++it, ++i) { + std::size_t i = 1; + for (const auto& viewer : viewers) { lua_pushnumber(L, static_cast(i)); - lua_pushstring(L, it->c_str()); + lua_pushstring(L, viewer.c_str()); lua_settable(L, -3); + ++i; } lua_settable(L, -3); createCastTable(L, "mutes"); - const auto &mute = player->client->getLivestreamMutes(); - auto mute_it = mute.begin(); - for (std::size_t i = 1; mute_it != mute.end(); ++mute_it, ++i) { + const auto &mutes = player->client->getLivestreamMutes(); + i = 1; + for (const auto& mute : mutes) { lua_pushnumber(L, static_cast(i)); - lua_pushstring(L, mute_it->c_str()); + lua_pushstring(L, mute.c_str()); lua_settable(L, -3); } lua_settable(L, -3); createCastTable(L, "bans"); - const auto &banList = player->client->getLivestreamBans(); - - auto ban_it = banList.begin(); - for (std::size_t i = 1; ban_it != banList.end(); ++ban_it, ++i) { + const auto &bans = player->client->getLivestreamBans(); + i = 1; + for (const auto& [name, id] : bans) { lua_pushnumber(L, static_cast(i)); - lua_pushstring(L, ban_it->first.c_str()); + lua_pushstring(L, name.c_str()); lua_settable(L, -3); } diff --git a/src/lua/functions/items/imbuement_functions.cpp b/src/lua/functions/items/imbuement_functions.cpp index f24a3f564d7..0b9740e2088 100644 --- a/src/lua/functions/items/imbuement_functions.cpp +++ b/src/lua/functions/items/imbuement_functions.cpp @@ -57,7 +57,7 @@ int ImbuementFunctions::luaImbuementGetItems(lua_State* L) { return 1; } - const auto items = imbuement->getItems(); + const auto &items = imbuement->getItems(); lua_createtable(L, items.size(), 0); for (const auto &itm : items) { diff --git a/src/server/network/protocol/protocolgame.cpp b/src/server/network/protocol/protocolgame.cpp index 7363fc444fd..f6273eddd84 100644 --- a/src/server/network/protocol/protocolgame.cpp +++ b/src/server/network/protocol/protocolgame.cpp @@ -4889,7 +4889,7 @@ void ProtocolGame::sendLootStats(std::shared_ptr item, uint8_t count) { lootedItem = nullptr; } -void ProtocolGame::sendShop(std::shared_ptr npc) { +void ProtocolGame::sendShop(const std::shared_ptr &npc) { Benchmark brenchmark; NetworkMessage msg; msg.addByte(0x7A); @@ -7958,7 +7958,7 @@ void ProtocolGame::addImbuementInfo(NetworkMessage &msg, uint16_t imbuementId) c msg.addByte(imbuement->isPremium() ? 0x01 : 0x00); - const auto items = imbuement->getItems(); + const auto &items = imbuement->getItems(); msg.addByte(items.size()); for (const auto &itm : items) { @@ -8009,7 +8009,7 @@ void ProtocolGame::openImbuementWindow(std::shared_ptr item) { for (const Imbuement* imbuement : imbuements) { addImbuementInfo(msg, imbuement->getID()); - const auto items = imbuement->getItems(); + const auto &items = imbuement->getItems(); for (const auto &itm : items) { if (!needItems.count(itm.first)) { needItems[itm.first] = player->getItemTypeCount(itm.first); @@ -9339,21 +9339,25 @@ void ProtocolGame::sendTakeScreenshot(Screenshot_t screenshotType) { } #if FEATURE_LIVESTREAM > 0 -std::unordered_map, ProtocolGame*> ProtocolGame::m_livestreamCasters; +std::unordered_map, ProtocolGame*>& ProtocolGame::getLivestreamCasters() { + static std::unordered_map, ProtocolGame*> livestreamCasters; + return livestreamCasters; +} +#endif void ProtocolGame::insertLivestreamCaster() { - const auto &cast = m_livestreamCasters.find(player); - if (cast != m_livestreamCasters.end()) { + const auto &cast = getLivestreamCasters().find(player); + if (cast != getLivestreamCasters().end()) { return; } - m_livestreamCasters.insert(std::make_pair(player, this)); + getLivestreamCasters().insert(std::make_pair(player, this)); } void ProtocolGame::removeLivestreamCaster() { - for (const auto &it : getLiveStreamCasters()) { + for (const auto &it : getLivestreamCasters()) { if (it.first == player) { - m_livestreamCasters.erase(player); + getLivestreamCasters().erase(player); break; } } diff --git a/src/server/network/protocol/protocolgame.hpp b/src/server/network/protocol/protocolgame.hpp index 24f95365bc4..9a19a3fb548 100644 --- a/src/server/network/protocol/protocolgame.hpp +++ b/src/server/network/protocol/protocolgame.hpp @@ -341,7 +341,7 @@ class ProtocolGame final : public Protocol { void sendCreatureSkull(std::shared_ptr creature); void sendCreatureType(std::shared_ptr creature, uint8_t creatureType); - void sendShop(std::shared_ptr npc); + void sendShop(const std::shared_ptr &npc); void sendCloseShop(); void sendClientCheck(); void sendGameNews(); @@ -493,15 +493,11 @@ class ProtocolGame final : public Protocol { void syncLivestreamViewerCloseContainers(); bool canWatchCast(const std::shared_ptr &foundPlayer, const std::string &password) const; - static const std::unordered_map, ProtocolGame*> &getLiveStreamCasters() { - return m_livestreamCasters; - } + static std::unordered_map, ProtocolGame*> &getLivestreamCasters(); void insertLivestreamCaster(); void removeLivestreamCaster(); - static std::unordered_map, ProtocolGame*> m_livestreamCasters; - friend class Livestream; #endif diff --git a/src/server/network/protocol/protocollogin.cpp b/src/server/network/protocol/protocollogin.cpp index 4333ae3d07d..cd2be84be6b 100644 --- a/src/server/network/protocol/protocollogin.cpp +++ b/src/server/network/protocol/protocollogin.cpp @@ -247,7 +247,7 @@ void ProtocolLogin::getLivestreamViewersList(const std::string &password) { output->addByte(uint8_t()); output->addByte(uint8_t()); output->add(uint32_t()); - output->add(uint32_t()); + output->add(uint16_t()); send(std::move(output)); disconnect(); diff --git a/src/utils/tools.cpp b/src/utils/tools.cpp index d3b5e52bdad..a3863b94bde 100644 --- a/src/utils/tools.cpp +++ b/src/utils/tools.cpp @@ -1958,3 +1958,7 @@ uint8_t convertWheelGemAffinityToDomain(uint8_t affinity) { return 0; } } + +bool isNameLengthValid(const std::string &name) { + return name.length() > MIN_NAME_LENGTH && name.length() < MAX_NAME_LENGTH; +} diff --git a/src/utils/tools.hpp b/src/utils/tools.hpp index f5e876bf365..db1bf5fc171 100644 --- a/src/utils/tools.hpp +++ b/src/utils/tools.hpp @@ -221,3 +221,5 @@ template (value); } + +bool isNameLengthValid(const std::string &name); \ No newline at end of file