Complete.Org: Mailing Lists: Archives: freeciv-dev: March 2004:
[Freeciv-Dev] (PR#8393) transported_by at the client
Home

[Freeciv-Dev] (PR#8393) transported_by at the client

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: undisclosed-recipients: ;
Subject: [Freeciv-Dev] (PR#8393) transported_by at the client
From: "Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>
Date: Sat, 27 Mar 2004 21:15:52 -0800
Reply-to: rt@xxxxxxxxxxx

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

> [jdorje - Fri Mar 26 17:38:52 2004]:
> 
> Currently a unit that is transported is seen by some of the clients 
> only.  For these clients this unit has punit->transported_by treated as 
> a _boolean_ at the client side.  So the client knows the unit is 
> transported (and thus knows not to animate it in movement) but doesn't 
> know what unit is transporting it.  This was necessary in the past 
> because punit->transported_by didn't have a consistent meaning even at 
> the server.  But now it does.
> 
> There are several reasons why the client needs to know which unit is 
> carrying the cargo.  They all boil down to one thing: managing 
> transported units is now the responsibility of the player, not the 
> server.  In the unit popup dialog the player needs to easily see which 
> units are transporting which.  And to implement a load/unload command, 
> the client needs to be able to determine which load/unloadings are
possible.
> 
> So my proposal is that punit->transported_by should be sent to the 
> client.  This is a fairly simple change.  The new invariant would be 
> that for all units seen by the client punit->transported_by has the 
> correct value.

A simple patch.

The packet can't just contain transported_by as a UNIT, because UNITs
are unsigned.  Thus either a signed value is needed...or what I do is
add a second boolean value transported.

It's worth pointing out that transported_by is included in both short
and full unit packets.  This may seem counter-intuitive at first.  But
consider (from a realism POV) that for any unit that the player can see
he can surely see what unit is transporting it (remember that
transported units can only be seen by allies).  And (from a gameplay
POV) the player must know what transporters' his allies units are in
(they may be in _his_ transporters) for a load command to be possible.

Some ugliness is removed from the client.  Previously the "carried"
variable was passed around to the control code which helped to determine
whether drawing was necessary.  (This is from Rafal's earlier patch,
which cut out some of the drawing but at a cost in extra code.  At that
time transported_by had no consistent meaning even in the server so it
was infeasible to send it to the client.)  Now punit->transported_by can
be accessed directly to determine this.

The transported_by field should never have an inaccurate value at the
client, but it may have a useless value.  There are some situations
where the client may know a unit but not know about the transported_by
unit.  Diplomats investigating cities is one such case.  It _may_ also
be possible when the client connects to a server that he receives the
cargo before the transporter (this is probably bad...).

Aside from that everything is pretty straightforward.  Since the client
doesn't use this value yet (except to determine if the unit is
transported at all) there's not much potential for error.

jason

? Womoks
Index: client/control.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/control.c,v
retrieving revision 1.130
diff -u -r1.130 control.c
--- client/control.c    15 Mar 2004 06:05:16 -0000      1.130
+++ client/control.c    28 Mar 2004 04:37:55 -0000
@@ -344,14 +344,14 @@
      (always return first in stack). */
   unit_list_iterate(ptile->units, punit)
     if (unit_owner(punit) == game.player_ptr) {
-      if (!punit->transported_by) {
+      if (punit->transported_by == -1) {
         if (get_transporter_capacity(punit) > 0) {
          return punit;
         } else if (!panyowned) {
          panyowned = punit;
         }
       }
-    } else if (!ptptother && !punit->transported_by) {
+    } else if (!ptptother && punit->transported_by == -1) {
       if (get_transporter_capacity(punit) > 0) {
        ptptother = punit;
       } else if (!panyother) {
@@ -1191,7 +1191,7 @@
   Called to have the client move a unit from one location to another,
   updating the graphics if necessary.
 **************************************************************************/
-void do_move_unit(struct unit *punit, struct unit *target_unit, bool carried)
+void do_move_unit(struct unit *punit, struct unit *target_unit)
 {
   int x, y;
   bool was_teleported;
@@ -1201,14 +1201,16 @@
   x = punit->x;
   y = punit->y;
 
-  if (!was_teleported && punit->activity != ACTIVITY_SENTRY && !carried) {
+  if (!was_teleported
+      && punit->activity != ACTIVITY_SENTRY
+      && punit->transported_by == -1) {
     audio_play_sound(unit_type(punit)->sound_move,
                     unit_type(punit)->sound_move_alt);
   }
 
   unit_list_unlink(&map_get_tile(x, y)->units, punit);
 
-  if (!carried) {
+  if (punit->transported_by == -1) {
     refresh_tile_mapcanvas(x, y, FALSE);
   }
   
@@ -1222,7 +1224,7 @@
     center_tile_mapcanvas(target_unit->x, target_unit->y);
   }
 
-  if (!carried && !was_teleported) {
+  if (punit->transported_by == -1 && !was_teleported) {
     int dx, dy;
 
     map_distance_vector(&dx, &dy, punit->x, punit->y,
@@ -1252,7 +1254,8 @@
     }
   } square_iterate_end;
   
-  if (!carried && tile_get_known(punit->x, punit->y) == TILE_KNOWN) {
+  if (punit->transported_by == -1
+      && tile_get_known(punit->x, punit->y) == TILE_KNOWN) {
     refresh_tile_mapcanvas(punit->x, punit->y, FALSE);
   }
 
Index: client/control.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/control.h,v
retrieving revision 1.40
diff -u -r1.40 control.h
--- client/control.h    16 Dec 2003 15:07:52 -0000      1.40
+++ client/control.h    28 Mar 2004 04:37:55 -0000
@@ -34,7 +34,7 @@
 extern bool draw_goto_line;
 extern bool non_ai_unit_focus;
 
-void do_move_unit(struct unit *punit, struct unit *target_unit, bool carried);
+void do_move_unit(struct unit *punit, struct unit *target_unit);
 void do_unit_goto(int x, int y);
 void do_unit_nuke(struct unit *punit);
 void do_unit_paradrop_to(struct unit *punit, int x, int y);
Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.355
diff -u -r1.355 packhand.c
--- client/packhand.c   22 Mar 2004 20:58:11 -0000      1.355
+++ client/packhand.c   28 Mar 2004 04:37:57 -0000
@@ -73,7 +73,7 @@
 
 static void handle_city_packet_common(struct city *pcity, bool is_new,
                                       bool popup, bool investigate);
-static bool handle_unit_packet_common(struct unit *packet_unit, bool carried);
+static bool handle_unit_packet_common(struct unit *packet_unit);
 static int *reports_thaw_requests = NULL;
 static int reports_thaw_requests_size = 0;
 
@@ -111,10 +111,10 @@
   punit->connecting = packet->connecting;
   punit->done_moving = packet->done_moving;
   punit->occupy = packet->occupy;
-  if (packet->carried) {
-    punit->transported_by = 1;
+  if (packet->transported) {
+    punit->transported_by = packet->transported_by;
   } else {
-    punit->transported_by = 0;
+    punit->transported_by = -1;
   }
   punit->has_orders = packet->has_orders;
   punit->orders.length = packet->orders_length;
@@ -152,8 +152,12 @@
   punit->hp = packet->hp;
   punit->activity = packet->activity;
   punit->occupy = (packet->occupied ? 1 : 0);
-  punit->transported_by = 0;
-  
+  if (packet->transported) {
+    punit->transported_by = packet->transported_by;
+  } else {
+    punit->transported_by = -1;
+  }
+
   return punit;
 }
 
@@ -899,7 +903,7 @@
   }
 
   punit = unpackage_unit(packet);
-  if (handle_unit_packet_common(punit, packet->carried)) {
+  if (handle_unit_packet_common(punit)) {
     free(punit);
   }
 }
@@ -925,7 +929,7 @@
   is because the server can never deny a request for idle, and should not
   be concerned about which unit the client is focusing on.
 **************************************************************************/
-static bool handle_unit_packet_common(struct unit *packet_unit, bool carried)
+static bool handle_unit_packet_common(struct unit *packet_unit)
 {
   struct city *pcity;
   struct unit *punit;
@@ -1074,7 +1078,7 @@
       moved = TRUE;
 
       /* Show where the unit is going. */
-      do_move_unit(punit, packet_unit, carried);
+      do_move_unit(punit, packet_unit);
 
       if(pcity)  {
        if (can_player_see_units_in_city(game.player_ptr, pcity)) {
@@ -1179,7 +1183,7 @@
           unit_name(punit->type), punit->x, punit->y, punit->id,
           punit->homecity, (pcity ? pcity->name : _("(unknown)")));
 
-    repaint_unit = !carried;
+    repaint_unit = (punit->transported_by == -1);
     agents_unit_new(punit);
 
     if ((pcity = map_get_city(punit->x, punit->y))) {
@@ -1274,7 +1278,7 @@
   }
 
   punit = unpackage_short_unit(packet);
-  if (handle_unit_packet_common(punit, packet->carried)) {
+  if (handle_unit_packet_common(punit)) {
     free(punit);
   }
 }
Index: common/capstr.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/capstr.c,v
retrieving revision 1.159
diff -u -r1.159 capstr.c
--- common/capstr.c     18 Feb 2004 22:26:45 -0000      1.159
+++ common/capstr.c     28 Mar 2004 04:37:57 -0000
@@ -76,7 +76,7 @@
 
 #define CAPABILITY "+1.14.delta +last_turns_shield_surplus veteran +orders " \
                    "+starter +union +iso_maps +orders2client " \
-                   "+change_production +tilespec1 +no_earth"
+                   "+change_production +tilespec1 +no_earth +trans"
 
 /* "+1.14.delta" is the new delta protocol for 1.14.0-dev.
  *
@@ -102,6 +102,8 @@
  *
  * "no_earth" means that the map.is_earth value is gone; replaced by
  * ptile->spec_sprite
+ *
+ * "trans" means that the transported_by field is sent to the client.
  */
 
 void init_our_capability(void)
Index: common/packets.def
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/packets.def,v
retrieving revision 1.13
diff -u -r1.13 packets.def
--- common/packets.def  22 Mar 2004 20:58:11 -0000      1.13
+++ common/packets.def  28 Mar 2004 04:37:57 -0000
@@ -594,9 +594,10 @@
 
   UINT8 veteran; add-cap(veteran)
   BOOL veteran_old; remove-cap(veteran)
-  BOOL ai, paradropped, connecting, carried, done_moving;
+  BOOL ai, paradropped, connecting, transported, done_moving;
 
   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;
   COORD goto_dest_x,goto_dest_y;
@@ -618,12 +619,12 @@
 
   UINT8 veteran; add-cap(veteran)
   BOOL veteran_old; remove-cap(veteran)
-  BOOL occupied, goes_out_of_sight;
+  BOOL occupied, goes_out_of_sight, transported;
 
   UINT8 hp, activity;
+  UNIT transported_by; /* Only valid if transported is set. */
 
   /* in packet only, not in unit struct */
-  UNIT carried;                /* FIXME: should not send carried units at all? 
*/
   UINT8 packet_use;    /* see enum unit_info_use */
   CITY info_city_id;   /* for UNIT_INFO_CITY_SUPPORTED
                           and UNIT_INFO_CITY_PRESENT uses */
Index: server/diplomats.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/diplomats.c,v
retrieving revision 1.54
diff -u -r1.54 diplomats.c
--- server/diplomats.c  25 Feb 2004 20:23:50 -0000      1.54
+++ server/diplomats.c  28 Mar 2004 04:37:58 -0000
@@ -157,13 +157,13 @@
      As this is a special case we bypass send_unit_info. */
   first_packet = TRUE;
   unit_list_iterate(pcity->units_supported, punit) {
-    package_short_unit(punit, &unit_packet, FALSE,
+    package_short_unit(punit, &unit_packet,
                        UNIT_INFO_CITY_SUPPORTED, pcity->id, first_packet);
     lsend_packet_unit_short_info(&pplayer->connections, &unit_packet);
     first_packet = FALSE;
   } unit_list_iterate_end;
   unit_list_iterate(map_get_tile(pcity->x, pcity->y)->units, punit) {
-    package_short_unit(punit, &unit_packet, FALSE,
+    package_short_unit(punit, &unit_packet,
                        UNIT_INFO_CITY_PRESENT, pcity->id, first_packet);
     lsend_packet_unit_short_info(&pplayer->connections, &unit_packet);
     first_packet = FALSE;
Index: server/unithand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unithand.c,v
retrieving revision 1.290
diff -u -r1.290 unithand.c
--- server/unithand.c   8 Mar 2004 03:00:22 -0000       1.290
+++ server/unithand.c   28 Mar 2004 04:37:59 -0000
@@ -698,11 +698,11 @@
   combat.defender_hp=pdefender->hp;
   combat.make_winner_veteran=vet;
 
-  package_unit(punit, &unit_att_full_packet, FALSE);
-  package_unit(pdefender, &unit_def_full_packet, FALSE);
-  package_short_unit(punit, &unit_att_short_packet, FALSE,
+  package_unit(punit, &unit_att_full_packet);
+  package_unit(pdefender, &unit_def_full_packet);
+  package_short_unit(punit, &unit_att_short_packet,
                     UNIT_INFO_IDENTITY, 0, FALSE);
-  package_short_unit(pdefender, &unit_def_short_packet, FALSE,
+  package_short_unit(pdefender, &unit_def_short_packet,
                     UNIT_INFO_IDENTITY, 0, FALSE);
   
   players_iterate(other_player) {
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.286
diff -u -r1.286 unittools.c
--- server/unittools.c  15 Mar 2004 21:57:34 -0000      1.286
+++ server/unittools.c  28 Mar 2004 04:37:59 -0000
@@ -1892,8 +1892,7 @@
   Package a unit_info packet.  This packet contains basically all
   information about a unit.
 **************************************************************************/
-void package_unit(struct unit *punit, struct packet_unit_info *packet,
-                 bool carried)
+void package_unit(struct unit *punit, struct packet_unit_info *packet)
 {
   packet->id = punit->id;
   packet->owner = punit->owner;
@@ -1925,7 +1924,13 @@
   packet->paradropped = punit->paradropped;
   packet->connecting = punit->connecting;
   packet->done_moving = punit->done_moving;
-  packet->carried = carried;
+  if (punit->transported_by == -1) {
+    packet->transported = FALSE;
+    packet->transported_by = 0;
+  } else {
+    packet->transported = TRUE;
+    packet->transported_by = punit->transported_by;
+  }
   packet->occupy = get_transporter_occupancy(punit);
   packet->has_orders = punit->has_orders;
   if (punit->has_orders) {
@@ -1951,8 +1956,9 @@
   information about the unit, and is sent to players who shouldn't know
   everything (like the unit's owner's enemies).
 **************************************************************************/
-void package_short_unit(struct unit *punit, struct packet_unit_short_info 
*packet,
-                       bool carried, enum unit_info_use packet_use,
+void package_short_unit(struct unit *punit,
+                       struct packet_unit_short_info *packet,
+                       enum unit_info_use packet_use,
                        int info_city_id, bool new_serial_num)
 {
   static unsigned int serial_num = 0;
@@ -1983,7 +1989,18 @@
     packet->activity = punit->activity;
   }
 
-  packet->carried  = carried;
+  /* Transported_by information is sent to the client even for units that
+   * aren't fully known.  Note that for non-allied players, any transported
+   * unit can't be seen at all.  For allied players we have to know if
+   * transporters have room in them so that we can load units properly. */
+  if (punit->transported_by == -1) {
+    packet->transported = FALSE;
+    packet->transported_by = 0;
+  } else {
+    packet->transported = TRUE;
+    packet->transported_by = punit->transported_by;
+  }
+
   packet->goes_out_of_sight = FALSE;
 }
 
@@ -2016,15 +2033,13 @@
 {
   struct packet_unit_info info;
   struct packet_unit_short_info sinfo;
-  bool carried = punit->transported_by != -1;
   
   if (!dest) {
     dest = &game.game_connections;
   }
 
-  package_unit(punit, &info, carried);
-  package_short_unit(punit, &sinfo, carried,
-                    UNIT_INFO_IDENTITY, FALSE, FALSE);
+  package_unit(punit, &info);
+  package_short_unit(punit, &sinfo, UNIT_INFO_IDENTITY, FALSE, FALSE);
             
   conn_list_iterate(*dest, pconn) {
     struct player *pplayer = pconn->player;
Index: server/unittools.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.h,v
retrieving revision 1.65
diff -u -r1.65 unittools.h
--- server/unittools.h  12 Mar 2004 06:20:32 -0000      1.65
+++ server/unittools.h  28 Mar 2004 04:37:59 -0000
@@ -64,10 +64,9 @@
 void kill_unit(struct unit *pkiller, struct unit *punit);
 
 /* sending to client */
-void package_unit(struct unit *punit, struct packet_unit_info *packet,
-                 bool carried);
+void package_unit(struct unit *punit, struct packet_unit_info *packet);
 void package_short_unit(struct unit *punit,
-                       struct packet_unit_short_info *packet, bool carried,
+                       struct packet_unit_short_info *packet,
                        enum unit_info_use packet_use, int info_city_id,
                        bool new_serial_num);
 void send_unit_info(struct player *dest, struct unit *punit);

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