[Freeciv-Dev] Re: [PATCH] more small directional cleanups

[Jason Dorje Short <jshort@xxxxxxxxxxxxx>]

Raimar Falke wrote:
> On Mon, Aug 20, 2001 at 09:03:20AM -0400, Jason Dorje Short wrote:

> > Unfortunately, I appear to have missed at least one instance of "magic"
> > code to handle the directional system in my previous patch.  A new patch
> > fixes this, as well as cleaning up new magic code that I introduced.
> 
> Looks good. Can the do while loop get a counter which bails out after
> say 1000 iterations? There is the currently unthinkable case of
> (x0,y0) are one a realness isle. But the list is discussing topologies
> which are weird I want just to be sure.

I added an assertion to make sure the loop terminates (if debugging;
otherwise we assume it terminates).  Bailing out won't really do any
good since it would leave the game in an unstable state.

jason

Jason Short
jshort@xxxxxxxxxxxxx

This small patch fixes one remaining direction-specific bit of code, as well
as cleaning the dir_get_name function so that it's not dependent on the
directional system.

I still can't be sure I've found everything, but simulations with different
directional systems don't show any problems.

Advantages: Cleaner; independent of directional system.
Disadvantages: Ugly algorithm for rand_neighbour.

Index: common/map.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.c,v
retrieving revision 1.80
diff -u -r1.80 map.c
--- common/map.c        2001/08/20 07:56:01     1.80
+++ common/map.c        2001/08/20 20:59:08
@@ -1318,20 +1318,22 @@
 **************************************************************************/
 void rand_neighbour(int x0, int y0, int *x, int *y)
 {
-  int choice;
+  int x1, y1;
+  int count = 0;
+  assert(is_real_tile(x0, y0));
 
-  if (y0 == 0) {
-    choice = 3 + myrand(5);
-  } else if(y0 == map.ysize-1){
-    choice = myrand(5);
-  } else {
-    choice = myrand(8);
-  }
-
-  *x = x0 + DIR_DX[choice];
-  *y = y0 + DIR_DY[choice];
+  /* This loop is theoretically unbounded in time, but the alternative
+   * is to either have ugly code to check realness or make assumptions
+   * about the topology.  In practice, it should work fine.  --JDS */
+  do {
+    int choice = myrand(8);
+    x1 = x0 + DIR_DX[choice];
+    y1 = y0 + DIR_DY[choice];
+    assert(++count < 10000);
+  } while (!is_real_tile(x1, y1));
 
-  assert(is_real_tile(*x, *y));
+  *x = x1;
+  *y = y1;
   normalize_map_pos(x, y);
 }
 
@@ -1340,9 +1342,17 @@
 **************************************************************************/
 const char *dir_get_name(enum direction8 dir)
 {
-  static const char *names[8] = { "NW", "N", "NE", "W",
-    "E", "SW", "S", "SE"
-  };
-  assert(dir >= 0 && dir < 8);
-  return names[dir];
+  /* a switch statement is used so the ordering can be changed easily */
+  switch (dir) {
+    case DIR8_NORTH:     return "N";
+    case DIR8_NORTHEAST: return "NE";
+    case DIR8_EAST:      return "E";
+    case DIR8_SOUTHEAST: return "SE";
+    case DIR8_SOUTH:     return "S";
+    case DIR8_SOUTHWEST: return "SW";
+    case DIR8_WEST:      return "W";
+    case DIR8_NORTHWEST: return "NW";
+  }
+  assert(0);
+  return "";
 }