[Freeciv-Dev] (PR#12833) consider_settler_action simplification

["Brian Dunstan" <bdunstan149@xxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=12833 >

I was looking at the auto settler code to see what
there is to be done, and one thing I came upon is that
consider_settler_action has some unnecessary steps. 
All it has to do is call the amortize() function,
which will do all the necessary calculation of the
value of a future benefit. 

I have also added few lines that will prevent stacks
of greater than MAX_WORKER_STACK from being used at
one time.  A large stack of workers is vulnerable to
attack and potentially wasteful.

In addition, I have removed the penalty for improving
tiles not currently worked by a city.  The reason for
this is that the penalty creates a catch-22; if you
wouldn't work the tile until it is improved, but would
not improve the tile until it is worked, you will be
stuck.

Brian


                
__________________________________ 
Do you Yahoo!? 
Plan great trips with Yahoo! Travel: Now over 17,000 guides!
http://travel.yahoo.com/p-travelguide

diff -Nur -Xfreeciv/diff_ignore freeciv/server/settlers.c 
freeciv_altered/server/settlers.c
--- freeciv/server/settlers.c   2005-04-17 13:57:00.255465488 -0400
+++ freeciv_altered/server/settlers.c   2005-04-18 14:32:05.992548304 -0400
@@ -766,45 +766,44 @@
                                    struct tile **best_tile,
                                     struct tile *ptile)
 {
-  int discount_value, base_value = 0;
-  int total_value;
-  bool consider;
+#define MAX_WORKER_STACK 2
 
+  bool consider;
+  int discount_value=0, base_value=0, total_value=0;
+  int worker_stack = 0; /* number of workers already at job site */
+  
   if (extra >= 0) {
     consider = TRUE;
   } else {
     consider = (new_tile_value > old_tile_value);
   }
 
+  /* find the present value of the future benefit of this action */
   if (consider) {
-    int diff = new_tile_value - old_tile_value;
-
-    /* The 64x is because we are dealing with small ints, usually from 0-20,
-     * which are insufficiently large to use directly in amortize().  Tiles
-     * which are not currently in use do not give us an improvement until
-     * a citizen works them, so they are reduced in value by 1/4. */
-    base_value = diff * (in_use ? 64 : 16) + extra * 64;
-    base_value = MAX(0, base_value);
-
+    base_value = MAX(0, new_tile_value - old_tile_value) + extra;
     discount_value = amortize(base_value, delay);
-
-    /* The total value is (roughly) equal to the base value multiplied by
-     * d / (1 - d), where d is the discount. (discount_value / base value)
-     * The MAX is a guard against the base value being greater or equal
-     * than the discount value, which would only happen if it or the 
-     * delay is <= 0. */
-    total_value = ((discount_value * base_value)
-                  / (MAX(1, base_value - discount_value))) / 64;
+    total_value = MAX(0, discount_value);
   } else {
     total_value = 0;
   }
+  
+  unit_list_iterate(ptile->units, wunit) {
+    if(unit_flag(wunit, F_SETTLERS))
+      worker_stack++;
+  } unit_list_iterate_end;
+
+  /* big worker stacks are a security risk */
+  if(worker_stack >= MAX_WORKER_STACK) {
+    total_value = 0;
+  }
 
   if (total_value > *best_value
       || (total_value == *best_value
          && old_tile_value > *best_old_tile_value)) {
     freelog(LOG_DEBUG,
-           "Replacing (%d, %d) = %d with %s (%d, %d) = %d [d=%d b=%d]",
-           TILE_XY(*best_tile), *best_value, get_activity_text(act),
+           "Replacing %s (%d, %d) = %d with %s (%d, %d) = %d [d=%d b=%d]",
+           get_activity_text(best_act), TILE_XY(*best_tile), 
+           *best_value, get_activity_text(act),
            TILE_XY(ptile), total_value,
             delay, base_value);
     *best_value = total_value;