[Freeciv-Dev] Re: (PR#7279) Macro optimizations

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=7279 >

Arnstein Lindgard wrote:

> +#define normalize_map_pos(PX, PY)                            \
> +( map_to_native_pos(&_FC_NAT_X, &_FC_NAT_Y, *(PX), *(PY)),   \
> +  !(   (topo_has_flag(TF_WRAPX) ||                           \
> +       (_FC_NAT_X >= 0 && _FC_NAT_X < map.xsize))            \
> +    && (topo_has_flag(TF_WRAPY) ||                           \
> +       (_FC_NAT_Y >= 0 && _FC_NAT_Y < map.ysize)) )          \
> +  ? FALSE                                                    \
> +  : ((_FC_NAT_X = topo_has_flag(TF_WRAPX)                    \
> +                  ? FC_WRAP(_FC_NAT_X, map.xsize)            \
> +                  : _FC_NAT_X),                                      \
> +     (_FC_NAT_Y = topo_has_flag(TF_WRAPY)                    \
> +                  ? FC_WRAP(_FC_NAT_Y, map.ysize)            \
> +                  : _FC_NAT_Y),                                      \
> +    native_to_map_pos((PX), (PY), _FC_NAT_X, _FC_NAT_Y),     \
> +    TRUE) )
> +
> +/*
> + * Yields TRUE if the map position is normal. "Normal" here means
> + * that it is both a real/valid coordinate set and that the
> + * coordinates are in their canonical/proper form. In plain English:
> + * the coordinates must be on the map.
> + */
> +#define is_normal_map_pos(PX, PY)                            \
> +( _FC_MAP_X = (PX), _FC_MAP_Y = (PY),                                \
> +  normalize_map_pos(&_FC_MAP_X, &_FC_MAP_Y) &&                       \
> +  _FC_MAP_X == (PX) &&                                               \
> +  _FC_MAP_Y == (PY) )
> +

1.  I would rather see normalize_map_pos written as an inline function. 
  That allows you to use global and local variables without having to 
worry about shadowing.

2.  is_normal_map_pos can easily be rewritten without normalize_map_pos: 
and it will be much faster.

   {
     map_to_native_pos(&x, &y, x, y);
     return (x >= 0 && y >= 0 && x < map.xsize && y < map.ysize);
   }

The only reason it's done the way it is is that Raimar wanted the 
"logic" to be concentrated in normalize_map_pos only.

Note that unless you compile with DEBUG is_normal_map_pos is rarely 
called.  So this isn't a big issue IMO.

jason