[Freeciv-Dev] (PR#7344) Patch: Expanded Orders

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=7344 >

Attached is a new up-to-date "expanded orders" patch.  This patch
introduces just one new order: ORDER_BUILD_CITY.  This order is
supported at the server.  Also included are some client changes to
provide a menu option to goto-location-and-build-city (ctrl-shift-b).

This is a preliminary patch.  Please test and comment on the design.

--- Common design

I considered making a new ORDER_ACTION, with a separate enum
unit_action.  The list of unit actions could then grow and grow.  But I
figured this didn't give any advantage over making all ACTIONs into
their own orders.  So instead we just have a new order type
ORDER_BUILD_CITY.  Note that there are many more (potentially dozens) of
possible actions, so this could grow a bit.

--- Server design

I was tempted to change the server design a bit.  But in the end the
only changes I made to unittools was to add support for the new order. 
However this design won't scale well because eventually execute_orders()
will be hundreds or thousands of lines long.  I think a callback
mechanism with a lookup into an array of functions would work ok.  Also
note that calling handle_xxx() really isn't the best thing to do here
since handle_xxx() functions are pretty fuzzy.  This is the same problem
the AI runs into - for instance handle_city_build may build a new city
or add on to an existing city, it gives an error message for some
failures but returns silently for others, and never is there a return
value to indicate what happened.

--- Client design

This is pretty ugly.  We have the hover state (HOVER_PATROL,
HOVER_CONNECT, HOVER_GOTO, HOVER_NUKE, etc.).  We have the connect
activity (an enum unit_activity), which is only valid for HOVER_CONNECT.
 And we have the last order (enum unit_orders), which is only valid for
HOVER_GOTO.  send_path_orders() handles CONNECT, GOTO (except air goto
which is completely separate), and PATROL, and so must take both an
activity and a last order.  Confused yet?  The code is.  Getting rid of
server goto would help.

jason

Index: client/control.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/control.c,v
retrieving revision 1.150
diff -u -r1.150 control.c
--- client/control.c    13 Nov 2004 08:27:46 -0000      1.150
+++ client/control.c    7 Dec 2004 09:12:51 -0000
@@ -55,6 +55,7 @@
 int hover_unit = 0; /* id of unit hover_state applies to */
 enum cursor_hover_state hover_state = HOVER_NONE;
 enum unit_activity connect_activity;
+enum unit_orders goto_last_order; /* Last order for goto */
 /* This may only be here until client goto is fully implemented.
    It is reset each time the hower_state is reset. */
 bool draw_goto_line = TRUE;
@@ -77,13 +78,18 @@
                         enum quickselect_type qtype);
 
 /**************************************************************************
-...
+  Enter the given hover state.
+
+    activity => The connect activity (ACTIVITY_ROAD, etc.)
+    order => The last order (ORDER_BUILD_CITY, ORDER_LAST, etc.)
 **************************************************************************/
 void set_hover_state(struct unit *punit, enum cursor_hover_state state,
-                    enum unit_activity activity)
+                    enum unit_activity activity,
+                    enum unit_orders order)
 {
   assert(punit != NULL || state == HOVER_NONE);
   assert(state == HOVER_CONNECT || activity == ACTIVITY_LAST);
+  assert(state == HOVER_GOTO || order == ORDER_LAST);
   draw_goto_line = TRUE;
   if (punit)
     hover_unit = punit->id;
@@ -91,6 +97,7 @@
     hover_unit = 0;
   hover_state = state;
   connect_activity = activity;
+  goto_last_order = order;
   exit_goto_state();
 }
 
@@ -222,7 +229,7 @@
   struct unit *punit_old_focus = punit_focus;
   struct unit *candidate = find_best_focus_candidate(FALSE);
 
-  set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST);
+  set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
   if (!can_client_change_view()) {
     return;
   }
@@ -593,9 +600,9 @@
 }
 
 /**************************************************************************
-...
+  Do a goto with an order at the end (or ORDER_LAST).
 **************************************************************************/
-void request_unit_goto(void)
+void request_unit_goto(enum unit_orders last_order)
 {
   struct unit *punit = punit_focus;
 
@@ -603,7 +610,7 @@
     return;
 
   if (hover_state != HOVER_GOTO) {
-    set_hover_state(punit, HOVER_GOTO, ACTIVITY_LAST);
+    set_hover_state(punit, HOVER_GOTO, ACTIVITY_LAST, last_order);
     update_unit_info_label(punit);
     /* Not yet implemented for air units, including helicopters. */
     if (is_air_unit(punit) || is_heli_unit(punit)) {
@@ -695,7 +702,7 @@
 
   if (hover_state != HOVER_CONNECT || connect_activity != activity) {
     /* Enter or change the hover connect state. */
-    set_hover_state(punit_focus, HOVER_CONNECT, activity);
+    set_hover_state(punit_focus, HOVER_CONNECT, activity, ORDER_LAST);
     update_unit_info_label(punit_focus);
 
     enter_goto_state(punit_focus);
@@ -762,15 +769,19 @@
   }
 
   if ((path = path_to_nearest_allied_city(punit))) {
-    enum unit_activity activity = ACTIVITY_LAST;
     int turns = pf_last_position(path)->turn;
 
     if (punit->hp + turns * get_player_bonus(game.player_ptr,
                                             EFT_UNIT_RECOVER)
        < unit_type(punit)->hp) {
-      activity = ACTIVITY_SENTRY;
+      struct unit_order order;
+
+      order.order = ORDER_ACTIVITY;
+      order.activity = ACTIVITY_SENTRY;
+      send_goto_path(punit, path, &order);
+    } else {
+      send_goto_path(punit, path, NULL);
     }
-    send_goto_path(punit, path, activity);
     pf_destroy_path(path);
   }
 }
@@ -995,7 +1006,7 @@
   if(punit->moves_left == 0)
     do_unit_nuke(punit);
   else {
-    set_hover_state(punit, HOVER_NUKE, ACTIVITY_LAST);
+    set_hover_state(punit, HOVER_NUKE, ACTIVITY_LAST, ORDER_LAST);
     update_unit_info_label(punit);
   }
 }
@@ -1012,7 +1023,7 @@
   if(!can_unit_paradrop(punit))
     return;
 
-  set_hover_state(punit, HOVER_PARADROP, ACTIVITY_LAST);
+  set_hover_state(punit, HOVER_PARADROP, ACTIVITY_LAST, ORDER_LAST);
   update_unit_info_label(punit);
 }
 
@@ -1027,7 +1038,7 @@
     return;
 
   if (hover_state != HOVER_PATROL) {
-    set_hover_state(punit, HOVER_PATROL, ACTIVITY_LAST);
+    set_hover_state(punit, HOVER_PATROL, ACTIVITY_LAST, ORDER_LAST);
     update_unit_info_label(punit);
     /* Not yet implemented for air units, including helicopters. */
     if (is_air_unit(punit) || is_heli_unit(punit)) {
@@ -1447,7 +1458,7 @@
       do_unit_patrol_to(punit, ptile);
       break;   
     }
-    set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST);
+    set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
     update_unit_info_label(punit);
   }
 
@@ -1641,7 +1652,7 @@
     }
   }
 
-  set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST);
+  set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
 }
 
 /**************************************************************************
@@ -1680,7 +1691,7 @@
     }
   }
 
-  set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST);
+  set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
 }
  
 /**************************************************************************
@@ -1705,7 +1716,7 @@
     }
   }
 
-  set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST);
+  set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
 }
  
 /**************************************************************************
@@ -1724,7 +1735,7 @@
   if (hover_state != HOVER_NONE && !popped) {
     struct unit *punit = player_find_unit_by_id(game.player_ptr, hover_unit);
 
-    set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST);
+    set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
     update_unit_info_label(punit);
 
     keyboardless_goto_button_down = FALSE;
@@ -1842,7 +1853,7 @@
 void key_unit_goto(void)
 {
   if (punit_focus) {
-    request_unit_goto();
+    request_unit_goto(ORDER_LAST);
   }
 }
 
Index: client/control.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/control.h,v
retrieving revision 1.45
diff -u -r1.45 control.h
--- client/control.h    29 Sep 2004 02:24:19 -0000      1.45
+++ client/control.h    7 Dec 2004 09:12:51 -0000
@@ -32,6 +32,7 @@
 extern int hover_unit; /* unit hover_state applies to */
 extern enum cursor_hover_state hover_state;
 extern enum unit_activity connect_activity;
+extern enum unit_orders goto_last_order;
 extern bool draw_goto_line;
 extern bool non_ai_unit_focus;
 
@@ -47,7 +48,8 @@
 void do_map_click(struct tile *ptile, enum quickselect_type qtype);
 
 void set_hover_state(struct unit *punit, enum cursor_hover_state state,
-                    enum unit_activity activity);
+                    enum unit_activity connect_activity,
+                    enum unit_orders goto_last_order);
 void request_center_focus_unit(void);
 void request_move_unit_direction(struct unit *punit, int dir);
 void request_new_unit_activity(struct unit *punit, enum unit_activity act);
@@ -63,7 +65,7 @@
 void request_unit_connect(enum unit_activity activity);
 void request_unit_disband(struct unit *punit);
 void request_unit_fortify(struct unit *punit);
-void request_unit_goto(void);
+void request_unit_goto(enum unit_orders last_order);
 void request_unit_move_done(void);
 void request_unit_nuke(struct unit *punit);
 void request_unit_paradrop(struct unit *punit);
Index: client/goto.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/goto.c,v
retrieving revision 1.79
diff -u -r1.79 goto.c
--- client/goto.c       22 Nov 2004 19:14:41 -0000      1.79
+++ client/goto.c       7 Dec 2004 09:12:51 -0000
@@ -793,7 +793,7 @@
 **************************************************************************/
 static void send_path_orders(struct unit *punit, struct pf_path *path,
                             bool repeat, bool vigilant,
-                            enum unit_activity final_activity)
+                            struct unit_order *final_order)
 {
   struct packet_unit_orders p;
   int i;
@@ -834,10 +834,11 @@
     old_tile = new_tile;
   }
 
-  if (final_activity != ACTIVITY_LAST) {
-    p.orders[i] = ORDER_ACTIVITY;
-    p.dir[i] = -1;
-    p.activity[i] = final_activity;
+  if (final_order) {
+    p.orders[i] = final_order->order;
+    p.dir[i] = (final_order->order == ORDER_MOVE) ? final_order->dir : -1;
+    p.activity[i] = (final_order->order == ORDER_ACTIVITY)
+      ? final_order->activity : ACTIVITY_LAST;
     p.length++;
   }
 
@@ -851,9 +852,9 @@
   Send an arbitrary goto path for the unit to the server.
 **************************************************************************/
 void send_goto_path(struct unit *punit, struct pf_path *path,
-                   enum unit_activity final_activity)
+                   struct unit_order *final_order)
 {
-  send_path_orders(punit, path, FALSE, FALSE, final_activity);
+  send_path_orders(punit, path, FALSE, FALSE, final_order);
 }
 
 /**************************************************************************
@@ -887,7 +888,7 @@
   pf_destroy_map(map);
   pf_destroy_path(return_path);
 
-  send_path_orders(punit, path, TRUE, TRUE, ACTIVITY_LAST);
+  send_path_orders(punit, path, TRUE, TRUE, NULL);
 
   pf_destroy_path(path);
 }
@@ -987,7 +988,14 @@
     path = pft_concat(path, goto_map.parts[i].path);
   }
 
-  send_goto_path(punit, path, ACTIVITY_LAST);
+  if (goto_last_order == ORDER_LAST) {
+    send_goto_path(punit, path, NULL);
+  } else {
+    struct unit_order order;
+
+    order.order = goto_last_order;
+    send_goto_path(punit, path, &order);
+  }
   pf_destroy_path(path);
 }
 
Index: client/goto.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/goto.h,v
retrieving revision 1.16
diff -u -r1.16 goto.h
--- client/goto.h       29 Sep 2004 02:24:19 -0000      1.16
+++ client/goto.h       7 Dec 2004 09:12:51 -0000
@@ -34,7 +34,7 @@
 
 void request_orders_cleared(struct unit *punit);
 void send_goto_path(struct unit *punit, struct pf_path *path,
-                   enum unit_activity final_activity);
+                   struct unit_order *last_order);
 void send_patrol_route(struct unit *punit);
 void send_goto_route(struct unit *punit);
 void send_connect_route(struct unit *punit, enum unit_activity activity);
Index: client/mapctrl_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapctrl_common.c,v
retrieving revision 1.43
diff -u -r1.43 mapctrl_common.c
--- client/mapctrl_common.c     22 Nov 2004 19:31:30 -0000      1.43
+++ client/mapctrl_common.c     7 Dec 2004 09:12:52 -0000
@@ -427,7 +427,7 @@
         player_find_unit_by_id(game.player_ptr, hover_unit);
 
     do_unit_goto(ptile);
-    set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST);
+    set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
     update_unit_info_label(punit);
   }
   keyboardless_goto_active = FALSE;
@@ -447,7 +447,7 @@
       && !same_pos(keyboardless_goto_start_tile, ptile)
       && can_client_issue_orders()) {
     keyboardless_goto_active = TRUE;
-    request_unit_goto();
+    request_unit_goto(ORDER_LAST);
   }
 }
 
Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.163
diff -u -r1.163 mapview_common.c
--- client/mapview_common.c     6 Dec 2004 18:01:14 -0000       1.163
+++ client/mapview_common.c     7 Dec 2004 09:12:52 -0000
@@ -1910,7 +1910,7 @@
   anim_timer = renew_timer_start(anim_timer, TIMER_USER, TIMER_ACTIVE);
 
   if (punit == get_unit_in_focus() && hover_state != HOVER_NONE) {
-    set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST);
+    set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
     update_unit_info_label(punit);
   }
 
Index: client/gui-gtk-2.0/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/mapview.c,v
retrieving revision 1.147
diff -u -r1.147 mapview.c
--- client/gui-gtk-2.0/mapview.c        1 Nov 2004 03:39:45 -0000       1.147
+++ client/gui-gtk-2.0/mapview.c        7 Dec 2004 09:12:53 -0000
@@ -188,7 +188,7 @@
 
   if(punit) {
     if (hover_unit != punit->id)
-      set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST);
+      set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
 
     switch (hover_state) {
     case HOVER_NONE:
Index: client/gui-gtk-2.0/menu.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/menu.c,v
retrieving revision 1.57
diff -u -r1.57 menu.c
--- client/gui-gtk-2.0/menu.c   25 Nov 2004 06:57:17 -0000      1.57
+++ client/gui-gtk-2.0/menu.c   7 Dec 2004 09:12:53 -0000
@@ -129,6 +129,7 @@
   MENU_ORDER_CONNECT_ROAD,
   MENU_ORDER_CONNECT_RAIL,
   MENU_ORDER_CONNECT_IRRIGATE,
+  MENU_ORDER_GO_BUILD_CITY,
   MENU_ORDER_PATROL,
   MENU_ORDER_GOTO,
   MENU_ORDER_GOTO_CITY,
@@ -447,6 +448,9 @@
    case MENU_ORDER_CONNECT_ROAD:
     key_unit_connect(ACTIVITY_ROAD);
     break;
+  case MENU_ORDER_GO_BUILD_CITY:
+    request_unit_goto(ORDER_BUILD_CITY);
+    break;
    case MENU_ORDER_CONNECT_RAIL:
     key_unit_connect(ACTIVITY_RAILROAD);
     break;
@@ -787,6 +791,8 @@
    orders_menu_callback, MENU_ORDER_CONNECT_RAIL},
   {"/" N_("Orders") "/" N_("_Connect") "/" N_("_Irrigate"), "<ctrl><shift>i",
    orders_menu_callback, MENU_ORDER_CONNECT_IRRIGATE},
+  {"/" N_("Orders") "/" N_("Go _to") "/" N_("_Build city"), "<ctrl><shift>b",
+   orders_menu_callback, MENU_ORDER_GO_BUILD_CITY},
   { "/" N_("Orders") "/" N_("Patrol (_Q)"),            "q",
        orders_menu_callback,   MENU_ORDER_PATROL                               
        },
   { "/" N_("Orders") "/" N_("_Go to"),                 "g",
Index: common/unit.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/unit.h,v
retrieving revision 1.130
diff -u -r1.130 unit.h
--- common/unit.h       6 Dec 2004 18:01:15 -0000       1.130
+++ common/unit.h       7 Dec 2004 09:12:53 -0000
@@ -34,9 +34,10 @@
   ACTIVITY_LAST   /* leave this one last */
 };
 
-/* Changing this enum will break savegame and network compatability. */
+/* Changing this enum will break network compatability. */
 enum unit_orders {
-  ORDER_MOVE, ORDER_FULL_MP, ORDER_ACTIVITY,
+  ORDER_MOVE, ORDER_ACTIVITY,
+  ORDER_FULL_MP, ORDER_BUILD_CITY, /* and plenty more for later... */
   ORDER_LAST
 };
 
Index: server/savegame.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/savegame.c,v
retrieving revision 1.210
diff -u -r1.210 savegame.c
--- server/savegame.c   30 Nov 2004 08:37:03 -0000      1.210
+++ server/savegame.c   7 Dec 2004 09:12:54 -0000
@@ -266,6 +266,9 @@
   case 'w':
   case 'W':
     return ORDER_FULL_MP;
+  case 'b':
+  case 'B':
+    return ORDER_BUILD_CITY;
   case 'a':
   case 'A':
     return ORDER_ACTIVITY;
@@ -287,6 +290,8 @@
     return 'w';
   case ORDER_ACTIVITY:
     return 'a';
+  case ORDER_BUILD_CITY:
+    return 'b';
   case ORDER_LAST:
     break;
   }
@@ -2662,6 +2667,7 @@
          act_buf[j] = activity2char(punit->orders.list[j].activity);
          break;
        case ORDER_FULL_MP:
+       case ORDER_BUILD_CITY:
        case ORDER_LAST:
          break;
        }
Index: server/unithand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unithand.c,v
retrieving revision 1.316
diff -u -r1.316 unithand.c
--- server/unithand.c   4 Dec 2004 04:44:08 -0000       1.316
+++ server/unithand.c   7 Dec 2004 09:12:54 -0000
@@ -1607,6 +1607,9 @@
 
 
   for (i = 0; i < packet->length; i++) {
+    if (packet->orders[i] < 0 || packet->orders[i] > ORDER_LAST) {
+      packet->orders[i] = ORDER_LAST;
+    }
     switch (packet->orders[i]) {
     case ORDER_MOVE:
       if (!is_valid_dir(packet->dir[i])) {
@@ -1636,10 +1639,10 @@
       }
       break;
     case ORDER_FULL_MP:
+    case ORDER_BUILD_CITY:
       break;
-    default:
+    case ORDER_LAST:
       /* An invalid order.  This is handled in execute_orders. */
-      packet->orders[i] = ORDER_LAST;
       break;
     }
   }
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.312
diff -u -r1.312 unittools.c
--- server/unittools.c  6 Dec 2004 18:01:15 -0000       1.312
+++ server/unittools.c  7 Dec 2004 09:12:55 -0000
@@ -3075,6 +3075,21 @@
        send_unit_info(NULL, punit);
       }
       break;
+    case ORDER_BUILD_CITY:
+      handle_unit_build_city(pplayer, unitid,
+                            city_name_suggestion(pplayer, punit->tile));
+      freelog(LOG_DEBUG, "  building city");
+      if (player_find_unit_by_id(pplayer, unitid)) {
+       /* Build failed. */
+       cancel_orders(punit, " orders canceled; failed to build city");
+       notify_player_ex(pplayer, punit->tile, E_UNIT_ORDERS,
+                        _("Game: Orders for %s aborted because building "
+                          "of city failed."), unit_name(punit->type));
+       return TRUE;
+      } else {
+       /* Build succeeded => unit "died" */
+       return FALSE;
+      }
     case ORDER_ACTIVITY:
       activity = order.activity;
       if (!can_unit_do_activity(punit, activity)) {