[Freeciv-Dev] Re: (PR#6379) implement get_mapview_corners for iso-maps

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

rwetmore@xxxxxxxxxxxx wrote:
> Jason Short wrote:
> 
>>   map_to_overview_pos(&x[0], &y[0],
>>                    mapview_canvas.map_x0, mapview_canvas.map_y0);
>> 
>>-  if (is_isometric) {
>>+  /* Note: these calculations manipulate native coordinates directly. */
> 
>                                              ^^^^^^
>  > +  if (is_isometric && !topo_has_flag(TF_ISO)) {
> 
> No they don't. These are overview coordinates, no?

This code assumes the association between native and overview 
coordinates.  It manipulates the overview coordinates as if they were 
native coordinates.  Thus it manipulates native coordinates directly.

This is generally a bad thing, except that this is one of the core 
overview functions so it's okay to write implementation-specific code.

> As a general comment, the GUI gen-topology patch is really not doing a very
> good job of encapsulating the topology code. It is really just a lot of
> rearrangement of the previous isometric hacks into the baseline standard
> code at the moment. One really shouldn't see code like is_isometric and
> topo_has_flag() in general code flow, but rather only buried in gen_topo code
> routines, right?

Originally I had wanted to keep all the topology operations in 
common/map.[ch].  For the overview and some mapview operations this 
doesn't appear to be feasible; every implementation so far has 
hard-coded the topology in the overview code.  However this is still 
localized into client/mapview_common.[ch].

jason