[Freeciv-Dev] Re: PATCH: remove map_adjuxt_[xy] invocations

[Jason Dorje Short <vze2zq63@xxxxxxxxxxx>]

Jason Dorje Short wrote:

> The attached patch removes all remaining invocations of map_adjust_[xy]
> in server code.

> @@ -1336,33 +1338,23 @@
>    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;
> -
> +
> +  /* This overwrites itself as it runs, creating an unpredictable
> +     feedback.  Is this desired?  --JDS */
>    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);
> 
> -    a += hmap(x, my);
> -    a += hmap(mx, y);
> +    adjc_iterate(x, y, x2, y2) {
> +      a += hmap(x2, y2);
> +    } adjc_iterate_end;
> 
> -    a += hmap(x, py);
> -    a += hmap(px, y);
> +    a += myrand(60) - 30;
> 
> -    a += myrand(60);
> -    a -= 30;
>      if (a < 0)
>        a = 0;
>      hmap(x, y) = a / 10;
>    } whole_map_iterate_end;
>  }

This part isn't correct.

First, the code has problems right now since (as the comment says) the
array itself gets overwritten as the map is iterated.  The feedback is
unpredictible since it depends on the ordering of whole_map_iterate. 
Thus changing whole_map_iterate will change the behavior, which is Not
Good.  It also means extra smoothing, which probably wasn't the intent
of the code.

The new code is not the same as the old code since the old code wouldn't
just skip over unreal (invalid) positions, it would take the nearest
tile and double-count that position.  I don't think this is necessarily
desired, though; it should be close enough just to skip over these
positions (although savegames won't be identical).  Otherwise we can
loop over the 8 directions and use nearest_real_map_pos or something
like that to get the same behavior.

The new code is incorrect because it divides by 10 in any case, even if
all 10 tile positions have not been added on.  This is easily corrected.

A new patch will follow shortly.

jason