[Freeciv-Dev] (PR#6257) recodify map_pos<->index conversion to use native positions

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

Index positions are ordered based off of native ones.  So the current 
conversion operations (e.g., map_pos_to_index and index_to_map_pos) are 
no good when we start using native positions.

Unfortunately going through native coordinates requires an intermediate 
pair of variables, which is a problem with macros.  There are three 
possible solutions:


1.  Change them both into standard functions.

This is the prettiest solution.  The only drawback is that it's slower - 
about a 5-10% slowdown in autogame performance in a few tests I did.


2.  Change them both into inline functions.

This is almost as pretty, and just as fast as the current code (for 
reasonable compilers).  The only real drawback is that we're not 
"supposed" to use inline functions in freeciv.  Also compilers that 
don't recognize inline will give warnings in files that don't use the 
functions (unavoidable when using inline).


3.  Keep them as macros with temporary (global) variables in the header 
file.

The obvious drawback is that this is incredibly ugly.  The less obvious 
drawback is that the line

   array[map_pos_to_index(x, y)] = array[map_pos_to_index(x1, y1)]

is not legal C (just like a ^= b ^= a ^= b).  So without some other 
cleverness either all lines like this will have to be changed (see 
settlers.c and mapgen.c) or this is not an option.  Gcc 3.2.1 just gives 
a warning, and *hopefully* compiles it into the proper code:

   settlers.c:167: warning: operation on `_fc_nat_y' may be undefined
   settlers.c:167: warning: operation on `_fc_nat_x' may be undefined
   settlers.c:169: warning: operation on `_fc_nat_y' may be undefined
   settlers.c:169: warning: operation on `_fc_nat_x' may be undefined

If only C had tuples...


Patches for all three variants are attached.  The macro one won't 
compile under strictly conformant C compilers.

jason

Index: common/map.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.c,v
retrieving revision 1.144
diff -u -r1.144 map.c
--- common/map.c        2003/09/23 16:10:23     1.144
+++ common/map.c        2003/09/23 17:17:18
@@ -1319,6 +1319,36 @@
   return (x1 == x2 && y1 == y2);
 }
 
+/****************************************************************************
+  Convert from standard map coordinates into a compact, scalar index
+  value in the range [0..MAP_MAX_INDEX).
+  The conversion can never fail, but it does assume the given map
+  position is normal (and returns a normal index).
+****************************************************************************/
+int map_pos_to_index(int map_x, int map_y)
+{
+  int nat_x, nat_y;
+
+  CHECK_MAP_POS(map_x, map_y);
+  map_to_native_pos(&nat_x, &nat_y, map_x, map_y);
+  return nat_x + nat_y * map.xsize;
+}
+
+/****************************************************************************
+  Convert from a scalar index to standard map coordinates.
+  This conversion can never fail, but it does assume the given index is
+  a normal position (and returns a normal map position)..
+****************************************************************************/
+void index_to_map_pos(int *pmap_x, int *pmap_y, int index)
+{
+  int nat_x, nat_y;
+
+  CHECK_INDEX(index);
+  nat_x = index % map.xsize;
+  nat_y = index / map.xsize;
+  native_to_map_pos(pmap_x, pmap_y, nat_x, nat_y);
+}
+
 bool is_real_map_pos(int x, int y)
 {
   return normalize_map_pos(&x, &y);
Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.153
diff -u -r1.153 map.h
--- common/map.h        2003/09/19 13:17:12     1.153
+++ common/map.h        2003/09/23 17:17:18
@@ -241,16 +241,6 @@
 #define map_to_native_pos(pnat_x, pnat_y, map_x, map_y) \
   (*(pnat_x) = (map_x), *(pnat_y) = (map_y))
 
-#define map_pos_to_index(map_x, map_y)        \
-  (CHECK_MAP_POS((map_x), (map_y)),           \
-   (map_x) + (map_y) * map.xsize)
-
-/* index_to_map_pos(int *, int *, int) inverts map_pos_to_index */
-#define index_to_map_pos(pmap_x, pmap_y, index) \
-  (CHECK_INDEX(index),                          \
-   *(pmap_x) = (index) % map.xsize,             \
-   *(pmap_y) = (index) / map.xsize)
-
 #define DIRSTEP(dest_x, dest_y, dir)   \
 (    (dest_x) = DIR_DX[(dir)],         \
      (dest_y) = DIR_DY[(dir)])
@@ -279,8 +269,6 @@
 void map_set_special(int x, int y, enum tile_special_type spe);
 void map_clear_special(int x, int y, enum tile_special_type spe);
 void map_clear_all_specials(int x, int y);
-bool is_real_map_pos(int x, int y);
-bool is_normal_map_pos(int x, int y);
 
 /* implemented in server/maphand.c and client/climisc.c */
 enum known_type map_get_known2(int x, int y, struct player *pplayer);
@@ -308,6 +296,11 @@
   ((x) < (dist) || (x) >= map.xsize - (dist)                  \
    || (y) < (dist) || (y) >= map.ysize - (dist))
 
+int map_pos_to_index(int map_x, int map_y);
+void index_to_map_pos(int *map_x, int *map_y, int index);
+
+bool is_real_map_pos(int x, int y);
+bool is_normal_map_pos(int x, int y);
 bool normalize_map_pos(int *x, int *y);
 void nearest_real_pos(int *x, int *y);
 void map_distance_vector(int *dx, int *dy, int x0, int y0, int x1, int y1);

Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.153
diff -u -r1.153 map.h
--- common/map.h        2003/09/19 13:17:12     1.153
+++ common/map.h        2003/09/23 17:31:56
@@ -241,16 +241,6 @@
 #define map_to_native_pos(pnat_x, pnat_y, map_x, map_y) \
   (*(pnat_x) = (map_x), *(pnat_y) = (map_y))
 
-#define map_pos_to_index(map_x, map_y)        \
-  (CHECK_MAP_POS((map_x), (map_y)),           \
-   (map_x) + (map_y) * map.xsize)
-
-/* index_to_map_pos(int *, int *, int) inverts map_pos_to_index */
-#define index_to_map_pos(pmap_x, pmap_y, index) \
-  (CHECK_INDEX(index),                          \
-   *(pmap_x) = (index) % map.xsize,             \
-   *(pmap_y) = (index) / map.xsize)
-
 #define DIRSTEP(dest_x, dest_y, dir)   \
 (    (dest_x) = DIR_DX[(dir)],         \
      (dest_y) = DIR_DY[(dir)])
@@ -279,8 +269,6 @@
 void map_set_special(int x, int y, enum tile_special_type spe);
 void map_clear_special(int x, int y, enum tile_special_type spe);
 void map_clear_all_specials(int x, int y);
-bool is_real_map_pos(int x, int y);
-bool is_normal_map_pos(int x, int y);
 
 /* implemented in server/maphand.c and client/climisc.c */
 enum known_type map_get_known2(int x, int y, struct player *pplayer);
@@ -308,6 +296,11 @@
   ((x) < (dist) || (x) >= map.xsize - (dist)                  \
    || (y) < (dist) || (y) >= map.ysize - (dist))
 
+static inline int map_pos_to_index(int map_x, int map_y);
+static inline void index_to_map_pos(int *map_x, int *map_y, int index);
+
+bool is_real_map_pos(int x, int y);
+bool is_normal_map_pos(int x, int y);
 bool normalize_map_pos(int *x, int *y);
 void nearest_real_pos(int *x, int *y);
 void map_distance_vector(int *dx, int *dy, int x0, int y0, int x1, int y1);
@@ -687,5 +680,35 @@
 #define MAP_DEFAULT_SEPARATE_POLES   TRUE
 #define MAP_MIN_SEPARATE_POLES       FALSE
 #define MAP_MAX_SEPARATE_POLES       TRUE
+
+/****************************************************************************
+  Convert from standard map coordinates into a compact, scalar index
+  value in the range [0..MAP_MAX_INDEX).
+  The conversion can never fail, but it does assume the given map
+  position is normal (and returns a normal index).
+****************************************************************************/
+static inline int map_pos_to_index(int map_x, int map_y)
+{
+  int nat_x, nat_y;
+
+  CHECK_MAP_POS(map_x, map_y);
+  map_to_native_pos(&nat_x, &nat_y, map_x, map_y);
+  return nat_x + nat_y * map.xsize;
+}
+
+/****************************************************************************
+  Convert from a scalar index to standard map coordinates.
+  This conversion can never fail, but it does assume the given index is
+  a normal position (and returns a normal map position)..
+****************************************************************************/
+static inline void index_to_map_pos(int *pmap_x, int *pmap_y, int index)
+{
+  int nat_x, nat_y;
+
+  CHECK_INDEX(index);
+  nat_x = index % map.xsize;
+  nat_y = index / map.xsize;
+  native_to_map_pos(pmap_x, pmap_y, nat_x, nat_y);
+}
 
 #endif  /* FC__MAP_H */

Index: common/map.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.c,v
retrieving revision 1.144
diff -u -r1.144 map.c
--- common/map.c        2003/09/23 16:10:23     1.144
+++ common/map.c        2003/09/23 17:39:51
@@ -31,6 +31,9 @@
 
 #include "map.h"
 
+/* Intermediate variables used in map.h macros. */
+int _fc_nat_x, _fc_nat_y;
+
 /* the very map */
 struct civ_map map;
 
Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.153
diff -u -r1.153 map.h
--- common/map.h        2003/09/19 13:17:12     1.153
+++ common/map.h        2003/09/23 17:39:51
@@ -235,21 +235,25 @@
 #define map_adjust_y(Y) \
   (((Y)<0) ? 0 : (((Y)>=map.ysize) ? map.ysize-1 : (Y)))
 
+extern int _fc_nat_x, _fc_nat_y;
+
 #define native_to_map_pos(pmap_x, pmap_y, nat_x, nat_y) \
   (*(pmap_x) = (nat_x), *(pmap_y) = (nat_y))
 
 #define map_to_native_pos(pnat_x, pnat_y, map_x, map_y) \
   (*(pnat_x) = (map_x), *(pnat_y) = (map_y))
 
-#define map_pos_to_index(map_x, map_y)        \
-  (CHECK_MAP_POS((map_x), (map_y)),           \
-   (map_x) + (map_y) * map.xsize)
+#define map_pos_to_index(map_x, map_y)                                      \
+  (CHECK_MAP_POS((map_x), (map_y)),                                         \
+   map_to_native_pos(&_fc_nat_x, &_fc_nat_y, map_x, map_y),                 \
+   (_fc_nat_x) + (_fc_nat_y) * map.xsize)
 
 /* index_to_map_pos(int *, int *, int) inverts map_pos_to_index */
-#define index_to_map_pos(pmap_x, pmap_y, index) \
-  (CHECK_INDEX(index),                          \
-   *(pmap_x) = (index) % map.xsize,             \
-   *(pmap_y) = (index) / map.xsize)
+#define index_to_map_pos(pmap_x, pmap_y, index)                             \
+  (CHECK_INDEX(index),                                                      \
+   *(pmap_x) = (index) % map.xsize,                                         \
+   *(pmap_y) = (index) / map.xsize,                                         \
+   native_to_map_pos(pmap_x, pmap_y, *(pmap_x), *(pmap_y)))
 
 #define DIRSTEP(dest_x, dest_y, dir)   \
 (    (dest_x) = DIR_DX[(dir)],         \