wmlunits: Handle code paths for missing attributes (#7580)

data/tools/unit_tree/helpers.py

@@ -187,6 +187,7 @@ class WesnothList:
         n = 0
         for terrain in self.parser.get_all(tag="terrain_type"):
             tstring = terrain.get_text_val("string")
+            if tstring is None: continue
             self.terrain_lookup[tstring] = terrain
             n += 1
         return n
@@ -205,6 +206,8 @@ class WesnothList:
         for locale in parser.get_all(tag="locale"):
             isocode = locale.get_text_val("locale")
             name = locale.get_text_val("name")
+            if isocode is None or name is None:
+                continue
             if isocode == "ang_GB":
                 continue
             self.languages_found[isocode] = name
@@ -220,7 +223,7 @@ class WesnothList:
         era.faction_lookup = {}
         for multiplayer_side in era.get_all(tag="multiplayer_side"):
             fid = multiplayer_side.get_text_val("id")
-            if fid == "Random":
+            if fid == "Random" or fid in era.faction_lookup:
                 continue
             era.faction_lookup[fid] = multiplayer_side
             recruit = multiplayer_side.get_text_val("recruit", "").strip()
@@ -284,7 +287,7 @@ class WesnothList:
         # Find all unit types.
         newunits = getall("unit_type") + getall("unit")
         for unit in newunits:
-            uid = unit.get_text_val("id")
+            uid = unit.get_text_val("id", "none")
             unit.id = uid
 
             if unit.get_text_val("do_not_list", "no") not in ["no", "false"] or \
@@ -309,18 +312,18 @@ class WesnothList:
         for race in newraces:
             rid = race.get_text_val("id")
             if rid is None:
-                rid = race.get_text_val("name")
+                rid = race.get_text_val("name", "none")
             self.race_lookup[rid] = race
 
         # Find all movetypes.
         newmovetypes = getall("movetype")
         for movetype in newmovetypes:
             mtname = movetype.get_text_val("name")
+            if mtname is None: continue
             self.movetype_lookup[mtname] = movetype
 
         # Store race/movetype/faction of each unit for easier access later.
         for unit in newunits:
-            uid = unit.get_text_val("id")
             race = self.get_unit_value(unit, "race")
             try:
                 unit.race = self.race_lookup[race]
@@ -360,21 +363,9 @@ class WesnothList:
     def check_units(self):
         """
         Once all units have been added, do some checking.
+        This function used to handle now-removed advancefrom tags
         """
-        # handle advancefrom tags
+        return
-        for uid, unit in list(self.unit_lookup.items()):
-            for advancefrom in unit.get_all(tag="advancefrom"):
-                fromid = advancefrom.get_text_val("unit")
-                if fromid:
-                    try:
-                        fromunit = self.unit_lookup[fromid]
-                    except KeyError:
-                        error_message(
-                            "Error: Unit '%s' references non-existant [advancefrom] unit '%s'" % (
-                                uid, fromid))
-                        continue
-                    if uid not in fromunit.advance:
-                        fromunit.advance.append(uid)
 
     def find_unit_factions(self):
         for unit in list(self.unit_lookup.values()):
@@ -503,7 +494,7 @@ class UnitNode:
     def __init__(self, unit):
         self.unit = unit
         self.children = []
-        self.id = unit.get_text_val("id")
+        self.id = unit.get_text_val("id", "none")
         self.child_ids = []
         self.parent_ids = []
         self.child_ids.extend(unit.advance)

data/tools/unit_tree/html_output.py

@@ -665,7 +665,7 @@ class HTMLOutput:
                     name = self.wesnoth.get_unit_value(un, "name",
                                                        translation=self.translation.translate)
                     if not name:
-                        error_message("Warning: Unit uid=" + uid + " has no name.\n")
+                        error_message("Warning: Unit uid=%s has no name.\n" % uid)
                         name = uid
                     add_menuitem(link, name)
                 end_menu()
@@ -758,7 +758,7 @@ class HTMLOutput:
                 try:
                     id = ability.get_text_val("id")
                 except AttributeError as e:
-                    error_message("Error: Ignoring ability " + ability.debug())
+                    error_message("Error: Ignoring ability %s" % ability.debug())
                     continue
                 if id in already:
                     continue
@@ -1299,7 +1299,7 @@ class HTMLOutput:
             if ticon:
                 terrainlist.append((name, tid, ticon))
             else:
-                error_message("Terrain " + tid + " has no symbol_image\n")
+                error_message("Terrain %s has no symbol_image\n" % tid)
         terrainlist.sort()
 
         for tname, tid, ticon in terrainlist:
@@ -1340,7 +1340,7 @@ class HTMLOutput:
 
             write('<tr>\n')
             picname = image_collector.add_image(self.addon,
-                                                "terrain/" + ticon + ".png",
+                                                "terrain/%s.png" % ticon,
                                                 no_tc=True)
             icon = os.path.join(PICS_LOCATION, picname)
             write('<td><img src="%s" alt="(icon)" /></td>\n' % cleanurl(icon))
@@ -1368,9 +1368,9 @@ def generate_campaign_report(addon, isocode, campaign, wesnoth):
     else:
         cid = "mainline"
     if not cid:
-        cid = addon + "_" + campaign.get_text_val("define")
+        cid = "%s_%s" % (addon, campaign.get_text_val("define"))
 
-    print(("campaign " + addon + " " + cid + " " + isocode))
+    print("campaign %s %s %s" % (addon, cid, isocode))
 
     path = os.path.join(options.output, addon, isocode)
     if not os.path.isdir(path):
@@ -1396,7 +1396,7 @@ def generate_campaign_report(addon, isocode, campaign, wesnoth):
 def generate_era_report(addon, isocode, era, wesnoth):
     eid = era.get_text_val("id")
 
-    print("era " + addon + " " + eid + " " + isocode)
+    print("era %s %s %s" % (addon, eid, isocode))
 
     path = os.path.join(options.output, addon, isocode)
     if not os.path.isdir(path):

data/tools/unit_tree/wiki_output.py

@@ -16,29 +16,28 @@ def main():
 
     punits = {}
 
-    defines = "NORMAL,ENABLE_ARMAGEDDON_DRAKE,ENABLE_DWARVISH_ARCANISTER," +\
+    base_defines = ["NORMAL"]
-        "ENABLE_DWARVISH_RUNESMITH,ENABLE_ANCIENT_LICH,ENABLE_DEATH_KNIGHT," +\
-        "ENABLE_TROLL_SHAMAN,ENABLE_WOLF_ADVANCEMENT,ENABLE_WOSE_SHAMAN," +\
-        "ENABLE_PARAGON,ENABLE_SAURIAN_SPEARTHROWER,DISABLE_GRAND_MARSHAL"
 
     sys.stderr.write("Parsing core units...\n")
-    wesnoth.parser.parse_text("{core/units.cfg}", defines)
+    wesnoth.parser.parse_text("{core/units.cfg}", ",".join(base_defines))
     punits["mainline"] = wesnoth.parser.get_all(tag = "units")
     punits["mainline"] += wesnoth.parser.get_all(tag = "+units")
 
     all_campaigns = {}
     sys.stderr.write("Parsing campaigns...\n")
-    wesnoth.parser.parse_text("{campaigns}", defines)
+    wesnoth.parser.parse_text("{campaigns}", ",".join(base_defines))
     campaigns = wesnoth.parser.get_all(tag = "campaign")
     for campaign in campaigns:
+        campaign_defines = base_defines[:]
         define = campaign.get_text_val("define")
         ed = campaign.get_text_val("extra_defines")
-        if ed: define += "," + ed
+        if define is not None: campaign_defines.append(define)
+        if ed is not None: campaign_defines.extend(ed.split(","))
         name = campaign.get_text_val("name", translation = translated.translate)
         sys.stderr.write("Parsing " + name + "...\n")
         campaign.name = name
         all_campaigns[campaign.get_text_val("id")] = campaign
-        wesnoth.parser.parse_text("{campaigns}", defines + "," + define)
+        wesnoth.parser.parse_text("{campaigns}", ",".join(campaign_defines))
         punits[name] = wesnoth.parser.get_all(tag = "units")
         punits[name] += wesnoth.parser.get_all(tag = "+units")
 
@@ -47,7 +46,12 @@ def main():
     for campaign, unitslists in list(punits.items()):
         for unitlist in unitslists:
             for race in unitlist.get_all(tag = "race"):
-                races[race.get_text_val("id")] = race
+                race_id = race.get_text_val("id")
+                if race_id is None: race_id = race.get_text_val("name")
+                if race_id is None:
+                    continue
+                else:
+                    races[race_id] = race
 
     # Go through all units and put them into a dictionary.
     all_units = {}
@@ -57,6 +61,8 @@ def main():
                 if unit.get_text_val("do_not_list") in ["yes", "true"]: continue
                 if unit.get_text_val("hide_help") in ["yes", "true"]: continue
                 unit.id = unit.get_text_val("id")
+                if unit.id is None:
+                    continue
                 unit.campaign = campaign
                 all_units[unit.id] = unit
                 unit.children = []
@@ -66,7 +72,10 @@ def main():
         x = unit.get_text_val(val, translation = translation)
         if x: return x
         for base_unit in unit.get_all(tag = "base_unit"):
-            base = all_units[base_unit.get_text_val("id")]
+            base_uid = base_unit.get_text_val("id")
+            if base_uid is None or not base_uid in all_units:
+                continue
+            base = all_units[base_uid]
             x = base_val(base, val, translation = translation)
             if x: return x
         return None
@@ -91,6 +100,8 @@ def main():
     # Find children and parents of all units.
     for unit in list(all_units.values()):
         for aid in unit.advances_to:
+            if not aid in all_units:
+                continue
             unit.children.append(all_units[aid])
             all_units[aid].parents.append(unit)
         # [advancefrom] was removed

data/tools/wmlunits

@@ -314,28 +314,25 @@ def list_contents():
             info["eras"] = list_eras(batchlist, addon)
             info["campaigns"] = list_campaigns(batchlist, addon)
             info["version"] = version
-            sys.stdout.write("ok\n")
+            print("ok")
         except wmlparser3.WMLError as e:
-            ef = open(logname, "w")
+            with open(logname, "w") as ef
                 ef.write("<PARSE ERROR>\n")
                 ef.write(str(e))
-            ef.write("</PARSE ERROR>\n")
+                ef.write("\n</PARSE ERROR>\n")
-            ef.close()
+            print("failed")
-            sys.stdout.write("failed\n")
         except queue.Empty as e:
-            ef = open(logname, "w")
+            with open(logname, "w") as ef
                 ef.write("<TIMEOUT ERROR>\n")
                 ef.write("Failed to parse the WML within " + str(TIMEOUT) + " seconds.")
-            ef.write("</TIMEOUT ERROR>\n")
+                ef.write("\n</TIMEOUT ERROR>\n")
-            ef.close()
+            print("failed")
-            sys.stdout.write("failed\n")
         except Exception as e:
-            ef = open(logname, "w")
+            with open(logname, "w") as ef
                 ef.write("<INTERNAL ERROR>\n")
                 ef.write(repr(e))
-            ef.write("</INTERNAL ERROR>\n")
+                ef.write("\n</INTERNAL ERROR>\n")
-            ef.close()
+            print("failed")
-            sys.stdout.write("failed\n")
         finally:
             move(os.path.join(options.config_dir, "data", "add-ons"), options.addons, addon)
             for d in get_dependencies(addon):
@@ -471,21 +468,29 @@ def batch_process():
                 campaign["units"] = n
 
         except wmlparser3.WMLError as e:
-            ef = open(logname, "a")
+            print("    " + name + " failed")
+            with open(logname, "a") as ef
                 ef.write("<WML ERROR>\n")
                 ef.write(str(e))
-            ef.write("</WML ERROR>\n")
+                ef.write("\n</WML ERROR>\n")
-            ef.close()
+        except (AttributeError, KeyError, TypeError) as e:
+            # Common logic errors.
+            traceback.print_exc()
             print("    " + name + " failed")
+            with open(logname, "a") as ef
+                ef.write("<INTERNAL ERROR>\n")
+                ef.write("please report as bug\n")
+                # Show last few stack frames of traceback to make bug reports more useful.
+                ef.write(traceback.format_exc(limit=-2))
+                ef.write("\n</INTERNAL ERROR>\n")
         except Exception as e:
             traceback.print_exc()
             print("    " + name + " failed")
-            ef = open(logname, "a")
+            with open(logname, "a") as ef
                 ef.write("<INTERNAL ERROR>\n")
                 ef.write("please report as bug\n")
                 ef.write(str(e))
-            ef.write("</INTERNAL ERROR>\n")
+                ef.write("\n</INTERNAL ERROR>\n")
-            ef.close()
         finally:
             if name != "mainline":
                 move(os.path.join(options.config_dir, "data", "add-ons"), options.addons, name)