Colliding 2 times with the same object

Question

I want to start by saying that I need some guidance, the first thing is that my player is colliding with the upgrade(Cube) object 2 times every time, so instead of receiving 2damage, I receive 4 damage. This is the colliding code:

public void OnTriggerEnter(Collider other)
    {
        if (other.CompareTag("Upgrade") && this != null)
        {
            Debug.Log("Collide:" + other.name);

            Upgrades upgradesScript = other.GetComponent<Upgrades>();
            
            if(upgradesScript != null)
            {
                Damage += upgradesScript.Damage;
                Health += upgradesScript.Health;
                Xspeed += upgradesScript.Xspeed;

            }

        }

    }

And here is the upgrade script:

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class Upgrades : MonoBehaviour
{

    public float Damage; //id 0
    public float Xspeed; //id 1
    public float Health; //id 2
    public float itemID;
    public float stage = 1;



    void Start()
    {
        if(itemID == 0)
        {
            RandomDamage();
        }
        else if(itemID == 1)
        {
            RandomXspeed();
        }
        else
        {
            RandomHealth();
        }

  
    }

    void Update()
    {
        
    }

    public void RandomDamage()
    {
        Damage = Random.Range(1 , 3 );
    }

    public void RandomXspeed()
    {
       Xspeed = Random.Range(1 * stage, 3 * stage);

    }

    public void RandomHealth()
    {
        Health = Random.Range(5 , 10 );

    }

}

This is from inspector:

Now, if you could also tell me if I took the right approach, or what could I do to optimize my code more, regarding the switch instead of if's, I heard is better but I don't fully understand how it is better that's why I haven't used it. Thank you!

Answer 1

Regarding switch vs if statements, switches have at least two main advantages.

1. They are faster, even if only slightly for smaller numbers of cases. This is because with switches, the compiler generates a "jump table". This means that it checks "what case to go to", while if statements check "if each case can be satisfied".

2. It's more readable, which is very important. Each case is a lot shorter than any if statement, and the more unique word choice and structure means it will blend in less with the rest of your code.

If statements can have a lot more variety and are good if you have multiple elements to track, or one condition much more likely than the others, but if you're only making multiple options based on the condition of one element, always use a switch.

Additionally, they're very good for enums, which I think you should use here. If you don't know, an enum is a set of named values called elements. In effect, they CAN work just like ints, but are much more readable and easy to remember (especially in editor).

So, for your if statements, try this instead.

    public float Damage; //id 0
    public float Xspeed; //id 1
    public float Health; //id 2
    public UpgradeTypes itemID; //Will default to damage
    public float stage = 1;
    
    public enum UpgradeTypes
    {
        Damage,
        Speed,
        Health
    }
    
    void Start () {
        switch (itemID)
        {
            case UpgradeTypes.Damage:
                RandomDamage();
                break;
            case UpgradeTypes.Speed:
                RandomXspeed();
                break;
            case UpgradeTypes.Health:
                RandomHealth();
                break;
        }
    }

Here are a few articles and videos for more details on switches. https://www.youtube.com/watch?v=-f9PMeYbnMc https://www.youtube.com/watch?v=fjUG_y5ZaL4 https://medium.com/@michellekwong2/switch-vs-if-else-1d458e7b0711#:~:text=A%20switch%20statement%20works%20much,case%20has%20to%20be%20executed.

As for your main question. My first recommendation is to add an OnTriggerEnter method to your Upgrades script, and have this Debug.Log the name of whatever collider object it comes into contact with. This will let you know if it's the same player collider hitting the upgrades twice, or two different ones.

Because if your player has a rigidbody (and I can only assume it does), then any and all colliders it has, even if they're attached to child objects, will trigger the OnTriggerEnter(). E.G. if you have a main collider and a smaller collider on a child for checking walls or something, both will trigger the Upgrades object.

If the issue is the effects being called twice, a simple bool can make it only be done so once.

    private bool didUpgradeThisFrame;
            
    public void LateUpdate() 
    {
            didUpgradeThisFrame = false;
    }
            
    public void OnTriggerEnter(Collider other)
    {
                    if (other.CompareTag("Upgrade") && this != null)
                    {
                        Debug.Log("Collide:" + other.name);
            
                        Upgrades upgradesScript = other.GetComponent<Upgrades>();
                        
                        if(upgradesScript != null && !didUpgradeThisFrame)
                        {
                            didUpgradeThisFrame = true;
                            Damage += upgradesScript.Damage;
                            Health += upgradesScript.Health;
                            Xspeed += upgradesScript.Xspeed;
            
                        }
            
                    }
    }