Angband Forums

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.

Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2019, vBulletin Solutions Inc.