[Freeciv-Dev] Re: topology RFC (again)

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

At 03:31 PM 01/10/29 -0500, Jason Dorje Short wrote:
>"Ross W. Wetmore" wrote:
>> 
>> At 08:38 PM 01/10/28 -0500, Jason Dorje Short wrote:
>> >"Ross W. Wetmore" wrote:
>> >So, I think you may be right that such naming is needed - either than or
>> >my functions must be renamed.
>> 
>> Probably both.
>
>Yes.  My preferred naming would be:
>
>"normal": real and lying within a region that we choose
>
>"proper": lying within the unique chosen set of class representatives
>(what you call N(0, 0)).
>
>This is really the only correct use of the word "proper", although if
>you want to have "normal" mean the unique set than we can come up with
>another name for the varying sets.

Right, ditch "proper", leave "normal" to mean the N(0,0) set which
coincides with the current code usage, and then do a better definition
of what you called normal above before you hunt for a name for it.

>Regular is not being confused with normal, except in the code (at least,
>I don't think it has been...).  I think you may have been confusing
>them, though, since you were arguing a check for normalcy was not needed
>in whole_map_iterate and elsewhere.

whole_map_iterate() iterates over the "whole map" which is the N(0,0)
set. I suspect it is you that are confusing this with some other
iterator that has to date not been required just because the form of
whole map iterate implementation is currently a rectangular iteration 
over the xsize x ysize set. The implementation should change not the
interface if new capabilities are introduced.

Any other defintion of whole map is totally inconsistent with the sorts
of things it is currently used for.

>> And note, regular in most of this discussion is being used synonymously
>> with N(0,0), or least belonging to the canonical set that one associates
>> with N(0,0) in some other coordinate set. Regular here is thus always real.
>> This concept is perhaps more useful and needs a definition if you define
>> regular as the bounding rectangle.
>
>Regular is most certainly not always real.  This is one of the problems
>the current code has.

Actually, the problem is you are taking "normal" code like iterators, 
redefining them to be "regular" and running into problems in that you
suddenly need to do all kinds of patches.

You should just leave the iterator definition to run over normal 
coordinates even if it takes code changes to the iterators to make this
happen.

If you are having problems with xsize x ysize needing to be changed
to width x height because you relabelled them as regualr and now need
new normal variables, then maybe you should backoff and consider adding
the regualr stuff as new code and leave the normal alone.

It is a lot simpler if you don't keep constantly doing cosmetic name
changes to basic elements.

It also means we don't have to keep remembering what today's meaning
of these things are. I was never in favour of Orwellian methodology.

>> >  normalize_map_pos_rect(&x, &y, x0, y0, width, height);
>> >
>> >wraps (x, y) into the specified region.  However, the specified region
>> >itself doesn't have anything to do with a particular set of class
>> >representatives (a particular N).  Some positions may have no class
>> >representative in this region, others may have multiple
>> >representatives.  In the latter case, the function must be sure to
>> >always give the same representative no matter which original member of
>> >the class it is given.  (The function might better be called
>> >wrap_map_pos_rect, but we've already decided we don't like that name
>> >:-).)
>> 
>> This function is a mishmosh of several different concepts and operations
>> that need to be separated and handled in different layers. As it stands
>> it will cause more pain and grief than benefit. Normalizing map_pos is
>> one part, but I think it is actually unnormalizing it to a canonical set
>> that has x0,y0 as origin. Clipping to gui windows is another. And the
>> fact that you needed to convert the gui window positions to a map_pos to
>> do this should tell you that this is probably not the right spot.
>
>Just like normalize_map_pos, this function does several different steps
>at once.  This makes it very easy to use.

Actually normalize_map_pos does one thing, and returns a status that
says whether it did it.

You aren't making a lot of sense.

>"Unnormalizing" to a chosen set of representatives would be nice, but we
>should not/can not define such a set just by one corder (as in N(0, 0))
>or a bounding rectangle, as many such sets may exist. 
>normalize_map_pos_iso demonstrates this by "unnormalizing" (as you call
>it) to a different set of representatives.  Your concept of an
>"unnormalize_map_pos" would not work with isometric maps, since even
>after this "unnormalizing" step further wrapping would be needed.

unnormalize_map_pos(&x, &y, x0, y0) unnormalizes the coordinates to
the particular N(*,*) that includes the point (x0, y0). If you want
to unnormalize to a different canonical set, choose a different
point to select the set.

Why would you want to "wrap" after this? the whole point was to
"unwrap" the initial (x,y) value to another cannonical set, say
the one lying under the floating GUI window so you can compute
the correct offset to display it.

Are you adding further complexities by trying to fold in iso
coordinates in addition to gui and map coordinates here? That would
certainly leave me confused and inarticulate too.

>In short, you must make a distinction between the canonical set of
>positions and the range into which you are wrapping a position. 
>unnormalize_map_pos() does not do this, and so is itself confusing
>concepts.

Unnormalize_map_pos() by trying to do one simple thing reasonably
independently from other activites, probably is a less confusing
concept to most minds :-).

>With normalize_map_pos_[rect|iso], we give a whole range of values into
>which the position can be wrapped.  This range may contain no
>representatives of the position we are wrapping (in which case we return
>0 and be done with it), it may contain one representative (in which case
>all is well), or it may contain multiple representatives.  For the
>latter case, the function must clearly define which representative will
>be used.

Very interesting, but why would you want to do such a thing? 
Transforming a coordinate into a multivalued coordinate doesn't seem 
too useful. Even you seem to think you need to fix this up if it
happens. So why even do it?

>> Ok, but it still needs the concept of the 0'th or first set in some
>> natural indexing scheme, and it still needs to guarantee that the
>> coordinates are in the first positive range to reflect the way in
>> which they are stored/accesssed :-).
>
>No.  There is only one canonical set. 

Jason, you are playing goofy word games. 

One set, right ...

> There are infinitely many
>(perhaps uncountably infinitely many) possible sets, 

As you say, there are now infinitely many sets (sigh) ...

>and trying to
>represent such a set by a single coordinate will not work. 

But labelling (we are back to) one set with a single index can't be
done ... right. Now tell me why?

> The calling
>code needs to wrap a position into a fixed area, not into a set that can
>be defined however the topology wishes to define it.

The calling code needs to unwrap a position into a particular N(*,*)
space. The correct one can be defined if you know a point and a 
relationship to that space. The typical relationship is to require
that (x,y) be a positive offset from (x0,y0) in the range 
(0:xsize,0:ysize). Note, the offset range is defined by the map
topology N, i.e. there is only one corresponding coordinate in 
each N set that matches, and there is only one N that fulfils the
condition relative to (x0,y0).

If you want to choose the range in the relationship to be an arbitary 
rectangle, then you certainly will have the chance to do everything
from missing it to hitting it multiple times. But this doesn't
correspond to any unnormalizing operation, or any other useful
operation that I can think of.

What I suspect is the case is that your arbitrary rectangle is a
GUI window, and you are trying to do a GUI filtering operation at
the same time. It might be a topology filter like an ellipse, but
that would not really be expressible with the parameters you have.
Anyway ...

It is much easier to just unnormalize which is single valued and
quite uncomplicated, then apply a filter operation to see if the 
arbitrary rectangle clips it, or whatevr shape your filter accepts
clips it.

Separating things into two independent steps makes life much less
complex. You don't miss things or get confused by cross product
interactions.

>> >> To play a little naming game to unscramble past confusion in usage. It
>> might
>> >> be easier to use some term like "canonical" in place of "normal"
above, and
>> >> reserve "normal" for the specific "regular" case of canonical.
>> >
>> >Hmmm.  I'd rather do it the other way, that is use Gaute's term
>> >"canonical" or "proper" to mean lying within N(0, 0) and use "normal"
>> >and "normalize" to refer to whatever particular set you're talking about
>> >at the time.  This seems more in line with mathematical definitions.
>> >
>> >Gaute?
>> 
>> But this is not the current concept that normalized has in the code,
>> and therefore means a massive risky and error prone cosmetic rename.
>
>Yes :-(

I'm glad you at least understand how bad your rename is. Hopefully 
in time you will overcome this peculiar desire to break code.

>> I can accept canonical as defining a more generalized representation,
>> i.e. a single valued set of which there can be more than one. I think
>> normalized readily conveys the more specific case of canonical.
>
>There can only be one canonical position.  So we need a new word.

Fine pick a word. Meanwhile I will use this word for the concept
until you come up with a renamed version.

>Overhead view requires that positions be wrapped into a rectangular
>set.  Isometric view requires that they be wrapped into an
>iso-rectangular set.

Actually, it is better to phrase this as the wrapping is into a 
particular N set, then clipped by the GUI view. Wrapping is a 
topological operation that applies to the map coordinates and
is governed by the map related wrapsizes. The rectangular set 
is really the GUI window clipping rectangle. If you just do the
clipping operations in GUI space, then you don't need to try
and figure out what an iso-rectangular form this might need.

If you do this you will keep everything straight.

>They are two different concepts here, and must be kept separate.  You
>are trying to merge them, which will only result in problems.

I think I should be saying this ... is there an echo somewhere?

>No reverse of normalize_map_pos is possible, because normalize_map_pos
>is not injective.  When the GUI code iterates over the selection of GUI
>positions, converts them to map positions, and normalizes them it
>converts a flat-rectangular set (in the case of overhead view) into an
>iso-rectangular set (in the case of an iso-rectangular map).  No
>translation of the canonical set will ever return the iso-rectangular
>set to a flat-rectangular one.

I won't even try to unravel this .. you obviously think you are 
conveying some profound understanding though.

>> You can legitimately
>> apply neither wrapping nor unwrapping operations to unreal positions
>> which is why you always need to be assured the coordinates are real
>> before you do this or the result is indeterminate/useless.
>
>Correct.

Synergy ... let me enjoy this moment.

>> It may in
>> fact do real damage like change the realness characteristic of the
>> position.
>
>Again, I assume you mean wrap_map_pos.  wrap_map_pos is only a valid
>function if the topology is defined in a linear-algebra way, i.e. how
>Gaute wants it.  If the topology does not fit this restriction, the
>function will have no validity and will most certainly change the
>realness characteristic of the position.  This is why I wish to avoid
>wrap_map_pos - to leave the option open.

Two in a row, you have to stop agreeing with me, I might come to like
it too much :-)

[...]
>> Just to push what you said a little further. I don't think GUI/citymap
>> functions should call topological functions directly. The GUI/citymap
>> coordinates need to be converted to gaming/map coordinates where the
>> topological characteristics are well defined and call the appropriate
>> map topological functions. This keeps things straight and code in one
>> localized spot where it makes sense.
>
>If you mean something like map_to_city_map and city_map_to_map, then I
>agree with you.

We are on a roll.

>> In general this is a one-way layer traversal, as in accessing tile data
>> from a position in a GUI window. The GUI window position is converted to
>> some canonical map_pos which may then need to be topologically normalized
>> to access the memory where the tile data is stored.
>
>Yes, except that it should not be called "canonical" since it is, by
>definition, not.

My word, my definition ... until you come up with a better one.

[...]
>
>> But the completely different name from these current versions makes
>> things a lot more difficult to keep straight. Some sort of convention
>> other than the longest self defining phrase should be applied to
>> maintain recognizable relationships and consistency in the sets of
>> related functions.
>
>Yes, we have had no good naming ideas.

And too many bad naming ones ...

>> Width and height are meaningless for a topology represented by polar
>> coordinates. This is enough of an example to show that they are bad
>> names. Use dx,dy or du,dv if you think x,y is too closely associated
>> with rectangular coordinates.
>
>u and v will work, but will be less readable by non-math people.  As
>long as it's not x and y, though, I'll be happy.

Maybe someone else can come up with a good general pair.

>> Simple is to keep the GUI and map stuff in separate layers, and thus you
>> can plug different map layers underneath the GUI without needing to
>> change it. One GUI clipping window routine can handle all topologies
>> if you don't first have to convert to map coordinates to do the clip.
>
>The topology layer should be kept separate from the GUI layer.  The GUI
>code should not be topology-dependent.

We have the same basic goals.

>> >> Change "normal"/"canonical" as appropriate to resolve the confusion and
>> >> implementation backwards compatibility issues.
>> >
>> >Yes, but we must decide on the naming.  Current code uses your naming
>> >system, but mine (Gaute's) is more "correct".
>> 
>> Correct is what is consistent, well-defined and implemented with
>> backwards compatibility always holding the deciding vote, i.e. practical
>> considerations.
>
>So we're agreed "normal" will keep its same meaning.

Same as in normalize_map_pos() defines normal, yes.

>> >> Another way to model this is through a strictly ordered layering ...
>> >>
>> >>   various GUI (drawing) primitives
>> >>     *** use ***
>> >>   normal_GUI_pos             - gui pos clipped to the current window
object
>> >>   GUI_pos                    - rectangular (x,y), window origin (no
wrap sets)
>> >>   <-map_to_gui_pos->         - game/map to gui coordinate transformation
>> >>   map_view_filter            - topology specific filters, e.g.
ellipse cutout
>> >>   canonical_map_pos          - map pos in a translated set, e.g. gui
origin,
>> >>   normal_map_pos             - map pos in "normal"/"regular" form
>> >>     *** refers to ***
>> >>   game memory storage
>> >>
>> >> One, does normalize or translation/transformation operations to move
from
>> >> one form to its adjacent level form.
>> >
>> >This is the same model, you've just put "gui" functions in front of the
>> >normalize_map_pos_[rect|iso] calls.  The exact same backend code will be
>> >necessary.
>> 
>> No, the ordering is actually a really important part and the fact that
>> GUI functions work only on GUI coordinates means they work with any
>> topology or underlying map_pos implementation. The glue layer between
>> the two is the (easily parameterizable) transformation step.
>
>The ordering will not change.  I'm talking about the implementation of
>map_pos_to_gui_pos.  This must use either diff_map_pos or
>normalize_map_pos_[rect|iso].

Must is a pretty strong word ... I think both of these functions are
sufficiently discredited to make dubious cnadidates for the honour.

>> I think that width, height in the above are bad for these reasons and
>> duplicate the information content in xsize,ysize.
>
>No, you have it backwards.  xsize, ysize duplicate information content
>in width, height (or usize, vsize).  It is not necessary to record them
>in the savegame.  However different topologies may have the same
>xsize/ysize - look at the thread on iso-rectangular maps a little while
>back.

Since xsize,ysize were there first, and I still use them in their 
original context, I think that the new kids width,height are the
unnecessary ones.

You should probably come up with something to justify them and explain
what they are doing for you.

>Because they are all necessary.  map.xsize and map.ysize are the least
>necessary since they can be recomputed from usize/vsize on demand, but
>it's much easier to compute them once and store them.

This I think says we really don't need them, though, except as transients.

>> Use u,v as general coordinates, and du,dv, size_u,size_v as offsets or
>> ranges or something similar.
>
>How about u_size, v_size, u_wrap, v_wrap?

Sure

>> BTW: I think you can combine shape, isometric, and the wrapping flags
>> into a single map_type index with macros/functions to return the
>> quantities in the struct. But the underlying implementation is not
>> that critical as long as one thinks of these elements as a single
>> object, and not a collection of unrelated characteristics. For example
>> all you need to save/load in savegames is the index this way. And
>> underlying code implementations can switch on this index.
>
>Yes, I agree the underlying implementation isn't too big of an issue. 
>What I do think is that it's easiest to present to the user (for setting
>server options) in this way.  Much of the code will be more readable if
>stuff is stored in this way, as well.  Ultimately, though, only the
>topology backend and communications code needs to access this, so it can
>be treated as an opaque object.

It is actually easiest to not have a collection of odd properties, but
a single identifier. That is why "flat-earth" is better than "cartesian
rectangular, standard, no_wrap_x, no_wrap_y". People will even remember 
"0". Only geeks thrive on long complex strings of weird symbols that
they somehow find self-defining or something.

>jason

Cheers,
RossW
=====