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

mix: Add support for the majority of missing & new functions in newer SDL_mixer versions #612

Merged
merged 4 commits into from
Dec 5, 2024

Conversation

Necklaces
Copy link
Contributor

This PR adds support for:

2.8.0

  • GetNumTracks
  • PauseAudio
  • StartTrack

2.6.0

  • GetMusicAlbumTag
  • GetMusicArtistTag
  • GetMusicCopyrightTag
  • GetMusicLoopEndTime
  • GetMusicLoopLengthTime
  • GetMusicLoopStartTime
  • GetMusicPosition
  • GetMusicTitle
  • GetMusicTitleTag
  • GetMusicVolume
  • HasMusicDecoder
  • MasterVolume
  • ModMusicJumpToOrder
  • MusicDuration
  • SetTimidityCfg

2.0.2

  • HasChunkDecoder

2.0.0

  • LinkedVersion

The only remaining one as far as I can tell is UnregisterEffect, which itself says "You may not need to call this at all, unless you need to stop an effect from processing in the middle of a chunk's playback.", so I let it be for now.

Then there are a couple, like GetSynchroValue and SetSynchroValue which say "This function does nothing, do not use.", I didn't touch those. There's also GetMusicHookData but a comment in the code says data is never sent in so there's no need to ever get data, I didn't touch that one either.

I also updated the TODO,md to reflect these changes, and crossed the ones I found to already be added, perhaps I was wrong about some of those but they looked complete to me at least.

I tested the ones I could, maybe we should add some examples to show them being used.

Let me know if there's anything I need to change or add!

Including SDL_mixer version 2.0.0, 2.0.2, 2.6.0, and 2.8.0
Added the following functions:

in sdl_mixer.go:
- LinkedVersion
- HasChunkDecoder
- HasMusicDecoder
- GetMusicAlbumTag
- GetMusicArtistTag
- GetMusicCopyrightTag
- GetMusicLoopEndTime
- GetMusicLoopStartTime
- GetMusicPosition
- GetMusicTitle
- GetMusicTitleTag
- GetMusicVolume
- MasterVolume
- ModMusicJumpToOrder
- GetNumTracks
- PauseAudio
- StartTrack

in midi.go:
- SetTimidityCfg
@veeableful
Copy link
Contributor

That's a lot of work, thank you so much @Necklaces! Do you think it makes sense to convert functions that involve objects such as *sdl.Music into methods? I tried to make such functions into methods and remove Get if it's a retrieval function in an attempt to follow Go's conventions.

@Necklaces
Copy link
Contributor Author

Right! I missed that, just to be clear you want all of the *sdl.Music functions to be callable from the music object? Like the "Play" function is set up with func (music *Music) Play([...], I assume?
Come to think of it, some of them do say in the SDL_mixer documentation that they will fall back to the music that is already playing if a NULL is given, so how would we solve that use case? Perhaps we could keep it as it is and also add the relevant methods to the music object as well?

These functions, correct?

  • Mix_GetMusicAlbumTag
  • Mix_GetMusicArtistTag
  • Mix_GetMusicCopyrightTag
  • Mix_MusicDuration
  • Mix_GetMusicLoopEndTime
  • Mix_GetMusicLoopLengthTime
  • Mix_GetMusicLoopStartTime
  • Mix_GetMusicPosition
  • Mix_GetMusicTitle
  • Mix_GetMusicVolume
  • Mix_GetNumTracks
  • Mix_StartTrack

And I'll fix that error, I copy pasted the function without renaming it haha.

@veeableful veeableful merged commit 83cbc51 into veandco:master Dec 5, 2024
1 check failed
@veeableful
Copy link
Contributor

Thank you very much for the changes @Necklaces! You're right, let's keep them like this for now and add the methods later if needed.

@Necklaces
Copy link
Contributor Author

Oh, I was thinking about adding them to the PR after hearing your thoughts! I suppose I could make a new PR, hehe

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