/*
   * get a random visible player who is in the room with the mob.
   * *  If there isn't a random player in the room, rndm stays NULL.
   * *  If you do a $r, $R, $j, or $k with rndm = NULL, you'll crash
   * *  in mprog_translate.
   * *  Adding appropriate error checking in mprog_translate.
   * *    -Haus
   * * This used to ignore players MAX_LEVEL - 3 and higher (standard
   * * Merc has 4 immlevels).  Thought about changing it to ignore all
   * * imms, but decided to just take it out.  If the mob can see you, 
   * * you may be chosen as the random player. -Narn
   */
  count = 0;
  for(vch = mob->in_room->first_person; vch; vch = vch->next_in_room)
    if(!IS_NPC(vch) && can_see(mob, vch))
    {
      if(number_range(0, count) == 0)
        rndm = vch;
      count++;
    }

Okay, so, as we can see from the above loop, the rndm value will always be the first vch that falls into the ifcheck (seen and not an NPC). What's the point of having a number_range with a counter?

Second, I'm getting the below crash from this loop:

#0  0x08195e06 in mprog_driver (
    com_list=0x9459fb8 "if quest(changeofheart) == 2\n\r  mpechoat $n &CVaskar says, 'Well that is a start, but it's not unlike\n\r  mpechoat $n &CSergeant Durgas to sacrifice fifty men to get a spy in place.\n\r  mpechoat $n &CYo"...,
    mob=0x9398ed8, actor=0x943f170, obj=0x0, vo=0x0, single_step=0 '\0')
    at mud_prog.c:2193
2193      for(vch = mob->in_room->first_person; vch; vch = vch->next_in_room)
(gdb) bt
#0  0x08195e06 in mprog_driver (
    com_list=0x9459fb8 "if quest(changeofheart) == 2\n\r  mpechoat $n &CVaskar says, 'Well that is a start, but it's not unlike\n\r  mpechoat $n &CSergeant Durgas to sacrifice fifty men to get a spy in place.\n\r  mpechoat $n &CYo"...,
    mob=0x9398ed8, actor=0x943f170, obj=0x0, vo=0x0, single_step=0 '\0')
    at mud_prog.c:2193
#1  0x08198329 in mppause_update () at mud_prog.c:2878
#2  0x08220fba in update_handler () at update.c:3622
#3  0x080f3452 in game_loop () at comm.c:1350
#4  0x080f6eda in main (argc=Cannot access memory at address 0x40000
) at comm.c:859

Any thoughts? I just don't see the problem. Seems pretty random.

Okay, so I went through and created a true random name generator. It looks pretty clunky to me, so if anyone is able to steamline it a bit more, that'd be great...otherwise, it works the way the original was intended to work.

  count = 0;
  /* First we need to count how many the mob can see before we create the
   * array. - Aurin
   */	 
  for(vch = mob->in_room->first_person; vch; vch = vch->next_in_room)
    if(!IS_NPC(vch) && can_see(mob, vch))
    {
      count++;
    }
  // Initialize the array.
  CHAR_DATA *random [count];
  // We use the second int to keep track of where we are in the array.
  int i = 0;
  for(vch = mob->in_room->first_person; vch; vch = vch->next_in_room)
    if(!IS_NPC(vch) && can_see(mob, vch))
    {
	  random[i] = vch;
	  i++;
    }
  /* And here we have a true random pick based on how many names fill
   * the array.  We use count-1 so that a null value isn't taken into account
   * during the random name generation, as the above count doesn't take 0
   * into account as a valid number.  - Aurin
   */
  rndm = random[number_range(0, count-1)];

The old way does more or less the exact same thing in about the simplest way possible. Not sure on the crashing issue on that line unless your character list got messed up. In the event the character list gets messed up again even the way you set it up will crash it also.

Out of 20 tests with 5 pfiles in the room, the same name was returned all but a couple times. While I admit that could theoretically be a 20% chance to be someone else, it did not show this to me during the test. My way, out of 20 tests with the same 5 pfiles, returned random names almost every time. Not trying to talk down on what Haus and Narn did, but it simply was not creating the appropriate results it was designed for.

As for the crash, yeah, I figure it would crash again since the loop is basically the same...I'll just chalk it up to a random crash, once again. Unless there's a way to test the character list?

Because I'm bored, gonna run through the iterations on the old random generator and show why it's not a random pick. Random means there should be an equal chance for all players involved:

  count = 0;
  for(vch = mob->in_room->first_person; vch; vch = vch->next_in_room)
    if(!IS_NPC(vch) && can_see(mob, vch))
    {
      if(number_range(0, count) == 0)
        rndm = vch;
      count++;
    }

We'll use 5 players for this.
Player A
Player B
Player C
Player D
Player E

For the sake of argument, 0 is not returned for any of these iterations, which is increasingly likely with the more players this particular code loops through. The only "true" randomness to this generator is if only 2 players are in the room and no more.

First iteration of loop, Player A is seen with a 100% chance on the number_range(0, 0) == 0. Player A is now rndm.

Second iteration of loop, Player A is seen with a 50% chance on the number_range(0, 1) == 0...keep in mind, Player A is already set to rndm, so Player B is the one that has the 50% chance of being rndm, not Player A.

Third iteration of loop, Player A is seen with a 66.6% chance on the number_range(0, 2) == 0. This is because the only way Player A would NOT be picked, is if Player C happens to get that 33.3% chance of a 0. By the way, Player B is not even an option here.

Fourth iteration of loop, Player A is seen with a 75% chance on the number_range(0, 3) == 0. This is because the only way Player A would NOT be picked, is if Player D happens to get that 25% chance of a 0. By the way, Player B and Player C are not even an option here.

Fifth, and final, iteration of the loop, Player A is seen with an 80% chance on the number_range(0, 4) == 0. This is because the only way Player A would NOT be picked, is if Player E happens to get that 20% chance of 0. By the way, Player B, Player C and Player D are not even an option here.

So, as we can see, with the more people that show up on the previous randomly generated code, it's heavily biased to the first person picked, and becomes even more so once 3 or more people are added into the loop.

So, I've been experiencing crashes 9 months later in regards to this particular portion of code. And, I think I've solved it, even though there are supposedly checks to solve for this.

The problem:
The randomly generated code would return a NULL value for a name (or garbage data, whatever). This caused a crash and whatever was handling this later in the code, simply wasn't doing the job.

Solution:
I've solved this by slightly changing the end of the random generator:

count = 0;
  /* First we need to count how many the mob can see before we create the
   * array. - Aurin
   */	 
  for(vch = mob->in_room->first_person; vch; vch = vch->next_in_room)
    if(!IS_NPC(vch) && can_see(mob, vch))
    {
      count++;
    }
  // Initialize the array.
  CHAR_DATA *random [count];
  // We use the second int to keep track of where we are in the array.
  int i = 0;
  for(vch = mob->in_room->first_person; vch; vch = vch->next_in_room)
    if(!IS_NPC(vch) && can_see(mob, vch))
    {
	  random[i] = vch;
	  i++;
    }
  /* And here we have a true random pick based on how many names fill
   * the array.  We use count-1 so that a null value isn't taken into account
   * during the random name generation, as the above count doesn't take 0
   * into account as a valid number.  - Aurin
   */
  /* So, I was a bit naive to think this still wouldn't return a NULL, but it does.
   * to stop the NULL seen, have to make sure that we're not checking for an
   * array value below the minimum, which was allowed before.  If it does show
   * up, let's stop the crash by making rndm be the mob trying to use the $r
   * token. - Aurin
   */
  if(count-1 < 0)  // Crash fix
    rndm = mob;
  else
    rndm = random[number_range(0, count-1)];