Minor stuff

[Goor]

SECTR_PropagationSource has some unneeded null checks. If portal is null, then you will already get exception when getting the Center property value. I believe you can just remove the 'portal' null checks.

I would not bother you with this unless I wasn't sure if it was a possible bug. Maybe portal can actually be null? 😄

Does it make sense to have this thread and maybe post some minor things I noticed here and there? Not simple micro optimizations 🙂 Only when interchangeable with a possible bug.

See the attached screenshot for more details.

[Peter]

Yes, those null check look like they are not needed. I would need to review this some more, but I think the code should rather check if the portal is null once at the beginning, and if yes, skip this loop iteration to the next. I've just noted this down in our backlog for review for the next Sectr update. Feel free to add more things that you find in here.

[Goor]

Thanks @Peter for the quick response. You guys rock. Attention to detail is insane.

I found one more thing during my journey through the code as I attempt to understand everything about the system. We actually use it to determine what AI can/cannot hear as well and it works SO MUCH BETTER than just using raycasts and maybe nav meshes. SECTR graph just gives you free and accurate information 😄

The thing I found is duplicated volume attenuation code in SECTR_PropagationSource class. It is even mentioned there, that it should just be a copy of same code from AudioSystem itself. But when you compare the two, they don't match.

It seems to me that the code in the AudioSystem has been updated as the time passed, but the programmer updating it didn't know about this copy/paste clone in the PropagationSource class.
I doubt that the PropagationSource attenuation version is actually newer. So it should probably be solved by replacing the PropagationSource attenuation with AudioSystem attenuation completely. Unless the differences were intentional, but the comment "Attenuation code is duplicated from SECTR_AudioSystem, but I don't want to pay extra function cal overhead so..." suggests otherwise.

Edited January 1, 2022 by Goor

[Bryan]

On 1/1/2022 at 3:24 PM, Goor said:

Thanks @Peter for the quick response. You guys rock. Attention to detail is insane.

I found one more thing during my journey through the code as I attempt to understand everything about the system. We actually use it to determine what AI can/cannot hear as well and it works SO MUCH BETTER than just using raycasts and maybe nav meshes. SECTR graph just gives you free and accurate information 😄

The thing I found is duplicated volume attenuation code in SECTR_PropagationSource class. It is even mentioned there, that it should just be a copy of same code from AudioSystem itself. But when you compare the two, they don't match.

It seems to me that the code in the AudioSystem has been updated as the time passed, but the programmer updating it didn't know about this copy/paste clone in the PropagationSource class.
I doubt that the PropagationSource attenuation version is actually newer. So it should probably be solved by replacing the PropagationSource attenuation with AudioSystem attenuation completely. Unless the differences were intentional, but the comment "Attenuation code is duplicated from SECTR_AudioSystem, but I don't want to pay extra function cal overhead so..." suggests otherwise.

Thank you for this information. 
We will be looking into this! 
 

[Goor]

Hi,

One more thing I noticed. It would be nice to use 'Renderer.forceRenderingOff' instead of 'renderer.enabled' for SECTR driven culling.

The new forceRenderingOff property was introduced in Unity 2019.3 specifically for custom occlusion solutions. It is more performant and already being used by the Perfect Culling plugin for example.

Just a hint on how a few more FPS could be squeezed with very little cost 🙂

Thanks for being so responsive with this autistic thread ❤️

[Peter]

18 hours ago, Goor said:

One more thing I noticed. It would be nice to use 'Renderer.forceRenderingOff' instead of 'renderer.enabled' for SECTR driven culling.

Good suggestion, I was not aware that this had an impact on performance.