Angband Forums

Angband Forums (http://angband.oook.cz/forum/index.php)
-   Development (http://angband.oook.cz/forum/forumdisplay.php?f=10)
-   -   r1676 - Random Ability Bug on Ego (http://angband.oook.cz/forum/showthread.php?t=2307)

Sirridan September 22, 2009 05:15

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...

PowerDiver September 22, 2009 06:58

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.

Sirridan September 22, 2009 07:29

Quote:

Originally Posted by PowerDiver (Post 24161)
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 September 22, 2009 18:48

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?

zaimoni September 22, 2009 20:13

Quote:

Originally Posted by Sirridan (Post 24175)
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.

Sirridan September 22, 2009 20:34

Quote:

Originally Posted by zaimoni (Post 24177)
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.

Magnate September 23, 2009 13:09

Now trac'd as http://trac.rephial.org/ticket/969 - thanks for the report.

d_m September 23, 2009 15:32

Should be fixed in r1678. Thanks for finding the bug.

zaimoni September 23, 2009 18:18

Quote:

Originally Posted by zaimoni (Post 24177)
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)

Sirridan September 24, 2009 01:50

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?


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

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