[Freeciv-Dev] Re: (PR#8959) remove CAR_DIR_D[XY]

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8959 >

Gregory Berkolaiko wrote:

> This, I feel, will have the best features of Ross' model and be more
> extensible because the subset of dirs over which you want to iterate next
> doesn't have to form an arithmetic progression in the total set of all
> directions.

Here's a patch.  enum direction8 had to be moved up above struct civ_map 
to get the "right" types (of course we could just use int instead).

This is not a full solution to the problem because the CAR_DIR_DX arrays 
are used in a couple of other places.  However this addresses the most 
complicated user.

In a later patch I'd like to rename "cartesian" as "cardinal" (currently 
the two are used interchangably, at least by this part of the code), and 
introduce a new macro cardinal_adjc_dir_iterate that gives the direction 
as well (which is used in mapgen.c and currently hard-coded).

I think we can safely redefine "cardinal" (which is a pretty fuzzy term 
anyway) so that in a hex topology there are 6 cardinal directions. 
However if anyone has a better naming idea that would be, well, better.

jason

? gmon.out
Index: common/map.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.c,v
retrieving revision 1.170
diff -u -r1.170 map.c
--- common/map.c        12 Jun 2004 17:42:27 -0000      1.170
+++ common/map.c        22 Jun 2004 14:55:34 -0000
@@ -38,7 +38,21 @@
 /* these are initialized from the terrain ruleset */
 struct terrain_misc terrain_control;
 
-/* used to compute neighboring tiles */
+/* used to compute neighboring tiles.
+ *
+ * using
+ *   x1 = x + DIR_DX[dir];
+ *   y1 = y + DIR_DY[dir];
+ * will give you the tile as shown below.
+ *   -------
+ *   |0|1|2|
+ *   |-+-+-|
+ *   |3| |4|
+ *   |-+-+-|
+ *   |5|6|7|
+ *   -------
+ * Note that you must normalize x1 and y1 yourself.
+ */
 const int DIR_DX[8] = { -1, 0, 1, -1, 1, -1, 0, 1 };
 const int DIR_DY[8] = { -1, -1, -1, 0, 0, 1, 1, 1 };
 
@@ -294,6 +308,8 @@
 ****************************************************************************/
 void map_init_topology(bool set_sizes)
 {
+  enum direction8 dir;
+
   /* Changing or reordering the topo_flag enum will break this code. */
   const int default_ratios[4][2] =
       {AUTO_RATIO_FLAT, AUTO_RATIO_CLASSIC,
@@ -309,6 +325,21 @@
     /* Set map.size based on map.xsize and map.ysize. */
     map.size = (float)(map.xsize * map.ysize) / 1000.0 + 0.5;
   }
+
+  map.num_valid_dirs = map.num_cardinal_dirs = 0;
+  for (dir = 0; dir < 8; dir++) {
+    if (is_valid_dir(dir)) {
+      map.valid_dirs[map.num_valid_dirs] = dir;
+      map.num_valid_dirs++;
+    }
+    if (DIR_IS_CARDINAL(dir)) {
+      map.cardinal_dirs[map.num_cardinal_dirs] = dir;
+      map.num_cardinal_dirs++;
+    }
+  }
+  assert(map.num_valid_dirs > 0 && map.num_valid_dirs <= 8);
+  assert(map.num_cardinal_dirs > 0
+        && map.num_cardinal_dirs <= map.num_valid_dirs);
 }
 
 /***************************************************************
Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.188
diff -u -r1.188 map.h
--- common/map.h        12 Jun 2004 17:42:27 -0000      1.188
+++ common/map.h        22 Jun 2004 14:55:34 -0000
@@ -115,8 +115,32 @@
   char *helptext;
 };
 
-struct civ_map { 
+/* The direction8 gives the 8 possible directions.  These may be used in
+ * a number of ways, for instance as an index into the DIR_DX/DIR_DY
+ * arrays.  Not all directions may be valid; see is_valid_dir and
+ * DIR_IS_CARDINAL. */
+enum direction8 {
+  /* The DIR8/direction8 naming system is used to avoid conflict with
+   * DIR4/direction4 in client/tilespec.h
+   *
+   * Changing the order of the directions will break network compatability.
+   *
+   * Some code assumes that the first 4 directions are the reverses of the
+   * last 4 (in no particular order).  See client/goto.c. */
+  DIR8_NORTHWEST = 0,
+  DIR8_NORTH = 1,
+  DIR8_NORTHEAST = 2,
+  DIR8_WEST = 3,
+  DIR8_EAST = 4,
+  DIR8_SOUTHWEST = 5,
+  DIR8_SOUTH = 6,
+  DIR8_SOUTHEAST = 7
+};
+
+struct civ_map {
   int topology_id;
+  enum direction8 valid_dirs[8], cardinal_dirs[8];
+  int num_valid_dirs, num_cardinal_dirs;
   int size; /* used to calculate [xy]size */
   int xsize, ysize; /* native dimensions */
   int seed;
@@ -527,26 +551,35 @@
   } adjc_dir_iterate_end;                                                     \
 }
 
+#define adjc_dir_iterate(center_x, center_y, x_itr, y_itr, dir_itr)        \
+  adjc_dirlist_iterate(center_x, center_y, x_itr, y_itr, dir_itr,          \
+                      map.valid_dirs, map.num_valid_dirs)
+
+#define adjc_dir_iterate_end adjc_dirlist_iterate_end
+
 /* Iterate through all tiles adjacent to a tile.  dir_itr is the
    directional value (see DIR_D[XY]).  This assumes that center_x and
    center_y are normalized. --JDS */
-#define adjc_dir_iterate(center_x, center_y, x_itr, y_itr, dir_itr)           \
-{                                                                             \
-  int x_itr, y_itr, dir_itr;                                                  \
-  bool MACRO_border = IS_BORDER_MAP_POS((center_x), (center_y), 1);           \
-  int MACRO_center_x = (center_x);                                            \
-  int MACRO_center_y = (center_y);                                            \
-  CHECK_MAP_POS(MACRO_center_x, MACRO_center_y);                              \
-  for (dir_itr = 0; dir_itr < 8; dir_itr++) {                                 \
-    DIRSTEP(x_itr, y_itr, dir_itr);                                           \
-    x_itr += MACRO_center_x;                                                  \
-    y_itr += MACRO_center_y;                                                  \
-    if (MACRO_border && !normalize_map_pos(&x_itr, &y_itr)) {                 \
-       continue;                                                             \
+#define adjc_dirlist_iterate(center_x, center_y, x_itr, y_itr, dir_itr,     \
+                             dirlist, dircount)                                
    \
+{                                                                          \
+  int x_itr, y_itr, _dir_index;                                                
    \
+  enum direction8 dir_itr;                                                 \
+  bool MACRO_border = IS_BORDER_MAP_POS((center_x), (center_y), 1);        \
+  int MACRO_center_x = (center_x);                                         \
+  int MACRO_center_y = (center_y);                                         \
+  CHECK_MAP_POS(MACRO_center_x, MACRO_center_y);                           \
+  for (_dir_index = 0; _dir_index < (dircount); _dir_index++) {                
    \
+    dir_itr = dirlist[_dir_index];                                         \
+    DIRSTEP(x_itr, y_itr, dir_itr);                                        \
+    x_itr += MACRO_center_x;                                               \
+    y_itr += MACRO_center_y;                                               \
+    if (MACRO_border && !normalize_map_pos(&x_itr, &y_itr)) {              \
+      continue;                                                                
    \
     }
 
-#define adjc_dir_iterate_end                                                  \
-  }                                                                           \
+#define adjc_dirlist_iterate_end                                           \
+    }                                                                      \
 }
 
 /* Iterate over all positions on the globe. */
@@ -563,40 +596,6 @@
   }                                                                         \
 }
 
-/*
-used to compute neighboring tiles:
-using
-x1 = x + DIR_DX[dir];
-y1 = y + DIR_DX[dir];
-will give you the tile as shown below.
--------
-|0|1|2|
-|-+-+-|
-|3| |4|
-|-+-+-|
-|5|6|7|
--------
-Note that you must normalize x1 and y1 yourself.
-*/
-
-enum direction8 {
-  /* The DIR8/direction8 naming system is used to avoid conflict with
-   * DIR4/direction4 in client/tilespec.h
-   *
-   * Changing the order of the directions will break network compatability.
-   *
-   * Some code assumes that the first 4 directions are the reverses of the
-   * last 4 (in no particular order.  See client/goto.c. */
-  DIR8_NORTHWEST = 0,
-  DIR8_NORTH = 1,
-  DIR8_NORTHEAST = 2,
-  DIR8_WEST = 3,
-  DIR8_EAST = 4,
-  DIR8_SOUTHWEST = 5,
-  DIR8_SOUTH = 6,
-  DIR8_SOUTHEAST = 7
-};
-
 BV_DEFINE(dir_vector, 8);
 
 struct unit_order {
@@ -625,23 +624,11 @@
 extern const int CAR_DIR_DX[4];
 extern const int CAR_DIR_DY[4];
 
-#define cartesian_adjacent_iterate(x, y, IAC_x, IAC_y)                        \
-{                                                                             \
-  int IAC_i;                                                                  \
-  int IAC_x, IAC_y;                                                           \
-  bool _is_border = IS_BORDER_MAP_POS(x, y, 1);                               \
-  CHECK_MAP_POS(x, y);                                                        \
-  for (IAC_i = 0; IAC_i < 4; IAC_i++) {                                       \
-    IAC_x = x + CAR_DIR_DX[IAC_i];                                            \
-    IAC_y = y + CAR_DIR_DY[IAC_i];                                            \
-                                                                              \
-    if (_is_border && !normalize_map_pos(&IAC_x, &IAC_y)) {                   \
-      continue;                                                               \
-    }
+#define cartesian_adjacent_iterate(center_x, center_y, x_itr, y_itr)       \
+  adjc_dirlist_iterate(center_x, center_y, x_itr, y_itr, _dir_itr,         \
+                      map.cardinal_dirs, map.num_cardinal_dirs)
 
-#define cartesian_adjacent_iterate_end                                        \
-  }                                                                           \
-}
+#define cartesian_adjacent_iterate_end adjc_dirlist_iterate_end
 
 /* Used for network transmission; do not change. */
 #define MAP_TILE_OWNER_NULL     MAX_UINT8