Problem with Ienumerator.

Code (CSharp):

1. using UnityEngine;
2. using System.Collections;
3.  
4. public class Player : MonoBehaviour
5. {
6.     public float MineSpeed = 1;
7.  
8.     public GameObject ItemHit;
9.  
10.     public Item ItemScript;
11.  
12.     public RaycastHit hit;
13.  
14.  
15.     void Start ()
16.     {  
17.  
18.     }
19.     void Update()
20.     {
21.         if (Input.GetMouseButton(0))
22.         {
23.             Ray ray = Camera.main.ScreenPointToRay(Input.mousePosition);
24.             if (Physics.Raycast (ray, out hit, 5))
25.             {
26.                 ItemHit = hit.transform.gameObject;
27.                 ItemScript  = ItemHit.GetComponent<Item>();
28.  
29.                 StartCoroutine(Mine());
30.             }
31.         }
32.     }
33.  
34.     IEnumerator Mine()
35.     {
36.         yield return new WaitForSeconds (ItemScript.ItemMiningSpeed * MineSpeed);
37.  
38.         if (Input.GetMouseButton (0))
39.         {
40.             Destroy(ItemHit);
41.             StopCoroutine(Mine());
42.         }
43.     }
44. }
45.  

This code is ment to mine the block the player looks at while holding mouse1 down.
It all works well but after the first block is mined if the player holds mouse1 down every block the player looks at gets instantly destroyed.

 

What you are doing here is starting a coroutine every frame that will destroy the object stored in a global variable, which also gets updated every frame, independently of the delay in your coroutine. Once the delay of the first coroutine that was started is over, every next frame the next coroutine you started will finish, resulting in blocks instantly being destroyed.

What you should do instead is define a sort of "health" property on your block class, and have that decrease every frame you are clicking on it. When it reaches 0, you destroy the block - no coroutines involved, just Update() stuff.

Also, as general advice, you shouldn't start multiple threads for the same question.

 

Just create a bool "canMine" that becomes false when the CoRoutine starts, but becomes true when the Mouse is released.

Then only allow the CoRoutine to start when "canMine" is true.

This only allows the IEnumerator to be handled once per click.

 

But if you click repeatedly, you will still start multiple coroutines while the others are running. If you then hold down the mouse while the coroutines' delays are passing, you will still mine any block that you look at instantly.

You could then stop a running coroutine when you release the mouse, but this is just not a good way of working. You can't easily add any overlays on the block to show how close the block is to breaking, for instance, or mine it with multiple people. Going with an object based approach results in far cleaner code that is more versatile.