Search Unity

Question Best interaction practices

Discussion in 'Scripting' started by bvidosits, Jul 9, 2020.

  1. bvidosits

    bvidosits

    Joined:
    Sep 22, 2019
    Posts:
    5
    So currently I have my (2d) interaction set up like this:

    My setup:
    Player has a trigger circle collider.
    OnTriggerEnter places other.gameObject in a list if other.CompareTag("interactable")
    OnTriggerExit removes it.
    Update calculates the closest gameObject and saves it as a separate variable.
    Update also checks if I've pressed F.
    If I indeed did and I have a saved closest gameObject:

    closestInteractable.GetComponent<Interactable>().OnInteraction(gameObject);


    My questions/problems:
    Does this method look fine up until the actual interaction or is there a better practice for this?

    How could I replace the OnInteraction part?
    I have several problems with it:

    I do not know how to enforce all interactable objects to have the Interactable script/class. There has to be a way to tell the game that it has to define that method, like an interface.
    This way I would have to define to all interactions in a single script, so far I've only set up the part to handle money interactions and it's already getting clunky:

    Code (CSharp):
    1. [Header("Options")]
    2. [SerializeField] private int _moneyAmount = 0;
    3. [SerializeField] private bool _destroyOnInteract = false;
    4. [SerializeField] private bool _destroyOnEmpty = false;
    5.  
    6. [Header("References")]
    7. [SerializeField] private MoneyOptions moneyOptions = null;
    And these are just the variables...
    If I would make an apple now that you could pick up, half of the code would already be just clutter to that item, because an apple has no connection to money whatsoever.

    My original idea was to make an interactable prefab, then set the sprite and script values on the instances, but the code would look like hell with more options than Unity has itself. Then I would have more problems when I want to expand and have slightly different variations of things.

    TL;DR:
    Is the way I'm detecting and handling close interactable objects good? Is there a better way?
    How do I enforce the definition of OnInteraction on certain objects?
    How do I make a universal factory of interactable objects?

    Thanks in advance!

    EDIT:
    After further research (Because of course I find a possible solution after I post my question after 2 days of researching and testing...), it seems that a way to go about this would be:
    Code (CSharp):
    1. public abstract class Interactable : MonoBehavior {
    2.     // common options/variables
    3.  
    4.     public abstract void OnInteract();
    5. }
    6.  
    7. public class InteractableMoney : Interactable {
    8.     // money specific things
    9. }
    Then I would make a prefab of a money that's set up.

    Is this the correct way to go or is there a better way?
     
    Last edited: Jul 9, 2020
    rubcc95 likes this.
  2. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,338
    Instead of checking for the "interactable" tag, just check if there's an interactable object there:

    Code (csharp):
    1.  
    2.     private void OnTriggerEnter2D(Collider2D other) {
    3.         if (other.TryGetComponent<Interactable>(out var interactable)) {
    4.             interactables.Add(interactable);
    5.         }
    6.     }
    7. // 2018.4 version, where TryGetComponent doesn't exist:
    8.     private void OnTriggerEnter2D(Collider2D other) {
    9.         var interactable = other.GetComponent<Interactable>();
    10.         if (interactable != null) {
    11.             interactables.Add(interactable);
    12.         }
    13.     }
    14.  
    You can also use an interface (IInteractable), as GetComponent/TryGetComponent works with interfaces.


    The big problem with your approach is that if you deactivate the interactor object, and then move it, you won't get OnTriggerExit calls. Probably - that's how it works in 3D, not quite sure about 2D. So in those cases, you'll have things in your list that's actually outside the collider.

    You can solve that either by manually purging the list at times when that happens, or by dropping the list and instead using an overlap circle whenever you press interact.
     
    bvidosits likes this.
  3. bvidosits

    bvidosits

    Joined:
    Sep 22, 2019
    Posts:
    5
    This is genius and much simpler, thank you!

    I think I cannot drop the list since if 2 items are in range to be interacted with, I still have to keep track of which one is closer, since I can keep them in range while moving closer from item1 to item2.

    However so far I've used
    Object.Destroy(gameObject)
    on the interactable item, which is either smart enough to kill my reference or triggers
    OnTriggerExit2D
    .
    But nice catch, I'll keep this mind.
    I could also just remove the interactable from the list on use via checking if it will be disabled, something like
    interactable.disableOnInteract


    Thanks again!
     
  4. rubcc95

    rubcc95

    Joined:
    Dec 27, 2019
    Posts:
    222
    I didn't know TryGetComponent would be faster than CompareTag :eek:
    About how to remove it from the List if it's destroyed before OnTriggerExit:
    If your List of Interactable is static, you can remove them from the list with
    Code (CSharp):
    1. private void OnDestroy()
    2. {
    3.     Player.interactables.Remove(this);
    4. }
    If you've more than one instance you could do it with a Func delegate like this
    Code (CSharp):
    1.     private void OnTriggerEnter2D(Collider2D other) {
    2.         if (other.TryGetComponent<Interactable>(out var interactable)) {
    3.             interactables.Add(interactable);
    4.             interactable.func += interactables.Remove;
    5.         }
    6.     }
    7.  
    8.     private void OnTriggerExit2D(Collider2D other) {
    9.         other.GetComponent<Interactable>(out var interactable);
    10.         interactables.Remove(interactable);
    11.         interactable.func -= interactables.Remove;      
    12.     }
    Code (CSharp):
    1. public abstract class Interactable : MonoBehavior {
    2.  
    3.     public Func<Interactable, bool> func;
    4.     public abstract void OnInteract();
    5.  
    6.     protected void OnDestroy()
    7.     {
    8.         func();
    9.     }
    10. }
     
    Last edited: Jul 9, 2020
  5. bvidosits

    bvidosits

    Joined:
    Sep 22, 2019
    Posts:
    5
    I don't know if it's faster, it is a lot simpler to implement though.

    Thanks, but as I said:

    Unity seems to do this automatically :)
     
  6. rubcc95

    rubcc95

    Joined:
    Dec 27, 2019
    Posts:
    222
    Well no sure about what reference to OnTriggerEnter2D you talk about. I'm talking about the list of interactables you have. When you loop through it to see whos the closest one... if some Item of the list has been destroyed without remove it (Transform, Interactable, GameObject or whatever the list is) you will get a missing reference exception if you want to search some value inside it. For example:
    Code (CSharp):
    1. public List<Transform> items = new List<Transform>();
    2. foreach(Transform item in items)
    3. {
    4.     Debug.Log(item.position);
    5. }
    6.  
    I could have misunderstood you, but I'm 100% sure unity doesn't remove all references some Object has at Lists or any Collection type automatically when you call OnDestroy(); Unity knows the object has been destroyed, but if you try to access it, it breaks;
     
  7. bvidosits

    bvidosits

    Joined:
    Sep 22, 2019
    Posts:
    5
    Hmn, interesting.

    I do exactly that, however it doesn't seem to break.

    PlayerController.cs (just the parts that matter)
    Code (CSharp):
    1.  
    2.     private List<GameObject> closeInteractables = new List<GameObject>();
    3.     private GameObject closestInteractable;
    4.  
    5.     private void OnTriggerEnter2D(Collider2D other) {
    6.         if (other.CompareTag("Interactable")) {
    7.             closeInteractables.Add(other.gameObject);
    8.         }
    9.     }
    10.  
    11.     private void OnTriggerExit2D(Collider2D other) {
    12.         if (closeInteractables.Contains(other.gameObject)) {
    13.             closeInteractables.Remove(other.gameObject);
    14.         }
    15.     }
    16.  
    17.     private void ProcessInput() {
    18.         ProcessMovement();
    19.         ProcessInteraction();
    20.     }
    21.  
    22.     private void ProcessInteraction() {
    23.         if (
    24.             Keyboard.current.fKey.wasPressedThisFrame &&
    25.             closestInteractable &&
    26.             closestInteractable.TryGetComponent<Interactable>(out Interactable interactable)
    27.         ) {
    28.             interactable.OnInteract(gameObject);
    29.         }
    30.     }
    31.  
    32.     private void SetClosestInteractable() {
    33.         GameObject closest = null;
    34.         float closestDistance = 0f;
    35.  
    36.         foreach (GameObject item in closeInteractables) {
    37.             float newDistance = Vector2.Distance(gameObject.transform.position, item.transform.position);
    38.             if (closest == null || newDistance < closestDistance) {
    39.                 closest = item;
    40.                 closestDistance = newDistance;
    41.             }
    42.         }
    43.  
    44.         closestInteractable = closest;
    45.  
    46.         interactionIndicator.SetActive(closestInteractable != null);
    47.         if (closestInteractable != null) {
    48.             interactionIndicator.transform.position = new Vector2(
    49.                 closestInteractable.transform.position.x,
    50.                 closestInteractable.transform.position.y +
    51.                     closestInteractable.GetComponent<SpriteRenderer>().bounds.extents.y +
    52.                     0.25f
    53.             );
    54.         }
    55.     }
    56.  
    InteractableMoney.OnInteract:
    Code (CSharp):
    1.     public override void OnInteract(GameObject interactee) {
    2.         if (_moneyAmount > 0) {
    3.             GiveCash();
    4.             if (_destroyOnInteract)
    5.                 GameObject.Destroy(gameObject);
    6.         }
    7.     }
    TL;DR: Only
    OnTriggerExit2D
    removes from the list and nothing redefines it.

    I went on a few test runs with the list set to public and even in the editor, everything seems to work.
     
  8. rubcc95

    rubcc95

    Joined:
    Dec 27, 2019
    Posts:
    222
    If you destroy at inspector one object at closeInteractables list you've not a Missing Reference Exception? So weird.

    Apart from this just 2 tips:
    Code (CSharp):
    1. //This calcs a root square to give you the distance
    2. Vector2.Distance(gameObject.transform.position, item.transform.position);
    3.  
    4. //This doesn't calc the root square and gives you distance^2
    5. (gameObject.transform.position - item.transform.position).sqrMagnitude;
    Since you doesn't need the distance value (you only need to know if it's bigger) you save the machine to calc a bunch of square roots (they're expensive).


    And if:
    Code (CSharp):
    1. float closestDistance = Mathf.Infinity;
    You don't need to ensure about if closest == null each loop
    Code (CSharp):
    1. if (/** closest == null || */ newDistance < closestDistance) {
     
  9. bvidosits

    bvidosits

    Joined:
    Sep 22, 2019
    Posts:
    5
    No, I mean I displayed the list on the editor and the object is removed from the list as soon as I call
    Object.Destroy()
    on it in the script.
    Removing the object through the editor results in a MissingReferenceException.
    However I don't plan on anyone playing the game through Unity's editor :)

    Huh, good to know!

    Smart :)

    Thanks!