Repository: missionpinball/mpf

Issue:
Text templates ng
Current text templates look like this:

```
Player (player|number) Score: (player|score) Color: (machine|score)
```

Those work fine mostly. But they have a few limitations:
* No internationalization (i18n)
* No math/conditionals
* No formating
* Pipes are confusing because we use dots for other placeholders
* Not all placeholders are supported

Inline example (everything is converted to string):
```
Player {current_player.number}
```

Minimal example:
```
text: "Player {player_number:n}"
placeholders:
  - player_number:
        type: int
        value: current_player.number
```

Plural example (points is padded to 10 chars and centered):
```
text: "{points:^10d} point"
text_plural: "{points:^10d} points"
plural: current_player.score
placeholders:
  - points:
        type: int
        value: current_player.score
        default: 0 # in case there is no current player
```

Float formating example (fill with zero and max two decimals):
```
text: "{credits:0.2f} credits"
placeholders:
  - credits:
        type: int
        value: machine.credit_units
```

Multiple vars:
```
text: "Player {player_number:n} Score: {score:n} Color: {color}"
placeholders:
  - player_number:
        type: int
        value: current_player.number
  - score:
        type: float
        value: current_player.score
  - color:
        type: string
        value: "red" if current_player.score > 100 else: "blue"
```

All texts can be translated using gettext. We need to generate .po files and that is all. This also supports other plural forms which exist in some languages (e.g. for 21, 31, 41 and so on).

For number grouping {score:n} will work depending on the locale (e.g. in en_US or en_GB but not in de_DE). If you want to enfore it use {score:,} and it will happen in all locales.

Discussion:

Patch:
diff --git a/mpf/core/placeholder_manager.py b/mpf/core/placeholder_manager.py
index 90badc6af..0295c4f80 100644
--- a/mpf/core/placeholder_manager.py
+++ b/mpf/core/placeholder_manager.py
@@ -226,7 +226,7 @@ class MpfFormatter(string.Formatter):
 
 class TextTemplate:
 
-    """Legacy text placeholder."""
+    """Text placeholder."""
 
     var_finder = re.compile("(?<=\\()[a-zA-Z_0-9|]+(?=\\))")
     string_finder = re.compile("(?<=\\$)[a-zA-Z_0-9]+")
@@ -251,93 +251,12 @@ class TextTemplate:
         if not subscriptions:
             future = asyncio.Future(loop=self.machine.clock.loop)
         elif len(subscriptions) == 1:
-            future = subscriptions
+            future = subscriptions[0]
         else:
             future = Util.any(subscriptions, loop=self.machine.clock.loop)
         future = Util.ensure_future(future, loop=self.machine.clock.loop)
         return value, future
 
-    def monitor_changes(self, callback):
-        """Monitor variables for changes and call callback on changes."""
-        self._change_callback = callback
-        self._setup_variable_monitors()
-
-    def stop_monitor(self):
-        """Stop monitoring for changes."""
-        self._change_callback = None
-        self.machine.events.remove_handler(self._var_changes)
-
-    def _add_player_var_handler(self, name: str) -> None:
-        self.machine.events.add_handler('player_{}'.format(name), self._var_changes)
-
-    def _add_current_player_handler(self) -> None:
-        self.machine.events.add_handler('player_turn_started', self._var_changes)
-
-    def _add_machine_var_handler(self, name: str) -> None:
-        self.machine.events.add_handler('machine_var_{}'.format(name), self._var_changes)
-
-    def _var_changes(self, **kwargs) -> None:
-        del kwargs
-        if self._change_callback:
-            self._change_callback()
-
-    def _setup_variable_monitors(self) -> None:
-        for var_string in self.vars:
-            if '|' not in var_string:
-                self._add_player_var_handler(name=var_string)
-                self._add_current_player_handler()
-            else:
-                source, variable_name = var_string.split('|')
-                if source.lower().startswith('player'):
-
-                    if source.lstrip('player'):  # we have player num
-                        self._add_player_var_handler(name=variable_name)
-                    else:  # no player num
-                        self._add_player_var_handler(name=var_string)
-                        self._add_current_player_handler()
-                elif source.lower() == 'machine':
-                    self._add_machine_var_handler(name=variable_name)
-
-    def evaluate_text(self) -> str:
-        """Evaluate placeholder to string."""
-        text = self.text
-        for var_string in self.vars:
-            if var_string.startswith('machine|'):
-                _, var_name = var_string.split('|')
-                if self.machine.is_machine_var(var_name):
-                    replacement = str(self.machine.get_machine_var(var_name))
-                else:
-                    replacement = ''
-
-                text = text.replace('(' + var_string + ')', replacement)
-
-            elif self.machine.game and self.machine.game.player:
-                if var_string.startswith('player|'):
-                    text = text.replace('(' + var_string + ')', str(self.machine.game.player[var_string.split('|')[1]]))
-                elif var_string.startswith('player') and '|' in var_string:
-                    player_num, var_name = var_string.lstrip('player').split('|')
-                    try:
-                        value = self.machine.game.player_list[int(player_num) - 1][var_name]
-
-                        if value is not None:
-                            text = text.replace('(' + var_string + ')', str(value))
-                        else:
-                            text = text.replace('(' + var_string + ')', '')
-                    except IndexError:
-                        text = text.replace('(' + var_string + ')', '')
-                elif self.machine.game.player.is_player_var(var_string):
-                    value = self.machine.game.player[var_string]
-                    if value is not None:
-                        text = text.replace('(' + var_string + ')', str(value))
-                    else:
-                        text = text.replace('(' + var_string + ')', '')
-            else:
-                # set var to empty otherwise
-                if var_string.startswith('player') or var_string.startswith('player') and '|' in var_string:
-                    text = text.replace('(' + var_string + ')', '')
-
-        return text
-
 
 class BasePlaceholder(object):
 
@@ -442,10 +361,10 @@ class PlayerPlaceholder(BasePlaceholder):
 
     """Wraps the player."""
 
-    def __init__(self, player, machine):
+    def __init__(self, machine, number=None):
         """Initialise placeholder."""
-        self._player = player       # type: Player
         self._machine = machine     # type: MachineController
+        self._number = number
 
     def subscribe(self):
         """Subscribe to player changes."""
@@ -457,11 +376,52 @@ class PlayerPlaceholder(BasePlaceholder):
 
     def __getitem__(self, item):
         """Array access."""
-        return self._player[item]
+        if self._machine.game and self._machine.game.player:
+            if self._number is not None:
+                if len(self._machine.game.player_list) <= self._number:
+                    raise ValueError("Player not in game")
+                return self._machine.game.player_list[self._number][item]
+            else:
+                return self._machine.game.player[item]
+        else:
+            raise ValueError("Not in a game")
+
+    def __getattr__(self, item):
+        """Attribute access."""
+        if self._machine.game and self._machine.game.player:
+            if self._number is not None:
+                if len(self._machine.game.player_list) <= self._number:
+                    raise ValueError("Player not in game")
+                return getattr(self._machine.game.player_list[self._number], item)
+            else:
+                return getattr(self._machine.game.player, item)
+        else:
+            raise ValueError("Not in a game")
+
+
+class PlayersPlaceholder(BasePlaceholder):
+
+    """Wraps the player list."""
+
+    def __init__(self, machine):
+        """Initialise placeholder."""
+        self._machine = machine     # type: MachineController
+
+    def subscribe(self):
+        """Subscribe to player list changes."""
+        return self._machine.events.wait_for_any_event(["player_added", "game_ended"])
+
+    def subscribe_attribute(self, item):
+        """Subscribe player variable changes."""
+        return self._machine.events.wait_for_event('player_{}'.format(item))
+
+    def __getitem__(self, item):
+        """Array access."""
+        return PlayerPlaceholder(self._machine, item)
 
     def __getattr__(self, item):
         """Attribute access."""
-        return getattr(self._player, item)
+        return PlayerPlaceholder(self._machine, item)
 
 
 class MachinePlaceholder(BasePlaceholder):
@@ -612,7 +572,13 @@ class BasePlaceholderManager(MpfController):
         if isinstance(slice_value, dict) and node.attr in slice_value:
             ret_value = slice_value[node.attr]
         else:
-            ret_value = getattr(slice_value, node.attr)
+            try:
+                ret_value = getattr(slice_value, node.attr)
+            except ValueError:
+                if subscribe:
+                    raise TemplateEvalError(subscription + [slice_value.subscribe_attribute(node.attr)])
+                else:
+                    raise
         if subscribe:
             return ret_value, subscription + [slice_value.subscribe_attribute(node.attr)]
         else:
@@ -622,7 +588,10 @@ class BasePlaceholderManager(MpfController):
         value, subscription = self._eval(node.value, variables, subscribe)
         if isinstance(node.slice, ast.Index):
             slice_value, slice_subscript = self._eval(node.slice.value, variables, subscribe)
-            return value[slice_value], subscription + slice_subscript
+            try:
+                return value[slice_value], subscription + slice_subscript
+            except ValueError:
+                raise TemplateEvalError(subscription + slice_subscript)
         elif isinstance(node.slice, ast.Slice):
             lower, lower_subscription = self._eval(node.slice.lower, variables, subscribe)
             upper, upper_subscription = self._eval(node.slice.upper, variables, subscribe)
@@ -697,7 +666,7 @@ class BasePlaceholderManager(MpfController):
         if not subscriptions:
             future = asyncio.Future(loop=self.machine.clock.loop)
         elif len(subscriptions) == 1:
-            future = subscriptions
+            future = subscriptions[0]
         else:
             future = Util.any(subscriptions, loop=self.machine.clock.loop)
         future = Util.ensure_future(future, loop=self.machine.clock.loop)
@@ -749,11 +718,12 @@ class PlaceholderManager(BasePlaceholderManager):
             return DevicesPlaceholder(self.machine)
         elif name == "mode":
             return ModePlaceholder(self.machine)
+        elif name == "current_player":
+            return PlayerPlaceholder(self.machine)
+        elif name == "players":
+            return PlayersPlaceholder(self.machine)
         elif self.machine.game:
-            if name == "current_player":
-                return PlayerPlaceholder(self.machine.game.player, self.machine)
-            elif name == "players":
-                return self.machine.game.player_list
-            elif name == "game":
+            if name == "game":
                 return self.machine.game
+
         return False
diff --git a/mpf/devices/segment_display.py b/mpf/devices/segment_display.py
index 2153d5627..0beceea62 100644
--- a/mpf/devices/segment_display.py
+++ b/mpf/devices/segment_display.py
@@ -70,7 +70,6 @@ class SegmentDisplay(SystemWideDevice):
             self.hw_display.set_text("", flashing=False)
             if self._current_placeholder:
                 self.text = ""
-                self._current_placeholder.stop_monitor()
                 self._current_placeholder = None
             return
 
@@ -79,19 +78,18 @@ class SegmentDisplay(SystemWideDevice):
         # get top entry
         top_entry = self._text_stack[0]
 
-        if self._current_placeholder:
-            self._current_placeholder.stop_monitor()
-
         self._current_placeholder = TextTemplate(self.machine, top_entry.text)
-        self._current_placeholder.monitor_changes(self._update_display)
         self._update_display()
 
-    def _update_display(self) -> None:
+    def _update_display(self, *args, **kwargs) -> None:
         """Update display to current text."""
+        del args
+        del kwargs
         if not self._current_placeholder:
             new_text = ""
         else:
-            new_text = self._current_placeholder.evaluate_text()
+            new_text, future = self._current_placeholder.evaluate_and_subscribe({})
+            future.add_done_callback(self._update_display)
 
         # set text to display if it changed
         if new_text != self.text:


Test patch:
diff --git a/mpf/tests/machine_files/segment_display/config/config.yaml b/mpf/tests/machine_files/segment_display/config/config.yaml
index f87d5f055..2c79237a1 100644
--- a/mpf/tests/machine_files/segment_display/config/config.yaml
+++ b/mpf/tests/machine_files/segment_display/config/config.yaml
@@ -30,15 +30,15 @@ segment_display_player:
 
   test_score:
     display1:
-      text: "1: (player1|score)"
+      text: "1: {players[0].score:d}"
     display2:
-      text: "2: (machine|test)"
+      text: "2: {machine.test:d}"
 
   test_score_two_player:
     display1:
-      text: "(player1|score)"
+      text: "{players[0].score:d}"
     display2:
-      text: "(player2|score)"
+      text: "{players[1].score:d}"
 
   test_flash:
     display1:
diff --git a/mpf/tests/machine_files/segment_display/config/game.yaml b/mpf/tests/machine_files/segment_display/config/game.yaml
index 671f389ca..98289207a 100644
--- a/mpf/tests/machine_files/segment_display/config/game.yaml
+++ b/mpf/tests/machine_files/segment_display/config/game.yaml
@@ -24,9 +24,21 @@ segment_display_player:
     display4:
       text: ""
     display5:
-      text: "(player|ball)"
+      text: "{current_player.ball:d}"
 
   # clear only display5 after game
+  game_ended{machine.player1_score > 0}:
+    display1:
+      text: "{machine.player1_score:d}"
+  game_ended{machine.player2_score > 0}:
+    display2:
+      text: "{machine.player2_score:d}"
+  game_ended{machine.player3_score > 0}:
+    display3:
+      text: "{machine.player3_score:d}"
+  game_ended{machine.player4_score > 0}:
+    display4:
+      text: "{machine.player4_score:d}"
   game_ended:
     display5:
       text: ""
@@ -60,14 +72,14 @@ segment_display_player:
   # show score when adding players
   player_added.1{num==1}:
     display1:
-      text: "(player1|score)"
+      text: "{players[0].score:d}"
   player_added.2{num==2}:
     display2:
-      text: "(player2|score)"
+      text: "{players[1].score:d}"
   player_added.3{num==3}:
     display3:
-      text: "(player3|score)"
+      text: "{players[2].score:d}"
   player_added.4{num==4}:
     display4:
-      text: "(player4|score)"
+      text: "{players[3].score:d}"
 
diff --git a/mpf/tests/test_SegmentDisplay.py b/mpf/tests/test_SegmentDisplay.py
index 360de311b..cb9c32cf8 100644
--- a/mpf/tests/test_SegmentDisplay.py
+++ b/mpf/tests/test_SegmentDisplay.py
@@ -1,7 +1,7 @@
 from mpf.tests.MpfFakeGameTestCase import MpfFakeGameTestCase
 
 
-class TestShots(MpfFakeGameTestCase):
+class TestSegmentDisplay(MpfFakeGameTestCase):
 
     def getConfigFile(self):
         if self._testMethodName == "test_game":
@@ -164,13 +164,13 @@ class TestShots(MpfFakeGameTestCase):
         self.post_event("test_score")
         self.advance_time_and_run()
 
-        self.assertEqual("1: ", display1.hw_display.text)
-        self.assertEqual("2: ", display2.hw_display.text)
+        self.assertEqual("1: 0", display1.hw_display.text)
+        self.assertEqual("2: 0", display2.hw_display.text)
 
         self.machine.set_machine_var("test", 42)
         self.advance_time_and_run()
 
-        self.assertEqual("1: ", display1.hw_display.text)
+        self.assertEqual("1: 0", display1.hw_display.text)
         self.assertEqual("2: 42", display2.hw_display.text)
 
         self.start_game()
@@ -225,13 +225,13 @@ class TestShots(MpfFakeGameTestCase):
 
         # first display shows score. second empty
         self.assertEqual("0", display1.hw_display.text)
-        self.assertEqual("", display2.hw_display.text)
+        self.assertEqual("0", display2.hw_display.text)
 
         # player scores
         self.machine.game.player.score += 42
         self.advance_time_and_run(.01)
         self.assertEqual("42", display1.hw_display.text)
-        self.assertEqual("", display2.hw_display.text)
+        self.assertEqual("0", display2.hw_display.text)
 
         # add player
         self.add_player()