Problem Stoping Firing Coroutine

[Antonio_Bautista]

Hello guys! I was following the lesson step by step and I found this problem, when I Get my button up the coroutine doesnt stop. I tried to place a Debug.Log to see if it didnt entered the if stamente but it does. Here’s the two options of coding I tried:
1—

void Fire()
    {
        if (Input.GetButtonDown("Fire1"))
        {
            StartCoroutine(FireContinuously());
        }
        if (Input.GetButtonUp("Fire1"))
        {
            StopCoroutine(FireContinuously());
            Debug.Log("Paramos");
        }
    }
    IEnumerator FireContinuously()
    {
        while (true)
        {
            GameObject laser = Instantiate(laserPrefab, transform.position, Quaternion.identity) as GameObject;
            laser.GetComponent<Rigidbody2D>().velocity = new Vector2(0, projectileSpeed);
            yield return new WaitForSeconds(projectileFiringPeriod);
        }
    }

2----

void Fire()
    {
        if (Input.GetButtonDown("Fire1"))
        {
            firing = StartCoroutine(FireContinuously());
        }
        if (Input.GetButtonUp("Fire1"))
        {
            StopCoroutine(firing);
            Debug.Log("Paramos");
        }
    }
    IEnumerator FireContinuously()
    {
        while (true)
        {
            GameObject laser = Instantiate(laserPrefab, transform.position, Quaternion.identity) as GameObject;
            laser.GetComponent<Rigidbody2D>().velocity = new Vector2(0, projectileSpeed);
            yield return new WaitForSeconds(projectileFiringPeriod);
        }
    }

[Tristan_Hedges]

Well, that’s pretty curious. Are you still having this issue or have you managed to figure it out? I completed Laser Defender a couple months ago and looking at my finished code, yours seems pretty much the same and mine worked just fine. Is there any additional context/code you can provide if you’re still having the issue?

[Antonio_Bautista]

Yes, I couldnt fix it yet. I’m using Unity 2020.1.17f1 and here’s the full code I’m using:

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

public class Player : MonoBehaviour
{
    [SerializeField] float moveSpeed = 10f;
    [SerializeField] float padding = 1f;
    [SerializeField] GameObject laserPrefab;
    [SerializeField] float projectileSpeed = 10f;
    [SerializeField] float projectileFiringPeriod = 0.1f;
    float xMin;
    float xMax;
    float yMin;
    float yMax;
    // Start is called before the first frame update
    void Start()
    {
        SetUpMoveBoundaries();
    }

    // Update is called once per frame
    void Update()
    {
        Move();
        Fire();
    }
    void Move()
    {
        var deltaX = Input.GetAxis("Horizontal") * Time.deltaTime * moveSpeed;
        var deltaY = Input.GetAxis("Vertical") * Time.deltaTime * moveSpeed;
        var newXPos = Mathf.Clamp(transform.position.x + deltaX, xMin, xMax);
        var newYPos = Mathf.Clamp(transform.position.y + deltaY, yMin, yMax);
        transform.position = new Vector2(newXPos, newYPos);
    }
    void Fire()
    {
        if (Input.GetButtonDown("Fire1"))
        {
            Coroutine firing = StartCoroutine(FireContinuously());
        }
        if (Input.GetButtonUp("Fire1"))
        {
            StopCoroutine("firing");
            Debug.Log("Paramos");
        }
    }
    IEnumerator FireContinuously()
    {
        while (true)
        {
            GameObject laser = Instantiate(laserPrefab, transform.position, Quaternion.identity) as GameObject;
            laser.GetComponent<Rigidbody2D>().velocity = new Vector2(0, projectileSpeed);
            yield return new WaitForSeconds(projectileFiringPeriod);
        }
    }
    void SetUpMoveBoundaries()
    {
        Camera gameCamera = Camera.main;
        //X axis
        xMin = gameCamera.ViewportToWorldPoint(new Vector3(0, 0, 0)).x + padding;
        xMax = gameCamera.ViewportToWorldPoint(new Vector3(1, 0, 0)).x - padding;
        //Y axis
        yMin = gameCamera.ViewportToWorldPoint(new Vector3(0, 0, 0)).y + padding;
        yMax = gameCamera.ViewportToWorldPoint(new Vector3(0, 1, 0)).y - padding;

    }
}

[TiltedWatcher]

@Antonio_Bautista

StopCoroutine("firing");

I believe that there is the problem. You’re stopping a string, not a coroutine.

Try:

StopCoroutine(FireContinuously());

instead, that should work.

[Antonio_Bautista]

I tried that too and didn’t work :c
Maybe it’s because of my Unity version (? )

[TiltedWatcher]

Wait, you’re assigning the coroutine to a variable. I missed that.
Putting it as:

StopCoroutine(firing),
instead of refering to it by string should fix it.

The issue is definitively in refering to the coroutine by string.
To quote the documentation:

Note: Do not mix the three arguments. If a string is used as the argument in StartCoroutine, use the string in StopCoroutine. Similarly, use the IEnumerator in both StartCoroutine and StopCoroutine. Finally, use StopCoroutine with the Coroutine used for creation.

So given that the Coroutine was started by assigning it to a variable, it should also be stopped via that variable

[Antonio_Bautista]

It’s still not working:

void Fire()
    {
        if (Input.GetButtonDown("Fire1"))
        {
            Coroutine firing = StartCoroutine(FireContinuously());
        }
        if (Input.GetButtonUp("Fire1"))
        {
            StopCoroutine(firing);
            Debug.Log("Paramos");
        }
    }

Unity is giving me an error:

[TiltedWatcher]

Ah that’s because you’re declaring the coroutine firing variable locally. So basically as soon as you’re out of the if statement where you declare it in, unity forgets that it exists.

If you declare it earlier, and then only assign it in the if statement, unity should realize it. You can tell because it’s not recognizing the name of the variable. Something like that is usually a good indicator that the variable is declared in a place you can’t acess from where you’re trying to call it.
So something like this:

void Fire()
    {
       Coroutine firing;
        if (Input.GetButtonDown("Fire1"))
        {
           firing = StartCoroutine(FireContinuously());
        }
        if (Input.GetButtonUp("Fire1"))
        {
            StopCoroutine(firing);
            Debug.Log("Paramos");
        }
    }

As added explanation:
Basically everytime you declare a variable.

ie:

Coroutine firing;
float exampleFloat; = 0f;
string exampleString;
string otherExample = "test"

everything before the = is you telling unity that you want that variable to exist. And…for unity that variable only exists between the closest curly brackets above and beyond.

So for example:

public class ExampleClass{

	float example; //since this is defined inside the class body, it can be accessed anywhere inside this class

	void ExampleMethod() {

		float otherExample; //since this is decladed inside ExampleMethod, you can access it anywhere inside ExampleMethod. But ExampleMethodTwo would not know that this float exists

		if (true) {
			float moreExamples //because you declade it inside the curly brackets of this if statement, you can only access it until this if statement ends. even the else block doesn't know that this variable exists.
		} else {
		
		}


	}

	void ExampleMethodTwo() {
		//nothing in here
	}
	
}

That’s why the error unity is giving you says that firing doesn’t exist in that context, because it’s not declared inside or on a higher level than that particular if statement.

[Antonio_Bautista]

It’s still not working:

void Fire()
    {
        Coroutine firing;
        if (Input.GetButtonDown("Fire1"))
        {
            firing = StartCoroutine(FireContinuously());
        }
        if (Input.GetButtonUp("Fire1"))
        {
            StopCoroutine(firing);
            Debug.Log("Paramos");
        }
    }

Then vissual studio recomended me to do this:

 void Fire()
    {
        Coroutine firing;
        if (Input.GetButtonDown("Fire1"))
        {
            firing = StartCoroutine(FireContinuously());
        }
        if (Input.GetButtonUp("Fire1"))
        {
            StopCoroutine(routine: firing);
            Debug.Log("Paramos");
        }
    }

[TiltedWatcher]

Interesting. But hey, if the error you get changes, that means you’re making progress when trying to chase a bug.

I just went back and checked the way I did. Which is a bit different to the way rick did it in the course, but fundamentally very similar.

So I’m not sure why it’s not working your case.
The main difference I see between my code and yours, is that I declade the coroutine variable as part of the class (so above your start method), rather than locally.

Not sure why visual studio made that recommendation either, it doesn’t seem to change the result at least.

[Antonio_Bautista]

Oh really thank you. That’s actually what it needed to work haha

public class Player : MonoBehaviour
{
    [SerializeField] float moveSpeed = 10f;
    [SerializeField] float padding = 1f;
    [SerializeField] GameObject laserPrefab;
    [SerializeField] float projectileSpeed = 10f;
    [SerializeField] float projectileFiringPeriod = 0.1f;
    float xMin;
    float xMax;
    float yMin;
    float yMax;

    Coroutine firing;

    // Start is called before the first frame update
    void Start()
    {
        SetUpMoveBoundaries();
    }

    // Update is called once per frame
    void Update()
    {
        Move();
        Fire();
    }
    void Move()
    {
        var deltaX = Input.GetAxis("Horizontal") * Time.deltaTime * moveSpeed;
        var deltaY = Input.GetAxis("Vertical") * Time.deltaTime * moveSpeed;
        var newXPos = Mathf.Clamp(transform.position.x + deltaX, xMin, xMax);
        var newYPos = Mathf.Clamp(transform.position.y + deltaY, yMin, yMax);
        transform.position = new Vector2(newXPos, newYPos);
    }
    void Fire()
    {
        if (Input.GetButtonDown("Fire1"))
        {
            firing = StartCoroutine(FireContinuously());
        }
        if (Input.GetButtonUp("Fire1"))
        {
            StopCoroutine(firing);
            Debug.Log("Paramos");
        }
    }
    IEnumerator FireContinuously()
    {
        while (true)
        {
            GameObject laser = Instantiate(laserPrefab, transform.position, Quaternion.identity) as GameObject;
            laser.GetComponent<Rigidbody2D>().velocity = new Vector2(0, projectileSpeed);
            yield return new WaitForSeconds(projectileFiringPeriod);
        }
    }
    void SetUpMoveBoundaries()
    {
        Camera gameCamera = Camera.main;
        //X axis
        xMin = gameCamera.ViewportToWorldPoint(new Vector3(0, 0, 0)).x + padding;
        xMax = gameCamera.ViewportToWorldPoint(new Vector3(1, 0, 0)).x - padding;
        //Y axis
        yMin = gameCamera.ViewportToWorldPoint(new Vector3(0, 0, 0)).y + padding;
        yMax = gameCamera.ViewportToWorldPoint(new Vector3(0, 1, 0)).y - padding;

    }
}

[Tristan_Hedges]

Glad someone else managed to help you while I slept
Take care!

[TiltedWatcher]

Nice glad it’s working out now