Why are these objects null?

I’m very confused. My script is super long, but ill paste in the bits I think are important.

Everything is in this class

public class Person : MonoBehaviour {

First, I instantiated stuff

public GameObject person;

Person shouldn’t be null, because it is set in unity inspector.

private GameObject spawner;

Spawner shouldn’t be null, because it is set by the script that calls the constructor that builds a new person.
This is the constructor

    public Person(int gender, int skinColor, int eyeColor, int hairColor, int hairStyle, int slot0, int slot1, int slot2, int slot3,
        int slot4, int slot5, int slot6, int slot7, int slot8, int slot9, int slot10, int slot11, int slot12, int slot13, int slot14, int slot15, GameObject spawner)

Within the constructor, a method is called

setupFailed = buildPerson();

In this method, which returns false if it fails, person is created. The console says its this line where the error is located.

Person_cur = (GameObject)Instantiate(person,spawner.transform.position, spawner.transform.rotation);

Any help?

Just one thing: why do you have variables for slots 0 through 15 instead of an array?
You should really post the whole code, because then we will see, if you have an error in your logic or you should nullcheck each variable in that instantiate, and see, which one is the problem if not both

I think GameObject is an abstract base class, so you cant create an instance by just saying
GameObject go = new GameObject();
Instantiate will create an instance though, so you might have to through your sequence around by doing that first and then set the attributes.

He doesn’t do that, at least not in the code he posted
BTW, if he even tried to do that, the problem wouldn’t be a nullreferenceexception

It’s not abstract, the editor just throws a warning when you try that. Worthy of note, though, is that “new GameObject(“Name”)” or “new GameObject(“Name”, typeof(ComponentToAdd)” are totally valid and useful when you want to create a GameObject that isn’t modeled after a prefab.

@OP: You haven’t given us your whole script. If you knew which bits were important, you wouldn’t be asking for help on a null reference error. Debugging a null reference should be one of the easiest things to do, so now’s the time to learn.

A couple of tips, though:
“My script is super long” means you’ve made your script too big and need to break it out into its components.

That constructor is insanely long, a constructor that long means you got problems, yo. Any time a constructor gets that long, it’s time to build a separate class or struct to pass as an argument, instead.

Thanks for that, then my guess would be to place it in the Start function (if possible) and not the constructor. I presume you are inheriting from monobehaviour, because you are using Instantiate, right?

Yes, I’m reworking this so its from a start constructor. I figured that might cause some sort of issue.

I’m not interested in posting this code, cause I don’t want people to copy it. Its rather large.

EDIT Fine I’ll post it. Its working, but not correctly now.

Basically, I’m making a system in order to make custom characters. It takes different models and stitches them to the armature, so you can have different armors etc. I got it working, but it wont spawn any parts. It spawns the base though, so thats a start.

I really doubt anybody wants to steal your broken, gigantic script…I think you’ll find that “I need help with my code but you can’t see it” won’t get you far around here.

I love how some people think anybody would copy some code that doesn’t work

Eh, I thought about it and I don’t care if you use it as long as you give credit to me as creator.

And, this is a complicated script. Its going to be very nice if I get it working. Sorry if you think I’m being too cautious.

Anyways stay on topic pls :smile:.

EDIT: I’m wondering, how would you reference a submodel of a model? for instance:

What if I wanted to get to the maleArms?

This is how I tried it, it wont seem to work.

person.setArmsPrefab(Resources.Load<GameObject>("maleArms"));

There are so much problem with these scripts:

  • getters would be very useful instead of creating a method for every variable
  • SkinColor, Gender and EyeColor… and HairColor are just datacontainers, you could’ve used an enum instead
  • the person constructor is waaay too long, either change that to an array, or give default values to the slots
  • again, {get; set;}
  • having a private variable then creating a getter and setter that does nothing except sets and gets it

and this is only in the second one
just as an experiment I reduced the size of the file solving the problems above (so not even looking into things, like what if instead of storing everything as ints (which results in a really bad user experience) we store them as strings, that would get rid of a lot of “classes”)
and I got it down to 9942 bytes from 19499

I think your problem is that it’s waaaay too complicated, there’s a reason you can make multiple files in unity

Nobody has pointed out yet that you can’t use constructors on MonoBehaviour? That might well be at the root of the problem

These two lines are fundamentally incompatible.

1 Like

The code is not long or large. Its just super verbose. A more experienced programmer would probably write the same thing in about 50-100 lines. And for the record its not worth copying.

Some hints to deal with verbosity

  • Don’t use properties unless you are actually going to do something with them
  • Don’t do custom enum to string conversions. Use enum.ToString() instead.
  • Don’t write your own enum implementations. (I’m looking at the EyeColour class)
  • Passing in an array is generally more readable then passing in 16 numbers and manually assigning them to an array.
  • Cache values you use a lot. Jumping through a dot chain every single time is difficult to read and maintain.

Come back with a 50-100 line readable version, and we can probably find your null references in a couple of minutes.

2 Likes

Well, I’m kinda new to programming. Never used C# until yesterday either, so I’m kinda guessing how some things work. Perhaps I should drop this for now, and come back later when I more, but really, I found I learn more by actually attempting at it. So I acknowledge the code is long/sloppy/verbose, and I apologize for that.

Maybe someone would like to help me learn more about how to do this better?

Good point, I’m adjusting things with this in mind and its working slightly better. Wont be long before I get it working, I hope! Then I can spend another few days cleaning it up a bit.

I’ve decided I’m going to release this for free for anyone to use, or at least make a tutorial on YouTube on how to do this. I see lots of people who want a system like this, and I’d love to lend them a hand.

Learning as I go.

Keep building it, but meanwhile read up on some design patterns and think about how you can do it better…think of this as your prototype. Ideally, you use it to get a handle on how some things should work, then throw it away and start from scratch.

One thing that’s going to help you a lot is the DRY principle. Don’t Repeat Yourself. Any time you see repetitive code, it’s code that almost certainly can be removed and replaced with a single line, or not replaced at all. For example, you have a lot of lines like this:

person.setHead((GameObject)Instantiate(person.getHeadPrefab(), person.getSpawner().transform.position, person.getSpawner().transform.rotation, person.getPerson_Cur().transform));
person.getHead().GetComponent<Renderer>().sharedMaterial = person.getSkinMaterial();

What if instead of “SetHead”, “SetEyes”, etc. you had a single “SetBodyPart” method? What if instead of passing all those parameters, you just passed it the person, and let it figure out the rest, since you’re getting everything you’re passing from the Person class anyway? What if that method also took responsibility for setting the material of the body part? You could reduce all those confusing, copy/paste lines with a single for loop and a single method. Suddenly, the length and complexity of your code has been reduced by almost half.

for (var i = 0; i < bodyParts.Length; i++)
{
    bodyPart = person.SetBodyPart(person, bodyParts[i]);
}

I’m also seeing you use a lot of Setters and Getters. You must be unaware of C# auto properties.
BodyPart Head { get; set; } is shorthand for writing

private BodyPart _head;
public BodyPart GetHead()
{
    return _head;
}
public void SetHead()
{
    _head = value;
}

So that’s how you reduce 9 ugly-looking lines into a single elegant line, while still maintaining encapsulation.

1 Like

I’ve never even herd of a struct before, but I’m going to make one now. Like I said, I know programming, but not C#. Or any C language for that matter, though I know GML which I’ve heard is simillar to C.

Thank you for the tips Dameon! I’m reading it now, going to add the new things you told me.

I’m getting this error, tell me what you think:

MissingComponentException: There is no 'Renderer' attached to the "maleEars(Clone)" game object, but a script is trying to access it.
You probably need to add a Renderer to the game object "maleEars(Clone)". Or your script needs to check if the component is attached before using it.
basic1NPC.buildPerson (.Person person) (at Assets/scripts/missionstemplates/basic1NPC.cs:68)
basic1NPC.Start () (at Assets/scripts/missionstemplates/basic1NPC.cs:16)

Is your game 2D? If so, you need to search for Renderer2D instead. Otherwise, the ears you’re instantiating either have no renderer, or it’s on a child object.

I think its on a child object. I must be doing this incorrectly, darn. I’m struggling with Resource.Load(“namegoeshere”) I think. Any tips how I should do this properly?

To start, Resources.Load() takes a path, not an object name.