Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pause and resume #30

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Pause and resume #30

wants to merge 16 commits into from

Conversation

lulugo19
Copy link

@lulugo19 lulugo19 commented Feb 5, 2020

Added functionality to pause and resume the behavior tree.

Pausing the tree means stopping the currently executing tasks, then doing nothing (the tree is just in idle mode) and when resuming the previously stopped tasks are started again.

Added Pause() and Resume() methods to the Node class.
Pause() stops the current active tasks on that branch, without effecting the children stop logic of the Container nodes above.
When Resume() is called the previously stopped tasks are started again.

I used this for my enemies in the game. So when they got hurt, I could pause the tree and play a hurt animation (stun) and then resume the tree again.

Changed some Decorator nodes to remove the timers on pause and register them on resume again

  • ObservingDecorator stops observing on Pause() and starts observing again on Resume()
  • Service stops on Pause() and resumes on Resume()
  • WaitForCondition stops checking the condition on Pause() and starts checking it again on Resume()

Lukas Gobelet added 6 commits February 5, 2020 17:59
There is now a 'Pause'-Method and a 'Resume'-Method on every node.
When Pause() is called on the behavior tree (the root), the current active tasks are stopped
but without affecting the decorators and composites stop logic.
When Resume() is called the previously stopped tasks are started again.

Pause() and Resume() should't be called on other nodes but the root.

Instead of changing the 'Root', 'Task' and 'Composite' class as suggested, I just changed the common parent class 'Container'.
Unregister Clock timers at Pause() and register them again at Resume() in Service decorator.
…ly, asserting the right values

Because Pause() is only cascaded on active nodes, we can assume that Pause() is only called on active node. The only exception is the first node on which Pause() is called, therefore there is an assert checking this condition.

There is also an assert checking that Resume() is only called on a paused container.
I have written five tests:
1.) pausing and resuming of simple behavior tree
2.) pausing and resuming of slighly more complex behavior tree
3.) checking if the behavior tree ignores the blackboard conditon when paused -> this fails, the ignoring works, but not the notifying after resuming
4.) service is inactive when paused
5.) ignore WaitForCondition when paused
@lulugo19
Copy link
Author

lulugo19 commented Feb 6, 2020

Ignoring BlackboardCondition is pause state not working - no notification after resuming

Hi, I have written some tests and have noted a problem and I don't know how I could fix this.
The fourth Test IgnoreBlackBoardConditionWhenPausedSelf() is failing. This test tests if the blackboard condition is ignored in the pause state.
The ignoring part is working but after resuming the tree with Resume(), there is no notification sent that the blackboard has updated and the blackboard conditon (with Stops.Self) will not stop.

Lukas Gobelet added 3 commits February 6, 2020 20:28
The problem of the tests failing is still that there is no notification after resuming that the blackboard condition has changed.
…so no restart)

When a ObservingDecorator is Paused, the decorator stops observing.

But there is also the case where another child in the parent composite is paused and the ObservingDecorator is inactive but still observes for changes to do his logic depending on the stops behavior.
For this case there is now an additional check so that no evaluation for stopping and restarting happens in the pause state.
@meniku
Copy link
Owner

meniku commented Feb 6, 2020

Ignoring BlackboardCondition is pause state not working - no notification after resuming

Hi, I have written some tests and have noted a problem and I don't know how I could fix this.
The fourth Test IgnoreBlackBoardConditionWhenPausedSelf() is failing. This test tests if the blackboard condition is ignored in the pause state.
The ignoring part is working but after resuming the tree with Resume(), there is no notification sent that the blackboard has updated and the blackboard conditon (with Stops.Self) will not stop.

Alright, I will look into this

@meniku
Copy link
Owner

meniku commented Feb 6, 2020

I've fixed it by putting this bit inside the BlackboardCondition:

        public override void Resume()
        {
            base.Resume();
            if( this.CurrentState == State.ACTIVE )
            {
                Evaluate();
            }
        }

it just re-evaluates after the resuming is done.

However I got three other failing tests from you. You need help with those, too?

… resume, made to the blackboard while the BT was paused
Copy link
Owner

@meniku meniku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very impressive work! I think it's pretty special and only very few people will actually ever need this, but for those that want to, it's definitely a good feature to have.

I think I found some edge cases that may be a bit problematic under special circumstances, however I should maybe also write some special tests to prove me right :-)

// Testing different composite types behavior when pausing and the blackboard condition with Stops.IMMEDIATE_RESTART is changed.
// The change of the condition should be ignored.
[Test]
public void IgnoreBlackboardConditionWhenPausedImmediateRestartSelector()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test fails for me

}

[Test]
public void IgnoreBlackboardConditionWhenPausedImmediateRestartSequence()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test fails for me

public override void Resume()
{
base.Resume();
startService();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You maybe want to check that the CurrentState is ACTIVE before calling startService, as it could be that your underlying branch just decided to immediately stop during the Resume operation. Or it might be better if you do call the startService before calling the base.Resume

public override void Resume()
{
base.Resume();
StartObserving();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as for the service, I think it's safer to either call StartObserving() before calling base.Resume()or to check the Node's current state for being Active when calling StartObserving()
(although I'm still not quite sure what's better tbh)

public override void Resume()
{
base.Resume();
addTimerOrStartImmediately();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might cause it to start the child node even though the base.Resume node might already have started it. You will need to do some more checking here.
You only want to call the addTimerOrStartImmediately if the Decoratee object wasn't yet started.

@lulugo19
Copy link
Author

lulugo19 commented Feb 7, 2020

I've fixed it by putting this bit inside the BlackboardCondition:

        public override void Resume()
        {
            base.Resume();
            if( this.CurrentState == State.ACTIVE )
            {
                Evaluate();
            }
        }

it just re-evaluates after the resuming is done.

However I got three other failing tests from you. You need help with those, too?

Thanks, this is a great solution. I think it would be better to put this code in the parent class ObservingDecorator, so this will also work for the BlackboardQuery. I will do this.

Yeah I know why the other tests are still failing. I may need your help on this, because right know I don't know how to solve this.

It's just the same problem. That there is no evaluation after resuming. But the above doesn't work because the BlackboardCondition isn't actually Paused and so Resume is never called.
Only active nodes are paused. Previously inactive nodes stay inactive. And when we have following setup of a tree.

     R (Paused)
     |
     X (Paused)
 ____|____
 |       |
BC      T2 (Paused)
 |
T1

The blackboard condtion (BC) isn't actually Paused an thus no Resume is called.

A possible solution would be to have the BC also paused.
Right now the Pause() method in the Container class is the following:

public override void Pause()
{
  Assert.AreEqual(this.currentState, State.ACTIVE, "Only an active container can be paused.");
            currentState = State.PAUSED;

  foreach (Node child in Children)
  {
    if (child.isActive) 
    {
      child.Pause();
      this.pausedChildren.Add(child);
    }
  }
}

We could change it to this:

public override void Pause()
{
  if (!isActive)
    return;
  currentState = State.PAUSED;
  foreach (Node child in Children)
  {
    child.Pause();
    if (child.CurrentState == State.PAUSED)
    {
      this.pausedChildren.Add(child);
    }
  }
}

and overwrite it in the ObservingDecorator class with this:

public override void Pause()
{
  currentState = State.PAUSED;
  StopObserving();
  base.Pause();
}

So that ObservingDecorators are paused altough there are inactive but don't propagate Pause() on its children when inactive.

I think it's a good solution. There is only one minor problem that Pause() can be called on an inactive node which is intended and we don't have the assert prohibiting this. But Pause() should actually only be called on the Root node anyway and I can put the assertion there.
Alright I am doing this that way.

Lukas Gobelet added 3 commits February 7, 2020 12:30
To fix the problem, I changed that an inactive blackboard condition within a paused composite is actually also paused, so that after resuming an evaluation happens.

For this to happen I changed the Pause() method in Container so that Pause() is always called even if the container is active. With exceptions to the tasks, on them Pause() should only be called when there are active, so that they can be stopped.
Then the child is only added to the pausedChildren when the state is paused or when it is a stopped task.
In ObservingDecorator the method Pause() was then overwritten to always
pause, no matter in which state in, but only propagate Pause() on the children when active.
Because it's Paused it will be evaluated after Resume().

The only other tricky thing was the StopLowerPriorityChildrenForChild() method in Composite. This method searches for a lower priority active child and thus it's important when resuming that the lower priority nodes are resumed before the higher priority nodes (from right to left).
So we need to resume from right to left. Therefore a Stack for the pausedChildren was used so that the nodes are resumed from right to left.
…n paused parent composite

all test are passing now.
When resuming the timer should be restarted before the node is activated again, because after activation the node could be immediatly stopped which would lead to inconsistent state.
@lulugo19
Copy link
Author

lulugo19 commented Feb 7, 2020

Everything is fixed now.
I am very satisfied with the solution and overall with the feature.
Yeah you're right, it's probably not something that get used so much, but I really like it that I managed to add it without adding more complexity to the rest of the library, because it's mostly independent from the rest.

Thank you very much for your help. 👍

I think I have finished it for now. Maybe I am going to add some additional tests for the Cooldown decorator or something...

@meniku
Copy link
Owner

meniku commented Feb 7, 2020

sweet, I'll take a look tomorrow! thanks for contributing in such a high quality!

@meniku
Copy link
Owner

meniku commented Feb 13, 2020

sorry for not responding. I'm trying to create a test for one case I found that breaks, however I've some trouble with my visual studio currently.
I didn't forget about this :)

Lukas Gobelet added 2 commits March 13, 2020 10:18
Else TimeMax stops the children which leads to an inconsistent state.
@lulugo19
Copy link
Author

Hello meniku,
I made a pause and now I am working further on my game.
I have noticed some more mistakes with the Pausing functionality. I am continuing fixing mistakes and will give you insight later whether the Pausing functionality worked for my game so you can consider adding this to the library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants