Non-intuitive loop setup ("infinite" vs "loop X times")

[Former user]

Current API is not intuitive to set setup of looping. I mean, the setting that enables infinite loop is hidden into non-related "gme_set_fade()" call...

For example, after this commit: https://bitbucket.org/mpyne/game-music-emu/commits/49238f9ae5792e16481228916cef50c4a6c6c5a0

That causes this https://github.com/coelckers/gzdoom/commit/df2bef2f60b8f12cb0d0400315f72e979e30cb9c because of unknowledge of set gme_set_fade to -1 to enable infinite loop which is non-intuitive.

My suggestion is: * Provide a separate function "gme_set_loops_count()" that will receive count of loops and -1 to loop infinite. * Why not to keep infinite loop behavior be default to keep compatibility with software that relies on this behavior? And let everyone who want "play X times", enable that by manual way.

[Wohlstand]

// wops! I’m an author of this issue, I have accidentally created it without be logged in…

[Michael Pyne]

I'm supportive of breaking this out into a dedicated method rather than overloading gme_set_fade.

Are you saying that gme should default to looping infinitely? Or are you referring to older behavior of gme_set_fade when you talk about restoring compatibility?

[Wohlstand]

Yes, make the infinite loop be default behavior which was originally. Most of software are used this version before this update, rely on that it’s playing infinitely.

It’s bad strategy to overload unrelated feature into gme_set_fade as it’s NOT obvious and very hard to figure out “where is to disable/enable loop“, the fade must control the fade time only. For an ability to control loops, you better to introduce the gme_set_loop call that will not only on/off looping, and will also give an ability to choose count of loops (for now it’s possible to run only one loop).

• Adding new functions is safe for ABI
• Behavior change of existing functions - is incompatibility that will affect runtime

[Michael Pyne]

I looked into this a bit over the past weekend and this change was actually intentional to address issue #25.

In that case, to support acting as a music decoder for plugin-based usage, infinite looping by default is problematic.

Instead of trying to control looping a track a specific number of times, maybe we can make the default loading of track playback times optional within the library (as a global function) that way it can be set once, when the libgme is initially created.

Then the VLC devs might still need to make a fix once the previous default behavior is restored, but it wouldn’t have to be a difficult fix.

[Wohlstand]

Anyway, yeah, you can make the "don't loop by default” like is made on my libADLMIDI/libOPNMIDI libraries, but instead of overloading gme_set_fade please create new function(s) to control loops (for example, `gme_set_loop()` )are will be obvious to understand even you’ll keep that overloading on `gme_set_fade` for compatibility.

[Michael Pyne]

WIP: Add a function to avoid automatic track ending.

This maintains automatic track ending as the default to address issue #25 while making it easier to restore the previous default behavior of having music loop infinitely.

To maintain binary compatibility on Music_Emu and its subclasses, I make this a global library setting that can be queried and set (or reset), instead of specific to a Music_Emu class.

This should resolve issue #29.

→ <<cset 39a83d4b11d5>>

[Michael Pyne]

WIP: Add a function to avoid automatic track ending.

This maintains automatic track ending as the default to address issue #25 while making it easier to restore the previous default behavior of having music loop infinitely.

To maintain binary compatibility on Music_Emu and its subclasses, I make this a global library setting that can be queried and set (or reset), instead of specific to a Music_Emu class.

This should resolve issue #29.

→ <<cset 0cf7cbbd5e73>>

[Michael Pyne]

Add a function to avoid automatic track ending.

This maintains automatic track ending as the default to address issue #25 while making it easier to restore the previous default behavior of having music loop infinitely.

This changes the structure of the Music_Emu object itself, but that has not been part of the binary interface (the C API from gme.h treats Music_Emu as an opaque handle). So as long as you don't mix two versions of the same library in the same application somehow then this shouldn't cause existing compiled apps to break.

Fixes issue #29. Thanks to Wohlstand for reviewing the patch!

→ <<cset 3f1e9bd24cec>>