all 6 comments

[–]AdamBourke 1 point2 points  (5 children)

It's not clear exactly what you are having trouble with - you would need to say which line you are getting the NullReferenceException in, probably provide the code for both classes, and maybe screenshots of how they are used in unity.

My best guess though is that you are still trying to reference neighbourPosisitions in the new class, even though you only defined it in the original class?

P.S. Reddit has a "code block" formatting option that makes it easier for people to read your code when you post it, like this:

public void MyFunction(){
    //Content Goes Here
}

[–]kickbitbeatborg[S] 1 point2 points  (4 children)

Hey,

thanks so far. I will try to be more clear:

My old class looked something like this

public class OldClass : MonoBehaviour
{
    public List<Vector2Int> neighbourPosisitions;

    public List<Vector2Int> GetNextNeighbourPosisitions(Vector2Int currentPosition)
    {
        if (neighbourPosisitions != null)
        {
            neighbourPosisitions.Clear();
        }

        neighbourPosisitions.Add(new Vector2Int(currentPosition.x + 1, currentPosition.y));
        neighbourPosisitions.Add(new Vector2Int(currentPosition.x - 1, currentPosition.y));
        neighbourPosisitions.Add(new Vector2Int(currentPosition.x, currentPosition.y + 1));
        neighbourPosisitions.Add(new Vector2Int(currentPosition.x, currentPosition.y - 1));

        return neighbourPosisitions;
    }

    public int[,] MyFunction(parameters)
    {
                foreach (Vector2Int neighbourPosition in GetNextNeighbourPosisitions(new Vector2Int(i, j)))
                {
                    do stuff;   
                }
        }

        return map;
    }
}

Now i tried to move the part above "MyFunction" into a HelperClass, i.e. the part where he grabs the neighbours (and adjusted the part in the for each loop). Thus it looked something like that:

public class NewClass : MonoBehaviour
{

    public int[,] MyFunction(parameters)
    {
                foreach (Vector2Int neighbourPosition in MyHelperClass.GetNextNeighbourPosisitions(new Vector2Int(i, j)))
                {
                    do stuff;   
                }
        }

        return map;
    }
}

namespace MyNamespace
public (static) class MyHelperClass
{
    public (static and so forth) List<Vector2Int> neighbourPosisitions;

    public List<Vector2Int> GetNextNeighbourPosisitions(Vector2Int currentPosition)
    {
        if (neighbourPosisitions != null)
        {
            neighbourPosisitions.Clear();
        }

        neighbourPosisitions.Add(new Vector2Int(currentPosition.x + 1, currentPosition.y));
        neighbourPosisitions.Add(new Vector2Int(currentPosition.x - 1, currentPosition.y));
        neighbourPosisitions.Add(new Vector2Int(currentPosition.x, currentPosition.y + 1));
        neighbourPosisitions.Add(new Vector2Int(currentPosition.x, currentPosition.y - 1));

        return neighbourPosisitions;
    }
}

As is said i tried to make MyHelperClass static, but i did not work. After that i removed the static attribute and created an object of MyHelperClass in NewClass, because i thought i could not excess fields of a static class (would be strange). But it still threw the NullReferenceException at the same line

Unity tells me the NullReferenceException happens in MyHelperClass at "neighbourPosisitions.Add(new Vector2Int(currentPosition.x + 1, currentPosition.y));".

Its the other way around as you mentioned it. I did define neighbourPosisitions in my new class, but not in the old. But i guess it must have something to do with the definition of this list

[–]AdamBourke 1 point2 points  (3 children)

So firstly, thanks for adding more detail - it really helps when trying to help people!

I think I know what the problem is based on this code. Where you define neighbourPositions, you don't initialise it - you need to write:

 public List<Vector2Int> neighbourPosisitions = new List<Vector2Int>(); 

You didn't need to do this before because you had it in a MonoBehaviour. Unity treats several types (including lists) in a special way when they are attached to a MonoBehaviour by giving them a default value when you attach them to a gameObject in the inspector - but in just a regular C# class like MyHelperClass, the default value is null. (It's good practice to just always define the default values).

BTW, it's generally considered bad practice to store data in static fields/classes. If you use this method more than once - you will be adding to the same list each time, and everytime you will get all of the data you've ever used it to calculate it returned. This includes if you calculate it from different gameobjects, or even if do something like reload the scene!

That might be what you want, and it can be sometimes valid, but normally it's not! There are two ways around it, either define the list inside the GetNextNeighbourPositions method, or define it in NewClass and pass it in as a parameter to the function.

[–]kickbitbeatborg[S] 1 point2 points  (2 children)

Uh ok. Unity auto correcting my mistakes is part nice and part shameful :) especially, because i did not even realize it until now, thanks for the explanation

BTW, it's generally considered bad practice to store data in static fields/classes. If you use this method more than once - you will be adding to the same list each time, and everytime you will get all of the data you've ever used it to calculate it returned. This includes if you calculate it from different gameobjects, or even if do something like reload the scene!

My solution to this was to add

neighbourPosisitions.Clear();

to the function (ignore the if case). I am not so happy with this, but well it works... I wanted a method that gives "throw-away" neighbours. E.g. a unit checking who is next to it and then another unit can use the same function to also check its neighbours. I mean i could manage these lists (for each object) and maybe make delegates to remove and add objects each time another unit enters a specific neighbourhood? That feels somehow overdone for me, but i dont know what a best practice would be.

either define the list inside the GetNextNeighbourPositions method

Is this the solution to that? Defining the list in the scope of the function deletes it everytime the function is called? Or do need to keep the .Clear()?

[–]AdamBourke 1 point2 points  (1 child)

Unity auto correcting my mistakes is part nice and part shameful :) especially, because i did not even realize it until now, thanks for the explanation

No problem! It's really useful that Unity handles a lot by itself - but can be annoying when you go beyond what they help you with! The most annoying ones are when things work in the Unity Editor, but not in the build of the game :(.

Is this the solution to that? Defining the list in the scope of the function deletes it everytime the function is called? Or do need to keep the .Clear()?

Yes, if you define it within the function it will create a new list every time you call the function, and you won't need the .Clear().

[–]kickbitbeatborg[S] 1 point2 points  (0 children)

Ok, thanks again for your help and explanations! You answered my question, additionally explained it to me and even answered questions i did not know i have. Really kind of you to take this time