[Freeciv-Dev] (PR#13394) Diplomats stealing techs cleanup

["Mateusz Stefek" <mstefek@xxxxxxxxx>]

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

This patch cleans up this part of the code.

give_a_teach() is changed into steal_a_tech() and much logic of
diplomat_get_tech() is moved there.

Special values in the protocol are changed, so it looks cleaner now.

The problem mentioned in PR#13393 is also fixed.

I've tested most use-cases.
--
mateusz

? civgame.log
? civscore.log
? regression
? server/civgame-1000.sav.gz
? server/civgame-1500.sav.gz
? server/civgame-2000.sav.gz
? server/civgame-2500.sav.gz
? server/civgame-3000.sav.gz
? server/civgame-3500.sav.gz
Index: client/gui-gtk-2.0/dialogs.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/dialogs.c,v
retrieving revision 1.107
diff -u -r1.107 dialogs.c
--- client/gui-gtk-2.0/dialogs.c        23 Jun 2005 07:24:38 -0000      1.107
+++ client/gui-gtk-2.0/dialogs.c        2 Jul 2005 10:49:17 -0000
@@ -374,7 +374,7 @@
   if(find_unit_by_id(diplomat_id) && 
      find_city_by_id(diplomat_target_id)) { 
     request_diplomat_action(DIPLOMAT_STEAL, diplomat_id,
-                           diplomat_target_id, 0);
+                           diplomat_target_id, A_UNSET);
   }
   gtk_widget_destroy(diplomat_dialog);
 }
@@ -425,7 +425,7 @@
                                 struct player *pvictim)
 {  
   GtkWidget *sw, *label, *vbox, *view;
-  int i, j;
+  int i;
   GtkListStore *store;
   GtkCellRenderer *rend;
   GtkTreeViewColumn *col;
@@ -482,8 +482,6 @@
   gtk_container_add(GTK_CONTAINER(vbox), sw);
 
   /* Now populate the list */
-  j = 0;
-
   if (pvictim) { /* you don't want to know what lag can do -- Syela */
     GtkTreeIter it;
     GValue value = { 0, };
@@ -500,19 +498,16 @@
        gtk_list_store_set_value(store, &it, 0, &value);
        g_value_unset(&value);
        gtk_list_store_set(store, &it, 1, i, -1);
-        j++;
       }
     }
 
-    if(j > 0) {
-      gtk_list_store_append(store, &it);
+    gtk_list_store_append(store, &it);
 
-      g_value_init(&value, G_TYPE_STRING);
-      g_value_set_static_string(&value, _("At Spy's Discretion"));
-      gtk_list_store_set_value(store, &it, 0, &value);
-      g_value_unset(&value);
-      gtk_list_store_set(store, &it, 1, game.control.num_tech_types, -1);
-    }
+    g_value_init(&value, G_TYPE_STRING);
+    g_value_set_static_string(&value, _("At Spy's Discretion"));
+    gtk_list_store_set_value(store, &it, 0, &value);
+    g_value_unset(&value);
+    gtk_list_store_set(store, &it, 1, A_UNSET, -1);
   }
 
   gtk_dialog_set_response_sensitive(GTK_DIALOG(spy_tech_shell),
Index: client/gui-win32/dialogs.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-win32/dialogs.c,v
retrieving revision 1.74
diff -u -r1.74 dialogs.c
--- client/gui-win32/dialogs.c  10 Jun 2005 08:11:01 -0000      1.74
+++ client/gui-win32/dialogs.c  2 Jul 2005 10:49:21 -0000
@@ -1053,7 +1053,7 @@
   if(find_unit_by_id(diplomat_id) &&
      find_city_by_id(diplomat_target_id)) {
     request_diplomat_action(DIPLOMAT_STEAL, diplomat_id,
-                           diplomat_target_id, 0);
+                           diplomat_target_id, A_UNSET);
   }
  
   process_diplomat_arrival(NULL, 0);
@@ -1138,7 +1138,7 @@
     
     if(j > 0) {
       ListBox_AddString(lb,_("At Spy's Discretion"));
-      advance_type[j++] = game.control.num_tech_types;
+      advance_type[j++] = A_UNSET;
     }
   }
   if(j == 0) {
Index: client/gui-xaw/dialogs.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-xaw/dialogs.c,v
retrieving revision 1.118
diff -u -r1.118 dialogs.c
--- client/gui-xaw/dialogs.c    8 Jun 2005 14:39:25 -0000       1.118
+++ client/gui-xaw/dialogs.c    2 Jul 2005 10:49:24 -0000
@@ -562,7 +562,7 @@
   if(find_unit_by_id(diplomat_id) && 
      find_city_by_id(diplomat_target_id)) { 
     request_diplomat_action(DIPLOMAT_STEAL, diplomat_id,
-                           diplomat_target_id, 0);
+                           diplomat_target_id, A_UNSET);
   }
 
   process_diplomat_arrival(NULL, 0);
@@ -743,10 +743,8 @@
         advance_type[j++] = i;
       }
     }
-    if(j > 0) {
-      advances_can_steal[j] = _("At Spy's Discretion");
-      advance_type[j++] = game.control.num_tech_types;
-    }
+    advances_can_steal[j] = _("At Spy's Discretion");
+    advance_type[j++] = A_UNSET;
   }
 
   if(j == 0) j++;
Index: server/citytools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.c,v
retrieving revision 1.330
diff -u -r1.330 citytools.c
--- server/citytools.c  29 Jun 2005 02:17:53 -0000      1.330
+++ server/citytools.c  2 Jul 2005 10:49:31 -0000
@@ -1247,7 +1247,7 @@
     gamelog(GAMELOG_LOSECITY, city_owner(pcity), pplayer, pcity, "liberated");
   }
 
-  get_a_tech(pplayer, cplayer);
+  steal_a_tech(pplayer, cplayer, A_UNSET);
   make_partisans(pcity);
 
   /* We transfer the city first so that it is in a consistent state when
Index: server/diplomats.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/diplomats.c,v
retrieving revision 1.84
diff -u -r1.84 diplomats.c
--- server/diplomats.c  1 Jul 2005 08:25:34 -0000       1.84
+++ server/diplomats.c  2 Jul 2005 10:49:34 -0000
@@ -498,7 +498,7 @@
 
 /****************************************************************************
   Try to steal a technology from an enemy city.
-  If "technology" is game.control.num_tech_types, steal a random technology.
+  If "technology" is A_UNSET, steal a random technology.
   Otherwise, steal the technology whose ID is "technology".
   (Note: Only Spies can select what to steal.)
 
@@ -509,7 +509,6 @@
   - If a technology has already been stolen from this city at any time:
     - Diplomats will be caught and executed.
     - Spies will have an increasing chance of being caught and executed.
-  - Determine target, given arguments and constraints.
   - Steal technology!
 
   - The thief may be captured and executed, or escape to its home town.
@@ -518,23 +517,52 @@
   not at war with
 ****************************************************************************/
 void diplomat_get_tech(struct player *pplayer, struct unit *pdiplomat, 
-                      struct city  *pcity, int technology)
+                      struct city  *pcity, Tech_type_id technology)
 {
   struct player *cplayer;
-  int count, which, target;
+  int count;
 
-  /* Fetch target civilization's player.  Sanity checks. */
-  if (!pcity)
+  /* We have to check arguments. They are sent to us by a client,
+     so we cannot trust them */
+  if (!pcity) {
     return;
+  }
+  
   cplayer = city_owner (pcity);
-  if ((cplayer == pplayer) || !cplayer)
+  if ((cplayer == pplayer) || !cplayer) {
     return;
+  }
+  
+  if (technology < 0 || technology == A_NONE || technology >= A_LAST) {
+    return;
+  }
+  
+  if (technology != A_FUTURE && technology != A_UNSET
+      && !tech_exists(technology)) {
+    return;
+  }
+  
+  if (technology == A_FUTURE) {
+    if (get_invention(pplayer, A_FUTURE) != TECH_REACHABLE
+        || get_player_research(pplayer)->future_tech >= 
+          get_player_research(pplayer)->future_tech) {
+      return;
+    }
+  } else if (technology != A_UNSET) {
+    if (get_invention(pplayer, technology) == TECH_KNOWN) {
+      return;
+    }
+    if (get_invention(cplayer, technology) != TECH_KNOWN) {
+      return;
+    }
+  }
 
   freelog (LOG_DEBUG, "steal-tech: unit: %d", pdiplomat->id);
 
   /* If not a Spy, do something random. */
-  if (!unit_flag (pdiplomat, F_SPY))
-    technology = game.control.num_tech_types;
+  if (!unit_flag (pdiplomat, F_SPY)) {
+    technology = A_UNSET;
+  }
 
   /* Check if the Diplomat/Spy succeeds against defending Diplomats/Spies. */
   if (!diplomat_infiltrate_tile(pplayer, cplayer, pdiplomat, 
@@ -547,14 +575,16 @@
   /* Check if the Diplomat/Spy succeeds with his/her task. */
   /* (Twice as difficult if target is specified.) */
   /* (If already stolen from, impossible for Diplomats and harder for Spies.) 
*/
-  if ((pcity->steal > 0) && (!unit_flag (pdiplomat, F_SPY))) {
+  if (pcity->steal > 0 && !unit_flag (pdiplomat, F_SPY)) {
     /* Already stolen from: Diplomat always fails! */
     count = 1;
     freelog (LOG_DEBUG, "steal-tech: difficulty: impossible");
   } else {
     /* Determine difficulty. */
     count = 1;
-    if (technology < game.control.num_tech_types) count++;
+    if (technology != A_UNSET) {
+      count++;
+    }
     count += pcity->steal;
     freelog (LOG_DEBUG, "steal-tech: difficulty: %d", count);
     /* Determine success or failure. */
@@ -565,6 +595,7 @@
       count--;
     }
   }
+  
   if (count > 0) {
     if (pcity->steal > 0 && !unit_flag (pdiplomat, F_SPY)) {
       notify_player_ex(pplayer, pcity->tile, E_MY_DIPLOMAT_FAILED,
@@ -584,132 +615,22 @@
     maybe_cause_incident(DIPLOMAT_STEAL, pplayer, NULL, pcity);
     wipe_unit(pdiplomat);
     return;
-  }
-
-  freelog (LOG_DEBUG, "steal-tech: succeeded");
-
-  /* Examine the civilization for technologies to steal. */
-  count = 0;
-  tech_type_iterate(index) {
-    if (get_invention(pplayer, index) != TECH_KNOWN
-       && get_invention(cplayer, index) == TECH_KNOWN
-       && tech_is_available(pplayer, index)) {
-      count++;
-    }
-  } tech_type_iterate_end;
-
-  freelog (LOG_DEBUG, "steal-tech: count of technologies: %d", count);
-
-  /* Determine the target (-1 is future tech). */
-  if (count == 0) {
-    /*
-     * Either only future-tech or nothing to steal:
-     * If nothing to steal, say so, deduct movement cost and return.
-     */
-    if (get_player_research(cplayer)->future_tech
-        > get_player_research(pplayer)->future_tech) {
-      target = -1;
-      freelog (LOG_DEBUG, "steal-tech: targeted future-tech: %d", target);
-    } else {
-      notify_player_ex(pplayer, pcity->tile, E_MY_DIPLOMAT_FAILED,
-                      _("No new technology found in %s."),
-                      pcity->name);
-      diplomat_charge_movement (pdiplomat, pcity->tile);
-      send_unit_info (pplayer, pdiplomat);
-      freelog (LOG_DEBUG, "steal-tech: nothing to steal");
-      return;
-    }
-  } else if (technology >= game.control.num_tech_types) {
-    /* Pick random technology to steal. */
-    target = -1;
-    which = myrand (count);
-    tech_type_iterate(index) {
-      if (get_invention(pplayer, index) != TECH_KNOWN
-         && get_invention(cplayer, index) == TECH_KNOWN
-         && tech_is_available(pplayer, index)) {
-       if (which > 0) {
-         which--;
-       } else {
-         target = index;
-         break;
-       }
-      }
-    } tech_type_iterate_end;
-    freelog(LOG_DEBUG, "steal-tech: random: targeted technology: %d (%s)",
-           target, get_tech_name(pplayer, target));
-  } else {
-    /*
-     * Told which technology to steal:
-     * If not available, say so, deduct movement cost and return.
-     */
-    if ((get_invention (pplayer, technology) != TECH_KNOWN) &&
-       (get_invention (cplayer, technology) == TECH_KNOWN)) {
-       target = technology;
-       freelog(LOG_DEBUG, "steal-tech: specified target technology: %d (%s)",
-               target, get_tech_name(pplayer, target));
-    } else {
-      notify_player_ex(pplayer, pcity->tile, E_MY_DIPLOMAT_FAILED,
-                      _("Your %s could not find the %s technology"
-                        " to steal in %s."),
-                      unit_name(pdiplomat->type),
-                      get_tech_name(pplayer, technology), pcity->name);
-      diplomat_charge_movement (pdiplomat, pcity->tile);
-      send_unit_info (pplayer, pdiplomat);
-      freelog(LOG_DEBUG, "steal-tech: target technology not found: %d (%s)",
-             technology, get_tech_name(pplayer, technology));
-      return;
-    }
-  }
+  } 
 
-  /* Now, the fun stuff!  Steal some technology! */
-  if (target < 0) {
-    /* Steal a future-tech. */
-
-    /* Do it. */
-    found_new_tech(pplayer, A_FUTURE, FALSE, TRUE);    
+  Tech_type_id tech_stolen = steal_a_tech(pplayer, cplayer, technology);
 
-    /* Report it. */
-    notify_player_ex(pplayer, pcity->tile, E_MY_DIPLOMAT_THEFT,
-                    _("Your %s stole Future Tech. %d from %s."),
-                    unit_name(pdiplomat->type),
-                    get_player_research(pplayer)->future_tech,
-                    cplayer->name);
-    notify_player_ex(cplayer, pcity->tile, E_ENEMY_DIPLOMAT_THEFT,
-                    _("Future Tech. %d stolen by %s %s from %s."),
-                    get_player_research(pplayer)->future_tech,
-                    get_nation_name(pplayer->nation),
-                    unit_name(pdiplomat->type), pcity->name);
-    freelog(LOG_DEBUG, "steal-tech: stole future-tech %d",
-           get_player_research(pplayer)->future_tech);
-  } else {
-    /* Steal a technology. */
-
-    /* Do it. */
-    do_conquer_cost (pplayer, target);
-    found_new_tech (pplayer, target, FALSE, TRUE);
-    /* Report it. */
-    notify_player_ex(pplayer, pcity->tile, E_MY_DIPLOMAT_THEFT,
-                    _("Your %s stole %s from %s."),
-                    unit_name(pdiplomat->type),
-                    get_tech_name(pplayer, target), cplayer->name);
-    notify_player_ex(cplayer, pcity->tile, E_ENEMY_DIPLOMAT_THEFT,
-                    _("%s's %s stole %s from %s."),
-                    pplayer->name, unit_name(pdiplomat->type),
-                    get_tech_name(cplayer, target), pcity->name);
-    notify_embassies(pplayer, cplayer,
-                    _("The %s have stolen %s from the %s."),
-                    get_nation_name_plural(pplayer->nation),
-                    get_tech_name(cplayer, target),
-                    get_nation_name_plural(cplayer->nation));
-    freelog(LOG_DEBUG, "steal-tech: stole %s",
-           get_tech_name(cplayer, target));
+  if (tech_stolen == A_NONE) {
+    notify_player_ex(pplayer, pcity->tile, E_MY_DIPLOMAT_FAILED,
+                     _("No new technology found in %s."),
+                    pcity->name);
+    diplomat_charge_movement (pdiplomat, pcity->tile);
+    send_unit_info (pplayer, pdiplomat);
+    return;
   }
 
-  gamelog(GAMELOG_TECH, pplayer, cplayer, target, "steal");
-
   /* Update stealing player's science progress and research fields */
   send_player_info(pplayer, pplayer);
-
+ 
   /* Record the theft. */
   (pcity->steal)++;
 
@@ -822,7 +743,7 @@
   nullify_prechange_production(pcity);
 
   /* You get a technology advance, too! */
-  get_a_tech (pplayer, cplayer);
+  steal_a_tech (pplayer, cplayer, A_UNSET);
 
   /* this may cause a diplomatic incident */
   maybe_cause_incident(DIPLOMAT_INCITE, pplayer, NULL, pcity);
Index: server/techtools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/techtools.c,v
retrieving revision 1.15
diff -u -r1.15 techtools.c
--- server/techtools.c  1 Jul 2005 08:25:34 -0000       1.15
+++ server/techtools.c  2 Jul 2005 10:49:34 -0000
@@ -629,75 +629,85 @@
 }
 
 /****************************************************************************
-  If target has more techs than pplayer, pplayer will get a random of
-  these, the clients will both be notified and the conquer cost
+  If victim has a tech which pplayer doesn't have, pplayer will get it.
+  The clients will both be notified and the conquer cost
   penalty applied. Used for diplomats and city conquest.
+  If preferred is A_UNSET one random tech will be choosen.
+  Returns the stolen tech or A_NONE if no tech was found.
 ****************************************************************************/
-void get_a_tech(struct player *pplayer, struct player *target)
+Tech_type_id steal_a_tech(struct player *pplayer, struct player *victim,
+                       Tech_type_id preferred)
 {
-  Tech_type_id stolen_tech;
-  int j = 0;
-
-  tech_type_iterate(i) {
-    if (get_invention(pplayer, i) != TECH_KNOWN
-       && get_invention(target, i) == TECH_KNOWN
-       && tech_is_available(pplayer, i)) {
-      j++;
-    }
-  } tech_type_iterate_end;
-  if (j == 0)  {
-    /* we've moved on to future tech */
-    if (get_player_research(target)->future_tech
-        > get_player_research(pplayer)->future_tech) {
-      found_new_tech(pplayer, A_FUTURE, FALSE, TRUE);  
-      stolen_tech
-       = game.control.num_tech_types
-         + get_player_research(pplayer)->future_tech;
-    } else {
-      return; /* nothing to learn here, move on */
-    }
-    return;
-  } else {
-    /* pick random tech */
-    j = myrand(j) + 1;
-    stolen_tech = A_NONE; /* avoid compiler warning */
+  Tech_type_id stolen_tech = A_NONE;
+  
+  if (preferred == A_UNSET) {
+    int j = 0;
     tech_type_iterate(i) {
       if (get_invention(pplayer, i) != TECH_KNOWN
-         && get_invention(target, i) == TECH_KNOWN
+         && get_invention(victim, i) == TECH_KNOWN
          && tech_is_available(pplayer, i)) {
-       j--;
-      }
-      if (j == 0) {
-       stolen_tech = i;
-       break;
+        j++;
       }
     } tech_type_iterate_end;
-    assert(stolen_tech != A_NONE);
+  
+    if (j == 0)  {
+      /* we've moved on to future tech */
+      if (get_player_research(victim)->future_tech
+        > get_player_research(pplayer)->future_tech) {
+        found_new_tech(pplayer, A_FUTURE, FALSE, TRUE);        
+        stolen_tech = A_FUTURE;
+      } else {
+        return A_NONE;
+      }
+    } else {
+      /* pick random tech */
+      j = myrand(j) + 1;
+      stolen_tech = A_NONE; /* avoid compiler warning */
+      tech_type_iterate(i) {
+        if (get_invention(pplayer, i) != TECH_KNOWN
+           && get_invention(victim, i) == TECH_KNOWN
+           && tech_is_available(pplayer, i)) {
+         j--;
+        }
+        if (j == 0) {
+         stolen_tech = i;
+         break;
+        }
+      } tech_type_iterate_end;
+      assert(stolen_tech != A_NONE);
+    }
+  } else { /* preferred != A_UNSET */
+    assert((preferred == A_FUTURE
+            && get_invention(victim, A_FUTURE) == TECH_REACHABLE)
+          || (tech_exists(preferred)
+              && get_invention(victim, preferred) == TECH_KNOWN));
+    stolen_tech = preferred;
   }
   script_signal_emit("tech_researched", 3,
                     API_TYPE_TECH_TYPE, &advances[stolen_tech],
                     API_TYPE_PLAYER, pplayer,
                     API_TYPE_STRING, "stolen");
-  gamelog(GAMELOG_TECH, pplayer, target, stolen_tech, "steal");
+  gamelog(GAMELOG_TECH, pplayer, victim, stolen_tech, "steal");
 
   notify_player_ex(pplayer, NULL, E_TECH_GAIN,
                   _("You steal %s from the %s."),
                   get_tech_name(pplayer, stolen_tech),
-                  get_nation_name_plural(target->nation));
+                  get_nation_name_plural(victim->nation));
 
-  notify_player_ex(target, NULL, E_ENEMY_DIPLOMAT_THEFT,
+  notify_player_ex(victim, NULL, E_ENEMY_DIPLOMAT_THEFT,
                    _("The %s stole %s from you!"),
                   get_nation_name_plural(pplayer->nation),
                   get_tech_name(pplayer, stolen_tech));
 
-  notify_embassies(pplayer, target,
+  notify_embassies(pplayer, victim,
                   _("The %s have stolen %s from the %s."),
                   get_nation_name_plural(pplayer->nation),
                   get_tech_name(pplayer, stolen_tech),
-                  get_nation_name_plural(target->nation));
+                  get_nation_name_plural(victim->nation));
 
   do_conquer_cost(pplayer, stolen_tech);
   found_new_tech(pplayer, stolen_tech, FALSE, TRUE);
+  return stolen_tech;
 }
 
 /****************************************************************************
Index: server/techtools.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/techtools.h,v
retrieving revision 1.6
diff -u -r1.6 techtools.h
--- server/techtools.h  1 Jul 2005 08:25:34 -0000       1.6
+++ server/techtools.h  2 Jul 2005 10:49:34 -0000
@@ -16,9 +16,9 @@
 #include "player.h"
 #include "tech.h"
 
-void do_dipl_cost(struct player *pplayer, Tech_type_id tech);
 void do_free_cost(struct player *pplayer, Tech_type_id tech);
 void do_conquer_cost(struct player *pplayer, Tech_type_id tech);
+void do_dipl_cost(struct player *pplayer, Tech_type_id tech);
 
 void do_tech_parasite_effect(struct player *pplayer);
 void found_new_tech(struct player *plr, Tech_type_id tech_found,
@@ -28,7 +28,8 @@
 void choose_tech(struct player *plr, Tech_type_id tech);
 void choose_random_tech(struct player* plr);
 void choose_tech_goal(struct player *plr, Tech_type_id tech);
-void get_a_tech(struct player *pplayer, struct player *target);
+Tech_type_id steal_a_tech(struct player *pplayer, struct player *target,
+                Tech_type_id preferred);
 
 Tech_type_id give_random_free_tech(struct player *pplayer);
 Tech_type_id give_immediate_free_tech(struct player *pplayer);