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

[Gordon]

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

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:

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]

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?

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

[Gordon]

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]

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]

Now my Wand of Annihilation is much more useful!

[AnonymousHero]

with:

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]

Whoops. Correction:

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]

This bug was fixed in 4.1

[Gordon]

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]

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

[Derakon]

The tradition in Angband is to keep changing, so I regard 4.1 as more traditional than 4.0.5

By that logic, the most traditional Angband is probably something like Celeste or The Witness or some other game in a completely different genre.

[Gordon]

I meant traditional in an historical sense there. Shall we say more retro then? Anyway, I believe that 4.0.5 is a major breakpoint in the evolution of the game and this bug breaks the entire device skill feature of this version. It's not exactly a minor bug.

[Pete Mack]

3.2 was by far a bigger change. And while a number of the features introduced there have been roload back or toned down, not all of them have. 3.0 is significantly more difficult transfer versions.

[Gordon]

Yes, I still play 3.1 occasionally. Always as a Ranger for the awesome bow damage and also for the challenge of giant packs of Zephyr hounds.

[Philip]

It doesn't actually break the entire device skill feature. In 4.0.5, as in previous versions of Angband, device skill affects your probability of activating a device. There is an intended, and in 4.0.5, non-functional change that does what your fix does, and the intended effect is apparent from device inspection.

Note: if you love giant packs of Zephyr hounds, you'll love Oangband (yes I am still on about this and it is the current comp so there).