🐛 [Bug] Creature damage configuration option does not apply to physical ranged attacks

[cbluebdk]

🐛 Bug report

It seems to me that the Rate.Creature.*.Damage option in mangosd.conf does not apply to physical ranged attacks (e.g. bow or crossbow).

I compiled vmangos on Arch Linux and used the default mangosd.conf.
I play with a client version 1.12.1, build number 5875.

I created a Level 13 Human Warrior,
set the Defense skill the 65/65,
used only the starting equipment,
i.e. I had nearly no armor.

Then I went to Westfall and got attacked by a Level 12 'Riverpaw Scout'.
This one attacks with a crossbow,
and switches to melee as soon as the player gets in melee range.
I set my HP to 100000 and watched the damage.
I ignored critical hits, blocked, or crushing damage.
The ranged attacks had a strength of 19-25,
while the melee attack did 16-21 damage to me.

Now I set Rate.Creature.Normal.Damage = 0.2 in mangosd.conf and restarted the server.
Same situation as before;
melee attacks now between 2-4,
but the ranged attacks were still at 19-25.
The config option apparently only had an effect on the melee damage,
but not on the range damage.

Expected behavior

Here is a part of mangosd.conf:

#    Rate.Creature.Normal.Damage
#    Rate.Creature.Elite.Elite.Damage
#    Rate.Creature.Elite.RAREELITE.Damage
#    Rate.Creature.Elite.WORLDBOSS.Damage
#    Rate.Creature.Elite.RARE.Damage
#        Creature Damage Rates.
#        Examples: 2 - creatures will deal double damage with melee attacks
#
#    Rate.Creature.Normal.SpellDamage
#    Rate.Creature.Elite.Elite.SpellDamage
#    Rate.Creature.Elite.RAREELITE.SpellDamage
#    Rate.Creature.Elite.WORLDBOSS.SpellDamag
#    Rate.Creature.Elite.RARE.SpellDamage
#        Creature Spell Damage Rates.
#        Examples: 2 - creatures will deal double damage with spells

It says 'Damage' affects melee damage,
but it doesn't say it only affects melee damage.
So it's not clear to me what is the intended behavior.
It would seem strange to me if one could modify melee and spell damage,
but not physical range damage.

Look in the source code where the config option is used:

grep -r "Rate.Creature.Normal.Damage" *

src/game/World.cpp:    setConfigPos(CONFIG_FLOAT_RATE_CREATURE_NORMAL_DAMAGE,               "Rate.Creature.Normal.Damage", 1.0f);
src/mangosd/mangosd.conf.dist.in:#    Rate.Creature.Normal.Damage
src/mangosd/mangosd.conf.dist.in:Rate.Creature.Normal.Damage = 1

The config option is set in src/game/World.cpp and stored in the array m_configFloatValues at index CONFIG_FLOAT_RATE_CREATURE_NORMAL_DAMAGE.
Look where this config option is used:

grep -r "CONFIG_FLOAT_RATE_CREATURE_NORMAL_DAMAGE" *

src/game/World.h:    CONFIG_FLOAT_RATE_CREATURE_NORMAL_DAMAGE,
src/game/Objects/Creature.cpp:            return sWorld.getConfig(CONFIG_FLOAT_RATE_CREATURE_NORMAL_DAMAGE);
src/game/World.cpp:    setConfigPos(CONFIG_FLOAT_RATE_CREATURE_NORMAL_DAMAGE,               "Rate.Creature.Normal.Damage", 1.0f);

In src/game/Objects/Creature.cpp, the function _GetDamageMod returns this config option value:

float Creature::_GetDamageMod(int32 rank)
{
    switch (rank)                                           // define rates for each elite rank
    {
        case CREATURE_ELITE_NORMAL:
            return sWorld.getConfig(CONFIG_FLOAT_RATE_CREATURE_NORMAL_DAMAGE);
        [...]
    }
}

Let's see where the function _GetDamageMod is called:

grep -r "_GetDamageMod" *

game/Objects/Creature.h:        static float _GetDamageMod(int32 rank);             // Get custom factor to scale damage (default 1, CONFIG_FLOAT_RATE_*_DAMAGE)
game/Objects/Pet.cpp:    float damageMod = owner->IsPlayer() ? 1.0f : _GetDamageMod(cinfo->rank);
game/Objects/Creature.cpp:    float const damageMod = _GetDamageMod(rank);
game/Objects/Creature.cpp:float Creature::_GetDamageMod(int32 rank)
game/Objects/Creature.cpp:        float const damageMod = _GetDamageMod(m_creatureInfo->rank);
game/Objects/Creature.cpp:    float const damageMod = _GetDamageMod(m_creatureInfo->rank);

So we have the declaration in the header file, the function header in Creature.cpp,
and 4 function calls in total: three in Creature.cpp and one in Pet.cpp.

The call in Pet.cpp seem to apply to enemy pets, which is not relevant here.

Another call is in the function GetDefaultDamageRange.

grep -r "GetDefaultDamageRange" *

src/game/Objects/Creature.h:        void GetDefaultDamageRange(float& dmgMin, float& dmgMax) const;
src/game/Objects/Creature.cpp:void Creature::GetDefaultDamageRange(float& dmgMin, float& dmgMax) const
src/scripts/eastern_kingdoms/burning_steppes/molten_core/boss_majordomo_executus.cpp:                            pAdd->GetDefaultDamageRange(dmgMin, dmgMax);
src/scripts/eastern_kingdoms/stranglethorn_vale/zulgurub/boss_arlokk.cpp:                m_creature->GetDefaultDamageRange(dmgMin, dmgMax);
src/scripts/eastern_kingdoms/stranglethorn_vale/zulgurub/boss_marli.cpp:                m_creature->GetDefaultDamageRange(dmgMin, dmgMax);
src/scripts/eastern_kingdoms/stranglethorn_vale/zulgurub/boss_marli.cpp:                m_creature->GetDefaultDamageRange(dmgMin, dmgMax);

This function is only called in some scripts for bosses in mc and zg,
so it is not relevant here.
The same applies to the function ResetStats.
Inside this function, the _GetDamageMod function is called,
but the ResetStats function itself is only called in some scripts for zg bosses:

grep -r "ResetStats" *

src/game/Chat/Chat.h:        bool HandleResetStatsCommand(char* args);
src/game/Chat/Chat.cpp:        { "stats",          SEC_DEVELOPER,      true,  &ChatHandler::HandleResetStatsCommand,          "", nullptr },
src/game/Objects/Creature.h:        void ResetStats();
src/game/Objects/Creature.cpp:void Creature::ResetStats()
src/game/Commands/CharacterCommands.cpp:static bool HandleResetStatsOrLevelHelper(Player* player)
src/game/Commands/CharacterCommands.cpp:    if (!HandleResetStatsOrLevelHelper(target))
src/game/Commands/CharacterCommands.cpp:bool ChatHandler::HandleResetStatsCommand(char* args)
src/game/Commands/CharacterCommands.cpp:    if (!HandleResetStatsOrLevelHelper(target))
src/scripts/eastern_kingdoms/stranglethorn_vale/zulgurub/boss_arlokk.cpp:        m_creature->ResetStats();
src/scripts/eastern_kingdoms/stranglethorn_vale/zulgurub/boss_mandokir.cpp:        m_creature->ResetStats();
src/scripts/eastern_kingdoms/stranglethorn_vale/zulgurub/boss_venoxis.cpp:        m_creature->ResetStats();
src/scripts/eastern_kingdoms/stranglethorn_vale/zulgurub/boss_marli.cpp:        m_creature->ResetStats();

That leaves only one call of _GetDamageMod in Creature.cpp.
It is in the function InitStatsForLevel:

void Creature::InitStatsForLevel(float percentHealth, float percentMana)
{
    // [...]

    // damage
    float const damageMod = _GetDamageMod(rank);
    float const meleeDamageAverage = pCLS->melee_damage * cinfo->damage_multiplier * damageMod;
    float const meleeDamageVariance = meleeDamageAverage * cinfo->damage_variance;
    float const rangedDamageAverage = pCLS->ranged_damage * cinfo->damage_multiplier * damageMod;
    float const rangedDamageVariance = rangedDamageAverage * cinfo->damage_variance;

    SetBaseWeaponDamage(BASE_ATTACK, MINDAMAGE, meleeDamageAverage - meleeDamageVariance);
    SetBaseWeaponDamage(BASE_ATTACK, MAXDAMAGE, meleeDamageAverage + meleeDamageVariance);

    SetBaseWeaponDamage(OFF_ATTACK, MINDAMAGE, (meleeDamageAverage - meleeDamageVariance) / 2.0f);
    SetBaseWeaponDamage(OFF_ATTACK, MAXDAMAGE, (meleeDamageAverage + meleeDamageVariance) / 2.0f);

    SetBaseWeaponDamage(RANGED_ATTACK, MINDAMAGE, rangedDamageAverage - rangedDamageVariance);
    SetBaseWeaponDamage(RANGED_ATTACK, MAXDAMAGE, rangedDamageAverage + rangedDamageVariance);

    // [...]
}

This seems to me the right place to look at.
I see that the ranged damage is multiplied by damageMod.
To me this looks how it is supposed to be,
however in the game the damage modifier is not applied.
This is where I don't know what to do anymore.

Why is this important?

I like to play alone locally,
and to do dungeons I set the HP and damage of creatures to 0.2.
This works fine,
but when I encounter an enemy that uses physical range attacks,
its damage output is far higher than all the other enemies.

I label this as a bug,
because I suspect this is not intended behavior.
If it is,
I would appreciate if someone can point me to what parts of the code need to be changed to make it work.
Then I could at least apply a patch for myself before compiling the core.

Steps to reproduce

1. Create a new character
2. .level 12
3. .setskill 95 65 // Set Defense skill to max for current level to avoid crushing blows
4. .tele westfall
5. Get attacked by a Riverpaw Scout and watch the Combat Log
6. Set Rate.Creature.Normal.Damage to 0.2 and compare damage

Version & Environment

Client Version: 1.12.1.5875

Commit Hash: https://github.com/vmangos/core/tree/9b3b375ef34e92b70e343f057eebea6df75601de

OS Client: Arch Linux
OS Server: Arch Linux

Crashlog

• None

[cbluebdk]

I edited the src/game/Objects/Creature.cpp and recompiled the core.

When changing line 1752
float const meleeDamageAverage = pCLS->melee_damage * cinfo->damage_multiplier * damageMod;
to
float const meleeDamageAverage = pCLS->melee_damage * cinfo->damage_multiplier * damageMod * 10;
then the creature's melee damage is multiplied by 10, as expected.

However, when I change line 1754
float const rangedDamageAverage = pCLS->ranged_damage * cinfo->damage_multiplier * damageMod;
to
float const rangedDamageAverage = pCLS->ranged_damage * cinfo->damage_multiplier * damageMod * 10;
then the creature's ranged damage does not change.

I would expect it to be multiplied by 10, like with melee damage.

[cbluebdk]

As stated in the OP, the min and max ranged damage is calculated in the function InitStatsForLevel. From there, they are written into m_weaponDamageby calling the function SetBaseWeaponDamage.

These values are read by the function UpdateDamagePhysical in StatSystem.cpp by calling GetWeaponDamageRange. The values are then slightly modified, and written into UNIT_FIELD_MINRANGEDDAMAGE and UNIT_FIELD_MAXRANGEDDAMAGE respectively, by calling SetStatFloatValue.

We can inspect those values in-game by executing .unit statinfo. I set Rate.Creature.Normal.Damage = 100 in mangosd.conf and executed .unit statinfo on a Level 12 Riverpaw Scout. The output was:

Min damage: 1691.77
Max damage: 2242.57
...
Min ranged damage: 1581
Max ranged damage: 2096

The correct damage values are written into the UNIT_FIELD_MINRANGEDDAMAGE and UNIT_FIELD_MAXRANGEDDAMAGE. While the enemy indeed does around 2000 melee damage, the actual ranged damage stays at 20.

These values are read by the function CalculateDamage in Unit.cpp. This function takes the attack type as an argument. If the attack type is RANGED_ATTACK, then the UNIT_FIELD_MINRANGEDDAMAGE and UNIT_FIELD_MAXRANGEDDAMAGE are read by calling GetFloatValue. However, this function is not called when an enemy makes a ranged attack!

I print a message into the mangos console every time this function is executed, and it turns out that this function is only called when an enemy makes an attack with either the main-hand (attack type BASE_ATTACK) or the off-hand (attack type OFF_ATTACK), but it is not called when an enemy makes a ranged attack.

Now I need to find out how the damage of a ranged attack is actually calculated.

[cbluebdk]

As I noted in the previous comment,
the function CalculateDamage is not executed on a ranged attack.
Instead, the function SpellCaster::CalculateSpellDamage is called.

This function differentiates between Weapon Attacks and Magical Attacks.
On a Magical Attack, the function SpellDamageBonusDone is called,
where the Rate.Creature.*.SpellDamage modifier from mangosd.conf is applied.
On a Weapon Attack, the function MeleeDamageBonusDone is called,
where no modifier from mangosd.conf is applied.

This makes sense for melee attacks.
The function MeleeDamageBonusDone is also called in Unit::CalculateMeleeDamage.
In this case the Rate.Creature.*.Damage modifier is already applied on the weapon damage itself.
But as I stated in an earlier comment,
the ranged weapon damage seems to be ignored;
here it would make sense to apply the damage modifier in MeleeDamageBonusDone.

My current solution is this:
In analogy to how the spell damage modifier is applied in SpellDamageBonusDone,
I applied the physical damage modifier in MeleeDamageBonusDone;
but only for ranged attacks, to avoid applying the modifier twice on melee attacks.
To apply the fix, insert the following at line 1170 of src/game/Objects/SpellCaster.cpp:

if (attType == RANGED_ATTACK)
    DonePercent *= Creature::_GetDamageMod(((Creature*)this)->GetCreatureInfo()->rank);

I tested this in Westfall with the Riverpaw Scouts and the Defias Pillagers in Moonbrook.
The Rate.Creature.Normal.Damage modifier is now correctly applied to both melee and physical ranged attacks.
The Rate.Creature.Normal.SpellDamage modifier is still correctly applied to ranged magic attacks (e.g. Fireball).

Still, I cannot be sure that there aren't any unwanted side-effects;
for now, I consider this a workaround, not a fix.
This solution requires that all ranged attacks ignore the ranged weapon damage set by InitStatsForLevel.
Otherwise, the modifier could be applied twice.
Are there any magical crossbows that cause fire damage,
where now both the Damage and SpellDamage modifier are applied?
Does this really only affect creatures,
or are some magical ranged attacks of players also affected by this, e.g. some Hunter abilities?
Maybe there are enemy pets that use ranged attacks, where the modifier is still is not applied?
I haven't understood the source code enough to be confident about this.
Maybe there are people around who are more experienced with the codebase who can comment on this solution.

[cbluebdk]

I can confirm that the issue is now fixed.

In my opinion it is not ideal that physical ranged attacks are affected by the SpellDamage configuration option, and not by the Damage option. However, given that the damage calculation of physical ranged attacks is done in SpellCaster.cpp, it kind of makes sense to use SpellDamage here. Also, at least I will always have the same value for Damage and SpellDamage, so in practise this is not relevant to me.

Thank you for working on this!