[Freeciv-Dev] (PR#7408) unit_list_size(&ptile->units) == 0 Again (Sea barbarians)

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

> [matusik_s@xxxxx - Sat Feb 14 12:09:45 2004]:

> And here is a bug! Mathew is receiving info about all units inside
> Galeon! This is wrong because when Galeon moves server will assume
> that Mathew cannot see it.
> 
> I'm not sure how this unit stack should be initialized. Jason?

Good catch.

create_unit calls send_unit_info to send the information to the clients.
 But since the unit isn't put inside the transporter until after
create_unit, it's not transported yet and so is sent to everyone
watching.  It looks like this problem occurrs in another part of the
code as well.

The solution is to pass another field to create_unit_fill.  Now we put
the unit in the transporter before sending it to the client.

This does highlight a problem in the code.  The server doesn't track
what units the client knows about.  It knows what the conditions are for
telling the client about a unit, and checks for those conditions to
change.  But if there's a bug it cannot recover.

> I found some stupid bug in try_summon_barbarians()
> Patch attached. Unfortunatelly problem is much more complicated. In
> this case xu = x and yu = y so the patch doesn't change anything.

I don't think this is correct.  (xu,yu) is not the same as (x,y).   
Unleash_barbarians is called when a player enters a hut.  The hut is at
(x,y) and the barbarians are unleashed at adjacent positions (xu,yu).

jason

Index: server/barbarian.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/barbarian.c,v
retrieving revision 1.72
diff -u -r1.72 barbarian.c
--- server/barbarian.c  2004/01/11 17:45:05     1.72
+++ server/barbarian.c  2004/02/14 17:53:37
@@ -391,7 +391,7 @@
     (void) create_unit(barbarians, xu, yu,
                       get_role_unit(L_BARBARIAN_LEADER, 0), 0, 0, -1);
   } else {                   /* sea raiders - their units will be veteran */
-    struct unit *punit, *ptrans;
+    struct unit *ptrans;
 
     barbarians = create_barbarian_player(FALSE);
     boat = find_a_unit_type(L_BARBARIAN_BOAT,-1);
@@ -399,13 +399,13 @@
     cap = get_transporter_capacity(unit_list_get(&map_get_tile(xu, yu)->units, 
0));
     for (i = 0; i < cap-1; i++) {
       unit = find_a_unit_type(L_BARBARIAN_SEA,L_BARBARIAN_SEA_TECH);
-      punit = create_unit(barbarians, xu, yu, unit, 0, 0, -1);
-      punit->transported_by = ptrans->id;
+      (void) create_unit_full(barbarians, xu, yu, unit, 0, 0, -1, -1,
+                             ptrans);
       freelog(LOG_DEBUG, "Created barbarian unit %s", unit_types[unit].name);
     }
-    punit = create_unit(barbarians, xu, yu,
-                       get_role_unit(L_BARBARIAN_LEADER, 0), 0, 0, -1);
-    punit->transported_by = ptrans->id;
+    (void) create_unit_full(barbarians, xu, yu,
+                           get_role_unit(L_BARBARIAN_LEADER, 0), 0, 0,
+                           -1, -1, ptrans);
   }
 
   unit_list_iterate(map_get_tile(x, y)->units, punit2) {
Index: server/citytools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.c,v
retrieving revision 1.252
diff -u -r1.252 citytools.c
--- server/citytools.c  2004/02/14 02:21:25     1.252
+++ server/citytools.c  2004/02/14 17:53:38
@@ -697,8 +697,7 @@
    * it's more work. */
   punit2 = create_unit_full(to_player, punit->x, punit->y, punit->type,
                            punit->veteran, tocity->id, punit->moves_left,
-                           punit->hp);
-  punit2->transported_by = punit->transported_by;
+                           punit->hp, punit);
 }
 
 /*********************************************************************
@@ -1199,8 +1198,8 @@
       notify_player(pplayer, _("Game: Changed homecity of %s in %s."),
                    unit_name(punit->type), new_home_city->name);
       (void) create_unit_full(city_owner(new_home_city), x, y,
-                      punit->type, punit->veteran, new_home_city->id,
-                      punit->moves_left, punit->hp);
+                             punit->type, punit->veteran, new_home_city->id,
+                             punit->moves_left, punit->hp, NULL);
     }
 
     wipe_unit(punit);
Index: server/diplomats.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/diplomats.c,v
retrieving revision 1.52
diff -u -r1.52 diplomats.c
--- server/diplomats.c  2004/02/14 02:21:26     1.52
+++ server/diplomats.c  2004/02/14 17:53:39
@@ -452,7 +452,7 @@
   (void) create_unit_full(pplayer, pvictim->x, pvictim->y,
                          pvictim->type, pvictim->veteran,
                          pdiplomat->homecity, pvictim->moves_left,
-                         pvictim->hp);
+                         pvictim->hp, NULL);
 
   /* Check if the unit gained veteran level */
   vet = maybe_make_veteran(pdiplomat);
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.278
diff -u -r1.278 unittools.c
--- server/unittools.c  2004/02/07 11:16:50     1.278
+++ server/unittools.c  2004/02/14 17:53:41
@@ -1495,7 +1495,7 @@
                          int homecity_id, int moves_left)
 {
   return create_unit_full(pplayer, x, y, type, veteran_level, homecity_id, 
-                          moves_left, -1);
+                          moves_left, -1, NULL);
 }
 
 /**************************************************************************
@@ -1504,7 +1504,8 @@
 **************************************************************************/
 struct unit *create_unit_full(struct player *pplayer, int x, int y,
                              Unit_Type_id type, int veteran_level, 
-                              int homecity_id, int moves_left, int hp_left)
+                              int homecity_id, int moves_left, int hp_left,
+                             struct unit *ptrans)
 {
   struct unit *punit = create_unit_virtual(pplayer, NULL, type, veteran_level);
   struct city *pcity;
@@ -1532,6 +1533,11 @@
   if (moves_left >= 0) {
     /* Override default full MP */
     punit->moves_left = MIN(moves_left, unit_move_rate(punit));
+  }
+
+  if (ptrans) {
+    /* Set transporter for unit. */
+    punit->transported_by = ptrans->id;
   }
 
   /* Assume that if moves_left < 0 then the unit is "fresh",
Index: server/unittools.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.h,v
retrieving revision 1.62
diff -u -r1.62 unittools.h
--- server/unittools.h  2004/01/28 06:02:44     1.62
+++ server/unittools.h  2004/02/14 17:53:41
@@ -55,8 +55,9 @@
 struct unit *create_unit(struct player *pplayer, int x, int y, Unit_Type_id 
type,
                         int veteran_level, int homecity_id, int moves_left);
 struct unit *create_unit_full(struct player *pplayer, int x, int y,
-                             Unit_Type_id type, int veteran_level, int 
homecity_id,
-                             int moves_left, int hp_left);
+                             Unit_Type_id type, int veteran_level,
+                             int homecity_id, int moves_left, int hp_left,
+                             struct unit *ptrans);
 void wipe_unit(struct unit *punit);
 void wipe_unit_spec_safe(struct unit *punit, bool wipe_cargo);
 void kill_unit(struct unit *pkiller, struct unit *punit);