Fix repeat sequence double running bug

bonsai/Cargo.toml

@@ -12,7 +12,7 @@ name = "bonsai-bt"
 readme = "../README.md"
 repository = "https://github.com/sollimann/bonsai.git"
 rust-version = "1.60.0"
-version = "0.6.2"
+version = "0.6.3"
 
 [lib]
 name = "bonsai_bt"

bonsai/src/behavior.rs

@@ -58,6 +58,49 @@ pub enum Behavior<A> {
     ///
     /// Panics if the given behavior sequence is empty.
     ///
+    ///
+    /// ```
+    ///
+    ///use bonsai_bt::{BT, Running, Failure, Success, Action, UpdateArgs, Behavior::RepeatSequence, ActionArgs};
+    ///use bonsai_bt::Event;
+    ///
+    ///#[derive(Clone, Debug)]
+    ///
+    ///enum Ex { A, B, C }
+    ///
+    ///let rs = RepeatSequence(
+    ///    Box::new(Action(Ex::A)),
+    ///    vec![Action(Ex::B), Action(Ex::C)],
+    ///);
+    ///
+    ///let (SUCCESS, FAILURE, RUNNING ) = ((Success, 0.0), (Failure, 0.0), (Running, 0.0));
+    ///
+    ///let mut bt = BT::new(rs, ());
+    ///
+    ///let mut i = 0;
+    ///let status = bt.tick(&Event::zero_dt_args(), &mut |args: ActionArgs<Event, Ex>, _| {
+    ///    match args.action {
+    ///        Ex::A => {
+    ///            i += 1;
+    ///            if i == 4 {
+    ///                SUCCESS
+    ///            }
+    ///            else {
+    ///                RUNNING
+    ///            }
+    ///        }
+    ///        Ex::B => {
+    ///            i += 1;
+    ///            SUCCESS
+    ///        }
+    ///        Ex::C => {
+    ///            i += 1;
+    ///            SUCCESS
+    ///        }
+    ///    }
+    ///});
+    ///assert!(i == 4);
+    /// ```
     RepeatSequence(Box<Behavior<A>>, Vec<Behavior<A>>),
     /// Runs all behaviors in parallel until all succeeded.
     ///

bonsai/src/event.rs

@@ -9,6 +9,13 @@ pub struct UpdateArgs {
     pub dt: f64,
 }
 
+impl UpdateArgs {
+    /// Creates [UpdateArgs] with `0.0` delta time.
+    pub fn zero_dt() -> UpdateArgs {
+        Self { dt: 0.0 }
+    }
+}
+
 /// Models loop events.
 #[derive(Copy, Clone, Debug, PartialEq, PartialOrd, serde::Deserialize, serde::Serialize)]
 pub enum Loop {
@@ -30,6 +37,12 @@ pub enum Event {
     /// Events that commonly used by event loops.
     Loop(Loop),
 }
+impl Event {
+    /// Creates [Event] from [UpdateArgs] with `0.0` delta time.
+    pub fn zero_dt_args() -> Self {
+        UpdateArgs::zero_dt().into()
+    }
+}
 
 /// When the application state should be updated.
 pub trait UpdateEvent: Sized {

bonsai/src/state.rs

@@ -54,7 +54,7 @@ pub enum State<A> {
     /// Keeps track of a `While` behavior.
     WhileState(Box<State<A>>, Vec<Behavior<A>>, usize, Box<State<A>>),
     /// Keeps track of a `RepeatSequence` behavior.
-    RepeatSequenceState(Box<State<A>>, Vec<Behavior<A>>, usize, Box<State<A>>),
+    RepeatSequenceState(Box<State<A>>, Vec<Behavior<A>>, usize, bool, Box<State<A>>),
     /// Keeps track of a `WhenAll` behavior.
     WhenAllState(Vec<Option<State<A>>>),
     /// Keeps track of a `WhenAny` behavior.
@@ -103,7 +103,7 @@ impl<A: Clone> State<A> {
                         .expect("RepeatSequence's sequence of behaviors to run cannot be empty!")
                         .clone(),
                 );
-                State::RepeatSequenceState(Box::new(State::new(*ev)), rep, 0, Box::new(state))
+                State::RepeatSequenceState(Box::new(State::new(*ev)), rep, 0, true, Box::new(state))
             }
         }
     }
@@ -320,51 +320,65 @@ impl<A: Clone> State<A> {
                     RUNNING
                 }
             }
-            (_, &mut RepeatSequenceState(ref mut ev_cursor, ref rep, ref mut i, ref mut cursor)) => {
-                let cur = cursor;
+            (
+                _,
+                &mut RepeatSequenceState(
+                    ref mut condition_behavior,
+                    ref all_sequence_behaviors,
+                    ref mut cur_seq_idx,
+                    ref mut can_check_condition,
+                    ref mut current_sequence_behavior,
+                ),
+            ) => {
                 let mut remaining_dt = upd.unwrap_or(0.0);
-                let mut remaining_e;
                 loop {
-                    // Only check the condition when the sequence starts.
-                    if *i == 0 {
-                        // If the event terminates, stop.
-                        match ev_cursor.tick(e, blackboard, f) {
+                    // check run condition only if allowed at this time:
+                    if *can_check_condition {
+                        *can_check_condition = false;
+                        debug_assert!(
+                            *cur_seq_idx == 0,
+                            "sequence index should always be 0 when condition is checked!"
+                        );
+                        match condition_behavior.tick(e, blackboard, f) {
+                            // if running, move to sequence:
                             (Running, _) => {}
+                            // if success or failure, get out:
                             x => return x,
                         };
                     }
 
-                    match cur.tick(
-                        match upd {
-                            Some(_) => {
-                                remaining_e = UpdateEvent::from_dt(remaining_dt, e).unwrap();
-                                &remaining_e
-                            }
-                            _ => e,
-                        },
-                        blackboard,
-                        f,
-                    ) {
+                    let remaining_e;
+                    let ev = match upd {
+                        Some(_) => {
+                            remaining_e = UpdateEvent::from_dt(remaining_dt, e).unwrap();
+                            &remaining_e
+                        }
+                        _ => e,
+                    };
+
+                    match current_sequence_behavior.tick(ev, blackboard, f) {
                         (Failure, x) => return (Failure, x),
-                        (Running, _) => break,
+                        (Running, _) => {
+                            break;
+                        }
                         (Success, new_dt) => {
-                            remaining_dt = match upd {
-                                // Change update event with remaining delta time.
-                                Some(_) => new_dt,
-                                // Other events are 'consumed' and not passed to next.
-                                _ => return RUNNING,
+                            // only success moves the sequence cursor forward:
+                            *cur_seq_idx += 1;
+
+                            // If end of repeated events,
+                            // start over from the first one
+                            // and allow run condition check to happen:
+                            if *cur_seq_idx >= all_sequence_behaviors.len() {
+                                *can_check_condition = true;
+                                *cur_seq_idx = 0;
                             }
+
+                            // Create a new cursor for next event.
+                            // Use the same pointer to avoid allocation.
+                            **current_sequence_behavior = State::new(all_sequence_behaviors[*cur_seq_idx].clone());
+                            remaining_dt = new_dt;
                         }
                     };
-                    *i += 1;
-                    // If end of repeated events,
-                    // start over from the first one.
-                    if *i >= rep.len() {
-                        *i = 0;
-                    }
-                    // Create a new cursor for next event.
-                    // Use the same pointer to avoid allocation.
-                    **cur = State::new(rep[*i].clone());
                 }
                 RUNNING
             }

bonsai/tests/behavior_tests.rs

@@ -446,13 +446,140 @@ fn test_repeat_sequence() {
         assert_eq!(a, 5);
         assert_eq!(s, Success);
         assert_eq!(dt, 0.0);
+    }
+}
 
-        let (a, s, dt) = tick(a, 0.0, &mut state);
+#[test]
+/// Ensure that if the condition behavior and the first sequence behavior both return
+/// running initially, then the condition behavior cannot run more than once until the whole
+/// sequence has succeeded.
+fn test_repeat_sequence_double_running() {
+    let after = RepeatSequence(
+        Box::new(Action(LessThanRunningSuccess(5))), // running...
+        vec![
+            Action(LessThanRunningSuccess(5)), // running... until current value is 5
+            Action(Dec),                       // success... 4
+            Action(Dec),                       // success... 3
+            Action(LessThan(0)),               // failure
+        ],
+    );
+    let mut state = State::new(after);
 
-        assert_eq!(a, 5);
-        assert_eq!(s, Success);
-        assert_eq!(dt, 0.0);
+    let mut current_value = 0;
+    loop {
+        let (a, s, _) = tick(current_value, 0.0, &mut state);
+        current_value = a;
+        match s {
+            Running => {
+                current_value += 1; // increase curent value everytime sequence behavior returns running
+            }
+            _ => {
+                break;
+            }
+        }
+    }
+
+    assert_eq!(current_value, 3);
+}
+
+#[test]
+fn test_repeat_sequence2() {
+    let after = RepeatSequence(
+        Box::new(Action(LessThanRunningSuccess(5))), // running...
+        vec![
+            Action(LessThanRunningSuccess(10)), // running... until current value is 5
+            Action(Dec),                        // success... 4
+            Action(Dec),                        // success... 3
+            Action(Dec),                        // success... 2
+        ],
+    );
+    let mut state = State::new(after);
+
+    let mut current_value = 0;
+    let mut current_status;
+    loop {
+        let (a, s, _) = tick(current_value, 0.0, &mut state);
+        current_value = a;
+        current_status = s;
+        match s {
+            Running => {
+                current_value += 1; // increase current value everytime sequence behavior returns running
+            }
+            _ => {
+                break;
+            }
+        }
     }
+    assert_eq!(current_status, bonsai_bt::Status::Success);
+    assert_eq!(current_value, 7);
+}
+
+#[test]
+fn test_repeat_sequence3() {
+    let after = RepeatSequence(
+        Box::new(Action(LessThanRunningSuccess(2))),
+        vec![
+            Action(LessThanRunningSuccess(10)),
+            Action(Dec),
+            Action(Dec),
+            Action(Dec),
+            Action(LessThanRunningSuccess(10)),
+            Action(Dec),
+        ],
+    );
+    let mut state = State::new(after);
+
+    let mut current_value = 0;
+    let mut current_status;
+    loop {
+        let (a, s, _) = tick(current_value, 0.0, &mut state);
+        current_value = a;
+        current_status = s;
+        match s {
+            Running => {
+                current_value += 1;
+            }
+            _ => {
+                break;
+            }
+        }
+    }
+    assert_eq!(current_status, Success);
+    assert_eq!(current_value, 9);
+}
+
+#[test]
+fn test_repeat_sequence_nested() {
+    let dec2 = Sequence(vec![Action(Dec), Action(Dec)]);
+    let inc1 = Sequence(vec![Action(Inc)]);
+
+    let nested = RepeatSequence(Box::new(Action(LessThanRunningSuccess(5))), vec![Action(Inc), inc1]);
+
+    let after = RepeatSequence(
+        Box::new(Action(LessThanRunningSuccess(1))),
+        vec![
+            nested,      // inc to 6
+            Action(Dec), // -1
+            dec2,        // -2
+        ], // == 3
+    );
+    let mut state = State::new(after);
+
+    let mut current_value = 0;
+    let mut current_status;
+    loop {
+        let (a, s, _) = tick(current_value, 0.0, &mut state);
+        current_value = a;
+        current_status = s;
+        match s {
+            Running => {}
+            _ => {
+                break;
+            }
+        }
+    }
+    assert_eq!(current_status, bonsai_bt::Status::Success);
+    assert_eq!(current_value, 3);
 }
 
 #[test]
@@ -465,7 +592,6 @@ fn test_repeat_sequence_fail() {
         );
         let mut state = State::new(after);
         let (a, s, dt) = tick(a, 0.0, &mut state);
-
         assert_eq!(a, 3);
         assert_eq!(s, Failure);
         assert_eq!(dt, 0.0);