Question help with movement code

My character controller sometimes speed up. I haven't noticed this problem so far if I put everything in the Update. Any idea why my private IEnumerator MoveCoroutine() speed up?

InputActions:

Action Properties
Action Type: Value
Control Type: Vector 2

Binding Properties
Composite Type: 2D Vector
Mode: Digital Normalized

Code (CSharp):

1.     public void Move(Vector2 input)
2.     {
3.         MoveInput = input;// Vector2.ClampMagnitude(input, 1);
4.  
5.         if (MoveInput != Vector3.zero)
6.         {
7.             if (IsMoving) return;
8.  
9.             IsMoving = true;
10.             StartCoroutine(MoveCoroutine());
11.         }
12.         else
13.         {
14.             IsMoving = false;
15.         }
16.     }

Code (CSharp):

1.     private IEnumerator MoveCoroutine()
2.     {
3.         while (IsMoving)
4.         {
5.             if (CanMove)
6.             {
7.                 if (!IsJumping)
8.                 {
9.                     SetAngle();
10.  
11.                     _interaction.TouchPushableObject();
12.  
13.                     _verticalVelocity += _gravity * Time.deltaTime;
14.  
15.                     _movement.x = MoveInput.x;
16.                     _movement.y = _verticalVelocity;
17.                     _movement.z = MoveInput.y;
18.  
19.                     _controller.Move(_movement * (Speed * Time.deltaTime));
20.                 }
21.             }
22.             yield return null;
23.         }
24.     }

 

Code (csharp):

1.  
2. _verticalVelocity += _gravity * Time.deltaTime;
3.  

your vertical velocity never ceases to grow.

 

Based on the code sniplets posted i dont see why your movement as a whole would speed up. You are multiplying gravity with Time.deltaTime twice tho. Why use Coroutines in this case anyways? Coroutines are.. horrible, really. In 9/10 cases they are detrimental, as they dont offer any major advantages and quickly deteriorate code readability. Get rid of IsMoving, get rid of the Coroutine. You already know how to solve your problem. What speaks against this solution?

 

did you try to set a debug/print for 'input'? to see what values your code is working with

 

The problem with coroutines is many of them can run within the same frame, since they do a whole separate function. So obviously the move script is running twice, or more, each frame.

Like Yoreki states, I personally hate coroutines. I much rather know what is running when, and on what frame. But I've seen this issue before, and found the result was a coroutine created many "timed" instances(was user error, to be fair). Either way, I don't like them.

 

Toss this code. It starts a coroutine every frame. Besides a movement controller never needs a coroutine.

If it is a FPS controller...

If you would prefer something more full-featured here is a super-basic starter prototype FPS based on Character Controller (BasicFPCC):

https://forum.unity.com/threads/a-basic-first-person-character-controller-for-prototyping.1169491/

That one has run, walk, jump, slide, crouch... it's crazy-nutty!!

 

I have other Coroutines for jump etc. in a special item script. It's a Zelda style game. IsMoving is also used to test if the character is moving for other actions. As far I know using Update for everything is a bad move. Coroutine is more elegant way. In the situation when I have add/activate more items or actions and attach them as separate scripts I prefer coroutines.

"did you try to set a debug/print for 'input'? to see what values your code is working with" yes, and I don't see anything strange.

 

They are still inappropriate and will cost you technical debt into the future.

 

And you won't see your speed any different than what it was set. What you're not seeing is the same coroutine is running twice or more each frame. So true, move speed will still be 2, but if you say "move 2, move 2, move 2" in one frame, you'll move by 6. ergo, moving faster

 

It's very possible that it's running 2 times because I can't see any other explanation why this is happening. I have (IsMoving) return; but maybe _controller.Move(_movement * (Speed * Time.deltaTime)); is not ended immediately after IsMoving is set to false.

I will try

Code (CSharp):

1. _movement = Vector3.zero;
2.         _verticalVelocity = 0;

after while (IsMoving) maybe this solve the issue.

 

A trick I do to test issues of this sort:

Code (CSharp):

1. // A class that always exists with Update()
2. public static int frameCounter;
3.  
4. void Update()
5. {
6.     frameCounter++;
7. }
8.  
9. // in problem functions
10. print($"Player moved {moveSpeed} at frame {class.frameCounter}");

Which I'm sure is archaic, and I'm soon to receive hate mail over it, lol..

but you'll easily see printed:
"Player moved 1.5 at frame 36"
"Player moved 1.5 at frame 36"

 

Ok so i dont wanna be that guy, and there is a touch of subjective opinion in this, as always. But..

That's even worse then.

Why do you think that? Coroutines are worse in pretty much every regard, be it readability, maintainability, debugging, probably even performance (the latter being a non-issue tho). The only argument for Coroutines is if something doesnt need to run every frame. However for easy cases you may aswell implement your own timer mechanic running in Update, and for complex cases i would prefer Update over Coroutines any day, for debugging reasons.

I dont intend this to sound rude, but isnt you needing help debugging this rather simple scenario evidence enough for how Coroutines are harder to reason about? And this is a simple case. Imagine doing something more complex, where you keep the object and start or stop it, while potentially also having it manipulated / created through different sources. Coroutines are fine for odd jobs here and there, or for prototyping. Building your game architecture around them is asking for spaghetti code.

If you insist on using Coroutines i would suggest making sure that only one per task can ever exist. You will still have to deal with an annoying overhead to make sure you handle this Coroutine object properly, ie you need to detect if it still runs, not create a new one if it does, but recreate it if it still exists but stopped. Im not even sure if this description is entirely correct tho, as i simply dont bother with Coroutines. Why anybody would ever want to work that way is beyond me, when Update exists.

I asked this right at the beginning.. but why Coroutines? I realise you like them. But why? What advantage do you see with them that makes you insist on using them over Update? Maybe it's just a misconception that drives you to work this way, in which case im sure we can clear that up.

 

I truly lost it here.. that was a good laugh

As much as I hate coroutines, they do have some good places. Like particle effects, or even death animations(if object pooling, and want returned quickly to the pool), and I'm sure there's at least one other thing... maybe..

 

Well IEnumerators are of COURSE useful. They're a key language feature.

Coroutines can be useful, but I argue against their uses most of the time.

Here is my criteria for a coroutine:

- the code truly affects nothing else in a material way

- the code is tolerant of null references if it drives something external

- you never intend to stop it by any means except Destroy()-ing the underlying GameObject

And while we're at it, to dispel some of the silly beliefs about Update vs Coroutines:

https://docs.unity3d.com/Manual/ExecutionOrder.html

Hint: Update and Coroutines run in the same thread, one after the other, never at the same time.

------------------

Coroutines in a nutshell:

https://forum.unity.com/threads/des...en-restarting-the-scene.1044247/#post-6758470

https://forum.unity.com/threads/proper-way-to-stop-coroutine.704210/#post-4707821

Splitting up larger tasks in coroutines:

https://forum.unity.com/threads/bes...ion-so-its-non-blocking.1108901/#post-7135529

Coroutines are NOT always an appropriate solution: know when to use them!

"Why not simply stop having so many coroutines ffs." - orionsyndrome on Unity3D forums

https://forum.unity.com/threads/things-to-check-before-starting-a-coroutine.1177055/#post-7538972

https://forum.unity.com/threads/heartbeat-courutine-fx.1146062/#post-7358312

Our very own Bunny83 has also provided a Coroutine Crash Course:

https://discussions.unity.com/t/coroutines-ienumerator-not-working-as-expected/237287/3

 

I decided to rewrite my movement to Update but with state machines.