[Freeciv-Dev] Re: PATCH: rand_pos function andusage(PR#1017)

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

These are reasonable uses of rand_pos(). Looks good.

Cheers,
RossW
=====

At 06:50 PM 01/10/28 -0800, jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
>"Ross W. Wetmore" wrote:
>> 
>> At 01:19 AM 01/10/24 -0400, Jason Dorje Short wrote:
>> >"Ross W. Wetmore" wrote:

>Your mapgen code, like all your other corecleanup code, still assumes
>normal==regular and that the map will wrap only along the x and y axes. 
>In fact, you seem to keep forgetting that topologies will not always
>follow this. :-)

No, you keep forgetting that the map generators in mapgen are designed
for standard earth. They don't need any more capabilities to do this.
And they extend fine to the standard 8 rectangular cases which are still
not available and from the looks of it won't for some time :-).

Another factino, you don't need to consider "wrap" when generating maps. 
All you need is to guarantee you are working with N(0,0) coordinates. 
And for the most part this really only depends on using the right loops.
If you collect lists of valid positions and randomly select from the
lists, you don't need anything beyond safe loops. Certainly, there is
absolutely no hard dependency on a rand_pos()! 

The corecleanup version uses pretty much strictly iterate macros for its 
loops and a lot more filtered algorithms. One could certainly push this 
further. And it has been extensively tested for non-standard topologies.
This is hard reality, not wild conjectures of pending disaster.

A reasonable true iso-map generator would be put together somewhat 
differently - and it should when you have the rest of the design and
prototype for this a little further along.

This stuff is absolutely low priority on the pecking order, so there 
is no need for you to wreck the existing system for some time.

[...]
>In the meantime, can we agree on this patch?  It replaces all of the
>innocuous random position selection with rand_pos, getting rid of most
>of the "bad" code (including everything not in mapgen).  It does not use
>regular_map_pos_is_normal (which I'd be happy to add back in if
>desired).
>
>I hope we can bring an end to these simple rand_pos changes, leaving the
>harder ones for later.
>
>jason? rc
>? topology-current
>? old
>? topology
>? tilespec_patch
>? corecleanup_08.ReadMe
>? civserver-regular
>Index: common/map.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/common/map.c,v
>retrieving revision 1.97
>diff -u -r1.97 map.c
>--- common/map.c       2001/10/15 13:42:50     1.97
>+++ common/map.c       2001/10/29 02:47:13
>@@ -1373,6 +1373,17 @@
> }
> 
> /**************************************************************************
>+Random square anywhere on the map.  Only "normal" positions will be found.
>+**************************************************************************/
>+void rand_pos(int *x, int *y)
>+{
>+  do {
>+    *x = myrand(map.xsize);
>+    *y = myrand(map.ysize);
>+  } while (!is_normal_map_pos(*x, *y));
>+}
>+
>+/**************************************************************************
> Return the debugging name of the direction.
> **************************************************************************/
> const char *dir_get_name(enum direction8 dir)
>Index: common/map.h
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
>retrieving revision 1.100
>diff -u -r1.100 map.h
>--- common/map.h       2001/10/19 08:12:52     1.100
>+++ common/map.h       2001/10/29 02:47:13
>@@ -286,6 +286,7 @@
> void nearest_real_pos(int *x, int *y);
> 
> void rand_neighbour(int x0, int y0, int *x, int *y);
>+void rand_pos(int *x, int *y);
> 
> int is_water_adjacent_to_tile(int x, int y);
> int is_tiles_adjacent(int x0, int y0, int x1, int y1);
>Index: server/barbarian.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/server/barbarian.c,v
>retrieving revision 1.35
>diff -u -r1.35 barbarian.c
>--- server/barbarian.c 2001/10/14 21:02:16     1.35
>+++ server/barbarian.c 2001/10/29 02:47:14
>@@ -324,8 +324,10 @@
>   struct city *pc;
>   struct player *barbarians, *victim;
> 
>-  x = myrand(map.xsize);
>-  y = 1 + myrand(map.ysize-2);  /* No uprising on North or South Pole */
>+  /* No uprising on North or South Pole */
>+  do {
>+    rand_pos(&x, &y);
>+  } while (y == 0 || y == map.ysize-1);
> 
>   if( !(pc = dist_nearest_city(NULL, x, y, 1, 0)) )       /* any city */
>     return;
>Index: server/mapgen.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/server/mapgen.c,v
>retrieving revision 1.75
>diff -u -r1.75 mapgen.c
>--- server/mapgen.c    2001/10/14 21:02:16     1.75
>+++ server/mapgen.c    2001/10/29 02:47:15
>@@ -199,8 +199,7 @@
>   int forestsize=25;
>   forestsize=(map.xsize*map.ysize*map.forestsize)/1000;
>    do {
>-    x=myrand(map.xsize);
>-    y=myrand(map.ysize);
>+    rand_pos(&x, &y);
>     if (map_get_terrain(x, y)==T_GRASSLAND) {
>       make_forest(x,y, hmap(x, y), 25);
>     }
>@@ -227,8 +226,7 @@
>   for (swamps=0;swamps<map.swampsize;) {
>     forever++;
>     if (forever>1000) return;
>-    y=myrand(map.ysize);
>-    x=myrand(map.xsize);
>+    rand_pos(&x, &y);
>     if (map_get_terrain(x, y)==T_GRASSLAND && hmap(x, y)<(maxval*60)/100) {
>       map_set_terrain(x, y, T_SWAMP);
>       cartesian_adjacent_iterate(x, y, x1, y1) {
>@@ -672,11 +670,10 @@
>        iteration_counter < RIVERS_MAXTRIES) {
> 
>     /* Don't start any rivers at the poles. */
>-    y = myrand(map.ysize - 2) + 1; 
>+    do {
>+      rand_pos(&x, &y);
>+    } while (y == 0 || y == map.ysize-1);
> 
>-    /* Any x-coordinate is valid. */
>-    x = myrand(map.xsize);
>- 
>     /* Check if it is suitable to start a river on the current tile.
>      */
>     if (
>@@ -1140,8 +1137,7 @@
>   assert(game.nplayers<=nr+sum);
> 
>   while (nr<game.nplayers) {
>-    x=myrand(map.xsize);
>-    y=myrand(map.ysize); 
>+    rand_pos(&x, &y);
>     if (islands[(int)map_get_continent(x, y)].starters) {
>       j++;
>       if (!is_starter_close(x, y, nr, dist)) {
>@@ -1309,7 +1305,9 @@
>   } whole_map_iterate_end;
> 
>   for (i=0;i<1500;i++) {
>-    height_map[myrand(map.ysize*map.xsize)]+=myrand(5000);
>+    int x, y;
>+    rand_pos(&x, &y);
>+    hmap(x, y) += myrand(5000);
>     if (!(i%100)) {
>       smooth_map(); 
>     }
>@@ -1381,8 +1379,7 @@
>   int x,y,l;
>   int count=0;
>   while ((number*map.xsize*map.ysize)/2000 &&
count++<map.xsize*map.ysize*2) {
>-    x=myrand(map.xsize);
>-    y=myrand(map.ysize);
>+    rand_pos(&x, &y);
>     l=myrand(6);
>     if (map_get_terrain(x, y)!=T_OCEAN && 
>       ( map_get_terrain(x, y)!=T_ARCTIC || l<3 )
>@@ -1545,8 +1542,7 @@
> static int place_island(void)
> {
>   int x, y, xo, yo, i=0;
>-  yo = myrand(map.ysize);
>-  xo = myrand(map.xsize);
>+  rand_pos(&xo, &yo);
> 
>   /* this helps a lot for maps with high landmass */
>   for (y = n, x = w ; y < s && x < e ; y++, x++) {
>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/29 02:47:16
>@@ -75,8 +75,7 @@
> 
>   k = map.xsize * map.ysize;
>   while(effect && k--) {
>-    x = myrand(map.xsize);
>-    y = myrand(map.ysize);
>+    rand_pos(&x, &y);
>     if (map_get_terrain(x, y) != T_OCEAN) {
>       if (is_terrain_ecologically_wet(x, y)) {
>       switch (map_get_terrain(x, y)) {
>@@ -132,8 +131,7 @@
> 
>   k =map.xsize * map.ysize;
>   while(effect && k--) {
>-    x = myrand(map.xsize);
>-    y = myrand(map.ysize);
>+    rand_pos(&x, &y);
>     if (map_get_terrain(x, y) != T_OCEAN) {
>       switch (map_get_terrain(x, y)) {
>       case T_JUNGLE:
>