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

[Jason Dorje Short <vze2zq63@xxxxxxxxxxx>]

Gaute B Strokkenes wrote:
> 

> The current underlying assumption is that two coordinate pairs are
> considered to be the same if and only if their difference (as vectors
> of the Z-module ZxZ) can be expressed as a linear combination of a
> given set of vectors.

Assumption?  Where?  I know you are assuming this, but can you please
point out where the code uses this?

I know we do make assumptions about the continuity of the map; for
instance I think a goto from (x1,y1) to (x2,y2) will just set up a
direct path and assume everything in between it is valid.  This is
hardly a fundamental assumption.

> Currently, that set is {(map.xsize, 0)}.  If we
> implement a rectangular topology that becomes the empty set.  For a
> torus it would be {(map.xsize, 0), (0, map.ysize)}, and if we implment
> an isometric-view cylinder it would be something like {(map.xsize,
> map.ysize)} (I didn't check that, but you get the idea.)

Correct.  An isometric-view torus would have {(map.xsize,map.ysize),
(map.xsize,-map.ysize)}.

Again, I don't see that this is a fundamental assumption in the code
(though I could be wrong) - but you are certainly trying to make it one.

> Thus if (x, y) refers to a real tile, so must any other coordinate
> pair that you arrive at by repeatedly adding or subtracting vectors
> from that set.  Corrollary: if (x, y) does _not_ refer to a real tile,
> neither does any other coordinate pair obtained by adding or
> subtracting vectors from the appropriate set, since otherwise you
> would be violating the above.
> 
> In short: under the current regime, you will never be able to break
> anything by fiddling with unreal coordinates in this manner.

Please explain.

> The topology that you have above does not satisfy these conditions.
> That doesn't necessarily make it bad, but if you wish to implement it
> you will need to change everything that depends on this assumption.
> Amongst other things, that means removing normalize_map_pos() and all
> calls to it, so it is a bit backwards to use this as an argument to
> use or change normalize_map_pos().

Huh?

> A good first step would be to convert as many places as possible to
> use MAPSTEP() rather than manually twiddling coordinate pairs and
> relying on normalize_map_pos() to clean up the mess.  MAPSTEP() would
> still work under such a topology because it works by applying a
> direction to a point.  (Currently it uses normalize_map_pos()
> internally, but the point is that it doesn't have to.)

I agree; MAPSTEP conversions are in progress.
 
> Another good thing would be to make sure that as many places as
> possible create normalized coordinates at the outset.  That would also
> enable you to ditch a lot of normalize_map_pos() calls and replacing
> them with some sort of assert.

I agree with this as well, though since it's a global project I haven't
done much on it.

> > I say again: any time you are wrapping an unreal coordinate, or
> > doing anything at all to it for that matter outside of converting it
> > to a real coordinate, you are doing something fundamentally,
> > theoretically wrong.
> 
> I'm afraid that it is your reasoning that is fundamentally,
> theoretically wrong.

Naturally, I disagree.

> I'm not entirely sure what you mean by "converting it to a real
> coordinate".  I assume you mean something like map_adjust_y() or
> nearest_real_pos().  If you wish to take most abstract view possible,
> this is more wrong than playing games with unreal coordinates, since
> there is no particular guarantee that there is a well-defined nearest
> real tile in an arbitrarily nasty topology.

I mean nearest_real_pos().  This conversion is sometimes necessary, and
it's okay for it to be a bit fuzzy.  For instance, the client uses it to
convert out-of-bounds clicks to clicks on real tiles.

My underlying assumption is that every topology we implement will map
cleanly onto a plane.  Given that, there will always be at least one
nearest_real_pos.  There may be more than one (as in my example from the
previous post).

> > Give me enough chances, and I'll find a topology that proves it.
> 
> I consider that unlikely, given my above proof.

Proof?

> But really, I think all this talk about supporting arbitrarily strange
> topologies really has that much value.  The only topologies that I
> think we really want to support are the rectangular, cylindrical and
> isometric-view cylindrical variants.  Additionally, a torus might be
> so easy to do that there may well be no real reason not to.  But
> everything else, including the stuff that I have mentioned from time
> to time (Möbius strip, Klein bottle, RP^2 and so on) really strikes me
> as having little value beyond the gee-whizz factor.

Correct, but I believe once we have a good implementation of the
topology routines, these will fall into place - not that most of them
ever need to make it into the real distribution.

>  Also, since the
> greater part of the difficulty would be the creation of client support
> (you need to think about how you're going to draw them on the screen
> and so on) which I don't see anyone leaping up and down with eagerness
> to do, (though I do see people who would be willing to make the [mostly
> straightforward] corresponding changes to the core).  Nor do I think
> we wish to make the client stuff any more complicated than it already
> is just to support this.

This has been true so far.  I have been thinking about client changes,
but I've been doing the server changes first (you can always run the
server in all-AI mode to test things out; trying to debug the client
without a working server would be a nightmare).  One particular ugliness
is that each client has separate code and I can't compile all of it.

On a side note, I think the client stuff will become less complicated
since all of the "hard math" [1] will be moved into the topology
routines (except for the isometric stuff which is much harder than the
topology stuff...).

[1] Hard not from a mathematical perspective but from the coder's point
of view.

jason