[Freeciv-Dev] Re: Client core (PR#1107)

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

Raimar Falke wrote:

> On Sat, Dec 08, 2001 at 09:57:42PM -0600, Mike Kaufman wrote:
>
> > On Sat, Dec 08, 2001 at 07:20:40PM -0800, jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
> >
> > > Paul Zastoupil wrote:
> > >
> > > When you click on a tile like that, the game tries to associate a city 
> > > with it, so it calls find_city_near_tile.  Unfortunately, 
> > > find_city_near_tile is not CHECK_MAP_POS-safe, so it'll fail the 
> > > assertion any time a wrap-around is needed to find the city (i.e. any 
> > > time you're working near x==0 or x==map.xsize-1).
> > > This is easy enough to fix.  But, it's more complicated because each of 
> > > gui-gtk, gui-xaw, and gui-mui uses its own local (static) version of the 
> > > function; this code is not common.  However, nothing in the code is 
> > > gui-specific, and in fact all three versions of the code are completely 
> > > identical.  (I'm not sure what gui-win32 does for this...maybe it's not 
> > > implemented yet.)
> > > I therefore propose that this code be unified into find_city_near_tile() 
> > > in mapctrl_common.[ch] in client/.  A patch for this will follow.
Oops.  I had this patch several days ago, but it looks like I 
accidentally mailed it to myself instead of the bug tracker :-(.
----------

The attached patch unifies the code, and places it into mapview_common.

I have also changed the code slightly, including removing a seemingly 
unnecessary check (which I changed into an assert) and expanding the 
comments slightly.
mapview_common probably isn't the right place for this, but what is? 
mapctrl_common was my first thought, but I'm not sure.
jason

? 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/12/09 03:47:32
@@ -253,3 +253,35 @@
     *map_view_topleft_map_y = new_map_view_y0;
   }
 }
+
+/**************************************************************************
+  Find the "best" city to associate with the selected tile.
+    a.  A city working the tile is the best
+    b.  If another player is working the tile, return NULL.
+    c.  If no city is working the tile, choose a city that could work the tile
+    d.  If multiple cities could work it, choose the most recently "looked at"
+    e.  If none of the cities were looked at last, choose "randomly".
+    f.  If no cities can work it, return NULL.
+**************************************************************************/
+struct city *find_city_near_tile(int x, int y)
+{
+  struct tile *ptile=map_get_tile(x, y);
+  struct city *pcity, *pcity2;
+  static struct city *last_pcity=NULL;
+
+  if((pcity=ptile->worked))  {
+    if(pcity->owner==game.player_idx)  return last_pcity=pcity;   /* rule a */
+    else return NULL;   /* rule b */
+  }
+
+  pcity2 = NULL; /* rule f */
+  city_map_checked_iterate(x, y, city_x, city_y, map_x, map_y)  {
+    pcity = map_get_city(map_x, map_y);
+    if(pcity && pcity->owner==game.player_idx)  {  /* rule c */
+      assert(get_worker_city(pcity, CITY_MAP_SIZE-1-city_x, 
CITY_MAP_SIZE-1-city_y) == C_TILE_EMPTY);
+      if(pcity==last_pcity) return pcity;  /* rule d */
+      pcity2 = pcity;
+    }
+  } city_map_checked_iterate_end;
+  return last_pcity = pcity2; /* rule e */
+}
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/12/09 03:47:32
@@ -59,5 +59,7 @@
                                int *map_view_topleft_map_y,
                                int map_view_map_width,
                                int map_view_map_height);
+                               
+struct city *find_city_near_tile(int x, int y);
 
 #endif /* FC__MAPVIEW_COMMON_H */
Index: client/gui-gtk/mapctrl.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/mapctrl.c,v
retrieving revision 1.50
diff -u -r1.50 mapctrl.c
--- client/gui-gtk/mapctrl.c    2001/12/08 15:15:51     1.50
+++ client/gui-gtk/mapctrl.c    2001/12/09 03:47:33
@@ -337,38 +337,6 @@
 }
 
 /**************************************************************************
-  Find the "best" city to associate with the selected tile.  
-    a.  A city working the tile is the best
-    b.  If no city is working the tile, choose a city that could work the tile
-    c.  If multiple cities could work it, choose the most recently "looked at"
-    d.  If none of the cities were looked at last, choose "randomly"
-    e.  If another player is working the tile, or no cities can work it,
-       return NULL
-**************************************************************************/
-static struct city *find_city_near_tile(int x, int y)
-{
-  struct tile *ptile=map_get_tile(x, y);
-  struct city *pcity, *pcity2;
-  static struct city *last_pcity=NULL;
-
-  if((pcity=ptile->worked))  {
-    if(pcity->owner==game.player_idx)  return last_pcity=pcity;   /* rule a */
-    else return NULL;   /* rule e */
-  }
-
-  pcity2 = NULL;
-  city_map_iterate(i, j)  {
-    pcity = map_get_city(x+i-2, y+j-2);
-    if(pcity && pcity->owner==game.player_idx && 
-       get_worker_city(pcity,4-i,4-j)==C_TILE_EMPTY)  {  /* rule b */
-      if(pcity==last_pcity) return pcity;  /* rule c */
-      pcity2 = pcity;
-    }
-  } city_map_iterate_end;
-  return last_pcity = pcity2;
-}
-
-/**************************************************************************
   Adjust the position of city workers from the mapcanvas
 **************************************************************************/
 gint adjust_workers(GtkWidget *widget, GdkEventButton *ev)
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/09 03:47:34
@@ -53,38 +53,6 @@
 #include "overviewclass.h"
 #include "scrollbuttonclass.h"
 
-/**************************************************************************
-  Find the "best" city to associate with the selected tile.  
-    a.  A city working the tile is the best
-    b.  If no city is working the tile, choose a city that could work the tile
-    c.  If multiple cities could work it, choose the most recently "looked at"
-    d.  If none of the cities were looked at last, choose "randomly"
-    e.  If another player is working the tile, or no cities can work it,
-       return NULL
-**************************************************************************/
-static struct city *find_city_near_tile(int x, int y)
-{
-  struct tile *ptile=map_get_tile(x, y);
-  struct city *pcity, *pcity2;
-  static struct city *last_pcity=NULL;
-
-  if((pcity=ptile->worked))  {
-    if(pcity->owner==game.player_idx)  return last_pcity=pcity;   /* rule a */
-    else return NULL;   /* rule e */
-  }
-
-  pcity2 = NULL;
-  city_map_iterate(i, j)  {
-    pcity = map_get_city(x+i-2, y+j-2);
-    if(pcity && pcity->owner==game.player_idx && 
-       get_worker_city(pcity,4-i,4-j)==C_TILE_EMPTY)  {  /* rule b */
-      if(pcity==last_pcity) return pcity;  /* rule c */
-      pcity2 = pcity;
-    }
-  } city_map_iterate_end;
-  return last_pcity = pcity2;
-}
-
 /****************************************************************
  TilePopWindow Custom Class
 *****************************************************************/
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/09 03:47:35
@@ -272,38 +272,6 @@
 }
 
 /**************************************************************************
-  Find the "best" city to associate with the selected tile.  
-    a.  A city working the tile is the best
-    b.  If no city is working the tile, choose a city that could work the tile
-    c.  If multiple cities could work it, choose the most recently "looked at"
-    d.  If none of the cities were looked at last, choose "randomly"
-    e.  If another player is working the tile, or no cities can work it,
-        return NULL
-**************************************************************************/
-static struct city *find_city_near_tile(int x, int y)
-{
-  struct tile *ptile=map_get_tile(x, y);
-  struct city *pcity, *pcity2;
-  static struct city *last_pcity=NULL;
-
-  if((pcity=ptile->worked))  {
-    if(pcity->owner==game.player_idx)  return last_pcity=pcity;   /* rule a */
-    else return NULL;    /* rule e */
-  }
-
-  pcity2 = NULL;
-  city_map_iterate(i, j)  {
-    pcity = map_get_city(x+i-2, y+j-2);
-    if(pcity && pcity->owner==game.player_idx && 
-       get_worker_city(pcity,4-i,4-j)==C_TILE_EMPTY)  {  /* rule b */
-      if(pcity==last_pcity) return pcity;  /* rule c */
-      pcity2 = pcity;
-    }
-  } city_map_iterate_end;
-  return last_pcity = pcity2;
-}
-
-/**************************************************************************
 ...
 **************************************************************************/
 void update_line(int window_x, int window_y)