[Freeciv-Dev] PATCH: remove map_adjuxt_[xy] invocations

[Jason Dorje Short <jshort@xxxxxxxxxxxxx>]

[I tried to post this to freeciv-bugs much earlier today, but with still
no success I think I've got some problem at my end.  Well, here it is.]

The attached patch removes all remaining invocations of map_adjust_[xy]
in server code.

It has a bit of clutter, though: for one thing, it adds a function
is_normal_map_pos, which may still be controversial.  I also think the
assertions used in hmap, map_get_tile, and elsewhere would be better to
put into map_inx (a question for later).

The most questionable bit of code is the usage of hmap, but using
adjc_iterate should be safe.

As before, it should be safe to patch each file independently.

jason

Index: client/control.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/control.c,v
retrieving revision 1.60
diff -u -u -5 -r1.60 control.c
--- client/control.c    2001/09/15 15:31:19     1.60
+++ client/control.c    2001/10/02 15:44:40
@@ -592,16 +592,16 @@
   a dialog. (the server code has to be there anyway as goto's are entirely
   in the server)
 **************************************************************************/
 void request_move_unit_direction(struct unit *punit, int dx, int dy)
 {
-  int dest_x, dest_y;
+  int dest_x = punit->x + dx;
+  int dest_y = punit->y + dy;
   struct unit req_unit;
 
-  dest_x = map_adjust_x(punit->x+dx);
-  dest_y = punit->y+dy; /* Not adjusted since if it needed to be adjusted it
-                          would mean that we tried to move off the map... */
+  assert(is_real_tile(dest_x, dest_y));
+  normalize_map_pos(&dest_x, &dest_y);
 
   /* Catches attempts to move off map */
   if (!is_real_tile(dest_x, dest_y))
     return;
 
@@ -1066,11 +1066,13 @@
     if(y<0 || y>=map.ysize)
       continue;
     for(x=punit->x-2; x<punit->x+3; ++x) { 
       unit_list_iterate(map_get_tile(x, y)->units, pu)
        if(unit_flag(pu, F_PARTIAL_INVIS)) {
-         refresh_tile_mapcanvas(map_adjust_x(pu->x), y, 1);
+         /* Why not just use (x, y) instead of (pu->x, pu->y)?  --JDS */
+         assert(is_normal_map_pos(pu->x, pu->y));
+         refresh_tile_mapcanvas(pu->x, pu->y, 1);
        }
       unit_list_iterate_end
     }
   }
   
Index: common/map.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.c,v
retrieving revision 1.93
diff -u -u -5 -r1.93 map.c
--- common/map.c        2001/10/01 11:08:13     1.93
+++ common/map.c        2001/10/02 15:44:45
@@ -1293,10 +1293,21 @@
 
   return normalize_map_pos(&x1, &y1);
 }
 
 /**************************************************************************
+Returns TRUE iff the map position is normal.  "Normal" here means that it
+is both a real/valid coordinate set and that the coordinates are in their
+canonical/proper form.  In plain English: the coordinates must be on the
+map.
+**************************************************************************/
+int is_normal_map_pos(int x, int y)
+{
+  return 0 <= y && y < map.ysize && 0 <= x && x < map.xsize;
+}
+
+/**************************************************************************
 Normalizes the map position. Returns TRUE if it is real, FALSE otherwise.
 **************************************************************************/
 int normalize_map_pos(int *x, int *y)
 {
   while (*x < 0)
Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.94
diff -u -u -5 -r1.94 map.h
--- common/map.h        2001/09/27 22:49:53     1.94
+++ common/map.h        2001/10/02 15:44:48
@@ -222,10 +222,11 @@
 void map_clear_special(int x, int y, enum tile_special_type spe);
 void tile_init(struct tile *ptile);
 enum known_type tile_is_known(int x, int y);
 int check_coords(int *x, int *y);
 int is_real_tile(int x, int y);
+int is_normal_map_pos(int x, int y);
 int normalize_map_pos(int *x, int *y);
 void nearest_real_pos(int *x, int *y);
 
 void rand_neighbour(int x0, int y0, int *x, int *y);
 
Index: server/mapgen.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/mapgen.c,v
retrieving revision 1.72
diff -u -u -5 -r1.72 mapgen.c
--- server/mapgen.c     2001/09/30 21:55:34     1.72
+++ server/mapgen.c     2001/10/02 15:45:03
@@ -29,11 +29,13 @@
 #include "shared.h"
 
 #include "mapgen.h"
 
 /* Wrapper for easy access.  It's a macro so it can be a lvalue. */
-#define hmap(x,y) (height_map[(y) * map.xsize + map_adjust_x(x)])
+#define hmap(x,y)                               \
+  (height_map[(assert(is_normal_map_pos(x, y)), \
+              map_inx(x, y))])
 
 static void make_huts(int number);
 static void add_specials(int prob);
 static void mapgenerator1(void);
 static void mapgenerator2(void);
@@ -799,11 +801,11 @@
   for (y=2;y<map.ysize-2;y++) {
     for (x=0;x<map.xsize;x++) {
       if (terrain_is_clean(x,y)) {
        if (map_get_terrain(x, y) != T_RIVER &&
            !(map_get_special(x, y) & S_RIVER)) {
-         map_set_terrain(map_adjust_x(x), y, T_HILLS);
+         map_set_terrain(x, y, T_HILLS);
        }
        cartesian_adjacent_iterate(x, y, x1, y1) {
          if (myrand(100) > 66 &&
              map_get_terrain(x1, y1) != T_OCEAN
              && map_get_terrain(x1, y1) != T_RIVER
@@ -1336,33 +1338,23 @@
   smooth_map should be viewed  as a  corrosion function on the map, it levels
   out the differences in the heightmap.
 **************************************************************************/
 static void smooth_map(void)
 {
-  int mx,my,px,py;
   int a;
-  
+
+  /* This overwrites itself as it runs, creating an unpredictable
+     feedback.  Is this desired?  --JDS */
   whole_map_iterate(x, y) {
-    my = map_adjust_y(y - 1);
-    py = map_adjust_y(y + 1);
-    mx = map_adjust_x(x - 1);
-    px = map_adjust_x(x + 1);
     a = hmap(x, y) * 2;
-
-    a += hmap(px, my);
-    a += hmap(mx, my);
-    a += hmap(mx, py);
-    a += hmap(px, py);
 
-    a += hmap(x, my);
-    a += hmap(mx, y);
+    adjc_iterate(x, y, x2, y2) {
+      a += hmap(x2, y2);
+    } adjc_iterate_end;
 
-    a += hmap(x, py);
-    a += hmap(px, y);
+    a += myrand(60) - 30;
 
-    a += myrand(60);
-    a -= 30;
     if (a < 0)
       a = 0;
     hmap(x, y) = a / 10;
   } whole_map_iterate_end;
 }
Index: server/maphand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/maphand.c,v
retrieving revision 1.86
diff -u -u -5 -r1.86 maphand.c
--- server/maphand.c    2001/09/12 09:12:14     1.86
+++ server/maphand.c    2001/10/02 15:45:13
@@ -769,12 +769,15 @@
 /***************************************************************
 ...
 ***************************************************************/
 int map_get_known_and_seen(int x, int y, struct player *pplayer)
 {
-  int offset = map_adjust_x(x)+map_adjust_y(y)*map.xsize;
-  int playerid=pplayer->player_no;
+  int offset, playerid=pplayer->player_no;
+
+  assert(is_real_tile(x, y));
+  normalize_map_pos(&x, &y);
+  offset = map_inx(x, y);
 
   return ((map.tiles + offset)->known) & (1u << playerid) &&
       (pplayer->private_map + offset)->seen;
 }
 
Index: server/settlers.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/settlers.c,v
retrieving revision 1.110
diff -u -u -5 -r1.110 settlers.c
--- server/settlers.c   2001/09/30 22:03:43     1.110
+++ server/settlers.c   2001/10/02 15:45:23
@@ -384,14 +384,15 @@
 /**************************************************************************
  return 1 if there is already a unit on this square or one destined for it 
  (via goto)
 **************************************************************************/
 static int is_already_assigned(struct unit *myunit, struct player *pplayer, 
int x, int y)
-{ 
-  x=map_adjust_x(x);
-  y=map_adjust_y(y);
-  if (same_pos(myunit->x, myunit->y, x, y) || 
+{
+  assert(is_real_tile(x, y));
+  normalize_map_pos(&x, &y);
+
+  if (same_pos(myunit->x, myunit->y, x, y) ||
       same_pos(myunit->goto_dest_x, myunit->goto_dest_y, x, y)) {
 /* I'm still not sure this is exactly right -- Syela */
     unit_list_iterate(map_get_tile(x, y)->units, punit)
       if (myunit==punit) continue;
       if (punit->owner!=pplayer->player_no)
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.138
diff -u -u -5 -r1.138 unittools.c
--- server/unittools.c  2001/09/15 15:31:28     1.138
+++ server/unittools.c  2001/10/02 15:45:41
@@ -1625,12 +1625,16 @@
 
   punit->type=type;
   punit->id=get_next_id_number();
   idex_register_unit(punit);
   punit->owner=pplayer->player_no;
-  punit->x = map_adjust_x(x); /* was = x, caused segfaults -- Syela */
-  punit->y=y;
+
+  assert(is_real_tile(x, y));
+  normalize_map_pos(&x, &y); /* Should be handled by caller.  --JDS */
+  punit->x = x;
+  punit->y = y;
+
   if (y < 0 || y >= map.ysize) {
     freelog(LOG_ERROR, "Whoa!  Creating %s at illegal loc (%d, %d)",
            get_unit_type(type)->name, x, y);
   }
   punit->goto_dest_x=0;