Search Unity

How would you optimize my interaction algorithm?

Discussion in 'Scripting' started by aramosgz, Nov 20, 2019.

  1. aramosgz

    aramosgz

    Joined:
    Jan 28, 2016
    Posts:
    19
    Hello!

    I'm developing a First Person Perspective game in which one of the main mechanics is about interacting with objects found in the environment.

    The end result I'm looking for is: I move the camera around and when I'm looking at an interactable object at a given distance, a text will display on top of that object ("Pick up herb"). When this text is being displayed, you click a key and an action happens.

    Right now I have this system working but I'm curious as to what other approaches you guys would take for this. I feel like my approach is kind of ugly and there's room for optimizing.

    My approach looks like this:

    - My character controller / camera object has a Sphere Trigger on it, with a radius of 5 units or so. Interactable objects have also a sphere trigger slightly bigger than the objects themselves.
    - Interactable objects have a script (abstract class Interactable.cs) which has a OnTriggerEnter, checking if the colliding trigger is the Character's. If so, OnTriggerEnter starts the Interacting coroutine (found in the Interaction.cs script, in the Character Controller game object).
    - Interactable objects have a OnTriggerExit which, when the colliding trigger is the Character's, calls a Interaction.cs script with this code (to avoid stopping the interaction if at least 1 object is in interacting radius):

    Code (CSharp):
    1.         Collider[] intersectingColliders = Physics.OverlapSphere(transform.position, GetComponent<SphereCollider>().radius, m_interactableLayer, QueryTriggerInteraction.Collide);
    2.         if (intersectingColliders.Length == 0)
    3.         {
    4.             m_interactionObject = null;
    5.             m_interactionUI.SetActive(false);
    6.             m_coroutineRunning = false;
    7.             StopCoroutine("Interacting");
    8.         }
    - Interacting coroutine looks like this:

    Code (CSharp):
    1.        
    2.         m_coroutineRunning = true;
    3.         while (true)
    4.         {
    5.             m_ray = Camera.main.ViewportPointToRay(new Vector3(0.5f, 0.5f, 0)); // Middle point in the game screen
    6.             if (Physics.Raycast(m_ray, out m_rayHit, 5, m_interactableLayer, QueryTriggerInteraction.Collide))
    7.             {
    8.                 m_interactionObject = m_rayHit.collider.gameObject;
    9.  
    10.                 m_interactionUI.GetComponent<UITextPosition>().m_object = m_interactionObject;
    11.                 m_interactionUI.SetActive(true);
    12.                 Interactable interactable = m_interactionObject.GetComponent<Interactable>();
    13.                 m_interactionUI.GetComponentInChildren<TextMeshProUGUI>().text = interactable.m_textUI;
    14.             }
    15.             else
    16.             {
    17.                 m_interactionUI.SetActive(false);
    18.             }
    19.             yield return new WaitForSeconds(0.05f);
    20.         }
    - m_coroutineRunning is used to avoid starting this coroutine multiple times by multiple interactable objects near each other.
    - m_ray and m_rayHit are a Ray and a RaycastHit which I cache for better performance.
    - m_interactionObject is the interactable object. I keep a reference to it to know which object's Interact method to call after receiving input.
    - m_interactionUI is a Panel whose child is a TextMeshProUGUI where custom text (depending on the object) will be written. This panel will be active if a raycast hits an interactable object's sphere trigger, inactive otherwise.
    - I found 0.05f to be a good value for WaitForSeconds to give good (smooth) results, after trying 0.1f - 0.2f and not being satisfied with the results.

    Some extra code, for instance the calculations for figuring out the panel's location on the camera, are left out as they are not too important.

    I'm really looking forward to seeing what you guys would to to tackle this problem. My approach works just fine for me and I don't think any optimization is necessary but hopefully I can learn some nice tricks from you so I can apply them next time!

    Regards.
     
  2. mgear

    mgear

    Joined:
    Aug 3, 2010
    Posts:
    9,438
    if you just want mouseover effects,
    i'd use single raycast script from camera,
    with layermask (so it hits only interactive objects) and 5 meter range,
    then if raycast hits, call or get mouseover text from that object.
     
    aramosgz likes this.
  3. Madgvox

    Madgvox

    Joined:
    Apr 13, 2014
    Posts:
    1,317
    It seems you've implemented collision culling for your raycast! Unfortunately, that's not really necessary since Unity does that anyway.

    One raycast per frame will not break the bank by any means. Beware of micro-optimization -- it will make your code more convoluted for no benefit. The time to optimize your code is when your code is taking too long to execute, not before.

    Caching Ray and RaycastHit ironically makes your code less efficient, since as class members they will get allocated on the heap instead of the stack and are technically slower to read/write (minutely slower, maybe, depending on the cpu cache. Either way, we are talking optimizations that are so small as to be undetectable. Don't cache Ray or RaycastHit. There's no point).

    Your GetComponent and GetComponentInChildren call I would cache, though. It's a good habit to get into.
     
    aramosgz likes this.
  4. aramosgz

    aramosgz

    Joined:
    Jan 28, 2016
    Posts:
    19
    So both you guys suggest just casting rays on the Update method of the Camera script? That's what I thought at first, decided to spice my code up in the hopes of doing something a little more optimized, if a bit unnecessary.

    @Madgvox Alright, I'll stop caching Rays and RaycastHits. I also thought of caching GetComponent calls but didn't feel great to add 3 more members to the class. I'll definitely do it tho!

    Thanks for your input.
     
  5. Madgvox

    Madgvox

    Joined:
    Apr 13, 2014
    Posts:
    1,317
    There's no need to cache the
    GetComponent<Interactable>()
    , since it has the possibility of changing over time.

    Don't think of caching in terms of what members a class has. Think of caching in terms of how much effort it takes to retrieve data relative to how often you need to retrieve that data. Your
    GetComponent<UITextPosition>()
    and
    GetComponentInChildren<TextMeshProUGUI>()
    calls are relatively more expensive than simply retrieving a variable reference, and you're doing it potentially every frame. Couple that to the fact that the result of those two calls never change, and there's no real reason not to cache it.

    The
    GetComponent<Interactable>()
    , on the other hand could be getting any number of different instances of Interactable, and therefore "caching" it on the class would be moot.

    Caching is the act of saving yourself computation time. You don't save yourself computation time simply by making something a class member, you save yourself computation time by not doing the computation. You can't avoid doing the computation to retrieve the Interactable, so there's no real way to cache it. Same for your Ray and RaycastHit.
     
    Last edited: Nov 20, 2019
    aramosgz likes this.
  6. Stardog

    Stardog

    Joined:
    Jun 28, 2010
    Posts:
    1,913
    I would wrap the raycast hit code inside a check like this, so it only does it if it's a different object.
    Code (csharp):
    1. if (m_interactionObject != m_rayHit.collider.gameObject)
    2. {
    3.     m_interactionObject = m_rayHit.collider.gameObject;
    4.  
    5.     m_interactionUI.GetComponent<UITextPosition>().m_object = m_interactionObject;
    6.     ...
    7. }
    You can use one line instead of nesting:
    Code (csharp):
    1. if (m_interactionObject == m_rayHit.collider.gameObject) return;
    Also, I would use properties with OnChanged events and therefore rewrite the whole thing, of course...
     
  7. aramosgz

    aramosgz

    Joined:
    Jan 28, 2016
    Posts:
    19
    @Madgvox Right, I see I didn't fully grasp the concept of caching. Thanks a lot for making me understand it better.

    @Stardog There was a point where I actually did that same check, but I changed the code several times and at some point I took it out.
    Regarding OnChanged events I'm somewhat familiar with .NET OnPropertyChanged events, how would you go about doing that in Unity though?
    A solution I had in mind is having m_interactionObject as a property. On its setter, as well as assigning the new value, calling a function to update other interaction related properties. Won't implement it as I don't think there's much gain in it (at least for now) but would like to know of better ways to achieve property change events.
     
    Last edited: Nov 21, 2019