[Freeciv-Dev] Re: remove map_adjust_[xy] invocations (PR#1130)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Thu, Dec 13, 2001 at 02:31:07AM -0800, jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
> The attached patch removes most of the remaining uses of map_adjust_x 
> and map_adjust_y.
> 
> 
> Many of the changes follow this form:
> 
>    x = map_adjust_x(x);
>    y = map_adjust_y(y);
> 
> -- becomes --
> 
>    is_real = normalize_map_pos(&x, &y);
>    assert(is_real);
> 
> These should be self-explanatory.  The only question, really, is whether 
> a nearest_real_map_pos is intended instead of the normalize_map_pos.  In 
> all of these cases I think it is not.
> 
> 
> Some of the changes are of this form:
> 
>    for (y_itr = y; y_itr < y + height; y_itr++)
>      for (x_itr = x; x_itr < x + width; x_itr++) {
>        int map_x = map_adjust_x(x_itr);
>        int map_y - y_itr;
>        get_canvas_xy(map_x, map_y, &canvas_x, &canvas_y);
>        if (tile_visible_mapcanvas(map_x, map_y))
>          put_tile(..., map_x, map_y, canvas_x, canvas_y);
>      }
> 
> -- becomes --
> 
>    for (map_y = y; map_y < y + height; map_y++)
>      for (map_x = x; map_x < x + width; map_x++)
>        if (get_canvas_xy(map_x, map_y, &canvas_x, &canvas_y);
>          put_tile(..., map_x, map_y, canvas_x, canvas_y);
> 
> This change is a little confusing, but correct for several reasons. 
> First off, the use of tile_visible_mapcanvas is completely spurious in 
> the first piece of code - it has the same return value as get_canvas_xy 
> (in fact for iso-view it is just a wrapper for get_canvas_xy).  Second, 
> note that if we want to draw black tiles, we can't (or shouldn't) call 
> normalize_map_pos to wrap, since it may change the x and y coordinates 
> of an unreal position.  Third, put_tile calls normalize_map_pos on its 
> own, so it's quite safe to pass in the non-normal position.  I'm sure 
> Ross would point out that this code mixes together steps that should be 
> kept separate (and he's right), but doing a full cleanup of the code is 
> really a whole separate issue.
> 
> 
> Finally, there are a few straggler changes that don't fit either of 
> these molds.  Make of them what you will - I've done my best to 
> interpret the original intention of the code, but of course any advice 
> or criticism on this is welcome.
> 
> 
> For the record, the only map_adjust_x uses left behind this patch are in 
> map_canvas_adjust_x() and mapview_common.c, both of which will be 
> handled separately.

> It really needs testing, especially under the win32 and (if such a thing 
> even compiles...) mui platforms.  If I've misjudged the intent of the 
> code, a failed assertion will result and I'll probably need to switch in 
> nearest_real_map_pos for normalize_map_pos.

Since it loooks like we can't test the mui code what do you think
about giving the mui code its own map_adjust_[xy] and remove it from
the rest of the code (which is your goal)?

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "From what I am reading Win98 and NT5.0 will be getting rid of all that
  crap anyway. Seems that Microsoft has invented something called TCP/IP and
  another really revolutionary concept called DNS that eliminates the
  netbios crap too. All that arping from browsers is going to go away.
  I also hear rumors that they are on the verge of breakthrough discoveries
  called NFS, and LPD too. Given enough time and money, they might
  eventually invent Unix."
    -- George Bonser in linux-kernel