[Freeciv-Dev] Re: (PR#6442) gen5 wrapping

["rwetmore@xxxxxxxxxxxx" <rwetmore@xxxxxxxxxxxx>]

*Incredibly* poor implementation for the fix. Try again ...

Instead of changing large numbers of lines everywhere, just update the
two local variables with the appropriate calls once.

This will also be more efficient == faster execution, as usual. For one,
local variables will get optimized when non-local calls will force the
compiler to assume things might be different this time around :-).

It is always good practice to *create* local variables for such things
and cache the called result, so removing this behaviour is clearly bad.

But worse is propagation of the topology calls everywhere when they expose
the underlying implementation and thus will likely be fixed in the future.
Confined their use at least to a couple of spots.

Cheers,
RossW
=====

Jason Short wrote:
> This simple patch replaces two gen5 wrapping constants with queried 
> topology values.
> 
> jason
> 
> 
> 
> ------------------------------------------------------------------------
> 
> Index: server/mapgen.c
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/server/mapgen.c,v
> retrieving revision 1.118
> diff -u -r1.118 mapgen.c
> --- server/mapgen.c   2003/10/03 11:29:31     1.118
> +++ server/mapgen.c   2003/10/08 19:17:25
> @@ -2099,9 +2099,6 @@
>  **************************************************************************/
>  static void mapgenerator5(void)
>  {
> -  const bool xnowrap = FALSE;        /* could come from topology */
> -  const bool ynowrap = TRUE; /* could come from topology */
> -
>    /* 
>     * How many blocks should the x and y directions be divided into
>     * initially. 
> @@ -2109,11 +2106,11 @@
>    const int xdiv = 6;                
>    const int ydiv = 5;
>  
> -  int xdiv2 = xdiv + (xnowrap ? 1 : 0);
> -  int ydiv2 = ydiv + (ynowrap ? 1 : 0);
> +  int xdiv2 = xdiv + (topo_has_flag(TF_WRAPX) ? 0 : 1);
> +  int ydiv2 = ydiv + (topo_has_flag(TF_WRAPY) ? 0 : 1);
>  
> -  int xmax = map.xsize - (xnowrap ? 1 : 0);
> -  int ymax = map.ysize - (ynowrap ? 1 : 0);
> +  int xmax = map.xsize - (topo_has_flag(TF_WRAPX) ? 0 : 1);
> +  int ymax = map.ysize - (topo_has_flag(TF_WRAPY) ? 0 : 1);
>    int x, y, minval;
>    /* just need something > log(max(xsize, ysize)) for the recursion */
>    int step = map.xsize + map.ysize; 
> @@ -2136,7 +2133,7 @@
>  
>    /* if we aren't wrapping stay away from edges to some extent, try
>       even harder to avoid the edges naturally if separatepoles is true */
> -  if (xnowrap) {
> +  if (!topo_has_flag(TF_WRAPX)) {
>      for (y = 0; y < ydiv2; y++) {
>        hmap(0, y * ymax / ydiv) -= avoidedge;
>        hmap(xmax, y * ymax / ydiv) -= avoidedge;
> @@ -2149,7 +2146,7 @@
>      }
>    }
>  
> -  if (ynowrap) {
> +  if (!topo_has_flag(TF_WRAPY)) {
>      for (x = 0; x < xdiv2; x++) {
>        hmap(x * xmax / xdiv, 0) -= avoidedge;
>        hmap(x * xmax / xdiv, ymax) -= avoidedge;