whitexroft
Guest
Posts: 0
|
Post by whitexroft on Nov 4, 2020 10:52:01 GMT
I have a question regarding following snippet of code: imgur.com/a/Ys9MMBEThe way I see it, there is a foreach iteration through values, and then there is a check wether the dictionary contains value's name (it does, the key is the same as value's name), and then you retrieve the same value from the same dictionary and use it. Even thought you already had the value from the very beginning. I mean sure, it works, but considering a high amount of items in the dictionary, this creates an impressive performance spike on scene load. Another question regards the way you destroy dynamic buses in DynamicSoundGroupCreator. Imagine a situation, where your user, when setting a bus for an audio group, instead of selecting "EXISTING", would say "NEW", but then type in an existing bus, thinking it will do. What happens now, is that when the group is unloading (in a game with subscenes loading and unloading), this group would destroy a pre existing bus, and basically void all sounds that were running on in for the leftover entirety of game. |
|
|
|
Post by DarkTonic Dev on Nov 4, 2020 16:52:39 GMT
First question: the Sound Group could be destroyed (Scene change) or removed by a Dynamic Sound Group Creator after the 1st method and before the 2nd method runs. Dictionaries are crazy fast lookup - the fastest. It shouldn't be an issue.
I don't appreciate your thread title, and have renamed it.
Second question: you will know that is happening if you turn on "Error On Duplicate Items" in the DGSC Inspector. That is off by default in case you have multiple of the same DSGC in the Scene (on a prefab for its sounds) because that would be spam. If you made that mistake, the sounds in the bus won't be deleted, they'll just have no bus.
|
|
whitexroft
Guest
Posts: 0
|
Post by whitexroft on Nov 5, 2020 10:03:59 GMT
I apologize for being rude. So let's break this down. This is my metrics imgur.com/a/Mscb6PrWhat I am trying to get rid of is spike on my scene loading. What you see on the image is that, pretty much, only "Contains" and dict retrieval takes 50% of all Awake stage. So when you are saying that a sound group can be removed between 2 functions, I do not understand that. First function calls the other synchroniously, there is no idling between, and no removal code between them. Secondly, if that was a case, you would have gotten an exception prior calling the second function - meaning val.Group is null. Unless you have the second function called on a parallel thread, I do not see the issue you are describing. Regarding the second question, sure, yeah, "NO BUS" is pretty much equals to "no sound in the game". What I want to also point out is that once mistake is made, there is no way to easy fix it without going through all groups and reinitializing the Bus field, or setting isExisting in inspector debug mode. Considering there is a dynamic bus with the same name as in pre existing buses list. Why not simply fetch it from the list instead of voiding it? |
|
|
|
Post by DarkTonic Dev on Nov 5, 2020 21:34:49 GMT
I apologize for being rude. So let's break this down. This is my metrics imgur.com/a/Mscb6PrWhat I am trying to get rid of is spike on my scene loading. What you see on the image is that, pretty much, only "Contains" and dict retrieval takes 50% of all Awake stage. So when you are saying that a sound group can be removed between 2 functions, I do not understand that. First function calls the other synchroniously, there is no idling between, and no removal code between them. Secondly, if that was a case, you would have gotten an exception prior calling the second function - meaning val.Group is null. Unless you have the second function called on a parallel thread, I do not see the issue you are describing. Regarding the second question, sure, yeah, "NO BUS" is pretty much equals to "no sound in the game". What I want to also point out is that once mistake is made, there is no way to easy fix it without going through all groups and reinitializing the Bus field, or setting isExisting in inspector debug mode. Considering there is a dynamic bus with the same name as in pre existing buses list. Why not simply fetch it from the list instead of voiding it? I don't have anything running on a parallel thread, but I don't know if Unity might spawn another parallel thread when the Dynamic Sound Group Creator calls its Disable method. Perhaps me being paranoid, but no possibility of an exception occurring. If you can show me some documentation that Unity is entirely single-threaded (but I believe they changed some things to be separate thread awhile back) then this can be removed. No Bus does not equal "no sound in the game". Things assigned to "no bus" still play their Audio Sources unless the Sound Group is muted. If you make the mistake you can simply delete the "new bus" from DSGC and select "Existing Bus", and provide the name. If you are saying that it's not recoverable because you're using "Save Runtime Changes" option in Master Audio, yeah that checkbox is risky. I never use it. Don't forget you can use "Bulk Group Changes" to assign all selected Groups to a bus at once instead of individually. I would say that fetching an existing bus when you said "new" is user error and I'd want the user to be aware of it. I don't like automatically trying to fix something that may be unintended without any indication. We could consider logging a warning and using the existing one, then not delete the bus later if we fetched it. So with all that said what would you advise? -Brian |
|
whitexroft
Guest
Posts: 0
|
Post by whitexroft on Nov 6, 2020 10:16:20 GMT
Alright, since there was a warning checkbox option that I should have used, that is fair enough. I don't know your use cases, and I should't jump to conclusions. However I would mention that silently destroying an existing bus like that actually IS unintended, and happens without any indication  It does make sense to add a warning and ignore the destruction as you suggest. I can show you a monobeh callback documentation where it specifically tells it is coming from a parallel thread docs.unity3d.com/ScriptReference/MonoBehaviour.OnAudioFilterRead.htmlIt mentions that calling "into many Unity functions is not allowed", as from your own multi threaded code. It's not only that you cannot create/destroy, you wouldnt be able even read a gameObject's or unity assets' properties, like their name. But anyway, OnEnable/OnDisable are not threaded, as all MonoBeh callbacks, but the one above. There is no way the fuction can be interrupted. |
|
|
|
Post by DarkTonic Dev on Nov 6, 2020 20:56:25 GMT
Ok, I've removed the 2nd check in SilenceGroup and UnsilenceGroup. Thanks.
I'll be looking into fixing the other issue soon and will report back.
|
|
|
|
Post by DarkTonic Dev on Nov 6, 2020 23:25:55 GMT
Fixed the other bug. I had a field for this already but didn't use it. Modify the the top of this method as follows in DynamicSoundGroupCreator.cs
public void RemoveItems() {
// delete any buses we created too
// ReSharper disable once ForCanBeConvertedToForeach
for (var i = 0; i < groupBuses.Count; i++) {
var aBus = groupBuses[i];
if (aBus.isExisting) {
continue; // don't delete!
}
var existingBus = MasterAudio.GrabBusByName(aBus.busName);
if (existingBus != null && !existingBus.isTemporary)
{
continue; // don't delete, it was an existing bus you used because it already existed and you couldn't create it.
}
MasterAudio.DeleteBusByName(aBus.busName);
}
|
|
whitexroft
Guest
Posts: 0
|
Post by whitexroft on Nov 8, 2020 12:24:00 GMT
Regarding the checks, it's not only them I'd remove, but also the value retrievaæ, since you already have the group, that you are muting inside the foreach loop. Unless you have a use case where GameObjectName is not the same as the key in the dictionary? But hey, thanks for looking into it. If I find something else, I'll get back to you |
|
|
|
Post by DarkTonic Dev on Nov 10, 2020 16:48:05 GMT
Ah, I see. Ok, fixed that as well.
|
|
