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

[Jason Dorje Short <vze2zq63@xxxxxxxxxxx>]

Raimar Falke wrote:
> 
> On Fri, Oct 05, 2001 at 12:53:09PM -0400, Jason Dorje Short wrote:
> > Raimar Falke wrote:

> > 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.

Either way will work.

> > >    for (y=0;y<map.ysize;y++)
> > >     for (x=0;x<map.xsize;x++)
> > 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.

Pretty much, yes.  I'd label them as the "topology size" and the
"rectangle size".  The topology size must be stored with the save file;
the rectangle size can be computed from it at need.

The "topology size" is misleading, though; really it's the whole
topology info - something like "5x5 non-wrapping rectangular" or "6x5
horizontally-wrapping iso-rectangular".  Other topologies may require
different data.  The rectangle size is more of a storage issue and is
completely derived from the topology information.

Or perhaps this topology data should be stored as two strings: one
indicating the main topology type and the other indicating parameters. 
These can then be parsed by the appropriate function.  The above two
then coupd be stored as:

"rectangular" "nowrap nowrap 5 5"
"iso-rectangular" "wrap nowrap 6 5"

> >   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.

Err, this is a 6x5 iso-rectangle with a 5x5 rectangle size.

> >   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.

And this is a 5x5 iso-rectangle with a 5x5 rectangle size.

> Do you agree?

See above.  I think you just miscounted.

> There has to be methods which transform between.

Yes, though this need only be done upon map creation and/or savefile
loading.  I haven't given this much thought yet.

> > 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.

Interesting.  Without looking at the code again, I think you could
potentially eliminate as much as 100 lines of code doing this.  Would
you then not use [xy]_map_iterate at all?

> > > > 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.

Well, that would be the "correct" solution IMO, since it allows for the
impassable region to be tiled.  It's not the idea that was proposed
before, though...

jason