Skip to content

Commit

Permalink
fix: qodana reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
dudantas committed Sep 23, 2024
1 parent 51c64a6 commit 37b5112
Show file tree
Hide file tree
Showing 12 changed files with 67 additions and 102 deletions.
46 changes: 5 additions & 41 deletions .github/workflows/analysis-qodana.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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/[email protected]
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 }}
4 changes: 2 additions & 2 deletions src/config/configmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<ConfigManager>();
Expand Down
31 changes: 14 additions & 17 deletions src/creatures/players/livestream/livestream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,12 @@ const std::map<std::string, uint32_t> &Livestream::getLivestreamBans() const {

void Livestream::setBanViewer(const std::vector<std::string> &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) {
Expand Down Expand Up @@ -631,7 +633,7 @@ void Livestream::sendToChannel(const std::shared_ptr<Creature> &creature, SpeakC
}
}

void Livestream::sendShop(std::shared_ptr<Npc> npc) const {
void Livestream::sendShop(const std::shared_ptr<Npc> &npc) const {
if (m_owner) {
m_owner->sendShop(npc);
}
Expand Down Expand Up @@ -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<uint32_t>(m_viewers.size());
if (m_owner->player) {
m_owner->player->kv()->scoped("livestream-system")->set("live-record", static_cast<uint16_t>(m_livestreamCasterLiveRecord));
}
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
Expand Down Expand Up @@ -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<ProtocolGame_ptr, ViewerInfo>::iterator sit, const std::string &newName, bool isCastChannel) {
// If the viewer is a cast channel, notify others about the name change
if (isCastChannel) {
Expand All @@ -1550,12 +1550,9 @@ void Livestream::updateViewerName(const ProtocolGame_ptr &client, std::map<Proto
}

bool Livestream::isNameAvailable(const std::string &name) const {
for (const auto &it : m_viewers) {
if (strcasecmp(it.second.name.c_str(), name.c_str()) == 0) {
return false;
}
}
return true;
return std::ranges::none_of(m_viewers, [&](const auto &it) {
return strcasecmp(it.second.name.c_str(), name.c_str()) == 0;
});
}

void Livestream::handleChatMessage(const ProtocolGame_ptr &client, std::map<ProtocolGame_ptr, ViewerInfo>::iterator sit, const std::string &text, bool isCastChannel) {
Expand Down
17 changes: 8 additions & 9 deletions src/creatures/players/livestream/livestream.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<ProtocolGame> &client, const std::string &text, uint16_t channelId);
size_t getLivestreamViewerCount();
std::vector<std::string> getLivestreamViewers() const;
std::vector<std::shared_ptr<Player>> getLivestreamViewersByIP(uint32_t ip) const;
Expand All @@ -62,7 +62,7 @@ class Livestream {
void setLivestreamBroadcasting(bool value);
std::string getLivestreamBroadcastTimeString() const;
void addViewer(const std::shared_ptr<ProtocolGame> &client, bool spy = false);
void removeViewer(const ProtocolGame_ptr &client, bool spy = false);
void removeViewer(const std::shared_ptr<ProtocolGame> &client, bool spy = false);
int64_t getLivestreamBroadcastTime() const;
void setLivestreamBroadcastingTime(int64_t time);
uint32_t getLivestreamLiveRecord() const;
Expand Down Expand Up @@ -123,7 +123,7 @@ class Livestream {
void sendTextWindow(uint32_t windowTextId, const std::shared_ptr<Item> &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> &creature, SpeakClasses type, const std::string &text, uint16_t channelId);
void sendShop(std::shared_ptr<Npc> npc) const;
void sendShop(const std::shared_ptr<Npc> &npc) const;
void sendSaleItemList(const std::vector<ShopBlock> &shopVector, const std::map<uint16_t, uint16_t> &inventoryMap);
void sendCloseShop() const;
void sendTradeItemRequest(const std::string &traderName, const std::shared_ptr<Item> &item, bool ack) const;
Expand Down Expand Up @@ -257,13 +257,12 @@ class Livestream {
bool isLivestreamViewer() const;

private:
void processCommand(const ProtocolGame_ptr &client, const std::string &text, std::map<ProtocolGame_ptr, ViewerInfo>::iterator sit, bool isCastChannel);
void showViewers(const ProtocolGame_ptr &client);
void changeViewerName(const ProtocolGame_ptr &client, const std::vector<std::string> &CommandParam, std::map<ProtocolGame_ptr, ViewerInfo>::iterator sit, bool isCastChannel);
void processCommand(const std::shared_ptr<ProtocolGame> &client, const std::string &text, std::map<std::shared_ptr<ProtocolGame>, ViewerInfo>::iterator sit, bool isCastChannel);
void showViewers(const std::shared_ptr<ProtocolGame> &client);
void changeViewerName(const std::shared_ptr<ProtocolGame> &client, const std::vector<std::string> &CommandParam, std::map<std::shared_ptr<ProtocolGame>, ViewerInfo>::iterator sit, bool isCastChannel);
bool isNameAvailable(const std::string &name) const;
void handleChatMessage(const ProtocolGame_ptr &client, std::map<ProtocolGame_ptr, ViewerInfo>::iterator sit, const std::string &text, bool isCastChannel);
bool isNameLengthValid(const std::string &name) const;
void updateViewerName(const ProtocolGame_ptr &client, std::map<ProtocolGame_ptr, ViewerInfo>::iterator sit, const std::string &newName, bool isCastChannel);
void handleChatMessage(const std::shared_ptr<ProtocolGame> &client, std::map<std::shared_ptr<ProtocolGame>, ViewerInfo>::iterator sit, const std::string &text, bool isCastChannel);
void updateViewerName(const std::shared_ptr<ProtocolGame> &client, std::map<std::shared_ptr<ProtocolGame>, ViewerInfo>::iterator sit, const std::string &newName, bool isCastChannel);

friend class Player;
std::map<std::shared_ptr<ProtocolGame>, ViewerInfo> m_viewers;
Expand Down
4 changes: 2 additions & 2 deletions src/creatures/players/player.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1460,7 +1460,7 @@ void Player::onApplyImbuement(Imbuement* imbuement, std::shared_ptr<Item> 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<Player>()->getItemTypeCount(key) + this->getStashItemCount(itemType.id) < value) {
Expand Down Expand Up @@ -1750,7 +1750,7 @@ void Player::onCreatureAppear(std::shared_ptr<Creature> 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<uint32_t>(livestreamLiveRecord->getNumber()));
}
#endif

Expand Down
27 changes: 13 additions & 14 deletions src/lua/functions/creatures/player/player_functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4471,7 +4471,7 @@ int PlayerFunctions::luaPlayerGetLivestreamViewersCount(lua_State* L) {
return 1;
}

lua_pushnumber(L, player->client->getLivestreamViewerCount());
lua_pushnumber(L, static_cast<lua_Number>(player->client->getLivestreamViewerCount()));
return 1;
}

Expand All @@ -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<lua_Number>(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<lua_Number>(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<lua_Number>(i));
lua_pushstring(L, ban_it->first.c_str());
lua_pushstring(L, name.c_str());
lua_settable(L, -3);
}

Expand Down
2 changes: 1 addition & 1 deletion src/lua/functions/items/imbuement_functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
22 changes: 13 additions & 9 deletions src/server/network/protocol/protocolgame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4889,7 +4889,7 @@ void ProtocolGame::sendLootStats(std::shared_ptr<Item> item, uint8_t count) {
lootedItem = nullptr;
}

void ProtocolGame::sendShop(std::shared_ptr<Npc> npc) {
void ProtocolGame::sendShop(const std::shared_ptr<Npc> &npc) {
Benchmark brenchmark;
NetworkMessage msg;
msg.addByte(0x7A);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -8009,7 +8009,7 @@ void ProtocolGame::openImbuementWindow(std::shared_ptr<Item> 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);
Expand Down Expand Up @@ -9339,21 +9339,25 @@ void ProtocolGame::sendTakeScreenshot(Screenshot_t screenshotType) {
}

#if FEATURE_LIVESTREAM > 0
std::unordered_map<std::shared_ptr<Player>, ProtocolGame*> ProtocolGame::m_livestreamCasters;
std::unordered_map<std::shared_ptr<Player>, ProtocolGame*>& ProtocolGame::getLivestreamCasters() {
static std::unordered_map<std::shared_ptr<Player>, 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;
}
}
Expand Down
8 changes: 2 additions & 6 deletions src/server/network/protocol/protocolgame.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ class ProtocolGame final : public Protocol {
void sendCreatureSkull(std::shared_ptr<Creature> creature);
void sendCreatureType(std::shared_ptr<Creature> creature, uint8_t creatureType);

void sendShop(std::shared_ptr<Npc> npc);
void sendShop(const std::shared_ptr<Npc> &npc);
void sendCloseShop();
void sendClientCheck();
void sendGameNews();
Expand Down Expand Up @@ -493,15 +493,11 @@ class ProtocolGame final : public Protocol {
void syncLivestreamViewerCloseContainers();
bool canWatchCast(const std::shared_ptr<Player> &foundPlayer, const std::string &password) const;

static const std::unordered_map<std::shared_ptr<Player>, ProtocolGame*> &getLiveStreamCasters() {
return m_livestreamCasters;
}
static std::unordered_map<std::shared_ptr<Player>, ProtocolGame*> &getLivestreamCasters();

void insertLivestreamCaster();
void removeLivestreamCaster();

static std::unordered_map<std::shared_ptr<Player>, ProtocolGame*> m_livestreamCasters;

friend class Livestream;
#endif

Expand Down
2 changes: 1 addition & 1 deletion src/server/network/protocol/protocollogin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ void ProtocolLogin::getLivestreamViewersList(const std::string &password) {
output->addByte(uint8_t());
output->addByte(uint8_t());
output->add<uint32_t>(uint32_t());
output->add<uint16_t>(uint32_t());
output->add<uint16_t>(uint16_t());

send(std::move(output));
disconnect();
Expand Down
4 changes: 4 additions & 0 deletions src/utils/tools.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1958,3 +1958,7 @@ uint8_t convertWheelGemAffinityToDomain(uint8_t affinity) {
return 0;
}
}

bool isNameLengthValid(const std::string &name) {

Check warning on line 1962 in src/utils/tools.cpp

View workflow job for this annotation

GitHub Actions / cppcheck

[cppcheck] src/utils/tools.cpp#L1962

The function 'isNameLengthValid' is never used.
Raw output
src/utils/tools.cpp:1962:The function 'isNameLengthValid' is never used.
return name.length() > MIN_NAME_LENGTH && name.length() < MAX_NAME_LENGTH;
}
2 changes: 2 additions & 0 deletions src/utils/tools.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -221,3 +221,5 @@ template <typename EnumType, typename UnderlyingType = std::underlying_type_t<En
EnumType enumFromValue(UnderlyingType value) {
return static_cast<EnumType>(value);
}

bool isNameLengthValid(const std::string &name);

0 comments on commit 37b5112

Please sign in to comment.