Angband Forums (http://angband.oook.cz/forum/index.php)
-   Development (http://angband.oook.cz/forum/forumdisplay.php?f=10)
-   -   Fix for lack of damage boost from device skill in Angband 4.05 (http://angband.oook.cz/forum/showthread.php?t=8875)

 Gordon April 3, 2018 18:42

Fix for lack of damage boost from device skill in Angband 4.05

Since I received no response to my query about fixing this, I decided to fix it myself. The fix was easy. Just replace:
Code:

```int effect_calculate_value(effect_handler_context_t *context, bool use_boost) {         int final = 0;         if (context->value.base > 0 ||                 (context->value.dice > 0 && context->value.sides > 0))                 final = context->value.base +                         damroll(context->value.dice, context->value.sides);         if (use_boost)                 final *= (100 + context->boost) / 100;         return final; }```
with:
Code:

```int effect_calculate_value(effect_handler_context_t *context, bool use_boost) {         int final = 0;         if (context->value.base > 0 ||                 (context->value.dice > 0 && context->value.sides > 0))                 final = context->value.base +                         damroll(context->value.dice, context->value.sides);         if (use_boost)                 final *= (100 + context->boost);                 final /= 100;         return final; }```
in effects.c

 Derakon April 3, 2018 19:23

Interesting. It looks like the issue is with integer division effectively truncating the multiplier down to 1 unless boost was >= 100. Good catch!

This should be an equivalent single-line fix, right?

Code:

`final = (final * (100 + context->boost)) / 100`

 Gordon April 3, 2018 19:35

Yeah, that will work also. I usually prefer multiple lines when I'm doing integer math like this since I think it is clearer that way.

 Derakon April 3, 2018 19:43

Yeah, it's mostly just a style thing. I have a mild preference for one-liners when there's a possibility that otherwise the lines could get separated -- a single line has a clear "this is all one action" nature to it. But it really doesn't matter in this case.

In any case, thanks for finding the bug! :)

 Gordon April 3, 2018 19:53

Now my Wand of Annihilation is much more useful! :D

 AnonymousHero April 3, 2018 21:11

Quote:
 Originally Posted by Gordon (Post 128899) with: Code: ```int effect_calculate_value(effect_handler_context_t *context, bool use_boost) {         int final = 0;         if (context->value.base > 0 ||                 (context->value.dice > 0 && context->value.sides > 0))                 final = context->value.base +                         damroll(context->value.dice, context->value.sides);         if (use_boost)                 final *= (100 + context->boost);                 final /= 100;         return final; }``` in effects.c
I'm not sure what you indended with the code code but the indentation in the above lines is extremely misleading either way. The "final /= 100" bit will always apply, not just when "use_boost" is true. I would suggest always using braces.

(I believe recent versions of clang/gcc will even issue a warning for the above code because of the misleading indentation.)

 Gordon April 3, 2018 21:36

Whoops. Correction:
Code:

```int effect_calculate_value(effect_handler_context_t *context, bool use_boost) {         int final = 0;         if (context->value.base > 0 ||                 (context->value.dice > 0 && context->value.sides > 0))                 final = context->value.base +                         damroll(context->value.dice, context->value.sides);         if (use_boost) {                 final *= (100 + context->boost);                 final /= 100;         }         return final; }```
Thanks for noticing that!

 Pete Mack April 3, 2018 21:54

This bug was fixed in 4.1

 Gordon April 3, 2018 21:59

Yes, but 4.1 is basically a whole different game. For those who prefer a more traditional game, this should be fixed in 4.05 as well. It's a pretty nasty bug.

 Nick April 3, 2018 23:20

Quote:
 Originally Posted by Gordon (Post 128909) Yes, but 4.1 is basically a whole different game. For those who prefer a more traditional game, this should be fixed in 4.05 as well. It's a pretty nasty bug.
The tradition in Angband is to keep changing, so I regard 4.1 as more traditional than 4.0.5 :)

All times are GMT +1. The time now is 22:02.