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

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

The only addition here is to change the for to a do ... while. In the
normal case, apart from all the new dir[8] assignments, the code flow
should exit on the if, and so one should put all the looping stuff
behind it.

But ...

I really don't think that the bounded problem is worth this. If we have
maps where there are no adjacent tiles, then I suspect a lot of other 
things are going to be broken a lot worse than this. I would make the
caller check this condition before calling a function that is designed 
return a neighbour.

The other alternative is to define a "checked" wrapper function that
verifies that a neighbour does exist, then calls the simpler version of
this. Caller's then won't have to think about how to write their own
check. And you can spend the time once now to do this as efficiently as
possible.

Cheers,
RossW
=====

At 03:08 PM 01/08/20 -0700, Trent Piepho wrote:
>On Mon, 20 Aug 2001, Jason Dorje Short wrote:
>> I added an assertion to make sure the loop terminates (if debugging;
>> otherwise we assume it terminates).  Bailing out won't really do any
>> good since it would leave the game in an unstable state.
>
>
>Here are I think better ways to do it.  My rand_neighbor function will run in
>a bounded time, and will aways find a neighbor if one exists.
>
>void rand_neighbor(int x0, int y0, int *x, int *y)
>{
>  int n, choice;
>  int dirs[8] = {DIR8_NORTH, DIR8_NORTHEAST, DIR8_EAST, DIR8_SOUTHEAST,
>                 DIR8_SOUTH, DIR8_SOUTHWEST, DIR8_WEST, DIR8_NORTHWEST};
>  
>  for(n=8;n>0;n--) {            /* We get 8 chances to find a spot */
>    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! */
>
>    /* Choice was bad, so replace it with the last direction in the list.
>     * On the next iteration, one fewer choices will remain. */
>    dirs[choice] = dirs[n-1];
>  }
>  /* What the hell?  No valid neighbor tiles.  This is a 1x1 map?  */
>  assert(0);
>}