Complete.Org: Mailing Lists: Archives: freeciv-dev: February 2003:
[Freeciv-Dev] (PR#3524) another look at the goto_dest problem
Home

[Freeciv-Dev] (PR#3524) another look at the goto_dest problem

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: undisclosed-recipients:;
Subject: [Freeciv-Dev] (PR#3524) another look at the goto_dest problem
From: "Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>
Date: Wed, 26 Feb 2003 00:50:45 -0800
Reply-to: rt@xxxxxxxxxxxxxx

I took a step back from the goto_dest problem (see PR#3013), and I 
realized that:

- It really is not a problem for gen-topologies.  The current code just 
initializes the goto_dest to (0,0) and nothing overly bad results (the 
occasional city gets founded at (0,0); enemies are more likely to attack 
this city).  All we have to do is initialize the goto_dest to (0,0) 
native and things will work the same way under gen-topologies.  Under an 
isometric map, for instance, this may end up being (0,39), but as long 
as the map doesn't wrap in the native y direction there's no 
(additional) problem.  A civilization located close to (0,0) on a donut 
world would be strongly affected, I think.

- Nonetheless, the current situation is quite awful and should be 
corrected.  If you initialize the (0,0) as (-1,-1) instead, the number 
of segfaults you get is impressive.  This is evidence that the server 
actually _is_ using this (0,0) as a valid goto destination.

- goto_dest is rarely unset.  This may be a problem, but I suspect in 
most cases it's not.  When a goto target is reached, it doesn't matter 
since the unit's already at the goto_dest.  And once the action at that 
destionation is complete, a new goto_dest will be assigned.

- That the same goto_dest structure is used for client goto and AI goto 
is really ugly.  But separating this is a very large endeavor.

The problem with Per's patch (3013) was that it tried to fix more than 
just this problem.  It included partial fixes of client and AI goto as 
well.  But the inherent non-robustness of this code is a much bigger 
issue than just the initialization problem, and so we only ended up with 
problems.

This patch aims to just fix the initialization problem.  It introduces 
wrapper macros: set_goto_dest, clear_goto_dest, is_goto_dest_set, 
goto_dest_x, and goto_dest_y.  These can be used to save a little bit of 
code, but more importantly to avoid having to change all the callers 
when the goto_dest structure changes.

In the patch, clear_goto_dest sets the goto_dest to (-1,-1). 
set_goto_dest sets the destination (with sanity checking). 
is_goto_dest_set checks for the (-1,-1) case.  The rest of the patch is 
heavily based on Per's original one, except that the macros are used 
instead of direct accesses and the extra fixes are not included. 
(-1,-1) is used when sending the goto_dest across the network; it is 
interpreted correctly at both ends.

I have changed the goto_dest structure slightly, just to make it obvious 
that we've caught all cases.  But it should now be easy to change it to 
either use the old structure or to use the punit->go pointer with a NULL 
special-case.

Finally, I ran into what could end up being a big problem in the network 
code.  The network sends the goto_dest coordinates as an unsigned 8-bit 
integer.  Initially this meant sending (-1,-1) was translated into 
(255,255) and caused a client error.  To avoid this I changed it to a 
signed 16-bit integer value.  In the general case, the use of uint8 
could be a major problem since it places an unreasonable limitation on 
the size of the map - a 200x100 iso-rectangular map will exceed these 
bounds.  If the uint8 is used everywhere when sending coordinates (as I 
expect it is), then we'll have another set of problems. (I've tried 
running the corecleanups on large maps in the past, with very bad 
results.  Perhaps this is one of the reasons.)

jason

? _g
? diff
? ggz.diff
? lib
? nohup.out
? rc
? rc2
? ss
? test-game.gz
? test.pl
? common/fciconv.c
? common/fciconv.h
? data/civclient.dsc
? data/civclient.dsc.in
? data/civserver.dsc
? data/civserver.dsc.in
? data/civserver.room
? data/hills.spec
? data/mountains.spec
? data/theme
? data/nation/convert.pl
? data/trident/.new.tiles.xpm
? m4/ggz.m4
? m4/xaw-client.m4
Index: ai/aiair.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aiair.c,v
retrieving revision 1.12
diff -u -r1.12 aiair.c
--- ai/aiair.c  2003/02/17 22:49:27     1.12
+++ ai/aiair.c  2003/02/26 08:15:11
@@ -165,7 +165,7 @@
 /**********************************************************************
  * Find something to bomb
  * Air-units specific victim search
- * Returns the want for the best target, records target in goto_dest_x,y
+ * Returns the want for the best target, records target in punit's goto_dest
  * TODO: take counterattack dangers into account
  * TODO: make separate handicaps for air units seeing targets
  *       IMO should be more restrictive than general H_MAP, H_FOG
@@ -208,8 +208,7 @@
        && (air_can_move_between (max_dist, x, y, x1, y1, pplayer) >= 0)){
       int new_best = ai_evaluate_tile_for_air_attack(punit, x1, y1);
       if (new_best > best) {
-       punit->goto_dest_x = x1;
-       punit->goto_dest_y = y1;
+       set_goto_dest(punit, x1, y1);
        best = new_best;
        freelog(LOG_DEBUG, "%s wants to attack tile (%d, %d)", 
                unit_type(punit)->name, x1, y1);
@@ -305,19 +304,18 @@
 
     if (punit->activity == ACTIVITY_GOTO
       /* We are on a GOTO.  Check if it will get us anywhere */
-       && is_airunit_refuel_point(punit->goto_dest_x, punit->goto_dest_y, 
+       && is_airunit_refuel_point(goto_dest_x(punit), goto_dest_y(punit), 
                                   pplayer, punit->type, FALSE)
        && air_can_move_between (punit->moves_left/SINGLE_MOVE, 
                                  punit->x, punit->y, 
-                                punit->goto_dest_x, punit->goto_dest_y,
+                                goto_dest_x(punit), goto_dest_y(punit),
                                 pplayer) >= 0) {
       /* It's an ok GOTO, just go there */
       result = do_unit_goto(punit, GOTO_MOVE_ANY, FALSE);
     } else if (find_nearest_airbase(punit->x, punit->y, punit, 
                             &refuel_x, &refuel_y)) {
       /* Go refuelling */
-      punit->goto_dest_x = refuel_x;
-      punit->goto_dest_y = refuel_y;
+      set_goto_dest(punit, refuel_x, refuel_y);
       freelog(LOG_DEBUG, "Sent %s to refuel", unit_type(punit)->name);
       set_unit_activity(punit, ACTIVITY_GOTO);
       result = do_unit_goto(punit, GOTO_MOVE_ANY, FALSE);
@@ -338,9 +336,10 @@
   } else if (punit->fuel == unit_type(punit)->fuel
             && find_something_to_bomb(punit, punit->x, punit->y) > 0) {
 
-    /* Found target, coordinates are in punit->goto_dest_[xy]
+    /* Found target, coordinates are in punit's goto_dest.
      * TODO: separate attacking into a function, check for the best 
      * tile to attack from */
+    assert(is_goto_dest_set(punit));
     set_unit_activity(punit, ACTIVITY_GOTO);
     if (!ai_unit_gothere(punit)) {
       return; /* died */
@@ -350,11 +349,12 @@
 
     /* We could use ai_military_findvictim here, but I don't trust it... */
     set_unit_activity(punit, ACTIVITY_IDLE);
-    if (is_tiles_adjacent(punit->x, punit->y, 
-                         punit->goto_dest_x, punit->goto_dest_y)) {
+    if (is_goto_dest_set(punit)
+       && is_tiles_adjacent(punit->x, punit->y, 
+                            goto_dest_x(punit), goto_dest_y(punit))) {
       int id = punit->id;
-      (void) handle_unit_move_request(punit, punit->goto_dest_x,
-                                     punit->goto_dest_y, TRUE, FALSE);
+      (void) handle_unit_move_request(punit, goto_dest_x(punit),
+                                     goto_dest_y(punit), TRUE, FALSE);
       if ((punit = find_unit_by_id(id)) != NULL && punit->moves_left > 0) {
        /* Fly home now */
        ai_manage_airunit(pplayer, punit);
@@ -372,8 +372,7 @@
               unit_type(punit)->name, dest_x, dest_y, 
               (map_get_city(dest_x, dest_y) ? 
                map_get_city(dest_x, dest_y)->name : ""));
-      punit->goto_dest_x = dest_x;
-      punit->goto_dest_y = dest_y;
+      set_goto_dest(punit, dest_x, dest_y);
       set_unit_activity(punit, ACTIVITY_GOTO);
       result = do_unit_goto(punit, GOTO_MOVE_ANY, FALSE);
     } else {
Index: ai/aidiplomat.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aidiplomat.c,v
retrieving revision 1.13
diff -u -r1.13 aidiplomat.c
--- ai/aidiplomat.c     2003/02/17 22:49:27     1.13
+++ ai/aidiplomat.c     2003/02/26 08:15:12
@@ -576,7 +576,8 @@
   /* If we have a role, we have a valid goto. Check if we have a valid city. */
   if (punit->ai.ai_role == AIUNIT_ATTACK
       || punit->ai.ai_role == AIUNIT_DEFEND_HOME) {
-    ctarget = map_get_city(punit->goto_dest_x, punit->goto_dest_y);
+    assert(is_goto_dest_set(punit));
+    ctarget = map_get_city(goto_dest_x(punit), goto_dest_y(punit));
   }
 
   /* Check if existing target still makes sense */
@@ -631,8 +632,7 @@
     }
 
     /* TODO: following lines to be absorbed in a goto wrapper */
-    punit->goto_dest_x = ctarget->x;
-    punit->goto_dest_y = ctarget->y;
+    set_goto_dest(punit, ctarget->x, ctarget->y);
     ai_unit_new_role(punit, task, ctarget->x, ctarget->y);
   }
 
@@ -654,12 +654,12 @@
   /* Check if we can do something with our destination now. */
   if (punit->ai.ai_role == AIUNIT_ATTACK) {
     int dist = real_map_distance(punit->x, punit->y,
-                                 punit->goto_dest_x,
-                                 punit->goto_dest_y);
+                                 goto_dest_x(punit),
+                                 goto_dest_y(punit));
     UNIT_LOG(LOG_DIPLOMAT, punit, "attack, dist %d to %s (%s goto)"
              "[%d mc]", dist, ctarget ? ctarget->name : "(none)", 
              punit->activity == ACTIVITY_GOTO ? "has" : "no",
-             WARMAP_COST(punit->goto_dest_x, punit->goto_dest_y));
+             WARMAP_COST(goto_dest_x(punit), goto_dest_y(punit)));
     if (dist == 1) {
       /* Do our stuff */
       ai_unit_new_role(punit, AIUNIT_NONE, -1, -1);
Index: ai/ailog.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/ailog.c,v
retrieving revision 1.6
diff -u -r1.6 ailog.c
--- ai/ailog.c  2003/02/17 22:49:27     1.6
+++ ai/ailog.c  2003/02/26 08:15:13
@@ -68,15 +68,23 @@
   char buffer2[500];
   va_list ap;
   int minlevel = MIN(LOGLEVEL_UNIT, level);
+  int gx, gy;
 
   if (minlevel > fc_log_level) {
     return;
   }
 
+  if (is_goto_dest_set(punit)) {
+    gx = goto_dest_x(punit);
+    gy = goto_dest_y(punit);
+  } else {
+    gx = gy = -1;
+  }
+  
   my_snprintf(buffer, sizeof(buffer), "%s's %s[%d] (%d,%d)->(%d,%d){%d} ",
               unit_owner(punit)->name, unit_type(punit)->name,
               punit->id, punit->x, punit->y,
-              punit->goto_dest_x, punit->goto_dest_y, 
+             gx, gy,
               punit->ai.bodyguard);
 
   va_start(ap, msg);
@@ -100,12 +108,20 @@
     char buffer[500];
     char buffer2[500];
     va_list ap;
+    int gx, gy;
+
+    if (is_goto_dest_set(punit)) {
+      gx = goto_dest_x(punit);
+      gy = goto_dest_y(punit);
+    } else {
+      gx = gy = -1;
+    }
 
     my_snprintf(buffer, sizeof(buffer),
                 "%s's %s[%d] on GOTO (%d,%d)->(%d,%d) %s : ",
                 unit_owner(punit)->name, unit_type(punit)->name,
                 punit->id, punit->x, punit->y,
-                punit->goto_dest_x, punit->goto_dest_y,
+               gx, gy,
                (result == GR_FAILED) ? "failed" : "fought");
 
     va_start(ap, msg);
Index: ai/aitools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aitools.c,v
retrieving revision 1.80
diff -u -r1.80 aitools.c
--- ai/aitools.c        2003/02/17 22:49:27     1.80
+++ ai/aitools.c        2003/02/26 08:15:14
@@ -87,8 +87,7 @@
   CHECK_MAP_POS(pcity->x, pcity->y);
   punit->x = pcity->x;
   punit->y = pcity->y;
-  punit->goto_dest_x = 0;
-  punit->goto_dest_y = 0;
+  clear_goto_dest(punit);
   punit->veteran = make_veteran;
   punit->homecity = pcity->id;
   punit->upkeep = 0;
@@ -144,8 +143,8 @@
 bool ai_unit_gothere(struct unit *punit)
 {
   CHECK_UNIT(punit);
-  assert(punit->goto_dest_x != -1 && punit->goto_dest_y != -1);
-  if (ai_unit_goto(punit, punit->goto_dest_x, punit->goto_dest_y)) {
+  assert(is_goto_dest_set(punit));
+  if (ai_unit_goto(punit, goto_dest_x(punit), goto_dest_y(punit))) {
     return TRUE; /* ... and survived */
   } else {
     return FALSE; /* we died */
@@ -161,19 +160,27 @@
 bool ai_unit_goto(struct unit *punit, int x, int y)
 {
   enum goto_result result;
-  int oldx = punit->goto_dest_x, oldy = punit->goto_dest_y;
+  int oldx = -1, oldy = -1;
   enum unit_activity activity = punit->activity;
+  bool is_set = is_goto_dest_set(punit);
 
+  if (is_set) {
+    oldx = goto_dest_x(punit);
+    oldy = goto_dest_y(punit);
+  }
+
   CHECK_UNIT(punit);
   /* TODO: log error on same_pos with punit->x|y */
-  punit->goto_dest_x = x;
-  punit->goto_dest_y = y;
+  set_goto_dest(punit, x, y);
   handle_unit_activity_request(punit, ACTIVITY_GOTO);
   result = do_unit_goto(punit, GOTO_MOVE_ANY, FALSE);
   if (result != GR_DIED) {
     handle_unit_activity_request(punit, activity);
-    punit->goto_dest_x = oldx;
-    punit->goto_dest_y = oldy;
+    if (is_set) {
+      set_goto_dest(punit, oldx, oldy);
+    } else {
+      clear_goto_dest(punit);
+    }
     return TRUE;
   }
   return FALSE;
@@ -195,8 +202,8 @@
   }
 
   if (punit->ai.ai_role == AIUNIT_BUILD_CITY) {
-    assert(is_normal_map_pos(punit->goto_dest_x, punit->goto_dest_y));
-    remove_city_from_minimap(punit->goto_dest_x, punit->goto_dest_y);
+    assert(is_goto_dest_set(punit));
+    remove_city_from_minimap(goto_dest_x(punit), goto_dest_y(punit));
   }
 
   if (charge && (charge->ai.bodyguard == punit->id)) {
@@ -206,9 +213,9 @@
   punit->ai.charge = BODYGUARD_NONE;
 
   punit->ai.ai_role = task;
-/* TODO:
-  punit->goto_dest_x = x;
-  punit->goto_dest_y = y; */
+#if 0 /* TODO */
+  set_goto_dest(punit, x, y);
+#endif
 
   if (punit->ai.ai_role == AIUNIT_NONE && bodyguard) {
     ai_unit_new_role(bodyguard, AIUNIT_NONE, -1, -1);
Index: ai/aiunit.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aiunit.c,v
retrieving revision 1.265
diff -u -r1.265 aiunit.c
--- ai/aiunit.c 2003/02/23 12:47:35     1.265
+++ ai/aiunit.c 2003/02/26 08:15:19
@@ -1373,10 +1373,10 @@
         handle_unit_activity_request(punit, ACTIVITY_SENTRY);
         ferryboat->ai.passenger = punit->id;
 /* the code that worked for settlers wasn't good for piles of cannons */
-        if (find_beachhead(punit, dest_x, dest_y, &ferryboat->goto_dest_x,
-               &ferryboat->goto_dest_y) != 0) {
-          punit->goto_dest_x = dest_x;
-          punit->goto_dest_y = dest_y;
+        if (is_goto_dest_set(punit)
+           && find_beachhead(punit, dest_x, dest_y, &goto_dest_x(ferryboat),
+                             &goto_dest_y(ferryboat)) != 0) {
+         set_goto_dest(punit, dest_x, dest_y);
           handle_unit_activity_request(punit, ACTIVITY_SENTRY);
          if (ground_unit_transporter_capacity(punit->x, punit->y, pplayer)
              <= 0) {
@@ -1411,8 +1411,7 @@
 bodyguard, or 3) we are going to an empty city.  Previously, cannons
 would disembark before the cruisers arrived and die. -- Syela */
 
-      punit->goto_dest_x = dest_x;
-      punit->goto_dest_y = dest_y;
+      set_goto_dest(punit, dest_x, dest_y);
 
 /* The following code block is supposed to stop units from running away from 
their
 bodyguards, and not interfere with those that don't have bodyguards nearby -- 
Syela */
@@ -1809,8 +1808,9 @@
   CHECK_UNIT(punit);
 
   if (dest) {
-    x = punit->goto_dest_x;
-    y = punit->goto_dest_y;
+    assert(is_goto_dest_set(punit));
+    x = goto_dest_x(punit);
+    y = goto_dest_y(punit);
   } else {
     x = punit->x;
     y = punit->y;
@@ -1915,8 +1915,9 @@
     /* dealing with invasion stuff */
     if (IS_ATTACKER(aunit)) {
       if (aunit->activity == ACTIVITY_GOTO) {
+       assert(is_goto_dest_set(aunit));
         invasion_funct(aunit, TRUE, 0, (COULD_OCCUPY(aunit) ? 1 : 2));
-        if ((pcity = map_get_city(aunit->goto_dest_x, aunit->goto_dest_y))) {
+        if ((pcity = map_get_city(goto_dest_x(aunit), goto_dest_y(aunit)))) {
           pcity->ai.attack += unit_belligerence_basic(aunit);
           pcity->ai.bcost += unit_type(aunit)->build_cost;
         } 
@@ -2472,7 +2473,11 @@
       }
       p++;
       bodyguard = unit_list_find(&map_get_tile(punit->x, punit->y)->units, 
aunit->ai.bodyguard);
-      pcity = map_get_city(aunit->goto_dest_x, aunit->goto_dest_y);
+      if (is_goto_dest_set(aunit)) {
+       pcity = map_get_city(goto_dest_x(aunit), goto_dest_y(aunit));
+      } else {
+       pcity = NULL;
+      }
       if (aunit->ai.bodyguard == BODYGUARD_NONE || bodyguard ||
          (pcity && pcity->ai.invasion >= 2)) {
        if (pcity) {
@@ -2501,15 +2506,13 @@
   if (p != 0) {
     freelog(LOG_DEBUG, "%s#%d@(%d,%d), p=%d, n=%d",
                  unit_name(punit->type), punit->id, punit->x, punit->y, p, n);
-    if (punit->moves_left > 0 && n != 0) {
+    if (is_goto_dest_set(punit) && punit->moves_left > 0 && n != 0) {
       (void) ai_unit_gothere(punit);
     } else if (n == 0 && !map_get_city(punit->x, punit->y)) { /* rest in a 
city, for unhap */
-      x = punit->goto_dest_x; y = punit->goto_dest_y;
       port = find_nearest_safe_city(punit);
       if (port && !ai_unit_goto(punit, port->x, port->y)) {
         return; /* oops! */
       }
-      punit->goto_dest_x = x; punit->goto_dest_y = y;
       send_unit_info(pplayer, punit); /* to get the crosshairs right -- Syela 
*/
     } else {
       UNIT_LOG(LOG_DEBUG, punit, "Ferryboat %d@(%d,%d) stalling.",
@@ -2530,8 +2533,7 @@
   punit->ai.passenger = 0;
   UNIT_LOG(LOG_DEBUG, punit, "Ferryboat is lonely.");
   handle_unit_activity_request(punit, ACTIVITY_IDLE);
-  punit->goto_dest_x = 0; /* FIXME: -1 */
-  punit->goto_dest_y = 0; /* FIXME: -1 */
+  clear_goto_dest(punit);
 
   /* Release bodyguard and let it roam */
   ai_unit_new_role(punit, AIUNIT_NONE, -1, -1);
@@ -2569,8 +2571,7 @@
   } unit_list_iterate_end;
   if (best < 4 * unit_type(punit)->move_rate) {
     /* Pickup is within 4 turns to grab, so move it! */
-    punit->goto_dest_x = x;
-    punit->goto_dest_y = y;
+    set_goto_dest(punit, x, y);
     UNIT_LOG(LOG_DEBUG, punit, "Found a friend and going to him @(%d, %d)",
              x, y);
     (void) ai_unit_gothere(punit);
@@ -2585,8 +2586,7 @@
   if (pcity) {
     if (!ai_handicap(pplayer, H_TARGETS) ||
         unit_move_turns(punit, pcity->x, pcity->y) < p) {
-      punit->goto_dest_x = pcity->x;
-      punit->goto_dest_y = pcity->y;
+      set_goto_dest(punit, pcity->x, pcity->y);
       UNIT_LOG(LOG_DEBUG, punit, "No friends.  Going home.");
       (void) ai_unit_gothere(punit);
       return;
Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.293
diff -u -r1.293 packhand.c
--- client/packhand.c   2003/02/20 23:00:55     1.293
+++ client/packhand.c   2003/02/26 08:15:21
@@ -97,8 +97,11 @@
   punit->upkeep_gold = packet->upkeep_gold;
   punit->ai.control = packet->ai;
   punit->fuel = packet->fuel;
-  punit->goto_dest_x = packet->goto_dest_x;
-  punit->goto_dest_y = packet->goto_dest_y;
+  if (packet->goto_dest_x == -1 && packet->goto_dest_y == -1) {
+    clear_goto_dest(punit);
+  } else {
+    set_goto_dest(punit, packet->goto_dest_x, packet->goto_dest_y);
+  }
   punit->activity_target = packet->activity_target;
   punit->paradropped = packet->paradropped;
   punit->connecting = packet->connecting;
@@ -1006,7 +1009,7 @@
           && (!game.player_ptr->ai.control || ai_popup_windows)
           && punit->owner==game.player_idx
           && (punit->activity!=ACTIVITY_GOTO ||
-              same_pos(punit->goto_dest_x, punit->goto_dest_y,
+              same_pos(goto_dest_x(punit), goto_dest_y(punit),
                        pcity->x, pcity->y))
           && (unit_can_help_build_wonder_here(punit)
               || unit_can_est_traderoute_here(punit))) {
@@ -1042,8 +1045,11 @@
     punit->moves_left=packet->movesleft;
     punit->bribe_cost=0;
     punit->fuel=packet->fuel;
-    punit->goto_dest_x=packet->goto_dest_x;
-    punit->goto_dest_y=packet->goto_dest_y;
+    if (packet->goto_dest_x == -1 && packet->goto_dest_y == -1) {
+      clear_goto_dest(punit);
+    } else {
+      set_goto_dest(punit, packet->goto_dest_x, packet->goto_dest_y);
+    }
     punit->paradropped=packet->paradropped;
     punit->connecting=packet->connecting;
   
Index: client/gui-gtk/mapctrl.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/mapctrl.c,v
retrieving revision 1.76
diff -u -r1.76 mapctrl.c
--- client/gui-gtk/mapctrl.c    2003/02/17 22:49:27     1.76
+++ client/gui-gtk/mapctrl.c    2003/02/26 08:15:22
@@ -158,8 +158,9 @@
                    ptype->hp, punit->veteran ? _(" V") : "", uc);
 
         if(punit->activity == ACTIVITY_GOTO || punit->connecting)  {
-         cross_head->x = punit->goto_dest_x;
-         cross_head->y = punit->goto_dest_y;
+         assert(is_goto_dest_set(punit));
+         cross_head->x = goto_dest_x(punit);
+         cross_head->y = goto_dest_y(punit);
          cross_head++;
         }
       } else {
Index: client/gui-gtk-2.0/mapctrl.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/mapctrl.c,v
retrieving revision 1.17
diff -u -r1.17 mapctrl.c
--- client/gui-gtk-2.0/mapctrl.c        2003/02/17 22:49:27     1.17
+++ client/gui-gtk-2.0/mapctrl.c        2003/02/26 08:15:22
@@ -167,8 +167,9 @@
                    ptype->hp, punit->veteran ? _(" V") : "", uc);
 
         if(punit->activity == ACTIVITY_GOTO || punit->connecting)  {
-         cross_head->x = punit->goto_dest_x;
-         cross_head->y = punit->goto_dest_y;
+         assert(is_goto_dest_set(punit));
+         cross_head->x = goto_dest_x(punit);
+         cross_head->y = goto_dest_y(punit);
          cross_head++;
         }
       } else {
Index: client/gui-win32/mapctrl.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-win32/mapctrl.c,v
retrieving revision 1.20
diff -u -r1.20 mapctrl.c
--- client/gui-win32/mapctrl.c  2003/02/17 22:49:27     1.20
+++ client/gui-win32/mapctrl.c  2003/02/26 08:15:23
@@ -193,8 +193,9 @@
                  ptype->hp, punit->veteran?_(" V"):"", uc);
       
       if(punit->activity==ACTIVITY_GOTO || punit->connecting)  {
-       cross_head->x = punit->goto_dest_x;
-       cross_head->y = punit->goto_dest_y;
+       assert(is_goto_dest_set(punit));
+       cross_head->x = goto_dest_x(punit);
+       cross_head->y = goto_dest_y(punit);
        cross_head++;
       }
     } else {
Index: client/gui-xaw/mapctrl.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-xaw/mapctrl.c,v
retrieving revision 1.64
diff -u -r1.64 mapctrl.c
--- client/gui-xaw/mapctrl.c    2003/02/17 22:49:27     1.64
+++ client/gui-xaw/mapctrl.c    2003/02/26 08:15:23
@@ -167,8 +167,9 @@
                ptype->hp, punit->veteran?_(" V"):"", uc);
 
         if(punit->activity==ACTIVITY_GOTO)  {
-         cross_head->x = punit->goto_dest_x;
-         cross_head->y = punit->goto_dest_y;
+         assert(is_goto_dest_set(punit));
+         cross_head->x = goto_dest_x(punit);
+         cross_head->y = goto_dest_y(punit);
          cross_head++;
         }
       } else {
Index: common/capstr.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/capstr.c,v
retrieving revision 1.126
diff -u -r1.126 capstr.c
--- common/capstr.c     2003/02/17 22:49:27     1.126
+++ common/capstr.c     2003/02/26 08:15:23
@@ -76,7 +76,7 @@
 
 #define CAPABILITY "+1.14.0 conn_info +occupied team tech_impr_gfx " \
                    "city_struct_minor_cleanup obsolete_last class_legend " \
-                   "+impr_req +waste +fastfocus +continent"
+                   "+impr_req +waste +fastfocus +continent +goto_dest"
   
 /* "+1.14.0" is protocol for 1.14.0 release.
  *
@@ -109,6 +109,9 @@
  * "fastfocus" removes the server from client unit focus.
  *
  * +continent": the server gives the client continent information
+ *
+ * +goto_dest: support (-1,-1) for no goto destination for units, and use a
+ * signed 16-bit value to transmit this over the network.
  */
 
 void init_our_capability(void)
Index: common/packets.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/packets.c,v
retrieving revision 1.240
diff -u -r1.240 packets.c
--- common/packets.c    2003/02/17 22:49:27     1.240
+++ common/packets.c    2003/02/26 08:15:25
@@ -1268,8 +1268,8 @@
   dio_put_uint8(&dout, req->unhappiness);
   dio_put_uint8(&dout, req->activity);
   dio_put_uint8(&dout, req->activity_count);
-  dio_put_uint8(&dout, req->goto_dest_x);
-  dio_put_uint8(&dout, req->goto_dest_y);
+  dio_put_sint16(&dout, req->goto_dest_x);
+  dio_put_sint16(&dout, req->goto_dest_y);
   dio_put_uint16(&dout, req->activity_target);
   dio_put_uint8(&dout, req->packet_use);
   dio_put_uint16(&dout, req->info_city_id);
@@ -1546,8 +1546,8 @@
   dio_get_uint8(&din, &packet->unhappiness);
   dio_get_uint8(&din, &packet->activity);
   dio_get_uint8(&din, &packet->activity_count);
-  dio_get_uint8(&din, &packet->goto_dest_x);
-  dio_get_uint8(&din, &packet->goto_dest_y);
+  dio_get_sint16(&din, &packet->goto_dest_x);
+  dio_get_sint16(&din, &packet->goto_dest_y);
   dio_get_uint16(&din, (int *) &packet->activity_target);
   dio_get_uint8(&din, &packet->packet_use);
   dio_get_uint16(&din, &packet->info_city_id);
Index: common/unit.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/unit.h,v
retrieving revision 1.95
diff -u -r1.95 unit.h
--- common/unit.h       2003/02/17 22:49:28     1.95
+++ common/unit.h       2003/02/26 08:15:26
@@ -121,7 +121,9 @@
   int bribe_cost;
   struct unit_ai ai;
   enum unit_activity activity;
-  int goto_dest_x, goto_dest_y;
+  struct {
+    int x, y;
+  } goto_dest;
   int activity_count;
   enum tile_special_type activity_target;
   enum unit_focus_status focus_status;
@@ -135,6 +137,17 @@
   struct goto_route *pgr;
 };
 
+/* Wrappers for accessing the goto destination of a unit.  This goto_dest
+ * is used by client goto as well as by the AI. */
+#define is_goto_dest_set(punit) ((punit)->goto_dest.x != -1    \
+                                 || (punit)->goto_dest.y != -1)
+#define goto_dest_x(punit) ((punit)->goto_dest.x)
+#define goto_dest_y(punit) ((punit)->goto_dest.y)
+#define set_goto_dest(punit, map_x, map_y) (CHECK_MAP_POS(map_x, map_y),  \
+                                            (punit)->goto_dest.x = map_x, \
+                                            (punit)->goto_dest.y = map_y)
+#define clear_goto_dest(punit) ((punit)->goto_dest.x = -1, \
+                                (punit)->goto_dest.y = -1)
 
 /* get 'struct unit_list' and related functions: */
 #define SPECLIST_TAG unit
Index: server/autoattack.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/autoattack.c,v
retrieving revision 1.42
diff -u -r1.42 autoattack.c
--- server/autoattack.c 2003/02/17 22:49:28     1.42
+++ server/autoattack.c 2003/02/26 08:15:28
@@ -166,8 +166,7 @@
                   unit_owner(enemy)->name, unit_name(enemy->type));
   
   set_unit_activity(punit, ACTIVITY_GOTO);
-  punit->goto_dest_x=enemy->x;
-  punit->goto_dest_y=enemy->y;
+  set_goto_dest(punit, enemy->x, enemy->y);
   
   send_unit_info(NULL, punit);
   (void) do_unit_goto(punit, GOTO_MOVE_ANY, FALSE);
@@ -176,8 +175,7 @@
   
   if (punit) {
     set_unit_activity(punit, ACTIVITY_GOTO);
-    punit->goto_dest_x=pcity->x;
-    punit->goto_dest_y=pcity->y;
+    set_goto_dest(punit, pcity->x, pcity->y);
     send_unit_info(NULL, punit);
     
     (void) do_unit_goto(punit, GOTO_MOVE_ANY, FALSE);
Index: server/gotohand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/gotohand.c,v
retrieving revision 1.168
diff -u -r1.168 gotohand.c
--- server/gotohand.c   2003/02/20 09:45:21     1.168
+++ server/gotohand.c   2003/02/26 08:15:30
@@ -745,8 +745,7 @@
          total_cost += unit_type(punit)->move_rate;
          freelog(LOG_DEBUG, "%s#%d@(%d,%d) dissuaded from (%d,%d) -> (%d,%d)",
                  unit_type(punit)->name, punit->id,
-                 punit->x, punit->y, x1, y1,
-                 punit->goto_dest_x, punit->goto_dest_y);
+                 punit->x, punit->y, x1, y1, dest_x, dest_y);
        }
        break;
 
@@ -1278,8 +1277,9 @@
   }
 
   unit_id = punit->id;
-  dest_x = waypoint_x = punit->goto_dest_x;
-  dest_y = waypoint_y = punit->goto_dest_y;
+  assert(is_goto_dest_set(punit));
+  dest_x = waypoint_x = goto_dest_x(punit);
+  dest_y = waypoint_y = goto_dest_y(punit);
 
   if (same_pos(punit->x, punit->y, dest_x, dest_y) ||
       !goto_is_sane(punit, dest_x, dest_y, FALSE)) {
@@ -1342,7 +1342,7 @@
       penemy = is_enemy_unit_tile(map_get_tile(x, y), unit_owner(punit));
       assert(punit->moves_left > 0);
 
-      last_tile = same_pos(x, y, punit->goto_dest_x, punit->goto_dest_y);
+      last_tile = same_pos(x, y, goto_dest_x(punit), goto_dest_y(punit));
 
       /* Call handle_unit_move_request for humans and ai_unit_move for AI */
       success = (!pplayer->ai.control 
Index: server/savegame.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/savegame.c,v
retrieving revision 1.114
diff -u -r1.114 savegame.c
--- server/savegame.c   2003/02/17 22:49:28     1.114
+++ server/savegame.c   2003/02/26 08:15:32
@@ -980,11 +980,17 @@
     punit->connecting=secfile_lookup_bool_default(file, FALSE,
                                                "player%d.u%d.connecting",
                                                plrno, i);
+    /* Load the goto information.  Older savegames will not have the
+     * "go" field, so we just load the goto destination by default. */
+    if (secfile_lookup_bool_default(file, TRUE,
+                                   "player%d.u%d.go", plrno, i)) {
+      int x = secfile_lookup_int(file, "player%d.u%d.goto_x", plrno, i);
+      int y = secfile_lookup_int(file, "player%d.u%d.goto_y", plrno, i);
+      set_goto_dest(punit, x, y);
+    } else {
+      clear_goto_dest(punit);
+    }
 
-    punit->goto_dest_x=secfile_lookup_int(file, 
-                                         "player%d.u%d.goto_x", plrno,i);
-    punit->goto_dest_y=secfile_lookup_int(file, 
-                                         "player%d.u%d.goto_y", plrno,i);
     punit->ai.control=secfile_lookup_bool(file, "player%d.u%d.ai", plrno,i);
     punit->ai.ai_role = AIUNIT_NONE;
     punit->ai.ferryboat = 0;
@@ -1409,9 +1415,20 @@
                                plrno, i);
     secfile_insert_int(file, punit->fuel, "player%d.u%d.fuel",
                                plrno, i);
+
+    if (is_goto_dest_set(punit)) {
+      secfile_insert_bool(file, TRUE, "player%d.u%d.go", plrno, i);
+      secfile_insert_int(file, goto_dest_x(punit),
+                        "player%d.u%d.goto_x", plrno, i);
+      secfile_insert_int(file, goto_dest_y(punit), "player%d.u%d.goto_y",
+                        plrno, i);
+    } else {
+      secfile_insert_bool(file, FALSE, "player%d.u%d.go", plrno, i);
+      /* for compatility with older servers */
+      secfile_insert_int(file, 0, "player%d.u%d.goto_x", plrno, i);
+      secfile_insert_int(file, 0, "player%d.u%d.goto_y", plrno, i);
+    }
 
-    secfile_insert_int(file, punit->goto_dest_x, "player%d.u%d.goto_x", plrno, 
i);
-    secfile_insert_int(file, punit->goto_dest_y, "player%d.u%d.goto_y", plrno, 
i);
     secfile_insert_bool(file, punit->ai.control, "player%d.u%d.ai", plrno, i);
     secfile_insert_int(file, punit->ord_map, "player%d.u%d.ord_map", plrno, i);
     secfile_insert_int(file, punit->ord_city, "player%d.u%d.ord_city", plrno, 
i);
Index: server/settlers.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/settlers.c,v
retrieving revision 1.165
diff -u -r1.165 settlers.c
--- server/settlers.c   2003/02/17 22:49:28     1.165
+++ server/settlers.c   2003/02/26 08:15:34
@@ -459,7 +459,8 @@
     int x, int y)
 {
   if (same_pos(myunit->x, myunit->y, x, y) ||
-      same_pos(myunit->goto_dest_x, myunit->goto_dest_y, x, y)) {
+      (is_goto_dest_set(myunit)
+       && same_pos(goto_dest_x(myunit), goto_dest_y(myunit), x, y))) {
 /* I'm still not sure this is exactly right -- Syela */
     unit_list_iterate(map_get_tile(x, y)->units, punit)
       if (myunit==punit) continue;
@@ -1222,8 +1223,7 @@
     }
     ferryboat = unit_list_find(&(map_get_tile(punit->x, punit->y)->units),
                                punit->ai.ferryboat);
-    punit->goto_dest_x = gx;
-    punit->goto_dest_y = gy;
+    set_goto_dest(punit, gx, gy);
 
     if (ferryboat && (ferryboat->ai.passenger == 0
                       || ferryboat->ai.passenger == punit->id)) {
@@ -1231,8 +1231,7 @@
                ferryboat->id);
       handle_unit_activity_request(punit, ACTIVITY_SENTRY);
       ferryboat->ai.passenger = punit->id;
-      ferryboat->goto_dest_x = gx;
-      ferryboat->goto_dest_y = gy;
+      set_goto_dest(ferryboat, gx, gy);
       if (!ai_unit_goto(ferryboat, gx, gy)) {
         return FALSE; /* died */
       }
@@ -1249,8 +1248,7 @@
       && (!ferryboat
           || (is_tiles_adjacent(punit->x, punit->y, gx, gy)
               && could_unit_move_to_tile(punit, gx, gy) != 0))) {
-    punit->goto_dest_x = gx;
-    punit->goto_dest_y = gy;
+    set_goto_dest(punit, gx, gy);
     if (!ai_unit_goto(punit, gx, gy)) {
       return FALSE; /* died */
     }
@@ -1426,7 +1424,8 @@
     if (unit_flag(punit, F_SETTLERS)
        || unit_flag(punit, F_CITIES)) {
       if (punit->activity == ACTIVITY_GOTO) {
-        ptile = map_get_tile(punit->goto_dest_x, punit->goto_dest_y);
+       assert(is_goto_dest_set(punit));
+        ptile = map_get_tile(goto_dest_x(punit), goto_dest_y(punit));
         ptile->assigned = ptile->assigned | i; /* assigned for us only */
       } else {
         ptile = map_get_tile(punit->x, punit->y);
Index: server/unithand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unithand.c,v
retrieving revision 1.255
diff -u -r1.255 unithand.c
--- server/unithand.c   2003/02/17 22:49:28     1.255
+++ server/unithand.c   2003/02/26 08:15:35
@@ -76,8 +76,7 @@
     return;
   }
 
-  punit->goto_dest_x = req->x;
-  punit->goto_dest_y = req->y;
+  set_goto_dest(punit, req->x, req->y);
 
   set_unit_activity(punit, ACTIVITY_GOTO);
 
@@ -133,8 +132,7 @@
     return;
   }
 
-  punit->goto_dest_x = req->dest_x;
-  punit->goto_dest_y = req->dest_y;
+  set_goto_dest(punit, req->dest_x, req->dest_y);
 
   set_unit_activity(punit, req->activity_type);
   punit->connecting = TRUE;
@@ -1034,7 +1032,7 @@
      * human players the server-side goto implementation should be
      * obsoleted for client usage. So in time, remove the code below. */
     if (punit->activity == ACTIVITY_GOTO && 
-        !same_pos(punit->goto_dest_x, punit->goto_dest_y, dest_x, dest_y)) {
+        !same_pos(goto_dest_x(punit), goto_dest_y(punit), dest_x, dest_y)) {
       notify_player_ex(pplayer, punit->x, punit->y, E_NOEVENT,
                       _("Game: %s aborted GOTO as there are units in the 
way."),
                       unit_type(punit)->name);
@@ -1445,8 +1443,8 @@
 #endif
 
   if (punit->activity == ACTIVITY_GOTO) {
-    punit->goto_dest_x = pgr->pos[pgr->last_index-1].x;
-    punit->goto_dest_y = pgr->pos[pgr->last_index-1].y;
+    set_goto_dest(punit, pgr->pos[pgr->last_index-1].x,
+                 pgr->pos[pgr->last_index-1].y);
     send_unit_info(pplayer, punit);
   }
 
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.214
diff -u -r1.214 unittools.c
--- server/unittools.c  2003/02/18 18:10:15     1.214
+++ server/unittools.c  2003/02/26 08:15:39
@@ -520,8 +520,7 @@
              (air_can_move_between
               (punit->moves_left / 3, punit->x, punit->y, x_itr, y_itr,
                unit_owner(punit)) >= 0)) {
-           punit->goto_dest_x = x_itr;
-           punit->goto_dest_y = y_itr;
+           set_goto_dest(punit, x_itr, y_itr);
            set_unit_activity(punit, ACTIVITY_GOTO);
            (void) do_unit_goto(punit, GOTO_MOVE_ANY, FALSE);
            notify_player_ex(pplayer, punit->x, punit->y, E_NOEVENT, 
@@ -1569,8 +1568,7 @@
   punit->x = x;
   punit->y = y;
 
-  punit->goto_dest_x=0;
-  punit->goto_dest_y=0;
+  clear_goto_dest(punit);
   
   pcity=find_city_by_id(homecity_id);
   punit->veteran=make_veteran;
@@ -1924,8 +1922,12 @@
   packet->upkeep_gold = punit->upkeep_gold;
   packet->ai = punit->ai.control;
   packet->fuel = punit->fuel;
-  packet->goto_dest_x = punit->goto_dest_x;
-  packet->goto_dest_y = punit->goto_dest_y;
+  if (is_goto_dest_set(punit)) {
+    packet->goto_dest_x = goto_dest_x(punit);
+    packet->goto_dest_y = goto_dest_y(punit);
+  } else {
+    packet->goto_dest_x = packet->goto_dest_y = -1;
+  }
   packet->activity_target = punit->activity_target;
   packet->paradropped = punit->paradropped;
   packet->connecting = punit->connecting;
@@ -2934,7 +2936,7 @@
   }
   /* A transporter should not take units with it when on an attack goto -- 
fisch */
   if ((punit->activity == ACTIVITY_GOTO) &&
-      get_defender(punit, punit->goto_dest_x, punit->goto_dest_y) &&
+      get_defender(punit, goto_dest_x(punit), goto_dest_y(punit)) &&
       !is_ocean(psrctile->terrain)) {
     transport_units = FALSE;
   }
@@ -3005,14 +3007,15 @@
   check_unit_activity(punit);
 
   /* set activity to sentry if boarding a ship unless the unit is just 
-     passing through the ship on its way somewhere else */
+   * passing through the ship on its way somewhere else.  If the unit is
+   * GOTOing and the ship isn't the final destination, then don't go
+   * to sleep. */
   if (is_ground_unit(punit)
       && is_ocean(pdesttile->terrain)
       && !(pplayer->ai.control)
-      && !(punit->activity == ACTIVITY_GOTO      /* if unit is GOTOing and the 
ship */ 
-          && (dest_x != punit->goto_dest_x      /* isn't the final destination 
*/
-              || dest_y != punit->goto_dest_y)) /* then don't go to sleep */
-      ) {
+      && !(punit->activity == ACTIVITY_GOTO
+          && !same_pos(goto_dest_x(punit), goto_dest_y(punit),
+                       dest_x, dest_y))) {
     set_unit_activity(punit, ACTIVITY_SENTRY);
   }
   send_unit_info_to_onlookers(NULL, punit, src_x, src_y, FALSE);

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