Repository: missionpinball/mpf

Issue:
Add conditional variables to shots and shot_groups
Expose the current state of shots and shot_groups to conditional events.

Discussion:

Patch:
diff --git a/mpf/core/placeholder_manager.py b/mpf/core/placeholder_manager.py
index 601650f00..29a98ba5a 100644
--- a/mpf/core/placeholder_manager.py
+++ b/mpf/core/placeholder_manager.py
@@ -44,10 +44,11 @@ class BaseTemplate(metaclass=abc.ABCMeta):
 
     """Base class for templates."""
 
-    __slots__ = ["template", "placeholder_manager", "default_value"]
+    __slots__ = ["template", "placeholder_manager", "default_value", "text"]
 
-    def __init__(self, template, placeholder_manger, default_value):
+    def __init__(self, template, text, placeholder_manger, default_value):
         """Initialise template."""
+        self.text = text
         self.template = template
         self.placeholder_manager = placeholder_manger
         self.default_value = default_value
@@ -166,7 +167,7 @@ class RawTemplate(BaseTemplate):
             result = self.placeholder_manager.evaluate_template(self.template, parameters)
         except (ValueError, IndexError):
             if fail_on_missing_params:
-                raise
+                raise ValueError("Error while evaluating: {} with paramters {}".format(self.text, parameters))
             return self.default_value
         return result
 
@@ -301,11 +302,12 @@ class DeviceClassPlaceholder:
 
     """Wrap a monitorable device."""
 
-    __slots__ = ["_devices"]
+    __slots__ = ["_devices", "_device_name"]
 
-    def __init__(self, devices):
+    def __init__(self, devices, device_name):
         """Initialise placeholder."""
         self._devices = devices
+        self._device_name = device_name
 
     def __getitem__(self, item):
         """Array access."""
@@ -315,7 +317,7 @@ class DeviceClassPlaceholder:
         """Attribute access."""
         device = self._devices.get(item)
         if not device:
-            raise AssertionError("Device {} does not exist in placeholders.".format(item))
+            raise AssertionError("Device {} of type {} does not exist.".format(item, self._device_name))
 
         return device.get_monitorable_state()
 
@@ -339,7 +341,7 @@ class DevicesPlaceholder:
         device = self._machine.device_manager.get_monitorable_devices().get(item)
         if not device:
             raise AssertionError("Device Collection {} not usable in placeholders.".format(item))
-        return DeviceClassPlaceholder(device)
+        return DeviceClassPlaceholder(device, item)
 
 
 class ModeClassPlaceholder:
@@ -611,7 +613,7 @@ class BasePlaceholderManager(MpfController):
         else:
             try:
                 ret_value = getattr(slice_value, node.attr)
-            except ValueError:
+            except (ValueError, AttributeError):
                 if subscribe:
                     raise TemplateEvalError(subscription + [slice_value.subscribe_attribute(node.attr)])
                 else:
@@ -662,27 +664,27 @@ class BasePlaceholderManager(MpfController):
         """Build a float template from a string."""
         if isinstance(template_str, (float, int)):
             return NativeTypeTemplate(float(template_str), self.machine)
-        return FloatTemplate(self._parse_template(template_str), self, default_value)
+        return FloatTemplate(self._parse_template(template_str), template_str, self, default_value)
 
     def build_int_template(self, template_str, default_value=0):
         """Build a int template from a string."""
         if isinstance(template_str, (float, int)):
             return NativeTypeTemplate(int(template_str), self.machine)
-        return IntTemplate(self._parse_template(template_str), self, default_value)
+        return IntTemplate(self._parse_template(template_str), template_str, self, default_value)
 
     def build_bool_template(self, template_str, default_value=False):
         """Build a bool template from a string."""
         if isinstance(template_str, bool):
             return NativeTypeTemplate(template_str, self.machine)
-        return BoolTemplate(self._parse_template(template_str), self, default_value)
+        return BoolTemplate(self._parse_template(template_str), template_str, self, default_value)
 
     def build_string_template(self, template_str, default_value=""):
         """Build a string template from a string."""
-        return StringTemplate(self._parse_template(template_str), self, default_value)
+        return StringTemplate(self._parse_template(template_str), template_str, self, default_value)
 
     def build_raw_template(self, template_str, default_value=None):
         """Build a raw template from a string."""
-        return RawTemplate(self._parse_template(template_str), self, default_value)
+        return RawTemplate(self._parse_template(template_str), template_str, self, default_value)
 
     def get_global_parameters(self, name):
         """Return global params."""
diff --git a/mpf/devices/shot.py b/mpf/devices/shot.py
index eac0eef90..4639466ab 100644
--- a/mpf/devices/shot.py
+++ b/mpf/devices/shot.py
@@ -3,6 +3,8 @@ import asyncio
 import uuid
 from copy import copy, deepcopy
 
+from mpf.core.device_monitor import DeviceMonitor
+
 from mpf.core.enable_disable_mixin import EnableDisableMixin
 
 import mpf.core.delays
@@ -12,6 +14,7 @@ from mpf.core.mode_device import ModeDevice
 from mpf.core.player import Player
 
 
+@DeviceMonitor("state", "state_name")
 class Shot(EnableDisableMixin, ModeDevice):
 
     """A device which represents a generic shot."""
diff --git a/mpf/devices/shot_group.py b/mpf/devices/shot_group.py
index 70b376fdd..769428bc9 100644
--- a/mpf/devices/shot_group.py
+++ b/mpf/devices/shot_group.py
@@ -2,6 +2,8 @@
 
 from collections import deque
 
+from mpf.core.device_monitor import DeviceMonitor
+
 from mpf.core.events import event_handler
 from mpf.core.mode import Mode
 from mpf.core.mode_device import ModeDevice
@@ -10,6 +12,7 @@ from mpf.core.system_wide_device import SystemWideDevice
 from mpf.core.utility_functions import Util
 
 
+@DeviceMonitor("common_state")
 class ShotGroup(ModeDevice):
 
     """Represents a group of shots in a pinball machine by grouping together multiple `Shot` class devices.
@@ -51,13 +54,26 @@ class ShotGroup(ModeDevice):
         super().device_removed_from_mode(mode)
         self.machine.events.remove_handler(self._hit)
 
-    def _check_for_complete(self):
-        """Check if all shots in this group are in the same state."""
+    @property
+    def common_state(self):
+        """Return common state if all shots in this group are in the same state.
+
+        Will return None otherwise.
+        """
         state = self.config['shots'][0].state_name
         for shot in self.config['shots']:
             if state != shot.state_name:
                 # shots do not have a common state
-                return
+                return None
+
+        return state
+
+    def _check_for_complete(self):
+        """Check if all shots in this group are in the same state."""
+        state = self.common_state
+        if not state:
+            # shots do not have a common state
+            return
 
         # if we reached this point we got a common state
 


Test patch:
diff --git a/mpf/tests/MpfDocTestCase.py b/mpf/tests/MpfDocTestCase.py
index 2ec7f8a56..7d3243d36 100644
--- a/mpf/tests/MpfDocTestCase.py
+++ b/mpf/tests/MpfDocTestCase.py
@@ -201,6 +201,5 @@ class MpfDocTestCase(MockConfigPlayers, MpfFakeGameTestCase):
         self.assertBallsInPlay(int(balls))
 
     def command_assert_bool_condition(self, expected, condition):
-        result = self.machine.placeholder_manager.build_bool_template(condition).evaluate([])
         expected_bool = expected == "True"
-        self.assertEqual(bool(expected_bool), result, "{} = {} != {}".format(condition, result, expected_bool))
+        self.assertPlaceholderEvaluates(expected_bool, condition)
diff --git a/mpf/tests/MpfTestCase.py b/mpf/tests/MpfTestCase.py
index 7a49405f9..87da408f4 100644
--- a/mpf/tests/MpfTestCase.py
+++ b/mpf/tests/MpfTestCase.py
@@ -215,6 +215,11 @@ class MpfTestCase(unittest.TestCase):
         self.machine.events.post(event_name, **params)
         self.machine_run()
 
+    def assertPlaceholderEvaluates(self, expected, condition):
+        result = self.machine.placeholder_manager.build_raw_template(condition).evaluate([],
+                                                                                         fail_on_missing_params=True)
+        self.assertEqual(expected, result, "{} = {} != {}".format(condition, result, expected))
+
     def assertNumBallsKnown(self, balls):
         """Assert that a certain number of balls are known in the machine."""
         self.assertEqual(balls, self.machine.ball_controller.num_balls_known)
diff --git a/mpf/tests/test_ShotGroups.py b/mpf/tests/test_ShotGroups.py
index a1459c632..10a05ac0c 100644
--- a/mpf/tests/test_ShotGroups.py
+++ b/mpf/tests/test_ShotGroups.py
@@ -35,6 +35,7 @@ class TestShotGroups(MpfFakeGameTestCase):
         self.assertEventCalledWith("test_group_complete", state="unlit")
         self.mock_event("test_group_complete")
         self.mock_event("test_group_hit")
+        self.assertPlaceholderEvaluates("unlit", "device.shot_groups.test_group.common_state")
 
         self.hit_and_release_switch("switch_1")
 
@@ -43,11 +44,13 @@ class TestShotGroups(MpfFakeGameTestCase):
         self.assertEventNotCalled("test_group_complete")
 
         self.hit_and_release_switch("switch_2")
+        self.assertPlaceholderEvaluates(None, "device.shot_groups.test_group.common_state")
         self.hit_and_release_switch("switch_3")
         self.hit_and_release_switch("switch_4")
 
         self.assertEventCalled("test_group_hit")
         self.assertEventCalledWith("test_group_complete", state="lit")
+        self.assertPlaceholderEvaluates("lit", "device.shot_groups.test_group.common_state")
 
         self.stop_game()
 
diff --git a/mpf/tests/test_Shots.py b/mpf/tests/test_Shots.py
index b5f97e4e5..934bd8db1 100644
--- a/mpf/tests/test_Shots.py
+++ b/mpf/tests/test_Shots.py
@@ -254,11 +254,15 @@ class TestShots(MpfTestCase):
         # unlit and two states in the beginning
         self.assertEqual(2, len(self.machine.shots.shot_1.config['profile'].config['states']))
         self.assertEqual("unlit", self.machine.shots.shot_1.state_name)
+        self.assertPlaceholderEvaluates("unlit", "device.shots.shot_1.state_name")
+        self.assertPlaceholderEvaluates(0, "device.shots.shot_1.state")
 
         # one hit and it lits
         self.hit_and_release_switch("switch_1")
         self.assertEqual("lit", self.machine.shots.shot_1.state_name)
         self.assertEqual(1, self.machine.game.player["shot_shot_1"])
+        self.assertPlaceholderEvaluates("lit", "device.shots.shot_1.state_name")
+        self.assertPlaceholderEvaluates(1, "device.shots.shot_1.state")
 
         # it stays lit
         self.hit_and_release_switch("switch_1")