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

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

"Ross W. Wetmore" wrote:
> 
> At 01:19 AM 01/10/24 -0400, Jason Dorje Short wrote:
> >"Ross W. Wetmore" wrote:
> >
> ><snip context: RAND_POS_CHECKED macro>
> >
> >> This clearly significantly increases the computation effort,
> >
> >Not necessarily.  In a mostly-grassland map the overhead isn't necessary,
> >but in a highly-forested or highly-desert map the fact that the loops are
> >bounded would be a big advantage.
> 
> You missed the point. The example explaining it is in another email.
> 
> BTW: It is bad that you missed the point. It shows you don't understand
>      the practical aspects/needs.

I think you missed the counter-point.  Although the "pick-at-random"
algorithm is many times slower, there is no measurable difference in map
generation speed while using it.

> Nothing currently depends on this and nothing needs to depend on this.

All the rest of the mapgen stuff depends on it.  If we ignore mapgen
stuff, then things become simpler.

> BTW: There is no problem running the mapgen.c replacement in any of the
>      4 rectangular topologies, and in corecleanup_07 the existing mapgen
>      had minimal fixes to do the same.

These all have normal==regular.  With an iso-rectangular topology, it
would quickly segfault.

> >Your corecleanup patch doesn't touch on mapgen.c so I have no other work to
> >go by here.  How would you change mapgen to accomplish the above two goals?
> 
> They are really more or less there. But if you need more ...
> 
> You can apply the mapgen.c replacement that has been in all the corecleanups.
> It might even work with CVS if DIR_CCW etc. is used in the places where it
> does really need to use rotational map_steps. This is pretty much the river
> code. Some form of block_iterate is also needed. But both of these can be
> put in as local macro fixes/hacks until the general CVS codebase reaches the
> necessary level of capability. The initial corecleanup versions actually ran
> this way, i.e. with self-contined corecleanups.

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. :-)

> I would also take responsibility for any bugfixes to keep these standard
> map generators running through the topology changes, and produce a new
> generator
> to handle general topology features agreed upon in some collective proposal.
> I actually have a strong interest in playing on good maps as opposed to the
> stuff that is currently produced, so would do some of this on my own anyway.

This sounds great.  Again, here is my position:

- I don't want to make the current mapgen code worse.
- I want mapgen code that is topology-independent, although not all
generators must be independent.
- It is quite easy to make the current mapgen code topology-independent
without impacting current map generation.
- It may not be so easy to make better mapgen code topology-independent.


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: