[Freeciv-Dev] (PR#4620) Assert in get_defender

["Gregory Berkolaiko" <Gregory.Berkolaiko@xxxxxxxxxxxx>]

Attached is a patch that 
(a) Removes the check for alliances from get_defender
(b) Makes the check for a positive result non-fatal
(c) Fixes the last (hopefully) place in code that relies on get_defender 
returning a non-ally.

I couldn't stop myself from making some minor style fixes.

This solution is less painful than making DS_NO_CONTACT equivalent to 
NEUTRAL.  The problem with that is that a lot of code in ai/advdiplomacy 
relies on DS_NO_CONTACT being enemy (like declaring war against a player 
doesn't check if we are in contact at all).

I am intent on committing this patch so shout if you dislike it,
G.

Index: common/combat.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/combat.c,v
retrieving revision 1.31
diff -u -r1.31 combat.c
--- common/combat.c     2003/06/04 20:05:40     1.31
+++ common/combat.c     2003/08/26 11:58:06
@@ -577,20 +577,18 @@
 struct unit *get_defender(struct unit *attacker, int x, int y)
 {
   struct unit *bestdef = NULL;
-  int bestvalue = -1, count = 0, best_cost = 0, rating_of_best = 0;
-  struct player *att_owner = unit_owner(attacker);
+  int bestvalue = -1, best_cost = 0, rating_of_best = 0;
+  struct tile *ptile = map_get_tile(x, y);
 
-  unit_list_iterate(map_get_tile(x, y)->units, defender) {
-    if (pplayers_allied(att_owner, unit_owner(defender)))
-      continue;
-    count++;
+  unit_list_iterate(ptile->units, defender) {
     if (unit_can_defend_here(defender)) {
       bool change = FALSE;
       int build_cost = unit_type(defender)->build_cost;
       int defense_rating = get_defense_rating(attacker, defender);
-
       /* This will make units roughly evenly good defenders look alike. */
-      int unit_def = (int) (100000 * (1 - unit_win_chance(attacker, 
defender)));
+      int unit_def 
+        = (int) (100000 * (1 - unit_win_chance(attacker, defender)));
+
       assert(unit_def >= 0);
 
       if (unit_def > bestvalue) {
@@ -614,8 +612,7 @@
     }
   } unit_list_iterate_end;
 
-  if (count > 0 && !bestdef) {
-    struct tile *ptile = map_get_tile(x, y);
+  if (unit_list_size(&ptile->units) > 0 && !bestdef) {
     struct unit *punit = unit_list_get(&ptile->units, 0);
 
     freelog(LOG_ERROR, "get_defender bug: %s's %s vs %s's %s (total %d"
@@ -623,7 +620,6 @@
             unit_type(attacker)->name, unit_owner(punit)->name,
             unit_type(punit)->name, unit_list_size(&ptile->units), 
             get_terrain_name(ptile->terrain), x, y);
-    assert(FALSE);
   }
 
   return bestdef;
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.244
diff -u -r1.244 unittools.c
--- server/unittools.c  2003/08/26 11:12:52     1.244
+++ server/unittools.c  2003/08/26 11:58:07
@@ -2755,8 +2755,7 @@
 {
   int src_x = punit->x;
   int src_y = punit->y;
-  int playerid = punit->owner;
-  struct player *pplayer = get_player(playerid);
+  struct player *pplayer = unit_owner(punit);
   struct tile *psrctile = map_get_tile(src_x, src_y);
   struct tile *pdesttile = map_get_tile(dest_x, dest_y);
   struct city *pcity;
@@ -2775,10 +2774,12 @@
       punit->ai.ferryboat = 0;
     }
   }
-  /* A transporter should not take units with it when on an attack goto -- 
fisch */
-  if ((punit->activity == ACTIVITY_GOTO) &&
-      get_defender(punit, goto_dest_x(punit), goto_dest_y(punit)) &&
-      !is_ocean(psrctile->terrain)) {
+  /* A transporter should not take units with it when on an attack goto 
+   * -- fisch */
+  if ((punit->activity == ACTIVITY_GOTO) 
+      && (is_non_allied_unit_tile(pdesttile, pplayer) 
+          || is_non_allied_city_tile(pdesttile, pplayer))
+      && !is_ocean(psrctile->terrain)) {
     transport_units = FALSE;
   }