[Freeciv-Dev] PATCH: remove map_adjust_[xy] invocations from server (PR#989)

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

The attached patch should remove all remaining map_adjust_[xy]
invocations from server code.

Behavior under the patch appears to be identical - savegames are
identical.

The only thing questionable is the smooth_map() code.  This gives
identical behavior to current code, but there are several possibilities
for improvement.  I'd rather just leave this part out than argue about
it, though.

Each file should be independent, so you can just leave out part of the
patch if you wish.  However, several parts of the patch depend on
is_normal_map_pos in map.[ch] so you must include that.

Otherwise it should be ready for inclusion.

jason

? rc
? profile.sh
Index: client/control.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/control.c,v
retrieving revision 1.60
diff -u -r1.60 control.c
--- client/control.c    2001/09/15 15:31:19     1.60
+++ client/control.c    2001/10/03 22:01:14
@@ -594,12 +594,12 @@
 **************************************************************************/
 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))
@@ -1068,7 +1068,7 @@
     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);
+         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 -r1.93 map.c
--- common/map.c        2001/10/01 11:08:13     1.93
+++ common/map.c        2001/10/03 22:01:15
@@ -1295,6 +1295,17 @@
 }
 
 /**************************************************************************
+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)
Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.94
diff -u -r1.94 map.h
--- common/map.h        2001/09/27 22:49:53     1.94
+++ common/map.h        2001/10/03 22:01:15
@@ -224,6 +224,7 @@
 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);
 
Index: server/mapgen.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/mapgen.c,v
retrieving revision 1.72
diff -u -r1.72 mapgen.c
--- server/mapgen.c     2001/09/30 21:55:34     1.72
+++ server/mapgen.c     2001/10/03 22:01:17
@@ -31,7 +31,9 @@
 #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);
@@ -801,7 +803,7 @@
       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 &&
@@ -1333,34 +1335,30 @@
 }
 
 /**************************************************************************
-  smooth_map should be viewed  as a  corrosion function on the map, it levels
-  out the differences in the heightmap.
+  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 that depends on the ordering of whole_map_iterate.  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);
+    int a = hmap(x, y) * 2; /* double-count this tile */
+    int dir;
+
+    /* what about a new macro, adjc_nearest_iterate? */
+    for (dir=0; dir<8; dir++) {
+      int dx, dy, x2, y2;
+      DIRSTEP(dx, dy, dir); /* can't use MAPSTEP */
+      x2 = x + dx, y2 = y + dy;
+      nearest_real_pos(&x2, &y2);
 
-    a += hmap(x, my);
-    a += hmap(mx, y);
+      a += hmap(x2, y2);
+    }
 
-    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;
Index: server/maphand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/maphand.c,v
retrieving revision 1.86
diff -u -r1.86 maphand.c
--- server/maphand.c    2001/09/12 09:12:14     1.86
+++ server/maphand.c    2001/10/03 22:01:18
@@ -771,8 +771,11 @@
 ***************************************************************/
 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 -r1.110 settlers.c
--- server/settlers.c   2001/09/30 22:03:43     1.110
+++ server/settlers.c   2001/10/03 22:01:19
@@ -386,10 +386,11 @@
  (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)
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.138
diff -u -r1.138 unittools.c
--- server/unittools.c  2001/09/15 15:31:28     1.138
+++ server/unittools.c  2001/10/03 22:01:21
@@ -1627,8 +1627,12 @@
   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);