Repository: AVEgame/AVE

Issue:
Write code to run detailed checks that a game works
Put this in `ave.test`. It can then be used by the AVE pytest tests, and for AVEgame/AVE-usergames#2

Discussion:

Patch:
diff --git a/ave/components/numbers.py b/ave/components/numbers.py
index 19fe14d..e321b64 100644
--- a/ave/components/numbers.py
+++ b/ave/components/numbers.py
@@ -10,6 +10,10 @@ class Number:
         """Get the value of the Number."""
         raise NotImplementedError()
 
+    def get_all_variables(self):
+        """Get a list of all variables used in this expression."""
+        raise NotImplementedError()
+
 
 class Constant(Number):
     """A constant."""
@@ -22,6 +26,10 @@ class Constant(Number):
         """Get the value of the Number."""
         return self.value
 
+    def get_all_variables(self):
+        """Get a list of all variables used in this expression."""
+        return []
+
 
 class Sum(Number):
     """The sum of multiple Numbers."""
@@ -34,6 +42,13 @@ class Sum(Number):
         """Get the value of the Number."""
         return sum(i.get_value(character) for i in self.items)
 
+    def get_all_variables(self):
+        """Get a list of all variables used in this expression."""
+        out = []
+        for i in self.items:
+            out += i.get_all_variables()
+        return out
+
 
 class Product(Number):
     """The product of multiple Numbers."""
@@ -49,6 +64,13 @@ class Product(Number):
             out *= i.get_value(character)
         return out
 
+    def get_all_variables(self):
+        """Get a list of all variables used in this expression."""
+        out = []
+        for i in self.items:
+            out += i.get_all_variables()
+        return out
+
 
 class Division(Number):
     """The result of a division."""
@@ -65,6 +87,13 @@ class Division(Number):
             out /= i.get_value(character)
         return out
 
+    def get_all_variables(self):
+        """Get a list of all variables used in this expression."""
+        out = []
+        for i in self.items:
+            out += i.get_all_variables()
+        return out
+
 
 class Negative(Number):
     """The negative of another Number."""
@@ -77,6 +106,10 @@ class Negative(Number):
         """Get the value of the Number."""
         return -self.item.get_value(character)
 
+    def get_all_variables(self):
+        """Get a list of all variables used in this expression."""
+        return self.item.get_all_variables()
+
 
 class Variable(Number):
     """The value of a variable."""
@@ -89,6 +122,10 @@ class Variable(Number):
         """Get the value of the Number."""
         return character.numbers[self.item]
 
+    def get_all_variables(self):
+        """Get a list of all variables used in this expression."""
+        return [self.item]
+
 
 class Random(Number):
     """A random number."""
@@ -111,3 +148,7 @@ class Random(Number):
         end = self.end.get_value(character)
         size = end - start
         return start + random.random() * size
+
+    def get_all_variables(self):
+        """Get a list of all variables used in this expression."""
+        return self.start.get_all_variables() + self.end.get_all_variables()
diff --git a/ave/components/requirements.py b/ave/components/requirements.py
index 4eb410f..b859c2e 100644
--- a/ave/components/requirements.py
+++ b/ave/components/requirements.py
@@ -10,6 +10,10 @@ class Requirement:
         """Check if the character satisifies this."""
         raise NotImplementedError()
 
+    def get_all(self):
+        """Get all items involved in this requirement."""
+        raise NotImplementedError()
+
 
 class RequiredItem(Requirement):
     """The character must have an item."""
@@ -22,6 +26,10 @@ class RequiredItem(Requirement):
         """Check if the character satisifies this."""
         return character.has(self.item)
 
+    def get_all(self):
+        """Get all items involved in this requirement."""
+        return [self.item]
+
 
 class RequiredNumber(Requirement):
     """A numerical variable must satisfy a condition."""
@@ -47,6 +55,10 @@ class RequiredNumber(Requirement):
         if self.sign == "=" or self.sign == "==":
             return v1 == v2
 
+    def get_all(self):
+        """Get all items involved in this requirement."""
+        return self.v1.get_all_variables() + self.v1.get_all_variables()
+
 
 class Or(Requirement):
     """One of a set of Requirements must be satisfied."""
@@ -62,6 +74,13 @@ class Or(Requirement):
                 return True
         return False
 
+    def get_all(self):
+        """Get all items involved in this requirement."""
+        out = []
+        for i in self.items:
+            out += i.get_all()
+        return out
+
 
 class And(Requirement):
     """A set of Requirements must all be satisfied."""
@@ -77,6 +96,13 @@ class And(Requirement):
                 return False
         return True
 
+    def get_all(self):
+        """Get all items involved in this requirement."""
+        out = []
+        for i in self.items:
+            out += i.get_all()
+        return out
+
 
 class Not(Requirement):
     """The negation of another Requirement."""
@@ -89,6 +115,10 @@ class Not(Requirement):
         """Check if the character satisifies this."""
         return not self.item.has(character)
 
+    def get_all(self):
+        """Get all items involved in this requirement."""
+        return self.item.get_all()
+
 
 class Satisfied(Requirement):
     """This requirement is always satisfied."""
@@ -96,3 +126,7 @@ class Satisfied(Requirement):
     def has(self, character):
         """Check if the character satisifies this."""
         return True
+
+    def get_all(self):
+        """Get all items involved in this requirement."""
+        return []


Test patch:
diff --git a/ave/test/__init__.py b/ave/test/__init__.py
new file mode 100644
index 0000000..d09c7cc
--- /dev/null
+++ b/ave/test/__init__.py
@@ -0,0 +1,2 @@
+"""Code used to test games."""
+from .game_validation import check_game  # noqa: F401
diff --git a/ave/test/error_handlers.py b/ave/test/error_handlers.py
new file mode 100644
index 0000000..8c60c30
--- /dev/null
+++ b/ave/test/error_handlers.py
@@ -0,0 +1,55 @@
+"""Error handlers."""
+
+
+class AVEErrorHandler:
+    """Class to store errors found when checking games."""
+
+    def __init__(self, description, error_type, error_value):
+        """Make the error handler."""
+        self.description = description
+        self.error_type = error_type
+        self.error_value = error_value
+
+    def __str__(self):
+        """Return a string describing the error."""
+        return self.error_type + ": " + self.description
+
+
+class AVEFatalError(AVEErrorHandler):
+    """This error is so bad that the game will not run."""
+
+    def __init__(self, description):
+        """Make the error handler."""
+        super().__init__(description, "Fatal Error", 5)
+
+
+class AVEError(AVEErrorHandler):
+    """There is an error but part of the game will still run."""
+
+    def __init__(self, description):
+        """Make the error handler."""
+        super().__init__(description, "Error", 4)
+
+
+class AVEWarning(AVEErrorHandler):
+    """The game will run, but there may is a small problem."""
+
+    def __init__(self, description):
+        """Make the error handler."""
+        super().__init__(description, "Warning", 3)
+
+
+class AVEInfo(AVEErrorHandler):
+    """The game will run, but there maybe is a small problem."""
+
+    def __init__(self, description):
+        """Make the error handler."""
+        super().__init__(description, "Info", 2)
+
+
+class AVENote(AVEErrorHandler):
+    """The game will run, but there maybe is a tiny problem."""
+
+    def __init__(self, description):
+        """Make the error handler."""
+        super().__init__(description, "Note", 1)
diff --git a/ave/test/game_validation.py b/ave/test/game_validation.py
new file mode 100644
index 0000000..34a5b1e
--- /dev/null
+++ b/ave/test/game_validation.py
@@ -0,0 +1,127 @@
+"""Check that games are valid."""
+from .error_handlers import (
+    AVEFatalError, AVEError, AVEWarning, AVEInfo, AVENote)
+from ..game import Character
+
+
+def check_game(game):
+    """Check a game for errors."""
+    errors = []
+    errors += check_first_room(game)
+    errors += get_inaccessible_rooms(game)
+    errors += get_undefined_rooms(game)
+    errors += get_trapped_rooms(game)
+    errors += get_undefined_numbers(game)
+    errors += explore_items(game)
+    return errors
+
+
+def check_first_room(game):
+    """Check that the first room of the game works."""
+    errors = []
+    if "start" not in game.rooms:
+        return [AVEFatalError("There is no 'start' room.")]
+    if len(game.rooms["start"].options) == 0:
+        errors.append(AVEFatalError("You cannot leave the first room."))
+    c = Character()
+    c.reset(game.items)
+    try:
+        game["start"].get_text(c)
+    except:  # noqa: E722
+        errors.append(AVEFatalError("Could not load text for first room."))
+    try:
+        game["start"].get_options(c)
+    except:  # noqa: E722
+        errors.append(AVEFatalError("Could not load options for first room."))
+    return errors
+
+
+def get_inaccessible_rooms(game):
+    """Find room that cannot be reached."""
+    ach = {"start"}
+    for room in game.rooms.values():
+        for option in room.options:
+            for d in option.get_all_destinations():
+                ach.add(d)
+    return [AVEWarning("The room '" + i + "' is not accessible.")
+            for i in game.rooms if i not in ach]
+
+
+def get_undefined_rooms(game):
+    """Get rooms that can be reached but are not defined."""
+    not_inc = set()
+    for room in game.rooms.values():
+        for option in room.options:
+            for d in option.get_all_destinations():
+                if d == "__GAMEOVER__" or d == "__WINNER__":
+                    continue
+                if d not in game.rooms:
+                    not_inc.add(d)
+    return [AVEError("The room '" + i + "' does not exist.")
+            for i in not_inc]
+
+
+def get_trapped_rooms(game):
+    """Get rooms that there is no option to leave."""
+    return [AVEError("There is no way to leave the room '" + id + "'.")
+            for id, room in game.rooms.items() if len(room.options) == 0]
+
+
+def get_undefined_numbers(game):
+    """Get numbers that can be added to but are not defined."""
+    not_def = set()
+    c = Character()
+    c.reset(game.items)
+    for room in game.rooms.values():
+        for thing in room.options + room.text:
+            for item in thing.items:
+                if not c.is_number(item.item) and item.value.get_value(c) != 1:
+                    not_def.add(item.item)
+    return [AVEWarning("The game wants to add a number to '" + i + "',"
+                       " but it is not a variable.") for i in not_def]
+
+
+def explore_items(game):
+    """Look for items that are required but never added and other issues."""
+    used = set()
+    asked_for = set()
+    c = Character()
+    c.reset(game.items)
+    for room in game.rooms.values():
+        for thing in room.options + room.text:
+            for item in thing.items:
+                used.add(item.item)
+            for item in thing.needs.get_all():
+                asked_for.add(item)
+
+    used_num = set()
+    numbers = set()
+    named_items = set()
+    for i in game.items:
+        if c.is_number(i):
+            numbers.add(i)
+            if i.default.get_value(c) != 0:
+                used_num.add(i)
+        else:
+            named_items.add(i)
+
+    errors = []
+    errors += [AVEWarning("A line in your file requires '" + i + "',"
+                          " but this item is never added.")
+               for i in asked_for - used - used_num]
+    errors += [AVEWarning("A line in your file asks about '" + i + "',"
+                          " but this number is never changed.")
+               for i in (asked_for - used).intersection(numbers)]
+    errors += [AVEInfo("The number '" + i + "' is defined but is not "
+                       "used in the game.")
+               for i in numbers - used - asked_for]
+    errors += [AVEInfo("The item '" + i + "' is named but is not "
+                       "used in the game.")
+               for i in named_items - used - asked_for]
+    errors += [AVENote("The item '" + i + "' is not named, so will be hidden "
+                       "from the inventory by default.")
+               for i in used.union(asked_for) - named_items - numbers]
+    return errors
+
+
+# Info: unnamed items will default to hidden
diff --git a/test/test_games.py b/test/test_games.py
index 78f2eb0..0020bf0 100644
--- a/test/test_games.py
+++ b/test/test_games.py
@@ -1,7 +1,8 @@
 import pytest
 import os
-from ave import config, AVE, Character, exceptions
+from ave import config, AVE, exceptions
 from ave import load_game_from_file, load_game_from_library
+from ave.test import check_game
 
 config.debug = True
 games_path = os.path.join(
@@ -30,58 +31,31 @@ def test_version_checking(filename):
 
 
 @pytest.mark.parametrize('filename', games)
-def test_all_rooms_acessible(filename):
+def test_games_for_errors(filename):
     game = load_game_from_file(filename)
     game.load()
-    ach = {"start"}
-    for room in game.rooms.values():
-        for option in room.options:
-            for d in option.get_all_destinations():
-                ach.add(d)
-    not_ach = [i for i in game.rooms if i not in ach]
 
-    if len(not_ach) > 0:
-        print("Rooms not accessible:")
-        for key in not_ach:
-            print(" ", key)
-        assert False
-
-
-@pytest.mark.parametrize('filename', games)
-def test_all_rooms_defined(filename):
-    game = load_game_from_file(filename)
-    game.load()
-    not_inc = set()
-    for room in game.rooms.values():
-        for option in room.options:
-            for d in option.get_all_destinations():
-                if d == "__GAMEOVER__" or d == "__WINNER__":
-                    continue
-                if d not in game.rooms:
-                    not_inc.add(d)
-
-    if len(not_inc) > 0:
-        print("Rooms not defined:")
-        for key in not_inc:
-            print(" ", key)
-        assert "test.ave" in filename
-        assert len(not_inc) == 1 and "fakeroom" in not_inc
-
-
-@pytest.mark.parametrize('filename', games)
-def test_has_start(filename):
-    game = load_game_from_file(filename)
-    game.load()
-    assert game["start"].id != "fail"
-
-
-@pytest.mark.parametrize('filename', games)
-def test_first_room(filename):
-    ave = AVE(character=Character())
-    game = load_game_from_file(filename)
-    game.load()
-    game["start"].get_text(ave.character)
-    game["start"].get_options(ave.character)
+    issues = check_game(game)
+    errors = [i for i in issues if i.error_value > 3]
+    info = [i for i in issues if i.error_value <= 3]
+    errors.sort(key=lambda e: -e.error_value)
+    info.sort(key=lambda e: -e.error_value)
+
+    if len(errors) > 0:
+        print("\n  " + str(len(errors)) + " errors(s) in " + filename)
+        for e in errors:
+            print(e)
+    if len(info) > 0:
+        print("\n  " + str(len(info)) + " info(s) in " + filename)
+        for e in info:
+            print(e)
+
+    if filename.endswith("test.ave"):
+        assert len(errors) == 1
+        assert "fakeroom" in errors[0].description
+    else:
+        # Assert that the worst error is a Warning or lower
+        assert len(errors) == 0
 
 
 def test_game_library():