[Freeciv-Dev] Re: PATCH: fix for smooth_map (PR#991)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Oct 08, 2001 at 12:04:15PM -0400, Jason Dorje Short wrote:
> Jason Dorje Short wrote:
> > 
> > Greg Wooledge wrote:
> > >
> > > Raimar Falke (hawk@xxxxxxxxxxxxxxxxxxxxxxx) wrote:
> > >
> > > > >  static void smooth_map(void)
> > > > >  {
> > > > > +  int new_hmap[map.xsize * map.ysize];
> > > >
> > > > I think that such constructs aren't allow in C. IMHO the size has to
> > > > be a compile time constant. Either I'm wrong and this is ok for local
> > > > function variables or it is an gcc extension.
> > >
> > > You're correct -- it's not allowed in standard C.  You need to use one
> > > of the malloc() family, and free() when you're done.
> > 
> > Wow, that's too bad.  I guess I'll fix it - and all of the other places
> > where I've done the same thing.
> 
> New (hopefully final) patch attached.
> 
> Let's get this thing over with.
> 
> jason
> Index: server/mapgen.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/server/mapgen.c,v
> retrieving revision 1.72
> diff -u -r1.72 mapgen.c
> --- server/mapgen.c   2001/09/30 21:55:34     1.72
> +++ server/mapgen.c   2001/10/08 16:02:29
> @@ -31,7 +31,7 @@
>  #include "mapgen.h"
>  
>  /* Wrapper for easy access.  It's a macro so it can be a lvalue. */
> -#define hmap(x,y) (height_map[(y) * map.xsize + map_adjust_x(x)])
> +#define hmap(x,y) (height_map[map_inx(x, y)])
>  
>  static void make_huts(int number);
>  static void add_specials(int prob);
> @@ -1333,38 +1333,35 @@
>  }
>  
>  /**************************************************************************
> -  smooth_map should be viewed  as a  corrosion function on the map, it levels
> -  out the differences in the heightmap.
> +  smooth_map should be viewed as a corrosion function on the map; it
> +  levels out the differences in the heightmap.
>  **************************************************************************/
>  static void smooth_map(void)
>  {
> -  int mx,my,px,py;
> -  int a;
> -  
> -  whole_map_iterate(x, y) {
> -    my = map_adjust_y(y - 1);
> -    py = map_adjust_y(y + 1);
> -    mx = map_adjust_x(x - 1);
> -    px = map_adjust_x(x + 1);
> -    a = hmap(x, y) * 2;
> -
> -    a += hmap(px, my);
> -    a += hmap(mx, my);
> -    a += hmap(mx, py);
> -    a += hmap(px, py);
> +  /* We make a new height map and then copy it back over the old one.

> +     Care must be taken so that the new height map uses the exact
> +     same storage structure as the real one - it must be the same
> +     size and use the same indexing.

We don't need the same indexing. The new one just have to hold all the
information of the old one. I would use "For this the new map has to
be of the same size as the old one".

>  The advantage of the new array is there's no feedback from
>  overwriting in-use values.

>   --JDS */

I think such personal statements are ugly. We done want a conversion
in the source code:

        } else if (igter)
          /* NOT c = 1 (Syela) [why not? - Thue] */
          move_cost = (ptile->move_cost[dir] ? SINGLE_MOVE : 0);
        else if (punit)

And imagine that I know add now my own comments:

        } else if (igter)
          /* NOT c = 1 (Syela) [why not? - Thue] {Maybe because foobar?! - 
Raimar} */
          move_cost = (ptile->move_cost[dir] ? SINGLE_MOVE : 0);
        else if (punit)

It is known by the CVS log message who made the changes.

And yes I have removed such personal tags 1 or 2 times what I applied
a patch.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 1 + 1 = 3, for large values of 1