Queuing Input Behavior Running a Dequeue twice when multiple keys are pressed during the same frame

Here's some short code snippets of the methods I'm running.

Code (csharp):

1.  
2.     public int Characterlocation;
3.     public bool Inputvalid;
4.     public bool Isplayercontrolled;
5.     public Movement movement;
6.     public bool coroutineAllowed = true;
7.     public GameObject Middle;
8.     private Queue Inputqueue = new Queue();
9.  
10. void Update()
11.     {
12.         QueuedInputs();
13.     }
14.     void FixedUpdate()
15.     {
16.         Queuedmovementapply();
17.     }
18.     IEnumerator Queuedmovementapply()
19.     {
20.         if (Inputqueue.Count == 0 || !coroutineAllowed)
21.         {
22.             yield break;
23.         }
24.         else if (Inputqueue.Count != 0 && coroutineAllowed)
25.         {
26.                 StopCoroutine(Queuedmovementapply());
27.                 Inputqueue.Dequeue();
28.         }
29.     }
30.     void QueuedInputs()
31.     {
32.         if (Isplayercontrolled)
33.         {
34.             if (Input.GetKeyDown(KeyCode.A))
35.             {
36.                 Debug.Log("Queuing A");
37.                 Inputqueue.Enqueue(StartCoroutine(movement.MovementV2(Characterlocation, "Left", gameObject, Middle)));
38.             }
39.             if (Input.GetKeyDown(KeyCode.D))
40.             {
41.                 Debug.Log("Queuing D");
42.                 Inputqueue.Enqueue(StartCoroutine(movement.MovementV2(Characterlocation, "Right", gameObject, Middle)));
43.             }
44.             if (Input.GetKeyDown(KeyCode.S))
45.             {
46.                 Debug.Log("Queuing S");
47.                 Inputqueue.Enqueue(StartCoroutine(movement.MovementV2(Characterlocation, "Down", gameObject, Middle)));
48.             }
49.             if (Input.GetKeyDown(KeyCode.W))
50.             {
51.                 Debug.Log("Queuing W");
52.                 Inputqueue.Enqueue(StartCoroutine(movement.MovementV2(Characterlocation, "Up", gameObject, Middle)));
53.             }
54.         }
55.     }
56.  

The expected Behavior is that When the player inputs multiple keys at the same time it will all be inputted into a queue then Dequeue one by one once each Coroutine is done running.

The resulting Behavior that I actually get from this script is that if multiple keys are pressed at the same time the if condition for checking if a Coroutine is running fails and Dequeues before the first Coroutine finishes.

I haven't been able to wrap my head on why this is happening.

Note: I've referenced an exterior script that will falsify coroutineAllowed and that will take in parameters I've set to effect the player. If interested in seeing the whole file I've uploaded it.

 

Please use code tags, and make sure you've included enough information to tell the data types of the key variables.

 

Thank you Will do.

 

I suspect you are misunderstanding several things about what's going on.

First of all, all of your movement coroutines are running immediately, in parallel, starting the same frame that you add them to the queue. Simply putting them inside of a queue doesn't pause or delay them; you're merely tracking references to them. It's like if I gave you my business card, and then you put that card in your wallet, and later took it out of your wallet. Taking the card in and out of your wallet won't do anything to me; in order to influence my behavior, you'd have to actually call the number on the card and talk to me.

Those coroutines start running when you call StartCoroutine (even if the result of that call is passed as an argument to some other function).

Secondly, you don't have anything in there that checks when a coroutine is finished running. Queuedmovementapply() checks whether your queue is empty or non-empty, and whether a particular boolean variable is true or false. Nothing in the posted code would cause either of those to be correlated with whether the coroutines are finished.

Thirdly, Queuedmovementapply() isn't actually affecting those movement coroutines in any way. Removing them from the queue doesn't do anything to them, just like adding them to the queue doesn't do anything to them--you're taking the business card out of your wallet and throwing it away, but that still doesn't affect the person whose name is written on it.

The call to StopCoroutine on line 26 isn't actually stopping any coroutines, because the argument you are passing to it isn't an active coroutine. You'd need to pass it the exact same IEnumerator used to start the coroutine (which you haven't saved anywhere), or else the object returned by the StartCoroutine call (which is what you've stored in the Queue, so you could stop it with that...although you'd need to cast it first, since you are using a non-generic queue).

(The Queuedmovementapply() function is also pretty overcomplicated. I can't see any reason for it to be a coroutine, since it never waits for anything, and the "yield break" line does nothing, since all the other code is in an "else", and the "else if" condition is precisely the inverse of the "if" condition, so you are doing extra work to get exactly the same result as if you'd just written "else".)


If you want the coroutines to happen one at a time, you could put the IEnumerators into your queue (without calling StartCoroutine on them), and then later call StartCoroutine when you remove them one at a time. (You'd still need to have some way of deciding when it's time to start running the next one, which isn't visible in the code you've posted.)

But in order to call StartCoroutine or StopCoroutine on objects you've removed from the queue, you'd need to cast them, or else switch to using a Queue<IEnumerator> instead of just a Queue. Because everything you put in a non-generic Queue just gets treated as an "object" by default.

 

Thank you so much for explaining the misconceptions I had. I'll take another look at my code and once I've successfully fixed it I'll post it here for people in the future to reference.

Cheers Antistone

 

Building on this, I don't think you need such a complicated coroutine structure.

Instead, every Update();

Check for a user intention (e.g, a key pressed)

- When the user intends to do something, set up a "movement event" that has the necessary info, such as:

- "Player should move from here to there"

- Put that information into a class / structure of things to execute. (the "movement event")

- Put that class / structure into a queue

Check if there is a "movement event" still happening and do it until it is done.

When the current movement event is done, it's done.

If there is no "movement event" happening, see if there is another "movement event" in the queue, and start it

That way you only have one "movement event" happening at a time, and yet you can stack as many as you like in the queue.

So it would be:

Code (csharp):

1. void Update()
2. {
3.   CollectUserIntentAndEnqueueStuff();
4.  
5.   if (ProcessingPendingEvent()) // returns true as long as still moving
6.   {
7.      return;
8.   }
9.  
10.   If (ThereArePendingEvents())
11.   {
12.      DequeueTheNextOneForProcessing();
13.   }
14. }