Complete.Org: Mailing Lists: Archives: freeciv-dev: September 2001:
[Freeciv-Dev] Re: [PATCH] Corecleanup_07Part2 has been put in incoming
Home

[Freeciv-Dev] Re: [PATCH] Corecleanup_07Part2 has been put in incoming

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: Gaute B Strokkenes <gs234@xxxxxxxxx>
Cc: "Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>, jdorje@xxxxxxxxxxxxxxxxxxxxx, freeciv-dev@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: [PATCH] Corecleanup_07Part2 has been put in incoming
From: "Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>
Date: Sat, 01 Sep 2001 15:42:49 -0400

At 02:41 AM 01/09/01 +0200, Gaute B Strokkenes wrote:
>On Thu, 30 Aug 2001, rwetmore@xxxxxxxxxxxx wrote:
[...]
>Inside these macros, of which there are
>not many, it is just as straightforward to use a for loop or two.

*_iterate macros "look" like a single loop. Using single loops is not
really that difficult over the double for loop you like and has a lot
better programming flexibility.

>> The code consolidation for things like iterate macros is
>> significant.
>
>I do not see that.  The basic map iteration macros are adjc_iterate(),
>square_iterate(), city_map_iterate(), and cardinal_adjc_iterate() (or
>whatever it's called); not much consolidation there.

adjc_iterate, cardinal_adjacent iterate can be combined simply, as they
are in the patch. Plus you get all the diagonal_adjacent_iterate and
second ring directions from the same basic code, just changing the 
loop constants. The *_iterate extensions were quite useful in mapgen.

Only rarely do you need a square_iterate with arbitrary radius, and 
here it should be a combined with a block_iterate not just a square.
Note, square_iterate is mostly used in a 2 or 3 tile radius and could 
be replaced everywhere with the same adjacent code as above. 

In fact this is what is done in city_map_iterate_outwards with a clone
of the basic 2 ring array. A simple masking extension like Trent favours
would allow one to do city_maps, pythagorean extensions etc with the
same underlying local array.

>Finally, I do not see how this is at all related to the ease of
>implementing new topologies.

Actually, once you are thinking in terms of cached local coordinates
instead of functional coded for loops, it becomes a simple matter to
pass a different cache array into the same basic loop code. 

This might be a very interesting implementation for dealing with non-
rectangular local coordiantes. I think it was Reinier that had some 
ideas along these lines awhile back, and Trent and Mike have come up
with some interesting locally rectangular, but odd border case
examples.

Hardcoded for loops, switches or anything are really not flexible in
the way data driven algorithms can achieve.

>Is there anyone here who can tell at a glance what block_xy_iterate()
>does?  I can't, though if I sat and studided it for a while I'm sure I
>could.  I think two letter variable names ought to have died out along
>with C64 basic, but apparently we're not that lucky.
>
>Compare to this:
>
>#define rectangle_iterate(x0, y0, x1, y1, x, y)       \

But your version doesn't handle rectangles defined by two arbitrary 
points and forces the user to deal with the ordering conditions that 
are a large part of the leadin code in block_iterate2().

You also don't take care of coordinate wrapping or multiple blocks that
you get by defining a rectangle across a wrap wall, or in the torus case
two wrap walls. 

And you haven't documented it very well. Does anyone actually understand
what this means, or any way one could interpret it to stating something
the is factually correct?
 /* We do not need to check map.topology & TOP_XWRAP \
     here since normalize_map_pos() will have taken   \
     care of that possibility.  */                    \

So in point of fact your rectangular_iterate is useless for most things
but the really trivial case. I'll admit it is simpler to undertand this 
case.

If you consider the full problem, then you will see that the block_iterate
macros while only marginally more code than you have deal with the entire
problem, so users don't need to do anything except feed it the points or
a point and delta vector and it will just crank out the correct block, 
appropriately wrapped, clipped and so on in any of the 4 topologies.

These also are still a single loop and thus easy for programmers to use
plus being consistent with the appearance of a single loop.

The point is that you and one or two others need to understand it to make
sure it is correct, but most users just need to follow the interface
directions and use it. Besides, it really isn't *that* complex for anyone
that is supposed to be of university caliber.

>--
>Big Gaute                               http://www.srcf.ucam.org/~gs234/
>The Osmonds!  You are all Osmonds!!  Throwing up on a freeway at dawn!!!

Cheers,
RossW




[Prev in Thread] Current Thread [Next in Thread]
  • [Freeciv-Dev] Re: [PATCH] Corecleanup_07Part2 has been put in incoming, Ross W. Wetmore <=