Combat-Skills increasing too fast on grey mobs due to integer underflow

[Mat2095]

Description

Due to 60311e0 an underflow can occur here:

TrinityCore/src/server/game/Entities/Player/Player.cpp

Line 5783 in 400aef3

uint8 lvldif = moblevel - greylevel;

in the Player::UpdateCombatSkills method. Before it was certain that moblevel >= greylevel, but now this isn't the case. If greylevel > moblevel, then moblevel - greylevel will be negative, but lvldif is unsigned, so it wraps around, resulting in a very high chance to increase the combat-skill.

Expected behaviour

Combat skills should have a low/moderate chance to increase when fighting grey mobs.

Steps to reproduce the problem

1. Fight a mob within grey level range
2. Notice the combat skills increasing way too often; way more often than when fighting a mob within green level range

Branch

3.3.5

TC rev. hash/commit

83c403c

Operating system

Windows 11 x64

Custom changes

None

[Mat2095]

AC fixed this in azerothcore/azerothcore-wotlk@f75aceb

[CraftedRO]

we don't really care about trashzerothcore and also this is not a big issue, if you ask me seems much more logically that the skill increase more on gray-level mobs that the other since their level is close to yours, and you also did not provide any proof.

[Mat2095]

their level is close to yours

That's wrong, green-level mobs are closer to the player than gray-level mobs. Also in general the chance decreases the lower the mob-level is (float chance = float(3 * lvldif * skilldif) / plevel;). It is not logical to then suddenly increase the chance tenfold if the mob-level is below some level. And that is not done explicitly, but implicitly due to an integer underflow.

Some example calculation: Assume a level 60 player, a mob that is 1 level below gray-level, and the player is at 295/300 skill.
With the underflow the chance is calculated as lvldif = 255, chance = 3 * 255 * 5 / 60 = 63.75.
If the mob was one level higher, the chance would be calculated as lvldif = 0, but it has a minimum of 3, so lvldif = 3, chance = 3 * 3 * 5 / 60 = 0.75, but it has a minimum of 1%, so chance = 1.
Doesn't seem very logical to me to have an increase from 1% to over 60% by going down one level.

we don't really care about trashzerothcore

This is not the right place for childish insults.

you also did not provide any proof

In my opinion it is obvious from the code and the changes made in 60311e0, that the underflow is an unintended side-effect that was simply overlooked at that time (I don't blame them). But if you want more proof, I can give the same source as the issue that introduced the underflow:
https://typhoonandrew.wordpress.com/2008/01/08/how-to-raise-your-weapon-skill/ second bullet-point
"If you swing 100 times at a level 1 mob you’ll gain the same amount as swinging at a much higher level mob."
It says you gain the "same amount", not a "much greater amount".

[github-actions]

95b83b5