[Freeciv-Dev] (PR#14014) negative production_bonus partly supported

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

The problem is not just with a negative bonus from a building but with a
net negative bonus.  For instance in the attached patch harbour is given
a -150% production bonus; building this you have a net -50% production
bonus and thus, a negative production (and surplus, of course).

When this happens there are several problems:

- The client is not correctly informed of the negative production.  This
is because the shield_prod and other values are unsigned values in
packets.def.  This symptom is hidden if you use CMA since then
generic_city_refresh is called at the client and recalculates this data.

- The server will crash in an auto_arrange_workers on a human player. 
The final fallback (cm_init_emergency_parameter) fails to find a valid
CM result so the assert fails (with assertions disabled the aaw will
simply have no effect).

Neither is particularly easy to fix.  My solution therefore is to impose
a minimum of 0 for all bonuses.  The attached patch does that for 2.0.

The problem is even worse in the development version, because a negative
bonus is possible for trade and this causes a negative value to be
passed to distribute which causes major problems.  The fix is the same,
of course.  However a separate fix for cm.c is needed.  The attached
patches show the problem and provide this fix.

-jason

? data/flags/new
? data/flags/old
Index: data/default/effects.ruleset
===================================================================
RCS file: /home/freeciv/CVS/freeciv/data/default/effects.ruleset,v
retrieving revision 1.12
diff -p -u -r1.12 effects.ruleset
--- data/default/effects.ruleset        22 Aug 2005 21:15:49 -0000      1.12
+++ data/default/effects.ruleset        28 Sep 2005 04:17:58 -0000
@@ -16,6 +16,14 @@ options="1.0"
 ; /* <-- avoid gettext warnings
 ; */ <-- avoid gettext warnings
 
+[effect_negative]
+name = "Output_Bonus"
+value = -150
+reqs =
+    {
+
+    }
+
 ; Barbarian effects
 
 [effect_barb1]

Index: common/city.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.c,v
retrieving revision 1.249.2.8
diff -p -u -r1.249.2.8 city.c
--- common/city.c       1 Mar 2005 20:04:53 -0000       1.249.2.8
+++ common/city.c       28 Sep 2005 04:10:55 -0000
@@ -1569,7 +1569,9 @@ static int content_citizens(struct playe
 **************************************************************************/
 int get_city_shield_bonus(const struct city *pcity)
 {
-  return (100 + get_city_bonus(pcity, EFT_PROD_BONUS));
+  const int bonus = 100 + get_city_bonus(pcity, EFT_PROD_BONUS);
+
+  return MAX(bonus, 0);
 }
 
 /**************************************************************************
@@ -1577,7 +1579,9 @@ int get_city_shield_bonus(const struct c
 **************************************************************************/
 int get_city_tax_bonus(const struct city *pcity)
 {
-  return (100 + get_city_bonus(pcity, EFT_TAX_BONUS));
+  const int bonus = 100 + get_city_bonus(pcity, EFT_TAX_BONUS);
+
+  return MAX(bonus, 0);
 }
 
 /**************************************************************************
@@ -1585,7 +1589,9 @@ int get_city_tax_bonus(const struct city
 **************************************************************************/
 int get_city_luxury_bonus(const struct city *pcity)
 {
-  return (100 + get_city_bonus(pcity, EFT_LUXURY_BONUS));
+  const int bonus = 100 + get_city_bonus(pcity, EFT_LUXURY_BONUS);
+
+  return MAX(bonus, 0);
 }
 
 /**************************************************************************
@@ -1620,7 +1626,7 @@ int get_city_science_bonus(const struct 
     science_bonus /= 2;
   }
 
-  return science_bonus;
+  return MAX(science_bonus, 0);
 }
 
 /**************************************************************************

? data/amplio
? data/amplio.tilespec
Index: data/default/buildings.ruleset
===================================================================
RCS file: /home/freeciv/CVS/freeciv/data/default/buildings.ruleset,v
retrieving revision 1.57.2.5
diff -p -u -r1.57.2.5 buildings.ruleset
--- data/default/buildings.ruleset      23 Jul 2005 07:34:23 -0000      1.57.2.5
+++ data/default/buildings.ruleset      28 Sep 2005 04:01:00 -0000
@@ -543,7 +543,7 @@ The amount of stored food will be set to
 
 [building_harbour]
 name           = _("Harbour")
-tech_req       = "Seafaring"
+tech_req       = "None"
 bldg_req       = "None"
 graphic        = "b.harbour"
 graphic_alt    = "-"
@@ -554,12 +554,13 @@ equiv_range       = "City"
 ;equiv_repl    =
 obsolete_by    = "None"
 is_wonder      = 0
-build_cost     = 40
+build_cost     = 5
 upkeep         = 1
 sabotage       = 100
 effect         =
     { "name", "value", "req_type", "req"
        "Food_Add_Tile", 1, "Terrain", "Ocean"
+       "Prod_Bonus", -150
     }
 sound          = "b_harbour"
 sound_alt      = "b_generic"

Index: common/city.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.c,v
retrieving revision 1.364
diff -p -u -r1.364 city.c
--- common/city.c       18 Aug 2005 06:44:27 -0000      1.364
+++ common/city.c       28 Sep 2005 04:31:49 -0000
@@ -1467,7 +1467,7 @@ int get_city_output_bonus(const struct c
   int bonus2 = 100 + get_city_tile_output_bonus(pcity, NULL, output,
                                                EFT_OUTPUT_BONUS_2);
 
-  return bonus1 * bonus2 / 100;
+  return MAX(bonus1 * bonus2 / 100, 0);
 }
 
 /**************************************************************************
Index: common/aicore/cm.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/aicore/cm.c,v
retrieving revision 1.67
diff -p -u -r1.67 cm.c
--- common/aicore/cm.c  4 Jul 2005 17:48:37 -0000       1.67
+++ common/aicore/cm.c  28 Sep 2005 04:31:51 -0000
@@ -1546,7 +1546,11 @@ static void init_min_production(struct c
      * 3.  Subtract off any "free" production (trade routes, tithes, and
      *     city-center). */
     min = pcity->usage[o] + state->parameter.minimal_surplus[o];
-    min = min * 100 / pcity->bonus[o];
+    if (pcity->bonus[o] > 0) {
+      /* FIXME: how does this work if the bonus is 0?  Then no production
+       * is possible and we can probably short-cut everything. */
+      min = min * 100 / pcity->bonus[o];
+    }
     city_map_iterate(x, y) {
       if (is_free_worked_tile(x, y)) {
        min -= base_city_get_output_tile(x, y, pcity, is_celebrating, o);