Shot Group State Complete triggered events finish before individual Shot Hit triggered events

[jimhenshaw]

I used lane shots hit to add 1 to a player variable. I wanted to use shot group state complete as a way to increase the add value from 1 to 3 for the the lane hits.

The behavior I saw was:

1. lane 1 hit, add 1 to player variable
2. lane 2 hit, add 1 to player variable
3. lane 3 hit, stack the event to add 1 to player variable, but don't execute yet
4. group shots state complete, modify the add value variable from 1 to 3
5. finish lane 3 hit by adding 3 to the player variable

I expected the lane 3 hit to add 1 to the player variable first.

Bosh's observation:

I've tracked down the root cause and it would be tricky, if not technically also behavior-breaking, to change. The shot sets its internal state on hit before issuing shot_hit events, and the shot_group listens to player variable changes, not shot hit events, and then it reads from each shot's internal state. So the internal state MUST be updated before completion is checked
However, it's probably better to process things in the order you expected/suggested, so you could write up an issue on the MPF repo and Anthony might find this compelling enough to dig in on. I'd include a player_var: definition, plus a shot, shot_group, the event_players, variable_players (skip the indirection of the points/score stuff, it's irrelevant here) and event sequence with current scoring behaviors at each step, vs desired scoring behavior

Per his request, I am submitting this issue.

His workaround solution was to add 1ms to the execution of the modifying of the add value variable under the group shots complete via |1ms. This works, but may have unforeseen outcomes in different scenarios.

[bosh]

Specifically, Shots update their state in hit here:

mpf/mpf/devices/shot.py

Line 318 in 7958c06

state = profile_settings['name']

which issues a player_(shot_state_variable_name) update event. Later in hit, the shot issues four different hit events.

Shot Group listens for the shot hit events as well as the player_shot_shotname variable update event, here:

mpf/mpf/devices/shot_group.py

Line 51 in 7958c06

self.machine.events.add_handler("player_shot_{}".format(shot.name), self._check_for_complete)

The player variable hook calls ShotGroup::_check_for_complete handler, which checks shot states, which already have been updated per the first link.

The completion check will then issue the shot group complete (with and without state) variants.

mpf/mpf/devices/shot_group.py

Line 98 in 7958c06

self.machine.events.post('{}_{}_complete'.format(self.name, state))

Therefore the shot group completion check will be handled before the shot hit events (which the shot group listens for to report its own additional hit events). Because the shot hit and shot group hits are the most obvious places to add scoring, that scoring will be processed after the group completion, and cannot be beaten by handler priority correction due to event queue ordering.

Actually, upon writing this up, it seems like my writeup ^ is incorrect. The event queue should be:

[switch_active] # initial triggering event of some switch hit, but otherwise empty

Then it processes and the shot changes state and posts events, so the queue is now:
[player_shot_shotname, shot_hit(4 variants)]

Then the shot_group processes the player var event, so queue is:
[shot_hit(4v), shot_group_complete(2 variants)]

Then the shot scoring actually does process before the group completion. Hmmmm
But the group listens for the shot_hits and reports its own shot group hit events:
[shot_group_complete(2v), shot_group_hit(2 variants)]

The shot_group_state_complete handler changes the rate variable (from 1 to 3, eg leveling up the points per shot):
[shot_group_hit(2v)]

And finally the group hit event processes and it uses the higher rate instead of original lower rate.

So maybe the most correct implementation in this case is to use per-shot scoring instead of shot-group scoring.

@jimhenshaw it would be helpful to include the minimum required code config (player_vars, variable_players, event_players, shots, shot_groups)

[jimhenshaw]

Minimal Code:

###base.yaml###

shots:
  lane_g:
    switch: s_glane
  lane_a:
    switch: s_alane
  lane_s:
    switch: s_slane

shot_groups:
  gas_lanes:
    shots: lane_g, lane_a, lane_s


event_player:
  gas_lanes_lit_complete: set_gas_multiplier_3

  lane_g_hit: score_lane_g_hit
  lane_a_hit: score_lane_a_hit
  lane_s_hit: score_lane_s_hit

  score_lane_g_hit:
    score_mult: {points: 100}   
  score_lane_a_hit:
    score_mult: {points: 100}
  score_lane_s_hit:
    score_mult: {points: 100}


variable_player:
  score_mult:
    score: points * current_player.PSI

  set_gas_multiplier_3:
    gas_multiplier:
      action: set
      int: 3

###config.yaml###

player_vars:
  gas:
    initial_value: 0
  gas_multiplier:
    initial_value: 1

[jimhenshaw]

Actual code

###base.yaml###

shots:

  lane_g:
    switch: s_glane
    show_tokens:
      led: l_glane
    profile: gas_profile
  lane_a:
    switch: s_alane
    show_tokens:
      led: l_alane
    profile: gas_profile
  lane_s:
    switch: s_slane
    show_tokens:
      led: l_slane
    profile: gas_profile

shot_groups:
  gas_lanes:
    shots: lane_g, lane_a, lane_s
    rotate_left_events: s_left_flipper_active
    rotate_right_events: s_right_flipper_active
    reset_events: reset_gas_lanes

event_player:

  #If you light up GAS, they flash and Pop/OCTANE targets give 3x gas units
  gas_lanes_lit_complete{current_player.gas_lit == 0}: play_flash_gas_lanes, set_gas_lit|1ms, set_gas_multiplier_3|1ms

  #If you light GAS a second time, it enables NOS to be used and Pop/Octane targets give 5x
  gas_lanes_lit_complete{current_player.gas_lit == 1}: enable_nos, set_gas_multiplier_5, send_nos_color_to_godot

  #handle addings gas
  lane_g_hit: add_gas_and_set_timer, score_lane_g_hit
  lane_a_hit: add_gas_and_set_timer, score_lane_a_hit
  lane_s_hit: add_gas_and_set_timer, score_lane_s_hit

  score_lane_g_hit:
    score_mult: {points: 100}   
  score_lane_a_hit:
    score_mult: {points: 100}
  score_lane_s_hit:
    score_mult: {points: 100}


variable_player:
  score_mult:
    score: points * current_player.PSI

  set_gas_multiplier_3:
    gas_multiplier:
      action: set
      int: 3

###config.yaml###

player_vars:
  gas:
    initial_value: 0
  gas_multiplier:
    initial_value: 1

[avanwinkle]

Wow this is quite the conundrum!

Thank you @bosh for the great investigation. It's necessary that the shot group checks its state change on the player variable, to allow for event-based advances as well as restoring state when a mode preserves the shot states. And yes, it's necessary that the player variable be updated first because that new state is propagated into the various hit events.

This behavior is counterintuitive, and a proper solution should be pondered. It will risk being a breaking change but hopefully breaking in a good way :)

I've poked around with the sample code (thank you @jimhenshaw !) and hoped that changing the number of chained events would help, and/or playing with priorities and relaying events, but I can't find any way to skip the call stack except the already-suggested 1ms delay. That's actually quite an optimal solution though, because it just lets other events in the cycle finish before proceeding. The only possible risk I'd see is if the mode was ending and managed to stop during that 1ms... highly unlikely though.