mmitch / gbsplay

gameboy sound player
https://mmitch.github.io/gbsplay/
Other
98 stars 20 forks source link

Implement single-song repeat mode #72

Closed ProfOak closed 2 years ago

ProfOak commented 2 years ago

Hey all, not sure how important this is to anyone else but I sometimes like to loop a song.

Using the command line flag -L will now loop a single song repeatedly.

codecov[bot] commented 2 years ago

Codecov Report

Merging #72 (128c633) into master (779e4fe) will decrease coverage by 0.06%. The diff coverage is 50.00%.

Impacted Files Coverage Δ
player.c 40.35% <50.00%> (-0.58%) :arrow_down:
ranma commented 2 years ago

Instead of adding a new flag, I think it would be reasonable that if you set -l for loop mode and request a specific subsong, that subsong will be played on loop.

ranma commented 2 years ago

Actually, you can already loop a single song, e.g.: gbsplay -l foo.gbs 2 2 will loop subsong 2 repeatedly

ProfOak commented 2 years ago

Oh wow I totally missed that. Thanks for the tip!

mmitch commented 2 years ago

Actually there is a subtle difference between -L $file 3 and -l $file 3 3:

Is this a use-case that would warrant the -L? It might be useful for situations like repeat it until you can't hear it any more; manually change subsong; goto 1.

ProfOak commented 2 years ago

I'll re-open and let you make the call as the project lead. As long as there's some way to loop songs I'll be happy.

For some reason this is the only gbs player that properly emulates everything thrown at it. Music players that integrate Game Music Emu fail to play songs correctly and even in some cases crash the music application. So thanks for creating and maintaining a great program!

ProfOak commented 2 years ago

Is this a use-case that would warrant the -L?

Sorry I wasn't clear before. Yes your assumption is correct that this is the intended behavior. You can keep single-song-repeat mode on and change the subsong without restarting the application.

mmitch commented 2 years ago

I think it's a useful feature and it also seems legit to use both -l and -L together. I'm currently thinking about how to describe the difference between both options. What about:

-l    loop playlist
-L    loop current subsong

My intention with current subsong is to point out that it is still possible to change the subsong manually. Would anybody besides me understand that? What are your thoughts?

ProfOak commented 2 years ago

Are you suggesting a sub-range of subsongs played as a playlist? For example, suppose a gbs pack had 10 subsongs. Then the user runs the following command.

$ gbsplay -L -l songs.gbs 2 4

subsong 2 begins playing and doesn't change until the user presses n or p n is pressed, subsong 3 plays n is pressed, subsong 4 plays n is pressed, would subsong 2 play in this scenario?

If that's the case I don't think this is how the -l flag currently works and might warrant a new GitHub issue to be made.

Running the command then pressing n a few times will take you to subsong 5 or later.

$ gbsplay -l songs.gbs 2 4

Only once the current subsong reaches 2 through 4, it will loop through subsongs (2-4] again.

mmitch commented 2 years ago

You're right: -l only loops the subsong playlist when the subsong change happens automatically. -L prevents the automatic change. So using both flags at the same time is bogus.

-l has always acted this way: First and last selected subsong could be 'overridden' by manual subsong changes. I don't know if this should change. Every change breaks somebody's workflow ;-)

I think the easiest course of action would be to just add something like -l and -L are mutually exclusive to the manpage.

But I don't know about the short help text. Should that be mentioned there? Or would it be clear that both options don't work together? (cough cough in my last post I wrote the opposite ;-)

I still like -L loop current subsong. What about -l loop between first and last selected subsong instead of -l loop playlist?

I'll merge the change, but we could still tweak the wording.

ranma commented 2 years ago

BTW I think a fair number of GBS have the subsong run forever, so if you disable the gbsplay subsong timeout using -t 0 you'll get better looping behavior for those.

With -l the other subsongs will play once until you reach song 3 again which will then loop indefinitely

That's fairly broken behavior, it should just behave like the -L option added here. :)

mmitch commented 2 years ago

Should the "loop single subsong" mode (-L) temporarily (regarding #74 which will add toggling of loop modes) reduce the subsong timeout (-t) to 0? If we want to loop the subsong, we don't have to cut it short just to restart it.

ranma commented 2 years ago

Should the "loop single subsong" mode (-L) temporarily (regarding #74 which will add toggling of loop modes) reduce the subsong timeout (-t) to 0?

Yes, I think it should. Implemented in https://github.com/mmitch/gbsplay/commit/39aeb12704f06e7f42e592dcddc1dd31c1dcd5b9

mmitch commented 2 years ago

I've added a description of the loop modes to the man page with 04d709f.
I've also changed the help text wording to match the interactive display with 857eaf9. (This also satisfies my urge to find a different wording.)

I think we're ready to close this issue or am I missing anything? In any case thanks for the nice feature!

mmitch commented 2 years ago

d'oh: "merged" already means closed, I just have to set the thread to done in my notifications ;-)