[Freeciv-Dev] Re: (PR#10437) Another assert failure for "punit->transported_by > 0"

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

Jason Short wrote:
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=10437 >
> 
> I can reproduce this in the stable branch.  Year is 1480.

Adding some assertions, I find:

- Around year -2400, we try to load a unit into a boat it's already 
transported by.  This triggers no assertions, but is surely a bug.

- Around year -100, we try to load a unit into a boat when it's already 
transported by another boat.  This triggers no assertions, but is surely 
a bug.


However the main bugs are still in year 1480, and with some more 
debugging I find them.

1.  punit->owner == ferryboat->owner == 4.  However pplayer->player_no 
== 3.  So somehow we are trying to control a unit that we don't own.

2.  In handle_unit_load, player_find_unit_by_id is used for both cargo 
and transporter.  For the transporter this should just be 
find_unit_by_id.  We are allowed to load units into transports owned by 
someone else - according to can_unit_load the rule is that the cargo and 
transporter's owners must be allied.

The attached patch should fix bug#2 and the symptoms of bug#1 (which is 
probably related to the 2 other bugs above).  However a proper fix for 
bug#1 is probably needed to avoid other problems (like trying to build 
boats to transport enemy units...that would be bad...).

jason

Index: ai/aiferry.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aiferry.c,v
retrieving revision 1.6
diff -u -r1.6 aiferry.c
--- ai/aiferry.c        29 Sep 2004 02:24:18 -0000      1.6
+++ ai/aiferry.c        7 Oct 2004 08:01:11 -0000
@@ -448,8 +448,10 @@
       (void) ai_unit_move(punit, ferryboat->tile);
     }
 
-    if (!same_pos(punit->tile, ferryboat->tile)) {
+    if (!can_unit_load(punit, ferryboat)) {
       /* Something prevented us from boarding */
+      /* FIXME: this is probably a serious bug, but we just skip past
+       * it and continue. */
       UNIT_LOG(LOGLEVEL_GOBYBOAT, punit, "couldn't board boat[%d](%d,%d)",
               ferryboat->id, ferryboat->tile);
       return FALSE;
Index: server/unithand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unithand.c,v
retrieving revision 1.308
diff -u -r1.308 unithand.c
--- server/unithand.c   1 Oct 2004 17:53:02 -0000       1.308
+++ server/unithand.c   7 Oct 2004 08:01:11 -0000
@@ -1499,8 +1499,11 @@
 ****************************************************************************/
 void handle_unit_load(struct player *pplayer, int cargo_id, int trans_id)
 {
+  /* A player may only load their units, but they may be loaded into
+   * other players transporters (depending on the rules in
+   * can_unit_load). */
   struct unit *pcargo = player_find_unit_by_id(pplayer, cargo_id);
-  struct unit *ptrans = player_find_unit_by_id(pplayer, trans_id);
+  struct unit *ptrans = find_unit_by_id(trans_id);
 
   if (!pcargo || !ptrans) {
     return;