[Freeciv-Dev] Re: PATCH: rand_pos function and usage (PR#1017)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Tue, Oct 23, 2001 at 02:05:13PM -0400, Jason Dorje Short wrote:
> Raimar Falke wrote:
> > 
> > > +/* Determines whether the position is normal given that it's in
> > > +   the range 0<=x<map.xsize and 0<=y<map.ysize.  It's primarily a
> > > +   faster version of is_normal_map_pos since such checks are pretty
> > > +   common. */
> > 
> > > +#define is_normal_map_pos2(x, y) (1)
> > 
> > IMHO this name is still not acceptable.
> 
> Ok, I agree...but what should we use?

We need a new name which describes map position for which
0<=x<map.xsize && 0<=y<map.ysize (this condition is independent of the
topology used). If _for example_ the term regular is used that this
function may be: is_regular_map_position_real or
is_regular_map_position_normal. And there would be a
is_regular_map_position.

> > In general I think we should use "struct map_position" more. This may
> > such a place.
> 
> We should use it always or never.  My preference would be for always, but
> currently we're much closer to never.

It can't easily used in all places because of memory management.

> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/server/mapgen.c,v
> retrieving revision 1.75
> diff -u -r1.75 mapgen.c
> --- server/mapgen.c   2001/10/14 21:02:16     1.75
> +++ server/mapgen.c   2001/10/23 18:04:21
> @@ -195,23 +195,30 @@
>  **************************************************************************/
>  static void make_forests(void)
>  {
> -  int x,y;

> -  int forestsize=25;

It looks like this assignment is useless in the old code.

> -  forestsize=(map.xsize*map.ysize*map.forestsize)/1000;
> -   do {
> -    x=myrand(map.xsize);
> -    y=myrand(map.ysize);
> -    if (map_get_terrain(x, y)==T_GRASSLAND) {
> -      make_forest(x,y, hmap(x, y), 25);
> -    }
> -    if (myrand(100)>75) {
> -      y=(myrand(map.ysize*2/10))+map.ysize*4/10;
> -      x=myrand(map.xsize);
> -      if (map_get_terrain(x, y)==T_GRASSLAND) {
> -     make_forest(x,y, hmap(x, y), 25);
> -      }
> -    }
> -  } while (forests<forestsize);
> +  int forestsize=(map.xsize*map.ysize*map.forestsize)/1000;
> +  int *x = fc_malloc(sizeof(*x) * forestsize);
> +  int *y = fc_malloc(sizeof(*y) * forestsize);

> +  int num = (forestsize+myrand(4))/4, num2, i;

A fifth of the total forrests created are from "tropical" forests: s|/4|/5|

> +  /* First make some extra tropical forests. */
> +  RAND_POS_CHECKED(x, y, num,

> +                y >= map.ysize * 4/10 && y < map.ysize * 6/10 && 
> map_get_terrain(x, y)==T_GRASSLAND,

You also have to exclude the poles for this. See make_forest. You may
also introduce IS_POLE or IS_ARCTIC.

Still unknown to me is the difference between:

    if (y>map.ysize*42/100 && y<map.ysize*58/100 && myrand(100)>50)
      map_set_terrain(x, y, T_JUNGLE);
and
      y=(myrand(map.ysize*2/10))+map.ysize*4/10;

These are similar but not equal. Odd.

> +                0);
> +
> +  /* Now finish by placing forests everywhere. */
> +  num2 = forestsize-num;
> +  RAND_POS_CHECKED(x+num, y+num, num2,
> +                map_get_terrain(x, y)==T_GRASSLAND,
> +                0);
> +
> +  /* Now make the forests. */
> +  forestsize = num + num2;
> +  for (i=0; i<forestsize; i++) {
> +    make_forest(x[i], y[i], hmap(x[i], y[i]), 25);
> +  }
> +
> +  free(x);
> +  free(y);

If I see this example I vote for an action part in the
RAND_POS_CHECKED macro. It is silly to accumulate positions and than
do a for loop over them. So it should possible to pass a
"make_forest(x, y, hmap(x, y), 25)" to the RAND_POS_CHECKED
macro. This would also save us the malloc/free.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 A life? Cool! Where can I download one?