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

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

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.
jason

? diff
? old
? other_patches
? topology
? data/nation/old
Index: client/gui-gtk/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/mapview.c,v
retrieving revision 1.109
diff -u -r1.109 mapview.c
--- client/gui-gtk/mapview.c    2001/12/08 15:15:51     1.109
+++ client/gui-gtk/mapview.c    2001/12/13 10:15:58
@@ -594,7 +594,7 @@
 void move_unit_map_canvas(struct unit *punit, int x0, int y0, int dx, int dy)
 {
   static struct timer *anim_timer = NULL; 
-  int dest_x, dest_y;
+  int dest_x, dest_y, is_real;
 
   /* only works for adjacent-square moves */
   if ((dx < -1) || (dx > 1) || (dy < -1) || (dy > 1) ||
@@ -607,8 +607,10 @@
     update_unit_info_label(punit);
   }
 
-  dest_x = map_adjust_x(x0+dx);
-  dest_y = map_adjust_y(y0+dy);
+  dest_x = x0 + dx;
+  dest_y = y0 + dy;
+  is_real = normalize_map_pos(&dest_x, &dest_y);
+  assert(is_real);
 
   if (player_can_see_unit(game.player_ptr, punit) &&
       (tile_visible_mapcanvas(x0, y0) ||
@@ -1262,16 +1264,16 @@
     }
 
   } else { /* is_isometric */
-    int x_itr, y_itr;
+    int map_x, map_y;
     int canvas_x, canvas_y;
-
-    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; /* not adjusted;, we want to draw black tiles */
 
-       get_canvas_xy(map_x, map_y, &canvas_x, &canvas_y);
-       if (tile_visible_mapcanvas(map_x, map_y)) {
+    for (map_y=y; map_y<y+height; map_y++) {
+      for (map_x=x; map_x<x+width; map_x++) {
+       /*
+        * We don't normalize until later because we want to draw
+        * black tiles for unreal positions.
+        */
+       if (get_canvas_xy(map_x, map_y, &canvas_x, &canvas_y)) {
          pixmap_put_tile(map_canvas_store,
                          map_x, map_y,
                          canvas_x, canvas_y, 0);
Index: client/gui-mui/mapclass.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-mui/mapclass.c,v
retrieving revision 1.70
diff -u -r1.70 mapclass.c
--- client/gui-mui/mapclass.c   2001/12/08 15:15:52     1.70
+++ client/gui-mui/mapclass.c   2001/12/13 10:16:00
@@ -1436,16 +1436,19 @@
              /* A diagonal scrolling has happened */
              int newwidth = xget(o, MUIA_Map_HorizVisible);
              int newheight = xget(o, MUIA_Map_VertVisible);
-             int x_itr,y_itr,map_x,map_y,canvas_x,canvas_y;
+             int map_x,map_y;
 
              /* Draw the upper or lower complete free horiz space */
-             for (y_itr = y; y_itr < y + height; y_itr++) {
-               for (x_itr = data->horiz_first; x_itr < x + newwidth; x_itr++) {
-                 map_x = map_adjust_x(x_itr);
-                 map_y = y_itr;
- 
-                 get_canvas_xy(map_x, map_y, &canvas_x, &canvas_y);
-                 put_tile(data->map_layer->rp, 
map_x,map_y,canvas_x,canvas_y,0);
+             for (map_y = y; map_y < y + height; map_y++) {
+               for (map_x = data->horiz_first; map_x < x + newwidth; map_x++) {
+                 /*
+                  * We don't normalize until later because we want to draw
+                  * black tiles for unreal positions.
+                  */
+                 int canvas_x, canvas_y;
+                 if (get_canvas_xy(map_x, map_y, &canvas_x, &canvas_y)) {
+                   put_tile(data->map_layer->rp, 
map_x,map_y,canvas_x,canvas_y,0);
+                 }
                }
              }
 
@@ -1583,16 +1586,15 @@
        }
       } else
       {
-       int x_itr, y_itr;
-       int canvas_x, canvas_y;
-
-       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; /* not adjusted;, we want to draw black tiles */
-
-           get_canvas_xy(map_x, map_y, &canvas_x, &canvas_y);
-           if (tile_visible_mapcanvas(map_x, map_y)) {
+        int map_x, map_y;
+       for (map_y=y; map_y<y+height; map_y++) {
+         for (map_x=x; map_x<x+width; map_x++) {
+           /*
+            * We don't normalize until later because we want to draw
+            * black tiles for unreal positions.
+            */
+           int canvas_x, canvas_y;
+           if (get_canvas_xy(map_x, map_y, &canvas_x, &canvas_y)) {
              put_tile(data->map_layer->rp,map_x, map_y, canvas_x, canvas_y, 0);
            }
          }
Index: client/gui-mui/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-mui/mapview.c,v
retrieving revision 1.36
diff -u -r1.36 mapview.c
--- client/gui-mui/mapview.c    2001/12/11 16:16:33     1.36
+++ client/gui-mui/mapview.c    2001/12/13 10:16:01
@@ -443,7 +443,7 @@
 **************************************************************************/
 void move_unit_map_canvas(struct unit *punit, int x0, int y0, int dx, int dy)
 {
-  int dest_x, dest_y;
+  int dest_x, dest_y, is_real;
   /* only works for adjacent-square moves */
   if ((dx < -1) || (dx > 1) || (dy < -1) || (dy > 1) ||
       ((dx == 0) && (dy == 0))) {
@@ -455,8 +455,10 @@
     update_unit_info_label(punit);
   }
 
-  dest_x = map_adjust_x(x0+dx);
-  dest_y = map_adjust_y(y0+dy);
+  dest_x = x0 + dx;
+  dest_y = y0 + dy;
+  is_real = normalize_map_pos(&dest_x, &dest_y);
+  assert(is_real);
 
   if (player_can_see_unit(game.player_ptr, punit) &&
       (tile_visible_mapcanvas(x0, y0) ||
Index: client/gui-mui/overviewclass.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-mui/overviewclass.c,v
retrieving revision 1.14
diff -u -r1.14 overviewclass.c
--- client/gui-mui/overviewclass.c      2001/08/05 14:44:59     1.14
+++ client/gui-mui/overviewclass.c      2001/12/13 10:16:01
@@ -109,6 +109,9 @@
 
 static VOID Overview_HandleMouse(Object * o, struct Overview_Data *data, LONG 
x, LONG y)
 {
+  /* For now we only ascertain that the _center_ position is real. */
+  nearest_real_pos(&x, &y);
+
   x = ((x - _mleft(o)) % _mwidth(o))/data->ov_ScaleX;
   y = (y - _mtop(o))/data->ov_ScaleY;
 
@@ -122,13 +125,6 @@
     y -= data->rect_height / 2;
   }
 
-  x = map_adjust_x(x);
-
-  if (y < 0)
-    y = 0;
-  if (y + data->rect_height > data->ov_MapHeight)
-    y = data->ov_MapHeight - data->rect_height;
-
   if (data->rect_left != x || data->rect_top != y)
   {
     SetAttrs(o,
@@ -262,16 +258,26 @@
   {
     LONG x1,x2,y1,y2,scalex,scaley;
     BOOL twoparts;
+    int is_real;
 
     scalex = data->ov_ScaleX;
     scaley = data->ov_ScaleY;
 
-    x1 = _mleft(o) + map_adjust_x(data->rect_left) * scalex;
-    x2 = _mleft(o) + map_adjust_x(data->rect_left + data->rect_width) * scalex;
+    /* This makes little sense to me. */
+    x1 = data->rect_left;
+    y1 = data->rect_top;
+    is_real = normalize_map_pos(&x1, &y1);
+    assert(is_real);
+    x1 = _mleft(o) + x1 * scalex;
+    y1 = _mtop(o) + y1 * scaley;
+
+    x2 = data->rect_left + data->rect_width;
+    y2 = data->rect_top + data->rect_height;
+    is_real = normalize_map_pos(&x2, &y2);
+    assert(is_real);
+    x2 = _mleft(o) + x2 * scalex;
+    y2 = _mtop(o) + y2 * scaley - 1;
 
-    y1 = _mtop(o) + data->rect_top * scaley;
-    y2 = y1 + data->rect_height * scaley - 1;
-
     if (x2 < x1)
       twoparts = TRUE;
     else
@@ -510,6 +516,7 @@
       {
        /* Refresh Single */
        LONG x,y,rx1,rx2,ry1,ry2,pix_x,pix_y;
+       int is_real;
 
        x = data->x;
        y = data->y;
@@ -521,11 +528,16 @@
        RectFill(_rp(o), pix_x, pix_y, pix_x + scalex - 1, pix_y + scaley - 1);
 
         /* Check if the view rectangle has been overwritten */
-       rx1 = map_adjust_x(data->rect_left);
-       rx2 = map_adjust_x(data->rect_left + data->rect_width - 1);
-       ry1 = data->rect_top;
-       ry2 = ry1 + data->rect_height - 1;
-
+        rx1 = data->rect_left;
+        ry1 = data->rect_top;
+        is_real = normalize_map_pos(&rx1, &ry1);
+        assert(is_real);
+
+       rx2 = data->rect_left + data->rect_width - 1;
+       ry2 = data->rect_top + data->rect_height - 1;
+       is_real = normalize_map_pos(&rx2, Yry2);
+       assert(is_real);
+       
         if (((x == rx1 || x == rx2) && (y >= ry1 && y <= ry2)) ||
             ((y == ry1 || y == ry2) && (x >= rx1 && x <= rx2))) {
          Overview_DrawRect(o,data);
Index: client/gui-win32/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-win32/mapview.c,v
retrieving revision 1.15
diff -u -r1.15 mapview.c
--- client/gui-win32/mapview.c  2001/12/11 16:16:35     1.15
+++ client/gui-win32/mapview.c  2001/12/13 10:16:02
@@ -689,8 +689,11 @@
 void
 get_center_tile_mapcanvas(int *x, int *y)
 {
-  *x=map_adjust_x(map_view_x+map_view_width/2);
-  *y=map_adjust_y(map_view_y+map_view_height/2);    
+  int is_real;
+  *x=map_view_x+map_view_width/2;
+  *y=map_view_y+map_view_height/2;
+  is_real = normalize_map_pos(x, y);
+  assert(is_real);
 }
 
 /**************************************************************************
@@ -701,16 +704,17 @@
                        int write_to_screen)
 {
   if (!is_isometric) {
-    int x_itr, y_itr;
+    int map_x, map_y;
     int canvas_x,canvas_y;
     int old_x;
     old_x=-1;
-    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; /* not adjusted;, we want to draw black tiles */
-       get_canvas_xy(map_x,map_y,&canvas_x,&canvas_y);
-       if (tile_visible_mapcanvas(map_x,map_y))
+    for(map_y=y; map_y<y+height; map_y++)
+      for(map_x=x; map_x<x+width; map_x++) {
+       /*
+        * We don't normalize until later because we want to draw
+        * black tiles for unreal positions.
+        */
+       if (get_canvas_xy(map_x,map_y,&canvas_x,&canvas_y))
          {
            old_x=map_x;
            pixmap_put_tile(mapstoredc, map_x, map_y,
@@ -1037,7 +1041,7 @@
 {
 
   static struct timer *anim_timer = NULL; 
-  int dest_x, dest_y;
+  int dest_x, dest_y, is_real;
   
   
   /* only works for adjacent-square moves */
@@ -1051,9 +1055,11 @@
     update_unit_info_label(punit);
   }
   
-  dest_x = map_adjust_x(x0+dx);
-  dest_y = map_adjust_y(y0+dy);
-  
+  dest_x = x0 + dx;
+  dest_y = y0 + dy;
+  is_real = normalize_map_pos(&dest_x, &dest_y);
+  assert(is_real);
+
   if (player_can_see_unit(game.player_ptr, punit) &&
       (tile_visible_mapcanvas(x0, y0) ||
        tile_visible_mapcanvas(dest_x, dest_y))) {
Index: client/gui-xaw/mapctrl.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-xaw/mapctrl.c,v
retrieving revision 1.46
diff -u -r1.46 mapctrl.c
--- client/gui-xaw/mapctrl.c    2001/10/14 15:28:37     1.46
+++ client/gui-xaw/mapctrl.c    2001/12/13 10:16:02
@@ -238,16 +238,18 @@
 **************************************************************************/
 void mapctrl_btn_wakeup(XEvent *event)
 {
-  int xtile, ytile;
+  int map_x, map_y, is_real;
   XButtonEvent *ev=&event->xbutton;
 
   if(get_client_state()!=CLIENT_GAME_RUNNING_STATE)
     return;
 
-  xtile=map_adjust_x(map_view_x0+ev->x/NORMAL_TILE_WIDTH);
-  ytile=map_adjust_y(map_view_y0+ev->y/NORMAL_TILE_HEIGHT);
+  map_x=map_view_x0+ev->x/NORMAL_TILE_WIDTH;
+  map_y=map_view_y0+ev->y/NORMAL_TILE_HEIGHT;
+  is_real = normalize_map_pos(&map_x, &map_y);
+  assert(is_real);
 
-  wakeup_sentried_units(xtile,ytile);
+  wakeup_sentried_units(map_x,map_y);
 }
 
 /**************************************************************************
@@ -308,12 +310,13 @@
 **************************************************************************/
 void update_line(int window_x, int window_y)
 {
-  int x, y, old_x, old_y;
- 
   if ((hover_state == HOVER_GOTO || hover_state == HOVER_PATROL)
       && draw_goto_line) {
-    x = map_adjust_x(map_view_x0 + window_x/NORMAL_TILE_WIDTH);
-    y = map_adjust_y(map_view_y0 + window_y/NORMAL_TILE_HEIGHT);
+    int old_x, old_y;
+    int x = map_view_x0 + window_x/NORMAL_TILE_WIDTH;
+    int y = map_view_y0 + window_y/NORMAL_TILE_HEIGHT;
+    int is_real = normalize_map_pos(&x, &y);
+    assert(is_real);
  
     get_line_dest(&old_x, &old_y);
     if (old_x != x || old_y != y) {
@@ -358,10 +361,10 @@
   if(get_client_state()!=CLIENT_GAME_RUNNING_STATE)
     return;
 
-  map_x = ev->x / NORMAL_TILE_WIDTH;
-  map_y = ev->y / NORMAL_TILE_HEIGHT;
-  map_x = map_adjust_x(map_view_x0 + map_x);
-  map_y = map_adjust_y(map_view_y0 + map_y);
+  map_x = map_view_x0 + ev->x / NORMAL_TILE_WIDTH;
+  map_y = map_view_y0 + ev->y / NORMAL_TILE_HEIGHT;
+  is_valid = normalize_map_pos(&map_x, &map_y);
+  assert(is_valid);
 
   if (!(pcity = find_city_near_tile(map_x, map_y)))
     return;
@@ -394,15 +397,17 @@
 **************************************************************************/
 void mapctrl_key_city_workers(XEvent *event)
 {
-  int x,y;
+  int x,y, is_real;
   XButtonEvent *ev=&event->xbutton;
   struct city *pcity;
 
   if(get_client_state()!=CLIENT_GAME_RUNNING_STATE)
     return;
 
-  x=ev->x/NORMAL_TILE_WIDTH; y=ev->y/NORMAL_TILE_HEIGHT;
-  x=map_adjust_x(map_view_x0+x); y=map_adjust_y(map_view_y0+y);
+  x=map_view_x0 + ev->x/NORMAL_TILE_WIDTH;
+  y=map_view_y0 + ev->y/NORMAL_TILE_HEIGHT;
+  is_real = normalize_map_pos(&x, &y);
+  assert(is_real);
 
   pcity = find_city_near_tile(x,y);
   if(pcity==NULL) return;
Index: client/gui-xaw/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-xaw/mapview.c,v
retrieving revision 1.89
diff -u -r1.89 mapview.c
--- client/gui-xaw/mapview.c    2001/12/08 15:15:54     1.89
+++ client/gui-xaw/mapview.c    2001/12/13 10:16:03
@@ -397,7 +397,7 @@
 void move_unit_map_canvas(struct unit *punit, int x0, int y0, int dx, int dy)
 {
   static struct timer *anim_timer = NULL; 
-  int dest_x, dest_y;
+  int dest_x, dest_y, is_real;
 
   /* only works for adjacent-square moves */
   if ((dx < -1) || (dx > 1) || (dy < -1) || (dy > 1) ||
@@ -405,8 +405,10 @@
     return;
   }
 
-  dest_x = map_adjust_x(x0+dx);
-  dest_y = map_adjust_y(y0+dy);
+  dest_x = x0 + dx;
+  dest_y = y0 + dy;
+  is_real = normalize_map_pos(&dest_x, &dest_y);
+  assert(is_real);
 
   if (player_can_see_unit(game.player_ptr, punit) &&
       (tile_visible_mapcanvas(x0, y0) ||
@@ -461,8 +463,11 @@
 **************************************************************************/
 void get_center_tile_mapcanvas(int *x, int *y)
 {
-  *x = map_adjust_x(map_view_x0+map_canvas_store_twidth/2);
-  *y = map_adjust_y(map_view_y0+map_canvas_store_theight/2);
+  int is_real;
+  *x = map_view_x0+map_canvas_store_twidth/2;
+  *y = map_view_y0+map_canvas_store_theight/2;
+  is_real = normalize_map_pos(x, y);
+  assert(is_real);
 }
 
 /**************************************************************************
@@ -689,16 +694,15 @@
 void update_map_canvas(int x, int y, int width, int height, 
                       int write_to_screen)
 {
-  int x_itr, y_itr;
-  int canvas_x, canvas_y;
-
-  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);
+  int map_x, map_y, canvas_x, canvas_y;
 
-      if (tile_visible_mapcanvas(map_x, map_y)) {
+  for (map_y=y; map_y<y+height; map_y++) {
+    for (map_x=x; map_x<x+width; map_x++) {
+      /*
+       * We don't normalize until later because we want to draw
+       * black tiles for unreal positions.
+       */
+      if (get_canvas_xy(map_x, map_y, &canvas_x, &canvas_y)) {
        pixmap_put_tile(map_canvas_store, map_x, map_y,
                        canvas_x, canvas_y, 0);
       }
@@ -1104,12 +1108,11 @@
 **************************************************************************/
 void put_cross_overlay_tile(int x,int y)
 {
-  int canvas_x, canvas_y;
-  x=map_adjust_x(x);
-  y=map_adjust_y(y);
-  get_canvas_xy(x, y, &canvas_x, &canvas_y);
+  int canvas_x, canvas_y, is_real;
+  is_real = normalize_map_pos(&x, &y);
+  assert(is_real);
 
-  if (tile_visible_mapcanvas(x, y)) {
+  if (get_canvas_xy(x, y, &canvas_x, &canvas_y)) {
     pixmap_put_overlay_tile(XtWindow(map_canvas), canvas_x, canvas_y,
                            sprites.user.attention);
   }
@@ -1198,7 +1201,7 @@
 void scrollbar_scroll_callback(Widget w, XtPointer client_data,
                             XtPointer position_val)
 {
-  int position=(int)position_val;
+  int position=(int)position_val, is_real;
 
 
   if(get_client_state()!=CLIENT_GAME_RUNNING_STATE)
@@ -1216,10 +1219,9 @@
     else if(position<0 && map_view_y0>0)
       map_view_y0--;
   }
-
-  map_view_x0=map_adjust_x(map_view_x0);
-  map_view_y0=map_adjust_y(map_view_y0);
 
+  is_real = normalize_map_pos(&map_view_x0, &map_view_y0);
+  assert(is_real);
 
   update_map_canvas_visible();
   update_map_canvas_scrollbars();