[Freeciv-Dev] (PR#6174) Loading transports

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

> [per - Tue Sep 16 15:35:24 2003]:
> 
> On Tue, 16 Sep 2003, Jason Short wrote:
> > As discussed on IRC, I think we need to clean up the current system
> > (including removal of assign_units_to_transporter) before we can get
> > into new behavior.
> 
> This is not possible. assign_units_to_transporter() is there for a reason.
> 
> Examples:
> - if a ground transport is produced in a city with lots of sentried ground
> units, are the sentried units to be considered transported?

No.  I've always considered this a bug.

> - if you unsentry a ground unit in a sea transport, do you consider it
> transported, or not?

If it's in the sea it has to be transported.  If it's in a city, then we
have a problem - see below.

> - if you sentry a ground unit in a city containing two sea transporters
> and one ground transporter, which unit now transports the first unit?

Which unit is the "first" unit?  The ground transporter carries the
ground unit.  Presumably the sea transporters don't carry anything.  The
current code is ill-equipped to handle sea transporters, or any
transport that has the possibility to be recursive.

Perhaps the best way to solve this is to give a "size" or "weight" to
units and limit the maximum capacity of the transport in those terms. 
Another alternative is a can_be_transported flag for unit types (however
this has big problems, I think).

Hmm, maybe the best way is to abstract away the "ground" versus "sea"
carrying into carry types.  Each transporter lists the types of units it
can carry, and each unit lists the type(s?) of transporters it can carry.

E.g.

[unit_transport]
carry_type = ground_small ground_large
fill_type = sea_large

[unit_musketeers]
carry_type = none
fill_type = ground_small

; hypothetical units follow
[unit_airtransport]
carry_type = ground_small
fill_type = air_large

[unit_flying_carrier]
carry_type = air_small
fill_type = air_large

[unit_apc]
carry_type = ground_small
fill_type = ground_large

> The answer to all these questions is, currently, 'no' and 'none'. The ugly
> function mentioned above is there to resolve such issues. If you just
> remove it, lots of stuff will break, and behaviour changes will ensue in
> any case.

Everything will work out if we put units into and take them out of
transporters when we're supposed to.

For instance the attached patch removes assign_units_to_transporter
entirely.  The only addition is in move_unit; even this one may not be
strictly necessary.  This works with very few problems.

Unfortunately very few is not none.  The most obvious (and hopefully
only) problem is that the only way to move units onto or off of
transporters is by physically moving them.  We can simulate the correct
behavior by:

- Any time a unit is sentried in a city, it is loaded into any ship there.
- Any time a unit is unsentried in a city, it is unloaded from its ship.

Of course this means to put a sentry into a newly-arrived ship you would
have to unsentry and then resentry it.  More problematic, there is no
way for the user to know if a unit is in a transporter.  For this we
need new graphics.

Ultimately a load/unload command is needed to put units into and out of
arbitrary transporters.  That way we can load ships on rivers, load
helicoptors, etc.  With an appropriate graphical interface the user
should even be able to determine _which_ transporter the unit should be
loaded into.

jason

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/27 02:18:42
@@ -82,15 +82,6 @@
 
   send_unit_info(NULL, punit);
 
-  /* 
-   * Normally units on goto does not pick up extra units, even if the
-   * units are in a city and are sentried. But if we just started the
-   * goto We want to take them with us, so we do this. 
-   */
-  if (get_transporter_capacity(punit) > 0) {
-    assign_units_to_transporter(punit, TRUE);
-  }
-
   (void) do_unit_goto(punit, GOTO_MOVE_ANY, TRUE);
 }
 
@@ -1550,7 +1541,6 @@
   }
 #endif
 
-  assign_units_to_transporter(punit, TRUE);
   if (execute_orders(punit) != GR_DIED) {
     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/27 02:18:43
@@ -2446,272 +2446,6 @@
   return ok;
 }
 
-/**************************************************************************
-  Assigns units on ptrans' tile to ptrans if they should be. This is done 
-  by setting their transported_by fields to the id of ptrans.
-
-  Checks a zillion things, some from situations that should never happen.
-  First drop all previously assigned units that do not fit on the 
-  transport.
-
-  If on land maybe pick up some extra units (decided by take_from_land 
-  variable)
-
-  This function is getting a bit larger and ugly. Perhaps it would be nicer
-  if it was recursive!?
-
-FIXME: If in the open (not city), and leaving with only those units that are
-       already assigned to us would strand some units try to reassign the
-       transports. This reassign sometimes makes more changes than it needs to.
-
-       Groundunit ground unit transports moving to and from ships never take
-       units with them. This is ok, but not always practical.
-**************************************************************************/
-void assign_units_to_transporter(struct unit *ptrans, bool take_from_land)
-{
-  int x = ptrans->x;
-  int y = ptrans->y;
-  int playerid = ptrans->owner;
-  int capacity = get_transporter_capacity(ptrans);
-  struct tile *ptile = map_get_tile(x, y);
-
-  /*** Ground units transports first ***/
-  if (is_ground_units_transport(ptrans)) {
-    int available =
-       ground_unit_transporter_capacity(x, y, unit_owner(ptrans));
-
-    /* See how many units are dedicated to this transport, and remove extra 
units */
-    unit_list_iterate(ptile->units, pcargo) {
-      if (pcargo->transported_by == ptrans->id) {
-       if (is_ground_unit(pcargo)
-           && capacity > 0
-           && (is_ocean(ptile->terrain)
-               || pcargo->activity == ACTIVITY_SENTRY)
-           && (pcargo->owner == playerid
-                || pplayers_allied(unit_owner(pcargo), unit_owner(ptrans)))
-           && pcargo->id != ptrans->id
-           && !(is_ground_unit(ptrans)
-            && (is_ocean(ptile->terrain)
-               || is_ground_units_transport(pcargo)))) {
-         capacity--;
-       } else {
-         pcargo->transported_by = -1;
-       }
-      }
-    } unit_list_iterate_end;
-
-    /** We are on an ocean tile. All units must get a transport **/
-    if (is_ocean(ptile->terrain)) {
-      /* While the transport is not full and units will strand if we don't take
-        them with we us dedicate them to this transport. */
-      if (available - capacity < 0 && !is_ground_unit(ptrans)) {
-       unit_list_iterate(ptile->units, pcargo) {
-         if (capacity == 0)
-           break;
-         if (is_ground_unit(pcargo)
-             && pcargo->transported_by != ptrans->id
-             && (pcargo->owner == playerid
-                  || pplayers_allied(unit_owner(pcargo), unit_owner(ptrans)))) 
{
-           capacity--;
-           pcargo->transported_by = ptrans->id;
-         }
-       } unit_list_iterate_end;
-      }
-    } else { /** We are on a land tile **/
-      /* If ordered to do so we take extra units with us, provided they
-       * are not already committed to another transporter on the tile. */
-      if (take_from_land) {
-       unit_list_iterate(ptile->units, pcargo) {
-         if (capacity == 0)
-           break;
-         if (is_ground_unit(pcargo)
-             && pcargo->transported_by != ptrans->id
-             && pcargo->activity == ACTIVITY_SENTRY
-             && pcargo->id != ptrans->id
-             && pcargo->owner == playerid
-             && !(is_ground_unit(ptrans)
-             && (is_ocean(ptile->terrain)
-                 || is_ground_units_transport(pcargo)))) {
-           bool has_trans = FALSE;
-
-           unit_list_iterate(ptile->units, ptrans2) {
-             if (ptrans2->id == pcargo->transported_by)
-               has_trans = TRUE;
-           } unit_list_iterate_end;
-           if (!has_trans) {
-             capacity--;
-             pcargo->transported_by = ptrans->id;
-           }
-         }
-       } unit_list_iterate_end;
-      }
-    }
-    return;
-    /*** Allocate air and missile units ***/
-  } else if (is_air_units_transport(ptrans)) {
-    struct player_tile *plrtile =
-       map_get_player_tile(x, y, unit_owner(ptrans));
-    bool is_refuel_point =
-       is_allied_city_tile(map_get_tile(x, y), unit_owner(ptrans))
-       || (contains_special(plrtile->special, S_AIRBASE)
-           && !is_non_allied_unit_tile(map_get_tile(x, y),
-                                       unit_owner(ptrans)));
-    bool missiles_only = unit_flag(ptrans, F_MISSILE_CARRIER)
-      && !unit_flag(ptrans, F_CARRIER);
-
-    /* Make sure we can transport the units marked as being transported by 
ptrans */
-    unit_list_iterate(ptile->units, pcargo) {
-      if (ptrans->id == pcargo->transported_by) {
-       if ((pcargo->owner == playerid
-             || pplayers_allied(unit_owner(pcargo), unit_owner(ptrans)))
-           && pcargo->id != ptrans->id
-           && (!is_sailing_unit(pcargo))
-           && (unit_flag(pcargo, F_MISSILE) || !missiles_only)
-           && !(is_ground_unit(ptrans) && is_ocean(ptile->terrain))
-           && (capacity > 0)) {
-         if (is_air_unit(pcargo))
-           capacity--;
-         /* Ground units are handled below */
-       } else
-         pcargo->transported_by = -1;
-      }
-    } unit_list_iterate_end;
-
-    /** We are at a refuel point **/
-    if (is_refuel_point) {
-      unit_list_iterate(ptile->units, pcargo) {
-       if (capacity == 0)
-         break;
-       if (is_air_unit(pcargo)
-           && pcargo->id != ptrans->id
-           && pcargo->transported_by != ptrans->id
-           && pcargo->activity == ACTIVITY_SENTRY
-           && (unit_flag(pcargo, F_MISSILE) || !missiles_only)
-           && (pcargo->owner == playerid
-                || pplayers_allied(unit_owner(pcargo), unit_owner(ptrans)))) {
-         bool has_trans = FALSE;
-
-         unit_list_iterate(ptile->units, ptrans2) {
-           if (ptrans2->id == pcargo->transported_by)
-             has_trans = TRUE;
-         } unit_list_iterate_end;
-         if (!has_trans) {
-           capacity--;
-           pcargo->transported_by = ptrans->id;
-         }
-       }
-      } unit_list_iterate_end;
-    } else { /** We are in the open. All units must have a transport if 
possible **/
-      int aircap = airunit_carrier_capacity(x, y, unit_owner(ptrans), TRUE);
-      int miscap = missile_carrier_capacity(x, y, unit_owner(ptrans), TRUE);
-
-      /* Not enough capacity. Take anything we can */
-      if ((aircap < capacity || miscap < capacity)
-         && !(is_ground_unit(ptrans) && is_ocean(ptile->terrain))) {
-       /* We first take nonmissiles, as missiles can be transported on 
anything,
-          but nonmissiles can not */
-       if (!missiles_only) {
-         unit_list_iterate(ptile->units, pcargo) {
-           if (capacity == 0)
-             break;
-           if (is_air_unit(pcargo)
-               && pcargo->id != ptrans->id
-               && pcargo->transported_by != ptrans->id
-               && !unit_flag(pcargo, F_MISSILE)
-               && (pcargo->owner == playerid
-                    || pplayers_allied(unit_owner(pcargo), 
unit_owner(ptrans)))) {
-             capacity--;
-             pcargo->transported_by = ptrans->id;
-           }
-         } unit_list_iterate_end;
-       }
-       
-       /* Just take anything there's left.
-          (which must be missiles if we have capacity left) */
-       unit_list_iterate(ptile->units, pcargo) {
-         if (capacity == 0)
-           break;
-         if (is_air_unit(pcargo)
-             && pcargo->id != ptrans->id
-             && pcargo->transported_by != ptrans->id
-             && (!missiles_only || unit_flag(pcargo, F_MISSILE))
-             && (pcargo->owner == playerid
-                  || pplayers_allied(unit_owner(pcargo), unit_owner(ptrans)))) 
{
-           capacity--;
-           pcargo->transported_by = ptrans->id;
-         }
-       } unit_list_iterate_end;
-      }
-    }
-
-    /** If any of the transported air units have land units on board take them 
with us **/
-    {
-      int totcap = 0;
-      int available =
-         ground_unit_transporter_capacity(x, y, unit_owner(ptrans));
-      struct unit_list trans2s;
-      unit_list_init(&trans2s);
-
-      unit_list_iterate(ptile->units, pcargo) {
-       if (pcargo->transported_by == ptrans->id
-           && is_ground_units_transport(pcargo)
-           && is_air_unit(pcargo)) {
-         totcap += get_transporter_capacity(pcargo);
-         unit_list_insert(&trans2s, pcargo);
-       }
-      } unit_list_iterate_end;
-
-      unit_list_iterate(ptile->units, pcargo2) {
-       bool has_trans = FALSE;
-
-       unit_list_iterate(trans2s, ptrans2) {
-         if (pcargo2->transported_by == ptrans2->id)
-           has_trans = TRUE;
-       } unit_list_iterate_end;
-       if (pcargo2->transported_by == ptrans->id)
-         has_trans = TRUE;
-
-       if (has_trans
-           && is_ground_unit(pcargo2)) {
-         if (totcap > 0
-             && (is_ocean(ptile->terrain)
-                 || pcargo2->activity == ACTIVITY_SENTRY)
-             && (pcargo2->owner == playerid
-                  || pplayers_allied(unit_owner(pcargo2), unit_owner(ptrans)))
-             && pcargo2 != ptrans) {
-           pcargo2->transported_by = ptrans->id;
-           totcap--;
-         } else
-           pcargo2->transported_by = -1;
-       }
-      } unit_list_iterate_end;
-
-      /* Uh oh. Not enough space on the square we leave if we don't
-        take extra units with us */
-      if (totcap > available && is_ocean(ptile->terrain)) {
-       unit_list_iterate(ptile->units, pcargo2) {
-         if (is_ground_unit(pcargo2)
-             && totcap > 0
-             && (pcargo2->owner == playerid
-                  || pplayers_allied(unit_owner(pcargo2), unit_owner(ptrans)))
-             && pcargo2->transported_by != ptrans->id) {
-           pcargo2->transported_by = ptrans->id;
-           totcap--;
-         }
-       } unit_list_iterate_end;
-      }
-    }
-  } else {
-    unit_list_iterate(ptile->units, pcargo) {
-      if (ptrans->id == pcargo->transported_by)
-       pcargo->transported_by = -1;
-    } unit_list_iterate_end;
-    freelog (LOG_VERBOSE, "trying to assign cargo to a non-transporter "
-            "of type %s at %i,%i",
-            get_unit_name(ptrans->type), ptrans->x, ptrans->y);
-  }
-}
-
 /*****************************************************************
 Will wake up any neighboring enemy sentry units or patrolling units
 *****************************************************************/
@@ -2893,6 +2627,7 @@
   struct tile *pdesttile = map_get_tile(dest_x, dest_y);
   struct city *pcity;
   struct unit *ptransporter = NULL;
+  struct unit_list cargo_units;
     
   CHECK_MAP_POS(dest_x, dest_y);
 
@@ -2936,31 +2671,14 @@
      then insert them again. The way this is done makes sure that the
      units stay in the same order. */
   if (get_transporter_capacity(punit) > 0 && transport_units) {
-    struct unit_list cargo_units;
-
     /* First make a list of the units to be moved. */
     unit_list_init(&cargo_units);
-    assign_units_to_transporter(punit, take_from_land);
     unit_list_iterate(psrctile->units, pcargo) {
       if (pcargo->transported_by == punit->id) {
        unit_list_unlink(&psrctile->units, pcargo);
        unit_list_insert(&cargo_units, pcargo);
       }
     } unit_list_iterate_end;
-
-    /* Insert them again. */
-    unit_list_iterate(cargo_units, pcargo) {
-      unfog_area(unit_owner(pcargo), dest_x, dest_y, 
unit_type(pcargo)->vision_range);
-      pcargo->x = dest_x;
-      pcargo->y = dest_y;
-
-      unit_list_insert(&pdesttile->units, pcargo);
-      check_unit_activity(pcargo);
-      send_unit_info_to_onlookers(NULL, pcargo, src_x, src_y, FALSE);
-      fog_area(unit_owner(pcargo), src_x, src_y, 
unit_type(pcargo)->vision_range);
-      handle_unit_move_consequences(pcargo, src_x, src_y, dest_x, dest_y);
-    } unit_list_iterate_end;
-    unit_list_unlink_all(&cargo_units);
   }
 
   /* We first unfog the destination, then move the unit and send the
@@ -2982,10 +2700,30 @@
   punit->x = dest_x;
   punit->y = dest_y;
   punit->moved = TRUE;
+
+  /* Update transporter/transportee information. */
   if (punit->transported_by != -1) {
     ptransporter = find_unit_by_id(punit->transported_by);
   }
-  punit->transported_by = -1;
+  punit->transported_by = -1; /* FIXME */
+  if (get_transporter_capacity(punit) > 0 && transport_units) {
+    /* Insert them again. */
+    unit_list_iterate(cargo_units, pcargo) {
+      unfog_area(unit_owner(pcargo), dest_x, dest_y,
+                unit_type(pcargo)->vision_range);
+      pcargo->x = dest_x;
+      pcargo->y = dest_y;
+
+      unit_list_insert(&pdesttile->units, pcargo);
+      check_unit_activity(pcargo);
+      send_unit_info_to_onlookers(NULL, pcargo, src_x, src_y, FALSE);
+      fog_area(unit_owner(pcargo), src_x, src_y,
+              unit_type(pcargo)->vision_range);
+      handle_unit_move_consequences(pcargo, src_x, src_y, dest_x, dest_y);
+    } unit_list_iterate_end;
+    unit_list_unlink_all(&cargo_units);
+  }
+
   punit->moves_left = MAX(0, punit->moves_left - move_cost);
   if (punit->moves_left == 0) {
     punit->done_moving = TRUE;
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/27 02:18:43
@@ -79,7 +79,6 @@
 bool try_move_unit(struct unit *punit, int dest_x, int dest_y); 
 bool do_airline(struct unit *punit, struct city *city2);
 bool do_paradrop(struct unit *punit, int dest_x, int dest_y);
-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);