Opened 6 years ago

Last modified 6 years ago

#1451 assigned bug

Fix random_value parsing so that you can specify certain negative values

Reported by: magnate Owned by: elly
Milestone: v4 Keywords: cleanup
Cc:

Description (last modified by takkaria)

At the moment -1 works and 1-2d3M4 works (both the die roll and the m_bonus are treated as negative), but -1-2d3M4 does not work. Nor does 1+2d3-M4 or 1-2d3+M4 (maybe the latter shouldn't, but the former should).

Change History (7)

comment:1 Changed 6 years ago by takkaria

  • Milestone changed from Future to 3.3.0
  • Owner set to elly
  • Status changed from new to assigned

comment:2 Changed 6 years ago by takkaria

  • Description modified (diff)
  • Summary changed from Allow the random_value struct to deal properly with negatives to Fix random_value parsing so that you can specify certain negative values

comment:3 in reply to: ↑ description Changed 6 years ago by elly

What semantics should your examples have?
Right now a leading '-' is commented to mean 'negate all components' but actually does this:

if (negative)
{

bonus->base *= -1;
bonus->base -= bonus->m_bonus;
bonus->base -= bonus->dice * (bonus->sides + 1);

}

so your examples therefore mean:

1) -1-2d3M4 -> base = 1, dice = -2, sides = 3, m_bonus = 4 -> base = 3, dice = -2, sides = 3, m_bonus = 4
2) 1+2d3-M4 -> base = 1, dice = 2, sides = 3, m_bonus = -4
3) 1-2d3+M4 -> base = 1, dice = -2, sides = 3, m_bonus = 4

If those semantics are agreeable, I can hack in support for these cases easily, but it looks like case 1 is not doing what you want.

(For case 1, base = 1 dice = -2 sides = 3 m_bonus = 4, so:

base *= -1; /* base == -1 now */
base -= m_bonus; /* base == -5 now */
base -= bonus->dice * (bonus->sides + 1) == -2 * 4 /* base == 3 now */

Please discuss.

comment:4 Changed 6 years ago by magnate

Thanks for the update, elly. I hadn't realised the parsing of these values was ... odd. IMO each of the three components should have its own sign. So:

1+2d3M4 implies +1+2d3+M4

-1-2d3-M4 does what it says

2d3-M4 implies +2d3-M4

-1+2d3-M4 likewise. Etc.

Any absent sign should imply +. Any minus sign should apply only to one component (base, dice or m_bonus).

comment:5 Changed 6 years ago by magnate

Oooh, I just realised that fixing this will break pvals if someone uses a random_value which comes out at 0 (e.g. -y+Mx where x >= y). This may be why MarbleDice? designed the random_value parsing the way he did - so that it couldn't come out at 0. I guess we'll just have to see what breaks ... I've fixed object_add_pval to ignore a pval of 0 so this should be ok ([r802d530]).

comment:6 Changed 6 years ago by magnate

  • Milestone changed from 3.3.0 to 3.4.0

Postponing to 3.4 per IRC

comment:7 Changed 6 years ago by magnate

  • Milestone changed from 3.4.0 to 4.0

These are changes which should first be tried in v4.

Note: See TracTickets for help on using tickets.