Issue Information
-
#008450
-
0 - None Assigned
-
New
Issue Confirmations
-
Yes (0)No (0)
1
Masao has brought it to my attention that if you use Frost Driver with Autospell, the damage output would be significantly high than normal.
in battle.c line 5897:
Should be:
in battle.c line 5897:
if (sc->data[SC_SOULLINK] && sc->data[SC_SOULLINK]->val2 == SL_SAGE) i = 0; //Max chance, no skill_lv reduction. [Skotlex] if (i >= 50) skill_lv -= 2; else if (i >= 15) skill_lv--; if (skill_lv < 1) skill_lv = 1; sp = skill->get_sp(skill_id,skill_lv) * 2 / 3;
Should be:
if (sc->data[SC_SPIRIT] && sc->data[SC_SPIRIT]->val2 == SL_SAGE)
i = 0; //Max chance, no skill_lv reduction. [Skotlex]
//reduction only for skill_lv > 1
if (skill_lv > 1) {
if (i >= 50) skill_lv -= 2;
else if (i >= 15) skill_lv--;
}
I guess the removal of these two lines isn't intentional, right?
Wouldn't it be better to fix it by changing skill_lv to a proper, signed, type (i.e. int16 instead of uint16), so the underflow doesn't happen?
if (skill_lv < 1) skill_lv = 1; sp = skill->get_sp(skill_id,skill_lv) * 2 / 3;The fix seems flawed though, if skill_lv == 2, and i >= 50, you get a skill_lv of 0, which I don't think is what you want there, or is it?
Wouldn't it be better to fix it by changing skill_lv to a proper, signed, type (i.e. int16 instead of uint16), so the underflow doesn't happen?
Well if you make sure the level isn't dropped below 1 in the first place then you don't need that check.
Could easily be fixed by using "skill_lv /= 2;" instead of "skill_lv -= 2;". Then it doesn't drop to 0. 2 -> 1. 3 -> 1 (rounded down).
Could easily be fixed by using "skill_lv /= 2;" instead of "skill_lv -= 2;". Then it doesn't drop to 0. 2 -> 1. 3 -> 1 (rounded down).
How is /= 2 equivalent to -= 2 (assuming we want maintainable code that doesn't make assumptions on the input values)?
What's wrong with using signed data types, and avoiding the problem altogether?
What's wrong with using signed data types, and avoiding the problem altogether?
Well you can do that too, I'm always unsure why anybody started using unsigned types for anything really, because signed is usually always better in regards of possible overflows. I always think "if someone want through the trouble and define something unsigned, he must have known what he was doing", so I try to get around it like that.
/= 2 makes sure that 2 becomes 1 and 3 also becomes 1. What should happen with higher input values is not defined anyway. Half level or two levels lower both would be reasonable approaches.
/= 2 makes sure that 2 becomes 1 and 3 also becomes 1. What should happen with higher input values is not defined anyway. Half level or two levels lower both would be reasonable approaches.
No idea. I'm just passing info along~ So don't quote me on the source changes xD
