From d6cac0c4e7f2261499ab2a4f7a84e7937f8a4e4b Mon Sep 17 00:00:00 2001 From: Steve Cotton Date: Mon, 27 May 2019 20:41:07 +0200 Subject: [PATCH] wmllint: Handle nested events and other things with filters in the death check (closes #4085) The main reason for moving this to a separate function was to make the per-event variables local to that function. (cherry picked from commit 7e69da7f68b10bdf231ebb785b3c73109dc378a8) --- data/tools/wesnoth/wmliterator3.py | 6 +- data/tools/wmllint | 89 +++++++++++++++++++++--------- 2 files changed, 67 insertions(+), 28 deletions(-) diff --git a/data/tools/wesnoth/wmliterator3.py b/data/tools/wesnoth/wmliterator3.py index 0f0b5499fc3..63c8146d501 100644 --- a/data/tools/wesnoth/wmliterator3.py +++ b/data/tools/wesnoth/wmliterator3.py @@ -448,7 +448,11 @@ Important Attributes: return isAttribute(self) def iterScope(self): - """Return an iterator for the current scope""" + """Return an iterator for the current scope. + + If the iterator is currently on a line that opens a new scope, + this returns an iterator of the outer scope not the inner one. + """ if not self.scopes: return WmlIterator(self.lines, self.fname) scopeItor = self.scopes[-1].copy() diff --git a/data/tools/wmllint b/data/tools/wmllint index 190373094f3..6bb4fb9870c 100755 --- a/data/tools/wmllint +++ b/data/tools/wmllint @@ -1345,43 +1345,78 @@ def local_sanity_check(filename, nav, key, prefix, value, comment): def global_sanity_check_events(filename, lines): """Part of global_sanity_check which finds each [event] tag. - todo: handle nested events, and ignore [filter] tags that belong to a - non-event tag. + To handle nested events, this is an multi-pass implementation. This + function will make one complete pass itself. Each subfunction called is + given its own copy of the iterator, to do its own pass on the contents of + the event without changing the iterator for this function. """ - - # Detect units that speak in their death events - filter_subject = None - die_event = False deathcheck = True for nav in WmllintIterator(lines, filename): if "wmllint: deathcheck off" in nav.text: deathcheck = False - continue elif "wmllint: deathcheck on" in nav.text: deathcheck = True - if "[/event]" in nav.text: - filter_subject = None - die_event = False - elif not nav.ancestors(): - continue - elif "[event]" in nav.ancestors(): - parent = nav.ancestors()[-1] - if parent == "[event]": - # Check if it's a death event + # Now the tests. These run even when the opening tag is on a line which + # turns that specific check off, in case the check is re-enabled before + # the closing tag. + if "[event]" in nav.text: + sanity_check_speaks_in_death_event(nav.copy(), deathcheck) + +def sanity_check_speaks_in_death_event(opening_tag, deathcheck): + """Detect units that speak in their death events + + Given an iterator pointing to an [event]'s opening tag, check whether it's + a die event, and if so check whether the already-dead unit speaks during + it. + + This will move the iterator that is passed into it, the caller should make + a copy if required. + + opening_tag -- an iterator on the [event] tag's line + deathcheck -- the status of the global "deathcheck on/off" flag at + the start of this event + """ + filter_subject = None + die_event = False + if not opening_tag.hasNext(): + # Either this is an empty tag, or the closing tag must be + # missing. There will be a separate error reported for that. + return + opening_tag.__next__() + event_scope = opening_tag.iterScope() + base_depth = len(event_scope.ancestors()) + + # First pass - find out if the event is a death event, and if it has a + # filter. Here the base_depth is used to ignore nested events and other + # tags that have a [filter] subtag. + for nav in event_scope.copy(): + ancestors = nav.ancestors() + if len(ancestors) == base_depth + 1: + fields = parse_attribute(nav.text) + if fields: + (key, prefix, value, comment) = fields + if key == 'name' and value == 'die': + die_event = True + if len(ancestors) == base_depth + 2: + if ancestors[-1] == "[filter]": fields = parse_attribute(nav.text) if fields: (key, prefix, value, comment) = fields - if key == 'name' and value == 'die': - die_event = True - elif die_event and not filter_subject and parent == "[filter]": - # Check to see if it has a filter subject - if "id" in nav.text: - try: - (key,prefix,value,comment) = parse_attribute(nav.text) + if key == 'id': filter_subject = value - except TypeError: - pass - elif die_event and filter_subject and parent == "[message]": + + # Second pass - check if the subject speaks in this event. This will + # descend in to nested events, but as the unit is already dead it shouldn't + # speak in those either. + if die_event and filter_subject: + for nav in event_scope: + if "wmllint: deathcheck off" in nav.text: + deathcheck = False + continue + elif "wmllint: deathcheck on" in nav.text: + deathcheck = True + parent = nav.ancestors()[-1] + if parent == "[message]": # Who is speaking? fields = parse_attribute(nav.text) if fields: @@ -1389,7 +1424,7 @@ def global_sanity_check_events(filename, lines): if key in ("id", "speaker"): if deathcheck and ((value == filter_subject) or (value == "unit")): print('"%s", line %d: %s speaks in his/her "die" event rather than "last breath"' \ - % (filename, nav.lineno+1, value)) + % (nav.fname, nav.lineno+1, value)) def global_sanity_check(filename, lines): "Perform sanity and consistency checks on input files."