rathena
rathena copied to clipboard
Add a "Direct Damage" skill flag for skill damage bonuses to YML
Is your request related to a missing official feature?
No
Describe the solution you'd like
From tests I did in various previous issues, see: https://github.com/rathena/rathena/issues/8218 https://github.com/rathena/rathena/issues/8211 https://github.com/rathena/rathena/issues/8193 https://github.com/rathena/rathena/issues/8187
I found that a lot of bonuses apply to the exactly same skills. Usually if one bonus doesn't apply to a skill, all related bonuses will also not apply to the skill.
1. "Direct Damage" skills
ATKPercent, Refine, Passive Masteries, Spirit Spheres, Star Crumbs and Skill-specific mastery damage all pretty much belong together. "Direct Damage" skills do not consider these unless it's manually coded into their skill code.
I already bundled a lot of the "if(skill_id != [skill_id])" checks into the single function "battle_skill_stacks_masteries_vvs" to clean it all up. The list is not complete yet, but all the skills in the list have been verified to be "Direct Damage" skills. In particular, previous bug reports I went through suggest that many NPC_ skills also should not consider these bonuses, but it's hard to test them on official servers as often only strong MVPs use those.
Once the skill list is pretty extensive and we are sure there are not too many hard-coded exceptions, someone could replace the whole function with an option in the skill.yml file.
2. "Ignore EDP/Elemental" skills
~The EDP damage increase and the elemental bonus damage you get from EDP and Magnum Break always apply to the exact same skills, not even a single exception could be found (though the list is slightly different in renewal).~
~Similar to above I created a function battle_skill_stacks_edp_element that lists up the skills that these bonuses do not apply to. Everything that is listed there was tested for pre-renewal, but there still might be more.~
~Note: The list should probably also apply for renewal, but someone would need to run in-depth tests. The status change "SC_SUB_WEAPONPROPERTY" is actually the same as "WATK_ELEMENT", we just use two different SCs to differentiate pre-renewal and renewal behavior right now. If it turns renewal works similar, we could rename WATK_ELEMENT to SC_SUB_WEAPONPROPERTY in battle.cpp and then grant this SC on EDP and Magnum Break even in pre-renewal.~
~Once unified, this function could also be turned into a flag in the skill.yml file.~
We found that this is not a separate flag, but is actually identical to the "IgnoreAtkCard" flag that we already have, see: https://github.com/rathena/rathena/issues/8250
Additional implementation is no longer needed.
Describe alternatives you've considered
We could also just leave the functions in the source code. It will work just as well, but requires code changes to customize.
Additional context
No response