[Freeciv-Dev] (PR#8672) punit->occupy has no value at the server

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

> [jdorje - Tue May 04 17:48:11 2004]:
> 
> Originally punit->client.occupy was created so the client could track if 
> units were occupied.  It was a boolean.
> 
> It was changed to punit->occupy and turned into an integer.  I don't 
> know why.  The server still doesn't use this value.
> 
> However the AI code has started using the value.  This won't work since 
> it's always 0 at the server.
> 
> The quick fix is that the AI should call get_transporter_occupancy() 
> instead.  However this function is slow.  Better would be to update the 
> punit->occupy value at the server.  It should be renamed 
> (punit->occupancy?) and updated in 
> load_unit_onto_transporter/unload_unit_from_transporter.

The fix for this isn't entirely trivial.

Using punit->client.occupy at the client doesn't work well.  Because at
the server get_unit_occupancy() must still count the units in the
transporter.  But at the client you _can't_ count the units in the
transporter, you can only check the occupy value (at least for some
units).  So the only solution is to do an is_server check, which is just
ugly.

The alternative is to make punit->occupy work at the server.  The
biggest problem here is when loading savegames this value must be
assembled, and it's not easy.  (It would be pointless to save this
value, since you'd still have to assemble it for old savegames.)

Therefore this patch does the latter.  For good measure I rename
punit->occupy (a sucky name since this was originally a boolean value)
as punit->occupancy and remove get_transporter_occupancy.  Perhaps this
is a little overboard but it does make things clearer.

jason

? convert.sh
? flags
? data/flags
Index: ai/aiunit.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aiunit.c,v
retrieving revision 1.316
diff -u -r1.316 aiunit.c
--- ai/aiunit.c 23 May 2004 02:05:37 -0000      1.316
+++ ai/aiunit.c 23 May 2004 22:39:50 -0000
@@ -2427,7 +2427,7 @@
   }
 
   /* Check if we are an empty barbarian boat and so not needed */
-  if (is_barbarian(pplayer) && !punit->occupy) {
+  if (is_barbarian(pplayer) && punit->occupancy == 0) {
     wipe_unit(punit);
     return;
   }
@@ -2511,7 +2511,7 @@
       /* Cannot select a passenger-in-charge */
       break;
     }
-  } while (punit->occupy != 0);
+  } while (punit->occupancy != 0);
 
   /* Not carrying anyone, even the ferryman */
 
Index: client/control.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/control.c,v
retrieving revision 1.135
diff -u -r1.135 control.c
--- client/control.c    17 May 2004 01:29:46 -0000      1.135
+++ client/control.c    23 May 2004 22:39:53 -0000
@@ -1414,7 +1414,7 @@
     maybe_goto = keyboardless_goto;
   }
   else if (unit_list_size(&ptile->units) == 1
-      && !unit_list_get(&ptile->units, 0)->occupy) {
+          && unit_list_get(&ptile->units, 0)->occupancy == 0) {
     struct unit *punit=unit_list_get(&ptile->units, 0);
     if(game.player_idx==punit->owner) {
       if(can_unit_do_activity(punit, ACTIVITY_IDLE)) {
Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.120
diff -u -r1.120 mapview_common.c
--- client/mapview_common.c     22 May 2004 18:12:21 -0000      1.120
+++ client/mapview_common.c     23 May 2004 22:39:54 -0000
@@ -665,7 +665,7 @@
     }
   }
 
-  if (punit->occupy != 0) {
+  if (punit->occupancy != 0) {
     canvas_put_sprite(pcanvas, canvas_x, canvas_y,
                      sprites.unit.stack,
                      unit_offset_x, unit_offset_y, unit_width, unit_height);
Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.370
diff -u -r1.370 packhand.c
--- client/packhand.c   23 May 2004 06:10:56 -0000      1.370
+++ client/packhand.c   23 May 2004 22:39:54 -0000
@@ -111,7 +111,7 @@
   punit->paradropped = packet->paradropped;
   punit->connecting = packet->connecting;
   punit->done_moving = packet->done_moving;
-  punit->occupy = packet->occupy;
+  punit->occupancy = packet->occupancy;
   if (packet->transported) {
     punit->transported_by = packet->transported_by;
   } else {
@@ -152,7 +152,7 @@
   punit->veteran = packet->veteran;
   punit->hp = packet->hp;
   punit->activity = packet->activity;
-  punit->occupy = (packet->occupied ? 1 : 0);
+  punit->occupancy = (packet->occupied ? 1 : 0);
   if (packet->transported) {
     punit->transported_by = packet->transported_by;
   } else {
@@ -962,7 +962,7 @@
     ret = TRUE;
     punit->activity_count = packet_unit->activity_count;
     punit->transported_by = packet_unit->transported_by;
-    punit->occupy = packet_unit->occupy;
+    punit->occupancy = packet_unit->occupancy;
     if (punit->ai.control != packet_unit->ai.control) {
       punit->ai.control = packet_unit->ai.control;
       repaint_unit = TRUE;
Index: client/tilespec.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/tilespec.c,v
retrieving revision 1.170
diff -u -r1.170 tilespec.c
--- client/tilespec.c   22 May 2004 18:12:21 -0000      1.170
+++ client/tilespec.c   23 May 2004 22:39:55 -0000
@@ -1607,7 +1607,7 @@
     }
   }
 
-  if (stack || punit->occupy) {
+  if (stack || punit->occupancy != 0) {
     ADD_SPRITE_FULL(sprites.unit.stack);
   } else {
     ADD_SPRITE_FULL(sprites.unit.vet_lev[punit->veteran]);
Index: client/gui-gtk-2.0/dialogs.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/dialogs.c,v
retrieving revision 1.67
diff -u -r1.67 dialogs.c
--- client/gui-gtk-2.0/dialogs.c        17 May 2004 01:29:47 -0000      1.67
+++ client/gui-gtk-2.0/dialogs.c        23 May 2004 22:39:56 -0000
@@ -1384,7 +1384,7 @@
 
     if (pleaf->transported_by == root_id) {
       unit_select_append(pleaf, &it_leaf, it_root);
-      if (pleaf->occupy > 0) {
+      if (pleaf->occupancy > 0) {
        unit_select_recurse(pleaf->id, &it_leaf);
       }
     }
Index: common/packets.def
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/packets.def,v
retrieving revision 1.23
diff -u -r1.23 packets.def
--- common/packets.def  21 May 2004 19:03:43 -0000      1.23
+++ common/packets.def  23 May 2004 22:39:57 -0000
@@ -600,7 +600,7 @@
   UNIT_TYPE type;
   UNIT transported_by; /* Only valid if transported is set. */
   UINT8 movesleft, hp, fuel, activity_count;
-  UINT8 unhappiness, upkeep, upkeep_food, upkeep_gold, occupy;
+  UINT8 unhappiness, upkeep, upkeep_food, upkeep_gold, occupancy;
   COORD goto_dest_x,goto_dest_y;
   ACTIVITY activity;
   SPECIAL activity_target;
Index: common/unit.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/unit.c,v
retrieving revision 1.210
diff -u -r1.210 unit.c
--- common/unit.c       19 May 2004 00:49:31 -0000      1.210
+++ common/unit.c       23 May 2004 22:39:57 -0000
@@ -654,8 +654,7 @@
   }
 
   /* Make sure there's room in the transporter. */
-  return (get_transporter_occupancy(ptrans)
-         < get_transporter_capacity(ptrans));
+  return (ptrans->occupancy < get_transporter_capacity(ptrans));
 }
 
 /****************************************************************************
@@ -1723,7 +1722,7 @@
   punit->ord_map = 0;
   punit->ord_city = 0;
   set_unit_activity(punit, ACTIVITY_IDLE);
-  punit->occupy = 0;
+  punit->occupancy = 0;
   punit->has_orders = FALSE;
 
   return punit;
@@ -1756,22 +1755,6 @@
 }
 
 /****************************************************************************
-  Expensive function to check how many units are in the transport.
-****************************************************************************/
-int get_transporter_occupancy(struct unit *ptrans)
-{
-  int occupied = 0;
-
-  unit_list_iterate(map_get_tile(ptrans->x, ptrans->y)->units, pcargo) {
-    if (pcargo->transported_by == ptrans->id) {
-      occupied++;
-    }
-  } unit_list_iterate_end;
-
-  return occupied;
-}
-
-/****************************************************************************
   Find a transporter at the given location for the unit.
 ****************************************************************************/
 struct unit *find_transporter_for_unit(struct unit *pcargo, int x, int y)
@@ -1823,8 +1806,7 @@
     }
   }
 
-  if (get_transporter_occupancy(punit) >
-      unit_types[to_unittype].transport_capacity) {
+  if (punit->occupancy > unit_types[to_unittype].transport_capacity) {
     /* TODO: allow transported units to be reassigned.  Check for
      * ground_unit_transporter_capacity here and make changes to
      * upgrade_unit. */
Index: common/unit.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/unit.h,v
retrieving revision 1.118
diff -u -r1.118 unit.h
--- common/unit.h       19 May 2004 00:49:31 -0000      1.118
+++ common/unit.h       23 May 2004 22:39:57 -0000
@@ -171,7 +171,7 @@
   bool done_moving;
 
   int transported_by;
-  int occupy; /* number of units that occupy transporter */
+  int occupancy; /* number of units that occupy transporter */
 
   bool has_orders;
   struct {
@@ -343,7 +343,6 @@
 void destroy_unit_virtual(struct unit *punit);
 void free_unit_orders(struct unit *punit);
 
-int get_transporter_occupancy(struct unit *ptrans);
 struct unit *find_transporter_for_unit(struct unit *pcargo, int x, int y);
 
 enum unit_upgrade_result test_unit_upgrade(struct unit *punit, bool is_free);
Index: server/sanitycheck.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/sanitycheck.c,v
retrieving revision 1.40
diff -u -r1.40 sanitycheck.c
--- server/sanitycheck.c        25 Jan 2004 08:04:53 -0000      1.40
+++ server/sanitycheck.c        23 May 2004 22:39:57 -0000
@@ -273,9 +273,9 @@
 static void check_units(void) {
   players_iterate(pplayer) {
     unit_list_iterate(pplayer->units, punit) {
-      int x = punit->x;
-      int y = punit->y;
-      struct city *pcity;
+      int x = punit->x, y = punit->y, occupancy = 0;
+      struct city *pcity = map_get_city(x, y);
+      struct tile *ptile = map_get_tile(x, y);
 
       assert(unit_owner(punit) == pplayer);
       CHECK_MAP_POS(x, y);
@@ -294,7 +294,6 @@
                get_activity_text(punit->activity));
       }
 
-      pcity = map_get_city(x, y);
       if (pcity) {
        assert(pplayers_allied(city_owner(pcity), pplayer));
       } else if (is_ocean(map_get_terrain(x, y))) {
@@ -312,9 +311,19 @@
        assert(!is_ground_unit(find_unit_by_id(punit->transported_by)));
       }
 
+      /* Check punit->occupancy against units actually in the transporter. */
+      unit_list_iterate(ptile->units, pcargo) {
+       if (pcargo->transported_by == punit->id) {
+         occupancy++;
+       }
+      } unit_list_iterate_end;
+      if (occupancy != punit->occupancy) {
+       assert(occupancy == punit->occupancy);
+       punit->occupancy = occupancy;
+      }
+
       /* Check for over-full transports. */
-      assert(get_transporter_occupancy(punit)
-            <= get_transporter_capacity(punit));
+      assert(punit->occupancy <= get_transporter_capacity(punit));
     } unit_list_iterate_end;
   } players_iterate_end;
 }
Index: server/savegame.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/savegame.c,v
retrieving revision 1.153
diff -u -r1.153 savegame.c
--- server/savegame.c   23 May 2004 18:40:42 -0000      1.153
+++ server/savegame.c   23 May 2004 22:39:58 -0000
@@ -2326,6 +2326,22 @@
        * (in player_map_load). */
       generic_city_refresh(pcity, FALSE, NULL);
     } cities_iterate_end;
+    whole_map_iterate(x, y) {
+      struct tile *ptile = map_get_tile(x, y);
+
+      unit_list_iterate(ptile->units, pcargo) {
+       if (pcargo->transported_by != -1) {
+         struct unit *ptrans = find_unit_by_id(pcargo->transported_by);
+
+         if (ptrans) {
+           ptrans->occupancy++;
+         } else {
+           freelog(LOG_ERROR, "Illegal transport at %d,%d.", x, y);
+           pcargo->transported_by = -1;
+         }
+       }
+      } unit_list_iterate_end;
+    } whole_map_iterate_end;
 
     /* Since the cities must be placed on the map to put them on the
        player map we do this afterwards */
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.293
diff -u -r1.293 unittools.c
--- server/unittools.c  19 May 2004 00:49:32 -0000      1.293
+++ server/unittools.c  23 May 2004 22:39:58 -0000
@@ -1701,15 +1701,12 @@
     /* Reassign existing units.  This is an O(n^2) operation as written. */
     unit_list_iterate(map_get_tile(x, y)->units, ptrans) {
       if (is_ground_units_transport(ptrans)) {
-       int occupancy = get_transporter_occupancy(ptrans);
-
        unit_list_iterate(map_get_tile(x, y)->units, pcargo) {
-         if (occupancy >= get_transporter_capacity(ptrans)) {
+         if (ptrans->occupancy >= get_transporter_capacity(ptrans)) {
            break;
          }
          if (is_ground_unit(pcargo) && pcargo->transported_by == -1) {
            put_unit_onto_transporter(pcargo, ptrans);
-           occupancy++;
          }
        } unit_list_iterate_end;
       }
@@ -1862,7 +1859,7 @@
     packet->transported = TRUE;
     packet->transported_by = punit->transported_by;
   }
-  packet->occupy = get_transporter_occupancy(punit);
+  packet->occupancy = punit->occupancy;
   packet->has_orders = punit->has_orders;
   if (punit->has_orders) {
     int i;
@@ -1912,7 +1909,7 @@
   packet->veteran = punit->veteran;
   packet->type = punit->type;
   packet->hp = punit->hp;
-  packet->occupied = (get_transporter_occupancy(punit) > 0);
+  packet->occupied = (punit->occupancy > 0);
   if (punit->activity == ACTIVITY_EXPLORE
       || punit->activity == ACTIVITY_GOTO) {
     packet->activity = ACTIVITY_IDLE;
@@ -2163,7 +2160,7 @@
     return FALSE;
   if (!unit_can_airlift_to(punit, city2))
     return FALSE;
-  if (get_transporter_occupancy(punit) > 0) {
+  if (punit->occupancy > 0) {
     return FALSE;
   }
   city1->airlift = FALSE;
@@ -2202,7 +2199,7 @@
     return FALSE;
   }
 
-  if (get_transporter_occupancy(punit) > 0) {
+  if (punit->occupancy > 0) {
     notify_player_ex(pplayer, punit->x, punit->y, E_NOEVENT,
                     _("Game: You cannot paradrop a transporter unit."));
   }
@@ -2456,7 +2453,10 @@
 {
   /* In the future we may updated ptrans->occupancy. */
   assert(punit->transported_by == -1);
+  assert(ptrans->occupancy < get_transporter_capacity(ptrans));
+
   punit->transported_by = ptrans->id;
+  ptrans->occupancy++;
 }
 
 /****************************************************************************
@@ -2467,7 +2467,10 @@
 {
   /* In the future we may updated ptrans->occupancy. */
   assert(punit->transported_by == ptrans->id);
+  assert(ptrans->occupancy > 0);
+
   punit->transported_by = -1;
+  ptrans->occupancy--;
 }
 
 /****************************************************************************