Rng.cpp code — bug? feature?

[Skyrunner]

The current code for rng.cpp is

[tt]long rng(long low, long high)
{
return low + long((high - low + 1) * double(rand() / double(RAND_MAX + 1.0)));
}[/tt]

Though apparently it hasn’t been a problem so far, adding the lines

[tt]//activating assert()
#ifdef NDEBUG
#undef NDEBUG
#endif

#include [/tt]

and inserting [tt]assert (low <= high)[/tt] causes

It seems that the game doesn’t follow the low/high conditions as it should. It potentially could cause problems with random numbers being out of range or stuff. I’m not sure how the algorithm works, currently, though it does seem to translate [tt]rand()[/tt] to the correct range then adding [tt]low[/tt] to the result. Adding

[tt]if (low > high)
{
long temp = high;
high = low;
low = temp;
}[/tt]

presumably fixes the issue.

Stumbled upon this while trying to figure out how to mod Cataclysm. o_O I’m not sure if it’s a bug or a feature. I wouldn’t even have known if not for boost.random’s assert, as I tried to use the uniform_int_distribution boost has instead of a rand()-using formula.

[KA101]

Huh. I know that one high-level encounter doesn’t always spawn its participants like it seems it should. Though there’s certainly other possible reasons for that (probably including my misreading its code) this is certainly interesting.

[Skyrunner]

A better investigation turned out that the assert fail only happens once, when you make a random character. The numbers involved are 0 and -1.

[oohara]

I ran the version 0.5 stable with the assert.
Here is what I found:

— a/overmap.cpp
+++ b/overmap.cpp
@@ -810,7 +810,7 @@

else if (ter(i, j, z + 1) == ot_lab_core ||
         (z == -1 && ter(i, j, z + 1) == ot_lab_stairs))

• lab_points.push_back(city(i, j, rng(1, 5 + z)));

• lab_points.push_back(city(i, j, 1));

else if (ter(i, j, z + 1) == ot_lab_stairs)
 ter(i, j, z) = ot_lab;

— a/mapgen.cpp
+++ b/mapgen.cpp
@@ -205,7 +205,8 @@
}
draw_map(terrain_type, t_north, t_east, t_south, t_west, t_above, turn, g, density);

• if ( one_in( oterlist[terrain_type].embellishments.chance ))

• // one_in(chance) returns true if chance <= 1

• if (oterlist[terrain_type].embellishments.chance > 0 && one_in( oterlist[terrain_type].embellishments.chance ))
  add_extra( random_map_extra( oterlist[terrain_type].embellishments ), g);

  post_process(g, zones);
  @@ -3126,9 +3127,9 @@
  case ot_s_sports_west:
  lw = rng(0, 3);
  rw = SEEX * 2 - 1 - rng(0, 3);

• tw = rng(3, 10);

• tw = rng(3, 8);
  bw = SEEY * 2 - 1 - rng(0, 3);

• cw = bw - rng(3, 5);

• cw = bw - rng(3, 4);
  for (int i = 0; i < SEEX * 2; i++) {
  for (int j = 0; j < SEEY * 2; j++) {
  if (((j == tw || j == bw) && i >= lw && i <= rw) ||
  @@ -10360,6 +10361,20 @@
  debugmsg(“map::place_items() called for an empty items list (list #%d)”, loc);
  return;
  }

• if (x1 > x2)

• {

• int temp;

• temp = x1;

• x1 = x2;

• x2 = temp;

• }

• if (y1 > y2)

• {

• int temp;

• temp = y1;

• y1 = y2;

• y2 = temp;

• }

  int item_chance = 0; // # of items
  for (int i = 0; i < eligible.size(); i++)
  — a/rng.cpp
  +++ b/rng.cpp
  @@ -24,6 +24,10 @@

int dice(int number, int sides)
{

• // this happens when the player uses a crowbar without the mechanics skill
• if (sides <= 0)
• return 0;
• int ret = 0;
  for (int i = 0; i < number; i++)
  ret += rng(1, sides);