LandSandBoat / server

:sailboat: LandSandBoat - a server emulator for Final Fantasy XI
https://landsandboat.github.io/server/
GNU General Public License v3.0
301 stars 612 forks source link

🐛 BRD March effect is not being properly calculated by spells and attacks for cd/delay reduction #2564

Open SirGouki opened 2 years ago

SirGouki commented 2 years ago

Branch affected by issue

base

Steps to reproduce

I used 75 BRD/37 NIN to test spells (Utsusemi Ichi) and 75 WAR/37 BRD with all skills capped to trigger. Equipment for Spells: Gjallarhorn (no other bonuses)

Casted Utsusemi Ichi with no haste effect, got 30 seconds CD each time (3 times) Repeated the test with marches: Utsusemi: Ichi - BRD75/NIN37 - Advancing March - 29 seconds Utsusemi: Ichi - BRD75/NIN37 - Victory March - 29 seconds Utsusemi: Ichi - BRD75/NIN37 - Both - 29 seconds

For comparison, I also tested with 75 RDM / 37 NIN: Ustsuemi: Ichi - RDM75/NIN37- Haste - 23 seconds Utsusemi: Ichi - RDM75/NIN37- no haste - 27 seconds

For melee I used the Chaosbringer (delay:666, approx 11 seconds per swing. Note: Chat window timestamps were used to measure delay) 75 WAR / 37 BRD - no haste effects - ~11 seconds per swing 75 WAR / 37 BRD - Advancing March - ~11 seconds per swing 99 WAR / 49 RDM (level diff for haste) - Haste - ~8 seconds

Expected behavior

Marches should reduce the delay between melee attacks. Per BGwiki (https://www.bg-wiki.com/ffxi/Category:March) Advancing March should be at 10.55% potency, and Victory March should be at 15.92% potency when skill capped (though the skill caps for these 2 need more info/verification). This means around a 3-4.5 (I don't know if both marches = 26.47% haste or something else) second reduction in cd for Utsusemi Ichi, depending on the march effect(s), and around 1-1.5 second reduction between swings for Chaosbringer, depending on the march effect(s), as opposed to what the testing I did returned.

kaincenteno commented 2 years ago

oh man thanks for the report @SirGouki i never noticed and my favourite job is BRD

CatsEyeXI commented 2 years ago

confirmed broken here too

WinterSolstice8 commented 2 years ago

2601 does not fix this, but it mitigates it. the haste values out of March are wrong, to the point where capped skill Advancing March is actually slightly better than Victory (with no March+)

I can't find data on how the skill bonus scales so I can't fix this yet, so this must stay open.

CatsEyeXI commented 2 years ago

2601 does not fix this, but it mitigates it. the haste values out of March are wrong, to the point where capped skill Advancing March is actually slightly better than Victory (with no March+)

I can't find data on how the skill bonus scales so I can't fix this yet, so this must stay open.

Thanks for looking at this

SirGouki commented 2 years ago

Got multiple reports from Catseye that RUN Inspiration is behaving the same way (its fast cast is not being calculated into spell recast reduction). I'll be testing this on my LAN. If it is the same thing I'll add it to the OP.

Before Inspiration: Reraise cooldown before

After Inspiration: Reraise cooldown after

WinterSolstice8 commented 2 years ago

Oops. Looks like I forgot to actually add the inspiration check into CalculateSpellRecastTime. my bad. I'll get a fix out.

p.s. it looks like Fast Cast was incorrectly capping too low as well

TeoTwawki commented 2 years ago

please see every past argument about 256ths and 1024ths before ever mentioning bg's pages on the subject ever again. please? PLEASE! dies

kidding sorta. It hurts tho how much this seems up. Mistakes have been made. Not using the same fractions isn't one of them tho, but thanks to BG explanations most ppl think it is. cries in floating points

WinterSolstice8 commented 2 years ago

I audited the recast function #2694

SirGouki commented 2 years ago

please see every past argument about 256ths and 1024ths before ever mentioning bg's pages on the subject ever again. please? PLEASE! dies

kidding sorta. It hurts tho how much this seems up. Mistakes have been made. Not using the same fractions isn't one of them tho, but thanks to BG explanations most ppl think it is. cries in floating points

Sorry Teo.

SirGouki commented 2 years ago

Was this fixed with #2694 ?? Or is it still not working correctly?