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 pitch support for FlxSound #1465

Closed
wants to merge 8 commits into from
Closed

Conversation

kevinresol
Copy link
Contributor

@Gama11
Copy link
Member

Gama11 commented Apr 6, 2015

In what OpenFL version was this added (i.e., would we be dropping support for any older OpenFL version that's still working fine with flixel by adding this without any openfl version conditionals)?.

What about openfl next? In that thread Joshua seems to suggest that this is only a thing for openfl legacy.

@kevinresol
Copy link
Contributor Author

from this commit
we can add the legacy compiler flag maybe

#if (cpp || neko)
_channel.pitch = v;
#else
FlxG.log.warn("Pitch is only supported on OpenAL targets (i.e. cpp & neko)");
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if it wouldn't be better to only have this property exist for cpp and neko in the first place. That's how we handle most of the input stuff in FlxG for example. Then again. other features like post processing use this approach (except for the warn() call), so I guess we're not really consistent about it...


private function set_pitch(v:Float):Float
{
return _pitch = _channel.pitch = v;
Copy link
Member

Choose a reason for hiding this comment

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

This can produce null exceptions since _channel might be null.

Gama11 added a commit that referenced this pull request Apr 12, 2015
@Gama11
Copy link
Member

Gama11 commented Apr 12, 2015

GitHub doesn't send notifications for new commits on pull requests, so it's a good idea to comment whenever you add one.

I just manually merged this: 415f410 Thanks for the implementation, this is a really cool feature!

@Gama11 Gama11 closed this Apr 12, 2015
@kevinresol
Copy link
Contributor Author

@Gama11 roger that, will comment next time.

@ashes999
Copy link
Contributor

ashes999 commented Apr 8, 2016

Sorry to necro. @Gama11 is this in a released version of HaxeFlixel? I didn't see any mention of pitch when I looked at the api.haxeflixel.org entry for FlxSound.

@Gama11
Copy link
Member

Gama11 commented Apr 8, 2016

Yes. The api docs are currently only generated for flash.

@ashes999
Copy link
Contributor

ashes999 commented Apr 8, 2016

Thanks for clarifying. (We should probably have an issue to address the docs ...)

@Gama11
Copy link
Member

Gama11 commented Apr 8, 2016

We do. HaxeFlixel/flixel-docs#132

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.

3 participants