[Freeciv-Dev] Re: example patch: [xy]_map_iterate

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Fri, Oct 05, 2001 at 12:53:09PM -0400, Jason Dorje Short wrote:
> Raimar Falke wrote:
> > 
> > On Fri, Oct 05, 2001 at 04:18:18AM -0400, Jason Dorje Short wrote:
> > > Raimar Falke wrote:
> > > >
> > > > On Thu, Oct 04, 2001 at 03:44:43PM -0400, Jason Dorje Short wrote:
> > > > > Raimar Falke wrote:
> > > > > >
> > > > > > On Thu, Oct 04, 2001 at 09:47:19AM -0400, Jason Dorje Short wrote:
> > > > > > > [Note: this explanation is long and repetitious.  I'm trying to 
> > > > > > > be as
> > > > > > > clear as possible.]
> > > > > > >
> > > > > > > Raimar Falke wrote:
> > > > >
> > > > > > > It is an isometric map, but not a correct one if you desire 
> > > > > > > wrapping.
> > > > > > > It can't wrap properly.  A correction would be:
> > > > > > >
> > > > > > >   a     <- y==0
> > > > > > >  bcd    <- y==1
> > > > > > > efghi      ...
> > > > > > >  jkln      ...
> > > > > > >   mo    <- y==4
> > > > > > >
> > > > > > > ^   ^
> > > > > > > |   |
> > > > > > > |   x==4
> > > > > > > |
> > > > > > > x==0
> > > > > > >
> > > > > > > With the addition of n and o it will now successfully wrap in one
> > > > > > > direction.  y==0 for just tile a.
> > > > > >
> > > > > > This means that wrapping need a odd number in this axis?
> > > > >
> > > > > There are different ways to measure, but...no.  Refer back to my
> > > > > previous post if you're still in doubt.
> > > > >
> > > > > Remember, measuring by x and y sizes won't work.  The two extra tiles 
> > > > > I
> > > > > added did not change x or y dimensions.
> > > >
> > > > But there need to be map sizes. The user want the set them.
> > >
> > > Correct, but they may not be the same as map.xsize/map.ysize.
> > >
> > > I think if we put restrictions on the form of the isometric graph
> > > (always wrappable in both dimensions, etc) then we can use
> > > map.xsize/map.ysize as the options to set the map size.  The problem is
> > > most users won't think of the graph like this:
> > >
> > >   a
> > >  bcd
> > > efghi
> > >  jkln
> > >   mo
> > >
> > > but instead like this
> > >
> > > a d i
> > >  c h n
> > > b g l
> > >  f k o
> > > e j m
> > >
> > > So it would be easier to have the interface treat this as a 6x5 map, and
> > > make the conversion internal.  This is a difficult interface question.
> > 
> > The above map has 15 tiles. This is a 3x5 map to me.
> 
> Actually it's a 4.24 x 3.54 map.  But it's easier to avoid the division
> by sqrt(2) and call it either 6x5 or 3x2.5.  If you must restrict things
> to cartesian counting, then call it a 5x5 map for the surrounding
> rectangle.  However, this makes it very difficult for the game to
> translate to the actual coordinates.  Is a 5x5 map this one
> 
>   a     a d i
>  bcd     c h n
> efghi   b g l
>  jkln    f k o
>   mo    e j m
> 
> or this
> 
>   a     a d i
>  bcd     c h
> efghi   b g l
>  jkl     f k
>   m     e j m
> 
> ?
> 
> The second one won't wrap in either direction, the first will wrap in
> only 1 direction.  

> If you restrict maps to only those that are capable of wrapping in
> both directions (which seems wise)

Why should this be restricted? If the user wants a certain wrap-type
we may adjust xsize and/or ysize by one.

> , then no 5x5 map is possible (I think).
> 
> > > In the case of
> > > an isometric map, it will iterate over the entire surrounding rectangle,
> > > but only real positions within that rectangle are treated regularly; the
> > > others are given dummy values.  This makes the savegame format remain
> > 
> > So this means that y_map_iterate and x_map_iterate won't change under
> > an isometric map? IMHO you have NOW to define what the semantic of
> > map.[xy]size are. Either
> > 
> >    for (y=0;y<map.ysize;y++)
> >     for (x=0;x<map.xsize;x++)
> > 
> > will always (under all possible topologies) catch all tiles or not. If
> > it does that the is_normal_map_pos check is necessary and the
> > [xy]_map_iterate is decoration/cleanup.
> 
> Well, that's why I've started using x_map_iterate/y_map_iterate instead
> of this loop.  So the [xy]_map_iterate is removing the dependancy on the
> storage structure (which may or may not change), while the
> is_normal_map_pos check is removing the dependancy on the topology being
> rectangular (which should change).
> 
> I you must make me decide, I'd say that the above loop DOES cover all
> squares, and in that case we could forget about [xy]_map_iterate.

Yes I'm pushing you ;) So we have two sizes:
 - one the user specifies at the map creation time (I'm not sure if
 this size is needed later)
 - one size which is used throughout the code (and saved in
 map.[xy]size) This MAY also be used for storage.

>   a     a d i
>  bcd     c h n
> efghi   b g l
>  jkln    f k o
>   mo    e j m

This one has a "user" size of 5x5 and a "code" size of 5x5.

>   a     a d i
>  bcd     c h
> efghi   b g l
>  jkl     f k
>   m     e j m

This one has a "user" size of 4x5 and a "code" size of 5x5.

Do you agree?

There has to be methods which transform between.

> I still think they are "prettier" than the loop, though - why else
> would we have whole_map_iterate?  They also allow secondary macros
> (like you suggested) like

Since this most places are in savegame we may also end up with a
savegame.c specific solution. Read:

Make code such as   
  /* get the terrain type */
  for(y=0; y<map.ysize; y++) {
    char *terline=secfile_lookup_str(file, "map.t%03d", y);
    for(x=0; x<map.xsize; x++) {
      char *pch;
      if(!(pch=strchr(terrain_chars, terline[x]))) {
        freelog(LOG_FATAL, "unknown terrain type (map.t) in map "
                "at position (%d,%d): %d '%c'", x, y, terline[x], terline[x]);
        exit(1);
      }
      map_get_tile(x, y)->terrain=pch-terrain_chars;
    }
  }

a macro and pass all required parameters in. Such changes make sense
even without the isometric changes. Savegame.c could tolerate a
facelifting which gets away the code duplication.

> #define yx_map_iterate(y, x_itr)
> {
>   int x_itr;
>   for (x_itr=0; x_itr<map.xsize; x_itr++) {
>     if (is_normal_map_pos(x_itr, y)) {
> 
> This macro automatically skips over invalid tiles, saving that check
> from the main code.  This could be used in most places.  But naming is
> difficult, and I'm not sure if the complexity of having extra macros is
> worth it.
> 
> So, whatever.  My original thought had been to use ONLY these macros;
> then I realized this wouldn't be possible since specialty handling must
> be done for invalid tiles in some places.
> 
> > > Finally, the concepts of "north" and "polar" are themselves
> > > topology-dependent.  In a taurus world we may have no polar regions; in
> > > a mobius strip world the concept of "north" versus "south" has no
> > > meaning (the two are synonymous or interchangable; this presents
> > > problems but is not impossible).  I would MUCH rather do this by having
> > > functions/macros like IS_POLAR and such; ignoring the idea of north
> > > entirely.
> > 
> > Lets forget the very weird other types out and concentrate on "simple"
> > isometric maps. IMHO you can define a "distance from the
> > north-wrapping border". This can return an integer. Depending on the
> > definition of map.ysize the value range of this method may be
> > 0..map.ysize or 0..2*map.ysize.
> 
> Sure.  It can't return an integer, though, unless you're willing to
> approximate.  I just think we're going to want to consider a solution
> that is more flexible in the long run - although (as I said) I'm not
> sure yet what it is.  This is a quite acceptable intermediate change.
> 
> We will then also want a value that represents the TOTAL north-south
> distance from the map - that way we can do the same comparisons we have
> now (like comparing distance to max_distance/10).
> 

> Another idea might be to have functions/macros to return the
> latitude/longitude of the coordinates.  This assumes everything maps
> onto a sphere, though, which might be tricky for taurus maps.

No. Don't make it even more complex.

> > > This map has a "hole" in it.  This hole is an invalid/unreal coordinate
> > > pair; the intent is to represent impassable mountains or something like
> > > that (I'm not too keen on the idea, but some people thought it was
> > > nifty).  However, something as simple as this is pretty much impossible
> > > under our current system.  map_adjust_[xy] will not work with this
> > > system since they assume regular wrapping (you can only tell when you're
> > > on the empty tile by considering both X and Y values), and
> > > whole_map_iterate (along with any manual iterations) will blithely
> > > iterate straight over the empty tile.  The corrections I'm proposing
> > > will fix things so that these assumptions are not made.
> > 
> > This is a lame excuse. There are solutions possible which require less
> > work. I'm just waiting for somebody proposing a worm hole.
> 
> I'm not sure that there are easier solutions, unless the involve the use
> of void_tile which I don't quite understand (but I thought was being
> phased out, not in).

Just made a terrain type which can be passed. Huge move cost and a
special case for air units.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  Two OS engineers facing a petri net chart:
        "dead lock in four moves!"