[Freeciv-Dev] PATCH: cityview unification (PR#1085)

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

The attached patch unifies city_get_canvas_xy/city_get_map_xy from all 
GUI's (except xaw) into one set of common functions.  I did this just 
because I've grown tired of seeing the identical code in so many places.
Some issues:

- These functions should rightfully go into a file cityview_common.[ch]. 
 However, no such file currently exists so I've placed them into 
mapview_common.[ch].  I'll fix this if desired (creating the new file).
- I don't clean up the functions any; I just copy them as-is.  I would 
like to clean them up, though (at the same time that I clean up the 
corresponding mapview functions).
- I've changed the names from city_get_[canvas|map]_xy to 
canvas_pos_to_city_pos/city_pos_to_canvas_pos.  This results in a lot of 
extra bloat to the diff file, since these name changes were made in many 
places.  If desired, I'll undo this - but the new name seems much more 
logical and matches the corresponding mapview functions.

Aside from general code cleanup, this isn't a crucial change.  It does 
make the codebase smaller and IMO more maintainable.  It will make it 
easier to supply isometric-view to the xaw client (since the xaw client 
can now use these functions).  And it will make it easier for me to 
clean up these functions to match the mapview ones (it will be necessary 
to clean up the mapview ones for the general-topologies change, and 
would be very bad IMO to not clean up this code at the same time).

All in all, this change shouldn't be very intrusive.

jason

? diff
? old
? topology
Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.2
diff -u -r1.2 mapview_common.c
--- client/mapview_common.c     2001/11/27 20:11:28     1.2
+++ client/mapview_common.c     2001/11/30 09:29:02
@@ -17,6 +17,7 @@
 
 #include <assert.h>
 
+#include "log.h"
 #include "map.h"
 #include "mapview_g.h"
 
@@ -252,4 +253,63 @@
     *map_view_topleft_map_x = new_map_view_x0;
     *map_view_topleft_map_y = new_map_view_y0;
   }
+}
+
+/**************************************************************************
+This converts a city coordinate position to citymap canvas coordinates
+(either isometric or overhead).  It should be in cityview.c instead.
+**************************************************************************/
+void city_pos_to_canvas_pos(int city_x, int city_y, int *canvas_x, int 
*canvas_y)
+{
+  if (is_isometric) {
+    int diff_xy;
+
+    /* The line at y=0 isometric has constant x+y=1(tiles) */
+    diff_xy = (city_x + city_y) - (1);
+    *canvas_y = diff_xy/2 * NORMAL_TILE_HEIGHT + (diff_xy%2) * 
(NORMAL_TILE_HEIGHT/2);
+
+    /* The line at x=0 isometric has constant x-y=-3(tiles) */
+    diff_xy = city_x - city_y;
+    *canvas_x = (diff_xy + 3) * NORMAL_TILE_WIDTH/2;
+  } else {
+    *canvas_x = city_x * NORMAL_TILE_WIDTH;
+    *canvas_y = city_y * NORMAL_TILE_HEIGHT;
+  }
+}
+
+/**************************************************************************
+This converts a citymap canvas position to a city coordinate position
+(either isometric or overhead).  It should be in cityview.c instead.
+**************************************************************************/
+void canvas_pos_to_city_pos(int canvas_x, int canvas_y, int *map_x, int *map_y)
+{
+  if (is_isometric) {
+    *map_x = -2;
+    *map_y = 2;
+
+    /* first find an equivalent position on the left side of the screen. */
+    *map_x += canvas_x / NORMAL_TILE_WIDTH;
+    *map_y -= canvas_x / NORMAL_TILE_WIDTH;
+    canvas_x %= NORMAL_TILE_WIDTH;
+
+    /* Then move op to the top corner. */
+    *map_x += canvas_y / NORMAL_TILE_HEIGHT;
+    *map_y += canvas_y / NORMAL_TILE_HEIGHT;
+    canvas_y %= NORMAL_TILE_HEIGHT;
+
+    assert(NORMAL_TILE_WIDTH == 2 * NORMAL_TILE_HEIGHT);
+    canvas_y *= 2;             /* now we have a square. */
+    if (canvas_x + canvas_y > NORMAL_TILE_WIDTH / 2)
+      (*map_x)++;
+    if (canvas_x + canvas_y > 3 * NORMAL_TILE_WIDTH / 2)
+      (*map_x)++;
+    if (canvas_x - canvas_y > NORMAL_TILE_WIDTH / 2)
+      (*map_y)--;
+    if (canvas_y - canvas_x > NORMAL_TILE_WIDTH / 2)
+      (*map_y)++;
+  } else {
+    *map_x = canvas_x / NORMAL_TILE_WIDTH;
+    *map_y = canvas_y / NORMAL_TILE_HEIGHT;
+  }
+  freelog(LOG_DEBUG, "canvas_pos_to_city_pos(pos=(%d,%d))=(%d,%d)", canvas_x, 
canvas_y, *map_x, *map_y);
 }
Index: client/mapview_common.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.h,v
retrieving revision 1.2
diff -u -r1.2 mapview_common.h
--- client/mapview_common.h     2001/11/27 20:11:28     1.2
+++ client/mapview_common.h     2001/11/30 09:29:02
@@ -60,4 +60,7 @@
                                int map_view_map_width,
                                int map_view_map_height);
 
+void city_pos_to_canvas_pos(int city_x, int city_y, int *canvas_x, int 
*canvas_y);
+void canvas_pos_to_city_pos(int canvas_x, int canvas_y, int *map_x, int 
*map_y);
+
 #endif /* FC__MAPVIEW_COMMON_H */
Index: client/gui-gtk/citydlg.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/citydlg.c,v
retrieving revision 1.102
diff -u -r1.102 citydlg.c
--- client/gui-gtk/citydlg.c    2001/11/11 11:04:36     1.102
+++ client/gui-gtk/citydlg.c    2001/11/30 09:29:04
@@ -266,10 +266,6 @@
 static void draw_map_canvas(struct city_dialog *pdialog);
 static gint city_map_canvas_expose(GtkWidget * w, GdkEventExpose * ev,
                                   gpointer data);
-static void city_get_canvas_xy(int map_x, int map_y, int *canvas_x,
-                              int *canvas_y);
-static void city_get_map_xy(int canvas_x, int canvas_y, int *map_x,
-                           int *map_y);
 
 static void buy_callback(GtkWidget * w, gpointer data);
 static gint buy_callback_delete(GtkWidget * w, GdkEvent * ev,
@@ -1778,7 +1774,7 @@
          && city_map_to_map(&map_x, &map_y, pcity, city_x, city_y)) {
        if (tile_is_known(map_x, map_y)) {
          int canvas_x, canvas_y;
-         city_get_canvas_xy(city_x, city_y, &canvas_x, &canvas_y);
+         city_pos_to_canvas_pos(city_x, city_y, &canvas_x, &canvas_y);
          put_one_tile_full(pdialog->map_canvas_store, map_x, map_y,
                            canvas_x, canvas_y, 1);
        }
@@ -1789,7 +1785,7 @@
   city_map_checked_iterate(pcity->x, pcity->y, x, y, map_x, map_y) {
     if (tile_is_known(map_x, map_y)) {
       int canvas_x, canvas_y;
-      city_get_canvas_xy(x, y, &canvas_x, &canvas_y);
+      city_pos_to_canvas_pos(x, y, &canvas_x, &canvas_y);
       if (pcity->city_map[x][y] == C_TILE_WORKER) {
        put_city_tile_output(pdialog->map_canvas_store,
                             canvas_x, canvas_y,
@@ -1808,7 +1804,7 @@
   city_map_checked_iterate(pcity->x, pcity->y, x, y, map_x, map_y) {
     if (tile_is_known(map_x, map_y)) {
       int canvas_x, canvas_y;
-      city_get_canvas_xy(x, y, &canvas_x, &canvas_y);
+      city_pos_to_canvas_pos(x, y, &canvas_x, &canvas_y);
       if (pcity->city_map[x][y] == C_TILE_UNAVAILABLE) {
        pixmap_frame_tile_red(pdialog->map_canvas_store,
                              canvas_x, canvas_y);
@@ -2858,7 +2854,7 @@
     int xtile, ytile;
     struct packet_city_request packet;
 
-    city_get_map_xy(ev->x, ev->y, &xtile, &ytile);
+    canvas_pos_to_city_pos(ev->x, ev->y, &xtile, &ytile);
     packet.city_id = pcity->id;
     packet.worker_x = xtile;
     packet.worker_y = ytile;
@@ -2913,70 +2909,6 @@
   draw_map_canvas(pdialog);
 
   return TRUE;
-}
-
-/**************************************************************************
-...
-**************************************************************************/
-static void city_get_canvas_xy(int map_x, int map_y, int *canvas_x,
-                              int *canvas_y)
-{
-  if (is_isometric) {
-    int diff_xy;
-
-    /* The line at y=0 isometric has constant x+y=1(tiles) */
-    diff_xy = (map_x + map_y) - (1);
-    *canvas_y =
-       diff_xy / 2 * NORMAL_TILE_HEIGHT +
-       (diff_xy % 2) * (NORMAL_TILE_HEIGHT / 2);
-
-    /* The line at x=0 isometric has constant x-y=-3(tiles) */
-    diff_xy = map_x - map_y;
-    *canvas_x = (diff_xy + 3) * NORMAL_TILE_WIDTH / 2;
-  } else {
-    *canvas_x = map_x * NORMAL_TILE_WIDTH;
-    *canvas_y = map_y * NORMAL_TILE_HEIGHT;
-  }
-  freelog(LOG_DEBUG, "city_get_canvas_xy(pos=(%d,%d))=(%d,%d)", map_x,
-         map_y, *canvas_x, *canvas_y);
-}
-
-/**************************************************************************
-...
-**************************************************************************/
-static void city_get_map_xy(int canvas_x, int canvas_y, int *map_x,
-                           int *map_y)
-{
-  if (is_isometric) {
-    *map_x = -2;
-    *map_y = 2;
-
-    /* first find an equivalent position on the left side of the screen. */
-    *map_x += canvas_x / NORMAL_TILE_WIDTH;
-    *map_y -= canvas_x / NORMAL_TILE_WIDTH;
-    canvas_x %= NORMAL_TILE_WIDTH;
-
-    /* Then move op to the top corner. */
-    *map_x += canvas_y / NORMAL_TILE_HEIGHT;
-    *map_y += canvas_y / NORMAL_TILE_HEIGHT;
-    canvas_y %= NORMAL_TILE_HEIGHT;
-
-    assert(NORMAL_TILE_WIDTH == 2 * NORMAL_TILE_HEIGHT);
-    canvas_y *= 2;             /* now we have a square. */
-    if (canvas_x + canvas_y > NORMAL_TILE_WIDTH / 2)
-      (*map_x)++;
-    if (canvas_x + canvas_y > 3 * NORMAL_TILE_WIDTH / 2)
-      (*map_x)++;
-    if (canvas_x - canvas_y > NORMAL_TILE_WIDTH / 2)
-      (*map_y)--;
-    if (canvas_y - canvas_x > NORMAL_TILE_WIDTH / 2)
-      (*map_y)++;
-  } else {
-    *map_x = canvas_x / NORMAL_TILE_WIDTH;
-    *map_y = canvas_y / NORMAL_TILE_HEIGHT;
-  }
-  freelog(LOG_DEBUG, "city_get_map_xy(pos=(%d,%d))=(%d,%d)", canvas_x,
-         canvas_y, *map_x, *map_y);
 }
 
 /********* Callbacks for Buy, Change, Sell, Worklist ************/
Index: client/gui-mui/mapclass.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-mui/mapclass.c,v
retrieving revision 1.68
diff -u -r1.68 mapclass.c
--- client/gui-mui/mapclass.c   2001/10/26 08:07:16     1.68
+++ client/gui-mui/mapclass.c   2001/11/30 09:29:05
@@ -2150,59 +2150,6 @@
  CityMap Custom Class
 *****************************************************************/
 
-/**************************************************************************
-...
-**************************************************************************/
-static void city_get_canvas_xy(int map_x, int map_y, int *canvas_x, int 
*canvas_y)
-{
-  if (is_isometric) {
-    int diff_xy;
-
-    /* The line at y=0 isometric has constant x+y=1(tiles) */
-    diff_xy = (map_x + map_y) - (1);
-    *canvas_y = diff_xy/2 * NORMAL_TILE_HEIGHT + (diff_xy%2) * 
(NORMAL_TILE_HEIGHT/2);
-
-    /* The line at x=0 isometric has constant x-y=-3(tiles) */
-    diff_xy = map_x - map_y;
-    *canvas_x = (diff_xy + 3) * NORMAL_TILE_WIDTH/2;
-  } else {
-    *canvas_x = map_x * NORMAL_TILE_WIDTH;
-    *canvas_y = map_y * NORMAL_TILE_HEIGHT;
-  }
-}
-
-/**************************************************************************
-...
-**************************************************************************/
-static void city_get_map_xy(int canvas_x, int canvas_y, int *map_x, int *map_y)
-{
-  if (is_isometric) {
-    *map_x = -2;
-    *map_y = 2;
-
-    /* first find an equivalent position on the left side of the screen. */
-    *map_x += canvas_x/NORMAL_TILE_WIDTH;
-    *map_y -= canvas_x/NORMAL_TILE_WIDTH;
-    canvas_x %= NORMAL_TILE_WIDTH;
-
-    /* Then move op to the top corner. */
-    *map_x += canvas_y/NORMAL_TILE_HEIGHT;
-    *map_y += canvas_y/NORMAL_TILE_HEIGHT;
-    canvas_y %= NORMAL_TILE_HEIGHT;
-
-    assert(NORMAL_TILE_WIDTH == 2*NORMAL_TILE_HEIGHT);
-    canvas_y *= 2; /* now we have a square. */
-    if (canvas_x + canvas_y > NORMAL_TILE_WIDTH/2) (*map_x)++;
-    if (canvas_x + canvas_y > 3 * NORMAL_TILE_WIDTH/2) (*map_x)++;
-    if (canvas_x - canvas_y > NORMAL_TILE_WIDTH/2)  (*map_y)--;
-    if (canvas_y - canvas_x > NORMAL_TILE_WIDTH/2)  (*map_y)++;
-  } else {
-    *map_x = canvas_x/NORMAL_TILE_WIDTH;
-    *map_y = canvas_y/NORMAL_TILE_HEIGHT;
-  }
-}
-
-
 struct MUI_CustomClass *CL_CityMap;
 
 Object *MakeCityMap(struct city *pcity)
@@ -2355,7 +2302,7 @@
       city_map_checked_iterate(pcity->x, pcity->y, x, y, map_x, map_y) {
        if (tile_is_known(map_x, map_y)) {
          int canvas_x, canvas_y;
-         city_get_canvas_xy(x, y, &canvas_x, &canvas_y);
+         city_pos_to_canvas_pos(x, y, &canvas_x, &canvas_y);
          put_one_tile_full(_rp(o), map_x, map_y, canvas_x + _mleft(o), 
canvas_y + _mtop(o), 1);
        }
       } city_map_checked_iterate_end;
@@ -2364,7 +2311,7 @@
       city_map_checked_iterate(pcity->x, pcity->y, x, y, map_x, map_y) {
        if (tile_is_known(map_x, map_y)) {
          int canvas_x, canvas_y;
-         city_get_canvas_xy(x, y, &canvas_x, &canvas_y);
+         city_pos_to_canvas_pos(x, y, &canvas_x, &canvas_y);
          if (pcity->city_map[x][y]==C_TILE_WORKER) {
            put_city_output_tile(_rp(o),
                             city_get_food_tile(x, y, pcity),
@@ -2383,7 +2330,7 @@
        if (tile_is_known(map_x, map_y))
        {
          int canvas_x, canvas_y;
-         city_get_canvas_xy(x, y, &canvas_x, &canvas_y);
+         city_pos_to_canvas_pos(x, y, &canvas_x, &canvas_y);
 
           canvas_x += _mleft(o);
           canvas_y += _mtop(o);
@@ -2470,7 +2417,7 @@
        if (_isinobject(msg->imsg->MouseX, msg->imsg->MouseY))
        {
          int x,y;
-         city_get_map_xy(msg->imsg->MouseX - _mleft(o), msg->imsg->MouseY - 
_mtop(o), &x, &y);
+         canvas_pos_to_city_pos(msg->imsg->MouseX - _mleft(o), 
msg->imsg->MouseY - _mtop(o), &x, &y);
          data->click.x = x;
          data->click.y = y;
          set(o, MUIA_CityMap_Click, &data->click);
Index: client/gui-win32/citydlg.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-win32/citydlg.c,v
retrieving revision 1.5
diff -u -r1.5 citydlg.c
--- client/gui-win32/citydlg.c  2001/11/07 19:16:12     1.5
+++ client/gui-win32/citydlg.c  2001/11/30 09:29:06
@@ -449,59 +449,6 @@
   /* FIXME Worklists */
 }
 
-/**************************************************************************
-...
-**************************************************************************/
-static void city_get_canvas_xy(int map_x, int map_y, int *canvas_x, int 
*canvas_y)
-{
-  if (is_isometric) {
-    int diff_xy;
-
-    /* The line at y=0 isometric has constant x+y=1(tiles) */
-    diff_xy = (map_x + map_y) - (1);
-    *canvas_y = diff_xy/2 * NORMAL_TILE_HEIGHT + (diff_xy%2) * 
(NORMAL_TILE_HEIGHT/2);
-    
-    /* The line at x=0 isometric has constant x-y=-3(tiles) */
-    diff_xy = map_x - map_y;
-    *canvas_x = (diff_xy + 3) * NORMAL_TILE_WIDTH/2;
-  } else {
-    *canvas_x = map_x * NORMAL_TILE_WIDTH;
-    *canvas_y = map_y * NORMAL_TILE_HEIGHT;
-  }
-}
-
-/**************************************************************************
-...
-**************************************************************************/
-static void city_get_map_xy(int canvas_x, int canvas_y, int *map_x, int *map_y)
-{
-  if (is_isometric) {
-    *map_x = -2;
-    *map_y = 2;
-
-    /* first find an equivalent position on the left side of the screen. */
-    *map_x += canvas_x/NORMAL_TILE_WIDTH;
-    *map_y -= canvas_x/NORMAL_TILE_WIDTH;
-    canvas_x %= NORMAL_TILE_WIDTH;
-
-    /* Then move op to the top corner. */
-    *map_x += canvas_y/NORMAL_TILE_HEIGHT;
-    *map_y += canvas_y/NORMAL_TILE_HEIGHT;
-    canvas_y %= NORMAL_TILE_HEIGHT;
-
-    assert(NORMAL_TILE_WIDTH == 2*NORMAL_TILE_HEIGHT);
-    canvas_y *= 2; /* now we have a square. */
-    if (canvas_x + canvas_y > NORMAL_TILE_WIDTH/2) (*map_x)++;
-    if (canvas_x + canvas_y > 3 * NORMAL_TILE_WIDTH/2) (*map_x)++;
-    if (canvas_x - canvas_y > NORMAL_TILE_WIDTH/2)  (*map_y)--;
-    if (canvas_y - canvas_x > NORMAL_TILE_WIDTH/2)  (*map_y)++;
-  } else {
-    *map_x = canvas_x/NORMAL_TILE_WIDTH;
-    *map_y = canvas_y/NORMAL_TILE_HEIGHT;
-  }
-}
-
-
 /****************************************************************
 Isometric.
 *****************************************************************/
@@ -519,7 +466,7 @@
           && city_map_to_map(&map_x, &map_y, pcity, city_x, city_y)) {
         if (tile_is_known(map_x, map_y)) {
           int canvas_x, canvas_y;
-          city_get_canvas_xy(city_x, city_y, &canvas_x, &canvas_y);
+          city_pos_to_canvas_pos(city_x, city_y, &canvas_x, &canvas_y);
           put_one_tile_full(hdc, map_x, map_y,
                             canvas_x, canvas_y, 1);
         }
@@ -530,7 +477,7 @@
   city_map_checked_iterate(pcity->x, pcity->y, x, y, map_x, map_y) {
     if (tile_is_known(map_x, map_y)) {
       int canvas_x, canvas_y;
-      city_get_canvas_xy(x, y, &canvas_x, &canvas_y);
+      city_pos_to_canvas_pos(x, y, &canvas_x, &canvas_y);
       if (pcity->city_map[x][y]==C_TILE_WORKER) {
         put_city_tile_output(hdc,
                              canvas_x, canvas_y,
@@ -548,7 +495,7 @@
   city_map_checked_iterate(pcity->x, pcity->y, x, y, map_x, map_y) {
     if (tile_is_known(map_x, map_y)) {
       int canvas_x, canvas_y;
-      city_get_canvas_xy(x, y, &canvas_x, &canvas_y);
+      city_pos_to_canvas_pos(x, y, &canvas_x, &canvas_y);
       if (pcity->city_map[x][y]==C_TILE_UNAVAILABLE) {
         pixmap_frame_tile_red(hdc,
                              canvas_x, canvas_y);
@@ -1784,7 +1731,7 @@
          int tile_x,tile_y;
          xr=x-pdialog->map_x;
          yr=y-pdialog->map_y;
-         city_get_map_xy(xr,yr,&tile_x,&tile_y);
+         canvas_pos_to_city_pos(xr,yr,&tile_x,&tile_y);
          city_dlg_click_map(pdialog,tile_x,tile_y);
        }
     }
Index: client/include/citydlg_g.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/include/citydlg_g.h,v
retrieving revision 1.3
diff -u -r1.3 citydlg_g.h
--- client/include/citydlg_g.h  2001/06/25 18:15:03     1.3
+++ client/include/citydlg_g.h  2001/11/30 09:29:07
@@ -13,6 +13,10 @@
 #ifndef FC__CITYDLG_G_H
 #define FC__CITYDLG_G_H
 
+/* Some of the city coordinate conversion functions
+    are located in mapview_common. */
+#include "mapview_common.h"
+
 struct unit;
 struct city;