[Freeciv-Dev] Re: [PATCH] map_adjust_* patch

[Gaute B Strokkenes <gs234@xxxxxxxxx>]

On Sat, 1 Sep 2001, hawk@xxxxxxxxxxxxxxxxxxxxxxx wrote:
>
> I still like the idea of a is_border_position better than to do code
> knowledge about the topologies into the iter macros

I think that coding knowledge about topologies into the (4 or so)
iteration macros is perfectly fine.  Since there are so few of them
and they are so critial to performance it makes sense to give them the
knowledge they need to walk the map in an efficient way.  

Aside from that, as far as I can see any code that calls
is_border_pos() incorporates topological knowledge by its very nature,
since it must attach a meaning to the return value, which can only be
interpreted topologically.  So using is_border_pos() along with
normalize_map_pos() does not win you very much in terms of complexity.

I don't quite see how you would apply is_border_pos() in most of the
code; I think it is in most cases more natural to do

  x += DIR_DX[dir]
  y += DIR_DY[dir]

  if (IS_SANE_POS(x,y) || normalize_map_pos(&x, &y)) {
    /* Frobnicate. */
  }

in the general case.    

Another thing is that normalize_map_pos() is not a magic bullet that
can solve all map position issues.  For instance, in an isometric
topology you can not normalize x and y values separately.  Therefore,
you need to be very careful about calling normalize_map_pos() on
coordinates that are also controlling variables in a loop.  I haven't
looked too closely, but I'm not satisfied that this is not the case
where Jason change outwared_iterate().

> (see Gautes point from above). AFAI see normalize_map_pos can/will
> become more complicated if other topologies are introduced. There is
> no nice way to code normalize_map_pos later as a macro.

So let's make normalize_map_pos() an inline function, that way we can
both have our cake and eat it too.  

> I would apply the patch in its current form.

So would I, but only if someone tests it with a normalize_map_pos()
that has been inlined and it shows no significant performance
regression.  Assuming it does not, we can change normalize_map_pos()
in the appropriate manner afterwards.

-- 
Big Gaute                               http://www.srcf.ucam.org/~gs234/
Yow!  It's a hole all the way to downtown Burbank!