Complete.Org: Mailing Lists: Archives: freeciv-dev: January 2004:
[Freeciv-Dev] (PR#6931) Returning GR_FAILED in execute_orders doesn't ca
Home

[Freeciv-Dev] (PR#6931) Returning GR_FAILED in execute_orders doesn't ca

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: a-l@xxxxxxx
Subject: [Freeciv-Dev] (PR#6931) Returning GR_FAILED in execute_orders doesn't cancel unit orders
From: "Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>
Date: Mon, 26 Jan 2004 09:04:49 -0800
Reply-to: rt@xxxxxxxxxxx

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

> [i-freeciv-lists@xxxxxxxxxxxxx - Mon Jan 26 08:46:13 2004]:

> What are the remaining uses of the enum? Can't it just be killed?
> 
> Gregory: you added the enum some time ago. You said this information
> was required and that the callers will make use of this. Please comment.

It's used by server goto and the AI.  But it seems like only GR_DIED is
ever checked, so it might as well be a boolean.

> There is one fixme in the patch about further event types. To all:
> please don't add fixmes if the solution is so simple. Jason just take
> these 10-30mins to add/make a new patch which adds event types for
> this.

I kindof think we shouldn't add more event types until we have a way to
filter them.  But here it is.

(10-30mins???  Try 2-5 mins...)

jason

Index: client/options.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/options.c,v
retrieving revision 1.90
diff -u -r1.90 options.c
--- client/options.c    2004/01/24 02:58:54     1.90
+++ client/options.c    2004/01/26 17:02:49
@@ -265,6 +265,7 @@
   GEN_EV(N_("Unit: Became More Veteran"),             E_UNIT_BECAME_VET),
   GEN_EV(N_("Unit: Production Upgraded"),             E_UNIT_UPGRADED),
   GEN_EV(N_("Unit: Relocated"),                       E_UNIT_RELOCATED),
+  GEN_EV(N_("Unit: Orders / goto events"),            E_UNIT_ORDERS),
   GEN_EV(N_("Wonder: Finished"),                      E_WONDER_BUILD),
   GEN_EV(N_("Wonder: Made Obsolete"),                 E_WONDER_OBSOLETE),
   GEN_EV(N_("Wonder: Started"),                       E_WONDER_STARTED),
Index: common/events.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/events.h,v
retrieving revision 1.24
diff -u -r1.24 events.h
--- common/events.h     2004/01/11 17:45:03     1.24
+++ common/events.h     2004/01/26 17:02:49
@@ -98,6 +98,7 @@
   E_UNIT_BECAME_VET,
   E_UNIT_UPGRADED,
   E_UNIT_RELOCATED,
+  E_UNIT_ORDERS,
   E_WONDER_BUILD,
   E_WONDER_OBSOLETE,
   E_WONDER_STARTED,
Index: server/unithand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unithand.c,v
retrieving revision 1.286
diff -u -r1.286 unithand.c
--- server/unithand.c   2004/01/25 08:04:53     1.286
+++ server/unithand.c   2004/01/26 17:02:50
@@ -1551,7 +1551,7 @@
 #endif
 
   assign_units_to_transporter(punit, TRUE);
-  if (execute_orders(punit) != GR_DIED) {
+  if (execute_orders(punit)) {
     send_unit_info(NULL, punit);
   }
 }
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.272
diff -u -r1.272 unittools.c
--- server/unittools.c  2004/01/26 07:13:27     1.272
+++ server/unittools.c  2004/01/26 17:02:51
@@ -900,7 +900,8 @@
   }
 
   if (unit_has_orders(punit)) {
-    if (execute_orders(punit) == GR_DIED) {
+    if (!execute_orders(punit)) {
+      /* Unit died. */
       return;
     }
   }
@@ -3162,13 +3163,19 @@
   Executes a unit's orders stored in punit->orders.  The unit is put on idle
   if an action fails or if "patrol" is set and an enemy unit is encountered.
 
+  The return value will be TRUE if the unit lives, FALSE otherwise.  (This
+  function used to return a goto_result enumeration, declared in gotohand.h.
+  But this enumeration was never checked by the caller and just lead to
+  confusion.  All the caller really needs to know is if the unit lived or
+  died; everything else is handled internally within execute_orders.)
+
   If the orders are repeating the loop starts over at the beginning once it
   completes.  To avoid infinite loops on railroad we stop for this
   turn when the unit is back where it started, even if it have moves left.
 
   A unit will attack under orders only on its final action.
 ****************************************************************************/
-enum goto_result execute_orders(struct unit *punit)
+bool execute_orders(struct unit *punit)
 {
   int dest_x, dest_y;
   bool res, last_order;
@@ -3179,7 +3186,10 @@
   assert(unit_has_orders(punit));
 
   freelog(LOG_DEBUG, "Executing orders for %s %d",
-         unit_name(punit->type), punit->id);
+         unit_name(punit->type), punit->id);   
+
+  /* Any time the orders are canceled we should give the player a message.
+   * In the future these should probably get their own event type. */
 
   while (TRUE) {
     struct unit_order order;
@@ -3187,24 +3197,29 @@
     if (punit->moves_left == 0) {
       /* FIXME: this check won't work when actions take 0 MP. */
       freelog(LOG_DEBUG, "  stopping because of no more move points");
-      return GR_OUT_OF_MOVEPOINTS;
+      return TRUE;
     }
 
     if (punit->done_moving) {
       freelog(LOG_DEBUG, "  stopping because we're done this turn");
-      return GR_WAITING;
+      return TRUE;
     }
 
     if (punit->orders.vigilant && maybe_cancel_patrol_due_to_enemy(punit)) {
       /* "Patrol" orders are stopped if an enemy is near. */
-      freelog(LOG_DEBUG, "  stopping because of nearby enemy");
-      return GR_FAILED;
+      freelog(LOG_DEBUG, "  stopping because of nearby enemy");               
+      free_unit_orders(punit);
+      notify_player_ex(pplayer, punit->x, punit->y, E_UNIT_ORDERS,
+                      _("Game: Orders for %s aborted as there "
+                        "are units nearby."),
+                      unit_name(punit->type));
+      return TRUE;
     }
 
     if (moves_made == punit->orders.length) {
       /* For repeating orders, don't repeat more than once per turn. */
       freelog(LOG_DEBUG, "  stopping because we ran a round");
-      return GR_ARRIVED;
+      return TRUE;
     }
 
     last_order = (!punit->orders.repeat
@@ -3229,13 +3244,23 @@
       /* Move unit */
       if (!MAPSTEP(dest_x, dest_y, punit->x, punit->y, order.dir)) {
        freelog(LOG_DEBUG, "  move order sent us to invalid location");
-       return GR_FAILED;
+       free_unit_orders(punit);
+       notify_player_ex(pplayer, punit->x, punit->y, E_UNIT_ORDERS,
+                        _("Game: Orders for %s aborted since they "
+                          "give an invalid location."),
+                        unit_name(punit->type));
+       return TRUE;
       }
 
       if (!last_order
          && maybe_cancel_goto_due_to_enemy(punit, dest_x, dest_y)) {
        freelog(LOG_DEBUG, "  orders canceled because of enemy");
-       return GR_FAILED;
+       free_unit_orders(punit);
+       notify_player_ex(pplayer, punit->x, punit->y, E_UNIT_ORDERS,
+                        _("Game: Orders for %s aborted as there "
+                          "are units in the way."),
+                        unit_name(punit->type));
+       return TRUE;
       }
 
       freelog(LOG_DEBUG, "  moving to %d,%d", dest_x, dest_y);
@@ -3243,34 +3268,41 @@
                                     FALSE, !last_order);
       if (!player_find_unit_by_id(pplayer, unitid)) {
        freelog(LOG_DEBUG, "  unit died while moving.");
-       return GR_DIED;
+       /* A player notification should already have been sent. */
+       return FALSE;
       }
 
       if (res && !same_pos(dest_x, dest_y, punit->x, punit->y)) {
        /* Movement succeeded but unit didn't move. */
        freelog(LOG_DEBUG, "  orders resulted in combat.");
-       return GR_FOUGHT;
+       return TRUE;
       }
 
       if (!res && punit->moves_left > 0) {
        /* Movement failed (ZOC, etc.) */
        freelog(LOG_DEBUG, "  attempt to move failed.");
-       return GR_FAILED;
+       free_unit_orders(punit);
+       notify_player_ex(pplayer, punit->x, punit->y, E_UNIT_ORDERS,
+                        _("Game: Orders for %s aborted because of "
+                          "failed move."),
+                        unit_name(punit->type));
+       return TRUE;
       }
 
       break;
     case ORDER_LAST:
       freelog(LOG_DEBUG, "  client sent invalid order!");
-      notify_player_ex(pplayer, punit->x, punit->y, E_NOEVENT,
+      free_unit_orders(punit);
+      notify_player_ex(pplayer, punit->x, punit->y, E_UNIT_ORDERS,
                       _("Game: Your %s has invalid orders."),
                       unit_name(punit->type));
-      return GR_FAILED;
+      return TRUE;
     }
 
     if (last_order) {
       assert(punit->has_orders == FALSE);
       freelog(LOG_DEBUG, "  stopping because orders are complete");
-      return GR_ARRIVED;
+      return TRUE;
     }
 
     /* We succeeded in moving one step forward */
Index: server/unittools.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.h,v
retrieving revision 1.61
diff -u -r1.61 unittools.h
--- server/unittools.h  2004/01/20 21:52:09     1.61
+++ server/unittools.h  2004/01/26 17:02:51
@@ -82,6 +82,6 @@
 void assign_units_to_transporter(struct unit *ptrans, bool take_from_land);
 bool move_unit(struct unit *punit, int dest_x, int dest_y,
              bool transport_units, bool take_from_land, int move_cost);
-enum goto_result execute_orders(struct unit *punit);
+bool execute_orders(struct unit *punit);
 
 #endif  /* FC__UNITTOOLS_H */

[Prev in Thread] Current Thread [Next in Thread]