Angband.oook.cz
Angband.oook.cz
AboutVariantsLadderForumCompetitionComicScreenshotsFunniesLinks

Go Back   Angband Forums > Angband > Development

Reply
 
Thread Tools Display Modes
Old September 22, 2009, 05:15   #1
Sirridan
Knight
 
Sirridan's Avatar
 
Join Date: May 2009
Posts: 560
Sirridan is on a distinguished road
r1676 - Random Ability Bug on Ego

When getting egos, (Blessed), Lothlorien, and Magi, I seem to only get slow metabolism as the random ability, and random resists on gear (Aman / Preservation) give rPoison. Not that I mind the rpoison, I always seem to lack it... but yeah.

2 aman, 1 preservation both had rPoison, and at least 30 random ability egos or more were all slow metabolism.

Unless this was found and fixed in a later version?

EDIT:

Checked out the code, seems that rPoison and Slow Digest are both the first element of their ego_xtra arrays... is anything wrong with get_new_attr?

Code:
u32b get_new_attr(u32b flags, const u32b attrs[])
{
	size_t i;
	int options = 0;
	u32b flag = 0;
	for (i = 0; i < N_ELEMENTS(attrs); i++)
	{
		/* skip this one if the flag is already present */
		if (flags & attrs[i]) continue;

		/* each time we find a new possible option, we have a 1-in-N chance of
		 * choosing it and an (N-1)-in-N chance of keeping a previous one */
		if (one_in_(++options)) flag = attrs[i];
	}
	return flag;
}
looking at this, it should be 1 in 1 for poison, then 1 in 2 to change it to the next, and so on... but it always chooses poison. The code segment looks fine to me, unless one_in_(x) was changed recently?

More edits:

Also noticed for random sustains, it always picks strength, which is also first in the sustain list...

EVEN MORE EDITS OMG: One more elvenkind with rPoison...

Last edited by Sirridan; September 22, 2009 at 06:34.
Sirridan is offline   Reply With Quote
Old September 22, 2009, 06:58   #2
PowerDiver
Prophet
 
Join Date: Mar 2008
Posts: 2,712
PowerDiver is on a distinguished road
I think the bug is with N_ELEMENTS. That should only be used on an array[size], not on *array, even though they appear to be the same type. I think the N_ELEMENTS is always returning 1, so it doesn't get past the first element.
PowerDiver is offline   Reply With Quote
Old September 22, 2009, 07:29   #3
Sirridan
Knight
 
Sirridan's Avatar
 
Join Date: May 2009
Posts: 560
Sirridan is on a distinguished road
Quote:
Originally Posted by PowerDiver View Post
I think the bug is with N_ELEMENTS. That should only be used on an array[size], not on *array, even though they appear to be the same type. I think the N_ELEMENTS is always returning 1, so it doesn't get past the first element.
Sounds like that would be it, because I just found another magi with slow dig... *sigh* maybe one of those elvenkinds would have had the resist holes my latest char has... oh well.

The nearly guaranteed rPoison was nice for a bit though
Sirridan is offline   Reply With Quote
Old September 22, 2009, 18:48   #4
Sirridan
Knight
 
Sirridan's Avatar
 
Join Date: May 2009
Posts: 560
Sirridan is on a distinguished road
Actually, I ran this code segment on the same basic array, and N_ELEMENTS was returning 12 for resists as it should...

Code:
#include <stdio.h>

#define N_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))

#define	        TR1_RES_POIS	0x1
#define	        TR1_RES_FEAR	0x2
#define	        TR1_RES_LITE	0x4
#define	        TR1_RES_DARK	0x8
#define	        TR1_RES_BLIND	0x10
#define	        TR1_RES_CONFU	0x20
#define	        TR1_RES_SOUND	0x40
#define	        TR1_RES_SHARD	0x80
#define	        TR1_RES_NEXUS	0x100
#define	        TR1_RES_NETHR	0x200
#define	        TR1_RES_CHAOS	0x400
#define	        TR1_RES_DISEN	0x800


void main()
{
	static const unsigned int ego_resists[] =
	{
	        TR1_RES_POIS,
	        TR1_RES_FEAR,
	        TR1_RES_LITE,
	        TR1_RES_DARK,
	        TR1_RES_BLIND,
	        TR1_RES_CONFU,
	        TR1_RES_SOUND,
	        TR1_RES_SHARD,
	        TR1_RES_NEXUS,
	        TR1_RES_NETHR,
	        TR1_RES_CHAOS,
	        TR1_RES_DISEN,
	};
	printf("%d\n",N_ELEMENTS(ego_resists));
	printf("done\n");
}
Any thoughts?
Sirridan is offline   Reply With Quote
Old September 22, 2009, 20:13   #5
zaimoni
Knight
 
zaimoni's Avatar
 
Join Date: Apr 2007
Posts: 590
zaimoni is on a distinguished road
Quote:
Originally Posted by Sirridan View Post
Actually, I ran this code segment on the same basic array, and N_ELEMENTS was returning 12 for resists as it should...
Which is completely irrelevant to the original bug.

Code:
#include <stdio.h>

#define N_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))

/* ... */

void main()
{
	static const unsigned int ego_resists[] =
	{
	        TR1_RES_POIS,
	        TR1_RES_FEAR,
	        TR1_RES_LITE,
	        TR1_RES_DARK,
	        TR1_RES_BLIND,
	        TR1_RES_CONFU,
	        TR1_RES_SOUND,
	        TR1_RES_SHARD,
	        TR1_RES_NEXUS,
	        TR1_RES_NETHR,
	        TR1_RES_CHAOS,
	        TR1_RES_DISEN,
	};
	printf("%d\n",N_ELEMENTS(ego_resists));
	printf("done\n");
}
The extent of the ego_resists array is calculated at compile time, and happens to be 12. While in the original code:
Code:
u32b get_new_attr(u32b flags, const u32b attrs[])
{
	size_t i;
	int options = 0;
	u32b flag = 0;
	for (i = 0; i < N_ELEMENTS(attrs); i++)
	{
		/* skip this one if the flag is already present */
		if (flags & attrs[i]) continue;

		/* each time we find a new possible option, we have a 1-in-N chance of
		 * choosing it and an (N-1)-in-N chance of keeping a previous one */
		if (one_in_(++options)) flag = attrs[i];
	}
	return flag;
}
The parameter const u32b attrs[] is decayed at compile time to const u32b *attrs . The language standards (C90/C99/C1X) require 1==N_ELEMENTS(attrs).

The most direct fix is
Code:
u32b get_new_attr(u32b flags, const u32b *attrs, u32b attrs_size)
{
	size_t i;
	int options = 0;
	u32b flag = 0;
	for (i = 0; i < attrs_size; i++)
	{
		/* skip this one if the flag is already present */
		if (flags & attrs[i]) continue;

		/* each time we find a new possible option, we have a 1-in-N chance of
		 * choosing it and an (N-1)-in-N chance of keeping a previous one */
		if (one_in_(++options)) flag = attrs[i];
	}
	return flag;
}
and use N_ELEMENTS on the array where it actually is an array.
__________________
Zaiband: end the "I shouldn't have survived that" experience. V3.0.6 fork on Hg.
Zaiband 3.0.10 ETA Mar. 7 2011 (Yes, schedule slipped. Latest testing indicates not enough assert() calls to allow release.)
Z.C++: pre-alpha C/C++ compiler system (usable preprocessor). Also on Hg. Z.C++ 0.0.10 ETA December 31 2011
zaimoni is offline   Reply With Quote
Old September 22, 2009, 20:34   #6
Sirridan
Knight
 
Sirridan's Avatar
 
Join Date: May 2009
Posts: 560
Sirridan is on a distinguished road
Quote:
Originally Posted by zaimoni View Post
Which is completely irrelevant to the original bug.

The extent of the ego_resists array is calculated at compile time, and happens to be 12. While in the original code:
Code:
u32b get_new_attr(u32b flags, const u32b attrs[])
{
	size_t i;
	int options = 0;
	u32b flag = 0;
	for (i = 0; i < N_ELEMENTS(attrs); i++)
	{
		/* skip this one if the flag is already present */
		if (flags & attrs[i]) continue;

		/* each time we find a new possible option, we have a 1-in-N chance of
		 * choosing it and an (N-1)-in-N chance of keeping a previous one */
		if (one_in_(++options)) flag = attrs[i];
	}
	return flag;
}
The parameter const u32b attrs[] is decayed at compile time to const u32b *attrs . The language standards (C90/C99/C1X) require 1==N_ELEMENTS(attrs).

The most direct fix is
Code:
u32b get_new_attr(u32b flags, const u32b *attrs, u32b attrs_size)
{
	size_t i;
	int options = 0;
	u32b flag = 0;
	for (i = 0; i < attrs_size; i++)
	{
		/* skip this one if the flag is already present */
		if (flags & attrs[i]) continue;

		/* each time we find a new possible option, we have a 1-in-N chance of
		 * choosing it and an (N-1)-in-N chance of keeping a previous one */
		if (one_in_(++options)) flag = attrs[i];
	}
	return flag;
}
and use N_ELEMENTS on the array where it actually is an array.
Ah forgive my ignorance, makes sense, now I know more of what PowerDiver was saying as well.
Sirridan is offline   Reply With Quote
Old September 23, 2009, 13:09   #7
Magnate
Angband Devteam member
 
Join Date: May 2007
Location: London, UK
Posts: 5,057
Magnate is on a distinguished road
Send a message via MSN to Magnate Send a message via Yahoo to Magnate
Now trac'd as http://trac.rephial.org/ticket/969 - thanks for the report.
Magnate is offline   Reply With Quote
Old September 23, 2009, 15:32   #8
d_m
Angband Devteam member
 
d_m's Avatar
 
Join Date: Aug 2008
Location: Philadelphia, PA, USA
Age: 38
Posts: 1,516
d_m is on a distinguished road
Should be fixed in r1678. Thanks for finding the bug.
__________________
linux->xterm->screen->pmacs
d_m is offline   Reply With Quote
Old September 23, 2009, 18:18   #9
zaimoni
Knight
 
zaimoni's Avatar
 
Join Date: Apr 2007
Posts: 590
zaimoni is on a distinguished road
Quote:
Originally Posted by zaimoni View Post
The parameter const u32b attrs[] is decayed at compile time to const u32b *attrs . The language standards (C90/C99/C1X) require 1==N_ELEMENTS(attrs).
Decay happens, but the equality is wrong. The result does have nothing to do with the original size of the static array, but is rather likely to be a small integer as the actual expression ends up being equal to sizeof(u32b*)/sizeof(u32b).

The most likely values on real platforms (8-bit bytes) would be:
0: 64-bit pointers (e.g., native Win64 build)
1: 32-bit pointers (e.g., native Win32 build)
2: 16-bit pointers (if non-extended DOS build were resurrected)
__________________
Zaiband: end the "I shouldn't have survived that" experience. V3.0.6 fork on Hg.
Zaiband 3.0.10 ETA Mar. 7 2011 (Yes, schedule slipped. Latest testing indicates not enough assert() calls to allow release.)
Z.C++: pre-alpha C/C++ compiler system (usable preprocessor). Also on Hg. Z.C++ 0.0.10 ETA December 31 2011
zaimoni is offline   Reply With Quote
Old September 24, 2009, 01:50   #10
Sirridan
Knight
 
Sirridan's Avatar
 
Join Date: May 2009
Posts: 560
Sirridan is on a distinguished road
Is it cheating to play a character for a bit until I find an elvenkind or Aman to guarantee early rPoison, then switch over to the new version?
Sirridan is offline   Reply With Quote
Reply


Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump

Similar Threads
Thread Thread Starter Forum Replies Last Post
Ego weapons you always wanted to find... Pete Mack AAR 5 July 5, 2009 04:37
pricing on slay ego weapons Malak Darkhunter Vanilla 2 July 1, 2009 21:02
Ego-Item indexes in defines.h PaulBlay Development 0 April 17, 2009 09:21
3.1 bug: ego armor squelch quality ChodTheWacko Vanilla 1 April 11, 2009 19:19
(Ego) weapons should not be immediately destroyed by acid Pete Mack Vanilla 0 April 26, 2007 18:34


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


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