problem with a coroutine

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

I'm having an issue with a coroutine. I believe it is working as it should, but it's not what I want and I'm having problems figuring out how to rework my code to do what I want. 

 

Here is what I want it to do: This is an arkanoid clone game. The ball come in and hits the paddle when a special pick up is active. When it hits the paddle, as it bounces away, the AssaultRifleShot method is called, it runs a few lines of code and then calls the StartCoroutine method, that method waits for a time, then calls the SpecialShot method. That method creates a new ball, does a few other things, then returns the control to the AssaultRifleShot method, which will keep going through the while loop as long as it needs to. The purpose of the coroutine is so that the balls all spawn in a line going in one direction, without hitting each other or spawning on top of each other.

Here is what it is actually doing: The coroutine timer is in fact waiting, but it is immediately returning control to the AssaultRifleShot even while the coroutine is still waiting. So by the time the first ball is firing, the other coroutines are nearly finished running, so all the balls end up spawning at almost the exact same time, which causes them to collide with each other and go flying all over the place.

So, is there a way to modify this so that the control is not given back to the AssaultRifleShot until AFTER the timer has finished running and the rest of SpecialShot method has been run?

 

Here is the code for the AssaultRiflePickUp:


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

	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 ()	//spawn a new ball. add it to the list of balls. set it's spawn location, set it's velocity to match the original ball
	{
		Debug.Log("SpecialShot");

		Instantiate(newBall, new Vector3(paddle.transform.position.x, (paddle.transform.position.y + 1), 0), Quaternion.identity);
		newBall.GetComponent<Ball>().hasStarted = true;
		GameManager.pickUpManager.allBalls.Add(newBall.GetComponent<Ball>());
		//newBall.GetComponent<Ball>().paddleToBallVector = newBall.transform.position - paddle.transform.position;
		//newBall.transform.position = paddle.transform.position + newBall.GetComponent<Ball>().paddleToBallVector;  
		newBall.GetComponent<Rigidbody2D>().velocity = mainBallVector; 		//use the original ball's vector3 to follow it directly
	}

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

		//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);
		mainBallVector = mainBall.GetComponent<Rigidbody2D>().velocity; //grab mainBall's direction and speed before making it a destroyable ball
		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) 
		{ 
			int i = 6;

			while(i > 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");
				i--;
			}
		} 
		else 
		{
			DestroyPickUp();	
		}
	}
}

 

Advertisement

Assuming the method you're asking about is called by something like Update or OnColliderEnter then you can't have it delay until the co-routine finishes without nasty consequences. Unless I'm mistaken Update and such block the game's main thread until they complete execution. The worst case is that you create a deadlock between the co-routine and the method waiting on it. The simplest solution I can see to get the behavior you want is to move the loop into the co-routine. [Edit: IMO this would also be the cleaner solution to your problem]

 


    //a timer used for timed pick ups
    public IEnumerator PowerUpTimer (float delay)
    {	
        Debug.Log("PowerUpTimer");
        int shotsLeft = 6;
        while(AmmoRemaining > 0 && shotsLeft > 0)
        {
            yield return new WaitForSeconds (delay);
            mainBallVector = mainBall.GetComponent<Rigidbody2D>().velocity; //grab mainBall's direction and speed before making it a destroyable ball
            SpecialShot();
            AmmoRemaining--;
            shotsLeft--;
        }
    }

A quick and dirty way of blocking execution on a co-routine would be to have a boolean set to true at the start of the co-routine with a while loop that executes until the boolean is set to false at the end of the co-routine. I suspect doing this will hard-lock the game but I've not tried it. Even if it doesn't I would recommend against it since tight loops like that are an inefficient way of waiting.


//a timer used for timed pick ups
public IEnumerator PowerUpTimer (float delay)
{	
	coroutineRunning = true;
	Debug.Log("PowerUpTimer");
	yield return new WaitForSeconds (delay);
	mainBallVector = mainBall.GetComponent<Rigidbody2D>().velocity; //grab mainBall's direction and speed before making it a destroyable ball
	SpecialShot();
	coroutineRunning = false;
}
 
// I'm pretty sure this will lock up
public void ThingThatWaitsForCoroutine()
{
	// other code
	while(coroutineRunning) {}
    
	// stuff after coroutine is done
}

 

--------------------------Insert witty comment here!--------------------------

The method is called by an event, the event is called by a ball colliding with the paddle, so yes OnCollisionEnter2D. As for putting the loop inside the coroutine, I don't believe that would work. I need to spawn ball, wait, spawn ball, wait, spawn ball, wait, etc. But if the loop is inside the coroutine it is going to wait...spawn ball, spawn ball, spawn ball, etc. I suppose I can possibly try  a boolean like you suggest, but also like you said I'm not sure that will work or that it's a good idea. Are there any alternatives to coroutines that would work as a timer that I could use in this situation? I'm open to other ways of doing this. I tried using a timer of some sort previously but it of course stopped everything else in the game while the timer was counting which won't work for what I need. 

I'm not using Unity right now so I can only speculate on the coroutine aspect of it.

That said, Dan DAMAN's solution does seem like it would produce the desired behavior (wait, spawn, wait, spawn, etc.). Again, I could be wrong, but it might at least be worth trying.

As you've observed, your current code (or so it appears to me) just starts a bunch of coroutines all at once, each of which waits the same amount of time before spawning a new ball, hence the behavior you're seeing. A possible solution (although perhaps not an optimal one) might be to increase the delay for each subsequent coroutine, e.g.:


StartCoroutine(timer, GameManager.monoBehaviour);
timer += <desired time interval between balls>;

Although I know coroutines are important in Unity and I imagine it would be valuable to understand them well, they're certainly not the only way to do this. In the absence of coroutines or coroutine-like functionality, something like this would typically be done with a simple state manager and some timing code.

I think I was thinking incorrectly, after reading your post I realized that putting the whole loop in the coroutine may actually fix the issue IF I put the stuff that is already in the coroutine inside the loop. The way I was first thinking about it was to have the timer, followed by the loop, in which case the timer would only be called once. Now it makes sense! I'll give it a shot!

This topic is closed to new replies.

Advertisement