[Freeciv-Dev] (PR#15656)

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

The current city-edit packet is complex.  It does things that should be
done one step at a time; Per's recent patch for in-dialog editing shows
that pretty clearly.

This patch changes the city-paint tool so that all it does is create
cities.  This is all the client GUI is set up to do anyway.

-jason 

Index: server/srv_main.c
===================================================================
--- server/srv_main.c   (revision 11626)
+++ server/srv_main.c   (working copy)
@@ -1032,7 +1032,7 @@
       || type == PACKET_EDIT_MODE
       || type == PACKET_EDIT_TILE
       || type == PACKET_EDIT_UNIT
-      || type == PACKET_EDIT_CITY
+      || type == PACKET_EDIT_CREATE_CITY
       || type == PACKET_EDIT_PLAYER) {
     if (!server_handle_packet(type, packet, NULL, pconn)) {
       freelog(LOG_ERROR, "Received unknown packet %d from %s",
Index: server/edithand.c
===================================================================
--- server/edithand.c   (revision 11626)
+++ server/edithand.c   (working copy)
@@ -176,89 +176,32 @@
 /****************************************************************************
   We do some checking of input but not enough.
 ****************************************************************************/
-void handle_edit_city(struct connection *pc, struct packet_edit_city *packet)
+void handle_edit_create_city(struct connection *pc,
+                            int owner, int x, int y)
 {
-  struct tile *ptile = map_pos_to_tile(packet->x, packet->y);
+  struct tile *ptile = map_pos_to_tile(x, y);
   struct city *pcity;
-  struct player *pplayer = get_player(packet->owner);
-  int i;
-  int old_traderoutes[NUM_TRADEROUTES];
-
+  struct player *pplayer = get_player(owner);
   if (pc->access_level != ALLOW_HACK || !game.info.is_edit_mode
       || !pplayer || !ptile) {
     return;
   }
 
-  pcity = tile_get_city(ptile);
-  if (!pcity) {
-    if (!city_can_be_built_here(ptile, NULL)) {
-      notify_player(pplayer, ptile, E_BAD_COMMAND,
-                    _("Cannot build city on this tile."));
-      return;
-    }
-
-    /* new city */
-    create_city(pplayer, ptile, city_name_suggestion(pplayer, ptile));
-    pcity = tile_get_city(ptile);
-
-    if (!pcity) {
-      notify_player(pplayer, ptile, E_BAD_COMMAND,
-                   _("Could not create city."));
-      return;
-    }
-  }
-
-  if (!city_change_size(pcity, CLIP(0, packet->size, MAX_CITY_SIZE))) {
-    /* City died. */
+  if (!city_can_be_built_here(ptile, NULL)) {
+    notify_conn(pc->self, ptile, E_BAD_COMMAND,
+               _("Cannot build city on this tile."));
     return;
   }
 
-  /* FIXME: should probably be a different packet_edit_trade_route. */
-  for (i = 0; i < NUM_TRADEROUTES; i++) {
-    old_traderoutes[i] = pcity->trade[i];
-  }
-  for (i = 0; i < NUM_TRADEROUTES; i++) {
-    struct city *oldcity = find_city_by_id(old_traderoutes[i]);
-    struct city *newcity = find_city_by_id(packet->trade[i]);
+  /* new city */
+  create_city(pplayer, ptile, city_name_suggestion(pplayer, ptile));
+  pcity = tile_get_city(ptile);
 
-    /*
-     * This complicated bit of logic either deletes or creates trade routes.
-     *
-     * FIXME: What happens if (oldcity && newcity && oldcity != newcity) ?
-     */
-    if (oldcity && !newcity) {
-      remove_trade_route(pcity, oldcity);
-    } else if (newcity && !oldcity && can_cities_trade(pcity, newcity)) {
-      establish_trade_route(pcity, newcity);
-    }
+  if (!pcity) {
+    notify_conn(pc->self, ptile, E_BAD_COMMAND,
+               _("Could not create city."));
+    return;
   }
-
-  pcity->food_stock = MAX(packet->food_stock, 0);
-  pcity->shield_stock = MAX(packet->shield_stock, 0);
-
-  pcity->did_buy = packet->did_buy;
-  pcity->did_sell = packet->did_sell;
-  pcity->was_happy = packet->was_happy;
-  pcity->airlift = packet->airlift;
-
-  pcity->turn_last_built = CLIP(0, packet->turn_last_built, game.info.turn);
-  pcity->turn_founded = CLIP(0, packet->turn_founded, game.info.turn);
-  pcity->before_change_shields = MAX(packet->before_change_shields, 0);
-  pcity->disbanded_shields = MAX(packet->disbanded_shields, 0);
-  pcity->caravan_shields = MAX(packet->caravan_shields, 0);
-  pcity->last_turns_shield_surplus
-    = MAX(packet->last_turns_shield_surplus, 0);
-
-  /* FIXME: Might want to check these values. */
-  pcity->changed_from.is_unit = packet->changed_from_is_unit;
-  pcity->changed_from.value = packet->changed_from_id;
-
-  /* make everything sane.  Note some refreshes may already have been
-   * done above. */
-  city_refresh(pcity);
-
-  /* send update back to client */
-  send_city_info(NULL, pcity);  
 }
 
 /**************************************************************************
Index: common/packets.def
===================================================================
--- common/packets.def  (revision 11626)
+++ common/packets.def  (working copy)
@@ -1383,26 +1383,9 @@
   # be controlled even by a privilidged client.
 end
 
-PACKET_EDIT_CITY=127;cs,handle-per-conn,lsend
+PACKET_EDIT_CREATE_CITY=127;cs,handle-per-conn
   PLAYER owner;
-  COORD x, y; key # cities are keyed by tile not by ID
-  UINT8 size;
-  UINT16 food_stock, shield_stock;
-  UINT16 trade[NUM_TRADEROUTES];
-  TURN turn_last_built;
-  UINT8 changed_from_id;
-  BOOL changed_from_is_unit;
-  UINT16 before_change_shields;
-  UINT16 disbanded_shields;
-  UINT16 caravan_shields;
-  UINT16 last_turns_shield_surplus;
-  BV_IMPRS improvements;
-  BOOL did_buy, did_sell, was_happy, airlift, diplomat_investigate;
-  TURN turn_founded;
-
-  # Other values of the city (worker placement, worklist, etc.) can be set
-  # directly through regular packets.  Some values are simply caches and
-  # cannot be controlled even by a privilidged client.
+  COORD x, y;
 end
 
 PACKET_EDIT_PLAYER=128;cs,handle-per-conn,lsend
Index: client/gui-gtk-2.0/editdlg.c
===================================================================
--- client/gui-gtk-2.0/editdlg.c        (revision 11626)
+++ client/gui-gtk-2.0/editdlg.c        (working copy)
@@ -53,9 +53,6 @@
 
 enum city_param {
   CPARAM_OWNER,
-  CPARAM_SIZE,
-  CPARAM_FOOD,
-  CPARAM_SHIELDS,
   CPARAM_LAST
 };
 
@@ -237,15 +234,6 @@
   case CPARAM_OWNER:
     pcity->owner = get_player(gtk_spin_button_get_value_as_int(spinbutton));
     return;
-  case CPARAM_SIZE:
-    pcity->size = gtk_spin_button_get_value_as_int(spinbutton);
-    return;
-  case CPARAM_FOOD:
-    pcity->food_stock = gtk_spin_button_get_value_as_int(spinbutton);
-    return;
-  case CPARAM_SHIELDS:
-    pcity->shield_stock = gtk_spin_button_get_value_as_int(spinbutton);
-    return;
   case CPARAM_LAST:
     break;
   }
@@ -382,13 +370,9 @@
   int i;
   struct city *pcity = editor_get_selected_city();
 
-  const char *names[CPARAM_LAST] = { _("Owner"), _("Size"),
-                                    _("Food"), _("Shields") };
+  const char *names[CPARAM_LAST] = { _("Owner"), };
   int inits[CPARAM_LAST][3] = {
     {pcity->owner->player_no, 0, game.info.nplayers - 1},
-    {pcity->size, 1, 63},
-    {pcity->food_stock, 0, 10000},
-    {pcity->shield_stock, 0, 10000},
   };
 
   vbox = gtk_vbox_new(FALSE, 5);
Index: client/editor.c
===================================================================
--- client/editor.c     (revision 11626)
+++ client/editor.c     (working copy)
@@ -152,46 +152,13 @@
 ****************************************************************************/
 static void do_city(struct tile *ptile)
 {
-  struct packet_edit_city packet;
-  struct city *pcity = selected_city;
-  int i;
+  struct packet_edit_create_city packet = {
+    .owner = selected_city->owner->player_no,
+    .x = ptile->x,
+    .y = ptile->y
+  };
 
-  packet.owner = pcity->owner->player_no;
-  packet.x = ptile->x;
-  packet.y = ptile->y;
-
-  packet.size = pcity->size;
-  for (i = 0; i < NUM_TRADEROUTES; i++) {
-    packet.trade[i] = pcity->trade[i];
-  }
-
-  packet.food_stock = pcity->food_stock;
-  packet.shield_stock = pcity->shield_stock;
-
-  packet.turn_last_built = pcity->turn_last_built;
-  packet.turn_founded = pcity->turn_founded;
-  packet.changed_from_is_unit = pcity->changed_from.is_unit;
-  packet.changed_from_id = pcity->changed_from.value;
-  packet.before_change_shields = pcity->before_change_shields;
-  packet.disbanded_shields = pcity->disbanded_shields;
-  packet.caravan_shields = pcity->caravan_shields;
-  packet.last_turns_shield_surplus = pcity->last_turns_shield_surplus;
-
-  packet.diplomat_investigate = FALSE; /* FIXME: this overwrites the value! */
-
-  packet.airlift = pcity->airlift;
-  packet.did_buy = pcity->did_buy;
-  packet.did_sell = pcity->did_sell;
-  packet.was_happy = pcity->was_happy;
-
-  BV_CLR_ALL(packet.improvements);
-  impr_type_iterate(building) {
-    if (city_got_building(pcity, building)) {
-      BV_SET(packet.improvements, building);
-    }
-  } impr_type_iterate_end;
-
-  send_packet_edit_city(&aconnection, &packet);
+  send_packet_edit_create_city(&aconnection, &packet);
 }
 
 #if 0