PaulQbFeng / too-tanky

3 stars 0 forks source link

ratio > percent / 100 #43

Closed Lysxia closed 1 year ago

Lysxia commented 1 year ago

There are a bunch of _percent stats and _ratio stats. We should standardize one representation (_ratio by popular vote).

PaulQbFeng commented 1 year ago

Would you change damage_modifier_percent to damage_modifier_ratio and compute the ratio before the function call ?

def pre_mitigation_auto_attack_damage(
    base_offensive_stats: float,
    bonus_offensive_stats: float,
    damage_modifier_flat: float,
    damage_modifier_percent: float,
    crit: bool = False,
    crit_damage: float = 0,
):
    """
    Calculates the pre-mitigation damage of a spell or an autoattack
    All values regarding damage modifiers should include the buffs/debuffs coming from spells, summoner spells, or items
    from both the attacker AND the defender
    """
    tot_off_stats = base_offensive_stats + bonus_offensive_stats
    dmg_mod_ratio = 1 + damage_modifier_percent

    if crit:
        crit_multiplier = 1.75 + crit_damage
    else:
        crit_multiplier = 1

    return (tot_off_stats * crit_multiplier + damage_modifier_flat) * dmg_mod_ratio
Lysxia commented 1 year ago

Yeah I would do that. Push all of the 1 + or / 100 operations to the edges of the application (mainly in parsing data), and then we are left with coefficients that just have to be multiplied. (also _coeff seems a slightly better name than _ratio, I'm not sure).

PaulQbFeng commented 1 year ago

for damage_modifier, it's a no brainer. for armor_pen_percent I am a bit reluctant to convert it to armor_pen_coeff at the data parsing time. Maybe keep both, or have the _coeff attributes wrapped into a property ?

PaulQbFeng commented 1 year ago

coeff can be nice as we ratio is already used for spell damage ratio

Lysxia commented 1 year ago

a property would be fine