Skip to content

Commit

Permalink
fix: forge correct slots (#2850)
Browse files Browse the repository at this point in the history 
Resolves #2718
  • Loading branch information
@dudantas
dudantas committed Sep 25, 2024
1 parent 4bc7a5c commit 11bd3a6
Showing 1 changed file with 29 additions and 11 deletions.
40 changes: 29 additions & 11 deletions src/server/network/protocol/protocolgame.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5370,7 +5370,8 @@ void ProtocolGame::sendForgingData() {
void ProtocolGame::sendOpenForge() {

Check warning on line 5370 in src/server/network/protocol/protocolgame.cpp

View workflow job for this annotation

GitHub Actions / Qodana for C/C++

function-cognitive-complexity 

function 'sendOpenForge' has cognitive complexity of 119 (threshold 25)
// We will use it when sending the bytes to send the item information to the client
std::map<uint16_t, std::map<uint8_t, uint16_t>> fusionItemsMap;
std::map<int32_t, std::map<uint16_t, std::map<uint8_t, uint16_t>>> convergenceItemsMap;
std::map<int32_t, std::map<uint16_t, std::map<uint8_t, uint16_t>>> convergenceFusionItemsMap;
std::map<int32_t, std::map<uint16_t, std::map<uint8_t, uint16_t>>> convergenceTransferItemsMap;
std::map<uint16_t, std::map<uint8_t, uint16_t>> donorTierItemMap;
std::map<uint16_t, std::map<uint8_t, uint16_t>> receiveTierItemMap;

Expand Down Expand Up @@ -5405,15 +5406,20 @@ void ProtocolGame::sendOpenForge() {
getForgeInfoMap(item, receiveTierItemMap);
}
if (itemClassification == 4) {
getForgeInfoMap(item, convergenceItemsMap[item->getClassification()]);
auto slotPosition = item->getSlotPosition();
if ((slotPosition & SLOTP_TWO_HAND) != 0) {
slotPosition = SLOTP_HAND;
}
getForgeInfoMap(item, convergenceFusionItemsMap[slotPosition]);
getForgeInfoMap(item, convergenceTransferItemsMap[item->getClassification()]);
}
}
}

// Checking size of map to send in the addByte (total fusion items count)
uint8_t fusionTotalItemsCount = 0;

Check warning on line 5420 in src/server/network/protocol/protocolgame.cpp

View workflow job for this annotation

GitHub Actions / Qodana for C/C++

misra-cpp2008-5-0-11 

MISRA 5-0-11: The plain char type shall only be used for the storage and use of character values
for (const auto &[itemId, tierAndCountMap] : fusionItemsMap) {
for (const auto [itemTier, itemCount] : tierAndCountMap) {
for (const auto &[itemTier, itemCount] : tierAndCountMap) {
if (itemCount >= 2) {
fusionTotalItemsCount++;
}
Expand All @@ -5430,7 +5436,7 @@ void ProtocolGame::sendOpenForge() {

msg.add<uint16_t>(fusionTotalItemsCount);
for (const auto &[itemId, tierAndCountMap] : fusionItemsMap) {
for (const auto [itemTier, itemCount] : tierAndCountMap) {
for (const auto &[itemTier, itemCount] : tierAndCountMap) {
if (itemCount >= 2) {
msg.addByte(0x01); // Number of friend items?
msg.add<uint16_t>(itemId);
Expand All @@ -5452,12 +5458,12 @@ void ProtocolGame::sendOpenForge() {
1 byte: tier
2 bytes: count
*/
for (const auto &[slot, itemMap] : convergenceItemsMap) {
for (const auto &[slot, itemMap] : convergenceFusionItemsMap) {
uint8_t totalItemsCount = 0;
auto totalItemsCountPosition = msg.getBufferPosition();
msg.skipBytes(1); // Total items count
for (const auto &[itemId, tierAndCountMap] : itemMap) {
for (const auto [tier, itemCount] : tierAndCountMap) {
for (const auto &[tier, itemCount] : tierAndCountMap) {
if (tier >= maxConfigTier) {
continue;
}
Expand Down Expand Up @@ -5488,11 +5494,15 @@ void ProtocolGame::sendOpenForge() {
// Let's access the itemType to check the item's (donator of tier) classification level
// Must be the same as the item that will receive the tier
const ItemType &donorType = Item::items[itemId];
auto donorSlotPosition = donorType.slotPosition;
if ((donorSlotPosition & SLOTP_TWO_HAND) != 0) {

Check warning on line 5498 in src/server/network/protocol/protocolgame.cpp

View workflow job for this annotation

GitHub Actions / Qodana for C/C++

misra-cpp2008-4-5-2 

MISRA 4-5-2: Expressions with type enum shall not be used as operands to built-in operators other than the subscript operator [ ], the assignment operator =, the equality operators == and !=, the unary & operator, and the relational operators <, <=, >, >=
donorSlotPosition = SLOTP_HAND;

Check warning on line 5499 in src/server/network/protocol/protocolgame.cpp

View workflow job for this annotation

GitHub Actions / Qodana for C/C++

misra-cpp2008-5-0-6 

MISRA 5-0-6: An implicit integral or floating-point conversion shall not reduce the size of the underlying type
}

// Total count of item (donator of tier)
auto donorTierTotalItemsCount = getIterationIncreaseCount(tierAndCountMap);
msg.add<uint16_t>(donorTierTotalItemsCount);
for (const auto [donorItemTier, donorItemCount] : tierAndCountMap) {
for (const auto &[donorItemTier, donorItemCount] : tierAndCountMap) {
msg.add<uint16_t>(itemId);
msg.addByte(donorItemTier);
msg.add<uint16_t>(donorItemCount);
Expand All @@ -5502,7 +5512,11 @@ void ProtocolGame::sendOpenForge() {
for (const auto &[iteratorItemId, unusedTierAndCountMap] : receiveTierItemMap) {
// Let's access the itemType to check the item's (receiver of tier) classification level
const ItemType &receiveType = Item::items[iteratorItemId];
if (donorType.upgradeClassification == receiveType.upgradeClassification) {
auto receiveSlotPosition = receiveType.slotPosition;
if ((receiveSlotPosition & SLOTP_TWO_HAND) != 0) {

Check warning on line 5516 in src/server/network/protocol/protocolgame.cpp

View workflow job for this annotation

GitHub Actions / Qodana for C/C++

misra-cpp2008-4-5-2 

MISRA 4-5-2: Expressions with type enum shall not be used as operands to built-in operators other than the subscript operator [ ], the assignment operator =, the equality operators == and !=, the unary & operator, and the relational operators <, <=, >, >=
receiveSlotPosition = SLOTP_HAND;

Check warning on line 5517 in src/server/network/protocol/protocolgame.cpp

View workflow job for this annotation

GitHub Actions / Qodana for C/C++

misra-cpp2008-5-0-6 

MISRA 5-0-6: An implicit integral or floating-point conversion shall not reduce the size of the underlying type
}
if (donorType.upgradeClassification == receiveType.upgradeClassification && donorSlotPosition == receiveSlotPosition) {
receiveTierTotalItemCount++;
}
}
Expand All @@ -5513,8 +5527,12 @@ void ProtocolGame::sendOpenForge() {
for (const auto &[receiveItemId, receiveTierAndCountMap] : receiveTierItemMap) {
// Let's access the itemType to check the item's (receiver of tier) classification level
const ItemType &receiveType = Item::items[receiveItemId];
if (donorType.upgradeClassification == receiveType.upgradeClassification) {
for (const auto [receiveItemTier, receiveItemCount] : receiveTierAndCountMap) {
auto receiveSlotPosition = receiveType.slotPosition;
if ((receiveSlotPosition & SLOTP_TWO_HAND) != 0) {

Check warning on line 5531 in src/server/network/protocol/protocolgame.cpp

View workflow job for this annotation

GitHub Actions / Qodana for C/C++

misra-cpp2008-4-5-2 

MISRA 4-5-2: Expressions with type enum shall not be used as operands to built-in operators other than the subscript operator [ ], the assignment operator =, the equality operators == and !=, the unary & operator, and the relational operators <, <=, >, >=
receiveSlotPosition = SLOTP_HAND;
}
if (donorType.upgradeClassification == receiveType.upgradeClassification && donorSlotPosition == receiveSlotPosition) {
for (const auto &[receiveItemTier, receiveItemCount] : receiveTierAndCountMap) {
msg.add<uint16_t>(receiveItemId);
msg.add<uint16_t>(receiveItemCount);
}
Expand All @@ -5540,7 +5558,7 @@ void ProtocolGame::sendOpenForge() {
2 bytes: item id
2 bytes: count
*/
for (const auto &[slot, itemMap] : convergenceItemsMap) {
for (const auto &[slot, itemMap] : convergenceTransferItemsMap) {
uint16_t donorCount = 0;
uint16_t receiverCount = 0;
auto donorCountPosition = msg.getBufferPosition();
Expand Down

2 comments on commit 11bd3a6

@lBaah
Copy link

@lBaah lBaah commented on 11bd3a6 Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this shoud be reverted and a discussion opened since there are other issues with forge, including crashing the server with it.

@dudantas
Copy link
Contributor Author

@dudantas dudantas commented on 11bd3a6 Sep 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this shoud be reverted and a discussion opened since there are other issues with forge, including crashing the server with it.

I believe reverting the merged pull request is not the right course of action. The pull request in question successfully addressed the issues it was intended to resolve, and no further problems with the forge (crashing server) have been reported since its implementation.

If there are other issues, such as crashes or unexpected behaviors with the forge, it would be more constructive to open a new issue and report the specific details. This way, we can investigate and resolve the remaining problems without rolling back changes that have already been effective.

I encourage you to open a detailed issue report if you encounter additional problems with the forge. We are happy to collaborate on a solution.

The pull request remained open from August 24th to September 24th, giving ample time for review and discussion. Considering this, it's clear that the changes were thoroughly evaluated before being merged.

Please sign in to comment.