fix: wrong calculation from elemental damage for weapons

[dudantas]

Description

This PR fixes an issue with the calculation of elemental weapon damage. Previously, the damage calculation for elemental weapons duplicated the damage instead of distributing it as 60% elemental and 40% physical. This resulted in incorrect damage values and an overpowered effect for certain weapons.

The corrected logic ensures that the total damage remains consistent, splitting appropriately based on the elemental and physical proportions while applying target resistances/weaknesses correctly.

Behaviour

Actual

Using elemental weapons caused the damage calculation to incorrectly duplicate the total damage, leading to inflated results that did not respect the intended 60/40% distribution.

Expected

Using elemental weapons distributes the total damage correctly as 60% elemental and 40% physical, with adjustments for the target's resistances or weaknesses to the specific element.

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested

The fix has been tested thoroughly to verify the proper distribution of elemental and physical damage. Test cases include:

• Attacking monsters with elemental weaknesses.
• Attacking monsters with elemental resistances.

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I checked the PR checks reports
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works

Font: https://forum.portaltibia.com.br/topic/36599-armas-dano-elemental/

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[dudantas]

This is ready for testing now.

[github-actions]

This PR is stale because it has been open 45 days with no activity.

[lBaah]

Function getElementDamageValue() returns the attack of the element damage: 47 for example and not the actual elemental damage.

Edit:
Actually, just change WeaponMelee::getWeaponDamage attackValue:

const int32_t attackValue = std::max<int32_t>(0, item->getAttack() + getElementDamageValue());

It also fixes this issue: #3341

I suggest to change getElementDamageValue function name to getElementAttack to avoid confusion in the future, since it returns the elemental attack rather than the damage.

[dudantas]

Function getElementDamageValue() returns the attack of the element damage: 47 for example and not the actual elemental damage.

Edit: Actually, just change WeaponMelee::getWeaponDamage attackValue:

const int32_t attackValue = std::max<int32_t>(0, item->getAttack() + getElementDamageValue());

It also fixes this issue: #3341

I suggest to change getElementDamageValue function name to getElementAttack to avoid confusion in the future, since it returns the elemental attack rather than the damage.

I think you didn't understand the formula, the calculation should be done based on the weapon's attack (configured in items.xml) and not the currently damage.

[lBaah]

Function getElementDamageValue() returns the attack of the element damage: 47 for example and not the actual elemental damage.
Edit: Actually, just change WeaponMelee::getWeaponDamage attackValue:
const int32_t attackValue = std::max<int32_t>(0, item->getAttack() + getElementDamageValue());
It also fixes this issue: #3341
I suggest to change getElementDamageValue function name to getElementAttack to avoid confusion in the future, since it returns the elemental attack rather than the damage.

I think you didn't understand the formula, the calculation should be done based on the weapon's attack (configured in items.xml) and not the currently damage.

Yes I did undertood the formula. The issue is that getWeaponDamage returns the damage using the physical attack only, ignoring the elemental part of the attack which does make part of the total damage.

The way it is, the damage distribuition will be calculated based on that attack.

Take phantasmal axe which has Atk:5 physical + 49 fire, for example. The "totalDamage" variable will be calculated using the 5 atk only, that will be returned from getWeaponDamage. The +49 attack will not contribute to the damage, so the damage calculated from 5 atk will be distributed according to proportional damage of phys/elemental, which is correct.

Theres no issue with the proportional distribution, but with the totalDamage calculation.

[dudantas]

Function getElementDamageValue() returns the attack of the element damage: 47 for example and not the actual elemental damage.

Edit: Actually, just change WeaponMelee::getWeaponDamage attackValue:

const int32_t attackValue = std::max<int32_t>(0, item->getAttack() + getElementDamageValue());

It also fixes this issue: #3341

I suggest to change getElementDamageValue function name to getElementAttack to avoid confusion in the future, since it returns the elemental attack rather than the damage.

Based on the logs and the breakpoint values, the current formula is intentional. We use the weapon’s base physical attack to compute totalDamage, then split that total proportionally according to item->getAttack() (physical) vs. getElementDamageValue() (elemental). If we added the elemental value directly into getWeaponDamage, we would be double-counting the elemental portion and inflating the final damage. So, getElementDamageValue() is correctly returning the weapon’s elemental attack for the distribution stage, rather than raw damage, and this matches our intended formula.

[lBaah]

Debug WeaponMelee::getWeaponDamage.

Where does the damage calculation accounts for the elemental attack? Because I can't see it. The 49 attack is completely ignored.

int32_t WeaponMelee::getWeaponDamage(const std::shared_ptr<Player> &player, const std::shared_ptr<Creature> &, const std::shared_ptr<Item> &item, bool maxDamage /*= false*/) const {
	using namespace std;
	const int32_t attackSkill = player->getWeaponSkill(item);
	const int32_t attackValue = std::max<int32_t>(0, item->getAttack());
	const float attackFactor = player->getAttackFactor();
	const uint32_t level = player->getLevel();

	const int32_t maxValue = static_cast<int32_t>(Weapons::getMaxWeaponDamage(level, attackSkill, attackValue, attackFactor, true) * player->getVocation()->meleeDamageMultiplier);

	const int32_t minValue = level / 5;

	if (maxDamage) {
		return -maxValue;
	}

	return -normal_random(minValue, (maxValue * static_cast<int32_t>(player->getVocation()->meleeDamageMultiplier)));
}

Edit

I hope this help you see the issue i'm talking about: https://onecompiler.com/lua/43c79juqw

local player = {
  attackSkill = 100,
	attackFactor = 1.0;
	level = 1000
}

local weapons = {
  ["phantasmal axe"] = {
    attack = 5,
    elementalAttack = 49
  },
  ["sickle"] = {
    attack = 5,
    elementalAttack = 0
  }
}

function getMaxWeaponDamage(level, attackSkill, attackValue, attackFactor)
  return (0.085 * attackFactor * attackValue * attackSkill) + level / 5
end

function getWeaponDamage(weapon)
  local maxDamage = getMaxWeaponDamage(player.level, player.attackSkill, weapons[weapon].attack, player.attackFactor)
  local minDamage = player.level / 5
  
  print(string.format("[getWeaponDamage]: %s min %s, max %s\n", weapon, minDamage, maxDamage))
end

function getWeaponDamageWithElementAttack(weapon)
  local maxDamage = getMaxWeaponDamage(player.level, player.attackSkill, weapons[weapon].attack + weapons[weapon].elementalAttack, player.attackFactor)
  local minDamage = player.level / 5
  
  print(string.format("[getWeaponDamageWithElementAttack]: %s min %s, max %s\n", weapon, minDamage, maxDamage))
end

getWeaponDamage("sickle")
getWeaponDamage("phantasmal axe")
getWeaponDamageWithElementAttack("sickle")
getWeaponDamageWithElementAttack("phantasmal axe")

[dudantas]

Debug WeaponMelee::getWeaponDamage.

Where does the damage calculation accounts for the elemental attack? Because I can't see it. The 49 attack is completely ignored.

int32_t WeaponMelee::getWeaponDamage(const std::shared_ptr<Player> &player, const std::shared_ptr<Creature> &, const std::shared_ptr<Item> &item, bool maxDamage /*= false*/) const {
	using namespace std;
	const int32_t attackSkill = player->getWeaponSkill(item);
	const int32_t attackValue = std::max<int32_t>(0, item->getAttack());
	const float attackFactor = player->getAttackFactor();
	const uint32_t level = player->getLevel();

	const int32_t maxValue = static_cast<int32_t>(Weapons::getMaxWeaponDamage(level, attackSkill, attackValue, attackFactor, true) * player->getVocation()->meleeDamageMultiplier);

	const int32_t minValue = level / 5;

	if (maxDamage) {
		return -maxValue;
	}

	return -normal_random(minValue, (maxValue * static_cast<int32_t>(player->getVocation()->meleeDamageMultiplier)));
}

Edit

I hope this help you see the issue i'm talking about: https://onecompiler.com/lua/43c79juqw

local player = {
  attackSkill = 100,
	attackFactor = 1.0;
	level = 1000
}

local weapons = {
  ["phantasmal axe"] = {
    attack = 5,
    elementalAttack = 49
  },
  ["sickle"] = {
    attack = 5,
    elementalAttack = 0
  }
}

function getMaxWeaponDamage(level, attackSkill, attackValue, attackFactor)
  return (0.085 * attackFactor * attackValue * attackSkill) + level / 5
end

function getWeaponDamage(weapon)
  local maxDamage = getMaxWeaponDamage(player.level, player.attackSkill, weapons[weapon].attack, player.attackFactor)
  local minDamage = player.level / 5
  
  print(string.format("[getWeaponDamage]: %s min %s, max %s\n", weapon, minDamage, maxDamage))
end

function getWeaponDamageWithElementAttack(weapon)
  local maxDamage = getMaxWeaponDamage(player.level, player.attackSkill, weapons[weapon].attack + weapons[weapon].elementalAttack, player.attackFactor)
  local minDamage = player.level / 5
  
  print(string.format("[getWeaponDamageWithElementAttack]: %s min %s, max %s\n", weapon, minDamage, maxDamage))
end

getWeaponDamage("sickle")
getWeaponDamage("phantasmal axe")
getWeaponDamageWithElementAttack("sickle")
getWeaponDamageWithElementAttack("phantasmal axe")

Function getElementDamageValue() returns the attack of the element damage: 47 for example and not the actual elemental damage.
Edit: Actually, just change WeaponMelee::getWeaponDamage attackValue:
const int32_t attackValue = std::max<int32_t>(0, item->getAttack() + getElementDamageValue());
It also fixes this issue: #3341
I suggest to change getElementDamageValue function name to getElementAttack to avoid confusion in the future, since it returns the elemental attack rather than the damage.

I think you didn't understand the formula, the calculation should be done based on the weapon's attack (configured in items.xml) and not the currently damage.

Yes I did undertood the formula. The issue is that getWeaponDamage returns the damage using the physical attack only, ignoring the elemental part of the attack which does make part of the total damage.

The way it is, the damage distribuition will be calculated based on that attack.

Take phantasmal axe which has Atk:5 physical + 49 fire, for example. The "totalDamage" variable will be calculated using the 5 atk only, that will be returned from getWeaponDamage. The +49 attack will not contribute to the damage, so the damage calculated from 5 atk will be distributed according to proportional damage of phys/elemental, which is correct.

Theres no issue with the proportional distribution, but with the totalDamage calculation.

Debug WeaponMelee::getWeaponDamage.

Where does the damage calculation accounts for the elemental attack? Because I can't see it. The 49 attack is completely ignored.

int32_t WeaponMelee::getWeaponDamage(const std::shared_ptr<Player> &player, const std::shared_ptr<Creature> &, const std::shared_ptr<Item> &item, bool maxDamage /*= false*/) const {
	using namespace std;
	const int32_t attackSkill = player->getWeaponSkill(item);
	const int32_t attackValue = std::max<int32_t>(0, item->getAttack());
	const float attackFactor = player->getAttackFactor();
	const uint32_t level = player->getLevel();

	const int32_t maxValue = static_cast<int32_t>(Weapons::getMaxWeaponDamage(level, attackSkill, attackValue, attackFactor, true) * player->getVocation()->meleeDamageMultiplier);

	const int32_t minValue = level / 5;

	if (maxDamage) {
		return -maxValue;
	}

	return -normal_random(minValue, (maxValue * static_cast<int32_t>(player->getVocation()->meleeDamageMultiplier)));
}

Edit

I hope this help you see the issue i'm talking about: https://onecompiler.com/lua/43c79juqw

local player = {
  attackSkill = 100,
	attackFactor = 1.0;
	level = 1000
}

local weapons = {
  ["phantasmal axe"] = {
    attack = 5,
    elementalAttack = 49
  },
  ["sickle"] = {
    attack = 5,
    elementalAttack = 0
  }
}

function getMaxWeaponDamage(level, attackSkill, attackValue, attackFactor)
  return (0.085 * attackFactor * attackValue * attackSkill) + level / 5
end

function getWeaponDamage(weapon)
  local maxDamage = getMaxWeaponDamage(player.level, player.attackSkill, weapons[weapon].attack, player.attackFactor)
  local minDamage = player.level / 5
  
  print(string.format("[getWeaponDamage]: %s min %s, max %s\n", weapon, minDamage, maxDamage))
end

function getWeaponDamageWithElementAttack(weapon)
  local maxDamage = getMaxWeaponDamage(player.level, player.attackSkill, weapons[weapon].attack + weapons[weapon].elementalAttack, player.attackFactor)
  local minDamage = player.level / 5
  
  print(string.format("[getWeaponDamageWithElementAttack]: %s min %s, max %s\n", weapon, minDamage, maxDamage))
end

getWeaponDamage("sickle")
getWeaponDamage("phantasmal axe")
getWeaponDamageWithElementAttack("sickle")
getWeaponDamageWithElementAttack("phantasmal axe")

Thanks for the review! Initially, your explanation was quite complex to follow, but your last comment was much more direct and clear. I appreciate your patience in breaking it down.

Now I see the issue more clearly—getWeaponDamage was only considering item->getAttack(), ignoring the elemental attack in the total damage calculation. Updating it to use combinedAttack (physical + elemental) makes sense.

Your input was valuable, and I appreciate the detailed breakdown!

Fixed here: 0a294da

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud