Jump to content

Minor stuff


Goor
 Share

Recommended Posts

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.

image.thumb.png.41bf7232a648974877e0b565ac3c644e.png

Link to comment
Share on other sites

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.

  • Like 1
Link to comment
Share on other sites

Posted (edited)

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.

image.thumb.png.6e5fae161d9fbcf8ba49ea5ff6003d32.png

Edited by Goor
Link to comment
Share on other sites

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.

image.thumb.png.6e5fae161d9fbcf8ba49ea5ff6003d32.png

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

  • Like 1
Link to comment
Share on other sites

  • 3 weeks later...

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 ❤️

  • Like 1
Link to comment
Share on other sites

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.

  • Like 1
Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

  • Tell a friend

    Love Unity 3D World & Game Creation Tools - Canopy - Procedural Worlds? Tell a friend!
  • Interesting Articles

    1. 2

      I'm getting tired of doing islands and craters

    2. 2

      I'm getting tired of doing islands and craters

    3. 1

      Problems with latest Gaia Update

    4. 1

      Adding shoreline or waterline effects, beaches, river banks, etc.

    5. 1

      Just upgraded my Canopy to Pro

×
×
  • Create New...