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

Enable developer customers to access AudioSource at development time #436

Merged

Conversation

david-c-kline
Copy link
Contributor

@david-c-kline david-c-kline commented Aug 22, 2023

This change serializes the grabReleaseAudioSource and passNotchAudioSource AudioSource fields, so that developer customers can configure options such as AudioMixerGroup, spatialization and play on awake.

Fixes: #435

Verified in Unity editor by:

  • Adding a single AudioSource to the slider in the HandInteractionExamplesScene
  • Assigning the manually attached AudioSource to the Grab/Release and PassNotch fields (newly serialized)
  • Playing the scene in the editor and moving the slider
  • Noting
    • No additional AudioSource objects were added to the slider
    • Audio plays correctly
    • The expected AudioSource parameters are updated while interacting with the slider

@david-c-kline david-c-kline added the Package: UX Core The Project ux core package is impacted by this issue. label Aug 22, 2023
@david-c-kline david-c-kline self-assigned this Aug 22, 2023
@david-c-kline david-c-kline marked this pull request as ready for review August 22, 2023 22:35
Copy link
Contributor

@AMollis AMollis left a comment

Choose a reason for hiding this comment

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

Signing off on changes.

@david-c-kline , can you please look into refactoring to use a single audio source? Looking at the history of the file (from our internal repository) looks like @MaxWang-MS added the two audio sources when they first created the file.

I've opened an issue to track refactoring the two audio sources.

@david-c-kline
Copy link
Contributor Author

Signing off on changes.

@david-c-kline , can you please look into refactoring to use a single audio source? Looking at the history of the file (from our internal repository) looks like @MaxWang-MS added the two audio sources when they first created the file.

I've opened an issue to track refactoring the two audio sources.

Will do. One thing to note is that using a single audio source will cause the grab/release sounds to have their pitch altered based on the position of the slider thumb (the pass notch code sweeps the frequency of the audio source).

If we are ok with that, I will look into #438, otherwise, we should continue using two audio sources.

@david-c-kline david-c-kline merged commit 1d61251 into MixedRealityToolkit:main Aug 23, 2023
@david-c-kline david-c-kline deleted the sliderAudioSource_435 branch July 24, 2024 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: UX Core The Project ux core package is impacted by this issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slider prefab should have and AudioSource pre-attached
2 participants