[Freeciv-Dev] Re: [Patch] MAPSTEP

[Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx>]

 --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote: 
> 
> The patch removes all occurrences of DIR_DX by:
>  - replacing loops with adjc_[dir_]iterate or
>  - useing the new macro MAPSTEP or _MAPSTEP

This patch is very badly needed and long waited for.  I cannot detect any
mistakes, but I did not read it thouroughly or/and carefully.

However I strongly object to your use of _MAPSTEP because:
1. It obscures the code.
2. It is unnecessary.
3. It's got bad name (MAPSTEP_FROM_0 would be better if you insist on
using it). 

> I'm sure I have made mistakes. Can you please go over the patch? I

how about test runs and comparisons between old/new savegames ?
you patch shouldn't change any behaviour, right?

[..]

> @@ -2083,8 +2067,9 @@
>    get_canvas_xy(x, y, &canvas_src_x, &canvas_src_y);
>    canvas_src_x += NORMAL_TILE_WIDTH/2;
>    canvas_src_y += NORMAL_TILE_HEIGHT/2;
> -  canvas_dest_x = canvas_src_x + (NORMAL_TILE_WIDTH * DIR_DX[dir])/2;
> -  canvas_dest_y = canvas_src_y + (NORMAL_TILE_WIDTH * DIR_DY[dir])/2;
> +  _MAPSTEP(canvas_dest_x, canvas_dest_y, dir);
> +  canvas_dest_x = canvas_src_x + (NORMAL_TILE_WIDTH * canvas_dest_x) /
> 2;

Best,
G.

____________________________________________________________
Do You Yahoo!?
Get your free @yahoo.co.uk address at http://mail.yahoo.co.uk
or your free @yahoo.ie address at http://mail.yahoo.ie