Skip to content
This repository has been archived by the owner on Nov 30, 2020. It is now read-only.

Feature: Use child colliders #227

Open
wants to merge 4 commits into
base: v2
Choose a base branch
from
Open

Conversation

mikegeig
Copy link

I added the ability to use children objects and have their colliders apply to the post processing volume. I also added the required Inspector toggle and gizmo functionality. This is very useful as it allows you to more easily place volumes by moving game objects around instead of moving the center points of colliders. Additionally, you can now rotate colliders by rotating the child game objects. Finally, you can also more easily organize and label the parts of your volume as the colliders are components on separate game objects.

unity_2017-07-31_17-19-00

@mikegeig mikegeig changed the title Feature use child colliders Feature: Use child colliders Jul 31, 2017
@mikegeig mikegeig requested a review from Chman July 31, 2017 21:23
@Chman
Copy link
Contributor

Chman commented Aug 10, 2017

Thanks for the PR. It's an interesting feature but I have a few concerns.

  • Performances: traversing game objects and their childrens to find components isn't free, and potentially doing it on every frame is a scary thought.
  • UI noise: the Layer and Volume components are getting quite crowded and I'm trying to make them as simple as possible to reduce any noise in this regards. I'm not entirely convinced that adding a toggle to every volume for a feature that only a handful of users might use is a good thing.
  • UX incoherencies: as nice as such a feature would be in some cases, it also fails to take into account what can be done currently with the system in place. That is, you can easily sort your volumes in the Hierarchy and essentially have volumes inside other volumes and so on while keeping it clean and easy to traverse (for the user anyway). Simple design, expected result. Adding a toggle to traverse child colliders means you can't do that anymore. Or rather, you can, but you have to make sure you haven't toggled the checkbox. And if you do, you have to remember not to add any sub-volumes or it'll break (what happens if a volumes with a profile is added inside another volume that takes sub-colliders into account?). Overall it'll get messy and confusing quite fast for the sake of having an alternative way of doing what you can already do by adding multiple colliders to a single volume.

So it is an interesting idea for sure, but one that will add another layer of global complexity to make another existing sub-feature of the framework a bit easier to use. I'm not sure we want to go into that direction... I'm opened to discussion though.

Edit: note that post-processing profiles are assets, you can use the same asset in different volumes and essentially do the same thing you propose to do here (instead of having multiple colliders for a single volume, although that would be more optimal).

@mikegeig
Copy link
Author

Hmm, I'm not sure I agree. In my opinion:

  • Performance: do we have a speed comparison of GetComponents versus GetComponentsInChildren? If not, I may have to benchmark that

  • UI Noise: certainly this is an understandable concern, but I believe this change would be of great use and far worth a single line addition to the component. One of the great advantages of our post fx stack is the ability to create custom volume shapes by stacking colliders. I feel this advantage is hinders by the "clunkiness" of manipulating multiple colliders on a single object. It also doesn't address the fact that rotating colliders is impossible without this feature

  • UX incoherencies: I can agree with this one, however I think this simply adds another way of working with the tool and more options for developers. Obviously, some code could be added to make it perform similar to a canvas, in that a canvas controls all child elements until another canvas is encountered in the hierarchy.

I guess I feel that using child colliders will be more popular than you indicate. I already use this version in many of my projects and the ability to easily move and rotate colliders as child objects (not to mention visualize them with a mesh renderer during prototyping) has been incredibly helpful. The entire reason I built this version is because I was trying to easily add a volume to a racetrack tunnel that was a part of a twisted spline. I was getting frustrated trying to get cubes to line up with a curved track. Using a mesh collider and increasing its blend distance also wasn't an option as the pivot of the mesh was perfectly centered (nor could it be with the shape). This solution solved the problem very quickly. Sure, it might add a UX quirk (though, could be cleaned up), but it improved my experience with the tool in many occasions by quite a lot.

@mikegeig
Copy link
Author

Out of curiosity I ran a benchmark for GetComponents versus GetComponentsInChildren. I used 1 object with 5 colliders and another object (with a collider) that had 4 children with colliders. I performed a million iterations of each.

  • GetComponents grabbed all 5 colliders (a million times) in .3 seconds on my PC.

  • GetComponentsInChildren grabbed all 5 colliders (a million times) in .35 seconds.

So, it is a little slower to find in children, but not by that much. Granted, the test wasn't exhaustive of all edge cases, but it is 2:15 am here :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants