Author: pepeto
Date: Tue Nov 25 10:14:23 2014
New Revision: 27128

URL: http://svn.gna.org/viewcvs/freeciv?rev=27128&view=rev
Log:
Allow a city with more than maximal trade route number to establish a new trade
route if the trade income of the new trade route is greater than the sum of
the weakest trade routes to cancel.

Reported by Jacob Nevins <jtn>

See gna bug #22243

Modified:
    trunk/common/aicore/caravan.c
    trunk/common/traderoutes.c
    trunk/common/traderoutes.h
    trunk/server/citytools.c
    trunk/server/unithand.c

Modified: trunk/common/aicore/caravan.c
URL: 
http://svn.gna.org/viewcvs/freeciv/trunk/common/aicore/caravan.c?rev=27128&r1=27127&r2=27128&view=diff
==============================================================================
--- trunk/common/aicore/caravan.c       (original)
+++ trunk/common/aicore/caravan.c       Tue Nov 25 10:14:23 2014
@@ -276,9 +276,8 @@
     /* if the city can handle this route, we don't break any old routes */
     losttrade = 0;
   } else {
-    int slot;
-    int oldtrade = get_city_min_trade_route(pcity, &slot);
-    struct city *losercity = game_city_by_number(pcity->trade[slot]);
+    struct city_list *would_remove = (countloser ? city_list_new() : NULL);
+    int oldtrade = city_trade_removable(pcity, would_remove);
 
     /* if we own the city, the trade benefit is only by how much
        better we are than the old trade route */
@@ -286,10 +285,15 @@
       newtrade -= oldtrade;
     }
 
-    /* if the city that lost a trade route is one of ours, and if we
+    /* if the cities that lost a trade route is one of ours, and if we
        care about accounting for the lost trade, count it. */
-    if (countloser && city_owner(losercity) == pplayer) {
-      losttrade = oldtrade;
+    if (countloser) {
+      city_list_iterate(would_remove, losercity) {
+        if (city_owner(losercity) == pplayer) {
+          losttrade += oldtrade;
+        }
+      } city_list_iterate_end;
+      city_list_destroy(would_remove);
     }
   }
 

Modified: trunk/common/traderoutes.c
URL: 
http://svn.gna.org/viewcvs/freeciv/trunk/common/traderoutes.c?rev=27128&r1=27127&r2=27128&view=diff
==============================================================================
--- trunk/common/traderoutes.c  (original)
+++ trunk/common/traderoutes.c  Tue Nov 25 10:14:23 2014
@@ -219,32 +219,58 @@
               > 0));
 }
 
-/**************************************************************************
-  Find the worst (minimum) trade route the city has.  The value of the
-  trade route is returned and its position (slot) is put into the slot
-  variable.
-**************************************************************************/
-int get_city_min_trade_route(const struct city *pcity, int *slot)
-{
-  int i;
-  int value = 0;
-  bool found = FALSE;
-
-  if (slot) {
-    *slot = 0;
-  }
-  /* find min */
+/****************************************************************************
+  Return the minimum value of the sum of trade routes which could be
+  replaced by a new one. The target cities of the concerned trade routes
+  are will be put into 'would_remove', if set.
+****************************************************************************/
+int city_trade_removable(const struct city *pcity,
+                         struct city_list *would_remove)
+{
+  int sorted[MAX_TRADE_ROUTES];
+  int num, i, j;
+
+  FC_STATIC_ASSERT(ARRAY_SIZE(sorted) == ARRAY_SIZE(pcity->trade),
+                   incompatible_trade_array_size);
+  FC_STATIC_ASSERT(ARRAY_SIZE(sorted) == ARRAY_SIZE(pcity->trade_value),
+                   incompatible_trade_value_array_size);
+
+  /* Sort trade route values. */
+  num = 0;
   for (i = 0; i < MAX_TRADE_ROUTES; i++) {
-    if (pcity->trade[i] && (!found || value > pcity->trade_value[i])) {
-      if (slot) {
-       *slot = i;
-      }
-      value = pcity->trade_value[i];
-      found = TRUE;
-    }
-  }
-
-  return value;
+    if (0 == pcity->trade[i]) {
+      continue;
+    }
+    for (j = num; j > 0 && (pcity->trade_value[i]
+                            < pcity->trade_value[sorted[j - 1]]); j--) {
+      sorted[j] = sorted[j - 1];
+    }
+    sorted[j] = i;
+    num++;
+  }
+
+  /* No trade routes at all. */
+  if (0 == num) {
+    return 0;
+  }
+
+  /* Adjust number of concerned trade routes. */
+  num += 1 - max_trade_routes(pcity);
+  if (0 >= num) {
+    num = 1;
+  }
+
+  /* Return values. */
+  for (i = j = 0; i < num; i++) {
+    j += pcity->trade_value[sorted[i]];
+    if (NULL != would_remove) {
+      struct city *pother = game_city_by_number(pcity->trade[sorted[i]]);
+
+      fc_assert(NULL != pother);
+      city_list_append(would_remove, pother);
+    }
+  }
+  return j;
 }
 
 /**************************************************************************
@@ -275,20 +301,20 @@
     return FALSE;
   }
     
-  if (city_num_trade_routes(pc1) == maxpc1) {
+  if (city_num_trade_routes(pc1) >= maxpc1) {
     trade = trade_between_cities(pc1, pc2);
     /* can we replace trade route? */
-    if (get_city_min_trade_route(pc1, NULL) >= trade) {
+    if (city_trade_removable(pc1, NULL) >= trade) {
       return FALSE;
     }
   }
   
-  if (city_num_trade_routes(pc2) == maxpc2) {
+  if (city_num_trade_routes(pc2) >= maxpc2) {
     if (trade == -1) {
       trade = trade_between_cities(pc1, pc2);
     }
     /* can we replace trade route? */
-    if (get_city_min_trade_route(pc2, NULL) >= trade) {
+    if (city_trade_removable(pc2, NULL) >= trade) {
       return FALSE;
     }
   }  

Modified: trunk/common/traderoutes.h
URL: 
http://svn.gna.org/viewcvs/freeciv/trunk/common/traderoutes.h?rev=27128&r1=27127&r2=27128&view=diff
==============================================================================
--- trunk/common/traderoutes.h  (original)
+++ trunk/common/traderoutes.h  Tue Nov 25 10:14:23 2014
@@ -20,6 +20,7 @@
 #include "support.h" /* bool */
 
 struct city;
+struct city_list;
 
 /* What to do with previously established traderoutes that are now illegal.
  * Used in the network protocol. */
@@ -84,7 +85,8 @@
 int city_num_trade_routes(const struct city *pcity);
 int get_caravan_enter_city_trade_bonus(const struct city *pc1, 
                                        const struct city *pc2);
-int get_city_min_trade_route(const struct city *pcity, int *slot);
+int city_trade_removable(const struct city *pcity,
+                         struct city_list *would_remove);
 
 #define trade_routes_iterate(c, t)                          \
 do {                                                        \

Modified: trunk/server/citytools.c
URL: 
http://svn.gna.org/viewcvs/freeciv/trunk/server/citytools.c?rev=27128&r1=27127&r2=27128&view=diff
==============================================================================
--- trunk/server/citytools.c    (original)
+++ trunk/server/citytools.c    Tue Nov 25 10:14:23 2014
@@ -2497,19 +2497,18 @@
   }
 }
 
-/**************************************************************************
-  Remove/cancel the city's least valuable trade route.
-**************************************************************************/
-static void remove_smallest_trade_route(struct city *pcity)
-{
-  int slot;
-
-  if (get_city_min_trade_route(pcity, &slot) <= 0) {
-    return;
-  }
-
-  remove_trade_route(pcity, game_city_by_number(pcity->trade[slot]),
-                     TRUE, FALSE);
+/****************************************************************************
+  Remove/cancel the city's least valuable trade routes.
+****************************************************************************/
+static void remove_smallest_trade_routes(struct city *pcity)
+{
+  struct city_list *remove = city_list_new();
+
+  (void) city_trade_removable(pcity, remove);
+  city_list_iterate(remove, pother_city) {
+    remove_trade_route(pcity, pother_city, TRUE, FALSE);
+  } city_list_iterate_end;
+  city_list_destroy(remove);
 }
 
 /**************************************************************************
@@ -2520,12 +2519,12 @@
 {
   int i;
 
-  if (city_num_trade_routes(pc1) == max_trade_routes(pc1)) {
-    remove_smallest_trade_route(pc1);
-  }
-
-  if (city_num_trade_routes(pc2) == max_trade_routes(pc2)) {
-    remove_smallest_trade_route(pc2);
+  if (city_num_trade_routes(pc1) >= max_trade_routes(pc1)) {
+    remove_smallest_trade_routes(pc1);
+  }
+
+  if (city_num_trade_routes(pc2) >= max_trade_routes(pc2)) {
+    remove_smallest_trade_routes(pc2);
   }
 
   for (i = 0; i < MAX_TRADE_ROUTES; i++) {
@@ -2534,12 +2533,14 @@
       break;
     }
   }
+  fc_assert(i < MAX_TRADE_ROUTES);
   for (i = 0; i < MAX_TRADE_ROUTES; i++) {
     if (pc2->trade[i] == 0) {
       pc2->trade[i] = pc1->id;
       break;
     }
   }
+  fc_assert(i < MAX_TRADE_ROUTES);
 
   /* recalculate illness due to trade */
   if (game.info.illness_on) {

Modified: trunk/server/unithand.c
URL: 
http://svn.gna.org/viewcvs/freeciv/trunk/server/unithand.c?rev=27128&r1=27127&r2=27128&view=diff
==============================================================================
--- trunk/server/unithand.c     (original)
+++ trunk/server/unithand.c     Tue Nov 25 10:14:23 2014
@@ -2035,7 +2035,7 @@
   int dest_max;
   struct city *pcity_homecity;
   struct unit *punit = player_unit_by_number(pplayer, unit_id);
-  struct city *pcity_out_of_home = NULL, *pcity_out_of_dest = NULL;
+  struct city_list *cities_out_of_home, *cities_out_of_dest;
   enum traderoute_bonus_type bonus_type;
   const char *bonus_str;
 
@@ -2085,6 +2085,8 @@
   }
 
   sz_strlcpy(punit_link, unit_tile_link(punit));
+  cities_out_of_home = city_list_new();
+  cities_out_of_dest = city_list_new();
 
   /* This part of code works like can_establish_trade_route, except
    * that we actually do the action of making the trade route. */
@@ -2101,50 +2103,52 @@
   }
 
   if (can_establish && (home_overbooked >= 0 || dest_overbooked >= 0)) {
-    int slot, trade = trade_between_cities(pcity_homecity, pcity_dest);
+    int trade = trade_between_cities(pcity_homecity, pcity_dest);
 
     /* See if there's a trade route we can cancel at the home city. */
     if (home_overbooked >= 0) {
-      /* Can cancel one route at max */
-      if (home_overbooked == 0
-          && home_max > 0 && get_city_min_trade_route(pcity_homecity, &slot) < 
trade) {
-        pcity_out_of_home = game_city_by_number(pcity_homecity->trade[slot]);
-        fc_assert(pcity_out_of_home != NULL);
-      } else {
+      if (home_max <= 0
+          || (city_trade_removable(pcity_homecity, cities_out_of_home)
+              >= trade)) {
         notify_player(pplayer, city_tile(pcity_dest),
                       E_BAD_COMMAND, ftc_server,
                      _("Sorry, your %s cannot establish"
                        " a trade route here!"),
                        punit_link);
-        notify_player(pplayer, city_tile(pcity_dest),
-                      E_BAD_COMMAND, ftc_server,
-                      _("      The city of %s already has %d "
-                        "better trade routes!"),
-                      homecity_link,
-                      max_trade_routes(pcity_homecity));
+        if (home_max > 0) {
+          notify_player(pplayer, city_tile(pcity_dest),
+                        E_BAD_COMMAND, ftc_server,
+                        PL_("      The city of %s already has %d "
+                            "better trade route!",
+                            "      The city of %s already has %d "
+                            "better trade routes!", home_max),
+                        homecity_link,
+                        home_max);
+        }
        can_establish = FALSE;
       }
     }
 
     /* See if there's a trade route we can cancel at the dest city. */
     if (can_establish && dest_overbooked >= 0) {
-      /* Can cancel one route at max */
-      if (dest_overbooked == 0
-          && dest_max > 0 && get_city_min_trade_route(pcity_dest, &slot) < 
trade) {
-        pcity_out_of_dest = game_city_by_number(pcity_dest->trade[slot]);
-        fc_assert(pcity_out_of_dest != NULL);
-      } else {
+      if (dest_max <= 0
+          || (city_trade_removable(pcity_dest, cities_out_of_dest)
+              >= trade)) {
         notify_player(pplayer, city_tile(pcity_dest),
                       E_BAD_COMMAND, ftc_server,
                       _("Sorry, your %s cannot establish"
                         " a trade route here!"),
                       punit_link);
-        notify_player(pplayer, city_tile(pcity_dest),
-                      E_BAD_COMMAND, ftc_server,
-                      _("      The city of %s already has %d "
-                        "better trade routes!"),
-                      destcity_link,
-                      max_trade_routes(pcity_dest));
+        if (dest_max > 0) {
+          notify_player(pplayer, city_tile(pcity_dest),
+                        E_BAD_COMMAND, ftc_server,
+                        PL_("      The city of %s already has %d "
+                            "better trade route!",
+                            "      The city of %s already has %d "
+                            "better trade routes!", dest_max),
+                        destcity_link,
+                        dest_max);
+        }
        can_establish = FALSE;
       }
     }
@@ -2240,14 +2244,14 @@
     }
 
     /* Now cancel any less profitable trade route from the home city. */
-    if (pcity_out_of_home) {
-      remove_trade_route(pcity_homecity, pcity_out_of_home, TRUE, FALSE);
-    }
+    city_list_iterate(cities_out_of_home, pcity) {
+      remove_trade_route(pcity_homecity, pcity, TRUE, FALSE);
+    } city_list_iterate_end;
 
     /* And the same for the dest city. */
-    if (pcity_out_of_dest) {
-      remove_trade_route(pcity_dest, pcity_out_of_dest, TRUE, FALSE);
-    }
+    city_list_iterate(cities_out_of_dest, pcity) {
+      remove_trade_route(pcity_dest, pcity, TRUE, FALSE);
+    } city_list_iterate_end;
 
     /* Actually create the new trade route */
     for (i = 0; i < MAX_TRADE_ROUTES; i++) {
@@ -2269,22 +2273,22 @@
     /* Refresh the cities. */
     city_refresh(pcity_homecity);
     city_refresh(pcity_dest);
-    if (pcity_out_of_home) {
-      city_refresh(pcity_out_of_home);
-    }
-    if (pcity_out_of_dest) {
-      city_refresh(pcity_out_of_dest);
-    }
+    city_list_iterate(cities_out_of_home, pcity) {
+      city_refresh(pcity);
+    } city_list_iterate_end;
+    city_list_iterate(cities_out_of_dest, pcity) {
+      city_refresh(pcity);
+    } city_list_iterate_end;
 
     /* Notify the owners of the cities. */
     send_city_info(pplayer, pcity_homecity);
     send_city_info(city_owner(pcity_dest), pcity_dest);
-    if (pcity_out_of_home) {
-      send_city_info(city_owner(pcity_out_of_home), pcity_out_of_home);
-    }
-    if (pcity_out_of_dest) {
-      send_city_info(city_owner(pcity_out_of_dest), pcity_out_of_dest);
-    }
+    city_list_iterate(cities_out_of_home, pcity) {
+      send_city_info(city_owner(pcity), pcity);
+    } city_list_iterate_end;
+    city_list_iterate(cities_out_of_dest, pcity) {
+      send_city_info(city_owner(pcity), pcity);
+    } city_list_iterate_end;
 
     /* Notify each player about the other cities so that they know about
      * its size for the trade calculation . */
@@ -2293,38 +2297,35 @@
       send_city_info(pplayer, pcity_dest);
     }
 
-    if (pcity_out_of_home) {
-      if (city_owner(pcity_dest) != city_owner(pcity_out_of_home)) {
-        send_city_info(city_owner(pcity_dest), pcity_out_of_home);
-        send_city_info(city_owner(pcity_out_of_home), pcity_dest);
-      }
-      if (pplayer != city_owner(pcity_out_of_home)) {
-        send_city_info(pplayer, pcity_out_of_home);
-        send_city_info(city_owner(pcity_out_of_home), pcity_homecity);
-      }
-      if (pcity_out_of_dest && city_owner(pcity_out_of_home) !=
-                                       city_owner(pcity_out_of_dest)) {
-        send_city_info(city_owner(pcity_out_of_home), pcity_out_of_dest);
-      }
-    }
-
-    if (pcity_out_of_dest) {
-      if (city_owner(pcity_dest) != city_owner(pcity_out_of_dest)) {
-        send_city_info(city_owner(pcity_dest), pcity_out_of_dest);
-        send_city_info(city_owner(pcity_out_of_dest), pcity_dest);
-      }
-      if (pplayer != city_owner(pcity_out_of_dest)) {
-        send_city_info(pplayer, pcity_out_of_dest);
-        send_city_info(city_owner(pcity_out_of_dest), pcity_homecity);
-      }
-      if (pcity_out_of_home && city_owner(pcity_out_of_home) !=
-                                       city_owner(pcity_out_of_dest)) {
-        send_city_info(city_owner(pcity_out_of_dest), pcity_out_of_home);
-      }
-    }
+    city_list_iterate(cities_out_of_home, pcity) {
+      if (city_owner(pcity_dest) != city_owner(pcity)) {
+        send_city_info(city_owner(pcity_dest), pcity);
+        send_city_info(city_owner(pcity), pcity_dest);
+      }
+      if (pplayer != city_owner(pcity)) {
+        send_city_info(pplayer, pcity);
+        send_city_info(city_owner(pcity), pcity_homecity);
+      }
+    } city_list_iterate_end;
+
+    city_list_iterate(cities_out_of_dest, pcity) {
+      if (city_owner(pcity_dest) != city_owner(pcity)) {
+        send_city_info(city_owner(pcity_dest), pcity);
+        send_city_info(city_owner(pcity), pcity_dest);
+      }
+      if (pplayer != city_owner(pcity)) {
+        send_city_info(pplayer, pcity);
+        send_city_info(city_owner(pcity), pcity_homecity);
+      }
+    } city_list_iterate_end;
   }
 
   conn_list_do_unbuffer(pplayer->connections);
+
+  /* Free data. */
+  city_list_destroy(cities_out_of_home);
+  city_list_destroy(cities_out_of_dest);
+
   return TRUE;
 }
 


_______________________________________________
Freeciv-commits mailing list
Freeciv-commits@gna.org
https://mail.gna.org/listinfo/freeciv-commits

Reply via email to