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

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Fri, Oct 05, 2001 at 02:04:56PM -0400, Jason Dorje Short wrote:
> 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.

It is used in the iterate macros. The storage is independent of
this. Storing the rectangle map in the same way as it done now is nice
but may change.

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

Strings may not be necessary but I would like a 

struct civ_map {
  struct {
    int size_x,size_y,is_isometric,wrap_x,wrap_y;
  } topology;
  int xsize, ysize; // the rectangle size
  int seed;

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

There are people out there who can't count till 10.

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

If [xy]_map_iterate is only a cosmetic thing and there are only two
instances (load and saving) of the construct in savegame: probably
not.

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

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  One nuclear bomb can ruin your whole day.