[Freeciv-Dev] Re: [PATCH] more small directional cleanups

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

At 12:45 PM 01/08/22 -0700, Trent Piepho wrote:
>On Wed, 22 Aug 2001, Ross W. Wetmore wrote:
>> 
>> But his code is about 2-3 times more expensive for the standard case and
>
>How so?  I think the generation of the random number is the most expensive
>part, and I only do that once for the common case.
>
>> I think that the problem of calling rand_neighbour() to generate a random
>> adjacent tile when there are no adjacent tiles is a programming error that
>
>It's not so much calling it when there're are no possibilities, but that you
>depend on randomly finding one of the valid neighbors.  You could guess a
>random number 1000 times and still not find it.  It seems bad to me
>to depend on "luck" to keep your code from locking up.

I'm not sure I'm concerned about the 1000 failure case. The universe is
something like 10^30 nanoseconds old, and the probability you are worried
about is 1 in 10^500 or so.

But would you be willing to amalgamate the flavours to something like this?

It even gets rid of the initial assert, since the loop is now bounded.

Note, that you don't need to drag the enum values into it, since you never
care about specific compass directions and the DIR_DX array is always size
8.

Cheers,
RossW
=====

/**************************************************************************
Random neighbouring square.
**************************************************************************/
void rand_neighbour(int x0, int y0, int *x, int *y)
{
  int choice;

  /* don't check twice for non-border cases (the norm), instead redo only
   * failed border ones, which are only 3 in 8 chances on average anyway.
   * This should also work for any map topography, not just cylindrical.
   */
  choice = myrand(8);
  *x = x0 + DIR_DX[choice];
  *y = y0 + DIR_DY[choice];
  if(!normalize_map_pos(x, y)) {
    int dirs[8] = {0, 1, 2, 3, 4, 5, 6, 7};
    int n = 7;
    do {
      /* Choice was bad, so replace it with the last direction in the list.
       * On the next iteration, one fewer choice will remain. */
      dirs[choice] = dirs[n];
      choice = myrand(n);
      *x = x0 + DIR_DX[dirs[choice]];
      *y = y0 + DIR_DY[dirs[choice]];
      if(normalize_map_pos(x, y)) 
        return;  /* Found a valid spot! */
    } while(n-- > 0);
    /* What the hell?  No valid neighbor tiles.  This is a 1x1 map?  */
    assert(0);
  }
  return;
}