[Freeciv-Dev] Re: [Patch] MAPSTEP

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

It looks good. Nice work.

Minor change, you should pass (int *) as the first two arguments, and
not ints. This does two things: 

1) it alerts people to the fact that these are LValues to be returned, 

2) more importantly, it makes the macro and functional form equivalent, 
   so if you ever move to a more complex system, all you need to do is
   remove the macro definition.

In anticipation of such a change, it might not be bad to make the names
look more like functions when you change the two arguments, i.e.
 _mapstep(px, py, dir)
  mapstep(px, py, x0, y0, dir)
Note this makes it much more equivalent in form to normalize_map_pos() 
which is not bad, since this is really a normalize adjacent position.

There is perhaps some risk in consolidating the GUI code with the 
general direction system at this point, but this will at least force
the problems out into the open when you start changing things.

Cheers,
RossW
=====

At 11:33 PM 01/09/21 +0200, Raimar Falke wrote:
>
>The patch removes all occurrences of DIR_DX by:
> - replacing loops with adjc_[dir_]iterate or
> - useing the new macro MAPSTEP or _MAPSTEP
>
>I'm sure I have made mistakes. Can you please go over the patch? I
>know that the patch may slowdown the goto code because more tests for
>realness will be carried out. Also note that I have only tested that
>the changed tree compiles.
>
>       Raimar
>-- 
> email: rf13@xxxxxxxxxxxxxxxxx
>  This message has been ROT-13 encrypted twice for extra security.
>
>Attachment Converted: "c:\program files\eudora\attach\dir_dx_remove1.dif"