Resolved Variable, collision.GetComponent or instance?

Hello,
I was wondering if there is any difference between the following 3 codes.

I'm calling a Buff() function from HealthSystem script on a BuffFlag object that has a BuffFlag script with following codes being called in a

private void OnTriggerEnter2D(Collider2D collision)

function.

Is there any main reason why I should use one instead of the other two?
Is one of them a "better" practice?

1.

Code (csharp):

1.  
2. if (collision.tag == "Player")
3.         {
4.             collision.GetComponent<HealthSystem>().Buff();
5.         }

2.

Code (csharp):

1. var player = collision.GetComponent<HealthSystem>();
2.        if (player != null)
3.        {
4.          player.Buff();
5.        }

or make the HealthSystem static with

Code (csharp):

1.  
2. public static HealthSystem instance;
3. private void Awake()
4.       {
5.         instance = this;
6.       }

and do 3.

Code (csharp):

1. if (collision.tag == "Player")
2.         {
3.             HealthSystem.instance.Buff();
4.         }

Thank you.

 

The first option will run the logic if the object that entered the collider is specifically tagged as "Player". Nothing will happen if any other objects enter.

In the second option, the logic will run when any object enters the collider, not just the player. If any object has a

HealthSystem

component attached and enters the collider, it's

Buff

method will be called.

I would avoid the third option entirely. Turning

HealthSystem

into a singleton doesn't make much sense and would cause complications.
What if you later decide to re-use this

HealthSystem

component for enemies? Can't do that if it's a singleton.

 

Hi, and thank you for the quick reply.

And yes, that makes complete sense given the info I have provided.
Perhaps I should have named my classes better and make

HealthSystem

into

PlayerHealth

since I know that the script will only be attached to a single object in the scene with the "Player" tag.

Given that, is there any real reason why I should use first method instead of the second one?
Is there any advantage to comparing tags over creating a variable and checking if it isn't null?

And if I know that "Player" is the only object that will use the

HealthSystem

script, would that justify making it a singleton?

I can't think of a situation where I would call anything from the

HealthSystem

script without the player being in collision with the object calling the function but I'm just trying to see all angles here.

Thank you.

 

In this case, there's no real difference. If the player is the only object that will have this component, you can skip the tag-checking and just check for the component instead. You could even shorten the check by using

TryGetComponent

, which will return true/false automatically if the component exists or not:

Code (CSharp):

1. if(collision.gameObject.TryGetComponent(out HealthSystem health)) {
2.    health.Buff();
3. }

I still wouldn't say so. There should be a good advantage to creating a singleton over a standard object, and just changing the way a method is called doesn't change anything about how the object functions; it still depends on the player entering the collider regardless.

Singletons are inherently not expandable. Again, it may work for your requirements now, but you may later change your requirements down the line.

HealthSystem

is only meant to be used by the player, right? Well what if you decide later to add a local-co-op feature to your game, where there can be multiple players?

Here's an example where a singleton could be used:

Let's say you're making an RTS game. You have a world map, some key-structures on the map that the player must destroy in a specific order, and every time they destroy a key-structure, some kind of event triggers (maybe enemies spawn somewhere, a cutscene plays, etc).

You could create an

ObjectiveManager

singleton that tracks which key-structures in the game are still alive, which one should be destroyed next, and what should happen when each one is destroyed.
The key-structures themselves don't need to worry about when they can be destroyed and what should happen after, they can just tell the manager that they were destroyed, and the manager decides what should happen.

Pseudo-code:

Code (CSharp):

1. public class KeyStructure : MonoBehaviour {
2.    public int health;
3.  
4.    public void TakeDamage(int damage) {
5.       health -= damage;
6.  
7.       if(health <= 0) {
8.          ObjectiveManager.Instance.DestroyKeyStructure(this);
9.       }
10.    }
11. }

Code (CSharp):

1. public class ObjectiveManager : MonoBehaviour {
2.    public static ObjectiveManager Instance { get; private set; }
3.  
4.    //The tags for each KeyStructure that must be destroyed in order.
5.    public string[] structureTags;
6.    private int nextStructIndex= 0;
7.  
8.    void Awake() => Instance = this;
9.  
10.    public void DestroyKeyStructure(KeyStructure structure) {
11.       if(structure.CompareTag(structureTags[nextStructIndex])) {
12.          OnKeyStructureDestroyed(structure);
13.          Destroy(structure.gameObject);      
14.       }
15.       else {
16.          throw new Exception("This KeyStructure's tag does not match any tags in ObjectiveManager, or it cannot yet be destroyed before another KeyStructure.");
17.       }
18.    }
19.  
20.    private void OnKeyStructureDestroyed(KeyStructure structure) {
21.       //Raise some event.
22.       //Update some objective.
23.       //Do whatever else.
24.  
25.       nextStructIndex++;
26.  
27.       if(nextStructIndex > structureTags.Length) {
28.          //All structures destroyed. End the game.
29.       }
30.    }
31. }

 

Oh I see, thank you for the advice and clarification, I really appreciate it.