Re: [Freeciv-Dev] (PR#40207) Should cities with Supermarket automatically get farmland benefit?

[Marko Lindqvist]

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

2009/5/4 Marko Lindqvist cazf...@gmail.com:
 2008/8/12 jacobn+freeci...@chiark.greenend.org.uk
 jacobn+freeci...@chiark.greenend.org.uk:

 In June, I wrote:
 Madeline Book writes:
  and for 2.2+ to expand the effect system to allow support for
  auto-irrigation and auto-farmland of the city center.

 I'll see if I can find time to have a go at this.

 I've finally found a bit of time. Draft attached. It continues to be a
 can of worms.

  Since no better alternative has shown up, we are going forward with
 this patch. I did some updates and fixes.

  - Updated against svn
  - Renamed is_terrain_alteration_in_range() as
 is_terrain_alter_possible_in_range()
  - Renamed VUT_CITYCENTER as VUT_CITYTILE and it now has type telling
 which kind of citytile is required. Currently only Center is
 supported
  - Target city to VUT_CITYTILE requirement is now optional
  - Added effect to civ2 ruleset also

 - Updated against svn
 - Fixed comparison of CityTile requirements
 - Corrected documentation (README.effects)


 - ML

diff -Nurd -X.diff_ignore freeciv/client/helpdata.c freeciv/client/helpdata.c
--- freeciv/client/helpdata.c   2009-01-25 16:51:10.0 +0200
+++ freeciv/client/helpdata.c   2009-05-12 03:05:47.0 +0300
@@ -234,6 +234,13 @@
  cat_snprintf(buf, bufsz, _(Requires %s terrain.\n\n),
   
terrain_class_name_translation(req-source.value.terrainclass));
  return;
+  case VUT_TERRAINALTER:
+ cat_snprintf(buf, bufsz, _(Requires terrain on which %s can be 
built.\n\n),
+  
terrain_alteration_name_translation(req-source.value.terrainalter));
+ return;
+  case VUT_CITYTILE:
+ cat_snprintf(buf, bufsz, _(Applies only to city centers.\n\n));
+ return;
   case VUT_LAST:
   default:
 break;
diff -Nurd -X.diff_ignore freeciv/common/city.c freeciv/common/city.c
--- freeciv/common/city.c   2008-11-09 00:26:56.0 +0200
+++ freeciv/common/city.c   2009-05-12 03:05:47.0 +0300
@@ -710,7 +710,6 @@
 int city_tile_output(const struct city *pcity, const struct tile *ptile,
 bool is_celebrating, Output_type_id otype)
 {
-  struct tile tile;
   int prod;
   struct terrain *pterrain = tile_terrain(ptile);
 
@@ -727,38 +726,27 @@
 prod += tile_resource(ptile)-output[otype];
   }
 
-  /* create dummy tile which has the city center bonuses. */
-  tile.terrain = pterrain;
-  tile.special = tile_specials(ptile);
-
-  if (NULL != pcity
-   is_city_center(pcity, ptile)
-   pterrain == pterrain-irrigation_result
-   terrain_control.may_irrigate) {
-/* The center tile is auto-irrigated. */
-tile_set_special(tile, S_IRRIGATION);
-
-if (player_knows_techs_with_flag(city_owner(pcity), TF_FARMLAND)) {
-  tile_set_special(tile, S_FARMLAND);
-}
-  }
-
   switch (otype) {
   case O_SHIELD:
-if (contains_special(tile.special, S_MINE)) {
+if (tile_has_special(ptile, S_MINE)) {
   prod += pterrain-mining_shield_incr;
 }
 break;
   case O_FOOD:
-if (contains_special(tile.special, S_IRRIGATION)) {
+/* The city center tile is auto-irrigated. */
+if (tile_has_special(ptile, S_IRRIGATION)
+|| (NULL != pcity
+ is_city_center(pcity, ptile)
+ pterrain == pterrain-irrigation_result
+ terrain_control.may_irrigate)) {
   prod += pterrain-irrigation_food_incr;
 }
 break;
   case O_TRADE:
-if (contains_special(tile.special, S_RIVER)  !is_ocean(tile.terrain)) {
+if (tile_has_special(ptile, S_RIVER)  !is_ocean_tile(ptile)) {
   prod += terrain_control.river_trade_incr;
 }
-if (contains_special(tile.special, S_ROAD)) {
+if (tile_has_special(ptile, S_ROAD)) {
   prod += pterrain-road_trade_incr;
 }
 break;
@@ -769,7 +757,7 @@
 break;
   }
 
-  if (contains_special(tile.special, S_RAILROAD)) {
+  if (tile_has_special(ptile, S_RAILROAD)) {
 prod += (prod * terrain_control.rail_tile_bonus[otype]) / 100;
   }
 
@@ -799,11 +787,11 @@
 }
   }
 
-  if (contains_special(tile.special, S_POLLUTION)) {
+  if (tile_has_special(ptile, S_POLLUTION)) {
 prod -= (prod * terrain_control.pollution_tile_penalty[otype]) / 100;
   }
 
-  if (contains_special(tile.special, S_FALLOUT)) {
+  if (tile_has_special(ptile, S_FALLOUT)) {
 prod -= (prod * terrain_control.fallout_tile_penalty[otype]) / 100;
   }
 
@@ -2633,3 +2621,15 @@
 
   return pcity != game_find_city_by_number(pcity-id);
 }
+
+/**
+  Return citytile type for a given rule name
+**/
+enum citytile_type find_citytile_by_rule_name(const char *name)
+{
+  if (!mystrcasecmp(name, center)) {
+return CITYT_CENTER;
+  }
+
+  return CITYT_LAST;
+}
diff -Nurd -X.diff_ignore freeciv/common/city.h freeciv/common/city.h
--- 

Re: [Freeciv-Dev] (PR#40207) Should cities with Supermarket automatically get farmland benefit?

[Marko Lindqvist]

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

2008/8/12 [EMAIL PROTECTED]:

 In June, I wrote:
 Madeline Book writes:
  and for 2.2+ to expand the effect system to allow support for
  auto-irrigation and auto-farmland of the city center.

 I'll see if I can find time to have a go at this.

 I've finally found a bit of time. Draft attached. It continues to be a
 can of worms.

 I've realised I need _another_ universal type which will allow me to
 restrict the auto-farmland effect to irrigable terrains. Having a
 source which allows the effects system to use this part of the terrain
 ruleset seems like a generally useful thing, though, so I've done
 that. The result (VUT_TERRAINALTER) is somewhat clumsy -- its name,
 for one thing -- but appears to work. Suggestions for improvement
 welcome.

 I really don't like the way this pollutes namespaces with entries
that are used for this one gola only and not otherwise usable. It's
not big problem internally, but for requirement names and the likes
that affect external ruleset format. Still, this is fixing a bug we
really want to get rid of (micromanaging your hidden city centers
should not be necessary, or rewarded). Since nobody seems to invent
any better methdos, I guess we go with this one.

 It might be less ugly if we split it to smaller pieces and
concentrate on making one piece correct at a time (this has worked
with most such ugly-at-first patches).

 Obviously acceptable part of this patch is
contains_special(ptile.special...) - tile_has_special(ptile... )
cleanup. If you can separate that from the rest of the patch, I'm
ready to commit it quickly.

 About terrain alteration requirements: naming is confusing when for
example is_terrain_alteration_in_range is not checking for tiles
where terrain alteration *is*, but tiles where terrain alteration
*could* exist. This needs some work.


 - ML



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

Re: [Freeciv-Dev] (PR#40207) Should cities with Supermarket automatically get farmland benefit?

[jacobn+freeci...@chiark.greenend.org.uk]

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

In June, I wrote:
 Madeline Book writes:
  and for 2.2+ to expand the effect system to allow support for
  auto-irrigation and auto-farmland of the city center.
 
 I'll see if I can find time to have a go at this.

I've finally found a bit of time. Draft attached. It continues to be a
can of worms.

I've realised I need _another_ universal type which will allow me to
restrict the auto-farmland effect to irrigable terrains. Having a
source which allows the effects system to use this part of the terrain
ruleset seems like a generally useful thing, though, so I've done
that. The result (VUT_TERRAINALTER) is somewhat clumsy -- its name,
for one thing -- but appears to work. Suggestions for improvement
welcome.

The universal I've added for target-city-is-on-target-tile
(VUT_CITYCENTER), while suitable for the job in hand, feels a bit wrong
-- it has no parameters and only one valid range. I can't think of any
way to generalise it, though. I've wondered about instead extending the
existing VUT_MINSIZE to allow use in tile output contexts (here we'd use
MinSize=1); would that be unnacceptably gross?

On the plus side, once all the bits are in place, it becomes easier to
move various bits of hard-coded stuff from city_tile_output() to the
effects system, if that's desirable:

 - auto-irrigation of city centres (requires new effect
   Apply_Irrigation_Bonus or similar)

 - min_city_center_output[] (requires new effect Output_Min_Tile)

(I don't intend to actually take these steps yet, however.)

Testing: I've done some ad-hoc testing to check it works as desired
and haven't spotted any bad side effects. I also did some
before-and-after autogame testing (with the new bit of effects.ruleset
commented out), which diverged after 107 turns for no reason I can
work out (it's not around the discovery of Refrigeration or anything
like that). 

I expect this will need some beating up before it can be committed. I
should be able to respond to comments for the next two weeks.

Index: doc/README.effects
===
--- doc/README.effects  (revision 15124)
+++ doc/README.effects  (working copy)
@@ -40,9 +40,11 @@
 
 A requirement type is the type of the requirement and can be one of None 
 (default), Tech, Gov, Building, Special, Terrain, UnitType, 
-UnitFlag, UnitClass, Nation, OutputType, MinSize, AI and
-TerrainClass. MinSize is the minimum size of a city required. AI is ai
-player difficulty level. TerrainClass is either Land or Oceanic.
+UnitFlag, UnitClass, Nation, OutputType, MinSize, AI,
+TerrainClass, TerrainAlter, or CityCenter. MinSize is the minimum
+size of a city required. AI is ai player difficulty level. TerrainClass
+is either Land or Oceanic. CityCenter takes no argument and is true
+if the target city is on the target tile.
 
 Effect types 
 
Index: server/cityturn.c
===
--- server/cityturn.c   (revision 15124)
+++ server/cityturn.c   (working copy)
@@ -1046,6 +1046,8 @@
case VUT_UCFLAG:
case VUT_OTYPE:
case VUT_SPECIALIST:
+   case VUT_TERRAINALTER: /* XXX could do this in principle */
+   case VUT_CITYCENTER:
  /* Will only happen with a bogus ruleset. */
  freelog(LOG_ERROR, worklist_change_build_target()
   has bogus preq);
Index: server/ruleset.c
===
--- server/ruleset.c(revision 15124)
+++ server/ruleset.c(working copy)
@@ -3733,6 +3733,8 @@
  case VUT_MINSIZE: /* Breaks nothing, but has no sense either */
  case VUT_AI_LEVEL:
  case VUT_TERRAINCLASS:
+ case VUT_TERRAINALTER: /* Local range only */
+ case VUT_CITYCENTER:
/* There can be only one requirement of these types (with current
 * range limitations)
 * Requirements might be identical, but we consider multiple
Index: data/default/effects.ruleset
===
--- data/default/effects.ruleset(revision 15124)
+++ data/default/effects.ruleset(working copy)
@@ -1543,6 +1543,17 @@
   OutputType, Food, Local
 }
 
+[effect_supermarket_2]
+name   = Output_Per_Tile
+value  = 50
+reqs   =
+{ type, name, range
+  CityCenter, , Local
+  TerrainAlter, CanIrrigate, Local
+  Building, Supermarket, City
+  OutputType, Food, Local
+}
+
 [effect_temple]
 name   = Make_Content
 value  = 1
Index: common/city.c
===
--- common/city.c   (revision 15124)
+++ common/city.c   (working copy)
@@ -710,7 +710,6 @@
 int city_tile_output(const struct city *pcity, const struct tile *ptile,
 bool is_celebrating, Output_type_id otype)
 {
-  struct tile tile;
   int prod;
   struct terrain 

Re: [Freeciv-Dev] (PR#40207) Should cities with Supermarket automatically get farmland benefit?

[Marko Lindqvist]

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

2008/8/12 [EMAIL PROTECTED]:
  I also did some
 before-and-after autogame testing (with the new bit of effects.ruleset
 commented out), which diverged after 107 turns for no reason I can
 work out (it's not around the discovery of Refrigeration or anything
 like that).

 I'm afraid we have a difficult to track memory problem somewhere.
I've got autogames to differ even with same (not recompiled) binary.
Valgrind is not giving any warnings :-(


 - ML



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev