[Freeciv-Dev] Re: [PATCH] check_map_pos change (PR#1031)

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

IS_BORDER_MAP_POS is actually incorrectly implemented for what it
is doing. Maybe it would help if you thought of it as the inverse
of IS_INTERIOR_MAP_POS. 

Interior tiles do not need to check normalization in adjacent loops, 
only tiles on the border or outside can cause problems, at least
until someone introduces random unreal tiles.

And the check_pos() does not make a lot of sense here. It is
actually quite reasonable to do an adjacent loop from a position
outside the map. Only adjacent tiles that actually lie within the 
map boundaries will pass the guard checks. Everything is always
safe.

Cheers,
RossW
=====

>> > +   (y) <= 0 || (x) <= 0 ||                   \
>> >     (y) >= map.ysize-1 || (x) >= map.xsize-1)


At 12:41 PM 01/10/29 -0500, Jason Dorje Short wrote:
>> >  #define IS_BORDER_MAP_POS(x, y)              \
>> > -  ((y) == 0 || (x) == 0 ||                   \
>> > +  (check_map_pos(&x, &y),                    \
>> > +   (y) == 0 || (x) == 0 ||                   \
>> >     (y) == map.ysize-1 || (x) == map.xsize-1)
>> 
>The iterate macros themselves work fine with non-normal coordinates:
>they just add on offsets and then normalize.  Only in this check is it
>specifically required that a position be normal (otherwise
>IS_BORDER_MAP_POS will always return 0).  I'd prefer to leave the
>check_map_pos near to where the actual assumption is being made.
>
>jason