[Freeciv-Dev] (PR#8389) refuel_air_units_from_carriers is awful

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

> [jdorje - Fri Mar 26 17:57:52 2004]:
> 
> > [jdorje - Fri Mar 26 03:41:07 2004]:
> > 
> > This function follows the old assign_units_to_transporter paradigm.  It 
> > should be rewritten to be much smaller.  And it should only refuel
units 
> > that are actually being transported.
> 
> IMO the new invariant should be: all units that are transported at the
> end of a turn are refueled.
> 
> However this isn't so good unless the player can see exactly which units
> are transported.  So we may need some other cleanups first.

Here's a patch.  Basically it just removes 120 lines of code that
redistributed the transported units to be the most important ones.  Now
only the currently transported units are refueled.  The new behavior is
better because it's consistent from the player's point of view (he knows
exactly which units will be refueled) and because it's much simpler. 
The only drawback is stupid players may lose their planes more often.

This works best with a better client UI for transported units, but it
doesn't have to depend on that.  So I think it should be applied.

jason

? Womoks
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  26 Mar 2004 22:40:45 -0000
@@ -63,11 +63,6 @@
 
 static void sentry_transported_idle_units(struct unit *ptrans);
 
-static void refuel_air_units_from_carriers(struct player *pplayer);
-static struct unit *find_best_air_unit_to_refuel(struct player *pplayer, 
-                                                int x, int y, bool missile);
-static struct unit *choose_more_important_refuel_target(struct unit *punit1,
-                                                       struct unit *punit2);
 static bool maybe_cancel_patrol_due_to_enemy(struct unit *punit);
 static int hp_gain_coord(struct unit *punit);
 
@@ -227,37 +222,6 @@
 }
 
 /***************************************************************************
-Select which unit is more important to refuel:
-It's more important to refuel plane which has less fuel.
-If those are equal then we refuel more valuable unit.
-If those are equal then we refuel veteran unit.
-If those are equal then we refuel unit which has more hp.
-If those are equal then we refuel the first unit.
-****************************************************************************/
-static struct unit *choose_more_important_refuel_target(struct unit *punit1,
-                                                       struct unit *punit2)
-{
-  if (punit1->fuel != punit2->fuel)
-    return punit1->fuel < punit2->fuel? punit1: punit2;
-  
-  if (unit_build_shield_cost(punit1->type)
-      != unit_build_shield_cost(punit2->type)) {
-    return (unit_build_shield_cost(punit1->type)
-           > unit_build_shield_cost(punit2->type)) ? punit1 : punit2;
-  }
-  
-  if (punit1->veteran != punit2->veteran) {
-    return punit1->veteran > punit2->veteran ? punit1 : punit2;
-  }
-
-  if (punit1->hp != punit2->hp) {
-    return punit1->hp > punit2->hp ? punit1: punit2;
-  }
-
-  return punit1;
-}
-
-/***************************************************************************
   Pay the cost of supported units of one city
 ***************************************************************************/
 void pay_for_units(struct player *pplayer, struct city *pcity)
@@ -289,89 +253,6 @@
 }
 
 /***************************************************************************
-...
-****************************************************************************/
-static struct unit *find_best_air_unit_to_refuel(struct player *pplayer, 
-                                                int x, int y, bool missile)
-{
-  struct unit *best_unit = NULL;
-
-  unit_list_iterate(map_get_tile(x, y)->units, punit) {
-    if (unit_owner(punit) == pplayer
-        && is_air_unit(punit)
-        && (!missile || unit_flag(punit, F_MISSILE))) {
-      /* We must check that it isn't already refuelled. */ 
-      if (punit->fuel < unit_type(punit)->fuel) { 
-        if (!best_unit) {
-          best_unit = punit;
-        } else {
-          best_unit = choose_more_important_refuel_target(best_unit, punit);
-        }
-      }
-    }
-  } unit_list_iterate_end;
-  return best_unit;
-}
-
-/***************************************************************************
-  Refuel fuel-based units up to carrying capacity of carriers on the
-  same tile.
-****************************************************************************/
-static void refuel_air_units_from_carriers(struct player *pplayer)
-{
-  struct unit_list carriers;
-  struct unit_list missile_carriers;
-  
-  struct unit *punit_to_refuel;
-  
-  unit_list_init(&carriers);
-  unit_list_init(&missile_carriers);
-
-  /* Make a list of all missile and aircraft carriers. */
-  unit_list_iterate(pplayer->units, punit) {
-    if (unit_flag(punit, F_CARRIER)) {
-      unit_list_insert(&carriers, punit);
-    } else if (unit_flag(punit, F_MISSILE_CARRIER)) {
-      unit_list_insert(&missile_carriers, punit);
-    }
-  } unit_list_iterate_end;
-
-  /* Now iterate through missile_carriers and refuel as many 
-   * missiles as possible. */
-  unit_list_iterate(missile_carriers, punit) {
-    int cargo_fuel = unit_type(punit)->transport_capacity;
-
-    while (cargo_fuel > 0) {
-      punit_to_refuel = find_best_air_unit_to_refuel(
-          pplayer, punit->x, punit->y, TRUE /*missile */);
-      if (!punit_to_refuel) {
-        break; /* Didn't find any */
-      }
-      punit_to_refuel->fuel = unit_type(punit_to_refuel)->fuel;
-      cargo_fuel--;
-    }
-  } unit_list_iterate_end;
-
-  /* Now refuel air units from carriers (also not yet refuelled missiles) */
-  unit_list_iterate(carriers, punit) {
-    int cargo_fuel = unit_type(punit)->transport_capacity;
-
-    while(cargo_fuel > 0) {
-      punit_to_refuel = find_best_air_unit_to_refuel(
-          pplayer, punit->x, punit->y, FALSE /* any */);
-      if (!punit_to_refuel) {
-        break; /* Didn't find any */
-      }
-      punit_to_refuel->fuel = unit_type(punit_to_refuel)->fuel;
-      cargo_fuel--;
-    }
-  } unit_list_iterate_end;
-
-  unit_list_unlink_all(&carriers);
-  unit_list_unlink_all(&missile_carriers);
-}
-
-/***************************************************************************
 Do Leonardo's Workshop upgrade if applicable.
 Restore unit hitpoints. (movepoint-restoration moved to update_unit_activities)
 Adjust air units for fuel: air units lose fuel unless in a city,
@@ -464,17 +345,17 @@
       /* 6) Update fuel */
       punit->fuel--;
 
-      /* 7) Automatically refuel air units in cities and airbases */
+      /* 7) Automatically refuel air units in cities, airbases, and
+       *    transporters (carriers). */
       if (map_get_city(punit->x, punit->y) ||
-         map_has_special(punit->x, punit->y, S_AIRBASE))
+         map_has_special(punit->x, punit->y, S_AIRBASE)
+         || punit->transported_by != -1) {
        punit->fuel=unit_type(punit)->fuel;
+      }
     }
   } unit_list_iterate_safe_end;
 
-  /* 8) Use carriers and submarines to refuel air units */
-  refuel_air_units_from_carriers(pplayer);
-  
-  /* 9) Check if there are air units without fuel */
+  /* 8) Check if there are air units without fuel */
   unit_list_iterate_safe(pplayer->units, punit) {
     if (is_air_unit(punit) && punit->fuel <= 0
         && unit_type(punit)->fuel > 0) {