Bug in item probabilities [angband-v4-v4-1107-g15f2bcb]

[kaypy]

Heres a quick-fix artifact.txt

I've refrained from doing anything too wacky- just split the A lines into bytes, trying for numbers that retain some semblance of readability.
The spellbooks are at the 3x255 limit.

artifact.zip

[fizzix]

Ouch. Ouch. This has been hidden for nearly a year, ever since v4 was released. Hard to believe I missed it - except that its effects are quite subtle and it doesn't trigger any obvious errors (compiler warnings, tests etc.). Interesting.

Thanks very much for the report.

Honestly, I should have spotted it when I was playing around with the allocation frequencies and running stats on the spellbooks!

I just thought we were hitting a hard limiting factor.

[Magnate]

Pretty much. And I don't even want to think about what is happening in randarts...


But still makes them 4 times as hard to find as expected- about the same as a commonish artifact, rather than an expected find.

Ouch. Ouch. This has been hidden for nearly a year, ever since v4 was released. Hard to believe I missed it - except that its effects are quite subtle and it doesn't trigger any obvious errors (compiler warnings, tests etc.). Interesting.

Thanks very much for the report.

[kaypy]

It sounds like the effects of stuffing things into bytes is that items that we tried to make very common (allocation probability well over 255) would have effectively random (prob % 255) probabilities instead. Is that accurate?

Pretty much. And I don't even want to think about what is happening in randarts...

Though the books' current allocations of 1000 give them an effective allocation probability of 235, which isn't much less than the actual maximum.

But still makes them 4 times as hard to find as expected- about the same as a commonish artifact, rather than an expected find.

[debo]

It sounds like the effects of stuffing things into bytes is that items that we tried to make very common (allocation probability well over 255) would have effectively random (prob % 255) probabilities instead. Is that accurate? Though the books' current allocations of 1000 give them an effective allocation probability of 235, which isn't much less than the actual maximum.

Still, we really ought to stop using bytes everywhere.

Byte your tongue

[Derakon]

It sounds like the effects of stuffing things into bytes is that items that we tried to make very common (allocation probability well over 255) would have effectively random (prob % 255) probabilities instead. Is that accurate? Though the books' current allocations of 1000 give them an effective allocation probability of 235, which isn't much less than the actual maximum.

Still, we really ought to stop using bytes everywhere.

[kaypy]

OK, bad form replying to myself, but I've been doing some digging and want to keep it separate from the bug report itself.

It looks like there is nothing in the allocation tables that forces you to have the the allocations non-overlapping.

So you could represent an allocation of say

A:1000:40 to 100

as

A:250:40 to 100
A:250:40 to 100
A:250:40 to 100
A:250:40 to 100

or maybe make things a bit more interesting 8-)

A:250:35 to 100
A:250:40 to 100
A:250:45 to 100
A:250:50 to 100

It would also be possible to tweak parse_a_a to automatically break the input into as many byte size chunks as needed.

edit: whoops, limited to 3 entries at the moment

[kaypy]

In object.h/line~379, the artifact allocation probability is a byte.

A large number of artifacts have probabilities over 255, which are then stuffed into that byte in parse_a_a @ init2.c/779

This is going to be causing wackiness with the allocations.

byte level; /** Difficulty level for activation */
byte rarity; /** Unused */
byte alloc_prob[ART_ALLOC_MAX]; /** Chance of being generated */
byte alloc_min[ART_ALLOC_MAX]; /** Minimum depth (can appear earlier) */
byte alloc_max[ART_ALLOC_MAX]; /** Maximum depth (will NEVER appear deeper) */

if (!a->alloc_prob[i]) {
a->alloc_prob[i] = parser_getint(p, "common");
a->alloc_min[i] = amin;
a->alloc_max[i] = amax;
break;

If the artifact.alloc_prob got tweaked, we would then still have issues with alloc_entry.prob3 @ types.h/l156

byte level; /* Base dungeon level */
byte prob1; /* Probability, pass 1 */
byte prob2; /* Probability, pass 2 */
byte prob3; /* Probability, pass 3 */

/* Looks good - add this artifact to the table */
table[entry].index = a_ptr->aidx;
table[entry++].prob3 = a_ptr->alloc_prob[j];
total += a_ptr->alloc_prob[j];