10

In Unity3D, this is my code:

void ActivateBuff1(){
    gun.equippedGun.msPerShot /= 2;
    gun.equippedGun.shotsLeftInMag += 10;
    StartCoroutine (WaitRage ());
}

void ActivateBuff2(){
    player.speedModifier *= 1.5f;
    StartCoroutine (WaitSpeed ());
}

IEnumerator WaitRage(){
    yield return new WaitForSeconds(powerUpDuration);
    gun.equippedGun.msPerShot *= 2;
}

IEnumerator WaitSpeed(){
    yield return new WaitForSeconds(powerUpDuration);
    player.speedModifier /= 1.5f;
}

Everytime a player run into a power up one of the ActivateBuff Methods gets called. Obviously powerUps effects don't last forever though so I used IEnumerators to reverse the effects of my original method after the wait for a certain number of seconds. For some reason though, the code within the IEnumerators never gets called. Please help...(and please suggest an alternative way of coding this perhaps as I know it isn't very clean)

r0128
  • 461
  • 1
  • 4
  • 10
  • 1
    Have you determined through logging/debugging that all the code leading up to starting the coroutine is being called? And logging before `WaitForSeconds` to determine if the coroutines are even being entered? – Serlite Nov 24 '16 at 18:18
  • I imagine your powerUpDuration is set somewhere else in your code? And you have spotted the ActivateBuff1 and ActivateBuff2 being called in your debugger? – Zephire Nov 24 '16 at 18:19
  • Could you add comments within your code to enlighten which sections are being hit and which aren't and which should? I'm having a little trouble figuring that out, I'm a little tired lol.. – Alox Nov 24 '16 at 18:23
  • yeah everything is getting called Serlite except the code in the IENnumerator does not seem to be running... – r0128 Nov 24 '16 at 18:26
  • Zephire yes, both ActivateBuff1 and ActivateBuff2 are being called its just the IEnumerators – r0128 Nov 24 '16 at 18:27
  • yes the IEnumerators are also being called and work its just the waitforseconds and the code after that which does not get called – r0128 Nov 24 '16 at 18:30
  • Interesting - have you tried logging out powerUpDuration inside the coroutines, to make sure you're getting an expected value? And where are these methods attached? If they're on a Gameobject that subsequently becomes disabled after the player runs over them, the coroutine will be cancelled. – Serlite Nov 24 '16 at 18:33
  • Serlite, yes!!! its because the gameObject get destroyed straight after.. thanks! – r0128 Nov 24 '16 at 18:49
  • Serlite would you have any suggestion to optimize my code though – r0128 Nov 24 '16 at 18:49
  • 1
    Hmm...that's a different sort of story - and it's pretty opinionated. For strictly more simple code, you could use [`Invoke()`](https://docs.unity3d.com/ScriptReference/MonoBehaviour.Invoke.html) instead to delay resetting the player attributes, so the relationship between `powerUpDuration` and the reset method is more clear. Saves you a couple lines of code too. – Serlite Nov 24 '16 at 18:52
  • @r0128 I've added an answer that expands on all these things I've talked about in the comments - let me know if anything is unclear. – Serlite Nov 24 '16 at 19:08
  • You **must not** use IEnumerator or coroutines. Just use "Invoke" for timers in Unity3D. – Fattie Nov 25 '16 at 11:00

3 Answers3

12

Under normal circumstances, the code you've supplied should work fine. However, as determined in the comments, there's a caveat - if the Gameobject calling the coroutine is disabled/destroyed before the delay from WaitForSeconds() has completed, the coroutine will be stopped and the remaining code won't be called at all. You will either need to wait for the coroutine to finish before destroying the Gameobject, or have some other Gameobject call the coroutine.

You mentioned that you were also looking for alternatives that might simplify your code - you might consider Invoke(), which lets you call a method after a specified delay. (As long as you're not triggering this very often, the overhead from reflection won't have an appreciable effect on your performance.) So your code could be rewritten to be somewhat shorter:

void ActivateBuff1(){
    gun.equippedGun.msPerShot /= 2;
    gun.equippedGun.shotsLeftInMag += 10;
    Invoke("ResetPlayerRage", powerUpDuration);
}

void ActivateBuff2(){
    player.speedModifier *= 1.5f;
    Invoke("ResetPlayerSpeed", powerUpDuration);
}

void ResetPlayerRage(){
    gun.equippedGun.msPerShot *= 2;
}

void ResetPlayerSpeed(){
    player.speedModifier /= 1.5f;
}

Unfortunately, Invoke() will also be cancelled if the Gameobject is destroyed - but unlike a coroutine, it won't be cancelled if the Gameobject is disabled. So you could disable the Gameobject first (so it becomes invisible and doesn't interact with anything), then destroy it only after running the delayed method:

void ActivateBuff1(){
    gun.equippedGun.msPerShot /= 2;
    gun.equippedGun.shotsLeftInMag += 10;
    gameObject.SetActive(false);
    Invoke("ResetPlayerRage", powerUpDuration);
}

void ResetPlayerRage(){
    gun.equippedGun.msPerShot *= 2;
    Destroy(gameObject);
}

Here's a summary of whether Invoke() and coroutines will be stopped depending on how you manipulate the script component or entire Gameobject:

..........................................................................
:                                  :                     :               :
:          Does it stop?           :   InvokeRepeating   :   Coroutine   :
:                                  :                     :               :
:..................................:.....................:...............:
:                                  :                     :               :
:   Disable the script component   :         No          :      No       :
:                                  :                     :               :
:..................................:.....................:...............:
:                                  :                     :               :
:   Destroy the script component   :         Yes         :      Yes      :
:                                  :                     :               :
:..................................:.....................:...............:
:                                  :                     :               :
:   Disable the game object        :         No          :      Yes      :
:                                  :                     :               :
:..................................:.....................:...............:
:                                  :                     :               :
:   Destroy the game object        :         Yes         :      Yes      :
:                                  :                     :               :
:..................................:.....................:...............:
Serlite
  • 12,130
  • 5
  • 38
  • 49
  • I find that the single hardest thing to remember in Unity!!! It's like a frickin' 3D table of stuff you have to remember. – Fattie Nov 25 '16 at 11:09
  • I took the liberty of starting of an ascii-art-table on your question, to finally address this issue! Maybe you know the values. – Fattie Nov 25 '16 at 11:27
  • Thanks, @JoeBlow! I filled in most of it with what I know, but I'll need to set up a test case later to verify the last two entries - I suspect they're both "Yes", but it's better to be sure! – Serlite Nov 25 '16 at 14:50
  • 2
    We've pretty much set the bar for ascii art on the site @serlite dude! :) – Fattie Dec 02 '16 at 11:35
  • Heh, a great honour indeed @JoeBlow. Thanks for your help with the answer! – Serlite Dec 02 '16 at 17:08
  • I like this table – Programmer Dec 09 '16 at 00:18
3

Man, Serlite said everything.. However I would have dealt with this kind of situation differently.

If I got it right, you setted this ActivateBuff function in the script attached to some kind of item that sets a modifier in the equipped gun and afterwards gets disabled. Instead of doing that, I would just create a Buff function in the equipped gun script(passing the modifier and the time as params) and let the gun itself to handle this.

Since the equipped gun won't disappear, it would solve your problem and ideally could even be a more generic solution. Once you could pass through another power up which could lately generate some unexpected behavior(because there would be many scripts buffing and unbuffing the gun).

Rafael Costa
  • 1,291
  • 2
  • 13
  • 30
  • 1
    So true - you do it on the "other" object. There was a QA about this yesterday... http://stackoverflow.com/questions/40912214 – Fattie Dec 02 '16 at 11:35
1

Take a look at my approach. It is using FixedUpdate method to handle timings and does not require Coroutines. Also I used Singleton Pattern in my buffers to enable easy access.

I have a BufferBase script where I handle start/end of buffers. I can have as many buffers as I like and derive them from this class.

BufferBase has

  • two members: _isBufferActive and _bufferRemainingTime.

  • A method named FixedUpdateBuffer which I must call in FixedUpdate of my buffers. it is responsible for timing of the buffer and calling EndBuffer() when the time is over.

  • And 3 virtual methods which I can override in my buffer classes.

Here is the code:

public class BufferBase : MonoBehaviour
{
    /// <summary>
    /// Indicates whether the buffer is activated
    /// </summary>
    protected bool _isBufferActive = false;
    /// <summary>
    /// Time until buffer ends
    /// </summary>
    protected float _bufferRemainingTime = 0f;


    protected void FixedUpdateBuffer()
    {
        if (_isBufferActive)
        {
            _bufferRemainingTime -= Time.fixedDeltaTime;
            if (_bufferRemainingTime <= 0)
            {
                EndBuffer();
            }
        }
    }

    /// <summary>
    /// Resets buffer
    /// </summary>
    protected void ResetBuffer()
    {
        _isBufferActive = false;
        _bufferRemainingTime = 0;
    }

    /// <summary>
    /// Marks the start of the buffer
    /// </summary>
    /// <param name="value"></param>
    protected virtual void StartOrExtendBuffer(float value)
    {
        //set buffer values
        _isBufferActive = true;
        _bufferRemainingTime = value;

        gameObject.SetActive(true);
    }

    /// <summary>
    /// Marks the end of buffer
    /// </summary>
    protected virtual void EndBuffer()
    {
        _bufferRemainingTime = 0;
        _isBufferActive = false;

        gameObject.SetActive(false);
    }
}

Now for the actual buffer. I have several scripts derived from BufferBase. All of them have those virtual methods implemented in them.

I can easily:

  1. check whether a specific type of buffer is active or not through RageController.IsActive

  2. activate a buffer using RageController.AddRage(t) where t specifies the duration. (its duration will be reset to t each time AddRage is called)

  3. turn a buffer off using RageController.Reset()

here is a sample buffer script:

public class RageController : BufferBase
{
    public static RageController instance;

    public static bool IsActive { get { return instance._isBufferActive; } }

    #region Static Methods
    internal static void AddRage(float value)
    {
        instance.StartOrExtendBuffer(value);
    }

    internal static void Reset()
    {
        instance.ResetBuffer();
    }
    #endregion

    #region Overriden Methods
    protected override void StartOrExtendBuffer(float value)
    {
        base.StartOrExtendBuffer(value);

        //----
        //add speed etc..
        //----
    }

    protected override void EndBuffer()
    {
        base.EndBuffer();

        //----
        //remove speed etc..
        //----
    }
    #endregion   

    #region Unity Methods
    void Awake()
    {
        instance = this;
    }
    void FixedUpdate()
    {
        FixedUpdateBuffer();

        if (_isBufferActive)
        {
            //----
            //anything that changes by time
            //----
        }
    }
    #endregion
}

Notice that at the end of RageController in FixedUpdate method you can smoothly change your desired values or enable the buffer with a delay or so by reading the value of _bufferRemainingTime

Bizhan
  • 16,157
  • 9
  • 63
  • 101