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

> Can you make version for other branches (S2_2 and TRUNK) too?

Attached is a straight-ish port to S2_2 (auto-farmland-ter-2-2.diff). It
is functional, but I'm unhappy with it, as it's no longer robust :(

On S2_2, various tests (e.g., is_city_center()) have been modified to
use pointer comparison on (struct tile *), in change 14416 (PR#40104).
This means that p_dummy_tile can't be passed to such functions; while
I've defused the two instances that appear directly in
city_tile_output(), thus making the patch apparently functional, and I
haven't found any instances in currently called functions, this code is
not robust against invocations of is_city_center() and similar appearing
in future (e.g., deep within the effects system).

I don't see an easy way out of this, unless the policy of using pointer
comparison is reversed. Temporarily modifying the tile in-place seems
evil (especially as the comments for city_tile_output() make it clear
that it can be called speculatively).

I shan't attempt to port to the trunk until this issue with S2_2 is
resolved.

Index: common/city.c
===================================================================
--- common/city.c       (revision 14693)
+++ common/city.c       (working copy)
@@ -710,9 +710,12 @@
 int city_tile_output(const struct city *pcity, const struct tile *ptile,
                     bool is_celebrating, Output_type_id otype)
 {
-  struct tile tile;
+  struct tile dummy_tile = *ptile;      /* temporary dummy tile for city
+                                         * center bonuses */
+  const struct tile *p_dummy_tile = &dummy_tile;
+                                        /* drop-in ptile replacement */
   int prod;
-  struct terrain *pterrain = tile_terrain(ptile);
+  struct terrain *pterrain = tile_terrain(p_dummy_tile);
 
   assert(otype >= 0 && otype < O_LAST);
 
@@ -723,42 +726,41 @@
   }
 
   prod = pterrain->output[otype];
-  if (tile_resource_is_valid(ptile)) {
-    prod += tile_resource(ptile)->output[otype];
+  if (tile_resource_is_valid(p_dummy_tile)) {
+    prod += tile_resource(p_dummy_tile)->output[otype];
   }
 
-  /* create dummy tile which has the city center bonuses. */
-  tile.terrain = pterrain;
-  tile.special = tile_specials(ptile);
-
+  /* add city center bonuses to temporary copy of tile. */
   if (NULL != pcity
-      && is_city_center(pcity, ptile)
+      && is_city_center(pcity, ptile) /* relies on pointer comparison */
       && pterrain == pterrain->irrigation_result
       && terrain_control.may_irrigate) {
-    /* The center tile is auto-irrigated. */
-    tile_set_special(&tile, S_IRRIGATION);
+    /* The center tile is auto-irrigated, for the purposes of food
+     * production. */
+    tile_set_special(&dummy_tile, S_IRRIGATION);
 
     if (player_knows_techs_with_flag(city_owner(pcity), TF_FARMLAND)) {
-      tile_set_special(&tile, S_FARMLAND);
+      tile_set_special(&dummy_tile, S_FARMLAND);
     }
   }
 
   switch (otype) {
   case O_SHIELD:
-    if (contains_special(tile.special, S_MINE)) {
+    if (tile_has_special(p_dummy_tile, S_MINE)) {
       prod += pterrain->mining_shield_incr;
     }
     break;
   case O_FOOD:
-    if (contains_special(tile.special, S_IRRIGATION)) {
+    if (tile_has_special(p_dummy_tile, S_IRRIGATION)) {
       prod += pterrain->irrigation_food_incr;
     }
     break;
   case O_TRADE:
-    if (contains_special(tile.special, S_RIVER) && !is_ocean(tile.terrain)) {
+    if (tile_has_special(p_dummy_tile, S_RIVER)
+        && !is_ocean_tile(p_dummy_tile)) {
       prod += terrain_control.river_trade_incr;
     }
-    if (contains_special(tile.special, S_ROAD)) {
+    if (tile_has_special(p_dummy_tile, S_ROAD)) {
       prod += pterrain->road_trade_incr;
     }
     break;
@@ -769,28 +771,29 @@
     break;
   }
 
-  if (contains_special(tile.special, S_RAILROAD)) {
+  if (tile_has_special(p_dummy_tile, S_RAILROAD)) {
     prod += (prod * terrain_control.rail_tile_bonus[otype]) / 100;
   }
 
   if (pcity) {
     const struct output_type *output = &output_types[otype];
 
-    prod += get_city_tile_output_bonus(pcity, ptile, output,
+    prod += get_city_tile_output_bonus(pcity, p_dummy_tile, output,
                                       EFT_OUTPUT_ADD_TILE);
     if (prod > 0) {
-      int penalty_limit = get_city_tile_output_bonus(pcity, ptile, output,
+      int penalty_limit = get_city_tile_output_bonus(pcity, p_dummy_tile,
+                                                   output,
                                                    EFT_OUTPUT_PENALTY_TILE);
 
       if (is_celebrating) {
-        prod += get_city_tile_output_bonus(pcity, ptile, output,
+        prod += get_city_tile_output_bonus(pcity, p_dummy_tile, output,
                                            EFT_OUTPUT_INC_TILE_CELEBRATE);
         penalty_limit = 0; /* no penalty if celebrating */
       }
-      prod += get_city_tile_output_bonus(pcity, ptile, output,
+      prod += get_city_tile_output_bonus(pcity, p_dummy_tile, output,
                                          EFT_OUTPUT_INC_TILE);
       prod += (prod 
-               * get_city_tile_output_bonus(pcity, ptile, output,
+               * get_city_tile_output_bonus(pcity, p_dummy_tile, output,
                                             EFT_OUTPUT_PER_TILE)) 
               / 100;
       if (!is_celebrating && penalty_limit > 0 && prod > penalty_limit) {
@@ -799,11 +802,11 @@
     }
   }
 
-  if (contains_special(tile.special, S_POLLUTION)) {
+  if (tile_has_special(p_dummy_tile, S_POLLUTION)) {
     prod -= (prod * terrain_control.pollution_tile_penalty[otype]) / 100;
   }
 
-  if (contains_special(tile.special, S_FALLOUT)) {
+  if (tile_has_special(p_dummy_tile, S_FALLOUT)) {
     prod -= (prod * terrain_control.fallout_tile_penalty[otype]) / 100;
   }
 
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

Reply via email to