Discussion Script preformance improvement

So I wrote this script which is attached to my Interactor object in scene which has a polygon2d trigger collider and is used to detect all of the objects in the scene that my player can interact with. I want that each interactable object that my player sees at a time has a interact text written over them using coroutines. I feel like detecting many objects using raycasting2d.boxcast or circlecast in update is less preformant than using OnTriggerEnter2D and OnTriggerExit2D which are only called when a collider enters/exit trigger.(I may be wrong about this so if I am, please point it out in the comments)

To make sure coroutines stop when player stops looking at the objects or picks them up, I used a dictionary to store references to coroutines that I am using along with the interactable object transform as key and coroutine as value.

There are two things that I feel like I should change:

1) I want that when i press E key I interact with the nearest game object which means that i access the first game object in the dictionary and later on remove it where I used linq's method First() which means that dictionary may not be the most suitable collection here(maybe queue is better but I think code might become a lot longer)

2) I am not storing enough references and access them from time to time which means that maybe I should store somewhere but I am not sure where.
Here is my script:

Code (CSharp):

1. using System.Collections.Generic;
2. using System.Linq;
3. using UnityEngine;
4.  
5. interface IInteractable
6. {
7.     public void Interact(Transform interactor = null);
8. }
9.  
10. public class InteractCheck : MonoBehaviour
11. {
12.     private Dictionary<Transform, Coroutine> interactObjects = new Dictionary<Transform, Coroutine>();
13.  
14.     private void OnTriggerEnter2D(Collider2D other)
15.     {
16.         var entered = other.transform;
17.         if (entered.GetComponent<IInteractable>() != null)
18.         {
19.             var routine = StartCoroutine(entered.transform.GetChild(0).GetComponent<TypeWriter>().WriteText());
20.             interactObjects.Add(entered, routine);
21.         }
22.     }
23.  
24.     private void OnTriggerExit2D(Collider2D other)
25.     {
26.         if(interactObjects.ContainsKey(other.transform))
27.         {
28.             StopCoroutine(interactObjects[other.transform]);
29.             other.transform.GetChild(0).GetComponent<TypeWriter>().EmptyText();
30.             interactObjects.Remove(other.transform);
31.         }
32.     }
33.  
34.     private void Update()
35.     {
36.         if (Input.GetKeyDown(KeyCode.E) && interactObjects.Count > 0)
37.         {
38.             interactObjects.First().Key.GetComponent<IInteractable>().Interact(transform.parent);
39.             StopCoroutine(interactObjects.First().Value);
40.  
41.             interactObjects.First().Key.GetChild(0).GetComponent<TypeWriter>().EmptyText();
42.             interactObjects.Remove(interactObjects.First().Key);
43.         }
44.         else if (Input.GetKeyDown(KeyCode.F))
45.         {
46.             WeaponDrop.Instance.Drop();
47.         }
48.     }
49. }

 

Emotions and assumptions are the worst options you have to assess performance. The best is the Profiler (Windows => Analysis => Profiler).

I really don't see a performance issue here unless we're talking thousands of entries.

First() isn't going to give you the nearest transform. I'm not even sure what First() returns when used on a Dictionary, but I would expect that First() is not guaranteed to remain the same when the Dictionary changes (added or removed a key).

If you want the nearest you need to go over each transform and compare the distance to that Transform.position with the one for the player (or mouse) using this formula "(keyTransform.position - player.transform.position).magnitude" which gives you the distance to that transform.

 

You can first sort the transforms by distance (or distance squared for better performance) using OrderBy and then you can use First.

Code (CSharp):

1. var nearestInteractObject = interactObjects.Keys.OrderBy(interactable => (position - interactable.position).sqrMagnitude).First();

Also, instead of accessing the TypeWriter component like you have here using GetChild and GetComponent and calling very specific methods on it, it would be more robust to just call more generic methods like OnInteractionRadiusEntered and OnInteractionRadiusExited on IInteractable, and let the implementer of that interface worry about the implementation details with the TypeWriter. At the moment the system is very fragile, because altering the hierarchy structure of any interactable could cause things to break here without any warning.

A third minor note about naming: I think

Clear

or

ClearText

would be a better name than

EmptyText

for TypeWriter. EmptyText could be mistaken to mean something like string.Empty, while Clear is used in .NET a lot for disposing contents of various collections, so I think it would be a bit more intuitive

 

By preformance improvement I mean that I might be resizing my dictionary a lot which may be better with a queue, yet again with dictionary I can easily access value by its key which isn't possible with queue.
I think my approach will always work since OnTriggerEnter2D allows me to know who entered first in player's fov.

 

Read my reply above to understand why I didnt do this.

 

But maybe you are right, both of yours approach might be better. I think I will change my code to include things you have mentioned.

 

Resizing a Dictionary doesn't create garbage and isn't a heavy operation, so I wouldn't worry about its performance - just pick the collection that makes most sense in terms of readability and robustness. Especially since this code isn't anything that will be executed every frame.

If you move the logic for WriteText and EmptyText behind the IInteractable interface like I suggested, then you could use a simple List or a HashSet here.

Queue doesn't necessarily sound like a great choice, because removing specific items from it is difficult. Unless you just call Dequeue in OnTriggerExit2D and assume that objects always leave in the same order as they entered. But to me this sounds a bit error prone; what if you add interactables of different sizes or speeds in the future, for example?