<URL: http://bugs.freeciv.org/Ticket/Display.html?id=39556 >

Marko Lindqvist wrote:
>  Isn't that obvious from the caller?
> 
>  It's the tech making wonder obsole, probably none. Is
> buildings.ruleset loading code up to your iterator changes?
> 
Too many commits since your line numbers, I was looking at the wrong line.

Yes, that test should be done as pointers.  Fixed.

Moved the test to be first (before checking for wonder), as that quick
test obviates the need for all of the rest.

Also, the test for A_FUTURE should be *before* checking state.  It says so
in the comments, but was backward in the assert!

Also, checked for every instance of obsolete_by, found another case that
wasn't checked for validity before use -- in gui-mui, and the line before
it won't compile, either.  So, I just flagged it with FIXME.

Then, after committing those fixes, it seemed to me that we shouldn't
bother running the loop for A_FUTURE (or other invalid) advances.  So,
a quick one line commit to fix it.

Ha, identified four bugs with one assert!  Keep up the good work.

Index: server/techtools.c
===================================================================
--- server/techtools.c  (revision 13297)
+++ server/techtools.c  (working copy)
@@ -270,13 +270,13 @@
   bool can_switch[MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS]
                  [government_count()];
   struct player_research *research = get_player_research(plr);
+  struct advance *vap = valid_advance_by_number(tech_found);
 
   /* HACK: A_FUTURE doesn't "exist" and is thus not "available".  This may
    * or may not be the correct thing to do.  For these sanity checks we
    * just special-case it. */
-  assert((valid_advance_by_number(tech_found)
-         && player_invention_state(plr, tech_found) != TECH_KNOWN)
-        || tech_found == A_FUTURE);
+  assert(tech_found == A_FUTURE
+        || (vap && player_invention_state(plr, tech_found) != TECH_KNOWN));
 
   /* got_tech allows us to change research without applying techpenalty
    * (without loosing bulbs) */
@@ -290,9 +290,9 @@
   if (was_first) {
     /* Alert the owners of any wonders that have been made obsolete */
     improvement_iterate(pimprove) {
-      if (is_great_wonder(pimprove)
+      if (vap == pimprove->obsolete_by
+         && is_great_wonder(pimprove)
          && great_wonder_was_built(pimprove)
-         && advance_number(pimprove->obsolete_by) == tech_found
          && (pcity = find_city_from_great_wonder(pimprove))) {
        notify_player(city_owner(pcity), NULL, E_WONDER_OBSOLETE,
                         _("Discovery of %s OBSOLETES %s in %s!"), 
Index: client/gui-mui/helpdlg.c
===================================================================
--- client/gui-mui/helpdlg.c    (revision 13297)
+++ client/gui-mui/helpdlg.c    (working copy)
@@ -642,8 +642,8 @@
     DoMethod(help_wonder_cost_text, MUIM_SetAsString,
             MUIA_Text_Contents, "%ld", impr_build_shield_cost(imp));
 
-    UpdateTechButton(help_wonder_needs_button, advance_number(imp->tech_req));
-    UpdateTechButton(help_wonder_obsolete_button, 
advance_number(imp->obsolete_by));
+    UpdateTechButton(help_wonder_needs_button, FIXME 
advance_number(imp->tech_req));
+    UpdateTechButton(help_wonder_obsolete_button, FIXME 
advance_number(imp->obsolete_by));
   }
   else
   {
Index: server/techtools.c
===================================================================
--- server/techtools.c  (revision 13299)
+++ server/techtools.c  (working copy)
@@ -287,7 +287,7 @@
   research->techs_researched++;
   was_first = (!game.info.global_advances[tech_found]);
 
-  if (was_first) {
+  if (was_first && vap) {
     /* Alert the owners of any wonders that have been made obsolete */
     improvement_iterate(pimprove) {
       if (vap == pimprove->obsolete_by
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

Reply via email to