Cooldown node not being applied

I am trying to combine a “Cooldown” node and a “Repeat While” node in order to limit execution to once per second. Here’s how my graph looks:

Is there something I’m doing wrong? The log message is still being shown multiple times per second

Nice you noticed, its actually a Bug
What the Code does: Wait for its child, return Success if the Child returns success
CooldownModifier.cs (1.6 KB)

What the Code should be doing: Wait for its child to either succeed or fail, start the timer, wait for the timer, return success.
If fixed it, if you wanna use it, just download it and drag it into your project :wink:
FixedCooldownModifier.cs (2.1 KB)

You can use it as expected:

Hi @Hyperion90

One thing to note is that you have a repeat under the Cooldown, so the cooldown node won’t be taking into effect, it needs to be under the repeat for it to work.

@Aroboss ,

Thank you! I’ll add this fix :slight_smile:

Thank you! @ShaneeNishry is there any chance it would get included into 1.0.4? Also, a warning when adding a “Repeat” node as a child to a “Cooldown” node I think would be very helpful.

I’m trying to get it in right now! :slight_smile:

My fix is slightly different to @Aroboss , to prevent it from running and blocking execution of other nodes. I still need to test it and put it to review:

using System;
using UnityEngine;
using Unity.Behavior;
using Unity.Properties;
using Modifier = Unity.Behavior.Modifier;

namespace Unity.Behavior
{
    [Serializable, GeneratePropertyBag]
    [NodeDescription(
        name: "Cooldown",
        description: "Imposes a mandatory wait time between executions to regulate action frequency.",
        story: "Cooldowns for [duration] seconds after execution",
        category: "Flow",
        id: "a9ff45a058927aa68b4328a5daf34161")]
    internal partial class CooldownModifier : Modifier
    {
        [SerializeReference] public BlackboardVariable<float> Duration;
        [CreateProperty] private float m_CooldownRemainingTime;
        private float m_CooldownEndTime;

        protected override Status OnStart()
        {
            if (Time.time < m_CooldownEndTime)
            {
                return Status.Failure;
            }

            m_CooldownEndTime = Time.time + Duration.Value;

            if (Child == null)
            {
                return Status.Success;
            }

            var status = StartNode(Child);
            if (status == Status.Running)
            {
                return Status.Waiting;
            }
            return status;
        }

        protected override Status OnUpdate()
        {
            var status = Child.CurrentStatus;
            if (status == Status.Running)
            {
                return Status.Waiting;
            }
            return status;
        }

        protected override void OnSerialize()
        {
            m_CooldownRemainingTime = m_CooldownEndTime - Time.time;
        }

        protected override void OnDeserialize()
        {
            m_CooldownEndTime = Time.time + m_CooldownRemainingTime;
        }
    }
}

Ah I thought the to be expected behavior was to wait for the child to return completion (success or failure), then start the countdown, after countdown is finished, return success.

Good that we can access the child directly, forgot that, so in update it doesnt ask for child status haha.

EDIT: Ah I actually got it wrong, since we want to force wait after the completion of the next node. Sorry! What I did was not blocking the execution afterwards, but only succeeding the Cooldown node when the child node has finished - which only works with sequence.

That’s a good point, I should set the cooldown end time again after the node returned :slight_smile:

I made the following change to the Update. This is in review :slight_smile:

        protected override Status OnUpdate()
        {
            var status = Child.CurrentStatus;
            if (status == Status.Running)
            {
                return Status.Waiting;
            }
            // Set the cooldown again because the child was still running and we only want to set it after it finished.
            m_CooldownEndTime = Time.time + Duration.Value;
            return status;
        }

Hey @Hyperion90 ,

Following from the conversation on the other channel, I believe I may have found the issue.

RepeatWhileConditionModifier returns waiting right after returning the child, not checking its status. This means it might not be awaken correctly. Also for some reason the conditions aren’t started correctly before checking them for the first time. I’ll try and get this out tomorrow to fix for you.

@Hyperion90 ,

I think this should fix the issue. Can you try it locally? You can give it a different name and class name and edit the id if you want to use it without editing the package locally.

using System;
using System.Collections.Generic;
using Unity.Properties;
using UnityEngine;

namespace Unity.Behavior
{
    /// <summary>
    /// Repeats the branch while the condition is true.
    /// </summary>
    [Serializable, GeneratePropertyBag]
    [NodeDescription(
        name: "Repeat While", 
        description: "Repeats the flow underneath as long as the specified condition(s) are true.",
        category: "Flow",
        hideInSearch: true,
        icon: "Icons/repeat_until_change",
        id: "bcd62844ac1b14f074e31df34956441a")]
    internal partial class RepeatWhileConditionModifier : Modifier, IConditional
    {
        [SerializeReference]
        protected List<Condition> m_Conditions = new List<Condition>();
        public List<Condition> Conditions { get => m_Conditions; set => m_Conditions = value; }

        [SerializeField]
        protected bool m_RequiresAllConditions;
        public bool RequiresAllConditions { get => m_RequiresAllConditions; set => m_RequiresAllConditions = value; }

        /// <inheritdoc cref="OnStart" />
        protected override Status OnStart()
        {
            if (Child == null || Conditions.Count == 0)
            {
                return Status.Failure;
            }

            // Early out in case the condition is already filled and prevent DoWhile condition. 
            foreach (Condition condition in Conditions)
            {
                condition.OnStart();
            }

            bool conditionIsTrue = ConditionUtils.CheckConditions(Conditions, RequiresAllConditions);
            if (!conditionIsTrue)
            {
                return Status.Success;
            }
            
            return StartNode(Child) == Status.Running ? Status.Waiting : Status.Running;
        }

        /// <inheritdoc cref="OnUpdate" />
        protected override Status OnUpdate()
        {
            bool conditionIsTrue = ConditionUtils.CheckConditions(Conditions, RequiresAllConditions);
            // If condition is true, we need to restart the child
            if (conditionIsTrue)
            {
                return RestartChild() == Status.Running ? Status.Waiting : Status.Running;
            }

            return Status.Success;
        }

        private Status RestartChild()
        {
            EndNode(Child);
            return StartNode(Child);
        }
        
        protected override void OnEnd()
        {
            base.OnEnd();

            foreach (Condition condition in Conditions)
            {
                condition.OnEnd();
            }
        }
    }
}

It seems like it’s half fixed - the loop now correctly iterates once every second, but only the first node (the “Log” node) is being executed. The Navigation node doesn’t get executed anymore, not even once

I didn’t check for Waiting properly:

using System;
using System.Collections.Generic;
using Unity.Properties;
using UnityEngine;

namespace Unity.Behavior
{
    /// <summary>
    /// Repeats the branch while the condition is true.
    /// </summary>
    [Serializable, GeneratePropertyBag]
    [NodeDescription(
        name: "Repeat While", 
        description: "Repeats the flow underneath as long as the specified condition(s) are true.",
        category: "Flow",
        hideInSearch: true,
        icon: "Icons/repeat_until_change",
        id: "bcd62844ac1b14f074e31df34956441a")]
    internal partial class RepeatWhileConditionModifier : Modifier, IConditional
    {
        [SerializeReference]
        protected List<Condition> m_Conditions = new List<Condition>();
        public List<Condition> Conditions { get => m_Conditions; set => m_Conditions = value; }

        [SerializeField]
        protected bool m_RequiresAllConditions;
        public bool RequiresAllConditions { get => m_RequiresAllConditions; set => m_RequiresAllConditions = value; }

        /// <inheritdoc cref="OnStart" />
        protected override Status OnStart()
        {
            if (Child == null || Conditions.Count == 0)
            {
                return Status.Failure;
            }

            // Early out in case the condition is already filled and prevent DoWhile condition. 
            foreach (Condition condition in Conditions)
            {
                condition.OnStart();
            }

            bool conditionIsTrue = ConditionUtils.CheckConditions(Conditions, RequiresAllConditions);
            if (!conditionIsTrue)
            {
                return Status.Success;
            }

            Status childStatus = StartNode(Child);
            return (childStatus == Status.Running || childStatus == Status.Waiting) ? Status.Waiting : Status.Running;
        }

        /// <inheritdoc cref="OnUpdate" />
        protected override Status OnUpdate()
        {
            bool conditionIsTrue = ConditionUtils.CheckConditions(Conditions, RequiresAllConditions);
            // If condition is true, we need to restart the child
            if (conditionIsTrue)
            {
                Status childStatus = RestartChild();
                return (childStatus == Status.Running || childStatus == Status.Waiting) ? Status.Waiting : Status.Running;
            }

            return Status.Success;
        }

        private Status RestartChild()
        {
            EndNode(Child);
            return StartNode(Child);
        }
        
        protected override void OnEnd()
        {
            base.OnEnd();

            foreach (Condition condition in Conditions)
            {
                condition.OnEnd();
            }
        }
    }
}

Sorry! :grimacing: it’s too late :slight_smile:

Yes everything works great now, thanks! You guys are doing a great job and are incredibly quick to answer. Please know we greatly appreciate your help and take care of yourself, we wouldn’t want you to get burned out :grin:

Hehe yay! Thank you. Sorry for troubling you with a quick not-fully-working-fix :joy:

Hi @Hyperion90 ,

This is now in 1.0.5 which is now out :slight_smile: