I ask for help to fix the bug that causes the server to crash after adding code in the source

[Mateus Robeerto]

Please watch the videos where magic damage percentage is correct. However, we have a problem: when passing over fields of fire, energy, poison, etc., the server crashes. Also, when monsters try to attack me, a server crash also occurs. I can't identify the source of this bug and would like someone's help to adapt correctly to TFS 1.5. If someone is willing, they can take the code and make the necessary adaptations. When it's working properly, please let me know here and where I can fix bugs. I thank!

https://imgur.com/bWo02cC

The author released the codes and passed them to me. These codes were for TFS 1.2 in its original source. I tried adapting them for TFS 1.5, but the problem persists, causing the server to crash, as I mentioned earlier.
check the item, the 'increaseMagicPercent' function is working correctly and correctly displaying 'INC MAGIC', etc

You see teste robe (Arm:18, Inc.Magic +150%).
It weighs 71.00 oz

item id="41195" name="teste robe">
    <attribute key="weight" value="7100"/>
    <attribute key="armor" value="18"/>
    <attribute key="slotType" value="body"/>
    <attribute key="increaseMagicPercent" value="150"/>

</item>

In the second video, when I scroll over energy, fire, etc., the server crashes

https://imgur.com/vl3Kwzp

In the third video, when the monster tried to attack me, the server also crashed

https://imgur.com/dAWvrBu

Here are the codes I will make available:
In game.cpp file
after.

Player* attackerPlayer;
        if (attacker) {
            attackerPlayer = attacker->getPlayer();
        } else {
            attackerPlayer = nullptr;
        }


        Player* targetPlayer = target->getPlayer();
        if (attackerPlayer && targetPlayer && attackerPlayer->getSkull() == SKULL_BLACK && attackerPlayer->getSkullClient(targetPlayer) == SKULL_NONE) {
            return false;
        }


        damage.primary.value = std::abs(damage.primary.value);
        damage.secondary.value = std::abs(damage.secondary.value);


        int32_t healthChange = damage.primary.value + damage.secondary.value;
        if (healthChange == 0) {
            return true;
        }

add. Fix made by @lursky

// Inc Magic by lursky
        auto originList = { ORIGIN_RANGED, ORIGIN_MELEE, ORIGIN_CONDITION };
        auto it = std::find(originList.begin(), originList.end(), damage.origin);
        if (attackerPlayer && it == originList.end()) {
            int32_t magicPercentBonus = 0;
            for (int32_t slot = CONST_SLOT_FIRST; slot <= CONST_SLOT_LAST; ++slot) {
                Item* item = attackerPlayer->inventory[slot];
                if (item) {
                    const ItemType& iiType = Item::items[item->getID()];
                    const int32_t& slotPosition = item->getSlotPosition();
                    if (iiType.increaseMagicPercent && (iiType.slotPosition & slotPosition)) {
                        magicPercentBonus += iiType.increaseMagicPercent;
                    }
                }
            }

            if (magicPercentBonus > 0) {
                damage.primary.value += damage.primary.value * (magicPercentBonus / 100.0f);
            }
        }
        }

In item.cpp file
after.

if (it.abilities) {
            for (uint8_t i = SKILL_FIRST; i <= SKILL_LAST; i++) {
                if (!it.abilities->skills[i]) {
                    continue;
                }


                if (begin) {
                    begin = false;
                    s << " (";
                } else {
                    s << ", ";
                }


                s << getSkillName(i) << ' ' << std::showpos << it.abilities->skills[i] << std::noshowpos;
            }

add.

if (it.increaseMagicPercent) {
                if (begin) {
                    begin = false;
                    s << " (";
                }
                else {
                    s << ", ";
                }


                s << "Inc.Magic " << std::showpos << it.increaseMagicPercent << '%' << std::noshowpos;
            }

In items.cpp file
after.

{"worth", ITEM_PARSE_WORTH},

add:

{ "increasemagicpercent", ITEM_PARSE_INCREASEMAGICPERCENT },

Again, in the items.cpp file:
after.

case ITEM_PARSE_SUPPRESSCURSE: {
                    if (valueAttribute.as_bool()) {
                        abilities.conditionSuppressions |= CONDITION_CURSED;
                    }
                    break;
                }

add.

case ITEM_PARSE_INCREASEMAGICPERCENT: {
                    it.increaseMagicPercent = pugi::cast<int32_t>(valueAttribute.value());
                    break;
                }

In items.h file
after.

uint64_t worth = 0;

add.

int32_t increaseMagicPercent = 0;

Again, in the items.h file:
after.

ITEM_PARSE_WORTH,

add:

ITEM_PARSE_INCREASEMAGICPERCENT,

The percentage magic damage is working perfectly, however, the problem occurs when I step over fire, energy, etc., or when a monster attacks me, resulting in the server crash. Could someone review the codes and fix them here or via GitHub? Thank you for your attention.
@Evil Puncker @Nekiro @Sarah Wesker @Roddet @Xikini
Sorry, I'm not very good at C++ so xD

Note: Without source modifications, everything is working normally and there is no percentage magic damage. However, when making changes to the desired source, the server is crashing.

 

[lursky]

Use Visual studio call stack to debug the cause, or if you’re on linux generate core dump and check backtrace frame by frame by printing values.
Plain out of the posted code I see “attackerPlayer” can be nullptr and theres no null check so add “player” check like:

Player* player = attackerPlayer;
if (player && player->inventory[i]) {

 

[Mateus Robeerto]

Use Visual studio call stack to debug the cause, or if you’re on linux generate core dump and check backtrace frame by frame by printing values.
Plain out of the posted code I see “attackerPlayer” can be nullptr and theres no null check so add “player” check like:

Player* player = attackerPlayer;
if (player && player->inventory[i]) {

Now yes, everything is perfectly fine with no errors. I passed over fields of fire, energy, poison, etc., and even when the monsters attacked me, everything happened normally, without any problems. Percent magic damage is still working perfectly. Thank you very much!
Please, some administrator or moderator can change the title and other information, removing the videos and moving them to the C++ Codes area, intended for those who are interested in adding them related to "increaseMagicPercent".

 

[lursky]

Anyway I’d advice you to rewrite te code not to iterate over slots when “attackerPlayer” is not specified as it wont’t do anything.
I’m also not sure about percentage calculation as it’s not a percent from original damage so if you have for example three items it works like:

• damage without bonuses is 100
• player has 3 items that gives:

1. armor: 30%
2. legs: 20%
3. helmet: 10%

Shouldn’t that work more like sum of bonuses (30 + 20 + 10) which should give 60% bonus:

100 + (100 * 0.6) = 160

but you have it like:
100 + (100 * 0.3) = 130
130 + (130 * 0.2) = 156
156 + (156 * 0.1) = 171.6

 

[Mateus Robeerto]

I didn't even realize that, you noticed very quickly. I went to test it in the game and realized that I can't attack the player. Is there any way to resolve this? I'm not very good at C++... Oh, could you watch the video you mentioned about calculating the percentage? Is there any way to fix this?

https://imgur.com/MNJ8div

 

[lursky]

By the way you should also check if player has these items at correct slot but you're mixing slot ID's with slotPosition where slot position is meant to be used more like a mask e.g:

const int32_t& slotPosition = item->getSlotPosition();
if (slotPosition & SLOTP_HEAD) {
    ...
}

So checking whether slotPosition is less than slot ID is not a valid check.
and btw. this seems to only check items for these slots:

• CONST_SLOT_HEAD
• CONST_SLOT_NECKLACE
• CONST_SLOT_BACKPACK
• CONST_SLOT_ARMOR

Try with (remember about null checks for items as not always there's an item on player slot)

uint16_t itemId = item->getID();
const ItemType& it = Item::items[itemId];
const int32_t& slotPosition = item->getSlotPosition();
if (it.slotPosition & slotPosition) {
    ...
}

 

[Roddet]

uint16_t itemId = item->getID();
const ItemType& it = Item::items[itemId];
const int32_t& slotPosition = item->getSlotPosition();
if (it.slotPosition & slotPosition) {
    ...
}

By the way, this will always return true, the comparison must be based on the slot iteration
Also you need to unset the SLOTP_HAND if necessary, otherwise items will work on hands too

This could be a workaround

for (int32_t slot = CONST_SLOT_FIRST; slot <= CONST_SLOT_LAST; ++slot) {

    uint32_t slotPositionBits = 1 << (slot - 1);
    uint32_t slotPosition = item->getSlotPosition();

    if ((slotPosition != SLOTP_HAND && slotPosition != SLOTP_LEFT && slotPosition != SLOTP_RIGHT)
        && !(slotPosition & SLOTP_TWO_HAND))
        slotPosition &= ~SLOTP_HAND;

    if (slotPosition & slotPositionBits)
        // ok code
}

 

[lursky]

@Roddet Yeaa you're right, now when I look at it I see that Item::getSlotPosition is actually same thing and not like slot of that item instance:

Item::items[id]

So yea, I couldn't find any function for that in the core so maybe it's worth to acually define some utility for that as if someone would like to use for any other slot.

slots_t getSlotType(const ItemType& it) that looks promising but I'm not sure (need to look into how exactly is the slotPosition defined)

for (int32_t slot = CONST_SLOT_FIRST; slot <= CONST_SLOT_LAST; ++slot) {
    Item* item = attackerPlayer->inventory[slot];
    if (item) {
        const ItemType& iiType = Item::items[item->getID()];
        if (getSlotType(iiType) == slot) {
            ...
        }
    }
}

 

[Roddet]

Yeah, I could find it neither

I would personally go for something like this to arbirtialy get the slot bits, plus your method

inline uint32_t getSlotPositionBits(slots_t slot) {return 1 << ((uint8_t)slot - 1);}

Also I realized my code needs to check for the SLOTP_LEFT and SLOTP_RIGHT individually (incase the slot desired is "left-hand" or "right-hand")
Not the fanciest solution but should work xD

 

[lursky]

Yea at the begining it seemed easier but there are some edge cases that would require some additional logic
Not even speaking about different behavior based on ConfigManager::CLASSIC_EQUIPMENT_SLOTS (never used new slots logic but I guess it's not very different)

Edit: that would explain the current implementation regarding shields, nevered played 8.1+

Since you can only put a weapon at the right hand, and the left hand is just for shields, you cannot put two weapons nor two shields in your hands. Also if you don't have any weapon, you will fight with your fists only.