<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
[email protected]
https://mail.gna.org/listinfo/freeciv-dev
