rombust / Methane

Super Methane Brothers Game
GNU General Public License v2.0
6 stars 4 forks source link

Music doesn't loop on Linux/ALSA #7

Open xperia64 opened 1 year ago

xperia64 commented 1 year ago

Built using ClanLib v4.1.0 and mikmod 3.3.11.1.

The mod music doesn't loop for some reason in the latest v2.0.1 release.

I've traced it to ClanLib's SoundBuffer_Session_Impl::mix_to returning false when Methane's UpdateModule tries playing the module again, but I'm not really sure what it's supposed to be doing here.

And of course I do see SoundProvider_MikMod_Session::play being called here, but nothing else? I didn't dig too deep. Not really sure what's failing.

TheBoctor commented 8 months ago

It looks like the set_looping() function in ClanMikmod/soundprovider_mikmod_session.cpp is never actually called. This does properly set the wrap+loop bits in mikmod, but none of the interfaces exposing play() in target.cpp or sound.cpp ever set any BGM to loop.

Personally, I would suggest adding a second/optional bool argument to one of the wrappers around play(), which could simply pass the bit to set_looping() when playing a valid module. e.g., PlayModule(int id) becomes PlayModule(int id, bool looping = true).

For a quick and dirty fix to enjoy the game with looping BGM right now (I have only tried this on Linux, but like I said, this seems to be a high-level issue so it is probably not platform-specific) you can add a line to target.hpp. At the end of the CGameTarget::PlayModule definition, right before the m_bSessionActive = true; line, add: m_Session.set_looping(true);.

rombust commented 8 months ago

Fixed. Apologies in the delay, I had forgot. Thank you all for spotted and suggesting fixes, if really did help. After an hour of searching, and adding fixes, I noticed that it was just a silly error! "m_Session.play(); " should be "PlayModule(id);" in UpdateModule(int id);

TheBoctor commented 8 months ago

While this fix largely works, it seems to be enabling looping at the ClanLib/session level, instead of at the MikMod level, if that makes sense.

When the BGM reaches the end of the song, it does loop, but it also replays the whole intro. However, calling set_looping(true) on the session (in the PlayModule function) results in the correct(?) behavior: The MOD player itself reaches the command at the end of the song and seamlessly loops back to the correct pattern, skipping the intro.

I'm not sure which behavior you prefer, but when I test the original on my Amiga 500, it's definitely not repeating the intro.

rombust commented 8 months ago

I see. Using milkytracker on linux to play "tune1.mod", when the tune gets to the end, it skips the intro pattern.

This error would have been there since around 2001 !

I don't know why it simply didn't call "set_looping".

I started this project to learn C++ , using the CFront compiler. Coding decisions I made were certainly questionable! Despite there has been some work over the years, the code is certainly dated.

TheBoctor commented 7 months ago

C++ sources from the early 2000's might all be dated now, for different reasons. Things like STL containers weren't as fast back then, or so I've been told. You picked an ambitious project, though, and it looks like it paid off. The portability/readability are still good enough that I was able to skim through and add minor features in under an hour. On a personal fork of the code, I was able to patch in the BGM fix, 1P+2P gamepad support, and fix some wall collision exploits.

Since the music looping has been figured out, though, these are probably best discussed in separate issues. Or I can share the changed source in a forked repo, so that you or anyone interested can reference/cherrypick it at their own leisure.

rombust commented 7 months ago

I am sure any changes will be an improvement. When you have finished, send a pull request, and I will apply it.

I need to dig out the original 68000 code, and see how the pause mode was implemented. The game seems to be missing it.

TheBoctor commented 7 months ago

I'll work on preparing the known-good changes and open the request once I'm sure, cheers.

This is somewhat of a tangent, but if possible, would you be able to check how the velocities of little objects (points pickups, marbles, oil, flames, etc) are set when they're created? It seems like they fly much further in the original game.