[Freeciv-Dev] Re: Map coordinate cleanups.

[Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx>]

 --- Gaute B Strokkenes <gs234@xxxxxxxxx> wrote: 
> On Thu, 16 Aug 2001, gberkolaiko@xxxxxxxxxxx wrote:
> 
> Frankly, anyone who is unable to understand the difference between:
> 
>  * Real tile coordinates.
>  * Tile coordinates that are not in canonical form.
>  * Tile coordinates that are both.
> 
> should consider working on something other than Freeciv.  I agree that

Thanks for your friendly advice.  
A lot has being commented on this topic since you wrote this email, so I
won't add to the traffic.

> > it seems to me that MAP duplicates map_get_tile
> 
> Not quite.  You may have noticed that MAP() has different error
> handling, and that it is possible to change the way that MAP() deals
> with improper coordinates without worrying about the masses of code
> that calls map_get_tile().  I find this very helpful as I go through
> all the code.

ok, if you find it useful.
but look at your proposed form of map_get_tile:
first you adjust_x and then check if the tile is REAL
(why not just write if(!normalize_map_pos(&x,&y)) ?)
then you return MAP and in MAP you check if the tile is REAL (and NORM)
again.  I don't see much sense in this double check.  I guess you plan to
remove it later, but why introduce it at all?

> > I also think you should try to avoid using 
> > 
> > x=map_adjust_x(x)
> > if (!IS_REAL_TILE(x,y)){ ... }
> > 
> > in favour of normalise_map_pos.  The reason is that it's much easier
> > to adapt normalise_map_pos to different topologies, also those where
> > adjusting x will depend on y (one of them being proper civ2-style
> > isometric view).
> 
> Well, duh. Guess what?  That's just the sort of stuff that this patch
> is full off.  I didn't claim to have covered all cases, though I'm

I don't understand you here.  Are you claiming that your patch will make
transition to, say, isometric view topology smoother?  Well, map_adjust_x
is definitely better than what was there before your patch, but it's not
isometric-compatible and cannot be made so (unless you change the number
of arguments it takes).

> working on it.  If you've found a place in my patch that is not like
> that for a good reason, then you've found a bug.

BTW, I am sorry if my criticism offended you, I was just trying to offer
a different view.


> > I will try to try it on GTK ;)
> 
> Thanks.

seems to work just fine.

G.

____________________________________________________________
Do You Yahoo!?
Get your free @yahoo.co.uk address at http://mail.yahoo.co.uk
or your free @yahoo.ie address at http://mail.yahoo.ie