[3.3] Potential crash with multiple pval system

[PowerWyrm]

While implementing the latest 3.3 features for my variant, one of my test runs crashed in which_pval() with an assertion failure.

Tracing back the error in debug mode, I found out that the game crashed while trying to generate a specific item (ego):

- item kind (from object.txt):
N:578:& Pair~ of Witan Boots
G:]:b
I:30:7
W:60:0:80:5000
A:10:60 to 100
P:10:1d1:0:0:5
F:IGNORE_ACID | IGNORE_COLD | IGNORE_FIRE | IGNORE_ELEC
L:-2:STEALTH

- ego kind (from ego_item.txt):
N:57f Stealth
X:16:0
W:0:6
T:30:0:99
F:HIDE_TYPE
L:d3:0:STEALTH

Looking at the object_type fields, it seems that the game generated an ego with +2 to stealth, leading to the following inconsistencies:
- o_ptr->num_pvals = 1
- o_ptr->pval[0] = 0;
- o_ptr->flags contains OF_STEALTH

This explains the crash: trying to access the pval for "stealth" when there is actually none...

First problem: o_ptr->flags contains OF_STEALTH

In ego_apply_magic(), ego flags are added to object flags AFTER applying pvals:

	/* Apply pvals */
	for (i = 0; i < o_ptr->ego->num_pvals; i++) {
		of_copy(flags, o_ptr->ego->pval_flags[i]);
		x = randcalc(o_ptr->ego->pval[i], level, RANDOMISE);
		for (flag = of_next(flags, FLAG_START); flag != FLAG_END;
				flag = of_next(flags, flag + 1))
			object_add_pval(o_ptr, x, flag);
	}

	/* Apply flags */
	of_union(o_ptr->flags, o_ptr->ego->flags);

In object_add_pval(), if the resulting pval is zero, the flag is removed:

			if (o_ptr->pval[a] == 0) { /* Remove the flag */
				of_off(o_ptr->flags, flag);
				of_off(o_ptr->pval_flags[a], flag);
			}

Fix: don't add flags that are removed from o_ptr->ego->flags to o_ptr->flags again

Second problem: o_ptr->num_pvals = 1

In object_dedup_pvals(), pvals are moved down and num_pvals is decremented when pval is zero:

				/* Move any remaining pvals down one to fill the void */
				for (k = j + 1; k < o_ptr->num_pvals; k++) {
					of_copy(o_ptr->pval_flags[k - 1], o_ptr->pval_flags[k]);
					of_wipe(o_ptr->pval_flags[k]);
					o_ptr->pval[k - 1] = o_ptr->pval[k];
					o_ptr->pval[k] = 0;
				}
				/* We now have one fewer pval */
				o_ptr->num_pvals--;

Fix: apply the same code in object_add_pval() when pval = 0

[PowerWyrm]

Note that this is currently irrelevant for V3.3 as there are no objects with negative pvals that can become egos (so the bug is "hidden"). But this could happen in the future if such items are added in the game...

[dos350]

vouch this, legit~

[Magnate]

Note that this is currently irrelevant for V3.3 as there are no objects with negative pvals that can become egos (so the bug is "hidden"). But this could happen in the future if such items are added in the game...

Thank you - that's very helpful. It's especially helpful when the person who finds the bug also offers a solution!

Tracked as #1531 - this will be fixed in 3.3.1

[PowerWyrm]

Another crash related to the same problem...

I fixed the first two problems I mentioned in the previous post by modifying ego_apply_magic() and object_add_pval(). Now the "Pair of Witan Boots of Stealth" with zero pval is correctly generated, but (I)nspecting the object crashes with another assertion failed in which_pval().

In object_pval_flags_known(), a set of flags is constructed by taking the set of pval flags and making an intersection with the known object flags. This set will not contain the "phantom" pval flag. However, when flavor is aware or ego is "easy known", the set is expanded with the corresponding base item/ego item flags. These two sets contain the "phantom" pval flag.

	/* Kind and ego pval_flags may have shifted pvals so we iterate */
	if (object_flavor_is_aware(o_ptr))
	    for (i = 0; i < MAX_PVALS; i++)
			for (flag = of_next(o_ptr->kind->pval_flags[i], FLAG_START);
					flag != FLAG_END; flag = of_next(o_ptr->kind->pval_flags[i],
					flag + 1))
				of_on(flags[which_pval(o_ptr, flag)], flag);

	if (o_ptr->ego && easy_know(o_ptr))
	    for (i = 0; i < MAX_PVALS; i++)
			for (flag = of_next(o_ptr->ego->pval_flags[i], FLAG_START);
					flag != FLAG_END; flag = of_next(o_ptr->ego->pval_flags[i],
					flag + 1))
				of_on(flags[which_pval(o_ptr, flag)], flag);

Fix: check that the flag exists in the flag set before calling which_pval()

[PowerWyrm]

PAR TAU CETI!!!

I'm still getting an assert with Witan Boots of Stealth when the item is not the first pair of Witan Boots generated, this time in object_power()...

In object_flags_known(), the following is done:
- retrieve the object flags
- intersect with the set of known flags
- if the flavor is "aware", add the object kind flags
- if the item is an "easy known" ego, add the ego flags

For egos with "phantom" pvals, the object flags/known flags will be empty... but the resulting flag set will contain the phantom flags from the kind/ego flag sets.

I guess this has to be fixed by adding flags from the kind/ego flag sets only if the flags are present in the object flag set. A simple of_inter(flags, o_ptr->flags) should do the trick...

[Magnate]

Ok, I think I have addressed all these in commit fbd295f at http://github.com/magnate/angband/tree/affixes. You should be able to cherry-pick this commit cleanly. Please let me know if any of the problems you describe in this thread are still extant afterwards. Thanks for the report.