Fix penalizing buy-then-change-production

When a player buys a unit that cannot be built then switches to
building something else, penalize half of the shields that were bought.
This changes the previous overjealous behaviour where half the total
amount of shields would be lost.

Closes #1915.

client/packhand.cpp

@@ -791,7 +791,7 @@ void handle_city_info(const struct packet_city_info *packet)
 
   pcity->airlift = packet->airlift;
   pcity->did_buy = packet->did_buy;
-  pcity->did_buy_production = packet->did_buy_production;
+  pcity->bought_shields = packet->bought_shields;
   pcity->did_sell = packet->did_sell;
   pcity->was_happy = packet->was_happy;
 

common/city.cpp

@@ -1828,12 +1828,12 @@ int city_change_production_penalty(const struct city *pcity,
     penalized_shields = pcity->before_change_shields;
   }
 
-  // Penalize 50% if you buy unit last turn but didnt built
-  if ((pcity->did_buy_production
+  // Penalize 50% if you bought a unit but didn't build it
+  if ((pcity->bought_shields > 0
        && ((target->kind == VUT_UTYPE
             && target->value.utype != pcity->changed_from.value.utype)))) {
-    unpenalized_shields = 0;
-    penalized_shields = pcity->before_change_shields;
+    unpenalized_shields -= pcity->bought_shields;
+    penalized_shields += pcity->bought_shields;
   }
 
   /* Do not put penalty on these. It shouldn't matter whether you disband
@@ -1848,6 +1848,7 @@ int city_change_production_penalty(const struct city *pcity,
     penalized_shields += pcity->caravan_shields;
   }
 
+  qCritical() << unpenalized_shields << penalized_shields;
   shield_stock_after_adjustment =
       unpenalized_shields + penalized_shields / 2;
 
@@ -3329,7 +3330,7 @@ struct city *create_city_virtual(struct player *pplayer, struct tile *ptile,
 
   pcity->turn_plague = -1; // -1 = never
   pcity->did_buy = false;
-  pcity->did_buy_production = false;
+  pcity->bought_shields = 0;
   pcity->city_radius_sq = game.info.init_city_radius_sq;
   pcity->turn_founded = game.info.turn;
   pcity->turn_last_built = game.info.turn;

common/city.h

@@ -339,8 +339,8 @@ struct city {
 
   // turn states
   int airlift;
+  int bought_shields;
   bool did_buy;
-  bool did_buy_production;
   bool did_sell;
   bool was_happy;
 

common/networking/packets.def

@@ -730,9 +730,12 @@ PACKET_CITY_INFO = 31; sc, lsend, is-game-info, force, cancel(PACKET_CITY_SHORT_
   UINT16 disbanded_shields;
   UINT16 caravan_shields;
   UINT16 last_turns_shield_surplus;
+  UINT16 bought_shields; add-cap(bought-shields)
 
   UINT8 airlift;
-  BOOL did_buy, did_buy_production, did_sell, was_happy;
+  BOOL did_buy;
+  BOOL did_buy_production; remove-cap(bought-shields)
+  BOOL did_sell, was_happy;
 
   BOOL diplomat_investigate;
   UINT8 walls;

server/cityhand.cpp

@@ -336,10 +336,9 @@ void really_handle_city_buy(struct player *pplayer, struct city *pcity)
   if (pcity->shield_stock < total) {
     /* As we never put penalty on disbanded_shields, we can
      * fully well add the missing shields there. */
-    pcity->disbanded_shields += total - pcity->shield_stock;
+    pcity->bought_shields += total - pcity->shield_stock;
     pcity->shield_stock = total; // AI wants this -- Syela
     pcity->did_buy = true;       // !PS: no need to set buy flag otherwise
-    pcity->did_buy_production = true;
   }
   city_refresh(pcity);
 

server/citytools.cpp

@@ -2571,13 +2571,14 @@ void package_city(struct city *pcity, struct packet_city_info *packet,
   packet->disbanded_shields = pcity->disbanded_shields;
   packet->caravan_shields = pcity->caravan_shields;
   packet->last_turns_shield_surplus = pcity->last_turns_shield_surplus;
+  packet->bought_shields = pcity->bought_shields;
 
   worklist_copy(&packet->worklist, &pcity->worklist);
   packet->diplomat_investigate = dipl_invest;
 
   packet->airlift = pcity->airlift;
   packet->did_buy = pcity->did_buy;
-  packet->did_buy_production = pcity->did_buy_production;
+  packet->did_buy_production = pcity->bought_shields > 0;
   packet->did_sell = pcity->did_sell;
   packet->was_happy = pcity->was_happy;
 

server/cityturn.cpp

@@ -2373,10 +2373,10 @@ static bool city_build_building(struct player *pplayer, struct city *pcity)
       wonder_set_build_turn(pplayer, pimprove);
     }
     cost = impr_build_shield_cost(pcity, pimprove);
+    pcity->bought_shields = 0;
     pcity->before_change_shields -= cost;
     pcity->shield_stock -= cost;
     pcity->turn_last_built = game.info.turn;
-    pcity->did_buy_production = false;
     // to eliminate micromanagement
     if (is_great_wonder(pimprove)) {
       notify_player(nullptr, city_tile(pcity), E_WONDER_BUILD, ftc_server,
@@ -2575,7 +2575,6 @@ static bool city_build_unit(struct player *pplayer, struct city *pcity)
 
     // don't update turn_last_built if we returned above
     pcity->turn_last_built = game.info.turn;
-    pcity->did_buy_production = false;
     // check if we can build more than one unit (effect City_Build_Slots)
     (void) city_production_build_units(pcity, false, &num_units);
 
@@ -2614,6 +2613,7 @@ static bool city_build_unit(struct player *pplayer, struct city *pcity)
       // to eliminate micromanagement, we only subtract the unit's cost
       pcity->before_change_shields -= unit_shield_cost;
       pcity->shield_stock -= unit_shield_cost;
+      pcity->bought_shields = 0;
 
       if (pop_cost > 0) {
         // Additional message if the unit has population cost.

server/savegame/savegame3.cpp

@@ -4809,8 +4809,8 @@ static bool sg_load_player_city(struct loaddata *loading, struct player *plr,
                                       "%s.did_buy", citystr),
                   false, "%s", secfile_error());
   // May not be present in older saves
-  pcity->did_buy_production = secfile_lookup_bool_default(
-      loading->file, false, "%s.did_buy_production", citystr);
+  pcity->bought_shields = secfile_lookup_int_default(
+      loading->file, 0, "%s.bought_shields", citystr);
   sg_warn_ret_val(secfile_lookup_bool(loading->file, &pcity->did_sell,
                                       "%s.did_sell", citystr),
                   false, "%s", secfile_error());
@@ -5263,8 +5263,6 @@ static void sg_save_player_cities(struct savedata *saving,
     secfile_insert_int(saving->file, pcity->turn_founded, "%s.turn_founded",
                        buf);
     secfile_insert_bool(saving->file, pcity->did_buy, "%s.did_buy", buf);
-    secfile_insert_bool(saving->file, pcity->did_buy_production,
-                        "%s.did_buy_production", buf);
     secfile_insert_bool(saving->file, pcity->did_sell, "%s.did_sell", buf);
     secfile_insert_int(saving->file, pcity->turn_last_built,
                        "%s.turn_last_built", buf);
@@ -5287,6 +5285,8 @@ static void sg_save_player_cities(struct savedata *saving,
 
     secfile_insert_int(saving->file, pcity->before_change_shields,
                        "%s.before_change_shields", buf);
+    secfile_insert_int(saving->file, pcity->bought_shields,
+                       "%s.bought_shields", buf);
     secfile_insert_int(saving->file, pcity->caravan_shields,
                        "%s.caravan_shields", buf);
     secfile_insert_int(saving->file, pcity->disbanded_shields,

utility/fc_version.h.in

@@ -17,7 +17,8 @@
 #endif
 
 #define NETWORK_CAPSTRING                                                   \
-  "+Freeciv21.21April13 killunhomed-is-game-info player-intel-visibility"
+  "+Freeciv21.21April13 killunhomed-is-game-info player-intel-visibility " \
+  "bought-shields"
 
 #ifndef FOLLOWTAG
 #define FOLLOWTAG "S_HAXXOR"