Search Unity

I need a quick Code Review in my Script.

Discussion in 'Scripting' started by CortiWins, Dec 17, 2019.

  1. CortiWins

    CortiWins

    Joined:
    Sep 24, 2018
    Posts:
    150
    Hey folks!

    i did something that works and i'm not sure if it did solve it in a good way. The class is part of inheritence, but that is not part of the question here, so just ignore that.

    I want to notice when a trigger-area is entered and when it is left. Entering is easily detected via OnTriggerEnter, but if the object disappears within the are, the OnTriggerExit is not called. So i'm using OnTriggerStay to detect if there is still something in there. For the purpose of the script, it is not required to track exacly which object entered, only when the first one enters and the last one leaves.

    The solution with OnTriggerStay seems like polling to me. And compared to OnTriggerExit, which is more like an event, this seems somehow off to me.
    • Do i worry too much?
    • Do you got a hint for me?
    • How did you solve something like that?
    I'm at the point where i feel i'm getting familiar with Unity and thats the point where devs can fall into the trap of feeling smarter than they are, so give me your reality check! No tears, i promise ;-)

    Code (CSharp):
    1. /// <summary>
    2. /// Component that detects triggers within its collider and uses them to initiate ingame user logic.
    3. /// Inherits: ColliderTrigger > TriggerLink > TriggerBase > MonoBehaviour
    4. /// </summary>
    5. public class ColliderTrigger : TriggerLink
    6. {
    7.     /// <summary>
    8.     /// Contains the layers which are accepted for the purpose of this component.
    9.     /// </summary>
    10.     [Header("Settings: " + nameof(ColliderTrigger))]
    11.     [SerializeField]
    12.     private LayerMask detectionMask;
    13.  
    14.     /// <summary>
    15.     /// Value that indicates if isTriggered is is reset when the last collider left the triggerspace.
    16.     /// </summary>
    17.     [SerializeField]
    18.     private bool isResettingOnLeave;
    19.  
    20.     [Header("Debug: " + nameof(ColliderTrigger))]
    21.     [SerializeField]
    22.     private int actorsInsideCount;
    23.  
    24.     /// <summary>
    25.     /// Number of colliders within triggerspace as counted by <see cref="OnTriggerStay(Collider)"/>.
    26.     /// Reset and used in <see cref="FixedUpdate"/>.
    27.     /// </summary>
    28.     private int countInsideFixedUpdate = 0;
    29.  
    30.     /// <summary>
    31.     /// Works with the countInsideFixedUpdate value calculated in the previous cycle
    32.     /// as FixedUpdate runs before the OnTrigger* Methods.
    33.     /// </summary>
    34.     private void FixedUpdate()
    35.     {
    36.         if (this.actorsInsideCount > 0 && this.countInsideFixedUpdate == 0 && this.isResettingOnLeave)
    37.         {
    38.             // Last actor left the triggerspace.
    39.             this.Reset();
    40.         }
    41.  
    42.         this.actorsInsideCount = this.countInsideFixedUpdate;
    43.         this.countInsideFixedUpdate = 0;
    44.     }
    45.  
    46.     /// <summary>
    47.     /// OnTriggerEnter is called when the Collider enters the trigger.
    48.     /// Called in the physics cycle, after FixedUpdate, before OnTriggerStay.
    49.     /// </summary>
    50.     /// <param name="collider">The other Collider involved in this collision.</param>
    51.     private void OnTriggerEnter(Collider collider)
    52.     {
    53.         if (LayerHelper.CheckLayerMask(this.detectionMask.value, collider.gameObject.layer)
    54.             && this.actorsInsideCount == 0)
    55.         {
    56.             // New actor has entered the triggerspace.
    57.             this.Trigger();
    58.         }
    59.     }
    60.  
    61.     /// <summary>
    62.     /// OnTriggerStay is called once per physics update for every Collider that is touching the trigger.
    63.     /// Called in the physics cycle, after FixedUpdate & OnTriggerEnter.
    64.     /// </summary>
    65.     /// <param name="other">The other Collider involved in this collision.</param>
    66.     private void OnTriggerStay(Collider other)
    67.     {
    68.         if (LayerHelper.CheckLayerMask(this.detectionMask.value, other.gameObject.layer))
    69.         {
    70.             this.countInsideFixedUpdate++;
    71.         }
    72.     }
    73. }
     
  2. Joe-Censored

    Joe-Censored

    Joined:
    Mar 26, 2013
    Posts:
    11,847
    Seems like a complicated setup. I'd replace it with a physics overlap check and see if the performance is good. Using OverlapSphere or OverlapBox you'll get the information you're looking for directly instead of in this roundabout way, and it might even perform better than all these OnTriggerX calls.

    https://docs.unity3d.com/ScriptReference/Physics.OverlapSphere.html
    https://docs.unity3d.com/ScriptReference/Physics.OverlapBox.html

    You probably don't even need this information every single frame, so can improve performance if this is a problem just with a simple timer (checking every 0.2 seconds or so, probably won't notice the difference compared to every frame).
     
    CortiWins likes this.
  3. CortiWins

    CortiWins

    Joined:
    Sep 24, 2018
    Posts:
    150
    Hey @Joe-Censored , thanks for taking the time to help me.

    I thought about the Physics.Overlap variant as well but isn't that what Unity itself is doing already? The TriggerMethods are called as a result of checking coliders against each other. This is done by Unity anyway as part of the physics calculation. So it seems like
    • OnTriggerMethods -> use Unity's collidercheck
    • Physics.Overlap -> do your own
    One thing that it needs to detect are projectiles that enter/hit the space and then get deleted. Unity itself does a little magic with its collision detection and interpolate settings. Using the TriggerMethods i benefit from that. With Physics.Overlap, especially with a timer, i would have to worry if i missed something.
     
  4. kdgalla

    kdgalla

    Joined:
    Mar 15, 2013
    Posts:
    4,639
    If you don't want to "poll" with OnTriggerStay, why not just have the little objects inform the ColliderTrigger object directly when they disappear?

    I wouldn't bother unless you're seeing some performance problem, though.
     
    CortiWins and Joe-Censored like this.