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

Evaluate PluginPlayer Templates prior to BCP #1824

Merged
merged 4 commits into from
Sep 1, 2024

Conversation

avanwinkle
Copy link
Collaborator

Summary

This PR extends the behavior of PluginPlayer to find and evaluate placeholder templates prior to sending player events over BCP. The following PluginPlayers will be affected: BusPlayer, SlidePlayer, SoundPlayer, WidgetPlayer

Examples

This new behavior works with both path-based templates like player variables, as well as event kwarg templates.

Player Variable Example

sound_player:
  mode_rampage_started:
    rampage_music:
      bus: music
      start_at: (current_player.rampage_music_last_played_time)

Event Kwarg Example

event_player:
  some_triggering_event:
    play_music_starting_at:
      the_time: 65


sound_player:
  play_music_starting_at:
    victory_music:
      bus: music
      start_at: (the_time)

Nested Logic

PluginPlayer events are nested dictionaries because a triggering event may have multiple items to play (e.g. a mode_started event triggering both music and voice callouts via sound_player). Therefore this code must nest one level deep to look for placeholder templates on each played item.

For efficiency (?) a separate dictionary is made to hold the evaluated values and only item dicts that have templates will be copied and their values overridden. The settings argument comes from the raw parsed config itself, so it should not be modified in place. Instead, the temporary evaluated dictionary is passed as additional args into the settings dict, preserving the original values with (hopefully) minimum overhead.

This feels a bit clunky but in the past MPF has been bottlenecked by huge numbers of dict.copy() and dict.deepcopy() calls, which were the primary function call during profiling. My hope is that the implementation here will be low-profile enough to have negligible impact in almost all cases, and the occasional dict copy for template evaluations is an acceptable price.

evaluate me

@bernarma
Copy link

I have been testing this and in some cases the value for starts_at is set to <NativeTemplate 0.0> (but not always).

This is the snippet that I have that sometimes causes this behavior - keep in mind over on the Godot side I have added delay support for playing media which was previously ignored but should not have an bad side-effects for this use case.

    song1:
      delay: 5s
      action: play
      start_at: (current_player.active_song_seconds if current_player.active_song == "song1" else 0)
      events_when_stopped: song_finished

@bernarma
Copy link

I have been testing this and in some cases the value for starts_at is set to <NativeTemplate 0.0> (but not always).

This is the snippet that I have that sometimes causes this behavior - keep in mind over on the Godot side I have added delay support for playing media which was previously ignored but should not have an bad side-effects for this use case.

    song1:
      delay: 5s
      action: play
      start_at: (current_player.active_song_seconds if current_player.active_song == "song1" else 0)
      events_when_stopped: song_finished

Actually, after some further testing it has nothing to do with that snippet. It has to do with this snippet:

  song_name_completed:
    song1:
      delay: 5s
      action: play
      start_at: 0 # not being sent through as just the value
      events_when_stopped: song_finished

@avanwinkle
Copy link
Collaborator Author

Ah, good catch. Template properties with static values have a different python type that is escaping the template-detection code path.

I'm out of town this week but know exactly what the fix will be!

@bernarma
Copy link

Ah, good catch. Template properties with static values have a different python type that is escaping the template-detection code path.

I'm out of town this week but know exactly what the fix will be!

No probs... I thought you were busy as I hadn't heard from you, but no stress. I can workaround it in the meantime.

Copy link

sonarcloud bot commented Sep 1, 2024

@avanwinkle avanwinkle merged commit ba32634 into 0.80.x Sep 1, 2024
29 checks passed
@avanwinkle avanwinkle deleted the pluginplayer-template-eval branch September 1, 2024 19:04
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