Return None if ticking BT after it finishes. (#45)

Today, ticking a behavior tree after it has finished has confusing
results. Looking at the code, it seems the intent was that once a
`State` finishes, it is not expected to tick again. For example, if you
tick a Sequence/Select after it has finished, it always returns
`Running`. Here's an example test that shows this strange behavior:

```rust
#[test]
fn weird_behavior() {
    let a = 4;

    let mut bt = BT::new(
        Select(vec![
            Invert(Box::new(Action(Dec))),
            Invert(Box::new(Action(Dec))),
            Invert(Box::new(Action(Dec))),
        ]),
        (),
    );

    let (a, s, _) = tick(a, 0.1, &mut bt);
    assert_eq!(a, 1);
    assert_eq!(s, Failure);
    let (a, s, _) = tick(a, 0.1, &mut bt);
    assert_eq!(a, 1);
    assert_eq!(s, Running);
    let (a, s, _) = tick(a, 0.1, &mut bt);
    assert_eq!(a, 1);
    assert_eq!(s, Running);
}
```

This is clearly wrong. We could fix this particular issue (make
Sequence/Select return a finished status if you tick it again), but it's
not clear that ticking a finished behavior makes sense at all - so
perhaps we should stop that.

This PR solves this by just keeping track of the BT returns a Success or
Failure status and then prevents ticking in those cases (returning an
Option)

bonsai/src/bt.rs

@@ -18,6 +18,8 @@ pub struct BT<A, B> {
     /// referred to as a "blackboard". State is written to and read from a
     /// blackboard, allowing nodes to share state and communicate each other.
     bb: B,
+    /// Whether the tree has been finished before.
+    finished: bool,
 }
 
 impl<A: Clone, B> BT<A, B> {
@@ -29,10 +31,13 @@ impl<A: Clone, B> BT<A, B> {
             state: bt,
             initial_behavior: backup_behavior,
             bb: blackboard,
+            finished: false,
         }
     }
 
-    /// Updates the cursor that tracks an event.
+    /// Updates the cursor that tracks an event. Returns [`None`] if attempting
+    /// to tick after this tree has already returned [`Status::Success`] or
+    /// [`Status::Failure`].
     ///
     /// The action need to return status and remaining delta time.
     /// Returns status and the remaining delta time.
@@ -45,12 +50,21 @@ impl<A: Clone, B> BT<A, B> {
     /// it actually took to complete the traversal and propagate the
     /// results back up to the root node
     #[inline]
-    pub fn tick<E, F>(&mut self, e: &E, f: &mut F) -> (Status, f64)
+    pub fn tick<E, F>(&mut self, e: &E, f: &mut F) -> Option<(Status, f64)>
     where
         E: UpdateEvent,
         F: FnMut(ActionArgs<E, A>, &mut B) -> (Status, f64),
     {
-        self.state.tick(e, &mut self.bb, f)
+        if self.finished {
+            return None;
+        }
+        match self.state.tick(e, &mut self.bb, f) {
+            result @ (Status::Success | Status::Failure, _) => {
+                self.finished = true;
+                Some(result)
+            }
+            result => Some(result),
+        }
     }
 
     /// Retrieve a mutable reference to the blackboard for
@@ -72,7 +86,14 @@ impl<A: Clone, B> BT<A, B> {
     /// PS! invoking reset_bt does not reset the Blackboard.
     pub fn reset_bt(&mut self) {
         let initial_behavior = self.initial_behavior.to_owned();
-        self.state = State::new(initial_behavior)
+        self.state = State::new(initial_behavior);
+        self.finished = false;
+    }
+
+    /// Whether this behavior tree is in a completed state (the last tick returned
+    /// [`Status::Success`] or [`Status::Failure`]).
+    pub fn is_finished(&self) -> bool {
+        self.finished
     }
 }
 

bonsai/src/lib.rs

@@ -60,7 +60,7 @@
 //!             acc -= 1;
 //!             (Success, args.dt)
 //!         }
-//!     });
+//!     }).unwrap();
 //!
 //!     // update counter in blackboard
 //!     let bb = bt.get_blackboard();

bonsai/src/visualizer.rs

@@ -154,16 +154,18 @@ mod tests {
     // A test state machine that can increment and decrement.
     fn tick(mut acc: i32, dt: f64, bt: &mut BT<TestActions, HashMap<String, i32>>) -> (i32, Status, f64) {
         let e: Event = UpdateArgs { dt }.into();
-        let (s, t) = bt.tick(&e, &mut |args, blackboard| match args.action {
-            TestActions::Inc => {
-                acc += 1;
-                (Success, args.dt)
-            }
-            TestActions::Dec => {
-                acc -= 1;
-                (Success, args.dt)
-            }
-        });
+        let (s, t) = bt
+            .tick(&e, &mut |args, blackboard| match args.action {
+                TestActions::Inc => {
+                    acc += 1;
+                    (Success, args.dt)
+                }
+                TestActions::Dec => {
+                    acc -= 1;
+                    (Success, args.dt)
+                }
+            })
+            .unwrap();
         (acc, s, t)
     }
 

bonsai/tests/behavior_tests.rs

@@ -21,36 +21,38 @@ enum TestActions {
 fn tick(mut acc: i32, dt: f64, state: &mut BT<TestActions, ()>) -> (i32, bonsai_bt::Status, f64) {
     let e: Event = UpdateArgs { dt }.into();
     println!("acc {}", acc);
-    let (s, t) = state.tick(&e, &mut |args: ActionArgs<Event, TestActions>, _| match *args.action {
-        Inc => {
-            acc += 1;
-            (Success, args.dt)
-        }
-        Dec => {
-            acc -= 1;
-            (Success, args.dt)
-        }
-        LessThan(v) => {
-            println!("inside less than with acc: {}", acc);
-            if acc < v {
-                println!("success {}<{}", acc, v);
+    let (s, t) = state
+        .tick(&e, &mut |args: ActionArgs<Event, TestActions>, _| match *args.action {
+            Inc => {
+                acc += 1;
                 (Success, args.dt)
-            } else {
-                println!("failure {}>={}", acc, v);
-                (Failure, args.dt)
             }
-        }
-        TestActions::LessThanRunningSuccess(v) => {
-            println!("inside LessThanRunningSuccess with acc: {}", acc);
-            if acc < v {
-                println!("success {}<{}", acc, v);
-                (Running, args.dt)
-            } else {
-                println!("failure {}>={}", acc, v);
+            Dec => {
+                acc -= 1;
                 (Success, args.dt)
             }
-        }
-    });
+            LessThan(v) => {
+                println!("inside less than with acc: {}", acc);
+                if acc < v {
+                    println!("success {}<{}", acc, v);
+                    (Success, args.dt)
+                } else {
+                    println!("failure {}>={}", acc, v);
+                    (Failure, args.dt)
+                }
+            }
+            TestActions::LessThanRunningSuccess(v) => {
+                println!("inside LessThanRunningSuccess with acc: {}", acc);
+                if acc < v {
+                    println!("success {}<{}", acc, v);
+                    (Running, args.dt)
+                } else {
+                    println!("failure {}>={}", acc, v);
+                    (Success, args.dt)
+                }
+            }
+        })
+        .unwrap();
     println!("status: {:?} dt: {}", s, t);
 
     (acc, s, t)
@@ -60,17 +62,19 @@ fn tick(mut acc: i32, dt: f64, state: &mut BT<TestActions, ()>) -> (i32, bonsai_
 fn tick_with_ref(acc: &mut i32, dt: f64, state: &mut BT<TestActions, ()>) {
     let e: Event = UpdateArgs { dt }.into();
 
-    state.tick(&e, &mut |args: ActionArgs<Event, TestActions>, _| match *args.action {
-        Inc => {
-            *acc += 1;
-            (Success, args.dt)
-        }
-        Dec => {
-            *acc -= 1;
-            (Success, args.dt)
-        }
-        TestActions::LessThanRunningSuccess(_) | LessThan(_) => todo!(),
-    });
+    state
+        .tick(&e, &mut |args: ActionArgs<Event, TestActions>, _| match *args.action {
+            Inc => {
+                *acc += 1;
+                (Success, args.dt)
+            }
+            Dec => {
+                *acc -= 1;
+                (Success, args.dt)
+            }
+            TestActions::LessThanRunningSuccess(_) | LessThan(_) => todo!(),
+        })
+        .unwrap();
 }
 
 // Each action that terminates immediately
@@ -85,8 +89,12 @@ fn test_immediate_termination() {
     let mut state = BT::new(seq, ());
     tick_with_ref(&mut a, 0.0, &mut state);
     assert_eq!(a, 2);
+    assert!(state.is_finished());
+    state.reset_bt();
     tick_with_ref(&mut a, 1.0, &mut state);
-    assert_eq!(a, 2)
+    assert_eq!(a, 4);
+    assert!(state.is_finished());
+    state.reset_bt();
 }
 
 // Tree terminates after 2.001 seconds
@@ -218,14 +226,17 @@ fn test_if_less_than() {
     let (a, s, _) = tick(a, 0.1, &mut state);
     assert_eq!(a, 2);
     assert_eq!(s, Success);
+    state.reset_bt();
     let (a, s, _) = tick(a, 0.1, &mut state);
     assert_eq!(a, 1);
     assert_eq!(s, Success);
+    state.reset_bt();
     let (a, s, _) = tick(a, 0.1, &mut state);
     assert_eq!(a, 0);
     assert_eq!(s, Success);
+    state.reset_bt();
     let (a, s, _) = tick(a, 0.1, &mut state);
-    assert_eq!(a, -1);
+    assert_eq!(a, 1);
     assert_eq!(s, Success);
 }
 
@@ -270,50 +281,29 @@ fn test_select_succeed_on_first() {
 
     let (a, _, _) = tick(a, 0.1, &mut state);
     assert_eq!(a, 1);
+    state.reset_bt();
     let (a, _, _) = tick(a, 0.1, &mut state);
     assert_eq!(a, 2);
 }
 
 #[test]
-fn test_select_no_state_reset() {
-    let a: i32 = 3;
-    let sel = Select(vec![Action(LessThan(1)), Action(Dec), Action(Inc)]);
-    let mut state = BT::new(sel, ());
-
-    let (a, s, _) = tick(a, 0.1, &mut state);
-    assert_eq!(a, 2);
-    assert_eq!(s, Success);
-    let (a, s, _) = tick(a, 0.1, &mut state);
-    assert_eq!(a, 1);
-    assert_eq!(s, Success);
-    let (a, s, _) = tick(a, 0.1, &mut state);
-    assert_eq!(a, 0);
-    assert_eq!(s, Success);
-    let (a, s, _) = tick(a, 0.1, &mut state);
-    assert_eq!(a, -1);
-    assert_eq!(s, Success);
-}
-
-#[test]
-fn test_select_with_state_reset() {
+fn test_select_needs_reset() {
     let a: i32 = 3;
     let sel = Select(vec![Action(LessThan(1)), Action(Dec), Action(Inc)]);
-    let sel_clone = sel.clone();
     let mut state = BT::new(sel, ());
 
     let (a, s, _) = tick(a, 0.1, &mut state);
     assert_eq!(a, 2);
     assert_eq!(s, Success);
+    state.reset_bt();
     let (a, s, _) = tick(a, 0.1, &mut state);
     assert_eq!(a, 1);
     assert_eq!(s, Success);
+    state.reset_bt();
     let (a, s, _) = tick(a, 0.1, &mut state);
     assert_eq!(a, 0);
     assert_eq!(s, Success);
-
-    // reset state
-    state = BT::new(sel_clone, ());
-
+    state.reset_bt();
     let (a, s, _) = tick(a, 0.1, &mut state);
     assert_eq!(a, 0);
     assert_eq!(s, Success);
@@ -338,26 +328,28 @@ fn test_select_and_when_all() {
 fn test_select_and_invert() {
     let a: i32 = 3;
     let sel = Invert(Box::new(Select(vec![Action(LessThan(1)), Action(Dec), Action(Inc)])));
-    let whenall = WhenAll(vec![Wait(0.35), sel]);
-    let mut state = BT::new(whenall, ());
+    let mut state = BT::new(sel, ());
 
     // Running + Failure = Failure
     let (a, s, _) = tick(a, 0.1, &mut state);
     assert_eq!(a, 2);
     assert_eq!(s, Failure);
+    state.reset_bt();
     let (a, s, _) = tick(a, 0.3, &mut state);
     assert_eq!(a, 1);
     assert_eq!(s, Failure);
+    state.reset_bt();
     let (a, s, _) = tick(a, 0.1, &mut state);
     assert_eq!(a, 0);
     assert_eq!(s, Failure);
+    state.reset_bt();
     let (a, s, _) = tick(a, 0.1, &mut state);
-    assert_eq!(a, -1);
+    assert_eq!(a, 0);
     assert_eq!(s, Failure);
 }
 
 #[test]
-fn test_allways_succeed() {
+fn test_always_succeed() {
     let a: i32 = 3;
     let sel = Sequence(vec![
         Wait(0.5),
@@ -375,12 +367,14 @@ fn test_allways_succeed() {
     let (a, s, _) = tick(a, 0.7, &mut state);
     assert_eq!(a, 3);
     assert_eq!(s, Success);
-    let (a, s, _) = tick(a, 0.4, &mut state);
+    state.reset_bt();
+    let (a, s, _) = tick(a, 0.5, &mut state);
     assert_eq!(a, 3);
     assert_eq!(s, Success);
+    state.reset_bt();
     let (a, s, _) = tick(a, 0.1, &mut state);
     assert_eq!(a, 3);
-    assert_eq!(s, Success);
+    assert_eq!(s, Running);
 }
 
 #[test]

bonsai/tests/blackboard_tests.rs

@@ -17,16 +17,18 @@ pub enum TestActions {
 fn tick(mut acc: i32, dt: f64, bt: &mut BT<TestActions, HashMap<String, i32>>) -> i32 {
     let e: Event = UpdateArgs { dt }.into();
 
-    let (_s, _t) = bt.tick(&e, &mut |args, _| match *args.action {
-        Inc => {
-            acc += 1;
-            (Success, args.dt)
-        }
-        Dec => {
-            acc -= 1;
-            (Success, args.dt)
-        }
-    });
+    let (_s, _t) = bt
+        .tick(&e, &mut |args, _| match *args.action {
+            Inc => {
+                acc += 1;
+                (Success, args.dt)
+            }
+            Dec => {
+                acc -= 1;
+                (Success, args.dt)
+            }
+        })
+        .unwrap();
 
     // update counter in blackboard
     let bb = bt.get_blackboard();