[Freeciv-Dev] Re: [PATCH] base_real_map_distance (PR#1049)

[Jason Dorje Short <vze2zq63@xxxxxxxxxxx>]

Gaute B Strokkenes wrote:
> 
> On Thu, 1 Nov 2001, jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
> > This patch introduces a new topology function,
> > base_real_map_distance().  It performs a similar function to
> > real_map_distance, but also finds a distance vector from the source
> > to the destination coordinate pair.  It also replaces xdist and
> > ydist.
> >
> > A function like this is absolutely essential for iso-rectangular
> > topologies.  xdist() and ydist() will not work because you cannot
> > find the distance along one coordinate axis independently of the
> > other.
> >
> 
> >  int real_map_distance(int x0, int y0, int x1, int y1)
> >  {
> > -  int xd = xdist(x0, x1);
> > -  int yd = ydist(y0, y1);
> > -  return MAX(xd, yd);
> > +  int xd, yd;
> > +  return base_real_map_distance(&xd, &yd, x0, y0, x1, y1);
> >  }
> >
> >  /***************************************************************
> > @@ -317,8 +299,10 @@
> >  ***************************************************************/
> >  int sq_map_distance(int x0, int y0, int x1, int y1)
> >  {
> > -  int xd = xdist(x0, x1);
> > -  int yd = ydist(y0, y1);
> > +  /* We assume base_real_map_distance gives us the vector with
> > +     the minimum squared distance.  Right now this is true. */
> > +  int xd, yd;
> > +  base_real_map_distance(&xd, &yd, x0, y0, x1, y1);
> >    return (xd*xd + yd*yd);
> >  }
> 
> Before you propose such a change you should check that the function is
> being used the way you think it is.  There are only 3 places where
> this function is used.  In server/settlers.c the use is really not
> necessary, and in control.c it is entirely cosmetic.

Well, the results of the new code are identical to the old.  You're
right, I didn't really check further. :-)

None of this, though, really affects the internals of the patch.

> I propose to scrap it entirely, which is what I'm doing.  That is a
> much better idea than trying to apply a concept (Euclidean metric) to
> arbitrary nightmare topologies, where it does not have a sensible
> meaning.

Yes, although I don't think we can guarantee that it won't ever be
necessary.

As for your patch, though, I think you'd be much better off creating a
new macro (a variant of square_iterate, not sure what it should be
called) than rewriting the code to not use a macro.  Aside from that,
all looks good.

> > @@ -327,7 +311,11 @@
> >  ***************************************************************/
> >  int map_distance(int x0, int y0, int x1, int y1)
> >  {
> > -  return xdist(x0, x1) + ydist(y0, y1);
> > +  /* We assume base_real_map_distance gives us the vector with
> > +     the minimum map distance.  Right now this is true. */
> > +  int xd, yd;
> > +  base_real_map_distance(&xd, &yd, x0, y0, x1, y1);
> > +  return abs(xd) + abs(yd);
> >  }
> 
> We should just scrap this one altogether.  Again, the only good
> distance is the length-of-a-unit's walk one.  This one has no sensible
> meaning neither in terms of game mechanics (except possibly for some
> settler actions) nor in terms of "real life".

Hmmm, possibly.

> > Index: server/settlers.c
> > ===================================================================
> > RCS file: /home/freeciv/CVS/freeciv/server/settlers.c,v
> > retrieving revision 1.112
> > diff -u -r1.112 settlers.c
> > --- server/settlers.c 2001/10/14 21:02:17     1.112
> > +++ server/settlers.c 2001/11/01 14:09:33
> > @@ -769,8 +769,8 @@
> >    for (i = 0; i < game.nplayers; i++) {
> >      city_list_iterate(game.players[i].cities, pcity) {
> >        if (map_distance(x, y, pcity->x, pcity->y)<=8) {
> > -        dx = xdist(pcity->x, x);
> > -        dy = ydist(pcity->y, y);
> > +     base_real_map_distance(&dx, &dy, pcity->x, pcity->y, x, y);
> > +     dx = abs(dx), dy = abs(dy);
> >       /* these are heuristics... */
> >          if (dx<=5 && dy<5)
> >            return 0;
> 
> This is an instance of correct usage of map_distance() (I think, since
> building irrigation and suchlike does not work diagonally).

Yes.

> It is also an example of the fact that displacement verctors don't
> work in the
> 
>     ^
>  <###
> 
> topology that you proposed.

(Note: I haven't really "proposed" this topology, since as you've said I
haven't even contemplated how to get around the problems it presents. 
It just represents the worst-case scenario of the topologies I think
FreeCiv *should* be able to handle, so I find it interesting.)

Displacement vectors *do* work, they just aren't the same between
representative memebers (which for most practical purposes is the same
thing).  See my other post.  This will be very tricky to handle (if
indeed we ever do it).

jason