Re: [Freeciv-Dev] (PR#40261) Homecity change assert failure

2008-06-09 Thread Marko Lindqvist

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40261 

2008/6/8 Marko Lindqvist:

 S2_1:
 unithand.c:362: real_unit_change_homecity: Assertion `!unit_alive ||
 unit_owner(punit) == city_owner(new_pcity)' failed.

 I have not yet managed to reproduce.

 I remember tracking similar assert() last fall, and it turned out to
 be impossible to fix without major rewrites.

 I think it's always a bug that passenger may conquer cities. Consider
ruleset where some unit can act both as transporter and unit occupying
enemy cities. If such unit is carrying allied unit and tries to occupy
enemy city, city will end under ally's control.

 Fixing that so that passengers never occupy cities also avoids above
crash (which is caused by enemy unit on board conquering city when
transport arrives and then city is no longer owned by transport owner.
Enemy unit on board is not a bug during civil war)

 Patch attached.


 - ML

diff -Nurd -X.diff_ignore freeciv/server/citytools.c freeciv/server/citytools.c
--- freeciv/server/citytools.c  2008-05-02 20:18:11.0 +0300
+++ freeciv/server/citytools.c  2008-06-09 17:46:34.0 +0300
@@ -1259,9 +1259,13 @@
 }
 
 /**
-...
+  Handle unit entering city. During peace may enter peacefully, during
+  war conquers city.
+  Transported unit cannot conquer city. If transported unit is seen here,
+  transport is conquering city. Movement of the transported units is just
+  handled before transport itself.
 **/
-void unit_enter_city(struct unit *punit, struct city *pcity)
+void unit_enter_city(struct unit *punit, struct city *pcity, bool passenger)
 {
   bool do_civil_war = FALSE;
   int coins;
@@ -1272,7 +1276,7 @@
   /* If not at war, may peacefully enter city. Or, if we cannot occupy
* the city, this unit entering will not trigger the effects below. */
   if (!pplayers_at_war(pplayer, cplayer)
-  || !COULD_OCCUPY(punit)) {
+  || !COULD_OCCUPY(punit) || passenger) {
 return;
   }
 
diff -Nurd -X.diff_ignore freeciv/server/citytools.h freeciv/server/citytools.h
--- freeciv/server/citytools.h  2008-03-10 20:08:03.0 +0200
+++ freeciv/server/citytools.h  2008-06-09 17:47:00.0 +0300
@@ -45,7 +45,7 @@
 struct tile *ptile,
 bool sea_required,
 struct city *pexclcity);
-void unit_enter_city(struct unit *punit, struct city *pcity);
+void unit_enter_city(struct unit *punit, struct city *pcity, bool passenger);
 
 bool send_city_suppression(bool now);
 void send_city_info(struct player *dest, struct city *pcity);
diff -Nurd -X.diff_ignore freeciv/server/unittools.c freeciv/server/unittools.c
--- freeciv/server/unittools.c  2008-05-17 01:27:56.0 +0300
+++ freeciv/server/unittools.c  2008-06-09 17:48:37.0 +0300
@@ -2524,21 +2524,22 @@
doesn't care whether a unit is away or not.
 **/
 static void unit_move_consequences(struct unit *punit,
-  struct tile *src_tile,
-  struct tile *dst_tile)
+   struct tile *src_tile,
+   struct tile *dst_tile,
+   bool passenger)
 {
   struct city *fromcity = tile_city(src_tile);
   struct city *tocity = tile_city(dst_tile);
   struct city *homecity = NULL;
   struct player *pplayer = unit_owner(punit);
   bool refresh_homecity = FALSE;
-  
+
   if (0 != punit-homecity) {
 homecity = game_find_city_by_number(punit-homecity);
   }
 
   if (tocity) {
-unit_enter_city(punit, tocity);
+unit_enter_city(punit, tocity, passenger);
   }
 
   /* We only do this for non-AI players to now make sure the AI turns
@@ -2692,7 +2693,7 @@
   vision_clear_sight(old_vision);
   vision_free(old_vision);
 
-  unit_move_consequences(pcargo, psrctile, pdesttile);
+  unit_move_consequences(pcargo, psrctile, pdesttile, TRUE);
 } unit_list_iterate_end;
 unit_list_unlink_all(cargo_units);
 unit_list_free(cargo_units);
@@ -2818,10 +2819,10 @@
 } players_iterate_end;
   } square_iterate_end;
 
-  unit_move_consequences(punit, psrctile, pdesttile);
+  unit_move_consequences(punit, psrctile, pdesttile, FALSE);
 
   /* FIXME: Should signal emit be after sentried units have been
-   *waken up in case script causes unit death. */
+   *waken up in case script causes unit death? */
   script_signal_emit(unit_moved, 3,
 API_TYPE_UNIT, punit,
 API_TYPE_TILE, psrctile,
diff -Nurd -X.diff_ignore freeciv/server/citytools.c freeciv/server/citytools.c
--- freeciv/server/citytools.c  2008-01-15 04:04:13.0 +0200
+++ freeciv/server/citytools.c  2008-06-09 

Re: [Freeciv-Dev] (PR#40261) Homecity change assert failure

2008-06-09 Thread Marko Lindqvist

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40261 

2008/6/9 Jason Dorje Short:
  Fixing that so that passengers never occupy cities also avoids above
 crash (which is caused by enemy unit on board conquering city when
 transport arrives and then city is no longer owned by transport owner.
 Enemy unit on board is not a bug during civil war)

 Good explanation but how does that cause the assertion?

 In the failing assert
 unit_owner(punit) == city_owner(new_pcity)
 punit is transporter and new_pcity is new homecity transport is moving to.

 really_unit_change_homecity() is called for transporter when another
city is transferred to rebels. In the beginning of the function
transport owner == city owner. Then transport teleports ( move_unit()
) to new homecity and enemy on board conquers it. Now city is owned by
enemy - transport owner != city owner.


 --

 I don't like teleporting units in general, and teleporting with
passengers even less. But the alternative is to sometimes drown all
the passengers when transport teleports.
 And from ruleset author point of view: this allows special unit that
can airlift several others.


 - ML



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#40261) Homecity change assert failure

2008-06-08 Thread Marko Lindqvist

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40261 

S2_1:
unithand.c:362: real_unit_change_homecity: Assertion `!unit_alive ||
unit_owner(punit) == city_owner(new_pcity)' failed.

I have not yet managed to reproduce.

I remember tracking similar assert() last fall, and it turned out to
be impossible to fix without major rewrites.


- ML



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev