WaitForSeconds firing immediately

Question

I'm trying to set up a dash ability that updates the player velocity, waits a set amount of time (dashTime) and then sets the canDash to false for another set amount of time (dashCooldown).

However the waitForSeconds don't seem to be working as the canDash bool only remains false for a split second before switching back to true.

PlayerAbilities.cs

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class PlayerAbilities : MonoBehaviour
{
    // Scripts & Data
    [SerializeField] PlayerController s_PC;
    internal PlayerData pd = new PlayerData();

    private void Awake() {
        pd.canDash = true;
    }

    // Dash
    internal IEnumerator Dash()
    {
        pd.CanDash = false;
        Debug.Log("dash, dash baby");
        // Perform dash and update canDash
        s_PC.rb2d.velocity = s_PC.s_PlayerMovement.moveInput * pd.dashSpeed;
        yield return new WaitForSeconds(pd.dashTime); // Wait set time for dash to happen
        s_PC.UpdateState(PlayerController.State.Normal); // Change state back to normal

        // Cooldown dash
        yield return new WaitForSeconds(pd.dashCooldown); // Cooldown dash

        pd.CanDash = true;
    }
}

PlayerController.cs

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class PlayerController : MonoBehaviour
{
    // Scripts & Data
    [SerializeField] internal PlayerAbilities s_PlayerAbilities;
    [SerializeField] internal PlayerInput s_PlayerInput;
    internal PlayerData pd = new PlayerData();

    // Update is called once per frame
    private void FixedUpdate()
    {
        // Check if user has control of player
        if (pd.canControlPlayer)
        {
            // Update player state
            switch (playerState)
            {
                case State.Normal:
                    s_PlayerMovement.Move();
                    break;
                case State.Dash:
                    StartCoroutine(s_PlayerAbilities.Dash());
                    break;
                case State.DodgeRoll:
                    StartCoroutine(s_PlayerAbilities.DodgeRoll());
                    break;
            }
        }
    }
}

PlayerData.cs

using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using System;

[Serializable]
public class PlayerData
{
    // Dash
    internal float dashSpeed = 50f;
    internal float dashTime = 0.2f;
    internal float dashCooldown = 5f;
    internal bool canDash = true;
    
    internal bool CanDash {
        get { return canDash; }
        set { canDash = value; }
    }
}

PlayerInput.cs

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class PlayerInput : MonoBehaviour
{
    // Scripts & Data
    [SerializeField] PlayerController s_PC;
    internal PlayerData pd = new PlayerData();

    // Input Actions
    internal PlayerInputActions playerInputActions;

    private void Awake() {
        // Define new player input
        playerInputActions = new PlayerInputActions();
    }

    private void Update()
    {
        // Check if player state is normal
        if (s_PC.playerState == PlayerController.State.Normal)
        {
            // Check for button presses
            if (playerInputActions.Movement.DodgeRoll.triggered)
                s_PC.UpdateState(PlayerController.State.DodgeRoll);
        }

    }
}

What is `pd.dashCooldown` set to and how do you run this Coroutine? Also I would move `pd.CanDash = false;` to the top of the coroutine .. otherwise you can still start multiple routines until the `pd.dashTime` has passed ... Which I suspect is the source of your issue here — derHugo, May 10 '21 at 15:29
`pd.dashCooldown` is `5f`. The coroutine is called from `FixedUpdate` in the playerController using a switch statement. When the playerState case is `dash` then it fires this. I'll add it into the initial question. also, moving the `pd.canDash = false` to the start of the coroutine didn't fix the issue — MarkHughes88, May 10 '21 at 15:40
As a learner, I would truly encourage you to **simply use Invoke for simple timers in Unity.** `Invoke` is dead easy to use, read the manual. — Fattie, May 10 '21 at 16:28
just FYI @MarkHughes88 my guess is you can totally delete all the "[SerializeField]" and "internal" — Fattie, May 10 '21 at 16:28
is it necessary to reproduce the problem to include so many fields in `playerData`? Can't fields like `dashSpeed` `dashTime` `dashCooldown` and others simply be removed and then literals used in their place such as `yield return new WaitForSeconds(5f);`? Please see [mre], and I must emphasize the **minimal** part. A good way to do this is to start over with a blank project and only include what is necessary to produce the problem. — Ruzihm, May 10 '21 at 16:32
Longshot, but something worth checking as it tripped me up couple of times too is: you have serialised the field `dashCooldown` and if you changed it's value in the inspector (to 0 for example for testing purposes) it is going to use that value and ignore the default value of `5f`. — rokgin, May 10 '21 at 17:52
@Fattie I think most of these values are assigned via the Inspector and accessed from other scripts so yeah `[SerializeField]` is probably redundant but why would you remove all the `internal` ? — derHugo, May 10 '21 at 18:39
Cheers DH, my guess is our OP is not really aware of what it does, there's no need for it, and OP has just spammed "[SerializeField] internal" everywhere (perhaps having originally gotten it in a random code sample you know?) As U probably know, some people believe "internal" stinks a bit ( eg https://www.freecodecamp.org/news/is-the-c-internal-keyword-a-code-smell/ ), for me here the issue is not that sophisticated, I think it's just junk that can go :) — Fattie, May 10 '21 at 18:45
ah, sorry @rokgin - you already found the likely problem. Great call. I mentioned it in my answer: https://stackoverflow.com/a/35166004/294884 One of the Famous Unity Gotchyas. — Fattie, May 10 '21 at 18:54
thanks guys. new to c#. i have removed the [serializedfield] from the variables in the dataclass, but a lot of these variables/functions require either internal or public applied to them in order to be accessible from other scripts. im guessing this is because they default as private? removing them creates permission errors. and in terms of storing things like `dashSpeed` etc as variables, i guess i do this to keep them all in one place, which makes tweaking them easier IMO. as a JS react dev this just makes sense to me to do that. Im unfamiliar with invoke, so shall look into that — MarkHughes88, May 11 '21 at 09:01
side question: in terms of the `PlayerData` class i have set up that isn't a `MonoBehaviour` (as i dont have it attached to the player game object, which means i cant actually see the fields in the inspector so can't edit them there - so in those terms that gotcha you mentioned @fattie - isnt a problem), is it best to make this `MonoBehaviour` and use `GetComponent` to reference it? My plan was to store all player data in there, everything from health, to ability data, to unlocked weapons etc. My only concern being what happens on player death/scene change where the GO is nolonger in the scene — MarkHughes88, May 11 '21 at 09:41
hi @MarkHughes88 you've stumbled on a basic issue in Unity! since singletons are meaningless in Unity (there's no concept of a game object "singleton") you simply have a "preload scene" where you keep any "singleton-like general systems which are MonoBehaviors". Here's my world-famous article on the issue :) https://stackoverflow.com/a/35891919/294884 Cheers once you do that everything is trivial — Fattie, May 11 '21 at 11:22
great thanks. so I should load all my data into _app on a preload screen. So i would attach my `PlayerData` onto that, which would mean i could use `[SerializeField]` to allow me to edit them in the inspector. And i could use that scene to load save data etc? — MarkHughes88, May 11 '21 at 12:35
hi @MarkHughes88 ! simply *omit* SerializeField you never have to use it, that is the default! an inspector variable. to your other questions yes, yes, yes. that is absolutely precisely where you could have a "general" monobehavior (perhaps "DataHandling") which does saving, json etc. (as you obviously know that is then conveniently abstracted, you could completely change how that is done in the future for whatever reason, example easily add a few lines oif code and use Firebase or the like to store on the cloud, etc etc). you got it — Fattie, May 11 '21 at 15:23

score -1 · Answer 1 · answered May 10 '21 at 18:53

The code is so unnecessarily complicated, you're just making one of a zillion simple mistakes somewhere

Follow normal debugging.

To begin with change this

yield return new WaitForSeconds(pd.dashCooldown);

to this

Debug.Log("test with number");
yield return new WaitForSeconds(20f);
Debug.Log("tested with number");

and go from there.

BTW there's something completely wrong at FixedUpdate(). For some reason you're call Dash (and others) every frame. That seems totally wrong. You'd call that sort of thing once on key down.

ALSO:

You've probably suffered this Gotchya:

https://stackoverflow.com/a/35166004/294884

due to all the bizarre variables.

WaitForSeconds firing immediately

1 Answers1

ALSO: