[Freeciv-Dev] Re: (PR#14248) Radar station

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

Guest wrote:
> <URL: http://bugs.freeciv.org/Ticket/Display.html?id=14248 >
> 
> Hi
> 
> I have played with the version.diff patch. I have one comment:
> 
> - Why does a city with the vision radius 3 have non-square vision area, 
> while a unit with the vision radius 3 has a square vision area ? 

No good reason.  Unless there is great objection I plan to change units 
to use circular vision (note that vision=1 and vision=2 units are both 
circles).

> - As for me, SQ units are quite unclear. Suppose, we have two 
> improvements, each of which adds 5 SQ units to the base of value of 5. 
> Regardless of the improvement we build, we obtain 10 which will give us 
> 10, i.e. vision radius is 3. However, having built two buildings, we 
> will have the same vision radius since the sum is 15. Is it logical ? 
> In other words, there can be a case when the Radar Station does not 
> increase the vision radius because there is another building that has 
> already increased it. Personally, radius/diameter works more 
> predictable.

A vision radius_sq of 15 is not the same as that of 10.  The latter has 
radius 3.17 while the former has radius 3.88.

  +***+
+*****+
*******
***X***
*******
+*****+
  +***+

Additionally using a simple radius makes it harder to do things like

  ***
  *X*  <- musketeers: radius_sq 2, radius 1.42
  ***

  *****
  *****
  **X** <- destroyer: radius_sq 8, radius 2.83
  *****
  *****

we could work in terms of radius, allowing decimal values (as you do) 
but this means as the values get summed together the city gets to see 
quadratically more tiles (increasing vision range from n to n+1 
increases vision from n^2 to (n+1)^2), so I think it isn't balanced as well.

However in both cases you can get whatever behavior you want, by use of 
nreqs and multiple effects.  For instance you can have city walls 
provide +5 vision_radius_sq, but only when airport isn't present.  Then 
have airport provide +12 vision_radius_sq.  Now:

   *walls: 5+5 = 10
   *airport: 5+12 = 17
   *walls+airport: 5+12 = 17

> - Shouldn't we specify in a ruleset the percents by which the base 
> value is increased ? As for me, such an approach works better in those 
> cases when the base value changes for some reason.

Just to reiterate, the base value should be controlled in the ruleset in 
the same way.  In fact, here's a patch that does that.  I also removed 
the LOG_NORMAL, removed the extra ruleset changes, and merged two 
functions in maphand.c.  This one should be ready to be committed shortly.

-jason

Index: server/citytools.c
===================================================================
--- server/citytools.c  (revision 11108)
+++ server/citytools.c  (working copy)
@@ -795,7 +795,8 @@
   } built_impr_iterate_end;
 
   give_citymap_from_player_to_player(pcity, pgiver, ptaker);
-  map_unfog_pseudo_city_area(ptaker, pcity->tile);
+  map_refog_circle(ptaker, pcity->tile,
+                  -1, pcity->server.vision_radius_sq, TRUE);
 
   sz_strlcpy(old_city_name, pcity->name);
   if (game.info.allowed_city_names == 1
@@ -903,7 +904,8 @@
     update_tile_knowledge(pcity->tile);
   }
 
-  map_fog_pseudo_city_area(pgiver, pcity->tile);
+  map_refog_circle(pgiver, pcity->tile,
+                  pcity->server.vision_radius_sq, -1, TRUE);
 
   /* Build a new palace for free if the player lost her capital and
      savepalace is on. */
@@ -965,7 +967,7 @@
   }
 
   /* Before arranging workers to show unknown land */
-  map_unfog_pseudo_city_area(pplayer, ptile);
+  map_unfog_city_area(pcity);
 
   tile_set_city(ptile, pcity);
 
@@ -1054,6 +1056,7 @@
   struct tile *ptile = pcity->tile;
   bv_imprs had_small_wonders;
   char *city_name = mystrdup(pcity->name);
+  int old_vision_range;
 
   BV_CLR_ALL(had_small_wonders);
   built_impr_iterate(pcity, i) {
@@ -1139,6 +1142,8 @@
      alive in the client. As the number of removed cities is small the leak is
      acceptable. */
 
+  old_vision_range = pcity->server.vision_radius_sq;
+  pcity->server.vision_radius_sq = -1;
   game_remove_city(pcity);
   map_update_borders_city_destroyed(ptile);
 
@@ -1148,7 +1153,7 @@
     }
   } players_iterate_end;
 
-  map_fog_pseudo_city_area(pplayer, ptile);
+  map_refog_circle(pplayer, ptile, old_vision_range, -1, TRUE);
 
   /* Update available tiles in adjacent cities. */
   map_city_radius_iterate(ptile, tile1) {
@@ -1801,6 +1806,10 @@
      * the spaceship is lost. */
     spaceship_lost(owner);
   }
+
+  /* Re-update the city's visible area.  This updates fog if the vision
+   * range increases or decreases. */
+  map_unfog_city_area(pcity);
 }
 
 /**************************************************************************
Index: server/cityturn.c
===================================================================
--- server/cityturn.c   (revision 11108)
+++ server/cityturn.c   (working copy)
@@ -1120,6 +1120,9 @@
                       API_TYPE_BUILDING_TYPE, get_improvement_type(id),
                       API_TYPE_CITY, pcity);
 
+    /* Call this function since some buildings may change the
+     * the vision range of a city */
+    map_unfog_city_area(pcity);
 
     if ((mod = get_current_construction_bonus(pcity, EFT_GIVE_IMM_TECH))) {
       int i;
Index: server/maphand.c
===================================================================
--- server/maphand.c    (revision 11108)
+++ server/maphand.c    (working copy)
@@ -732,29 +732,40 @@
 }
 
 /**************************************************************************
-...
+  Changes the vision range of the city.  The new range is given in
+  the radius_sq.
 **************************************************************************/
-void map_fog_city_area(struct city *pcity)
+static void map_refog_city_area(struct city *pcity, int new_radius_sq)
 {
-  if (!pcity) {
-    freelog(LOG_ERROR, "Attempting to fog non-existent city");
-    return;
+  if (pcity) {
+    map_refog_circle(city_owner(pcity), pcity->tile,
+                    pcity->server.vision_radius_sq, new_radius_sq, TRUE);
+    pcity->server.vision_radius_sq = new_radius_sq;
+  } else {
+    freelog(LOG_ERROR, "Attempting to change fog for non-existent city");
   }
+}
 
-  map_fog_pseudo_city_area(city_owner(pcity), pcity->tile);
+/**************************************************************************
+  Fogs the area visible by the city.  Unlike other unfog functions this
+  one may be called multiple times in a row without any penalty.  Call it
+  before destroying the city (or go straight to the source and use
+  map_refog_circle by hand).
+**************************************************************************/
+void map_fog_city_area(struct city *pcity)
+{
+  map_refog_city_area(pcity, -1);
 }
 
 /**************************************************************************
-...
+  Unfogs the area visible by the city.  Unlike other unfog functions this
+  one may be called multiple times in a row without any penalty.  Basically
+  it should be called any time the city's vision range may have changed.
 **************************************************************************/
 void map_unfog_city_area(struct city *pcity)
 {
-  if (!pcity) {
-    freelog(LOG_ERROR, "Attempting to unfog non-existent city");
-    return;
-  }
-
-  map_unfog_pseudo_city_area(city_owner(pcity), pcity->tile);
+  map_refog_city_area(pcity,
+                     get_city_bonus(pcity, EFT_CITY_VISION_RADIUS_SQ));
 }
 
 /**************************************************************************
@@ -774,37 +785,33 @@
 /**************************************************************************
 There doesn't have to be a city.
 **************************************************************************/
-void map_unfog_pseudo_city_area(struct player *pplayer, struct tile *ptile)
+void map_refog_circle(struct player *pplayer, struct tile *ptile,
+                     int old_radius_sq, int new_radius_sq, bool pseudo)
 {
-  freelog(LOG_DEBUG, "Unfogging city area at %i,%i", TILE_XY(ptile));
+  if (old_radius_sq != new_radius_sq) {
+    int max_radius = MAX(old_radius_sq, new_radius_sq);
 
-  buffer_shared_vision(pplayer);
-  map_city_radius_iterate(ptile, tile1) {
-    if (map_is_known(tile1, pplayer)) {
-      unfog_area(pplayer, tile1, 0);
-    } else {
-      increment_pending_seen(pplayer, tile1);
-    }
-  } map_city_radius_iterate_end;
-  unbuffer_shared_vision(pplayer);
-}
+    freelog(LOG_DEBUG, "Refogging circle at %d,%d from %d to %d",
+           TILE_XY(ptile), old_radius_sq, new_radius_sq);
 
-/**************************************************************************
-There doesn't have to be a city.
-**************************************************************************/
-void map_fog_pseudo_city_area(struct player *pplayer, struct tile *ptile)
-{
-  freelog(LOG_DEBUG, "Fogging city area at %i,%i", TILE_XY(ptile));
-
-  buffer_shared_vision(pplayer);
-  map_city_radius_iterate(ptile, tile1) {
-    if (map_is_known(tile1, pplayer)) {
-      fog_area(pplayer, tile1, 0);
-    } else {
-      decrement_pending_seen(pplayer, tile1);
-    }
-  } map_city_radius_iterate_end;
-  unbuffer_shared_vision(pplayer);
+    buffer_shared_vision(pplayer);
+    full_circle_iterate(ptile, max_radius, tile1, dx, dy, dr) {
+      if (dr > old_radius_sq && dr <= new_radius_sq) {
+       if (!pseudo || map_is_known(tile1, pplayer)) {
+         unfog_area(pplayer, tile1, 0);
+       } else {
+         increment_pending_seen(pplayer, tile1);
+       }
+      } else if (dr > new_radius_sq && dr < old_radius_sq) {
+       if (!pseudo || map_is_known(tile1, pplayer)) {
+         fog_area(pplayer, tile1, 0);
+       } else {
+         decrement_pending_seen(pplayer, tile1);
+       }
+      }
+    } circle_iterate_end;
+    unbuffer_shared_vision(pplayer);
+  }
 }
 
 /**************************************************************************
Index: server/maphand.h
===================================================================
--- server/maphand.h    (revision 11108)
+++ server/maphand.h    (working copy)
@@ -71,8 +71,8 @@
 void map_unfog_city_area(struct city *pcity);
 void remove_unit_sight_points(struct unit *punit);
 void show_area(struct player *pplayer,struct tile *ptile, int len);
-void map_unfog_pseudo_city_area(struct player *pplayer, struct tile *ptile);
-void map_fog_pseudo_city_area(struct player *pplayer, struct tile *ptile);
+void map_refog_circle(struct player *pplayer, struct tile *ptile,
+                     int old_radius_sq, int new_radius_sq, bool pseudo);
 
 bool map_is_known_and_seen(const struct tile *ptile, struct player *pplayer);
 void map_change_seen(struct tile *ptile, struct player *pplayer, int change);
Index: server/savegame.c
===================================================================
--- server/savegame.c   (revision 11108)
+++ server/savegame.c   (working copy)
@@ -2134,7 +2134,7 @@
     }
     
     /* adding the cities contribution to fog-of-war */
-    map_unfog_pseudo_city_area(&game.players[plrno], pcity->tile);
+    map_unfog_city_area(pcity);
 
     pcity->units_supported = unit_list_new();
 
Index: data/civ1/effects.ruleset
===================================================================
--- data/civ1/effects.ruleset   (revision 11108)
+++ data/civ1/effects.ruleset   (working copy)
@@ -47,6 +47,12 @@
     }
 
 
+; Base vision range - radius of vision is sqrt(5) = 2.24
+[effect_city_vision]
+name    = "City_Vision_Radius_Sq"
+value   = 5
+
+
 [effect_civil_war_0]
 name    = "Civil_War_Chance"
 value   = 90
Index: data/civ2/effects.ruleset
===================================================================
--- data/civ2/effects.ruleset   (revision 11108)
+++ data/civ2/effects.ruleset   (working copy)
@@ -48,6 +48,12 @@
     }
 
 
+; Base vision range - radius of vision is sqrt(5) = 2.24
+[effect_city_vision]
+name    = "City_Vision_Radius_Sq"
+value   = 5
+
+
 [effect_civil_war_0]
 name    = "Civil_War_Chance"
 value   = 90
Index: data/default/effects.ruleset
===================================================================
--- data/default/effects.ruleset        (revision 11108)
+++ data/default/effects.ruleset        (working copy)
@@ -56,6 +56,12 @@
     }
 
 
+; Base vision range - radius of vision is sqrt(5) = 2.24
+[effect_city_vision]
+name    = "City_Vision_Radius_Sq"
+value   = 5
+
+
 ; Nuclear power gives +1 moves to sea units
 [effect_nuclear_powered_boats]
 name   = "Sea_Move"
Index: data/history/effects.ruleset
===================================================================
--- data/history/effects.ruleset        (revision 11108)
+++ data/history/effects.ruleset        (working copy)
@@ -47,6 +47,12 @@
     }
 
 
+; Base vision range - radius of vision is sqrt(5) = 2.24
+[effect_city_vision]
+name    = "City_Vision_Radius_Sq"
+value   = 5
+
+
 ; Nuclear power gives +1 moves to sea units
 [effect_nuclear_powered_boats]
 name   = "Sea_Move"
Index: common/city.c
===================================================================
--- common/city.c       (revision 11108)
+++ common/city.c       (working copy)
@@ -83,7 +83,7 @@
    * This diagram is for rectangular topologies only.  But this is taken
    * care of inside map_vector_to_sq_distance so it works for all topologies.
    */
-  return (CITY_MAP_RADIUS * CITY_MAP_RADIUS + 1 >= dist);
+  return dist <= CITY_MAP_RADIUS_SQ;
 }
 
 /**************************************************************************
@@ -2461,6 +2461,7 @@
 
   pcity->server.workers_frozen = 0;
   pcity->server.needs_arrange = FALSE;
+  pcity->server.vision_radius_sq = -1; /* No vision. */
 
   pcity->ai.founder_want = 0; /* calculating this is really expensive */
   pcity->ai.next_founder_want_recalc = 0; /* turns to recalc found_want */
Index: common/city.h
===================================================================
--- common/city.h       (revision 11108)
+++ common/city.h       (working copy)
@@ -42,6 +42,9 @@
 /* Changing this requires updating CITY_TILES and network capabilities. */
 #define CITY_MAP_RADIUS 2
 
+/* The city includes all tiles dx^2 + dy^2 <= CITY_MAP_RADIUS_SQ */
+#define CITY_MAP_RADIUS_SQ (CITY_MAP_RADIUS * CITY_MAP_RADIUS + 1)
+
 /* Diameter of the workable city area.  Some places harcdode this number. */
 #define CITY_MAP_SIZE (CITY_MAP_RADIUS * 2 + 1) 
 
@@ -290,6 +293,9 @@
     /* If set, workers need to be arranged when the city is unfrozen.  Only
      * set inside auto_arrange_workers. */
     bool needs_arrange;
+
+    /* The vision radius that is currently unfogged. */
+    int vision_radius_sq;
   } server;
 
   int turn_founded;            /* In which turn was the city founded? */
Index: common/effects.c
===================================================================
--- common/effects.c    (revision 11108)
+++ common/effects.c    (working copy)
@@ -95,6 +95,7 @@
   "Land_Regen",
   "Sea_Regen",
   "Air_Regen",
+  "City_Vision_Radius_Sq",
   "Land_Defend",
   "Sea_Defend",
   "Air_Defend",
Index: common/effects.h
===================================================================
--- common/effects.h    (revision 11108)
+++ common/effects.h    (working copy)
@@ -83,6 +83,7 @@
   EFT_LAND_REGEN,
   EFT_SEA_REGEN,
   EFT_AIR_REGEN,
+  EFT_CITY_VISION_RADIUS_SQ,
   EFT_LAND_DEFEND,
   EFT_SEA_DEFEND,
   EFT_AIR_DEFEND,
Index: common/game.c
===================================================================
--- common/game.c       (revision 11108)
+++ common/game.c       (working copy)
@@ -159,6 +159,8 @@
           get_nation_name(city_owner(pcity)->nation), pcity->tile->x,
          pcity->tile->y);
 
+  assert(pcity->server.vision_radius_sq < 0); /* Otherwise vision fails. */
+
   city_map_checked_iterate(pcity->tile, x, y, map_tile) {
     set_worker_city(pcity, x, y, C_TILE_EMPTY);
   } city_map_checked_iterate_end;
Index: common/map.h
===================================================================
--- common/map.h        (revision 11108)
+++ common/map.h        (working copy)
@@ -322,17 +322,27 @@
  * positions will be automatically discarded. 
  */
 #define circle_iterate(center_tile, sq_radius, tile_itr)                   \
-{                                                                           \
-  int _sq_radius = (sq_radius);                                                
    \
-  int _cr_radius = (int)sqrt((double)_sq_radius);                          \
-  square_dxy_iterate(center_tile, _cr_radius, tile_itr, _dx, _dy) {        \
-    if (map_vector_to_sq_distance(_dx, _dy) <= _sq_radius) {
+  full_circle_iterate(center_tile, sq_radius, tile_itr, _dx, _dy, _dr)
 
 #define circle_iterate_end                                                  \
-    }                                                                       \
-  } square_dxy_iterate_end;                                                 \
-}                                                                             
+  full_circle_iterate_end
 
+#define full_circle_iterate(center_tile, sq_radius,                        \
+                           tile_itr, dx, dy, dr)                           \
+{                                                                          \
+  const int _sq_radius = (sq_radius);                                      \
+  const int _cr_radius = (int)sqrt((double)_sq_radius);                        
    \
+                                                                           \
+  square_dxy_iterate(center_tile, _cr_radius, tile_itr, dx, dy) {          \
+    const int dr = map_vector_to_sq_distance(dx, dy);                      \
+                                                                           \
+    if (dr <= _sq_radius) {
+
+#define full_circle_iterate_end                                                
    \
+    }                                                                      \
+  } square_dxy_iterate_end;                                                \
+}
+
 /* Iterate through all map positions adjacent to the given center map
  * position, with normalization.  The order of positions is unspecified. */
 #define adjc_iterate(center_tile, itr_tile)                                \
Index: ai/aicity.c
===================================================================
--- ai/aicity.c (revision 11108)
+++ ai/aicity.c (working copy)
@@ -347,6 +347,13 @@
       case EFT_NO_DIPLOMACY:
          break;
 
+      case EFT_CITY_VISION_RADIUS_SQ:
+       /* Wild guess.  "Amount" is the number of tiles (on average) that
+        * will be revealed by the effect.  Note that with an omniscient
+        * AI this effect is actually not useful at all. */
+       v += c * amount;
+       break;
+
       case EFT_SLOW_DOWN_TIMELINE:
        /* AI doesn't care about these. */
        break;