Search Unity

Extending Colliders

Discussion in 'Scripting' started by PedroGV, Jul 26, 2016.

  1. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    Question for Scripting Team: have you guys considered to extend all base collider components so that they include a public scriptable-object reference?

    This way we could use scriptable objects as a replacement of the tag property in order to avoid garbage allocation (and interop calls on .tag?). Also, there would be no need to create MonoBehaviour scripts on objects where you don't need a script at all but you need a collider (since the collider would expose a reference to the scriptable object).

    We could program alternative collision checks that avoid string comparisons with such feature.
     
    Vincent454 likes this.
  2. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,686
    Unity is really designed for component "has-a" approach, and you are suggesting a more "is-a" approach with inheritance. I recommend getting comfortable with the component "has-a" approach, as it will "play much nicer" with everything else in Unity.
     
    davidrochin likes this.
  3. Kiwasi

    Kiwasi

    Joined:
    Dec 5, 2013
    Posts:
    16,860
    Typically you can use GetComponent if you want to avoid tag checking.
     
  4. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    I'm already using GetComponent for such purpose.

    And no, this has nothing to do with deep hierarchies vs entity-component approach.

    The reference will avoid one script and therefore one GetComponent lookup (even though they're cached). And will also be backward compatible. So the "has a" still applies. You just get that info directly from the collider/colliderinfo itself. You don't need a collider and a markup script.
     
  5. Kiwasi

    Kiwasi

    Joined:
    Dec 5, 2013
    Posts:
    16,860
    Throw it on the feedback page.

    I personally think it's already solved between the Layers, Tags and GetComponent. But others might agree with you.
     
  6. Eric5h5

    Eric5h5

    Volunteer Moderator Moderator

    Joined:
    Jul 19, 2006
    Posts:
    32,401
    Tags don't allocate garbage anyway; just use CompareTag rather than string comparisons.

    --Eric
     
    djfunkey and Kiwasi like this.
  7. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    Sometimes you need more than a full phrase comparison, like StartsWith. Scriptable objects are a great workaround for that.
     
  8. LightStriker

    LightStriker

    Joined:
    Aug 3, 2013
    Posts:
    2,717
    That must be new, because back then, calling it created a new string everytime it was used since the tag was store on the native side. Did it changed recently? String being not a value type, it meant a new object on the heap.
     
  9. Eric5h5

    Eric5h5

    Volunteer Moderator Moderator

    Joined:
    Jul 19, 2006
    Posts:
    32,401
    You can see in the profiler that string comparisons = allocations, CompareTag = no allocations. It's always been that way.

    --Eric
     
  10. LightStriker

    LightStriker

    Joined:
    Aug 3, 2013
    Posts:
    2,717
    So your first statement saying "Tags don't allocate garbage anyway" is right if and only if you don't read what's inside the tag and use the native CompareTag.

    Personally, I would just advise to ignore the tags and to never use them. I would even like it if Unity had options that would allow us to build our games while stripping some data we never use... like tags. Or object names. I wonder how much overall memory could be save by not having any tags or names.
     
  11. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    CompareTag still does an interop call, right? That may not happen for free, like calling Update and so on on a loop. Ditto for physic/trigger collision checks.

    My suggestion presents an alternative for those who deem tags as a limited road. You will be able to define your own category containers and do checks with neither garbage nor interop calls and without having to call GetComponent. It's just an exposed field in a collider.
     
  12. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
  13. LightStriker

    LightStriker

    Joined:
    Aug 3, 2013
    Posts:
    2,717
  14. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,686
    What on earth are you talking about?! It is just a function call that wraps up the string comparison, to make your code read better, rather than a string equality check inline.

    Everything Unity has done since Unity4 has gotten away from this sort of hard-wired design. It doesn't scale and it makes a bowl of spaghetti out of your API and object structure.

    Remember, we used to have GameObject.audio, .renderer, .camera and a whole host of other helper properties dating back to the earliest origins of Unity, and they were a persistent eyesore on GameObjects, permanently tying together huge clouds of varied and completely-unrelated objects.

    Now granted, those shortcuts were very handy and they were a great idea originally but their time has passed, so thank goodness they have been removed.

    Your proposal to hang a ScriptableObject on a Collider is even more specific and edge-case-y, and does not really help you solve any problem you can't solve another way. And then every time a Collider gets used somewhere, there would have to be a definition for a ScriptableObject available too, tying those two completely unrelated objects together.
     
  15. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    First of all, if you want to discuss ideas with me, please do it with respect.

    Now, for what I understand based on @Eric5h5 said, it's way more than a readability wrap-up since it does not allocate additional bytes.

    That is true for getters over components.

    But ScriptableObjects are used as a data repository that do not need to be attached to a gameobject as a component. And since it still can be associated using the editor as a reference in any component that expects it, it does not threaten the move of removing decorators.

    That is your opinion. Not mine.

    Not necessarily, it could be null. Nothing obliges you to use the feature, but if you want to use it, the scriptableobject you look for is expected to exists and be of a particular type, and thus, related.

    Maybe. But my idea here is to better categorize a gameobject even if you do not use it for physics.
     
  16. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    Being null is irrelevant. @Kurt-Dekker is commenting on the fact that ScriptableObject and Collider would be coupled together if you exposed one as a member of Collider.

    Honestly, I also don't see the problem this is trying to solve.
     
    Kurt-Dekker likes this.
  17. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    I don't see a problem if coupling them, since the new public member can be null. If you need it is there. If you don't it does not affect your code at all.

    Regarding the problem: there are many gameobjects that do not need a Monobehavior at all but do have a collider and sometimes a Tag is way to simplistic since you need more info and thus end up creating a complex tag like say "Env_Floor" or "Enemy_Deamon", so you end up using string comparisons like StartWith.

    Without the field you need to create a MonoBehaviour and attach it to those objects which with time becomes cumbersome, specially as the team in the project grows.
     
  18. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,686
    In C# the term "interop call" has a very specific meaning related to the namespace System.Runtime.InteropServices.

    It broadly refers to method calls that transition between managed code and unmanaged (sometimes called "native") code. Calling .CompareTag() shouldn't have anything to do with interop.

    If the definition of Collider includes a reference of ScriptableObject, then it creates a dependency, whether you use it or not.

    If you find yourself wanting to extend a well-established API, it is likely a signal for you to take a step back and assess what you are trying to accomplish.

    This will tend to guide you to better design of your code overall. If after using the API as intended you find it still lacking, then write wrappers to get your job done.

    If you find performance issues due to garbage collection, take a moment to measure where the garbage might be coming from before assuming it is from string comparisons and then suggesting modifying the API.
     
  19. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    Again - null is completely irrelevant. The existence of the member definition creates a dependency. Your code might be unaffected but it affects the design and maintainability of the Unity API. If you couple Collider to ScriptableObject then any changes to one could impact the other. Now you've got something meant to hold data and not exist in the game world tied, circuitously, to the Physics engine.
     
    Kurt-Dekker likes this.
  20. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    In this particular case it won't impact it since Unity won't use it neither within the collider itself nor in the physic engine. Think of it as a new tag datatype attached to the collider.
     
  21. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    Btw, I read that the tag was stored on the native side, so if ContainsTag accesses that field (or calls a native op that does the comparison) there must be a interop call somewhere, unless cached on the managed side and compared, say, as an integer hash.
     
  22. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    You're still confusing usage with definition. The mere existence of that field, regardless of who uses it or what it gets filled with, creates coupling.

    They did the opposite of what you're proposing when they removed the rigidbody property from GameObject. That created a dependency between GameObject and the physics engine. Whether or not you used it was irrelevant and this case actually caused WebPlayer downloads to be bigger because the physics engine had to be included whether you were actually using it or not, otherwise the engine didn't understand what a RigidBody was.
     
  23. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    I'm not confusing it at all. I know it creates coupling but I believe that, contrary to the rigidbody case, this won't present a problem and will bring an improvement to the engine.

    This is not a matter of not knowing concepts and principles of software engineering. It's a situation of different opinions. You think in this case adding a new dependency is not justified and I think it is.
     
  24. Eric5h5

    Eric5h5

    Volunteer Moderator Moderator

    Joined:
    Jul 19, 2006
    Posts:
    32,401
    It just seems really messy and unnecessarily complicated to me, sorry, far more so than the problem you're trying to solve.

    --Eric
     
    Kurt-Dekker likes this.
  25. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    Well, it may depend on the use case. But I believe it will improve workflow, specially on large levels.
     
  26. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,686
    It seems like any workflow issue that could be solved by adding a SO to a collider could be easily handled with a custom editor script. You _are_ using custom editor scripts to improve your workflow, right?
     
  27. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    Not for this case.
     
  28. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    Then I'm not sure why you kept trying to justify your point by saying that folks could just leave it null.

    In either case - I still think this is a situation easily solved by mechanisms that already exist in the engine and from an API perspective having a ScriptableObject field on Collider doesn't make any sense. I personally would not intuit the purpose of that field just by looking at the documentation for Collider. I'd say "why does Collider hold a reference to some data object that doesn't exist in the game world?". And "If SO's are just data and have no representation in the world why can one suddenly be bound to a real-world thing?"

    Then of course there's the implementation vagueness. If I have this Collider on a prefab do they all reference the same SO? Does the Collider magically create a new instance of this SO when it's parent GameObject is created? If the containing Collider is destroyed is the SO destroyed as well or is it the responsibility of the programmer to just know that they have to clean it up?
     
  29. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    For the same reason that you insist with the opposite: because I belive I'm right.

    It's simple: The SO already exists and persisted, so you just manually assign it to the public field of the corresponding colliders with Unity's inspector.

    I already do this but with a Monobehaviour that only has a public field of an SO. Then when a colision is checked I search for such SO to preceed properly.
     
  30. superpig

    superpig

    Drink more water! Unity Technologies

    Joined:
    Jan 16, 2011
    Posts:
    4,655
    I think we probably will not do this.

    Firstly, adding it at the Collider level would be really weird. Why colliders and not anything else? If we were to do it we'd do it at a higher level in the hierarchy, like Object.

    But secondly: it would add both increased memory usage (storing the references) and speed hits (processing the extra field, resolving the SO reference, etc) for everybody, when it's not at all clear that everybody would use it on a lot of objects.

    As @BoredMormon said, go with the GetComponent approach.
     
    KelsoMRK and Kurt-Dekker like this.
  31. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    I already have (please read my previous comment) and that's the reason for my suggestion: I believe it will speed up things regardless the increase in memory.

    Why the Collider and not a UnityObject? For the exact same reason you mention, there is not guarantee that everybody use it at such a high level like an Object. But if you're adding a Collider you will be probably end up using it, specially on projects where you need a multi-tagging category System for collision checking.

    One question: why process and or resolve the extra field always?
     
  32. superpig

    superpig

    Drink more water! Unity Technologies

    Joined:
    Jan 16, 2011
    Posts:
    4,655
    I don't think it will. I mean, it might for yours, but I don't think it will for the majority of games.

    Because if the field is there, you might be using it - the only way we could know is to process it. So we would always have to process it, just in case.
     
  33. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    Yes, but process will be mainly done at deserialization time and that will be fast in cases were the reference is null. The only cases were that will add some extra time is when a SO is assigned and serialized with the collider, generally, at design time.

    Now, a different story is in collision checking where things will speed up since you will have a direct acces to assigned SO without further lookups, and only when you expect or need a specific SO. Ditto for design time: no more need to add a specific Monobehaviour to an object with a collider. You just now drop an SO where needed directly on the exposed field in the collider on Inspector.
     
  34. superpig

    superpig

    Drink more water! Unity Technologies

    Joined:
    Jan 16, 2011
    Posts:
    4,655
    Which happens in many more situations than you realize - instantiation, Asset GC...

    No, it'll be slow in all cases. It'll be very slow if the reference is non-null.

    You're making a lot of assumptions about how the engine works and about what kind of requirements we have for performance. I promise you: this is not something we can 'just do.' Nor do I think it is a good idea for us to do it.
     
    Kurt-Dekker and KelsoMRK like this.
  35. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    Actually I am aware of them (and please guys, stop making assumptions of what I may know or not). The thing is that I believe the benefits (not only perf-wise but also in handiness) will surpass the costs it may introduce to the equation.

    I'm indeed worried more about the perf costs due to the way Unity's gameloop currently works than the costs associated to my suggestion. Fortunately, and contrary to collision checks, workarounds exists for them to avoid most of the interop calls in the loop.

    This suggestion is the workaround I believe would be broadly use as an alternative to current Tags. You cannot do it due to the current state of the engine, fine. But do not throw it to the trashcan, because you can take it as a base for a similar/alternative solution for future versions where the current Tagging system gets deem as too limited.
     
  36. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    And you've gotten feedback from several forum members and a Senior Developer from Unity that it doesn't make sense to use something like that and that it would be detrimental to performance, memory consumption, and the clarity/integrity of the API. You can say "well I think it would be faster" all day long but, according to what @superpig is saying, you're wrong. I'm sure you're a really intelligent person but the fact of the matter is - he works on the engine; you don't. He is in a better position to make a judgement on the impact of what you're suggesting.

    It's great that you're trying to think of ways to improve Unity - this suggestion isn't one of those.
     
    Kiwasi and Kurt-Dekker like this.
  37. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    The fact that many people says the same thing does not indicate they are right. Plus, many perf issues are still present in Unity for bad decisions done back then on initial versions. Some fixes are being worked on but experimentally, and for other issues, no solution has been found yet. So, being a Senior Dev, Highlander or Master of the Universe, does not make you right, either.

    One thing that I cannot do is introduce experimental changes to the engine. They can. And since they're starting to think how to tackle down perf issues for good -that have been there for years, maybe they could also think on ways to improve the tagging system to support more complex use-cases.

    But as I said before, if this suggestion does not fit for whatever reason, fine. But don't throw away the idea of improving the tagging system in a way that plays ok with Unity's engine.
     
  38. Kiwasi

    Kiwasi

    Joined:
    Dec 5, 2013
    Posts:
    16,860
    We still on about this?

    Let's look at where it would be bad for performance.
    • Any collider that doesn't need tag information. So basically every single static collider in your average platformer or FPS.
    • Any collider that needs tag information and components attached. So basically every dynamic collider in almost every game ever.
    Where it would be good for performance
    • A collider that needed tag info, but not any components attatched. While this comes up a lot, it's far less common then the other two cases.
    So if I'm being generous, this might help on 20% of colliders. You really want to drop the performance of the entire physics system, just to help out with a few edge cases?
     
    KelsoMRK and Kurt-Dekker like this.
  39. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    No, actually you are. I have finished taking about this in my previous post; not because I think you guys are right but because I see that the suggestion won't make into the engine.

    Now, regarding the cases you've just mentioned, that depends on how you structure colliders on a game object hierarchy which may vary per team per project.
     
  40. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    Ok - now you're sort of getting combative. I'm not sure what - beyond an actual developer on the engine you're suggesting a change to saying it's not a good idea - would convince you.

    I'll say again - it's super awesome that you're thinking of ways to improve the engine. That's a valuable thing to have in any community. The suggestion you've presented simply doesn't make much sense. That's all. You're not dumb; or a bad person. You offered a suggestion and some folks in the community along with someone who works on Unity (which it was great that you got feedback from them FWIW) said "nah - I don't think this is good". If I got defensive the first time I presented an idea that got shot down my career would have ended a really, really long time ago. Heck - if I quit the last time I presented a bad idea I would have been out of job at around lunch time yesterday.
     
  41. PedroGV

    PedroGV

    Joined:
    Nov 1, 2010
    Posts:
    415
    I'm not getting combative, I'm saying that going on discussing about an idea that won't be implemented is not worth my time any longer, but that does not mean I'm wrong. As a matter of fact we could go on theorizing for a long time about the suggestion but we'll never know for sure who's right or wrong without a testing environment. But that (who is right/wrong) is not what matters to me, but as you say to improve the engine. So, to wrap it up, no more post for me on this thread. Kind regards.
     
  42. LightStriker

    LightStriker

    Joined:
    Aug 3, 2013
    Posts:
    2,717
    CompareTag;

    Code (CSharp):
    1. [WrapperlessIcall]
    2. [MethodImpl(MethodImplOptions.InternalCall)]
    3. public extern bool CompareTag(string tag);
    Looks like an unmanaged call to me. Otherwise, it would perform garbage allocation to compare a tag. Let's be clear, the tag (or .name) does not live in the managed side until you call it. At that moment, garbage is generated. CompareTag calls for a comparison on the unmanaged side.

    To be honest, if performance and memory were that important, you would give us an option to dump all name/tag in a build. I'm all for smaller memory footprint and faster loading.
     
    Last edited: Jul 29, 2016
    PedroGV and Kiwasi like this.
  43. Fressbrett

    Fressbrett

    Joined:
    Apr 11, 2018
    Posts:
    97
    Necro post, but I want to stress on the importance of such a feature, even in 2020.

    In my case, one tag doesn't solve the issue, I need multiple tags per object.

    Currently I went with MonoBehaviours containing my tagging enums and GetCompoent calls within my OnCollisionEnter function to execute certain logic depening on what I collided with. Problem is, this gets really slow when thousands of objects are colliding with each other. Stripping the GetComponent away significantly improves my performance, though I lose the option to check against multiple variables instead of just one tag.

    A script or scriptableObject reference which I can somehow get from the Collider parameter inside my OnCollisionEnter/OnTriggerEnter functions would be really neat, OR a better Unity tagging system with multiple possible tags.
     
    Last edited: Jun 17, 2020
  44. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,686
    If I need this sort of multi-tag classification I just make multiple Monobehaviors:

    TagIsPlayer

    TagIsEnemy


    etc.

    This still obviously requires a .GetComponent<> and nullcheck but it's pretty clean:

    Code (csharp):
    1. var thingy = ...;
    2.  
    3. if (thingy.GetComponent<TagIsPlayer>())
    4. {
    5.   // do player-y stuff
    6. }
    7.  
    8. if (thingy.GetComponent<TagIsEnemy>())
    9. {
    10.   // do enemy-y stuff
    11. }
    Best of all you can't mistype a type (compiler error) and you can instantly find all references to them. Perhaps you could even bottle it up in an extension method that takes the thing and the type.
     
  45. Fressbrett

    Fressbrett

    Joined:
    Apr 11, 2018
    Posts:
    97
    I did consider this in my last answer, though it doesn't solve the problem, GetComponent is part of the problem. It is not a good option for lots of collision checks, it simply doesn't perform well enough.
    Tag comparisons perform well enough, but there can only be one tag per object.

    Thus it would be nice to either have the option to tag an object with multiple tags or to store a reference to a script or scriptableObject (which could then include enums) inside the Collider class. All of this is to PREVENT GetComponents from being used inside OnCollision/OnTrigger callbacks.
     
  46. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    Is there any reason you can't just have a MultiTagComponent that exposes a list of enums that you can use GetComponent on during the collision? I would think that combined with putting specific types of objects on specific layers (either to prevent interaction or to check the layer in the collision) would be more than enough.
     
  47. Fressbrett

    Fressbrett

    Joined:
    Apr 11, 2018
    Posts:
    97
    It blunders my performance, GetComponent is not an option. I have thousands of Rigidbodies and colliders that collide with each other. There must be a way to avoid GetComponent calls during collisions if its just comparing types...
     
  48. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    Sounds like layers or implementing something in ECS is the way to go in your case then.
     
    Kiwasi and Fressbrett like this.
  49. darbotron

    darbotron

    Joined:
    Aug 9, 2010
    Posts:
    352
    Just to throw something into the mix here, exposing "user data" is a standard pattern for Physics Middleware.

    Bullet, Box2d, and every other standalone physics engine I've ever used expose a "user data" slot on the collider, the rigidbody, or somewhere related which allows the client code to store data pertinent to their use cases right there in the object.

    I've been dismayed by the lack of a similar feature in Unity since I first started using it in like 2010.

    All it would need to be is a System.Object ref (e.g. userData) the client code could set and then cast back to the type they know they stored in it when they get an OnCollisionxxx/OnTriggerxxx callback.

    It would certainly save a lot of tedious messing about with tags and GetComponentxxxx functions.

    For example; in the game I'm currently working on the function which takes a projectile raycast and works out wtf it has hit and whether that's gameplay significant is long, and currently calls GetComponent, GetComponentInParent, and GetComponentInChildren lots of times (to be fair we're still in pre-production so no-one's gone through and cleaned it up yet).

    With user data on the collider / rigidbody we could pre-cache most if not all the refs in a data class and store a ref to it in the userData property.

    I guess it's possible to approximate the same behaviour as having custom data by attaching a monobehaviour (e.g. GameplaySignificantComponentCollisionCache) to every collider / rigidbody which finds all the various components we might want to check for in Awake() / Start() and just do "GetComponent< GameplaySignificantComponentCollisionCache >()" in the collision callback to grab it but I'd still prefer to have a userData property on collider / rigidbody...
     
    Marrlie and Vincent454 like this.
  50. Vincent454

    Vincent454

    Joined:
    Oct 26, 2014
    Posts:
    167
    I would love to have this too. You could easily have an enum that way that told you which material type it uses and spawn an according effect on hit without having to get any component. Or as said before, multiple tags in general would just be amazing.
     
    darbotron likes this.