Set the nav mesh visibility when a nav mesh is loaded #6320
Conversation
| if (isEnabled) { | ||
| addDebugMaterial(world, navEid); | ||
| } else { | ||
| removeDebugMaterial(world, navEid); |
There was a problem hiding this comment.
Does removeDebugMaterial really need to be called here? Does it mean there is a chance that debug material is already added?
|
Honestly setting the default visibility for nav mesh in audio debug system is very unintuitive to me. Ideally it is defaulted (as invisible) somewhere else like where nav mesh is created? It might be ok to merge this PR (after reflecting the review comments) for now and revisit this design later tho. |
|
Not sure about that. The audio-debug service is the only point were we manage the nav-mesh visibility so it doesn't feel wrong to me if we keep that responsibility there instead of spreading it. I'm going to merge this and we can think about it in the future. |
|
It is ok that debug audio system controls nav meshes visibility. But I think the default nav mesh visibility should be set up when it's initialized. The default nav mesh visibility shouldn't really rely to audio debug system. Imagine a case that audio debug system is removed due to a certain reason. It shouldn't affect the default nav mesh visibility. In general less dependencies better design. Also imagine that a new reader want to know where the default nav mesh is set. It is hard for the one to find from audio debug system because it requires to know background. More intuitive design more maintainable. |
Fixes #6315
This PR sets the nav-mesh visibility when a nav-mesh component is added to make sure that the mesh visibility is always aligned with the preference setting.