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

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

Here is a correction to the previous patch.

Changes include:

- The client code was cut out; that wasn't supposed to be there anyway.

- A redundante is_real_tile check was removed.  In this case I have now
simply replaced a logging-of-error with a simple assertion.  It's
cleaner but slightly less safe in non-debug mode.

Unless I'm badly mistaken this should be ready for inclusion.

jason

? rc
? profile.sh
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/05 08:25:04
@@ -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/05 08:25:05
@@ -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/05 08:25:06
@@ -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
@@ -1331,38 +1333,34 @@
   make_land();
   free(height_map);
 }
 
 /**************************************************************************
-  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;
   } 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/05 08:25:07
@@ -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.111
diff -u -u -5 -r1.111 settlers.c
--- server/settlers.c   2001/10/04 20:23:37     1.111
+++ server/settlers.c   2001/10/05 08:25:08
@@ -402,14 +402,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.139
diff -u -u -5 -r1.139 unittools.c
--- server/unittools.c  2001/10/04 20:09:30     1.139
+++ server/unittools.c  2001/10/05 08:25:11
@@ -1625,16 +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;
-  if (y < 0 || y >= map.ysize) {
-    freelog(LOG_ERROR, "Whoa!  Creating %s at illegal loc (%d, %d)",
-           get_unit_type(type)->name, x, y);
-  }
+
+  assert(is_real_tile(x, y));
+  normalize_map_pos(&x, &y); /* Should be handled by caller.  --JDS */
+  punit->x = x;
+  punit->y = y;
+
   punit->goto_dest_x=0;
   punit->goto_dest_y=0;
   
   pcity=find_city_by_id(homecity_id);
   punit->veteran=make_veteran;