How and where is this variable becoming null?

public class Unit : MonoBehaviour
{
protected bool selected = false;
protected MediumTank unitSelected;

    protected void TerrainOn(Transform clop)
    {
        Debug.Log(unitSelected); // This line returns null

        Collider[] colliderList = Physics.OverlapSphere(clop.position, 0.1f);
        if (colliderList.Length != 0)
        {

            switch (colliderList[0].tag)
            {
                case "town":
                    GameObject townhexon = GameObject.Find("townHex");
                    unitSelected.CurrentDefence = unitSelected.InitialDefence + townhexon.GetComponent<Town>().town.VehDefence;
                    break;

                case "plains":
                    GameObject plainsHexOn = GameObject.Find("plainsHex");

                    unitSelected.CurrentDefence = unitSelected.InitialDefence + plainsHexOn.GetComponent<Plains>().plains.VehDefence;
                    break;
            }
        }
    }

    private void Update()
    {
        if (Input.GetMouseButtonDown(0))
        {
            RaycastHit rayHit;

            if (Physics.Raycast(Camera.main.ScreenPointToRay(Input.mousePosition), out rayHit, Mathf.Infinity, unitLayer))
            {
                unitSelected = rayHit.collider.GetComponent<MediumTank>();
                Debug.Log(rayHit.collider.name);
                unitSelected.selected = !unitSelected.selected;
            }
            Debug.Log(unitSelected); // This line does not return null
        }

    }

}

In this script, unitSelected returns whichever unit is selected, but when I call TerrainOn() unitSelected is suddenly null, even though nothing is happening between assigning a unit to unitSelected and calling TerrainOn(). I put Debug.Log(unitSelected) everywhere I could think of but it never returned null until TerrainOn() was called here:

public class MediumTank : Unit
{
    public int Lives;
    public int InitialDefence;
    public int CurrentDefence;
    public int[] DiceInfantry;
    public int[] DiceArtillery;
    public int[] DiceTruck;
    public int[] DiceAV;
    public int[] DiceLTank;
    public int[] DiceMTank;
    public int[] DiceHTank;
    public Transform clod;

    private void Update()
    {
        if (selected)
        {
            if (Input.GetKeyDown(KeyCode.T))
            {
                TerrainOn(clod);
                OpenFire(gameObject, GetComponent<MediumTank>());
            }
        }
    }
}

There are at the moment two objects with MediumTank attached, and they behave properly when I do other stuff with them, but not when calling TerrainOn()

Not to take any thunder from @Captain_Pineapple , as he truly has gone above and beyond to help you with this question… But I must add that calling GetComponent during runtime for calculations and Gets is extremely slow(plus severely hurts performance).

So your initial call for GetComponent is probably too slow to handle said function, so the first return in value is obviously null.

My solution to this problem(is kinda like Caching), but instead I use static variables. As anything static is basically in every class since it always exists, so no time at all is used to perform complex calculations from script to script. A prime example of this is my post and answer : Code Optimizing - Cache an Iteration From a List - Questions & Answers - Unity Discussions

So even if this is the correct answer, or my post helps you extremely, please reward the answer to CaptainPineapple. And if you feel credit is due to me in some way, just say thanks or give a upvote within my post I linked :slight_smile:

So in general i cannot really see why this should be null assuming that you never touch this anywhere else.

So lets try to trust you on this on. I’d suggest to simply change the script as this perhaps is a bit of a problem that results from a system that imo should not be created in the way that it is.

Firstly: Classes should be named after the function they have/the objects they represent. They should contain only the code that is needed for this actual object.

If code is needed for more then that it should be somewhere else.

Given this basic rule i’d suggest to change your current setup completly.

Keep the Unit Script. Keep the Medium Tank script. BUT: The Unit script should only be a container for behaviour and data of a unit. So selection should be done somewhere else.
Most pressingly because you would currently trigger 100 raycasts if you have 100 Medium Tanks in your scene and press the left mouse button.

So as a suggestion for new classes that might work better for you and give you a better structure to build from:

public class Unit : MonoBehaviour
{
    protected bool selected = false;
    public int Lives;
    public int InitialDefence;
    public int CurrentDefence;
    public int[] Dice;

    public void TerrainOn(Transform clop)
    {
        Collider[] colliderList = Physics.OverlapSphere(clop.position, 0.1f);
        if (colliderList.Length != 0)
        {
            switch (colliderList[0].tag)
            {
                case "town":
                    GameObject townhexon = GameObject.Find("townHex");
                    CurrentDefence = InitialDefence + townhexon.GetComponent<Town>().town.VehDefence;
                    break;

                case "plains":
                    GameObject plainsHexOn = GameObject.Find("plainsHex");

                    CurrentDefence = InitialDefence + plainsHexOn.GetComponent<Plains>().plains.VehDefence;
                    break;
            }
        }
    }
}

public class MediumTank : Unit
{
    public Transform clod;
}

So the scripts above would be all that a Unit/Tank should know about itself. It should contain functions that itself does. (Perhaps the OpenFire also belongs here?)

Then the thing below is something that you add as to each gamescene on an empty transform somewhere. It manages the clicking of units in a centralized setting. It also can keep track of all currently selected units and trigger things on all of them should you want this:

public class SelectionManager : MonoBehaviour
{
    //base for multi unit selection: 
    public List<Unit> currentSelectedUnits = new List<Unit>();
    //Use this if only one Unit should be selected:
    //public Unit selectedUnit;
    private void Update()
    {
        if (Input.GetMouseButtonDown(0))
        {
            RaycastHit rayHit;

            if (Physics.Raycast(Camera.main.ScreenPointToRay(Input.mousePosition), out rayHit, Mathf.Infinity, unitLayer))
            {
                //check if we have clicked on a "unit"
                var unitSelected = rayHit.collider.GetComponent<Unit>();
                //check if it is already slected:
                if (unitSelected.selected)
                {
                    currentSelectedUnits.Remove(unitSelected);
                }
                else
                {
                    currentSelectedUnits.Add(unitSelected);   
                }
                Debug.Log(rayHit.collider.name);
                unitSelected.selected = !unitSelected.selected;
            }
        }
        if (Input.GetKeyDown(KeyCode.T))
        {
            foreach(var unit in currentSelectedUnits)
            {
                 //this here is just a suggestion, you might ahve to adjust this as i don't know what OpenFire does or where it is:
                TerrainOn(unit.clod);
                OpenFire(...);
            }
        }
    }
}

This would in my opinion be a way better way to go for this. (Ofc the List of Units is not needed, this was just my personal touch. Change it back to just be one “Unit” without the List if the player should ever only be able to command one Unit at a time)

This should also take care of your nullreference problem.

As an additional Rule that is important: If you use Inheritance and your baseclass contains a reference to a class that is a child you are doing something wrong. (See protected MediumTank unitSelected;in your original code - this must not happen)

Also if you use inheritance and have additionally different named variables per different type (see DiceMTank, DiceLTank and so on) you are doing something wrong. With inheritance this can all be done in a more efficient way. ->see Dice in the Baseclass.

Let me know if you have questions regarding this or something is unclear.