Getting a null reference but method is being called from object?

Started by
8 comments, last by ethancodes 5 years, 9 months ago

I've got a bit of a weird setup here so to explain real briefly, I have pickups that when hit, the game object is destroyed, but the script of the object is cloned and put into a list. This is so my pick up queue knows which pick ups we have collected, and which one to activate next. This is the error I'm getting:

 

NullReferenceException: Object reference not set to an instance of an object
AssaultRiflePickUp.StartCoroutine (Single delay, UnityEngine.MonoBehaviour coroutineHost) (at Assets/Scripts/AssaultRiflePickUp.cs:76)
AssaultRiflePickUp.AssaultRifleShot (UnityEngine.GameObject mainBall) (at Assets/Scripts/AssaultRiflePickUp.cs:93)
Ball.OnCollisionEnter2D (UnityEngine.Collision2D collider) (at Assets/Scripts/Ball.cs:63)
 

And I've put several debug.Log messages to narrow down exactly when it is throwing the error, in the following code, it is thrown right after AssaultRifleShot - 2, but before AssaultRifleShot - 3, so basically it's something to do with my coroutine I believe. Hopefully someone can help me figure out what is going on with it because I'm confused how it can say null exception when it's already calling a method from the object. Here is the code:


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

public class AssaultRiflePickUp : BasePickUp, ISpecialShotPickUp
{
	private int AmmoRemaining = 35;
	private float timer = 0.3f;

	public GameObject newBall;

	void Start ()
	{	
		//assign paddle to variable
		paddle = GameObject.FindObjectOfType<Paddle> ();

		//find the ball and assign it to this variable
		foreach (Ball ball in GameManager.pickUpManager.allBalls) 
		{
			if (ball.mainBall) 
			{
				mainBall = ball;
			}
		}
	}

	public override void ActivatePickUp (MonoBehaviour coroutineHost)
	{
		Debug.Log("ActivatePickUp");
		//subscribe to events
		mainBall.onBallHitPaddle += AssaultRifleShot;
		mainBall.onBallCollided += DestroyExtraBalls;
	}

	public void SpecialShot ()
	{
		Debug.Log("SpecialShot");
		Vector3 mainBallVector = mainBall.GetComponent<Rigidbody2D>().velocity; //grab mainBall's direction and speed before making it a destroyable ball
		mainBall.GetComponent<Ball>().mainBall = false;

			Instantiate(newBall);
			GameManager.pickUpManager.allBalls.Add(newBall.GetComponent<Ball>());
			newBall.transform.position = paddle.transform.position + ballSpawnOffset; //use the original ball's vector3 to follow it directly
			newBall.GetComponent<Rigidbody2D>().velocity = mainBallVector;
	}

	public void DestroyExtraBalls (GameObject ball)
	{
		if (GameManager.pickUpManager.allBalls.Count > 1) 
		{
		}

		//TODO: Implement observer pattern to watch for balls colliding with something. If they do and they are not the main ball, destroy them.
	}

	public void DestroyPickUp ()
	{
			Debug.Log("DestroyPickUp");
			Destroy (GameManager.pickUpManager.pickUpQueue [0]);
			GameManager.pickUpManager.pickUpQueue.RemoveAt (0);
			GameManager.pickUpManager.pickUpActive = false;
	}

	//a timer used for timed pick ups
	public IEnumerator PowerUpTimer (float delay)
	{	
		Debug.Log("PowerUpTimer");
		yield return new WaitForSeconds (delay);
		SpecialShot();
	}

	//The coroutine is used to start a short timer so that the balls don't spawn on top of each other all at the same time.
	public void StartCoroutine (float delay, MonoBehaviour coroutineHost)
	{
		coroutineHost.StartCoroutine(PowerUpTimer(delay));
	}


	//Controls the pick up's cycle
	public void AssaultRifleShot (GameObject mainBall)
	{
		Debug.Log("AssaultRifleShot called");

		if (AmmoRemaining > 0) 
		{ 
			Debug.Log("AssaultRifleShot - Inside If Statement");
			//if there is ammo remaining, set the main ball to just a normal ball so it can be destroyed on impact and then start coroutine
			mainBall.GetComponent<Ball> ().mainBall = false;
			Debug.Log("AssaultRifleShot - 1");
			mainBall.GetComponent<Ball> ().onBallHitPaddle -= AssaultRifleShot; //unsubscribe to event when mainBall is set to false.
			Debug.Log("AssaultRifleShot - 2");
			StartCoroutine(timer, GameManager.monoBehaviour);
			Debug.Log("AssaultRifleShot - 3");
			AmmoRemaining -= 1;
			Debug.Log("AssaultRifleShot - 4");
		} 
		else 
		{
			DestroyPickUp();	
		}
	}
}

 

I'll post the cloning code here just in case it has anything to do with it. Maybe I'm not cloning it correctly? But it's working for my other pick ups, which also use coroutines.


	public object Clone ()
	{	//clone the pick up object to prevent a null reference when the pick up game object is destroyed
		BasePickUp newPickUp = (BasePickUp)this.MemberwiseClone();

		return newPickUp;
	}

 

Advertisement

I don't think you can clone object in Unity like that.  At any rate, I think that's a code smell.  Read further down for a cleaner way to implement this type of behavior.  In Unity, C# objects that wrap C++ objects where the C++ object has been destroyed compare true with null, but are not null.  This is done because if something still holds a reference to the C# object, it can't be destroyed, that's just not how C# works.  If you clone a MonoBehaviour and its GameObject is destroyed, for example, I'm guessing it's in this state.  I haven't tested it, but I'm quite certain you can't clone components in this way.  The only way I've found to properly clone a component is to use the reflection API to copy all the fields over to a new instance of it.  But even that is smelly, I do it in hacky editor scripts but not in game code.

I would move this type of code into a ScriptableObject.  I use ScriptableObjects as "drivers" for objects, meaning they start coroutines on those objects and modify their behavior in some way, almost like they're an evil spirit possessing them.  ScriptableObjects are easy to refer to in all manner of scripts and their references are global, so you don't have to worry about cloning anything.  You then just need a minimal pickup object that detects the collision and tells the player "Hey, you picked up this powerup implemented by this ScriptableObject."  The player can then call a method on the ScriptableObject to start the coroutines on it that implement its behavior.  The thing is here, you more or less already have this in your code.  With minimal modifications you can move this to a much less smelly place.

This is also really great for pickups, because with one script you can then define different variations of those pickups.  So a Health pickup just needs a value of how much it heals, and then you create different scriptable objects with this script on it and different health values.  If at any time you need to rebalance how much the small, medium and large health boosts give you, all you need to do is modify the scriptable objects and it happens globally.  Doing this with prefabs is a much more difficult and error-prone process.

Any tips on how I can accomplish this? I know nothing about scriptable objects, and from what I'm reading I'm getting confused.

From what I'm understanding, you are suggesting changing all of my pick ups to scriptable objects, right? if that is the case, i'd really like to find another solution. Not because your solution is wrong, in fact it may be a better idea to do it that way next time. But at this point I have so much completed already that this seems like it would be a huge undertaking to modify my code to make it work, all because of a coroutine issue. Instead, i'd like to try to find around this issue, or an alternative way to accomplish the same thing. Essentially, my problem is this: When my ball hits my paddle, I need to spawn 6 additional balls. As the original ball bounces away from the paddle, the other balls will spawn in front of the paddle and follow the original. If I just let them all spawn without the coroutine, they will basically spawn on top of each other and go in all kinds of different directions. However, if I make it wait each time before spawning, then the other ball will be already far enough out that they won't collide. Can anyone think of any other solution to my problem that does not include using the coroutine, since that seems to be the weak link here? I'm wondering if maybe there is a way I can put a collider around the paddle and say spawn next ball when no other balls are present inside the collider? I'm just not sure how easy it is to check for things already inside a collider? Thanks for any input.

Wait... is the coroutineHost null this time?  Check to see when/how GameManager.monoBehaviour might be getting destroyed.  Or make sure it's actually getting assigned properly.  You'll need that to be an active, non-destroyed monobehavior to use it as a coroutine host.

7 hours ago, Nypyren said:

Wait... is the coroutineHost null this time?  Check to see when/how GameManager.monoBehaviour might be getting destroyed.  Or make sure it's actually getting assigned properly.  You'll need that to be an active, non-destroyed monobehavior to use it as a coroutine host.

It's just a static variable on the GameManager. So it's public static MonoBehaviour monobehaviour;

Since it kept saying it wasn't assigned to an instance of an object, I thought perhaps I needed to new up this variable, so I tried adding a Start function to GameManager and doing monobehaviour = new Monobehaviour(); but that gives me an error saying you can't use the word 'new' with Monobehaviour. But the GameManager never gets destroyed obviously, so I don't know why monobehaviour would not be attached to the instance of GameManager?

All member variables (even static ones) start out as 0/false/null.  GameManager should have something like this to initialize the variable:


void Awake()
{
    monoBehaviour = this;
}

 

Ah I see. I'll try it out tonight! Thanks!

This did fix the error I was getting and is making the ball wait to spawn as desired! Thanks!

This topic is closed to new replies.

Advertisement