Added multi-hit critical damage display support

[secretdataz]

• Addressed Issue(s): #1788

• Server Mode: Renewal (skill part), Both (clif part)

• Description of Pull Request:

  • Double Attack and Fear Breeze can now critically strike.
  • Critical damage from those skills will now displayed as critical damage properly (Client date 20161207 or later).
  • The code is hacky and need some tests.

• To-do:

  • Test interaction with endure'd enemies
  • Test interaction with MVPs
  • Check for position desyncs caused by the new damage type (especially with the two types of enemies above)

[zackdreaver]

iirc, the client date should be around dec 2016, not 2017.

[secretdataz]

@zackdreaver Oh, yeah. I got the year off by one year. Thanks!

[BrOgBr]

It seems to work. xD

Critical:

Normal:

[yashimwong]

using your code, auto-counter does not display the crit animation anymore. several changes need to be put. maybe add:

if(skill_id == KN_AUTOCOUNTER)
return true;

at the is_attack_critical line? im not sure if it's just me though

[iubantot]

im testing this PR, and i tried using snake hat + fear breeze result is the fear breeze is not effect is i think is canceled cause its only double attacks showing.

[talesofragnarok]

i try this but as i notice critical working 100% but the double attack are gone

[Badarosk0]

I tested it with all the skills and it seems to work fine. But I found something.

Not working with Raging Trifecta Blow.

[idamonli]

@BrOgBr
What skills are used?

[idamonli]

Active attack skills don't seem to support them.

[secretdataz]

According to my test results on kRO, triple attack’s change is likely to be reverted or not implemented in the first place.

[BrOgBr]

@idamonli I didn't understand you. oO
In the SS, I was using Double Attack and it's working.

[erenaryana]

excuse me. sorry but where do i get a client date 2017 as mentioned ? i wanted to test this on the offline rathena

[cydh]

in skill.hpp, member of struct s_skill_db
please change the uint8 nk; to uint16 nk; at least.

[secretdataz]

@cydh Good catch. Thanks

[mazvi]

How about this update?

[Atemo]

See the to-do list above ^

[jezztify]

Tested this PR.

• Test interaction with endure'd enemies
• Test interaction with MVPs
  extra:
• Test interaction in PVP: Doesn't work

[secretdataz]

@jezznar Thanks for testing. Do you feel any position desync when critting on endured enemies or MVPs?

[Lemongrass3110]

Shouldnt this be inside an if to avoid multiple calculations for ither types?

[BrOgBr]

Against MVPs I've already tested it and confirmed that it was working xD

[Atemo]

Yeha the system displays the crit on mvp in this PR. But previously crit + high aspd could 'hit lock' the mob, in vulgar term. The question is: the multi hit crit can 'hit lock' a mvp or a mob under endure on official? The answer will probably be subjective

[redlightliu]

Update those source will let Fist Spell(SO_SPELLFIST) can be crit.
Fist Spell should not crit.

[jezztify]

@secretdataz what do you mean about positional desync? I'm not quite sure about what you mean but now that you mentioned something about positions, I still get to attack a monster when it pushes me off. somehow, the attack became ranged? here's a video. https://www.youtube.com/watch?v=SgaFJAupveI

by the way, I merged my copy with the current master branch.
client: 20180308

[AsurielRO]

is it normal that when you use a katar the critical wont show and if you use a dagger critical always showing?

[luan122]

is it normal that when you use a katar the critical wont show and if you use a dagger critical always showing?

is the damage different when you do critical and when you don't with katar?

[darhylism]

is this good to go?

[Miraakol]

maybe this one can help idathena/trunk@49f428a#diff-c861cb68eadb0408c3c20bb633904e69

[aleffmoura]

Its not working?

[secretdataz]

Make sure you are using exe, hexed, or whatever you call your client dated 2016-12-07 or later.
I will not help you if you don't state that you are using client version 2016-12-07 or newer.