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.

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

Block tilt warnings once game has tilted #1529

Merged
merged 8 commits into from
Jan 4, 2021

Conversation

enteryourinitials
Copy link
Contributor

Any further tilt warnings that were triggered once the game tilted would affect the tilt warning count for the players next ball. This change will not trigger any further warnings once the game is in a tilted state and ensure their tilt warning count is at zero when the next ball begins.

@jabdoa2
Copy link
Collaborator

jabdoa2 commented Sep 1, 2020

Thanks! I guess we should also add a regression test for your case so we won't break it in the future.

@enteryourinitials
Copy link
Contributor Author

Yeah a regression test would be a good idea I think. I'm not sure what's involved or how to go about that though. Also the build is currently failing on Python 3.5 with "SystemError: Parent module 'setuptools' not loaded, cannot perform relative import". I'm not sure what I need to do to fix that.

@jabdoa2
Copy link
Collaborator

jabdoa2 commented Sep 2, 2020

Don't worry about the tests. That looks spurious and I will take care.

Tilt tests are here: https://github.com/missionpinball/mpf/blob/dev/mpf/tests/test_Tilt.py. You could extend that if you want. Those can be run using:"python3 -m unittest mpf.tests.test_Tilt".

@jabdoa2
Copy link
Collaborator

jabdoa2 commented Sep 2, 2020

Seems like a known bug: pypa/setuptools#2352. Hope this gets fixed upstream.

@jabdoa2
Copy link
Collaborator

jabdoa2 commented Sep 4, 2020

I added an test for this but I cannot reproduce the issue you describe. Seems like the warnings are counted for the correct player. It counts higher as the limit but it does not count anything for the next player. Please have a look at the test.

@enteryourinitials
Copy link
Contributor Author

The scenario I encountered was with a single player game.

To outline the steps (assuming tilt warnings set to 3):

  • Start 1 player game
  • (Activate tilt switch) Tilt warning
  • (Activate tilt switch) Tilt warning
  • (Activate tilt switch) Player tilts
  • (Activate tilt switch) Tilt warning
  • (Activate tilt switch) Tilt warning
  • Ball drains to outhole
  • Ball 2 starts
  • (Activate tilt switch) Player tilts

I should have kept my log file to make it easier to see. In the log I could see the tilt warnings get reset to 0 via an event, but additional warnings could be activated before the ball drains, which then carried forward for that same player on the next ball.

@jabdoa2
Copy link
Collaborator

jabdoa2 commented Sep 6, 2020

Can you turn that case into a test? Just copy my test and adjust it accordingly.

@jabdoa2
Copy link
Collaborator

jabdoa2 commented Sep 6, 2020

I just played with your test. To me it looks like the problem still exists even with your changes. I removed the reset of tilt_warnings in the test because that is not what happens in reality and it will then instantly tilt on the next ball. Try this test:

    def test_carry_over_single_player(self):
        self._add_tilt_handler()
        self._prepare_trough()
        self.start_game()

        self.advance_time_and_run(10)
        self.assertBallsOnPlayfield(1)
        self.assertPlayerVarEqual(0, "tilt_warnings")

        # tilt machine
        self.hit_and_release_switch("s_tilt_warning")
        self.advance_time_and_run(2)
        self.assertFalse(self._is_tilted)
        self.assertPlayerVarEqual(1, "tilt_warnings")
        self.hit_and_release_switch("s_tilt_warning")
        self.advance_time_and_run(2)
        self.assertFalse(self._is_tilted)
        self.assertPlayerVarEqual(2, "tilt_warnings")

        self.hit_and_release_switch("s_tilt_warning")
        self.advance_time_and_run(2)
        self.assertTrue(self._is_tilted)
        self.assertPlayerVarEqual(3, "tilt_warnings")

        # tilt_warnings player var no longer increases because machine is tilted
        self.hit_and_release_switch("s_tilt_warning")
        self.advance_time_and_run(2)
        self.assertPlayerVarEqual(3, "tilt_warnings")

        # reset tilt warnings and ensure player var no longer increases until next ball
        #self.machine.game.player['tilt_warnings'] = 0

        self.hit_and_release_switch("s_tilt_warning")
        self.advance_time_and_run(2)
        self.assertPlayerVarEqual(3, "tilt_warnings")
        self.hit_and_release_switch("s_tilt_warning")
        self.advance_time_and_run(2)
        self.assertPlayerVarEqual(3, "tilt_warnings")
        self.assertPlayerNumber(1)

        # advance to ball 2
        self.drain_one_ball()
        self.advance_time_and_run(10)
        self.assertPlayerNumber(1)

        #self.assertPlayerVarEqual(0, "tilt_warnings")
        self._is_tilted = False

        self.assertBallNumber(2)
        self.assertBallsOnPlayfield(1)

        # generate tilt warning and ensure no carry over from previous ball
        self.hit_and_release_switch("s_tilt_warning")
        self.advance_time_and_run(2)
        #self.assertPlayerVarEqual(1, "tilt_warnings")
        self.assertFalse(self._is_tilted)

I guess the real problem is that tilt_warnings does not get reset, right?

@enteryourinitials
Copy link
Contributor Author

The tilt_warnings value does get reset by an event around the time the "ball_will_end" event triggers.

EventManager : Event: ======'tilt'====== Args={}
EventManager : Event: ======'machine_var_audits_switches_s_tilt_bob'====== Args={'value': 218, 'prev_value': 217, 'change': 1}
EventManager : Event: ======'ball_will_end'====== Args={}
EventManager : Event: ======'player_tilt_warnings'====== Args={'value': 0, 'prev_value': 3, 'change': -3, 'player_num': 1}

However, if I remove my change, then any further activity on the tilt switch will start increasing this value again before the ball drains. This value then carries across for the player to their next ball. Also, if you have event listeners looking at tilt_warning events, these will pick up the extra events. A way around that would be to use conditional checks there to ignore them if the game has tilted. But the underlying issue is the tilt_warnings value will continue to increase after it's reset without my change.

Another option for fixing could be to reset the tilt_warnings value once the ball is in the trough or at the beginning of a ball if people want the extra warnings.

@jabdoa2
Copy link
Collaborator

jabdoa2 commented Sep 7, 2020

The tilt_warnings value does get reset by an event around the time the "ball_will_end" event triggers.

EventManager : Event: ======'tilt'====== Args={}
EventManager : Event: ======'machine_var_audits_switches_s_tilt_bob'====== Args={'value': 218, 'prev_value': 217, 'change': 1}
EventManager : Event: ======'ball_will_end'====== Args={}
EventManager : Event: ======'player_tilt_warnings'====== Args={'value': 0, 'prev_value': 3, 'change': -3, 'player_num': 1}

However, if I remove my change, then any further activity on the tilt switch will start increasing this value again before the ball drains. This value then carries across for the player to their next ball. Also, if you have event listeners looking at tilt_warning events, these will pick up the extra events. A way around that would be to use conditional checks there to ignore them if the game has tilted. But the underlying issue is the tilt_warnings value will continue to increase after it's reset without my change.

Another option for fixing could be to reset the tilt_warnings value once the ball is in the trough or at the beginning of a ball if people want the extra warnings.

Where does that happen? Try my test from above where I removed the line which did that in the test. This is what I am seeing:

$ python3 -m unittest mpf.tests.test_Tilt.TestTilt.test_carry_over_single_player_jan -v 2>&1 | grep -iE "player_tilt|ball_will"
2.001 : INFO : EventManager : Event: ======'ball_will_start'====== Args={'player': 1, 'ball': 1, 'balls_remaining': 1, 'is_extra_ball': False}
13.001 : INFO : EventManager : Event: ======'player_tilt_warnings'====== Args={'value': 1, 'prev_value': 0, 'change': 1, 'player_num': 1}
15.001 : INFO : EventManager : Event: ======'player_tilt_warnings'====== Args={'value': 2, 'prev_value': 1, 'change': 1, 'player_num': 1}
17.001 : INFO : EventManager : Event: ======'player_tilt_warnings'====== Args={'value': 3, 'prev_value': 2, 'change': 1, 'player_num': 1}
17.001 : INFO : EventManager : Event: ======'ball_will_end'====== Args={}
28.001 : INFO : EventManager : Event: ======'ball_will_start'====== Args={'player': 1, 'ball': 2, 'balls_remaining': 0, 'is_extra_ball': False}
35.001 : INFO : EventManager : Event: ======'player_tilt_warnings'====== Args={'value': 4, 'prev_value': 3, 'change': 1, 'player_num': 1}
35.001 : INFO : EventManager : Event: ======'ball_will_end'====== Args={}

@jabdoa2
Copy link
Collaborator

jabdoa2 commented Sep 14, 2020

@enteryourinitials are you still on this? Or do you want me to change the tilt code? I got an idea what to change. Your change definitely improves the situation. However, I think one more change is needed to fix your problem highlighted in your test.

enteryourinitials and others added 8 commits January 4, 2021 19:43
Any further tilt warnings that were triggered once the game tilted would affect the tilt warning count for the players next ball. This change will not trigger any further warnings once the game is in a tilted state and ensure their tilt warning count is at zero when the next ball begins.
@jabdoa2
Copy link
Collaborator

jabdoa2 commented Jan 4, 2021

Sorry that it took me so long. I changed the test to use the default tilt configuration and rebased it to dev

@jabdoa2 jabdoa2 merged commit 2d69c0f into missionpinball:dev Jan 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants