[Freeciv-Dev] (PR#40167) Client crash on scenario load

2008-03-24 Thread Marko Lindqvist

http://bugs.freeciv.org/Ticket/Display.html?id=40167 >

 With #40166 fixing server crash, scenario load now ends to client
crash. Once I enter game, there's only explorer unit visible on
otherwise black screen. It moves around (autoexplore in the beginning
of the first turn?) and then client segfaults in somewhat random
portion of code (but there seems to be invalid terrain pointer always
involved)

 ... just quick check ...

 All tiles seem to have NULL terrain and resource.



 - ML



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


Re: [Freeciv-Dev] (PR#40166) Failing assert on scenario load

2008-03-24 Thread Marko Lindqvist

http://bugs.freeciv.org/Ticket/Display.html?id=40166 >

On 24/03/2008, Marko Lindqvist  wrote:
>
>  On 24/03/2008, Marko Lindqvist wrote:
>  >
>  >   Current S2_2 svn, trying to start Tutorial, results in server crash.
>  >
>  >  rand.c:87: myrand_debug: Assertion `rand_state.is_init' failed.
>
>   This myrand() -call originates from savegame.c:3983 call to
>  randomize_base64url_string().

 Fix attached. Not that this alone is enough to allow scenario loading.

>   I think we need to make separate rand to be used outside actual game 
> context.

 Leaving that to future tickets.


  - ML

diff -Nurd -X.diff_ignore freeciv/server/savegame.c freeciv/server/savegame.c
--- freeciv/server/savegame.c	2008-03-12 21:58:28.0 +0200
+++ freeciv/server/savegame.c	2008-03-24 18:50:55.0 +0200
@@ -3978,11 +3978,9 @@
"game.scoreturn");
 sz_strlcpy(server.game_identifier,
secfile_lookup_str_default(file, "", "game.id"));
-if (0 == strlen(server.game_identifier)
- || !is_base64url(server.game_identifier)) {
-  randomize_base64url_string(server.game_identifier,
- sizeof(server.game_identifier));
-}
+/* We are not checking game_identifier legality just yet.
+ * That's done when we are sure that rand seed has been initialized,
+ * so that we can generate new game_identifier, if needed. */
 
 game.info.fogofwar = secfile_lookup_bool_default(file, FALSE, "game.fogofwar");
 game.fogofwar_old = game.info.fogofwar;
@@ -4229,6 +4227,12 @@
 }
   }
 
+  if (0 == strlen(server.game_identifier)
+  || !is_base64url(server.game_identifier)) {
+/* This uses myrand(), so random state has to be initialized before this. */
+randomize_base64url_string(server.game_identifier,
+   sizeof(server.game_identifier));
+  }
 
   game.info.is_new_game = !secfile_lookup_bool_default(file, TRUE,
 		  "game.save_players");
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#40166) Failing assert on scenario load

2008-03-24 Thread Marko Lindqvist

http://bugs.freeciv.org/Ticket/Display.html?id=40166 >

On 24/03/2008, Marko Lindqvist wrote:
>
>   Current S2_2 svn, trying to start Tutorial, results in server crash.
>
>  rand.c:87: myrand_debug: Assertion `rand_state.is_init' failed.

 This myrand() -call originates from savegame.c:3983 call to
randomize_base64url_string().

 I'm not happy with that call even in cases where rand is already
initialized. Shouldn't random URL differ even between games with same
gameseed setting?
 I think we need to make separate rand to be used outside actual game context.


 - ML



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


Re: [Freeciv-Dev] (PR#39668) [Patch] Fix some potential crashes after script_signal_emit()

2008-03-24 Thread Marko Lindqvist

http://bugs.freeciv.org/Ticket/Display.html?id=39668 >

On 04/09/2007, Marko Lindqvist  wrote:
>
>   Calling script_signal_emit() means that almost anything can happen.
>  Most definitely cities or units can disappear.

 Actually, they can't disappear just yet. Scripting improvements allowing
city destruction and unit death are waiting for this ticket.

>   I just glanced where it has been called, and fixed most obvious cases
>  where some now potentially invalid pointer was used immediately after
>  script_signal_emit().

 Updated against current S2_2 svn. Untested, as #40166 prevented testing.


 - ML

diff -Nurd -X.diff_ignore freeciv/common/city.c freeciv/common/city.c
--- freeciv/common/city.c	2008-03-12 21:58:28.0 +0200
+++ freeciv/common/city.c	2008-03-24 16:14:28.0 +0200
@@ -2592,3 +2592,17 @@
   memset(pcity, 0, sizeof(*pcity)); /* ensure no pointers remain */
   free(pcity);
 }
+
+/**
+  Check if city with given id still exist. Use this before using
+  old city pointers when city might have disappeared.
+**/
+bool city_exist(int id)
+{
+  /* Check if city exist in game */
+  if (game_find_city_by_number(id)) {
+return TRUE;
+  }
+
+  return FALSE;
+}
diff -Nurd -X.diff_ignore freeciv/common/city.h freeciv/common/city.h
--- freeciv/common/city.h	2008-03-12 21:58:28.0 +0200
+++ freeciv/common/city.h	2008-03-24 15:40:04.0 +0200
@@ -610,6 +610,8 @@
 			 int *pollu_prod, int *pollu_pop, int *pollu_mod);
 int city_pollution(const struct city *pcity, int shield_total);
 
+bool city_exist(int id);
+
 /*
  * Iterates over all improvements, skipping those not yet built in the
  * given city.
diff -Nurd -X.diff_ignore freeciv/server/cityturn.c freeciv/server/cityturn.c
--- freeciv/server/cityturn.c	2008-03-12 21:58:27.0 +0200
+++ freeciv/server/cityturn.c	2008-03-24 16:01:55.0 +0200
@@ -584,6 +584,7 @@
   struct tile *pcenter = city_tile(pcity);
   struct player *powner = city_owner(pcity);
   struct impr_type *pimprove = pcity->production.value.building;
+  int saved_id = pcity->id;
 
   if (!city_can_grow_to(pcity, pcity->size + 1)) { /* need improvement */
 if (get_current_construction_bonus(pcity, EFT_SIZE_ADJ, RPT_CERTAIN) > 0
@@ -655,8 +656,10 @@
 pcity->size);
   script_signal_emit("city_growth", 2,
 		  API_TYPE_CITY, pcity, API_TYPE_INT, pcity->size);
-
-  sanity_check_city(pcity);
+  if (city_exist(saved_id)) {
+/* Script didn't destroy this city */
+sanity_check_city(pcity);
+  }
   sync_cities();
 }
 
@@ -781,14 +784,30 @@
   struct universal target;
   bool success = FALSE;
   int i;
+  int saved_id = pcity->id;
+  bool city_checked = TRUE; /* This is used to avoid spurious city_exist() calls */
 
   if (worklist_is_empty(&pcity->worklist))
 /* Nothing in the worklist; bail now. */
 return FALSE;
 
   i = 0;
-  while (!success && worklist_peek_ith(&pcity->worklist, &target, i++)) {
-success = can_city_build_now(pcity, target);
+  while (!success) {
+
+if (!city_checked) {
+  if (!city_exist(saved_id)) {
+/* Some script has removed useless city that cannot build
+ * what it is told to! */
+return FALSE;
+  }
+  city_checked = TRUE;
+}
+
+if (worklist_peek_ith(&pcity->worklist, &target, i++)) {
+  success = can_city_build_now(pcity, target);
+} else {
+  success = FALSE;
+}
 if (success) {
   break; /* while */
 }
@@ -811,6 +830,7 @@
 			   API_TYPE_UNIT_TYPE, ptarget,
 			   API_TYPE_CITY, pcity,
 			   API_TYPE_STRING, "need_tech");
+city_checked = FALSE;
 	break;
   }
   success = can_city_build_unit_later(pcity, pupdate);
@@ -829,8 +849,13 @@
 			   API_TYPE_UNIT_TYPE, ptarget,
 			   API_TYPE_CITY, pcity,
 			   API_TYPE_STRING, "never");
-	/* Purge this worklist item. */
-	worklist_remove(&pcity->worklist, --i);
+if (city_exist(saved_id)) {
+  city_checked = TRUE;
+  /* Purge this worklist item. */
+  worklist_remove(&pcity->worklist, --i);
+} else {
+  city_checked = FALSE;
+}
   } else {
 	/* Yep, we can go after pupdate instead.  Joy! */
 	notify_player(pplayer, pcity->tile, E_WORKLIST,
@@ -860,9 +885,13 @@
 			   API_TYPE_BUILDING_TYPE, ptarget,
 			   API_TYPE_CITY, pcity,
 			   API_TYPE_STRING, "never");
-
-	/* Purge this worklist item. */
-	worklist_remove(&pcity->worklist, --i);
+if (city_exist(saved_id)) {
+  city_checked = TRUE;
+  /* Purge this worklist item. */
+  worklist_remove(&pcity->worklist, --i);
+} else {
+  city_checked = FALSE;
+}
 	break;
   }
 
@@ -1012,7 +1041,16 @@
 	};
 	break;
 	  }
+
+  /* Almost all cases emit signal in the end, so city check needed. */
+  if (!city_exist(saved_id)) {
+  

[Freeciv-Dev] (PR#40166) Failing assert on scenario load

2008-03-24 Thread Marko Lindqvist

http://bugs.freeciv.org/Ticket/Display.html?id=40166 >

  Current S2_2 svn, trying to start Tutorial, results in server crash.

rand.c:87: myrand_debug: Assertion `rand_state.is_init' failed.


 - ML



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


Re: [Freeciv-Dev] (PR#35708) [Patch] Triremes cannot enter Deep Ocean

2008-03-24 Thread Marko Lindqvist

http://bugs.freeciv.org/Ticket/Display.html?id=35708 >

On 11/02/2007, Marko Lindqvist  wrote:
>
>   Untested patch to limit Triremes to Ocean tiles. Applies on top of
>  Deep Ocean patch #34265.
>   Also, Triremes do not get move bonus from Nuclear Power.

  Updated against S2_2 svn.

 There is no longer dependency to #34265 (as ruleset already has more
than one ocean type).

 This one contains only ruleset changes. I'll commit this one first.

 Removing support for old style triremes from the codebase (Jason's
patch) is a separate issue - and I'd like to hear comments from
modpack authors before doing that.


 - ML

diff -Nurd -X.diff_ignore freeciv/data/default/effects.ruleset freeciv/data/default/effects.ruleset
--- freeciv/data/default/effects.ruleset	2008-01-15 04:53:36.0 +0200
+++ freeciv/data/default/effects.ruleset	2008-03-24 14:23:52.0 +0200
@@ -905,6 +905,15 @@
   "UnitClass", "Sea", "Local"
 }
 
+[effect_coastal_defense_trireme]
+name	= "Defend_Bonus"
+value	= 100
+reqs	=
+{ "type", "name", "range"
+  "Building", "Coastal Defense", "City"
+  "UnitClass", "Trireme", "Local"
+}
+
 [effect_colosseum]
 name	= "Make_Content"
 value	= 3
@@ -1267,6 +1276,24 @@
   "UnitClass", "Sea", "Local"
 }
 
+[effect_port_facility_trireme]
+name	= "Veteran_Build"
+value	= 1
+reqs	=
+{ "type", "name", "range"
+  "Building", "Port Facility", "City"
+  "UnitClass", "Trireme", "Local"
+}
+
+[effect_port_facility_trireme_1]
+name	= "HP_Regen"
+value	= 100
+reqs	=
+{ "type", "name", "range"
+  "Building", "Port Facility", "City"
+  "UnitClass", "Trireme", "Local"
+}
+
 [effect_power_plant]
 name	= "Output_Bonus"
 value	= 25
@@ -1766,6 +1793,24 @@
   "UnitClass", "Sea", "Local"
 }
 
+[effect_lighthouse_trireme_move]
+name	= "Move_Bonus"
+value	= 1
+reqs	=
+{ "type", "name", "range"
+  "Building", "Lighthouse", "Player"
+  "UnitClass", "Trireme", "Local"
+}
+
+[effect_lighthouse_trireme_veteran]
+name	= "Veteran_Build"
+value	= 1
+reqs	=
+{ "type", "name", "range"
+  "Building", "Lighthouse", "Player"
+  "UnitClass", "Trireme", "Local"
+}
+
 [effect_magellans_expedition]
 name	= "Move_Bonus"
 value	= 2
@@ -1775,6 +1820,15 @@
   "UnitClass", "Sea", "Local"
 }
 
+[effect_magellan_trireme]
+name	= "Move_Bonus"
+value	= 2
+reqs	=
+{ "type", "name", "range"
+  "Building", "Magellan's Expedition", "Player"
+  "UnitClass", "Trireme", "Local"
+}
+
 [effect_manhattan_project]
 name	= "Enable_Nuke"
 value	= 1
diff -Nurd -X.diff_ignore freeciv/data/default/terrain.ruleset freeciv/data/default/terrain.ruleset
--- freeciv/data/default/terrain.ruleset	2007-10-29 21:10:36.0 +0200
+++ freeciv/data/default/terrain.ruleset	2008-03-24 14:31:02.0 +0200
@@ -206,7 +206,7 @@
 warmer_drier_result  = "no"
 cooler_wetter_result = "no"
 cooler_drier_result  = "no"
-native_to= "Sea", "Air", "Missile", "Helicopter"
+native_to= "Sea", "Air", "Missile", "Helicopter", "Trireme"
 flags= "Oceanic", "NoCities", "NoPollution", "UnsafeCoast"
 property_ocean_depth = 0
 helptext = _("\
@@ -244,7 +244,7 @@
 warmer_drier_result  = "no"
 cooler_wetter_result = "no"
 cooler_drier_result  = "no"
-native_to= "Sea", "Air", "Missile", "Helicopter"
+native_to= "Sea", "Air", "Missile", "Helicopter", "Trireme"
 flags= "Oceanic", "NoCities", "NoPollution"
 property_ocean_depth = 0
 helptext = _("\
@@ -282,7 +282,7 @@
 warmer_drier_result  = "no"
 cooler_wetter_result = "no"
 cooler_drier_result  = "no"
-native_to= "Sea", "Air", "Missile", "Helicopter"
+native_to= "Sea", "Air", "Missile", "Helicopter", "Trireme"
 flags= "Oceanic", "NoCities", "NoPollution"
 property_ocean_depth = 0
 helptext = _("\
@@ -320,7 +320,7 @@
 warmer_drier_result  = "no"
 cooler_wetter_result = "no"
 cooler_drier_result  = "no"
-native_to= "Sea", "Air", "Missile", "Helicopter"
+native_to= "Sea", "Air", "Missile", "Helicopter", "Trireme"
 flags= "Oceanic", "NoCities", "NoPollution"
 property_ocean_depth = 36
 helptext = _("\
@@ -358,7 +358,7 @@
 warmer_drier_result  = "no"
 cooler_wetter_result = "no"
 cooler_drier_result  = "no"
-native_to= "Sea", "Air", "Missile", "Helicopter"
+native_to= "Sea", "Air", "Missile", "Helicopter", "Trireme"
 flags= "Oceanic", "NoCities", "NoPollution", "UnsafeCoast"
 property_ocean_depth = 48
 helptext = _("\
@@ -400,8 +400,8 @@
 flags= "Oceanic", "NoCities", "NoPollution", "UnsafeCoast"; "UnsafeOcean"
 property_ocean_depth = 87
 helptext = _("\
-Oceans cover much of the world, and only sea units (Triremes and\
- other boats) can travel on them.\
+Oceans cover much of the world, and only seaworthy units\
+ can travel on them.\