Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a "Direct Damage" skill flag for skill damage bonuses to YML #8226

Open
Playtester opened this issue Apr 5, 2024 · 0 comments
Open

Add a "Direct Damage" skill flag for skill damage bonuses to YML #8226

Playtester opened this issue Apr 5, 2024 · 0 comments
Labels
mode:prerenewal A fault that exists within the pre-renewal mode mode:renewal A fault that exists within the renewal mode status:need more info Issue that needs more information from a creditable source type:enhancement Issue that is an enhancement to rAthena

Comments

@Playtester
Copy link
Member

Playtester commented Apr 5, 2024

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:
#8218
#8211
#8193
#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: #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

@Playtester Playtester added mode:renewal A fault that exists within the renewal mode mode:prerenewal A fault that exists within the pre-renewal mode status:need more info Issue that needs more information from a creditable source type:enhancement Issue that is an enhancement to rAthena labels Apr 5, 2024
@Playtester Playtester changed the title Rework / Add some skill flags to YML Add some skill flags for skill damage bonuses to YML Apr 5, 2024
@Playtester Playtester changed the title Add some skill flags for skill damage bonuses to YML Add a "Direct Damage" skill flag for skill damage bonuses to YML Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mode:prerenewal A fault that exists within the pre-renewal mode mode:renewal A fault that exists within the renewal mode status:need more info Issue that needs more information from a creditable source type:enhancement Issue that is an enhancement to rAthena
Projects
None yet
Development

No branches or pull requests

1 participant