[Freeciv-Dev] PATCH: check_map_pos

[Jason Dorje Short <vze2zq63@xxxxxxxxxxx>]

The attached patch introduces a check_map_pos() function.

It's done as an inline function within map.h, but this is unimportant at
this point.  As said before, the advantage of a function is that it can
be used within map_inx if desired; there's not really any advantage to a
macro.

It does not touch client code.  With all the different GUI's and the
tendancy to pass around coordinates picked by the user, the client is
not yet ready for this function.

I have run several full (endyear=2000) autogames, and have not had any
problems yet.

Note that check_map_pos is not called on coordinates that are simply
passed off to another function; for instance most of the code in map.c
just calls MAP_TILE or map_inx with the coordinates so it's not
necessary to call check_map_pos directly.

jason

? rc
? old
? topology
? mydiff
? check_map_pos
? client/mapview_common.c
? client/mapview_common.h
Index: common/map.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.c,v
retrieving revision 1.96
diff -u -r1.96 map.c
--- common/map.c        2001/10/11 12:37:04     1.96
+++ common/map.c        2001/10/15 08:26:52
@@ -1068,8 +1068,7 @@
   int maxcost = 72; /* should be big enough without being TOO big */
   struct tile *tile0, *tile1;
 
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
+  check_map_pos(&x, &y);
 
   tile0 = map_get_tile(x, y);
   debug_log_move_costs("Resetting move costs for", x, y, tile0);
@@ -1156,10 +1155,6 @@
 ***************************************************************/
 struct tile *map_get_tile(int x, int y)
 {
-  int is_real = normalize_map_pos(&x, &y);
-
-  assert(is_real);
-
   return MAP_TILE(x, y);
 }
 
@@ -1168,10 +1163,7 @@
 ***************************************************************/
 signed short map_get_continent(int x, int y)
 {
-  if (!normalize_map_pos(&x, &y))
-    return -1;
-  else
-    return MAP_TILE(x, y)->continent;
+  return MAP_TILE(x, y)->continent;
 }
 
 /***************************************************************
@@ -1179,8 +1171,6 @@
 ***************************************************************/
 void map_set_continent(int x, int y, int val)
 {
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
   MAP_TILE(x, y)->continent = val;
 }
 
@@ -1190,10 +1180,7 @@
 ***************************************************************/
 enum tile_terrain_type map_get_terrain(int x, int y)
 {
-  if (!normalize_map_pos(&x, &y))
-    return T_UNKNOWN;
-  else
-    return MAP_TILE(x, y)->terrain;
+  return MAP_TILE(x, y)->terrain;
 }
 
 /***************************************************************
@@ -1201,10 +1188,7 @@
 ***************************************************************/
 enum tile_special_type map_get_special(int x, int y)
 {
-  if (!normalize_map_pos(&x, &y))
-    return S_NO_SPECIAL;
-  else
-    return MAP_TILE(x, y)->special;
+  return MAP_TILE(x, y)->special;
 }
 
 /***************************************************************
@@ -1212,8 +1196,6 @@
 ***************************************************************/
 void map_set_terrain(int x, int y, enum tile_terrain_type ter)
 {
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
   MAP_TILE(x, y)->terrain = ter;
 }
 
@@ -1222,9 +1204,6 @@
 ***************************************************************/
 void map_set_special(int x, int y, enum tile_special_type spe)
 {
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
-
   MAP_TILE(x, y)->special |= spe;
 
   if (spe & (S_ROAD | S_RAILROAD))
@@ -1236,8 +1215,6 @@
 ***************************************************************/
 void map_clear_special(int x, int y, enum tile_special_type spe)
 {
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
   MAP_TILE(x, y)->special &= ~spe;
 
   if (spe & (S_ROAD | S_RAILROAD))
@@ -1249,8 +1226,6 @@
 ***************************************************************/
 struct city *map_get_city(int x, int y)
 {
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
   return MAP_TILE(x, y)->city;
 }
 
@@ -1260,8 +1235,6 @@
 ***************************************************************/
 void map_set_city(int x, int y, struct city *pcity)
 {
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
   MAP_TILE(x, y)->city = pcity;
 }
 
@@ -1271,10 +1244,7 @@
 ***************************************************************/
 enum known_type tile_is_known(int x, int y)
 {
-  if (!normalize_map_pos(&x, &y))
-    return TILE_UNKNOWN;
-  else
-    return (enum known_type) (MAP_TILE(x, y)->known);
+  return (enum known_type) (MAP_TILE(x, y)->known);
 }
 
 /***************************************************************
@@ -1283,14 +1253,13 @@
 ***************************************************************/
 int same_pos(int x1, int y1, int x2, int y2)
 {
-  assert(is_real_tile(x1, y1) && is_real_tile(x2, y2));
-  normalize_map_pos(&x1, &y1);
-  normalize_map_pos(&x2, &y2);
+  check_map_pos(&x1, &y1);
+  check_map_pos(&x2, &y2);
   return (x1 == x2 && y1 == y2);
 }
 
 /**************************************************************************
-...
+This function needs to be removed and merged with check_map_pos().
 **************************************************************************/
 int check_coords(int *x, int *y)
 {
@@ -1443,9 +1412,8 @@
 **************************************************************************/
 int is_move_cardinal(int start_x, int start_y, int end_x, int end_y)
 {
-  assert(is_real_tile(start_x, start_y) && is_real_tile(end_x, end_y));
-  normalize_map_pos(&start_x, &start_y);
-  normalize_map_pos(&end_x, &end_y);
+  check_map_pos(&start_x, &start_y);
+  check_map_pos(&end_x, &end_y);
   assert(is_tiles_adjacent(start_x, start_y, end_x, end_y));
 
   /* FIXME: this check will not work with an orthogonal map */
Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.98
diff -u -r1.98 map.h
--- common/map.h        2001/10/14 21:15:59     1.98
+++ common/map.h        2001/10/15 08:26:52
@@ -13,6 +13,9 @@
 #ifndef FC__MAP_H
 #define FC__MAP_H
 
+#include <assert.h>
+
+#include "log.h"
 #include "player.h"
 #include "terrain.h"
 #include "unit.h"
@@ -208,7 +211,7 @@
   (((Y)<0) ? 0 : (((Y)>=map.ysize) ? map.ysize-1 : (Y)))
 
 #define map_inx(x,y) \
-  ((x)+(y)*map.xsize)
+  (check_map_pos(&x, &y), (x)+(y)*map.xsize)
 
 #define DIRSTEP(dest_x, dest_y, dir)   \
 (    (dest_x) = DIR_DX[(dir)],         \
@@ -242,6 +245,20 @@
 
 int normalize_map_pos(int *x, int *y);
 void nearest_real_pos(int *x, int *y);
+
+static inline void check_map_pos(int *x, int *y)
+{
+  /* When we're pretty sure things work it'll be worthwhile to remove this
+     code.  Until then, something like this is safest. */
+  if (!is_normal_map_pos(*x, *y)) {
+    assert(0);
+    freelog(LOG_ERROR, "Bad coordinates (%d, %d) passed to check_map_pos.", 
*x, *y);
+    if (!normalize_map_pos(x, y)) {
+      nearest_real_pos(x, y);
+      freelog(LOG_FATAL, "Unreal coordinates 'fixed' to (%d, %d).", *x, *y);
+    }
+  }
+}
 
 void rand_neighbour(int x0, int y0, int *x, int *y);
 
Index: common/unit.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/unit.c,v
retrieving revision 1.135
diff -u -r1.135 unit.c
--- common/unit.c       2001/09/16 09:38:08     1.135
+++ common/unit.c       2001/10/15 08:26:53
@@ -1255,9 +1255,8 @@
   }
 
   /* 2) */
-  if (!normalize_map_pos(&dest_x, &dest_y)) {
-    return MR_BAD_MAP_POSITION;
-  }
+  /* NOTE: MR_BAD_MAP_POSITION probably no longer needed */
+  check_map_pos(&dest_x, &dest_y);
 
   /* 3) */
   if (!is_tiles_adjacent(src_x, src_y, dest_x, dest_y)) {
Index: server/maphand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/maphand.c,v
retrieving revision 1.87
diff -u -r1.87 maphand.c
--- server/maphand.c    2001/10/11 12:37:06     1.87
+++ server/maphand.c    2001/10/15 08:26:54
@@ -771,11 +771,8 @@
 ***************************************************************/
 int map_get_known_and_seen(int x, int y, struct player *pplayer)
 {
-  int is_real = normalize_map_pos(&x, &y);
   int playerid=pplayer->player_no;
   int offset = map_inx(x, y);
-
-  assert(is_real);
 
   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.112
diff -u -r1.112 settlers.c
--- server/settlers.c   2001/10/14 21:02:17     1.112
+++ server/settlers.c   2001/10/15 08:26:55
@@ -405,8 +405,7 @@
 **************************************************************************/
 static int is_already_assigned(struct unit *myunit, struct player *pplayer, 
int x, int y)
 {
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
+  check_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 */
@@ -647,8 +646,8 @@
   int ii[12] = { -1, 0, 1, -1, 1, -1, 0, 1, 0, -2, 2, 0 };
   int jj[12] = { -1, -1, -1, 0, 0, 1, 1, 1, -2, 0, 0, 2 };
   struct tile *ptile;
-  if (!normalize_map_pos(&x, &y))
-    return 0;
+
+  check_map_pos(&x, &y);
 
   for (k = 0; k < 12; k++) {
     int x1 = x + ii[k], y1 = y + jj[k];
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.141
diff -u -r1.141 unittools.c
--- server/unittools.c  2001/10/14 21:02:17     1.141
+++ server/unittools.c  2001/10/15 08:26:57
@@ -1628,8 +1628,7 @@
   idex_register_unit(punit);
   punit->owner=pplayer->player_no;
 
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
+  check_map_pos(&x, &y);
   punit->x = x;
   punit->y = y;