[freeciv-ai] Re: (PR#9247) cm shouldn't count waste as a good thing

["Benoit Hudson" <bh@xxxxxxxxxxxxxxxxxxx>]

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

On Sat, Jul 17, 2004 at 06:17:53PM -0700, Jason Short wrote:
> dio_*** is just network stuff.  Removing the factor_target will probably
> require some network changes.  But now that factor_target is ignored we
> need to remove it lest someone be tempted to use it.
>
> Can you make a patch?

Attached.  I created a cm_init_parameter that sets everything to
something sane, so a GUI that doesn't allow setting an option doesn't
necessarily give garbage results.  Ideally, cm_init_parameter would have
the initial settings in auto_arrange_workers and all that work would be
centralized so that the GUI can access those values.  Analogously, I
created a cm_init_emergency_parameter which merges some repeated code in
auto_arrange_workers.

The patch applies to the cvs of about 1 minute 45 seconds ago; I only tested
building and running under gtk-2.0, although I made the same 1-liner
change to all the other GUIs.

        -- Benoît

? cm.factor-target-removed.diff
Index: ai/aihand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aihand.c,v
retrieving revision 1.90
diff -b -u -p -r1.90 aihand.c
--- ai/aihand.c 12 Jul 2004 20:56:07 -0000      1.90
+++ ai/aihand.c 20 Jul 2004 02:58:31 -0000
@@ -153,11 +153,10 @@ static void ai_manage_taxes(struct playe
       pplayer->economic.science -= 10;
     }
 
-    memset(&cmp, 0, sizeof(struct cm_parameter));
+    cm_init_parameter(&cmp);
     cmp.require_happy = TRUE;    /* note this one */
     cmp.allow_disorder = FALSE;
     cmp.allow_specialists = TRUE;
-    cmp.factor_target = FT_SURPLUS;
     cmp.factor[FOOD] = 20;
     cmp.minimal_surplus[GOLD] = -FC_INFINITY;
 
Index: client/options.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/options.c,v
retrieving revision 1.99
diff -b -u -p -r1.99 options.c
--- client/options.c    25 Jun 2004 23:35:55 -0000      1.99
+++ client/options.c    20 Jul 2004 02:58:32 -0000
@@ -640,8 +640,6 @@ static void load_cma_preset(struct secti
   }
   parameter.require_happy =
       secfile_lookup_bool_default(file, FALSE, "cma.preset%d.reqhappy", inx);
-  parameter.factor_target =
-      secfile_lookup_int_default(file, 0, "cma.preset%d.factortarget", inx);
   parameter.happy_factor =
       secfile_lookup_int_default(file, 0, "cma.preset%d.happyfactor", inx);
   parameter.allow_disorder = FALSE;
@@ -668,8 +666,6 @@ static void save_cma_preset(struct secti
   }
   secfile_insert_bool(file, pparam->require_happy,
                      "cma.preset%d.reqhappy", inx);
-  secfile_insert_int(file, pparam->factor_target,
-                    "cma.preset%d.factortarget", inx);
   secfile_insert_int(file, pparam->happy_factor,
                     "cma.preset%d.happyfactor", inx);
 }
Index: client/agents/cma_core.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/agents/cma_core.c,v
retrieving revision 1.56
diff -b -u -p -r1.56 cma_core.c
--- client/agents/cma_core.c    18 Jul 2004 01:16:05 -0000      1.56
+++ client/agents/cma_core.c    20 Jul 2004 02:58:32 -0000
@@ -66,7 +66,7 @@
 #define SHOW_APPLY_RESULT_ON_SERVER_ERRORS              FALSE
 #define ALWAYS_APPLY_AT_SERVER                          FALSE
 
-#define SAVED_PARAMETER_SIZE                           29
+#define SAVED_PARAMETER_SIZE                           28
 
 /*
  * Misc statistic to analyze performance.
@@ -548,7 +548,10 @@ bool cma_is_city_under_agent(struct city
 }
 
 /**************************************************************************
- ...
+ Get the parameter.
+ Don't bother to cm_init_parameter, since we set all the fields anyway.
+ But leave the comment here so we can find this place when searching
+ for all the creators of a parameter.
 **************************************************************************/
 bool cma_get_parameter(enum attr_city attr, int city_id,
                       struct cm_parameter *parameter)
@@ -575,7 +578,6 @@ bool cma_get_parameter(enum attr_city at
   }
 
   dio_get_sint16(&din, &parameter->happy_factor);
-  dio_get_uint8(&din, (int *) &parameter->factor_target);
   dio_get_bool8(&din, &parameter->require_happy);
   /* These options are only for server-AI use. */
   parameter->allow_disorder = FALSE;
@@ -604,7 +606,6 @@ void cma_set_parameter(enum attr_city at
   }
 
   dio_put_sint16(&dout, parameter->happy_factor);
-  dio_put_uint8(&dout, (int) parameter->factor_target);
   dio_put_bool8(&dout, parameter->require_happy);
 
   assert(dio_output_used(&dout) == SAVED_PARAMETER_SIZE);
Index: client/agents/cma_fec.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/agents/cma_fec.c,v
retrieving revision 1.21
diff -b -u -p -r1.21 cma_fec.c
--- client/agents/cma_fec.c     18 Jul 2004 01:16:05 -0000      1.21
+++ client/agents/cma_fec.c     20 Jul 2004 02:58:32 -0000
@@ -114,22 +114,10 @@ void cmafec_get_fe_parameter(struct city
     cm_copy_parameter(dest, &parameter);
     cmafec_set_fe_parameter(pcity, dest);
   } else {
+    /* Create a dummy parameter to return. */
+    cm_init_parameter(dest);
     if (!cma_get_parameter(ATTR_CITY_CMAFE_PARAMETER, pcity->id, dest)) {
-
-      /* We haven't seen this city previously; create a new dummy parameter. */
-      int i;
-
-      for (i = 0; i < NUM_STATS; i++) {
-        dest->minimal_surplus[i] = 0;
-        dest->factor[i] = 1;
-      }
-
-      dest->happy_factor = 1;
-      dest->require_happy = FALSE;
-      dest->allow_disorder = FALSE;
-      dest->allow_specialists = TRUE;
-      dest->factor_target = FT_SURPLUS;
-
+      /* We haven't seen this city before; store the dummy. */
       cmafec_set_fe_parameter(pcity, dest);
     }
   }
Index: client/gui-gtk/cma_fe.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/cma_fe.c,v
retrieving revision 1.21
diff -b -u -p -r1.21 cma_fe.c
--- client/gui-gtk/cma_fe.c     5 May 2004 20:39:15 -0000       1.21
+++ client/gui-gtk/cma_fe.c     20 Jul 2004 02:58:32 -0000
@@ -673,7 +673,6 @@ static void hscale_changed(GtkAdjustment
     param.minimal_surplus[i] = (int) (pdialog->minimal_surplus[i]->value);
     param.factor[i] = (int) (pdialog->factor[i]->value);
   }
-  param.factor_target = FT_SURPLUS;
   param.require_happy =
       (GTK_TOGGLE_BUTTON(pdialog->happy_button)->active ? 1 : 0);
   param.happy_factor = (int) (pdialog->factor[NUM_STATS]->value);
Index: client/gui-gtk-2.0/cma_fe.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/cma_fe.c,v
retrieving revision 1.18
diff -b -u -p -r1.18 cma_fe.c
--- client/gui-gtk-2.0/cma_fe.c 11 May 2004 17:52:25 -0000      1.18
+++ client/gui-gtk-2.0/cma_fe.c 20 Jul 2004 02:58:33 -0000
@@ -710,7 +710,6 @@ static void hscale_changed(GtkAdjustment
     param.minimal_surplus[i] = (int) (pdialog->minimal_surplus[i]->value);
     param.factor[i] = (int) (pdialog->factor[i]->value);
   }
-  param.factor_target = FT_SURPLUS;
   param.require_happy =
       (GTK_TOGGLE_BUTTON(pdialog->happy_button)->active ? 1 : 0);
   param.happy_factor = (int) (pdialog->factor[NUM_STATS]->value);
Index: client/gui-mui/citydlg.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-mui/citydlg.c,v
retrieving revision 1.79
diff -b -u -p -r1.79 citydlg.c
--- client/gui-mui/citydlg.c    17 Jul 2004 05:53:20 -0000      1.79
+++ client/gui-mui/citydlg.c    20 Jul 2004 02:58:34 -0000
@@ -1154,7 +1154,6 @@ static void city_cma_changed(struct city
     param.minimal_surplus[i] = 
(int)xget(pdialog->minimal_surplus_slider[i],MUIA_Numeric_Value);
     param.factor[i] = (int)xget(pdialog->factor_slider[i],MUIA_Numeric_Value);
   }
-  param.factor_target = FT_SURPLUS;
   param.require_happy = xget(pdialog->celebrate_check, MUIA_Selected);
   param.happy_factor = xget(pdialog->factor_slider[6],MUIA_Numeric_Value);
 
Index: client/gui-xaw/cma_fe.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-xaw/cma_fe.c,v
retrieving revision 1.1
diff -b -u -p -r1.1 cma_fe.c
--- client/gui-xaw/cma_fe.c     28 Jul 2003 01:16:06 -0000      1.1
+++ client/gui-xaw/cma_fe.c     20 Jul 2004 02:58:34 -0000
@@ -604,7 +604,6 @@ static void sliders_scroll_callback(Widg
 
   XtVaGetValues(celebrate_toggle, XtNstate, &parameter.require_happy, NULL);
   parameter.happy_factor = factors[NUM_STATS];
-  parameter.factor_target = FT_SURPLUS;
 
   cmafec_set_fe_parameter(current_city, &parameter);
 
@@ -663,7 +662,6 @@ void sliders_jump_callback(Widget w, XtP
 
   XtVaGetValues(celebrate_toggle, XtNstate, &parameter.require_happy, NULL);
   parameter.happy_factor = factors[NUM_STATS];
-  parameter.factor_target = FT_SURPLUS;
 
   cmafec_set_fe_parameter(current_city, &parameter);
 
@@ -754,7 +752,6 @@ static void new_preset_callback(Widget w
 
     parameter.happy_factor = factors[NUM_STATS];
     parameter.require_happy = celebrate_setting;
-    parameter.factor_target = FT_SURPLUS;
 
     cmafec_preset_add(input_dialog_get_input(w), &parameter);
 
@@ -786,7 +783,6 @@ void celebrate_callback(Widget w, XtPoin
 
   XtVaGetValues(celebrate_toggle, XtNstate, &parameter.require_happy, NULL);
   parameter.happy_factor = factors[NUM_STATS];
-  parameter.factor_target = FT_SURPLUS;
 
   cmafec_set_fe_parameter(current_city, &parameter);
 
Index: common/aicore/cm.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/aicore/cm.c,v
retrieving revision 1.36
diff -b -u -p -r1.36 cm.c
--- common/aicore/cm.c  18 Jul 2004 01:16:05 -0000      1.36
+++ common/aicore/cm.c  20 Jul 2004 02:58:35 -0000
@@ -665,7 +665,6 @@ static void calc_fitness(struct city *pc
   *minor_fitness = 0;
 
   for (i = 0; i < NUM_STATS; i++) {
-    assert(parameter->factor_target == FT_SURPLUS);
     *major_fitness += result->surplus[i] * parameter->factor[i];
     *minor_fitness += result->surplus[i];
   }
@@ -1519,9 +1518,6 @@ bool cm_are_parameter_equal(const struct
   if (p1->allow_specialists != p2->allow_specialists) {
     return FALSE;
   }
-  if (p1->factor_target != p2->factor_target) {
-    return FALSE;
-  }
   if (p1->happy_factor != p2->happy_factor) {
     return FALSE;
   }
@@ -1537,3 +1533,38 @@ void cm_copy_parameter(struct cm_paramet
 {
   memcpy(dest, src, sizeof(struct cm_parameter));
 }
+
+
+/**************************************************************************
+ Initialize the parameter to sane default values.
+**************************************************************************/
+void cm_init_parameter(struct cm_parameter *dest) {
+  enum cm_stat stat;
+  for (stat = 0; stat < NUM_STATS; stat++) {
+    dest->minimal_surplus[stat] = 0;
+    dest->factor[stat] = 1;
+  }
+
+  dest->happy_factor = 1;
+  dest->require_happy = FALSE;
+  dest->allow_disorder = FALSE;
+  dest->allow_specialists = TRUE;
+}
+
+
+/**************************************************************************
+ Initialize the parameter to sane default values that will always produce
+ a result.
+**************************************************************************/
+void cm_init_emergency_parameter(struct cm_parameter *dest) {
+  enum cm_stat stat;
+  for (stat = 0; stat < NUM_STATS; stat++) {
+    dest->minimal_surplus[stat] = -FC_INFINITY;
+    dest->factor[stat] = 1;
+  }
+
+  dest->happy_factor = 1;
+  dest->require_happy = FALSE;
+  dest->allow_disorder = TRUE;
+  dest->allow_specialists = TRUE;
+}
Index: common/aicore/cm.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/aicore/cm.h,v
retrieving revision 1.8
diff -b -u -p -r1.8 cm.h
--- common/aicore/cm.h  18 Jul 2004 01:16:05 -0000      1.8
+++ common/aicore/cm.h  20 Jul 2004 02:58:35 -0000
@@ -23,17 +23,12 @@
  * The plan defines a minimal surplus. The module will try to get the
  * required surplus. If there are citizens free after allocation of
  * the minimal surplus these citizens will get arranged to maximize
- * the sum over base*factor. The base depends upon the factor_target.
+ * the weighted sum over the surplus of each type.
  */
 
 #include "city.h"              /* CITY_MAP_SIZE */
 #include "shared.h"            /* bool type */
 
-enum factor_target {
-  FT_SURPLUS,                  /* will use the surplus as base */
-  FT_EXTRA                     /* will use (minimal_surplus-surplus) as base */
-};
-
 enum cm_stat { FOOD, SHIELD, TRADE, GOLD, LUXURY, SCIENCE, NUM_STATS };
 
 /* A description of the goal. */
@@ -43,8 +38,6 @@ struct cm_parameter {
   bool allow_disorder;
   bool allow_specialists;
 
-  enum factor_target factor_target;
-
   int factor[NUM_STATS];
   int happy_factor;
 };
@@ -88,6 +81,8 @@ bool cm_are_parameter_equal(const struct
                            const struct cm_parameter *const p2);
 void cm_copy_parameter(struct cm_parameter *dest,
                       const struct cm_parameter *const src);
+void cm_init_parameter(struct cm_parameter *dest);
+void cm_init_emergency_parameter(struct cm_parameter *dest);
 
 void cm_print_city(const struct city *pcity);
 void cm_print_result(const struct city *pcity,
Index: server/cityturn.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/cityturn.c,v
retrieving revision 1.253
diff -b -u -p -r1.253 cityturn.c
--- server/cityturn.c   13 Jul 2004 18:16:54 -0000      1.253
+++ server/cityturn.c   20 Jul 2004 02:58:37 -0000
@@ -184,6 +184,8 @@ void auto_arrange_workers(struct city *p
   struct cm_result cmr;
   struct player *pplayer = city_owner(pcity);
 
+  cm_init_parameter(&cmp);
+
   /* HACK: make sure everything is up-to-date before continuing.  This may
    * result in recursive calls to auto_arrange_workers, but it's better
    * to have these calls here than while we're reassigning workers (when
@@ -198,7 +200,6 @@ void auto_arrange_workers(struct city *p
   cmp.require_happy = FALSE;
   cmp.allow_disorder = FALSE;
   cmp.allow_specialists = TRUE;
-  cmp.factor_target = FT_SURPLUS;
 
   /* We used to look at pplayer->ai.xxx_priority to determine the values
    * to be used here.  However that doesn't work at all because those values
@@ -234,18 +235,8 @@ void auto_arrange_workers(struct city *p
       cm_query_result(pcity, &cmp, &cmr);
 
       if (!cmr.found_a_valid) {
-       /* 
-        * If we can't get a resonable result force a disorder.
-        */
-       cmp.allow_disorder = TRUE;
-       cmp.allow_specialists = FALSE;
-       cmp.minimal_surplus[FOOD] = -FC_INFINITY;
-       cmp.minimal_surplus[SHIELD] = -FC_INFINITY;
-       cmp.minimal_surplus[TRADE] = -FC_INFINITY;
-       cmp.minimal_surplus[GOLD] = -FC_INFINITY;
-       cmp.minimal_surplus[LUXURY] = -FC_INFINITY;
-       cmp.minimal_surplus[SCIENCE] = -FC_INFINITY;
-
+       /* Emergency management.  Get _some_ result. */
+        cm_init_emergency_parameter(&cmp);
        cm_query_result(pcity, &cmp, &cmr);
       }
     } else {
@@ -262,13 +253,7 @@ void auto_arrange_workers(struct city *p
 
       if (!cmr.found_a_valid) {
        CITY_LOG(LOG_DEBUG, pcity, "emergency management");
-       cmp.minimal_surplus[FOOD] = -FC_INFINITY;
-       cmp.minimal_surplus[SHIELD] = -FC_INFINITY;
-       cmp.minimal_surplus[LUXURY] = -FC_INFINITY;
-       cmp.minimal_surplus[SCIENCE] = -FC_INFINITY;
-       cmp.allow_disorder = TRUE;
-       cmp.allow_specialists = FALSE;
-
+        cm_init_emergency_parameter(&cmp);
        cm_query_result(pcity, &cmp, &cmr);
       }
     }