[Freeciv-Dev] Re: [PATCH] Formatting cleanup.

[Gaute B Strokkenes <gs234@xxxxxxxxx>]

On Tue, 09 Oct 2001, vze2zq63@xxxxxxxxxxx wrote:
> Gaute B Strokkenes wrote:
>> 
>> On Sun, 07 Oct 2001, vze2zq63@xxxxxxxxxxx wrote:
>> >
>> > Assumption?  Where?  I know you are assuming this, but can you
>> > please point out where the code uses this?
> 
> [snip mathematical argument]
> 
> You have just proven [1] that given this assumption, things work as
> you say they do.  The topology I described before [2] obviously does
> not fit this assumption, so if we implement things that way it
> wouldn't work out.

Whether you like it or not, this is just a straightforward translation
of the fact that Freeciv uses map_adjust_x() / normalize_map_pos()
pretty much everywhere, and the properties of these functions
(slightly generalised to allow for isometric maps).

You may not wish to make this assumption, but you have not done a good
job at describing what sort of assumptions you do and do not wish to
make, and how the Freeciv codebase is supposed to deal with the
problem of supporting (almost) arbitrarily strange shapes.

> [2] The topology was (a larger version of)
>      ^
>      |
> <- X X
> 
> Obviously it does not fit the linear-combination assumption; in fact
> there are exactly 4 elements of each equivalency class of map
> positions (or 4 "worlds"):
> 
> X X X
> X   X
> X X X

Here are a few of the problems with your proposed topology:

  * DIR_REVERSE() does not work.  This is true even if you restrict
    yourself to normalised coordinates.  The minimal fix would be to
    equip DIR_REVERSE() with an additional pair of arguments saying
    which position you are going from.  Even then, you have to fix the
    places where Freeciv describes an adjacency uniquely as a pair
    (pos, dir) by insisting that dir < 4.

  * You cannot use a pair of offsets dx, dy to describe a move from a
    given tile to another.  That is, if I take a position (x, y) and
    consider (x + dx, y + dy) then the result is not independent of
    the equivalence class representative that I started with.
    Similarly for directions.

  * You can not interpret such an offset (dx, dy) as a series of dx
    moves east and dy moves south (or a combination thereof).  This
    will make your line drawing / goto code much more complicated.
    Came to think of it, the shortest path from one tile to another is
    not necessarily a straight line on your map.  (I would have said
    that it is not convex, but on a Freeciv map two points do not
    uniquely determine a line anyway).

This is just what I could come up with in 5 minutes.  Fixing this
would be a lot of work just to support a strange shape that no one
really wants to play on anyway.

For all that you have written on this subject, AFAICT you have not
even considered any of these difficulties.  Note that the second point
implies that your plan to have CHECK_POS() "repair" unnormalised
coordinates will not work reliably.

I see very no advantage in this approach versus the "arbitrary graph
which is locally grid-like" definition.  The only useful thing you can
do with unnormalised coordinates is to call normalise_map_pos() on
them at once.  If you pick your equivalence relation carefully that
might make it easier to implement square_iterate(); other than that
there are no apparent advantages.

> I have not made this assumption, and I don't know of anywhere in the
> code (outside of the GUI code, which needs fixing in any case IMO
> [3]) that does.

> [3] Actually it assumes something slightly different but related.

You seem to assume that a map can be expressed as the equivalence
classes of an equivalence relation on (a subset of) ZxZ.  You also
seem to assume that the relation is chosen to be suitably nice, but
you have not stated (clearly or otherwise) what those properties are.

So far, most of your work appears to consist in replacing fast
constructs with substantially slower ones, in the belief that the new
ones are more "general".  At the same time, you seem to have only a
vague idea about how general you want to be, what your exact
definitions are and what you are going to use that expensive
generality for.

If you just wanted to do something useful you could start by
implementing an isometric cylinder or a torus.  You could do that
without rearranging in slow, non-obvious ways just to suit your own
tastes, which is what you are currently doing.

> For instance there is code like
> 
> /* (x0, y0) is corner of drawing canvas */
> /* (width, height) are dimensions of drawing canvas */
> for (dx=0; dx<width; dx++)
>   for (dy=0; dy<height; dy++) {
>     dx2=dx, dy2=dy;
>     wrap(&dx2, &dy2);
>     draw_tile(x0+dx2, y0+dy2);
>   }
> 
> Here the code is assuming wrapping is independent from realness
> (which is true if all equivalency classes are just sets of elements
> that differ by defined linear combinations).  This is done in part
> to avoid wrap-around on the screen (though why it was done this way
> is beyond me).  If you look at this code long enough, I think you'll
> become very confused as to how it works at all.

Could you give a real example, rather than an abstracted one?

-- 
Big Gaute                               http://www.srcf.ucam.org/~gs234/
"THE LITTLE PINK FLESH SISTERS," I saw them at th' FLUORESCENT BULB
 MAKERS CONVENTION...