help with movement code

[bartuq]

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

    public void Move(Vector2 input)
    {
        MoveInput = input;// Vector2.ClampMagnitude(input, 1);

        if (MoveInput != Vector3.zero)
        {
            if (IsMoving) return;

            IsMoving = true;
            StartCoroutine(MoveCoroutine());
        }
        else
        {
            IsMoving = false;
        }
    }

    private IEnumerator MoveCoroutine()
    {
        while (IsMoving)
        {
            if (CanMove)
            {
                if (!IsJumping)
                {
                    SetAngle();

                    _interaction.TouchPushableObject();

                    _verticalVelocity += _gravity * Time.deltaTime;

                    _movement.x = MoveInput.x;
                    _movement.y = _verticalVelocity;
                    _movement.z = MoveInput.y;

                    _controller.Move(_movement * (Speed * Time.deltaTime));
                }
            }
            yield return null;
        }
    }

[dlorre]

_verticalVelocity += _gravity * Time.deltaTime;

your vertical velocity never ceases to grow.

[Yoreki]

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?

[ijmmai]

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

[wideeyenow_unity]

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.

[Kurt-Dekker]

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):

A Basic First Person Character Controller for prototyping

https://www.youtube.com/watch?v=t0oN-PFXZEM Gist Link After some interest in an old post here , I spent the weekend frankensteining snippets of code to make a single script FPCC that could be used like the FPCC from Unity 4~5, including the crouch and run from Aldo, and I also added farcry sliding. There is a Menu Item function to easily create and configure a new FPCC object. Otherwise very easy to set up, it only requires a layer to exclude itself from sphere/raycasts. There is a check to …

That one has run, walk, jump, slide, crouch… it’s crazy-nutty!!

[bartuq]

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.

[Kurt-Dekker]

@bartuq

I have other Coroutines for jump etc

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

[wideeyenow_unity]

@bartuq

… to see what values your code is working with" yes, and I don’t see anything strange.

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

[bartuq]

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

_movement = Vector3.zero;
        _verticalVelocity = 0;

after while (IsMoving) maybe this solve the issue.

[wideeyenow_unity]

A trick I do to test issues of this sort:

// A class that always exists with Update()
public static int frameCounter;

void Update()
{
    frameCounter++;
}

// in problem functions
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”

[Yoreki]

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

@bartuq

I have other Coroutines for jump etc.

That’s even worse then.

@bartuq

As far I know using Update for everything is a bad move.

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.

@bartuq

It’s very possible that it’s running 2 times because I can’t see any other explanation why this is happening

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.

[wideeyenow_unity]

@Yoreki

Ok so i dont wanna be that guy …

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…

[Kurt-Dekker]

@wideeyenow_unity

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:

Unity - Manual: Event function execution order

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

---

Coroutines in a nutshell:

Destroying the object then restarting the scene

Hey Epoch, here’s how it goes down. A coroutine is just an IEnumerator object, and you make one by just calling a coroutine. When you do that, nothing happens. It’s like a water pump that needs to have its crank pumped, but nobody is pumping it so it sits there and does nothing. In Unity there’s a special agreement: if you hand that IEnumerator object into StartCoroutine(), which is a method belonging to the object where that StartCoroutine() call is located, what Unity does is starts “pumpin…

Proper way to stop coroutine?

Coroutines are an IEnumerator object. They have their own internal state. It’s different from just calling InvokeRepeating(), which would restart a fresh function call each time. Here is ALL you need to know about coroutines in Unity: they are an IEnumerator, also known in some languages as a Generator. It is an object with a .MoveNext() method on it that “yields” the next thing with each subsequent call. That is what you create by calling an IEnumerator: you create the object, you do NOTHIN…

Splitting up larger tasks in coroutines:

Best Way to Split a Long-Running Function So it's Non-Blocking?

Technically, yeah, there’s always a way. But in practice, in the general sense it can get really hairy. Also, in the particular case of adding lots of GameObjects, I know they’re not linear: last I checked, adding 1000 GameObjects is WAY more than 10x slower than adding 100 Gameobjects, I presume due to inefficiencies in newly-added stuff, not sure/ No way to tell with a closed-source engine like Unity. Trying to tie into system timers and judge how long things have taken so far will get you …

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

Things to check before starting a coroutine?

The main thing to check before calling / making a coroutine is to ask yourself, “Does this even need to be a coroutine?” Generally, if your design requires you to call StopCoroutine(), then don’t make it a coroutine. If you just need something to happen a bit later, this is a more-robust approach: https://gist.github.com/kurtdekker/0da9a9721c15bd3af1d2ced0a367e24e If you’re flipping from one value to another, such as fading in, fading out, changing speeds slowly, etc., coroutines are an AW…

Heartbeat courutine fx

There’s never a need. As @PraetorBlue pointed out in a recent post of his, nobody NEEDS coroutines. They do solve a certain class of problem nicely however, primarily problems of sequencing a set of data or events to happen. Coroutines CAN be interrupted and restarted but doing so exponentially increases your chance of a bug, negating a lot of their inherent benefits. A lot of human behaviors are good coroutines: “Go take a shower.” So that breaks down to: get undressed start the shower …

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

Coroutines IEnumerator not working as expected

I’m kinda suprised that until now nobody has asked a question like this. Even though I have explained what a coroutine actually is several times in other questions it just shows that there is still quite a misunderstanding what a coroutine actually is. Lets just step back for a moment. Unity implements coroutines by utilizing a C# feature called “iterator blocks” or iterator methods (which are more generally known as Generator methods). C# ships with two relevant interfaces (IEnu…

[bartuq]

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