Multiple pvals

**Magnate** · April 18, 2011, 22:56

Originally posted by PowerWyrm

This is something I wanted to see implemented for a long time now... Nice to see it's gonna make it to 3.3...

I've checked the current source code, and something is bugging me though. It seems that the current implementation simply adds the pvals from base item and ego item (if the item is an ego).

For example: an Elven Cloak (+2 to stealth) can become "of the Magi" (+2 to int/stealth)

For what I've understood, this would become an Elven Cloak of the Magi (+4 to int/stealth). This is probably a bit overpowered...

A better item would be an Elven Cloak of the Magi <+2, +2> (+2 to int, +4 to stealth) where the ego pval is affected to the object's second pval slot, leaving the base item pval unaffected.

This would work for item/ego with single pval... No idea what to do with multiple pvals items/egos with the current system. For multiplayer Angband (MAngband), a new field "bpval" has been added on the object to keep track of the "base" pval given by the object kind. When an ego item is generated, the ego pval is affected to the object pval, leaving the base pval untouched. Of course, this would add another level of complexity...

Er, if you've described the current functionality correctly, then it's not what I intended. I intended it to work exactly as you suggested: that a base item with +2 of something, when combined with an ego template giving +2 of the same and one other thing, would end up with <+2, +4>, not +4 to both.

This is probably because I didn't pay sufficient attention to the generation of ego items - but since this is an area ripe for development at some point (see tickets #1080, #1179, #1394, #1395), I'm confident it will get fixed eventually.

**PowerWyrm** · April 19, 2011, 12:46

Here's the current code from obj_make:

/* Apply pvals */
for (i = 0; i < o_ptr->ego->num_pvals; i++)
{
if (!o_ptr->pval[i]) o_ptr->num_pvals++;
o_ptr->pval[i] += randcalc(o_ptr->ego->pval[i], level, RANDOMISE);
}

So base and ego pvals are added.

In object_pval_flags(), the flags on which the pvals are applied are ORed using of_union(). Let's examine the Elven Cloak of the Magi example again, using the latest txt files...

Base item: an Elven Cloak (L:2:STEALTH, L:1:SPEED)
Ego item: of the Magi (L:d2:1:INT, L:d3:1:STEALTH)

In object_prep(), the base pvals are assigned to the object pvals:
- pval 1: +2 to stealth
- pval 2: +1 to speed

In ego_apply_magic(), the ego pvals are assigned and added to the object pvals:
- pval 1: +1/+2 to int, making it +3/+4 to stealth/int
- pval 2: +1/+3 to stealth, making it +2/+4 to speed/stealth

The result:
an Elven Cloak of the Magi <+3/+4,+2/+4>
- +3/+4 to int
- +2/+4 to speed
- +5/+8 to stealth

If I'm correct, this is a hell of an ego item!

Sounds like a check must me made on the pval_flags when assigning the ego pvals to the object pvals:
- loop on ego pvals
- for each ego pval, get pval_flags
- for each pval_flag, check if it's already present on the object
- if yes, add the pval to the corresponding object pval
- if no, increase the number of object pvals and assign the pval to the new object pval

Some change must probably be made in object_pval_flags() to reflect this...

This means that object pvals must be an array of 2*MAX_PVALS, in case base item and ego item have a completely separate set of pval_flags...

**PowerDiver** · April 19, 2011, 17:48

Don't confuse pvals with values for flags. You can have 7 different flag non-zero values with 3 pvals.

A different approach is to give most items pvals <+1, +2, +4>. Compute what values you want for all flags, however you want to do it. So long as they are between 0 and 7, express them the obvious way. In special cases you might want something else, but this works for every standard item I can think of except rings or boots of speed, which are easily done with a single pval.

**Magnate** · April 19, 2011, 22:28

Originally posted by PowerWyrm

Here's the current code from obj_make:

/* Apply pvals */
for (i = 0; i < o_ptr->ego->num_pvals; i++)
{
if (!o_ptr->pval[i]) o_ptr->num_pvals++;
o_ptr->pval[i] += randcalc(o_ptr->ego->pval[i], level, RANDOMISE);
}

So base and ego pvals are added.

In object_pval_flags(), the flags on which the pvals are applied are ORed using of_union(). Let's examine the Elven Cloak of the Magi example again, using the latest txt files...

Base item: an Elven Cloak (L:2:STEALTH, L:1:SPEED)
Ego item: of the Magi (L:d2:1:INT, L:d3:1:STEALTH)

In object_prep(), the base pvals are assigned to the object pvals:
- pval 1: +2 to stealth
- pval 2: +1 to speed

In ego_apply_magic(), the ego pvals are assigned and added to the object pvals:
- pval 1: +1/+2 to int, making it +3/+4 to stealth/int
- pval 2: +1/+3 to stealth, making it +2/+4 to speed/stealth

The result:
an Elven Cloak of the Magi <+3/+4,+2/+4>
- +3/+4 to int
- +2/+4 to speed
- +5/+8 to stealth

If I'm correct, this is a hell of an ego item!

Sounds like a check must me made on the pval_flags when assigning the ego pvals to the object pvals:
- loop on ego pvals
- for each ego pval, get pval_flags
- for each pval_flag, check if it's already present on the object
- if yes, add the pval to the corresponding object pval
- if no, increase the number of object pvals and assign the pval to the new object pval

Some change must probably be made in object_pval_flags() to reflect this...

This means that object pvals must be an array of 2*MAX_PVALS, in case base item and ego item have a completely separate set of pval_flags...

Thank you for this analysis - I've had a niggling feeling for some time that something wasn't quite right here, and you've illustrated it very helpfully.

I agree with most of your prognosis, and will certainly check for existing pval flags during ego creation. I've opened ticket #1404 for this.

But on your last point, MAX_PVALS means exactly that. So once an object has all its pvals active (nonzero), any new flags will have to be attached to the pval closest to the desired outcome.

Let's take your Elven Cloak of the Magi example again: our base cloak has +2 stealth, +1 speed. First we add +1d2 INT as a third pval, then we add some more stealth to the first pval. Now let's say that the Magi ego has been given a third pval, +1d2 WIS. We have no more pvals to create, so we roll 1d2 and pick a pval closest to the result, and add the WIS flag to it.

**Derakon** · April 19, 2011, 22:42

So we're currently limited to three pvals per item? What was the reasoning behind picking that particular number?

**Magnate** · April 19, 2011, 23:52

Originally posted by Derakon

So we're currently limited to three pvals per item? What was the reasoning behind picking that particular number?

It's what Sangband has, and it seems to work perfectly well - I couldn't see a need for any more.

We're not actually limited though: if you change MAX_PVALS to 4 and recompile (and then add some fourth pvals to objects/egos/artifacts), it should work perfectly well. At least, that was the design intention ...

**Derakon** · April 19, 2011, 23:53

Okay, fair enough. I generally don't see the need to use limits that we might conceivably reach at any time unless there are good technical reasons for those limits.

**Magnate** · April 20, 2011, 00:02

Originally posted by Derakon

Okay, fair enough. I generally don't see the need to use limits that we might conceivably reach at any time unless there are good technical reasons for those limits.

Agreed. In this case I think the limits are not technical and are instead (i) space on a line for describing an object (+x, +y) [a, +b] <+c, +d, -e, -f, etc.> and (ii) space in our brains for retaining the concepts of so many numbers. So my idea was that we could set MAX_PVALS at whatever we liked and see what happened. So far I've only done the most cursory tweaks to items to actually use 2nd and 3rd pvals - just what I could see would make sense immediately without unbalancing anything (e.g. separating DEX/MIGHT/SHOTS on Buckland slings). I am shortly going to go through the edit files with a fine tooth comb and do this properly for 3.3 - doubtless to howls of outrage from Timo, Eddie and Pete Mack. But I figured it would be worth doing that before deciding whether we needed more than three pvals.

Oooh, it's late and I'm tired. You're referring to my "max is max" comment, about why we can't just add extra pvals whenever an ego template requires a value that isn't used yet. We could try that and see how it goes. I'll try it when I fix #1404 and if it breaks before 3.3 we can revert to max=3.

**PowerDiver** · April 20, 2011, 00:09

Originally posted by Magnate

Agreed. In this case I think the limits are not technical and are instead (i) space on a line for describing an object (+x, +y) [a, +b] <+c, +d, -e, -f, etc.>

That's an entirely different problem, in that the pvals shouldn't simply be listed. Much better, if you want to add wis to magi, to describe as

an elven cloak of magi () Int+2 Wis+3 Stl+4 Spd+1
and this is trivial to accomplish with 3 pvals in the current data structures.

**PowerWyrm** · April 20, 2011, 13:31

Ok I resume...

Base item: an Elven Cloak (L:2:STEALTH, L:1:SPEED)
Ego item: of the Magi (L:d2:1:INT, L:d3:1:STEALTH)

First, the base bonuses are rolled (in this case, they are fixed values) and affected to the corresponding object pvals:
pval1 = 2 (STEALTH)
pval2 = 1 (SPEED)
num_pvals = 2

Then the ego bonuses are rolled.

If 1 is rolled for INT, the INT flag is added to pval2 which stays 1. If 2 is rolled for INT, the INT flag is added to pval1 which stays 2.

Now the complex part...

If 1 is rolled for STEALTH, and 1 was rolled for INT, since the STEALTH flag on pval1 is the only flag, pval1 can be increased to 3.
pval1 = 3 (STEALTH)
pval2 = 1 (SPEED/INT)
num_pvals = 2

If 1 is rolled for STEALTH, and 2 was rolled for INT, the STEALTH flag is added to pval2 which stays 1.
pval1 = 2 (STEALTH/INT)
pval2 = 1 (SPEED/STEALTH)
num_pvals = 2

If 2 is rolled for STEALTH, and 1 was rolled for INT, since the STEALTH flag on pval1 is the only flag, pval1 can be increased to 4.
pval1 = 4 (STEALTH)
pval2 = 1 (SPEED/INT)
num_pvals = 2

If 2 is rolled for STEALTH, and 2 was rolled for INT, we're in trouble... pval1 cannot be used since it's +2 to STEALTH/INT, and pval2 cannot be used since it's +1, so the best thing to do is probably to add the STEALTH flag to pval3 which becomes 2 and num_pval is set to 3
pval1 = 2 (STEALTH/INT)
pval2 = 1 (SPEED)
pval3 = 2 (STEALTH)
num_pvals = 3

If 3 is rolled for STEALTH, and 1 was rolled for INT, since the STEALTH flag on pval1 is the only flag, pval1 can be increased to 5.
pval1 = 5 (STEALTH)
pval2 = 1 (SPEED/INT)
num_pvals = 2

If 3 is rolled for STEALTH, and 2 was rolled for INT, the STEALTH flag is added to pval3 which becomes 3 and num_pval is set to 3.
pval1 = 2 (STEALTH/INT)
pval2 = 1 (SPEED)
pval3 = 3 (STEALTH)
num_pvals = 3

In this system, the same pval flag can be assigned to different pvals. When the item requires a new value, num_pvals is increased until it reaches MAX_PVALS. If MAX_PVALS is reached, the new value is changed to the closest pval available.

This means rewriting the object description to sum the pvals for identical flags instead of just collecting all flags for each pval.

**PowerDiver** · April 20, 2011, 17:59

Originally posted by PowerWyrm

This means rewriting the object description to sum the pvals for identical flags instead of just collecting all flags for each pval.

All of the code should use a function like object_pval_of_flag(o_ptr, flag) rather than inspecting the data structures directly. Summing all applicable pvals is the obvious interpretation, but if that rule changes in the future it is better to modify a couple of lines rather than to search for every pval access.

**Magnate** · April 20, 2011, 21:24

Originally posted by PowerDiver

That's an entirely different problem, in that the pvals shouldn't simply be listed. Much better, if you want to add wis to magi, to describe as

an elven cloak of magi () Int+2 Wis+3 Stl+4 Spd+1
and this is trivial to accomplish with 3 pvals in the current data structures.

Er ... but it actually takes up more space on the object's title line! Don't forget we are designing with handhelds in mind, so things like long lines are to be avoided.

**PowerDiver** · April 20, 2011, 21:42

Originally posted by Magnate

Er ... but it actually takes up more space on the object's title line! Don't forget we are designing with handhelds in mind, so things like long lines are to be avoided.

Then omit the pvals altogether, or use 2 lines. The 3 choices { full, omit, two lines } should be a display option.

**Derakon** · April 20, 2011, 21:45

The big space-taker for equipment is the equipment type. You could replace "Studded Leather Armor" with "Armor" and save 16 characters right off the bat. Or just remove it entirely, since it has to be armor given that it's in the torso slot.

Wouldn't want to do it for non-handhelds since space isn't at a premium there, of course.

Multiple pvals

Multiple pvals

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment