[Freeciv-Dev] Re: (PR#2366) unification of tile_visible_mapcanvas

["Jason Short via RT" <rt@xxxxxxxxxxxxxx>]

rwetmore@xxxxxxxxxxxx via RT wrote:
> Don't use get_canvas_xy(). It is (or should be deprecated).
> 
> Instead take this opportunity to move to a standardized set of coordinate
> transformation functions, i.e. use map_to_canvas_pos(...) which in any
> event is more descriptive/less ambiguous than get_canvas_xy().
> 
> These functions should all (except for index) be of the form ...
>   bool <orig>_to_<dest>_pos(&ret_u, &ret_v, <const args>)
> 
> They do the 2-D transformation steps required and return a real/unreal
> status, i.e. unreal equals failure. In most cases, the return result
> will also be properly normalized, but this should not be an absolute
> condition, but part of each function's defined interface.

I agree with the sentiment.

get_canvas_xy can't currently be cleanly replaced by map_to_canvas_pos 
because there is no map_to_canvas_pos :-).  map_pos_to_canvas_pos does 
the same thing, but has a very ugly interface.

The mapview unification at least reduces the number of uses of 
get_canvas_xy, which is IMO enough for now.

Eventually I would like to rename get_canvas_xy as map_to_canvas_pos, 
and fold map_pos_to_canvas_pos into this function.  Thus in most cases 
the callers can simply be renamed.  I would accept a patch to do this, 
if you want to make one.  (The same goes for get_map_xy -> 
canvas_to_map_pos.)

jason