[Freeciv-Dev] Re: to wrap or not to wrap?

[Jason Dorje Short <vze2zq63@xxxxxxxxxxx>]

Raimar Falke wrote:
> 
> On Mon, Oct 15, 2001 at 03:05:47AM -0400, Jason Dorje Short wrote:
> > void wrap_map_pos(int *x, int *y)
> > {
> >   if (wrap_in_x_direction) {
> >     while (x < 0) x += map.xsize;
> >     while (x >= map.xsize) x -= map.xsize;
> >   }
> >
> >   if (wrap_in_y_direction) {
> >     while (y < 0) y += map.ysize;
> >     while (y >= map.ysize) y -= map.ysize;
> >   }
> > }
> 
> > The reason wrap_map_pos works while normalize_map_pos doesn't is that
> > the "position" that's being wrapped isn't really a position at all; it's
> > just a random vector expressing the difference between (map_x, map_y)
> > and (map_view_x0, map_view_y0).  But wrap_map_pos will handle it just
> > fine by wrapping it in only the correct (valid) directions.
> 
> Ok. But this wrapping isn't a map wrapping but a GUI wrapping.

It is a map wrapping; it's just that only the GUI needs it (all the rest
of the code behaves in a sane manner and doesn't need to).

> > The problem is exactly that the GUI does not behave in this manner.
> > It's not just a flat window sitting over a wrapping world; the
> > coordinates in the window itself are being wrapped (as above).  If it
> > were otherwise, then we'd just take our window coordinates (window_x,
> > window_y), adjust them to map coordinates (map_x, map_y), and call
> 
> > normalize_map_pos() to see what we had at that position.  This would be
> > the better solution, but unfortunately (1) it will require a significant
> > overhaul to achieve this and (2) we will need to do some additional
> > tricks to make sure we don't draw a tile more than once (since the
> > flat-earth window knows nothing of the underlying topology) [2].
> 
> IMHO we should do the real fix. Even if it requires more work. We can
> make the fixing in small steps. So far everybody agrees that the
> client is messed up. We also agreed that the drawing isn't the
> problem. A bit more difficult could be the transformation back
> (mouse-click position to map position) But this is only one method. So
> far nobody has stepped forward to do a real fix. If nobody volunteer I
> will do it. But please forget about the wrap_map_pos band-aid.
> 
> Jason can you point out any problems which the author of a cleanup
> patch may encounter?

I'm willing to make many of the changes; in fact in the past I've
scraped together a few demo patches that do this sort of thing.  There
are several problems that I anticipate:

1.  Doing it in "small steps" is difficult or impossible; since most of
the functions are closely tied together [1] we'll either need to convert
large groups of functions simultaneously or separate code out into "new"
and "old" versions of the same function.  The second is really poor, so
we'll probably end up with some lengthy patches.  If we choose to go
this way you must be prepared for this.

2.  A fair amount of code is duplicated in different GUI's.  This means
extra work is required to fix all of them (often they're not *quite*
identical), and extra work is required by people to test them on
different platforms.  The "correct" solution in many cases will be to
move code into common client functions, but this has the same problems
as above and is even more work (though with very large payoffs).

3.  Fixing the isometric view will be really ****ing hard.  Those
manipulations make my head hurt.

4.  Making sure the client doesn't draw a tile more than once will
require some additional work.  I have several ideas how this may be
accomplished; it should be fairly unintrusive but again it'll have to be
verified for all gui's.  Once this is done it should be easy enough to
explore with showing multiple tiles (as a compile-time, run-time,
tileset, etc. option).

Really, I don't think any of these problems are all that hard.  The
problem is they're multiplicative - we'll have to convert ALL functions
for ALL gui's INCLUDING isometric, AND make sure tiles aren't drawn more
than once.

For now I recommend we proceed with our small-step methods until we
reach a point where they no longer work.

> > [1] This word looks really ugly without the accent!
> 
> You forgot [2].

Yes :-).

[1] Take the low-level get_canvas_xy function shown in the previous mail
and its inverse (IIRC get_map_xy).  Converting these functions to the
new system requires that all functions that call them must be converted
as well, otherwise the coordinates they get won't be what they expect. 
Or you can make duplicate sets (get_canvas_xy_old and
get_canvas_xy_new), but this really pollutes CVS and you'd still have
the same problem at the next level.

jason