Mode configured to stop playing song when mode ends but configuration is invalid

[bernarma]

In the documentation the sound_player configuration item indicates you can specify the "mode_end_action" property but when attempting to start mpf it indicates that this is NOT valid.

Referring to this documentation: https://missionpinball.org/config/sound_player/#mode_end_action it appears as though it is supported. I have checked the installed version of MPF config_spec.yaml and that configuration property is NOT present which confirms the behavior I am seeing but looking at the master/main branch in GitHub I can see that the config_spec.yaml file has it.

The question I have, will this be coming in future dev builds? I am using Mission Pinball Framework v0.80.0.dev4 or is this going to be deprecated and if so, is there an alternate way of stopping a music file when a mode ends? Thanks for your assistance.

[avanwinkle]

You are correct, this config option was not migrated to MPF 0.80 and the default behavior is (or should be!) to stop all mode-based sounds when a mode stops.

The MPF mechanism for this is called "context", where each sound/slide/widget is played with its mode as the context and when a mode ends, a "clear" event is triggered with the mode name as an argument and all active sounds/slides/widgets with that name as their context will be gracefully removed.

For cases when this is not desired, you can manually specify your own context in the config player and your asset will respond to that context instead, so you can/must manually clear it.

If you are triggering a sound in a mode through sound_player and that sound is not stopping when the mode stops, I would love to see your logs to determine what the bug is.

[bernarma]

Thanks for your explanation. That makes sense. I have attached both logs and snippets from the mode config files (song2.yaml is effectively the same as song1.yaml). The godot file is included as it shows when it started playing the song. In godot, the music files are in the sounds directory called song1.wav with a corresponding song1.tres and a song2.wav and song2.tres. Hopefully this helps.

#song_select.yaml

mode:
  start_events: start_mode_song_select
  stop_events: song_select_item_selected
  code: mpf.modes.carousel.code.carousel.Carousel

mode_settings:
  selectable_items: song1, song2
  select_item_events: s_lockdown_active # or ball plunge
  next_item_events: s_flipper_left_active
  previous_item_events: s_flipper_right_active

variable_player:
  item_highlighted:
    selected_song:
      string: (item)
      action: set

event_player:
  item_highlighted{item=="song1"}:
    - mode_Song_CHANGE
    - mode_Song_1_start
  item_highlighted{item=="song2"}:
    - mode_Song_CHANGE
    - mode_Song_2_start

#song1.yaml

mode:
  start_events: ball_started{current_player.selected_song=="song1"}, mode_Song_1_start, timer_song2_timer_complete
  stop_events: ball_ending,mode_Song_CHANGE,timer_song1_timer_complete
  restart_on_next_ball: true

sound_player:
  mode_song1_started:
    song1:
      action: play
      loops: 0
      volume: 0.25

godot.log
godot2024-08-12T08.15.21.log

[bernarma]

I believe because it has a value, albeit null, it then uses that value instead of reverting to the default specified. So I can fix this but would this not be a larger issue in how defaults are set across the board?

[bernarma]

Here you can see (from the doco) that if the key exists it will return that:

[avanwinkle]

Ah yes. One of my annoyances with Godot is that it doesn't accept null for any typed variable and doesn't offer a nullable-variable type, AND a get() call that returns null will be considered truthy and not use the default. Ugh.

The simplest fix here is to remove the :float type annotation so that null can be passed, be falsey, and then if not fade_out will be executed. And a comment on the line to indicate that it's intentionally untyped.

Sidebar, recent godot tests demonstrated significant performance boosts with strong typing, so I went through and added as many type annotations as I could. I then had to go back and remove many of them because of exactly this issue, so establishing a pattern for noting an un-typed variable will be good to do. :)

[avanwinkle]

Or I suppose it could be like this?

var fade_out: float = settings["fade_out"] if settings.get("fade_out") else 0.0

Which would check the value regardless of type and anything falsey would fall back to 0.0. That makes two lookups on the settings dictionary, but it should work.

Too bad Godot doesn't return values in booleans, or we could do var fade_out = settings.get("fade_out") or 0.0, but that would just return true or false.

[bernarma]

Haha... yeah I was about to do that but you beat me to it.

I was in the process of raising an issue/bug so I could create a branch against it to submit for PR but it looks like you are on top of it... Did you want me to leave it with you or do you want me to do it?

[avanwinkle]

I’m away from my computer for a bit, feel free to open a PR!

…

On Sun, Aug 11, 2024 at 19:47 Mark Bernardinis ***@***.***> wrote: Haha... yeah I was about to do that but you beat me to it. I was in the process of raising an issue/bug so I could create a branch against it to submit for PR but it looks like you are on top of it... Did you want me to leave it with you or do you want me to do it? — Reply to this email directly, view it on GitHub <#142 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABMCOWFKTFG45QWO7AZBUIDZRAO33AVCNFSM6AAAAABMKP6R6OVHI2DSMVQWIX3LMV43URDJONRXK43TNFXW4Q3PNVWWK3TUHMYTAMZQHA4TKMY> . You are receiving this because you commented.Message ID: <missionpinball/missionpinball/repo-discussions/142/comments/10308953@ github.com>

[bernarma]

Done - I fixed 2 other issues relating to the same problem, fade_in and fade_out. Will submit PR as the changes are pushed.

BTW, I prefer the fully typed view as well so I went with that approach.

missionpinball/mpf-gmc#9