Possible off-by-one-error in PR_STATE

[pampl]

I didn't want to post this on the bug tracker yet because I'm a newbie programmer so I'm not sure whether this is my fault or a quirk in Angband. Anyway, in xtra3.c there's a function which draws in the player's state at the bottom of the screen. I modified it so that they player will always display as blessed and heroic when a certain condition is true (when they've killed the "standard bearer" monster for their species):

static size_t prt_tmd(int row, int col)
{
	size_t i, len = 0;

	for (i = 0; i < N_ELEMENTS(effects); i++)
	{
		/* If the player is the standard bearer, always display as blessed and heroic */
		if (p_ptr->timed[effects[i].value] || (((i == TMD_BLESSED) || (i == TMD_HERO)) && (p_ptr->standard_bearer)))
		{
			c_put_str(effects[i].attr, effects[i].str, row, col + len);
			len += effects[i].len;
		}
	}

	return len;
}

TMD_BLESSED works like it's supposed to but TMD_HERO displays Berserk.. one higher on the enumeration list. That off-by-oneness continues until TMD_STONESKIN, which doesn't display anything. I tried earlier values, like TMD_PROTEVIL, and they were off by two!

The thing is, the display seems to work fine in V, so I don't see how this bug could exist if it weren't for me introducing it. OTOH, I don't see how I could have introduced it.

[fizzix]

wait, you want it to print out hero or blessed even though the player is not heroic or blessed?

Wouldn't it make more sense to set Hero and Blessed to true and not touch the display. Probably won't affect the off-by-one problem if it exists.

To test if there's actually an off by one, make a new character an drink a !hero. If there's a problem, you should not see Hero displayed. If there is an error, you've somehow introduced a bug somewhere (this bug is not in V).

[pampl]

OK, I think I understand what the 'problem' is. The state_info effects array isn't actually in the same order as the enumeration of timers, it's just a happy coincidence that TMD_BLESSED lines up between the two. TMD_STONESKIN is the 27th timed effect, which means it's bigger than the max size of state_info effects so it's never reached (and therefore never displayed). So not a bug at all, just a mistaken assumption on my part.

wait, you want it to print out hero or blessed even though the player is not heroic or blessed?

Wouldn't it make more sense to set Hero and Blessed to true and not touch the display. Probably won't affect the off-by-one problem if it exists.

I originally was going to do it that way but they're timers so setting them to true just sets them to one turn. There's sanity checking for the duration and stuff so I didn't bother trying to set the duration to a hundred million turns or anything like that, I just created a value that's tested for everywhere heroism and blessing are tested for. So the player is effectively heroic and blessed, it's just not represented internally via the hero and bless timers. Not sure if it's the best way, but it seems to work.

[camlost]

OK, I think I understand what the 'problem' is. The state_info effects array isn't actually in the same order as the enumeration of timers, it's just a happy coincidence that TMD_BLESSED lines up between the two. TMD_STONESKIN is the 27th timed effect, which means it's bigger than the max size of state_info effects so it's never reached (and therefore never displayed). So not a bug at all, just a mistaken assumption on my part.


I originally was going to do it that way but they're timers so setting them to true just sets them to one turn. There's sanity checking for the duration and stuff so I didn't bother trying to set the duration to a hundred million turns or anything like that, I just created a value that's tested for everywhere heroism and blessing are tested for. So the player is effectively heroic and blessed, it's just not represented internally via the hero and bless timers. Not sure if it's the best way, but it seems to work.

Those functions are called every time the duration changes, including every turn when the decrement. Just add an if/return statement at the beginning of the function to prevent decrementation if you don't want it reduced. It's ugly, but fast and better than some of the other options.

[Derakon]

Make certain the timers are signed ints, then set them to -1 to indicate "always on".