-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
UAudioBank #1274
UAudioBank #1274
Conversation
… spatialized sounds.
@@ -17,13 +17,17 @@ private IEnumerator ContinouslyPlaySounds() | |||
{ | |||
while (true) | |||
{ | |||
UAudioManager.Instance.PlayEvent("Laser"); | |||
UAudioManager.Instance.PlayEvent("Vocals3d"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way we can make this not based on strings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every event needs a unique ID and to support audio event banks in downloadable asset bundles I'd like to avoid any build time dependencies (i.e. generating an ID table). There are several ideas kicking around in my head on how to do this, but each one requires some more thought / discussion.
For a pure unity editor experience, we could provide an editor attribute that would convert (in this case) a string field into a dropdown to select an event from, but this still doesn't help with the potential of event renaming and the connection being "lost".
Perhaps this is one for a future update after more thought.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might be able to do something similar with how we handle voice keywords and commands in the main toolkit, with a serialized array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a custom property drawer to alleviate having to enter string values manually.
source.SetCustomCurve(AudioSourceCurveType.CustomRolloff, audioEvent.AttenuationCurve); | ||
//source.SetCustomCurve(AudioSourceCurveType.SpatialBlend, audioEvent.SpatialCurve); | ||
//source.SetCustomCurve(AudioSourceCurveType.Spread, audioEvent.SpreadCurve); | ||
//source.SetCustomCurve(AudioSourceCurveType.ReverbZoneMix, audioEvent.ReverbCurve); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these are no longer needed, we can remove them outright.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
{ | ||
public T[] Events; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: extra line space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trimmed the space.
[Tooltip("The lowpass attenuation curve for simple 3D sounds. Only used when positioning is set to 3D")] | ||
public AnimationCurve LowPassCurve = AnimationCurve.EaseInOut(0f, 0f, 1f, 0f); // by default no lowpass | ||
//[Tooltip("The lowpass attenuation curve for simple 3D sounds. Only used when positioning is set to 3D")] | ||
//public AnimationCurve LowPassCurve = AnimationCurve.EaseInOut(0f, 0f, 1f, 0f); // by default no lowpass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove these as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
MinGain, | ||
MaxGain, | ||
UnityGainDistance | ||
RoomSize = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How will this affect people already using the parameters?
Will they get out of index exceptions?
Maybe we should catch these where it's used and reassign them, or warn the users that the parameter is set incorrectly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shouldn't, as these values map to the parameter indices in the spatializer plugin (in fact, they wind up cast as an int when calling into the plugin).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be safe, this enum is private to the class, and the functions that used them are now marked obsolete and have no code in them.
@@ -61,29 +59,29 @@ public static void SetRoomSize(AudioSource audioSource, SpatialSoundRoomSizes ro | |||
/// </summary> | |||
/// <param name="audioSource">The AudioSource on which the minimum gain will be set.</param> | |||
/// <param name="room">The desired minimum gain, in decibels.</param> | |||
[Obsolete("This parameter is no longer used, use the unity volume curve.")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Could you also call out the new ClassName.MethodName()
in this way please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the warning text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There isn't really a new function, volume attenuation is now controlled by the unity volume curve on the audio source. This is now a parameter in the audio event and replaces the Min/Max gain and UnityGainDistance parameters that used to be in the audio event.
Add an [AudioEvent] attribute to a string field to replace the default UI with a dropdown. Right-Click on the dropdown to enable/disable bank names in the list.
{ | ||
// Don't show this if there are no embedded events in this audio manager | ||
if (MyTarget.EditorEvents == null || MyTarget.EditorEvents.Length == 0) | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: braces.
[SerializeField] | ||
[System.Obsolete] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What replaces this?
What is the new usage?
Why didn't the usages get updated in the EditorEvents
down below?
@DaveSullivanAtWork What are the steps for devs to update to the new system? Trying to write this up for the next release. |
That should be it. You can now move the generated asset around and rename it as required. (Default asset name is ConvertedAudioBank) Points to note: |
Note: Addresses #1211 |
Update to the UAudioManager system.
The main purpose of this update was to bring the spatial sound support up to date with the changes in the spatilaizer and to separate out the audio event data from the scene. This allows for "skinnable" sound such as dynamically loading the correct sound bank based on language or user preferences.