Search Unity

  1. Unity Asset Manager is now available in public beta. Try it out now and join the conversation here in the forums.
    Dismiss Notice

ContactPoint2D.enabled is confusing

Discussion in '2D Experimental Preview' started by Jay-Pavlina, Jun 23, 2016.

  1. Jay-Pavlina

    Jay-Pavlina

    Joined:
    Feb 19, 2012
    Posts:
    195
    Just by the name, I thought ContactPoint2D.enabled meant that the contact somehow came from a disabled collider or game object, so when using a contact filter, I used ignoreDisabledContacts. It turned out the contacts I needed were disabled. Anyway, I looked at the documentation, and it says:
    So, shouldn't the property be called responseEnabled? I think that simple change would help to avoid a lot of confusion.
     
    Jay-Pavlina, Jun 23, 2016
    #1
    Xepherys likes this.

  2. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,459
    A disabled collider doesn't exist in the simulation so wouldn't produce any contacts.

    The ContactPoint2D.enabled is just a copy of the Collision2D.enabled (which has been in the general release for some time) that can contain multiple contact points. The only thing that disables contacts currently are effectors, specifically the PlatformEffector2D for one-way mode.

    Contacts are only used for collision response so I don't see the need to prefix everything with 'response'. We wouldn't use 'responseCollisionNormal', it's just implicit. We'd also need to refer to this as 'responseDisabled' everywhere such as 'ignoreResponseDisabledContacts' which is just too verbose. You can't change one property without referring to it explicitly as that too confuses people.

    I think your confusion was soon clarified by the docs. If the contact isn't enabled, the physics system isn't using it.
     
    Last edited: Jun 24, 2016
    MelvMay, Jun 24, 2016
    #2

  3. Jay-Pavlina

    Jay-Pavlina

    Joined:
    Feb 19, 2012
    Posts:
    195
    My confusion turned out to be caused by what appears to be a bug with ContactFilter2D. It only works properly if you call NoFilter() on it before using it. All I was doing was masking to a specific layer. If I didn't call NoFilter() on it, it would return nothing. If I called NoFilter() and then set the layer mask, it worked properly. I thought I was getting nothing because I set ignoreDisabled to true initially, but it turns out that wasn't the case.

    And more confusion... Since I was using kinematic contacts, there was no collision response, so I thought it meant that any contact caused by kinematic contacts would be disabled. So, lots of confusion mostly just caused by ContactFilter2D not working properly unless I called NoFilter() on it first.

    Long story short...

    This works:
    Code (CSharp):
    1. groundContactFilter.NoFilter();
    2. groundContactFilter.layerMask = Layers.ground.mask;
    This doesn't work:
    Code (CSharp):
    1. groundContactFilter = new ContactFilter2D();
    2. groundContactFilter.layerMask = Layers.ground.mask;
     
    Jay-Pavlina, Jun 24, 2016
    #3

  4. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,459
    Thanks for the detailed report! I'll look at that issue now.
     
    MelvMay, Jun 27, 2016
    #4

  5. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,459
    Forgot to get back to you on this. So yeah, ContactFilter2D is a struct so we can't create a parameterless constructor to initialize the values to something useful so the 'NoFilter()' does exactly that.

    This means that a new ContactFilter2D will have:

    ignoreDisabled = false;
    layerMask = 0;
    minDepth = 0.0;
    maxDepth = 0.0;
    minNormalAngle = 0.0f;
    maxNormalAngle = 0.0f;

    ... but 'NoFilter()' gives you ...

    ignoreDisabled = false;
    layerMask = Physics2D.AllLayers;
    minDepth = -Mathf.Infinity;
    maxDepth = Mathf.Infinity;
    minNormalAngle = -Mathf.Infinity;
    maxNormalAngle = Mathf.Infinity;

    Not sure there's a good way around this other than better doco and/or adding extra members that turn on/off each item i.e. bool useLayerMask, bool useDepth, bool useNormalAngle. At least that way the default would be to not filter anything.
     
    MelvMay, Aug 11, 2016
    #5

  6. Jay-Pavlina

    Jay-Pavlina

    Joined:
    Feb 19, 2012
    Posts:
    195
    I don't think you should add more members. I think good documentation would help, and you could add a static method called Create and just tell people to use that instead of the default constructor.

    Edit: Alternatively, would nullable types work?
     
    Jay-Pavlina, Aug 11, 2016
    #6

  7. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,459
    Various feedback from a bunch of people, including internally suggest that having extra members isn't an issue (we do this in many other places i.e. a bool to activate and some other values for the config) but we should have setters for them i.e. SetDepth (min, max) or SetLayerMask (mask) which also activates that filter. We could also potentially have an opposing ClearDepth () etc.

    I follow your concern about having extra members but it's probably the easiest way to have default no-filter state and also allows filters to be turned off without changing the config values themselves.
     
    MelvMay, Aug 15, 2016
    #7

  8. Jay-Pavlina

    Jay-Pavlina

    Joined:
    Feb 19, 2012
    Posts:
    195
    I've generally noticed that Unity's APIs favor bulk, but if it eliminates confusion, it's the right choice. At least now it will work the way it's expected to.
     
    Jay-Pavlina, Aug 15, 2016
    #8