[Freeciv-Dev] Re: (PR#6785) Assertion triggers in 1.14-beta2!

["mateusz stefek" <matusik_s@xxxxx>]

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


> > My solution _really_ removes bug from split_player().
> 
> Looks that way (haven't tested it yet). I didn't see your patch before I
> sent my post.
> 
>   - Per
There's exactly the same problem in CVS HEAD (I can't produce a core but it is 
possible)
Attached patch makes resolve_unit_stacks more accurate.
--
mateusz

--- freeorig/server/unittools.c 2003-11-05 08:05:52.000000000 +0100
+++ freeciv/server/unittools.c  2003-11-08 10:07:54.000000000 +0100
@@ -1322,41 +1322,34 @@
   }
 }
 
+
 /**************************************************************************
-  When in civil war or an alliance breaks there will potentially be units 
-  from both sides coexisting on the same squares.  This routine resolves 
-  this by first bouncing off non-allied units from their cities, then by 
-  bouncing both players' units in now illegal multiowner stacks.  To avoid
-  drowning due to removal of transports, we bounce everyone (including
-  third parties' units) from ocean tiles.
+  Throw pplayer's units from non allied cities
 
-  If verbose is true, the unit owner gets messages about where each
-  units goes.
+  If verbose is true, pplayer gets messages about where each units goes.
 **************************************************************************/
-void resolve_unit_stacks(struct player *pplayer, struct player *aplayer,
-                         bool verbose)
+static void throw_units_from_illegal_cities(struct player *pplayer,
+                                           bool verbose)
 {
-  /* Throw aplayer's units out of pplayer's cities */
-  city_list_iterate(pplayer->cities, pcity) {
-    struct unit *punit;
-    while ((punit = is_non_allied_unit_tile(map_get_tile(pcity->x, pcity->y),
-                                            pplayer))) {
+  unit_list_iterate_safe(pplayer->units, punit) {
+    struct tile *ptile = map_get_tile(punit->x, punit->y);
+    if (ptile->city && !pplayers_allied(city_owner(ptile->city), pplayer)) {
       bounce_unit(punit, verbose);
     }
-  } city_list_iterate_end;
+  } unit_list_iterate_safe_end;    
+}
 
-  /* Throw pplayer's units out of aplayer's cities */
-  city_list_iterate(aplayer->cities, pcity) {
-    struct unit *punit;
-    while ((punit = is_non_allied_unit_tile(map_get_tile(pcity->x, pcity->y),
-                                            aplayer))) {
-      bounce_unit(punit, verbose);
-    }
-  } city_list_iterate_end;
+/**************************************************************************
+  For each pplayer's unit, check if we stack illegally, if so,
+  bounce both players' units. If on ocean tile, bounce everyone
+  to avoid drowning. This function assumes that cities are clean.
 
-  /* Now cities are clean - do non-city stacks. For each unit, check
-   * if we stack illegally, if so, bounce both players' units. If
-   * on ocean tile, bounce everyone to avoid drowning. */
+  If verbose is true, the unit owner gets messages about where each
+  units goes.
+**************************************************************************/
+static void resolve_stack_conflicts(struct player *pplayer,
+                                    struct player *aplayer, bool verbose)
+{
   unit_list_iterate_safe(pplayer->units, punit) {
     int x = punit->x, y = punit->y;
     struct tile *ptile = map_get_tile(x, y);
@@ -1372,6 +1365,27 @@
     }    
   } unit_list_iterate_safe_end;
 }
+                               
+/**************************************************************************
+  When in civil war or an alliance breaks there will potentially be units 
+  from both sides coexisting on the same squares.  This routine resolves 
+  this by first bouncing off non-allied units from their cities, then by 
+  bouncing both players' units in now illegal multiowner stacks.  To avoid
+  drowning due to removal of transports, we bounce everyone (including
+  third parties' units) from ocean tiles.
+
+  If verbose is true, the unit owner gets messages about where each
+  units goes.
+**************************************************************************/
+void resolve_unit_stacks(struct player *pplayer, struct player *aplayer,
+                         bool verbose)
+{
+  throw_units_from_illegal_cities(pplayer, verbose);
+  throw_units_from_illegal_cities(aplayer, verbose);
+  
+  resolve_stack_conflicts(pplayer, aplayer, verbose);
+  resolve_stack_conflicts(aplayer, pplayer, verbose);
+}
 
 /**************************************************************************
 ...