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

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

Luckily, we are in violent agreement once we learn to speak the same
language. But the difference in cultural underpinings are still cause
for jihad - your infidel heresies are an affront to any pragmatic
programmer :-).

On with the war ... :-)

At 03:05 AM 01/10/15 -0400, Jason Dorje Short wrote:
>"Ross W. Wetmore" wrote:
>> At 01:01 AM 01/10/13 -0400, Jason Dorje Short wrote:
>> >"Ross W. Wetmore" wrote:
>> >> At 11:39 AM 01/10/10 -0400, Jason Dorje Short wrote:
>> >> >Raimar Falke wrote:
>> >> >wrap_map_pos(&x, &y) takes the vector (x, y) and physically wraps it
>> >> >into its proper position.  It is independent of (x, y) being real,
so it
>>
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> >> >will only work for topologies that wrap "regularly".  However, the fact
>>    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> >> >that it'll work on any position or vector makes usage much simpler.
>> >> >
>> >> >For a real position, it will have identical effect to
>> >> >normalize_map_pos.  For an unreal position, it will wrap the coordinate
>> >>                                                      ^^^^^^^^^^^^^^^^^^^
>> >> >normally just as if it were real (which is only possible under a
limited
>> >>  ^^^^^^^^  i.e. do bogus transformations of gaming coordinates.
>> >
>> >The transormation is not "bogus" if we define topologies as Gaute has
>> >proposed; i.e. such that equivalence classes of coordinates are defined
>> >as differing by a linear combination of given vectors.  It's a bit of a
>> >tautology, but if we are willing to limit our topologies in this way we
>> >can make the conversion a lot easier.
>> 
>> Unfortunately this does not work for the standard cylinder map. And it
>> does not allow you to decouple the wrapping in x and y to handle the
>> rectangular 4 set trivially.
>
>Yes, it does.
>
>For a theoretical argument, see:
>http://arch.freeciv.org/freeciv-dev-200110/msg00385.html.  I'll also
>explain it (hopefully) more understandably below.
>
>For a practical example, check out this code:
>
>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;
>  }
>}

Here's a slightly better version to compare to. It at least doesn't
have the Lvalue bugs. :-)

/*
 * _map_normalize() only updates wrapped coordinates and always returns TRUE.
 * It is a utility macro to simplify normalize_map_pos() and is not intended
 * for general use. If one does not care about partial coordinate updates,
 * there may be some savings in redundant checks between is_real_tile() and
 * _map_normalize() in a reimplemented normalize_map_pos().
 */
#define _map_normalize(WRAPTYPE,BASE,PPOS)
  \
( !has_mapwrap(WRAPTYPE)
  \
  || RANGE_CHECK_0((BASE),*(PPOS))
  \
  || (*(PPOS)= ( (*(PPOS))%(BASE) < 0
  \
               ? (*(PPOS))%(BASE)+(BASE) : (*(PPOS))%(BASE) ), TRUE))

#define normalize_map_pos(px,py)
  \
( is_real_tile(*(px),*(py))
  \
  && _map_normalize(MAP_TYPE_WRAPX,map.xsize,(px))
  \
  && _map_normalize(MAP_TYPE_WRAPY,map.ysize,(py)) )
   

>> You *cannot* wrap to a "normal" position first, then start thinking about
>> what restrictions you might want to put on. That is the reason why Gaute
>> is wrong, and why the initial Freeciv hacks to deal with unscrambling
>> things after the eggs were turned into omelet were so endemic and
>> un-generalizable.

You have implemented "normalize_map_pos()" in the generic way. This is
fine. BTW: normalize_map_pos() is the correct generic name for this
even if the current topologies are a restricted implementation.

Your function is badly misnamed, as it applies a normalizing operation
to each of its coordinate arguments. In one case maybe a wrap in the 
other maybe not. It does not apply a wrapping operation to each 
coordinate as you and the name claim.

Your explanation and documentation is also misleading as is all the
lengthy high-fallutin' sounding math stuff. 

Note: "wrapping" is a topological operation in these cases. As such 
it should not appear in something you want to be a topologically 
independet interface, only in the implementation as needed.

We just need to get these definitions straight in spite of the language
barrier.

>No, you just don't understand the algebra (Gaute's not very good at
>explaining it, for one thing).  It's quite applicable, I assure you.

FYI: My PhD is in Quantum Chemistry, and my thesis in part was about 
     recursive calculations of spin integrals based on the application 
     of Lie Algebras to the Heisenberg Equations.

You might just possibly be mistaken in your assumptions, no?

A theoretical possibility of course ...

[...]
>The main advantage of Gaute's system is that it allows use to separate
>wrapping from realness.  Even an unreal tile can have a "proper"
>position that is the wrapped version of the coordinate.  

Yes, but only under a bogus transformation that does not conform to
the realities of Freeciv, sigh. It does not really matter whether this
is a nearest_real_pos() or a wrap_to_real_pos(), it is still a bogus
transformation. (Damn, do I wish I could flip Gaute's mental bit here. 
Such blind persistence is a real asset if one could keep him pointed
in the right direction and an impossible obstacle when he is not :-).

This is where you and Gaute scramble your omelets. Stick to applying
reality to the game and your good coding patches, and not mathematical 
obfustication :-).

>From a practical perspective, though, the wrap_map_pos() function is
>extremely useful.  

Luckily for you the code above doesn't do the unreal transformations
you are so fond of. So we are still ok - except for the misnaming.

And the the code below should have already been converted to use the 
more general normalize_map_pos(). If you do it with a new name you 
are just guilty of obfustication of the code, rather than gross
incompetency :-).

I am still unhappy with your not doing the is_real_tile() check up
front, because you will half transform some coordinates, and I really
think that this confuses the situation even further.

>Since things are already implemented in this sort of
>way, it makes converting the client code to the new system much easier. 
>Code that would require major restructuring to work with juse
>normalize_map_pos() and the like can be fixed much more easily with
>wrap_map_pos().  For instance, the following code
>
>static int get_canvas_xy(int map_x, int map_y, int *canvas_x, int
>*canvas_y)
>{
>  if (map_view_x0+map_canvas_store_twidth <= map.xsize)
>    *canvas_x = map_x-map_view_x0;
>  else if(map_x >= map_view_x0)
>    *canvas_x = map_x-map_view_x0;
>  else if(map_x < map_adjust_x(map_view_x0+map_canvas_store_twidth))
>    *canvas_x = map_x+map.xsize-map_view_x0;
>  else *canvas_x = -1;
>
>  *canvas_y = map_y - map_view_y0;
>
>  *canvas_x *= NORMAL_TILE_WIDTH;
>  *canvas_y *= NORMAL_TILE_HEIGHT;
>
>  return *canvas_x >= 0
>      && *canvas_x < map_canvas_store_twidth * NORMAL_TILE_WIDTH
>      && *canvas_y >= 0
>      && *canvas_y < map_canvas_store_theight * NORMAL_TILE_HEIGHT;
>}
>
>cannot be written in terms of normalize_map_pos().  However, it can
>easily be written in terms of wrap_map_pos:
>
>static int get_canvas(xy(int map_x, int map_y, int *canvas_x, int
>*canvas_y)
>{
>  map_x -= map_view_x0;
>  map_y -= map_view_y0;
>  wrap_map_pos(&map_x, &map_y);
>  *canvas_x = map_x * NORMAL_TILE_WIDTH;
>  *canvas_y = map_y * NORMAL_TILE_WIDTH;
>  return map_x >= 0
>        && map_x < map_canvas_store_twidth
>        && map_y >= 0
>        && map_y < map_canvas_store_theight;
>}
>
>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). 

Take a lot at any version of the corecleanups.

I sometimes forget that you are still in the dark ages and haven't
learned to read yet.

> But wrap_map_pos will handle it just
>fine by wrapping it in only the correct (valid) directions.

As you say, it will correctly perform the required normalizing
condition to each coordinate based on the topology selected. 
Unfortunately (or fortunately), this is not always a wrap operation.

>> Vague mathematical terms just means it is a great theory, but has no
>> immediate practical applicability. I have refereed enough papers for
>> scientific journals to recognize the argument and discussion technique :-).
>
>I hope I have given a valid practical reason as well.

Luckily your practical is ok, as you haven't learned to apply your
theory as stated in this case. The two wrongs thankfully cancel :-).

>> The GUI code is a separate issue. It deals with a very specific flat-earth
>> type window object that can be thought of as floating over a game map.
>> And in this case the GUI coordinates are both clipped equivalently.
>> 
>> Note that while the game coordinates may wrap to give you the effect
>> of continuous scrolling in a given direction, the GUI ones never do.
>> 
>> The issue of transforming one set of coordinates to the other is not a
>> "generalized coordinate system" problem. But it is the GUI problem.
>
>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).  

Do you have reading dyslexia? 

>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].
>
>If you don't believe me, please prove me wrong by fixing the GUI to work
>with your system.  You may use only existing topology functions
>(normalize_map_pos, is_real_tile, nearest_real_pos, and
>is_normal_map_pos).

Since these functions are used to manipulate game coordinates and not
GUI window coordinates, You clearly don't yet recognize that other
functions like clipping are needed to deal with GUI window coordinates.

Either that or you are subtly setting the conditions for failure, but
I haven't seen any evidence of subtle capability yet :-).

This is one of the reasons why your claims of understanding the GUI
make me ask that you at least get someone knowledgable to second your
patches.

BTW: the GUI *is* fixed in my system (in this respect) and has been for
a couple or three months. The solution required map_wrap and map_clip
functions for specialized GUI code to replace map_adjust calls. These 
are in map.h. They are left as specialized functions because the GUI is 
not part of the generalized coordinate system in the corecleanups.

>> The game map coordinates are something one would like a general system
>> for, but it is very unlikely that the GUI and game systems will ever
>> be one and the same, so there are always going to be transformations
>> using techniques like clipping and wrapping for the specialized
>> transformation code.
>> 
>> And until you and Raimar, really internalize the concept that the GUI
>> coordinates and the game coordinates are two separate systems dealing
>> with two fundamentally different objects, you are going to continue to
>> confuse and amalgamate the two into a very broken Freeciv omelet.
>
>Like I said before, the omelete is there.  We can either deal with it as
>it is or unscramble it.  But right now, the GUI coordinates are NOT
>separate the way you claim they are; they are quite intertwined with the
>map coordinates.

Technically the GUI coordinate handling was completely separate until you 
and Raimar munged them together into the current omelet, not that I need
to remind you of this or my comments at the time :-)

The fact that Freeciv GUI requires as much work to clean it up or maybe
moreso should not be used as proof that broken systems are the ideal
model solution to follow. And no, I am not interested in a theoretical
analysis of why this is provably the case with no more than a few niggling 
assumptions.

The client needs to access game coordinates to get its data. It needs
to put the data into a fixed window object by applying an appropriate
transformation. The requirements are actually quite simple.

>But, we both agree the current system is broken as far as general
>topologies go.

Yes. The current system *is* broken :-). 

Let's pause for a minute to reflect on this moment of shared truth ...

>> >And any such
>> >"specialized code", no matter how little it is used, must go into
>> >map.[ch] so that it can be easily adjusted to handle alternate
>> >topologies (case in point: nearest_real_pos).
>> 
>> Specialized code goes where specialized code needs to go. By definition
>> it is not general and so your concern for trying to make it fit
>> general topologies is admirable but misplaced.
>> 
>> To make this easier for you to swallow, specialized code might be a
>> "subclass" that handled a particular specific topology. Only the class
>> interface needs to be in map.h.
>
>This would make the job much, much harder.  There is so much GUI code
>that changing it for every topology is not practical.  Additionally, I
>believe it is fundamentally very, very flawed.

You have not yet really internalized the right model.

The GUI code that draws into GUI windows is almost certainly going to
be 2-D cartesian arrays of pixels. I really can't see this ever changing
unless you rewrite GTK or XAW.

The game coordinates may change in many weird and wonderful ways in
the future. But there will be a small set of interface elements like
whole_map_iterate, normalize_map_pos() or map_step() to navigate it.

The only thing you need to supply for each new gaming coordinate
system is the transformation from game coordinate to window coordinate
or vice versa (it depends on whether you are filling a GUI window or
updating a game object) and the set of pixmaps you want to draw there.

For the set of rectangular topologies, there is a lot of shared code
in setting up the pixmap elements etc. It also unfortunately overlaps
a lot with rectangularly based GUI coordinates so the separation is
not as obvious as one would like. And adding in a GUI cache object 
obscures this even further as the cache tends to have some gaming 
coordinate features like wrapping so there are two half transformations
really involved for "optimization". But life is never simple ...

However if you really think about isometric, you may find that the 
correct GUI interfaces you need to use to handle it are not that hidden, 
or different from the rectangular as expected, even if there is a lot of 
special code that implements the transformations or object setup slightly 
differently.

>> You are getting there. I don't mean to dampen your enthusiasm, but you
>> aren't there yet. And I am sure you will in a couple more iterations.
>
>Right now we are making straightforward changes.  But as I said before,
>we will soon hit a point with the GUI code where we will have to decide
>what to do.

I'm actually torn by this comment ... 

You really need to figure out what to do, before you start to do it.

But on the otherhand even breaking the code may be a vehicle to get
there. 

I just never really had a use for the anarchist solution before Freeciv.

>jason

Cheers,
RossW
=====