[Freeciv-Dev] Re: (PR#4420) New traderoute rules

["Rafa³ Bursig" <bursig@xxxxxxxxx>]

Dnia 2003.07.18 06:28, Jason Short napisa³(a):

> - Why do so many checks do
> 
>   can_cities_trade(...) && can_establish_trade_route(...)
> 
> ?  If can_establish_trade_route returns TRUE isn't that all you need
> to know?  It should be...
> 
No, becouse I split existing can_establish_trade_route to 2 parts...
1) check ownership and distance of cities

           (pc1 && pc2 && (pc1 != pc2) && (pc1->owner != pc2->owner
           || map_distance(pc1->x, pc1->y, pc2->x, pc2->y) > 8));

2) check exist of traderoutes ( I add check for replace abbility )

current both checks are make in can_establish_trade_route, I remove 1) 
from this function to avoid duplicate of check.
TRUE in first check give us information that we can trade and we can 
try check traderoute ability.

Trade ability and Traderoute ability are different things but secound 
require TRUE in first. Basicaly Traderoute ability is extension of 
Trade ability.

> - can_establish_trade_route should have a helper function
> get_city_min_trade_route().
> 
>   int get_city_min_trade_route(struct city *pcity)
>   {
>     int slot = 0, value = pcity->trade_value[0];
>     if (city_num_trade_routes(pc1) < NUM_TRADEROUTES) return 0;
> 
>     /* find min */
>     for (i = 1; i < NUM_TRADEROUTES; i++) {
>       if (value > pc1->trade_value[i]) {
>       value = pc1->trade_value[i];
>       }
>     }
> 
>     return value;
>   }
> 
> thus making the function much simpler.  And why is the unused 
> variable 'slot' in there?
> 
'slot' is require by remove logic to know which traderoute we remove 
but you have right this helper function will clean code

int get_city_min_trade_route(struct city *pcity, int *slot)
{
    int i, value = pcity->trade_value[0];

    *slot = 0;
    /* find min */
    for (i = 1; i < NUM_TRADEROUTES; i++) {
       if (value > pcity->trade_value[i]) {
         *slot = i;
        value = pcity->trade_value[i];
       }
    }

   return value;
}

> - get_caravan_enter_city_trade_bonus should be cleaned up; there are 
> a lot of old worthless comments left in.
> 
be my guest

> - have_cities_trade_route has an odd implementation.  Why do you 
> check both cities values if it doesn't matter?
> 
no it does matter...
this can speed up exit from check loop when existing traderoute is foud 
in secound city

Here is new version with get_city_min_trade_route clean.

Rafal

diff -u -r freeciv/ai/aiunit.c fc/ai/aiunit.c
--- freeciv/ai/aiunit.c Thu Jul 17 20:27:19 2003
+++ fc/ai/aiunit.c      Thu Jul 17 21:37:11 2003
@@ -2477,7 +2477,8 @@
        players_iterate(aplayer) {
          if (pplayers_at_war(pplayer, aplayer)) continue;
          city_list_iterate(pplayer->cities,pdest) {
-           if (pcity && can_establish_trade_route(pcity, pdest)
+           if (pcity && can_cities_trade(pcity, pdest)
+              && can_establish_trade_route(pcity, pdest)
                && map_get_continent(pcity->x, pcity->y) 
                                 == map_get_continent(pdest->x, pdest->y)) {
              tradeval=trade_between_cities(pcity, pdest);
diff -u -r freeciv/client/gui-gtk/dialogs.c fc/client/gui-gtk/dialogs.c
--- freeciv/client/gui-gtk/dialogs.c    Thu Jul 10 13:36:39 2003
+++ fc/client/gui-gtk/dialogs.c Thu Jul 17 21:37:11 2003
@@ -1147,6 +1147,7 @@
                          struct city *phomecity, struct city *pdestcity)
 {
   char buf[128];
+  bool can_establish, can_trade;
   
   my_snprintf(buf, sizeof(buf),
              _("Your caravan from %s reaches the city of %s.\nWhat now?"),
@@ -1155,15 +1156,20 @@
   caravan_city_id=pdestcity->id; /* callbacks need these */
   caravan_unit_id=punit->id;
   
+  can_trade = can_cities_trade(phomecity, pdestcity);
+  can_establish = can_trade
+                && can_establish_trade_route(phomecity, pdestcity);
+  
   caravan_dialog = popup_message_dialog(top_vbox,
                                        _("Your Caravan Has Arrived"), buf,
                                        caravan_close_callback, NULL,
-                          _("Establish 
_Traderoute"),caravan_establish_trade_callback, 0,
+                          (can_establish ? _("Establish _Traderoute") :
+                          _("Enter 
Marketplace")),caravan_establish_trade_callback, 0,
                           _("Help build 
_Wonder"),caravan_help_build_wonder_callback, 0,
                           _("_Keep moving"),NULL, 0,
                           0);
   
-  if (!can_establish_trade_route(phomecity, pdestcity)) {
+  if (!can_trade) {
     message_dialog_button_set_sensitive(caravan_dialog, "button0", FALSE);
   }
   
diff -u -r freeciv/client/gui-gtk-2.0/dialogs.c fc/client/gui-gtk-2.0/dialogs.c
--- freeciv/client/gui-gtk-2.0/dialogs.c        Thu Jul 10 13:36:39 2003
+++ fc/client/gui-gtk-2.0/dialogs.c     Thu Jul 17 21:37:11 2003
@@ -1018,6 +1018,7 @@
                          struct city *phomecity, struct city *pdestcity)
 {
   char buf[128];
+  bool can_establish, can_trade;
   
   my_snprintf(buf, sizeof(buf),
              _("Your caravan from %s reaches the city of %s.\nWhat now?"),
@@ -1026,10 +1027,15 @@
   caravan_city_id=pdestcity->id; /* callbacks need these */
   caravan_unit_id=punit->id;
   
+  can_trade = can_cities_trade(phomecity, pdestcity);
+  can_establish = can_trade
+                 && can_establish_trade_route(phomecity, pdestcity);
+  
   caravan_dialog = popup_message_dialog(GTK_WINDOW(toplevel),
     _("Your Caravan Has Arrived"), 
     buf,
-    _("Establish _Traderoute"),caravan_establish_trade_callback, NULL,
+    (can_establish ? _("Establish _Traderoute") :
+    _("Enter Marketplace")),caravan_establish_trade_callback, NULL,
     _("Help build _Wonder"),caravan_help_build_wonder_callback, NULL,
     _("_Keep moving"), NULL, NULL,
     NULL);
@@ -1037,7 +1043,7 @@
   g_signal_connect(caravan_dialog, "destroy",
                   G_CALLBACK(caravan_destroy_callback), NULL);
   
-  if (!can_establish_trade_route(phomecity, pdestcity)) {
+  if (!can_trade) {
     message_dialog_button_set_sensitive(caravan_dialog, 0, FALSE);
   }
   
diff -u -r freeciv/client/gui-mui/dialogs.c fc/client/gui-mui/dialogs.c
--- freeciv/client/gui-mui/dialogs.c    Thu Jul 10 13:36:39 2003
+++ fc/client/gui-mui/dialogs.c Thu Jul 17 21:37:11 2003
@@ -971,15 +971,19 @@
   my_snprintf(buf, sizeof(buf),_("Your caravan from %s reaches the city of 
%s.\nWhat now?"),
           phomecity->name, pdestcity->name);
 
-  if(can_establish_trade_route(phomecity, pdestcity))
+  if (can_cities_trade(phomecity, pdestcity))
   {
     struct caravan_data *cd = malloc_struct(struct caravan_data);
     if(cd)
     {
       cd->caravan_city_id = pdestcity->id;
       cd->caravan_unit_id = punit->id;
-
-      msg_dlg[i].label = _("_Establish traderoute");
+      if (can_establish_trade_route(phomecity, pdestcity))
+      {
+        msg_dlg[i].label = _("_Establish traderoute");
+      } else {
+        msg_dlg[i].label = _("_Enter Marketplace");
+      }
       msg_dlg[i].function = (APTR)caravan_establish;
       msg_dlg[i].data = (APTR)cd;
       i++;
diff -u -r freeciv/client/gui-sdl/dialogs.c fc/client/gui-sdl/dialogs.c
--- freeciv/client/gui-sdl/dialogs.c    Thu Jul 17 17:56:33 2003
+++ fc/client/gui-sdl/dialogs.c Thu Jul 17 21:37:11 2003
@@ -1506,7 +1506,8 @@
   SDL_String16 *pStr;
   int w = 0, h , canvas_x , canvas_y;
   struct CONTAINER *pCont;
-    
+  char cBuf[128];
+  
   if (pCaravan_Dlg) {
     return;
   }
@@ -1521,8 +1522,11 @@
   
   map_to_canvas_pos(&canvas_x, &canvas_y, pUnit->x, pUnit->y);
   
+  my_snprintf(cBuf, sizeof(cBuf), _("Your Caravan Has Arrived to city %s"),
+                                                         pDestcity->name);
+  
   /* window */
-  pStr = create_str16_from_char(_("Your Caravan Has Arrived") , 12);
+  pStr = create_str16_from_char(cBuf, 12);
   pStr->style |= TTF_STYLE_BOLD;
   
   pWindow = create_window(NULL, pStr , 10 , 10 , WF_DRAW_THEME_TRANSPARENT);
@@ -1535,16 +1539,29 @@
   pCaravan_Dlg->pEndWidgetList = pWindow;
     
   /* ---------- */
-  if (can_establish_trade_route(pHomecity, pDestcity))
+  if (can_cities_trade(pHomecity, pDestcity))
   {
+    int revenue = get_caravan_enter_city_trade_bonus(pHomecity, pDestcity);
+    
+    if (can_establish_trade_route(pHomecity, pDestcity)) {
+      my_snprintf(cBuf, sizeof(cBuf),
+               _("Establish Traderoute with %s ( %d R&G + %d trade )"),
+               pHomecity->name, revenue,
+                       trade_between_cities(pHomecity, pDestcity));
+    } else {
+      revenue = (revenue + 2) / 3;
+      my_snprintf(cBuf, sizeof(cBuf),
+               _("Enter Marketplace ( %d R&G bonus )"), revenue);
+    }
+    
     create_active_iconlabel(pBuf, pWindow->dst, pStr,
-           _("Establish Traderoute"), caravan_establish_trade_callback);
+           cBuf, caravan_establish_trade_callback);
     pBuf->data.cont = pCont;
-    set_wstate(pBuf , FC_WS_NORMAL);
+    set_wstate(pBuf, FC_WS_NORMAL);
   
-    add_to_gui_list(ID_LABEL , pBuf);
+    add_to_gui_list(ID_LABEL, pBuf);
     
-    w = MAX(w , pBuf->size.w);
+    w = MAX(w, pBuf->size.w);
     h += pBuf->size.h;
   }
   
diff -u -r freeciv/client/gui-win32/dialogs.c fc/client/gui-win32/dialogs.c
--- freeciv/client/gui-win32/dialogs.c  Thu Jul 10 13:36:45 2003
+++ fc/client/gui-win32/dialogs.c       Thu Jul 17 21:37:11 2003
@@ -1035,7 +1035,8 @@
                           struct city *phomecity, struct city *pdestcity)
 {
   char buf[128];
- 
+  bool can_establish, can_trade;
+  
   my_snprintf(buf, sizeof(buf),
               _("Your caravan from %s reaches the city of %s.\nWhat now?"),
               phomecity->name, pdestcity->name);
@@ -1043,15 +1044,20 @@
   caravan_city_id=pdestcity->id; /* callbacks need these */
   caravan_unit_id=punit->id;
  
+  can_trade = can_cities_trade(phomecity, pdestcity);
+  can_establish = can_trade
+                 && can_establish_trade_route(phomecity, pdestcity);
+  
   caravan_dialog=popup_message_dialog(NULL,
                            /*"caravandialog"*/_("Your Caravan Has Arrived"),
                            buf,
-                           _("Establish 
_Traderoute"),caravan_establish_trade_callback, 0,
+                           (can_establish ? _("Establish _Traderoute") :
+                          _("Enter 
Marketplace")),caravan_establish_trade_callback, 0,
                            _("Help build 
_Wonder"),caravan_help_build_wonder_callback, 0,
                            _("_Keep moving"),caravan_keep_moving_callback, 0,
                            0);
  
-  if(!can_establish_trade_route(phomecity, pdestcity))
+  if (!can_enter_marketplace)
     {
       message_dialog_button_set_sensitive(caravan_dialog,0,FALSE);
     }
diff -u -r freeciv/client/gui-xaw/dialogs.c fc/client/gui-xaw/dialogs.c
--- freeciv/client/gui-xaw/dialogs.c    Thu Jul 10 13:36:45 2003
+++ fc/client/gui-xaw/dialogs.c Thu Jul 17 21:37:11 2003
@@ -1228,7 +1228,7 @@
                           caravan_keep_moving_callback, 0, 0,
                           NULL);
   
-  if(!can_establish_trade_route(phomecity, pdestcity))
+  if (!can_cities_trade(phomecity, pdestcity))
     XtSetSensitive(XtNameToWidget(caravan_dialog, "*button0"), FALSE);
   
   if(!unit_can_help_build_wonder(punit, pdestcity))
diff -u -r freeciv/common/city.c fc/common/city.c
--- freeciv/common/city.c       Fri Jul 11 18:15:44 2003
+++ fc/common/city.c    Fri Jul 18 09:42:01 2003
@@ -838,32 +838,68 @@
 /**************************************************************************
 ...
 **************************************************************************/
-bool can_establish_trade_route(struct city *pc1, struct city *pc2)
+bool can_cities_trade(struct city *pc1, struct city *pc2)
 {
-  int i, free1 = 0, free2 = 0;
+  return (pc1 && pc2 && (pc1 != pc2)
+          && (pc1->owner != pc2->owner
+          || map_distance(pc1->x, pc1->y, pc2->x, pc2->y) > 8));
+}
 
-  if (!pc1 || !pc2 || pc1 == pc2
-      || (pc1->owner == pc2->owner
-         && map_distance(pc1->x, pc1->y, pc2->x, pc2->y) <= 8)) {
-    return FALSE;
-  }
+/**************************************************************************
+...
+**************************************************************************/
+int get_city_min_trade_route(struct city *pcity, int *slot)
+{
+  int i, value = pcity->trade_value[0];
 
-  for (i = 0; i < NUM_TRADEROUTES; i++) {
-    if (pc1->trade[i] == pc2->id || pc2->trade[i] == pc1->id) {
-      /* cities already have a traderoute */
-      return FALSE;
+  *slot = 0;
+  /* find min */
+  for (i = 1; i < NUM_TRADEROUTES; i++) {
+    if (value > pcity->trade_value[i]) {
+      *slot = i;
+      value = pcity->trade_value[i];
     }
+  }
 
-    if (pc1->trade[i] == 0) {
-      free1++;
-    }
-    if (pc2->trade[i] == 0) {
-      free2++;
+  return value;
+}
+
+/**************************************************************************
+  Function don't check min cities distance then should be call with
+  connection of can_cities_trade(...) function.
+**************************************************************************/
+bool can_establish_trade_route(struct city *pc1, struct city *pc2)
+{
+  int trade = -1;
+  bool can_establish = TRUE;
+  
+  if (!pc1 || !pc2 || pc1 == pc2 || have_cities_trade_route(pc1, pc2)) {
+    return FALSE;
+  }
+    
+  if(city_num_trade_routes(pc1) == NUM_TRADEROUTES) {
+    int slot_dummy;
+    
+    trade = trade_between_cities(pc1, pc2);
+    /* can we replace traderoute */
+    if(get_city_min_trade_route(pc1, &slot_dummy) >= trade) {
+      can_establish = FALSE;
     }
   }
+  
+  if(can_establish && city_num_trade_routes(pc2) == NUM_TRADEROUTES) {
+    int slot_dummy;
+    
+    if(trade == -1) {
+      trade = trade_between_cities(pc1, pc2);
+    }
+    /* can we replace traderoute */
+    if(get_city_min_trade_route(pc2, &slot_dummy) >= trade) {
+      can_establish = FALSE;
+    }
+  }  
 
-  /* both cities need a free slot */
-  return (free1 > 0 && free2 > 0);
+  return can_establish;
 }
 
 /**************************************************************************
@@ -902,6 +938,56 @@
   return n;
 }
 
+/**************************************************************************
+  Returns the revenue trade route bonus.
+**************************************************************************/
+int get_caravan_enter_city_trade_bonus(struct city *pc1, struct city *pc2)
+{
+  int i, tb;
+
+  tb=(map_distance(pc1->x, pc1->y, pc2->x, pc2->y)+10);
+  /* should this be real_map_distance?  Leaving for now -- Syela */
+  tb=(tb*(pc1->trade_prod+pc2->trade_prod))/24;
+  /* 
+   * fudge factor to more closely approximate Civ2 behavior (Civ2 is
+   * really very different -- this just fakes it a little better) 
+   */
+  tb *= 3;
+
+  /* 
+   * one time bonus is not affected by factors in
+   * trade_between_cities() 
+   */
+  for (i = 0; i < num_known_tech_with_flag(city_owner(pc1),
+                                          TF_TRADE_REVENUE_REDUCE); i++) {
+    tb = (tb * 2) / 3;
+  }
+  /* was: A_RAILROAD, A_FLIGHT */
+  
+  /*
+   * this value will be reduced when both cities have established
+   * traderoute togeder -> tb = (tb + 2) / 3;
+   */
+  
+  return tb;
+}
+
+/**************************************************************************
+  Check if cities have established trade route.
+**************************************************************************/
+bool have_cities_trade_route(struct city *pc1, struct city *pc2)
+{
+  int i;
+  
+  for (i = 0; i < NUM_TRADEROUTES; i++) {
+    if (pc1->trade[i] == pc2->id || pc2->trade[i] == pc1->id) {
+      /* cities already have a traderoute */
+      return TRUE;
+    }
+  }
+  return FALSE;
+}
+
 /*************************************************************************
   Calculate amount of gold remaining in city after paying for buildings 
   and units.
diff -u -r freeciv/common/city.h fc/common/city.h
--- freeciv/common/city.h       Tue May 13 16:23:03 2003
+++ fc/common/city.h    Fri Jul 18 09:09:32 2003
@@ -396,10 +396,14 @@
 bool city_can_be_built_here(int x, int y);
 
 /* trade functions */
+bool can_cities_trade(struct city *pc1, struct city *pc2);
 bool can_establish_trade_route(struct city *pc1, struct city *pc2);
+bool have_cities_trade_route(struct city *pc1, struct city *pc2);
 int trade_between_cities(struct city *pc1, struct city *pc2);
 int city_num_trade_routes(struct city *pcity);
-
+int get_caravan_enter_city_trade_bonus(struct city *pc1, struct city *pc2);
+int get_city_min_trade_route(struct city *pcity, int *slot);
+  
 /* list functions */
 struct city *city_list_find_id(struct city_list *This, int id);
 struct city *city_list_find_name(struct city_list *This, const char *name);
diff -u -r freeciv/common/unit.c fc/common/unit.c
--- freeciv/common/unit.c       Thu Jul 17 20:27:19 2003
+++ fc/common/unit.c    Thu Jul 17 21:37:11 2003
@@ -243,8 +243,7 @@
   pdestcity = map_get_city(punit->x, punit->y);
   if (!pdestcity) return FALSE;
   phomecity = find_city_by_id(punit->homecity);
-  if (!phomecity) return FALSE;
-  return can_establish_trade_route(phomecity, pdestcity);
+  return (phomecity && can_cities_trade(phomecity, pdestcity));
 }
 
 /**************************************************************************
diff -u -r freeciv/server/citytools.c fc/server/citytools.c
--- freeciv/server/citytools.c  Thu Jul 17 19:19:28 2003
+++ fc/server/citytools.c       Fri Jul 18 09:21:38 2003
@@ -59,7 +59,6 @@
                                  struct player *pplayer);
 static void server_set_tile_city(struct city *pcity, int city_x, int city_y,
                                 enum city_tile_type type);
-static void remove_trade_route(struct city *pc1, struct city *pc2);
 static bool update_city_tile_status(struct city *pcity, int city_x,
                                    int city_y);
 
@@ -1695,7 +1694,7 @@
 /**************************************************************************
 ...
 **************************************************************************/
-static void remove_trade_route(struct city *pc1, struct city *pc2)
+void remove_trade_route(struct city *pc1, struct city *pc2)
 {
   int i;
 
@@ -1710,46 +1709,62 @@
 }
 
 /**************************************************************************
+...
+**************************************************************************/
+static void remove_smallest_trade_route(struct city *pcity)
+{
+  int i, slot;
+  struct city *pdest;
+    
+  assert(pcity);
+  
+  if(get_city_min_trade_route(pcity, &slot) <= 0) {
+    return;
+  }
+      
+  if((pdest = find_city_by_id(pcity->trade[slot]))) {
+    for (i = 0; i < NUM_TRADEROUTES; i++) {
+      if (pdest->trade[i] == pcity->id) {
+        pdest->trade[i] = 0;
+        break;
+      }
+    }
+  }
+  
+  pcity->trade[slot] = 0;
+}
+
+/**************************************************************************
 Establish a trade route, notice that there has to be space for them, 
-So use can_establish_Trade_route first.
-returns the revenue aswell.
+So you must use can_establish_trade_route first.
 **************************************************************************/
-int establish_trade_route(struct city *pc1, struct city *pc2)
+void establish_trade_route(struct city *pc1, struct city *pc2)
 {
-  int i, tb;
+  int i;
 
+  if(city_num_trade_routes(pc1) == NUM_TRADEROUTES) {
+    remove_smallest_trade_route(pc1);
+  }
+  
+  if(city_num_trade_routes(pc2) == NUM_TRADEROUTES) {
+    remove_smallest_trade_route(pc2);
+  }
+  
   for (i = 0; i < NUM_TRADEROUTES; i++) {
     if (pc1->trade[i] == 0) {
       pc1->trade[i]=pc2->id;
       break;
     }
   }
+  
   for (i = 0; i < NUM_TRADEROUTES; i++) {
     if (pc2->trade[i] == 0) {
       pc2->trade[i]=pc1->id;
       break;
     }
   }
-
-  tb=(map_distance(pc1->x, pc1->y, pc2->x, pc2->y)+10);
-/* should this be real_map_distance?  Leaving for now -- Syela */
-  tb=(tb*(pc1->trade_prod+pc2->trade_prod))/24;
-  /* 
-   * fudge factor to more closely approximate Civ2 behavior (Civ2 is
-   * really very different -- this just fakes it a little better) 
-   */
-  tb *= 3;
-
-  /* 
-   * one time bonus is not affected by factors in
-   * trade_between_cities() 
-   */
-  for (i = 0; i < num_known_tech_with_flag(city_owner(pc1),
-                                          TF_TRADE_REVENUE_REDUCE); i++) {
-    tb = (tb * 2) / 3;
-  }
-  /* was: A_RAILROAD, A_FLIGHT */
-  return tb;
+  
+  return;
 }
 
 /**************************************************************************
diff -u -r freeciv/server/citytools.h fc/server/citytools.h
--- freeciv/server/citytools.h  Thu Jul 17 19:19:28 2003
+++ fc/server/citytools.h       Thu Jul 17 23:02:30 2003
@@ -74,8 +74,9 @@
                 const char *name);
 void remove_city(struct city *pcity);
 
-int establish_trade_route(struct city *pc1, struct city *pc2);
-
+void establish_trade_route(struct city *pc1, struct city *pc2);
+void remove_trade_route(struct city *pc1, struct city *pc2);
+  
 void do_sell_building(struct player *pplayer, struct city *pcity, int id);
 void building_lost(struct city *pcity, int id);
 void change_build_target(struct player *pplayer, struct city *pcity, 
diff -u -r freeciv/server/unithand.c fc/server/unithand.c
--- freeciv/server/unithand.c   Thu Jul 17 20:27:21 2003
+++ fc/server/unithand.c        Fri Jul 18 09:38:24 2003
@@ -1152,58 +1152,130 @@
                                 struct packet_unit_request *req)
 {
   struct unit *punit = player_find_unit_by_id(pplayer, req->unit_id);
+  struct city *pcity_out_of_home = NULL, *pcity_out_of_dest = NULL;
   struct city *pcity_homecity, *pcity_dest = find_city_by_id(req->city_id);
-  int revenue;
+  int revenue, i;
+  bool home_full = FALSE, dest_full = FALSE, can_establish;
   
-  if (!punit || !unit_flag(punit, F_TRADE_ROUTE) || !pcity_dest) {
+  if (!punit || !pcity_dest || !unit_flag(punit, F_TRADE_ROUTE)) {
     return FALSE;
   }
 
   pcity_homecity = player_find_city_by_id(pplayer, punit->homecity);
-  if (!pcity_homecity) {
+  if (!pcity_homecity || pcity_homecity == pcity_dest) {
     return FALSE;
   }
     
   if (!is_tiles_adjacent(punit->x, punit->y, pcity_dest->x, pcity_dest->y)
-      && !same_pos(punit->x, punit->y, pcity_dest->x, pcity_dest->y))
+      && !same_pos(punit->x, punit->y, pcity_dest->x, pcity_dest->y)) {
     return FALSE;
+  }
 
   if (!(same_pos(punit->x, punit->y, pcity_dest->x, pcity_dest->y)
-       || try_move_unit(punit, pcity_dest->x, pcity_dest->y)))
+       || try_move_unit(punit, pcity_dest->x, pcity_dest->y))) {
     return FALSE;
+  }
 
-  if (!can_establish_trade_route(pcity_homecity, pcity_dest)) {
-    int i;
-    notify_player_ex(pplayer, pcity_dest->x, pcity_dest->y, E_NOEVENT,
-                    _("Game: Sorry, your %s cannot establish"
-                      " a trade route here!"),
-                    unit_name(punit->type));
-    for (i = 0; i < NUM_TRADEROUTES; i++) {
-      if (pcity_homecity->trade[i]==pcity_dest->id) {
+  if (!can_cities_trade(pcity_homecity, pcity_dest)) {
+    return FALSE;
+  }
+  
+  /* ------------------------------------------------------------------ */
+  /*
+   * this part of code work like can_establish_trade_route funtion
+   * with 2 differents ... 
+   * 1) remove traderutes.
+   * 2) send info msg. to city owners.
+   */
+  can_establish = !(have_cities_trade_route(pcity_homecity, pcity_dest));
+    
+  if (can_establish) {
+    home_full = (city_num_trade_routes(pcity_homecity) == NUM_TRADEROUTES);
+    dest_full = (city_num_trade_routes(pcity_dest) == NUM_TRADEROUTES);
+  }
+  
+  if (home_full || dest_full) {
+    int slot, trade = trade_between_cities(pcity_homecity, pcity_dest);
+    
+    if (home_full) {
+      if (get_city_min_trade_route(pcity_homecity, &slot) < trade) {
+       pcity_out_of_home = find_city_by_id(pcity_homecity->trade[slot]);
+      } else {
        notify_player_ex(pplayer, pcity_dest->x, pcity_dest->y, E_NOEVENT,
-                     _("      A traderoute already exists between %s and %s!"),
-                     pcity_homecity->name, pcity_dest->name);
-       return FALSE;
-      }
-    }
-    if (city_num_trade_routes(pcity_homecity) == NUM_TRADEROUTES) {
-      notify_player_ex(pplayer, pcity_dest->x, pcity_dest->y, E_NOEVENT,
+                    _("Game: Sorry, your %s cannot establish"
+                      " a trade route here!"), unit_name(punit->type));
+        notify_player_ex(pplayer, pcity_dest->x, pcity_dest->y, E_NOEVENT,
                       _("      The city of %s already has %d "
-                        "trade routes!"), pcity_homecity->name,
+                        "better trade routes!"), pcity_homecity->name,
                       NUM_TRADEROUTES);
-      return FALSE;
+        can_establish = FALSE;
+      }
     }
-    if (city_num_trade_routes(pcity_dest) == NUM_TRADEROUTES) {
-      notify_player_ex(pplayer, pcity_dest->x, pcity_dest->y, E_NOEVENT,
+    
+    if (dest_full && can_establish) {
+      if ((get_city_min_trade_route(pcity_dest, &slot) < trade)
+         && (pcity_out_of_dest = find_city_by_id(pcity_dest->trade[slot]))) {
+           
+       remove_trade_route(pcity_dest, pcity_out_of_dest);
+       notify_player_ex(city_owner(pcity_out_of_dest),
+                         pcity_out_of_dest->x, pcity_out_of_dest->y, E_NOEVENT,
+                    _("Game: Sorry, Your city %s cannot hold"
+                      " a trade route from %s!"), pcity_out_of_dest->name,
+                         pcity_dest->name);
+         
+       if (pcity_out_of_home) {
+         remove_trade_route(pcity_homecity, pcity_out_of_home);
+         notify_player_ex(city_owner(pcity_out_of_home),
+                         pcity_out_of_home->x, pcity_out_of_home->y, E_NOEVENT,
+                    _("Game: Sorry, Your city %s cannot hold"
+                      " a trade route from %s!"), pcity_out_of_home->name,
+                         pcity_homecity->name);
+       }
+      } else {
+        notify_player_ex(pplayer, pcity_dest->x, pcity_dest->y, E_NOEVENT,
+                    _("Game: Sorry, your %s cannot establish"
+                      " a trade route here!"), unit_name(punit->type));
+        notify_player_ex(pplayer, pcity_dest->x, pcity_dest->y, E_NOEVENT,
                       _("      The city of %s already has %d "
-                        "trade routes!"), pcity_dest->name,
+                        "better trade routes!"), pcity_dest->name,
                       NUM_TRADEROUTES);
-      return FALSE;
+       can_establish = FALSE;
+      }  
+    } else {
+      if (can_establish && pcity_out_of_home) {
+         remove_trade_route(pcity_homecity, pcity_out_of_home);
+         notify_player_ex(city_owner(pcity_out_of_home),
+                         pcity_out_of_home->x, pcity_out_of_home->y, E_NOEVENT,
+                    _("Game: Sorry, Your city %s cannot hold"
+                      " a trade route from %s!"), pcity_out_of_home->name,
+                         pcity_homecity->name);
+      }
     }
-    return FALSE;
+    
+  }
+  /* ------------------------------------------------------------------ */
+  
+  revenue = get_caravan_enter_city_trade_bonus(pcity_homecity, pcity_dest);
+  if (can_establish) {
+   /*  establish traderoute */
+    for (i = 0; i < NUM_TRADEROUTES; i++) {
+      if (pcity_homecity->trade[i] == 0) {
+        pcity_homecity->trade[i]=pcity_dest->id;
+        break;
+      }
+    }
+  
+    for (i = 0; i < NUM_TRADEROUTES; i++) {
+      if (pcity_dest->trade[i] == 0) {
+        pcity_dest->trade[i]=pcity_homecity->id;
+        break;
+      }
+    }
+  } else {
+    /* enter marketplace */
+    revenue = (revenue + 2) / 3;
   }
   
-  revenue = establish_trade_route(pcity_homecity, pcity_dest);
   conn_list_do_buffer(&pplayer->connections);
   notify_player_ex(pplayer, pcity_dest->x, pcity_dest->y, E_NOEVENT,
                   _("Game: Your %s from %s has arrived in %s,"
@@ -1213,21 +1285,70 @@
   wipe_unit(punit);
   pplayer->economic.gold+=revenue;
   update_tech(pplayer, revenue);
-  send_player_info(pplayer, pplayer);
-  city_refresh(pcity_homecity);
-  city_refresh(pcity_dest);
-
-  /* Notify the owners of the city. */
-  send_city_info(pplayer, pcity_homecity);
-  send_city_info(city_owner(pcity_dest), pcity_dest);
-
-  /* 
-   * Notify the players about the other city so that they get the
-   * tile_trade value.
-   */
-  send_city_info(city_owner(pcity_dest), pcity_homecity);
-  send_city_info(pplayer, pcity_dest);
+  
+  if (can_establish) {
+    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);
+    }
+  
+    /* Notify the owners of the city. */
+    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);
+    }
+  
+    /* 
+     * Notify the players about the other city so that they get the
+     * tile_trade value.
+     */
+    if(pplayer != city_owner(pcity_dest)) {
+      send_city_info(city_owner(pcity_dest), pcity_homecity);
+      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);
+      }
+    }
+    
+    send_player_info(pplayer, pplayer);
+    
+  }
   conn_list_do_unbuffer(&pplayer->connections);
   return TRUE;
 }