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

Add onVolumeChange to SoundFrontEnd #3148

Merged
merged 6 commits into from
May 21, 2024

Conversation

ACrazyTown
Copy link
Contributor

info: #3147

This acts the same as volumeHandler except it's just a FlxSignal. Not sure what to do with volumeHandler, should it be marked as deprecated?

@Geokureli
Copy link
Member

Geokureli commented May 20, 2024

can you walk me through what the existing field volumeHandler is, and how it was intended to be used? This is the first time I'm coming across it

@ACrazyTown
Copy link
Contributor Author

ACrazyTown commented May 20, 2024

can you walk me through what the existing field volumeHandler is, and how it was intended to be used? This is the first time I'm coming across it

volumeHandler is just a function callback that gets called whenever the global volume is changed. It's meant to be replaced by your own function like this:

FlxG.sound.volumeHandler = function(volume:Float) {
   // do something when the volume is changed
};

Not sure what other use cases there might be but I personally used it to get openfl.media.Video to play nicely with Flixel's sound system

@Geokureli
Copy link
Member

Should we deprecate it?

@ACrazyTown
Copy link
Contributor Author

Should we deprecate it?

I don't see why not

@@ -36,6 +36,7 @@ class SoundFrontEnd
* Set this hook to get a callback whenever the volume changes.
* Function should take the form myVolumeHandler(volume:Float).
*/
@;deprecated("volumeHandler is deprecated, use onVolumeChange, instead")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there was a typo here, i just pushed a commit to fix it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

D'oh!

@Geokureli
Copy link
Member

It's still giving deprecation warnings:

SoundFrontEnd.hx:343: characters 7-20 : Warning : volumeHandler is deprecated, use onVolumeChange, instead
SoundFrontEnd.hx:345: characters 4-17 : Warning : volumeHandler is deprecated, use onVolumeChange, instead
SoundFrontEnd.hx:466: characters 7-20 : Warning : volumeHandler is deprecated, use onVolumeChange, instead
SoundFrontEnd.hx:468: characters 4-17 : Warning : volumeHandler is deprecated, use onVolumeChange, instead

I'll have to mess with this locally, when i have time later, unless you can figure it out, first

@ACrazyTown
Copy link
Contributor Author

ACrazyTown commented May 21, 2024

I don't seem to be getting any deprecation warnings. I tried building the TurnBasedRPG demo on hl & html5, debug and release. Uncommenting the fix you added makes them show up, otherwise I get nothing

@Geokureli
Copy link
Member

Geokureli commented May 21, 2024

I was looking at the 4.2.5 job, which doesn't have the ability to suppress warnings in methods. it does seem to work fine on 4.3.3

@Geokureli Geokureli merged commit 05f510c into HaxeFlixel:dev May 21, 2024
11 checks passed
@Geokureli Geokureli added this to the 5.9.0 milestone May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants