Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

score reels no longer activate chime on score reset. #1784

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

score reels no longer activate chime on score reset. #1784

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
384d7d3
score reels no longer activate chime on score reset.
mrechte Mar 22, 2024
d991d94
Merge branch 'dev' into feat_scorereel_no_chime_on_reset
avanwinkle Jun 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions mpf/devices/score_reel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ def __init__(self, machine, name):

self._destination_value = 0
# Holds the index of the destination the reel is trying to advance to.

self._quiet = True
# Whether to use chime when advancing

self._runner = None
# asyncio task which advances the reel
Expand Down Expand Up @@ -154,7 +157,8 @@ async def _advance_reel_if_position_does_not_match(self):
while self._destination_value != self.assumed_value:
self.machine.events.post('reel_{}_will_advance'.format(self.name))
wait_ms = self.config['coil_inc'].pulse(max_wait_ms=500)
self.machine.events.post('reel_{}_advancing'.format(self.name))
if not self._quiet:
self.machine.events.post('reel_{}_advancing'.format(self.name))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice fix, but limited to your specific application. An alternative approach would be to post the event with an argument like reset: true to indicate whether the reel is resetting or not. This way there's still an event for people who want/need to know the advancement of the reel, and chimes can be made conditional on that argument to determine whether to chime or not.

previous_value = self.assumed_value

await asyncio.sleep((wait_ms + self.config['repeat_pulse_time']) / 1000)
Expand All @@ -180,7 +184,7 @@ def wait_for_ready(self):
"""Return a future for ready."""
return self._ready.wait()

def set_destination_value(self, value):
def set_destination_value(self, value, quiet=False):
"""Return the integer value of the destination this reel is moving to.

Args:
Expand All @@ -197,6 +201,7 @@ def set_destination_value(self, value):
self._destination_value, value)

self._destination_value = value
self._quiet = quiet

self._busy.set()
self._ready.clear()
4 changes: 2 additions & 2 deletions mpf/devices/score_reel_controller.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def _rotate_player(self, **kwargs):
# Make sure this score reel group is showing the right score
self.log.debug("Current player's score: %s",
self.machine.game.player.score)
self.active_scorereelgroup.set_value(self.machine.game.player.score)
self.active_scorereelgroup.set_value(self.machine.game.player.score, True)

self.active_scorereelgroup.light()

Expand Down Expand Up @@ -149,7 +149,7 @@ async def _game_starting(self, **kwargs):
raise AssertionError('Need a score reel group tagged "player1"')

for score_reel_group in self.machine.score_reel_groups.values():
score_reel_group.set_value(0)
score_reel_group.set_value(0, True) # No chime
await score_reel_group.wait_for_ready()

def _game_ending(self, **kwargs):
Expand Down
5 changes: 2 additions & 3 deletions mpf/devices/score_reel_group.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def chime(cls, chime, **kwargs):
del kwargs
chime.pulse()

def set_value(self, value):
def set_value(self, value, quiet=False):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aligned with my previous comment, changing the argument from quiet to is_reset will provide usecase-independent context and help future developers understand what's happening.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello. Please could you help me ?
Everything starts from the score_reel_controller, that will call set_value() of ScoreReelGroup class in 3 situations:

  1. _game_starting
  2. _rotate_player
  3. score_change
    If I understand correctly, ScoreReel would always 'post reel    {}_advancing', with an extra boolean argument 'reset'.
    Then we would modify the chime handler of ScoreReelGroup, that would not pulse if reset is True.
    How do I retrieve the reset argument in chime method ? Is it in kwargs, or do I just add a reset arg after chime ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you post an event, you can include kwargs like so:

self.machine.events.post(f'reel_{self.name}_advancing', is_reset=False)

And then in an event listener, you can make it conditional:

event_player:
  reel_myreel_advancing{not is_reset}: play_chime

"""Reset the score reel group to display the value passed.

This method will "jump" the score reel group to display the value
Expand Down Expand Up @@ -109,8 +109,7 @@ def set_value(self, value):
for i, reel in enumerate(self.reels):
if not reel:
continue

reel.set_destination_value(self.desired_value_list[i])
reel.set_destination_value(self.desired_value_list[i], quiet)

async def wait_for_ready(self):
"""Return a future which will be done when all reels reached their destination."""
Expand Down
Loading