[Freeciv-Dev] Re: (PR#4095) Allied Victory :-)

["Per I. Mathisen" <per@xxxxxxxxxxx>]

On Mon, 28 Apr 2003, Gregory Berkolaiko wrote:
> > On Mon, 28 Apr 2003, ChrisK@xxxxxxxx wrote:
> > > A allied B
> > > B allied C
> > > A war C
> > >
> > > B's trireme with C's bowmen on it can move into A's city ...
...
> But the above bug would still be a bug if we substitute war for "neutral"
> or "peace", or any "non-aliance".

I started fixing this bug, and I thought it was as simple as adding this
to handle_unit_move_request():

+  /* We cannot transport non-allied units unto a city. */
+  if (pcity && get_transporter_capacity(punit) > 0) {
+    unit_list_iterate(map_get_tile(punit->x, punit->y)->units, cargo) {
+      if (cargo->transported_by == punit->id
+          && !pplayers_allied(city_owner(pcity), unit_owner(cargo))) {
+        notify_player_ex(pplayer, punit->x, punit->y, E_NOEVENT,
+                         _("Game: We cannot transport %s's units into %s's "
+                         "city, as those players are not allied."),
+                         unit_owner(cargo)->name, city_owner(pcity)->name);
+        return FALSE;
+      }
+    } unit_list_iterate_end;
+  }

But then I looked deeper, and found that this doesn't help at all, because
at this point _we still don't know what units we will be transporting_ !!
This is done in move_unit(), which should not do any checks whatsoever. It
simply drops any units we have transported so far and then scoops up any
units eligible for transportation.

The rules for which units are to be transported and not seem to me be a
bit unclear. server/unittools.c: assign_units_to_transporter() is a total
nightmare function.

Since adding checks inside move_unit() is clearly wrong and moving the
transporter code is dangerous without a full code audit, I came up with
the attached patch.

Someone once suggested we split out the 'load transport' command out of
the 'sentry' command. That now seems like a good idea to me. In any case,
this part of the code really warrants some eyeballs and applied brain
cells.

  - Per

Index: server/unithand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unithand.c,v
retrieving revision 1.257
diff -u -r1.257 unithand.c
--- server/unithand.c   4 Apr 2003 16:03:08 -0000       1.257
+++ server/unithand.c   28 Apr 2003 19:06:19 -0000
@@ -1056,8 +1056,8 @@
     return FALSE;
   }
 
-  /* If there is a city it is empty.
-     If not it would have been caught in the attack case. */
+  /* If there is a non-allied city it is empty. If not it would have been 
+   * caught in the attack case. */
   if (pcity && !pplayers_allied(city_owner(pcity), unit_owner(punit))) {
     if (is_air_unit(punit) || !is_military_unit(punit) || 
is_sailing_unit(punit)) {
       notify_player_ex(pplayer, punit->x, punit->y, E_NOEVENT,
@@ -1073,6 +1073,25 @@
       how_to_declare_war(pplayer);
       return FALSE;
     }
+  }
+
+  /* We cannot transport non-allied units unto a city. This check is 
+   * not perfect, but it will err on the side of caution. It may 
+   * happen that an allied transporter is stacked with us, so that
+   * it might be perfectly legal to move into the city with our
+   * units while it moved away with its own, but this we cannot
+   * check here. */
+  if (pcity && get_transporter_capacity(punit) > 0) {
+    struct tile *psrctile = map_get_tile(punit->x, punit->y);
+    struct unit *pcargo = is_non_allied_unit_tile(psrctile,
+                                                  city_owner(pcity));
+    if (pcargo) {
+      notify_player_ex(pplayer, punit->x, punit->y, E_NOEVENT,
+                       _("Game: We cannot transport %s's units into %s's "
+                       "city, as those players are not allied."),
+                       unit_owner(pcargo)->name, city_owner(pcity)->name);
+    }
+    return FALSE;
   }
 
   /******* ok now move the unit *******/