You’re right, there was a “yield return null;” missing. I corrected this.
This would be a compiler error, though. I didn’t actually run this isolated script, which I will do now, to make sure.
EDIT:
I just updated the original post’s code to a full script that can run in the Editor and shows that some code cannot be executed.
@eisenpony
I see! Thanks for your answer! It does make sense, then!
In that case, using flags to check if the coroutine is running will probably be better:
bool someCoroutineIsRunning = false;
IEnumerator someCoroutine;
IEnumerator SomeCoroutine () {
someCoroutineIsRunning = true;
// stuff
someCoroutineIsRunning = false;
}
void SomeCoroutine_Stop () {
StopCoroutine(someCoroutine);
someCoroutineIsRunning = false;
}
IEnumerator Start () {
someCoroutine = SomeCoroutine();
StartCoroutine(someCoroutine);
while (someCoroutineIsRunning) {yield return null;}
// the above two lines instead of: yield return StartCoroutine(someCoroutine);
// code here should be executed
}
EDIT:
There may still be problems with that solution, though. For example, when you try stopping and starting the coroutine within the same frame:
bool someCoroutineIsRunning = false;
IEnumerator someCoroutine;
IEnumerator SomeCoroutine () {
someCoroutineIsRunning = true;
// stuff
someCoroutineIsRunning = false;
}
void SomeCoroutine_Stop () {
StopCoroutine(someCoroutine);
someCoroutineIsRunning = false;
}
IEnumerator Start () {
someCoroutine = SomeCoroutine();
StartCoroutine(someCoroutine);
while (someCoroutineIsRunning) {yield return null;}
// the above two lines instead of: yield return StartCoroutine(someCoroutine);
// code here should be executed
someCoroutine = SomeCoroutine();
StartCoroutine(someCoroutine);
yield return new WaitForSeconds(10.0f);
StartCoroutine(SomeCoroutine_Ended);
SomeCoroutine_Stop();
// stopping and starting within the same frame,
// probably setting someCoroutineIsRunning = true again before SomeCoroutine_Ended checks it
someCoroutine = SomeCoroutine();
StartCoroutine(someCoroutine);
}
IEnumerator SomeCoroutine_Ended () {
while (someCoroutineIsRunning) {yield return null;}
// probably not executed ?
}
I’m not sure what you’re trying to accomplish. Why not just execute the code you want to happen when the coroutine ends directly from Start after your waiting while loop?
Actually, I think it’s better to use the flag to cause the coroutine to complete rather than testing and setting it from all over the place. the only disadvantage is that the operation might not cancel immediately, which I think is more sensible anyways as it gives the coroutine control over any cleanup tasks.
public class SomeSubroutine
{
bool shouldContinue = true;
public event Action<bool> Completed;
IEnumerator Execute
{
while (shouldContinue && SomeOtherCondition())
{
// Some repeating code
yield return null;
}
// Clean up code
}
public IEnumerator Start()
{
yield return StartCoroutine(Execute);
OnComplete(!shouldContinue);
}
public void Cancel()
{
shouldContinue = false;
}
void OnComplete(bool wasCanceled)
{
var completed = Completed;
if (completed != null)
completed(wasCanceled);
}
}
I should add, a lot of this stuff has been thought about before. If you’re just here to learn, I’m happy to continue the discussion but if you prefer to ride on someone’s coat tails, I suggest @lordofduct 's radical coroutine’s. He provides quite a bit of infrastructural code for free.
I’ve been cleaning up SPFramework and moving it to Unity 2017+.
Note some features in it can be consider superfluous these days. Like the custom yield instructions. When I first started writing it, CustomYieldInstruction did not exist yet.
@eisenpony
I don’t really want to use code that I don’t fully understand, because I would like to be able to expand and maintain it. And before reading and understanding a system that is already there, I tend to just develop my own system fitting best to the project’s needs.
The workaround that you suggested may work in some cases, but in most cases that I could think of, it’s not suitable due to not stopping the coroutine immediately. (This may highly depend on the use cases, though.) Also, when you try to stop a coroutine using the flag and start the same coroutine again within one frame, you end up with two coroutines running, since the first one didn’t stop, because the second one sets shouldContinue to true before it yields (which it should, because otherwise the repeating code wouldn’t run?).
I just wrote some code that may be better for illustrating what I am trying to do (didn’t test it, though):
int attackStrength = 1;
void Update () {
if (Input.GetKeyDown(KeyCode.Space)) {
if (!newAttackRunning) {NewAttack();}
}
}
bool newAttackRunning = false;
void NewAttack () {
newAttackRunning = true;
// if (attack != null) {StopCoroutine(attack);}
// < this doesn't work since it stops the coroutine before destroying the vfx
// attackRunning = false;
// yield return null;
// < this sort of needlessly skips a frame
Attack_Stop(); // this may work without sacrifices
attack = Attack(attackStrength);
StartCoroutine(attack);
newAttackRunning = false;
}
void Attack_Stop () {
StopCoroutine(attack);
Destroy(cloned_attackVFX);
// < placing cleanup code between stopping and starting
// outside of Attack
}
public GameObject[] attackVFX;
GameObject cloned_attackVFX;
Transform cloned_attackVFXtransform;
public Transform attackVFXfollow; // its position is animated by other code
IEnumerator attack;
bool attackRunning = false;
IEnumerator Attack (int attackVFX_i) {
attackRunning = true;
cloned_attackVFX = Instantiate(
attackVFX[attackVFX_i],
attackVFXfollow.position,
Quaternion.identity
) as GameObject;
cloned_attackVFXtransform = cloned_attackVFX.GetComponent<Transform>();
while (attackRunning) {
cloned_attackVFXtransform.position = attackVFXfollow.position;
yield return null;
}
Destroy(cloned_attackVFX); // not necessary here when using Attack_Stop
}
So if you want a stateful coroutine. You can wrap it like my RadicalCoroutine with a state value.
Now my RadicalCoroutine has tons of features that we could actually scale back to just this stateful thing.
Like this, a much more understandable base version:
using UnityEngine;
using System.Collections;
public class StatefulCoroutine : IEnumerator
{
public enum OperatingState
{
Cancelled = -2,
Cancelling = -1,
Inactive = 0,
Active = 1,
Complete = 2
}
#region Fields
private IEnumerator _routine;
private Coroutine _token;
private OperatingState _state;
#endregion
#region CONSTRUCTOR
public StatefulCoroutine(IEnumerator routine)
{
if (routine == null) throw new System.ArgumentNullException("routine");
_routine = routine;
}
#endregion
#region Methods
public Coroutine Token
{
get { return _token; }
}
public OperatingState State
{
get { return _state; }
}
#endregion
#region Methods
public void Start(MonoBehaviour handle)
{
_token = handle.StartCoroutine(_routine);
_state = OperatingState.Active;
}
public void Cancel()
{
if(_state == OperatingState.Active)
{
_state = OperatingState.Cancelling;
}
}
#endregion
#region IEnumerator Interface
object IEnumerator.Current
{
get { return _routine != null ? _routine.Current : null; }
}
bool IEnumerator.MoveNext()
{
switch(_state)
{
case OperatingState.Cancelled:
return false;
case OperatingState.Cancelling:
_state = OperatingState.Cancelled;
return false;
case OperatingState.Inactive:
return false;
case OperatingState.Active:
if(_routine.MoveNext())
{
return true;
}
else
{
_state = OperatingState.Complete;
return false;
}
default:
return false;
}
}
void IEnumerator.Reset()
{
_routine.Reset();
}
#endregion
}
/// <summary>
/// Some extension methods for StatefulCoroutine
/// </summary>
public static class CoroutineUtil
{
public static StatefulCoroutine StartStatefulCoroutine(this MonoBehaviour handle, IEnumerator routine)
{
var r = new StatefulCoroutine(routine);
r.Start(handle);
return r;
}
public static StatefulCoroutine StartStatefulCoroutine(this MonoBehaviour handle, IEnumerable routine)
{
var r = new StatefulCoroutine(routine.GetEnumerator());
r.Start(handle);
return r;
}
public static StatefulCoroutine StartStatefulCoroutine(this MonoBehaviour handle, System.Func<IEnumerator> routine)
{
var r = new StatefulCoroutine(routine());
r.Start(handle);
return r;
}
}
Then your example could be rewritten as:
#region Fields
public int attackStrength = 1;
public GameObject[] attackVFX;
public Transform attackVFXfollow; // its position is animated by other code
private StatefulCoroutine _attackRoutine;
private GameObject _clone;
#endregion
#region Properties
public bool AttackRunning
{
get { return _attackRoutine != null; }
}
#endregion
#region Methods
public void CancelAttack()
{
if(_attackRoutine != null)
{
_attackRoutine.Cancel();
_attackRoutine = null;
}
if (_clone != null)
{
Destroy(_clone);
}
}
void Update()
{
if (!this.AttackRunning && Input.GetKeyDown(KeyCode.Space))
{
_attackRoutine = this.StartStatefulCoroutine(this.AttackRoutine(attackStrength));
}
}
private System.Collections.IEnumerator AttackRoutine(int attackVFX_i)
{
_clone = Instantiate(
attackVFX[attackVFX_i],
attackVFXfollow.position,
Quaternion.identity
) as GameObject;
var trans = _clone.GetComponent<Transform>();
while (true)
{
_clone.position = attackVFXfollow.position;
yield return null;
}
Destroy(_clone);
_attackRoutine = null;
}
#endregion
This way when you call cancel, the routine stops immediately. Since it gets flagged as ‘Cancelling’ and the logic of it does not progress the routine any further.
Of course if you cancel from within the routine, it’ll progress to the next yield statement. So always yield break after calling cancel on itself.
