Allow ego_min_pval to be zero

[magnate]

Important for mixed-blessing items. Derakon discovered that you can't put negative pval ranges in the current ego template:

​http://angband.oook.cz/forum/showthread.php?p=51066#post51066

[Derakon]

Oddly enough, I was able to make standard objects with negative pvals (e.g. a ring of CON that reduced INT) without trouble, so I suspect it's the "minimum pval" field in the ego definition that's causing trouble.

Also, the parser doesn't like formats like "-1-M5", complaining that they're "not random".

[magnate]

Maybe - were your CON rings fixed or random in their negative INT pval? The problem may be that we can't deal with a negative random_value. That would be more difficult than fixing the min pval issue.

[Derakon]

Uh, I don't remember, and I don't have the game handy to check right now. Sorry.

[magnate]

Punting in accordance with new milestone policy (that any other milestone is only set once someone is actually working on the ticket).

[Derakon]

I poked at this a bit; the problem is that the min_pval field is getting set erroneously, in parse_e_l. The call to parser_getint(p, "min") returns a massive value, which is then added on at ego creation time. The parser code is horribly hairy for me (who names a function "parse_e_l" and then doesn't comment on what it does?) so I don't really know how to go about fixing it, but that's the place to start.

[takkaria]

Re-assigning to 3.3 and CCing Elly because this is a parser bug (see above comment).

[elly]

This is not a parser bug.

object/object.h: byte min_pval[MAX_PVALS]; /* Minimum pval */

-1 in two's complement is 0xFFFFFFFF, which (when coerced down to a byte) is 0xFF, which is 255. If you want signed minima, you have to store them in a signed type. I believe Magnate is undertaking some clever hack to fix this.

(Our not accepting "-1-M5" as a pval expression is a separate problem, to do with the parser's refusal to accept random values with more than one minus sign. I am considering how to fix it.

[elly]

By the way, the function names: parse_X_Y() parses Y: lines for X_info, so parse_e_l parses L: lines for e_info. I couldn't really think of a nicer way to name these.

[magnate]

This is fixed in [r821f59d] by ensuring that a min_pval of 0 is not actioned. So go right ahead and put those negative pvals into ego_item.txt - just stick to either fixed numbers (-X) or random (-XdYMZ), but not both. See #1451 for the ticket to fix this.

[Derakon]

Awesome, thank you!

Elly: that makes sense, now that I know what it is. The problem was that I didn't know what it was when I saw it. That in mind, a bit of hairy vim search&replace has generated this diff that adds a simple comment to the top of each parse_foo_bar function. Next time someone who isn't use dives into this code, they'll need to spend a little less time figuring out where they are. :)

[magnate]

Ok, I'm taking this back out of pending, as once #1451 is fixed we will need to allow 0 to be a genuine minimum pval. So the obvious solution is to use 255 as the value that means "do not specify a minimum", but there may be a better solution. We may also want to specify a minimum which is below zero (e.g. for a pval of -3+M8 you might want a floor of -2), so we could change min_pval[] from byte to s16b - but we ought to have a "no minimum" even if we do that.

[magnate]

​http://angband.oook.cz/forum/showthread.php?t=4547 for more discussion.

[magnate]

[r40d63b8]

[magnate]

In master as of [ref5f4267]