From 1aa74afcb11603e86d5c7e941a70b2533e844e16 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Wed, 22 Jan 2025 14:00:19 -0800 Subject: [PATCH] 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 | 29 +++++- bonsai/src/lib.rs | 2 +- bonsai/src/visualizer.rs | 22 ++-- bonsai/tests/behavior_tests.rs | 136 ++++++++++++------------- bonsai/tests/blackboard_tests.rs | 22 ++-- bonsai/tests/bt_tests.rs | 42 ++++---- bonsai/tests/dynamic_behavior_tests.rs | 42 ++++---- examples/src/3d/main.rs | 2 +- examples/src/async_drone/main.rs | 2 +- examples/src/boids/boid.rs | 2 +- examples/src/simple_npc_ai/main.rs | 2 +- 11 files changed, 163 insertions(+), 140 deletions(-) diff --git a/bonsai/src/bt.rs b/bonsai/src/bt.rs index 17e0a84..cd17b51 100644 --- a/bonsai/src/bt.rs +++ b/bonsai/src/bt.rs @@ -18,6 +18,8 @@ pub struct BT { /// 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 BT { @@ -29,10 +31,13 @@ impl BT { 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 BT { /// it actually took to complete the traversal and propagate the /// results back up to the root node #[inline] - pub fn tick(&mut self, e: &E, f: &mut F) -> (Status, f64) + pub fn tick(&mut self, e: &E, f: &mut F) -> Option<(Status, f64)> where E: UpdateEvent, F: FnMut(ActionArgs, &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 BT { /// 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 } } diff --git a/bonsai/src/lib.rs b/bonsai/src/lib.rs index b978f79..a8f7b57 100644 --- a/bonsai/src/lib.rs +++ b/bonsai/src/lib.rs @@ -60,7 +60,7 @@ //! acc -= 1; //! (Success, args.dt) //! } -//! }); +//! }).unwrap(); //! //! // update counter in blackboard //! let bb = bt.get_blackboard(); diff --git a/bonsai/src/visualizer.rs b/bonsai/src/visualizer.rs index f4dfad8..63def0e 100644 --- a/bonsai/src/visualizer.rs +++ b/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>) -> (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) } diff --git a/bonsai/tests/behavior_tests.rs b/bonsai/tests/behavior_tests.rs index c685f45..bfaa03b 100644 --- a/bonsai/tests/behavior_tests.rs +++ b/bonsai/tests/behavior_tests.rs @@ -21,36 +21,38 @@ enum TestActions { fn tick(mut acc: i32, dt: f64, state: &mut BT) -> (i32, bonsai_bt::Status, f64) { let e: Event = UpdateArgs { dt }.into(); println!("acc {}", acc); - let (s, t) = state.tick(&e, &mut |args: ActionArgs, _| 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, _| 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) -> (i32, bonsai_ fn tick_with_ref(acc: &mut i32, dt: f64, state: &mut BT) { let e: Event = UpdateArgs { dt }.into(); - state.tick(&e, &mut |args: ActionArgs, _| 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, _| 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] diff --git a/bonsai/tests/blackboard_tests.rs b/bonsai/tests/blackboard_tests.rs index 7b4e5f0..558dc22 100644 --- a/bonsai/tests/blackboard_tests.rs +++ b/bonsai/tests/blackboard_tests.rs @@ -17,16 +17,18 @@ pub enum TestActions { fn tick(mut acc: i32, dt: f64, bt: &mut BT>) -> 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(); diff --git a/bonsai/tests/bt_tests.rs b/bonsai/tests/bt_tests.rs index 2d404c7..4b96552 100644 --- a/bonsai/tests/bt_tests.rs +++ b/bonsai/tests/bt_tests.rs @@ -18,26 +18,28 @@ enum TestActions { fn tick(mut acc: i32, dt: f64, bt: &mut BT>) -> (i32, bonsai_bt::Status, f64) { let e: Event = UpdateArgs { dt }.into(); println!("acc {}", acc); - let (s, t) = bt.tick(&e, &mut |args, _| 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) = bt + .tick(&e, &mut |args, _| match *args.action { + Inc => { + acc += 1; (Success, args.dt) - } else { - println!("failure {}>={}", acc, v); - (Failure, args.dt) } - } - }); + 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) + } + } + }) + .unwrap(); println!("status: {:?} dt: {}", s, t); (acc, s, t) @@ -54,14 +56,14 @@ fn test_select_succeed_on_second_last() { let (a, s, _) = tick(a, 0.1, &mut bt); assert_eq!(a, 2); assert_eq!(s, Success); + bt.reset_bt(); let (a, s, _) = tick(a, 0.1, &mut bt); assert_eq!(a, 1); assert_eq!(s, Success); + bt.reset_bt(); let (a, s, _) = tick(a, 0.1, &mut bt); assert_eq!(a, 0); assert_eq!(s, Success); - - // reset bt bt.reset_bt(); let (a, s, _) = tick(a, 0.1, &mut bt); assert_eq!(a, 0); diff --git a/bonsai/tests/dynamic_behavior_tests.rs b/bonsai/tests/dynamic_behavior_tests.rs index f682ee7..4ae1df4 100644 --- a/bonsai/tests/dynamic_behavior_tests.rs +++ b/bonsai/tests/dynamic_behavior_tests.rs @@ -15,30 +15,32 @@ enum TestActions { fn tick(mut acc: usize, dt: f64, t: &mut f64, counter: &mut usize, state: &mut BT) -> usize { let e: Event = UpdateArgs { dt }.into(); - let (_s, _t) = state.tick(&e, &mut |args: ActionArgs, _| match args.action { - Inc => { - acc += 1; - (Success, args.dt) - } - DynamicWait(times) => { - // reset dynamic timer - if *counter >= times.len() { - *counter = 0 + let (_s, _t) = state + .tick(&e, &mut |args: ActionArgs, _| match args.action { + Inc => { + acc += 1; + (Success, args.dt) } + DynamicWait(times) => { + // reset dynamic timer + if *counter >= times.len() { + *counter = 0 + } - let wait_t = times[counter.to_owned()]; + let wait_t = times[counter.to_owned()]; - if *t + dt >= wait_t { - let time_overdue = *t + dt - wait_t; - *counter += 1; - *t = -dt; - (Success, time_overdue) - } else { - *t += dt; - RUNNING + if *t + dt >= wait_t { + let time_overdue = *t + dt - wait_t; + *counter += 1; + *t = -dt; + (Success, time_overdue) + } else { + *t += dt; + RUNNING + } } - } - }); + }) + .unwrap(); acc } diff --git a/examples/src/3d/main.rs b/examples/src/3d/main.rs index 4612ea0..c4f6e79 100644 --- a/examples/src/3d/main.rs +++ b/examples/src/3d/main.rs @@ -194,7 +194,7 @@ fn game_tick( }, }, - ); + ).unwrap(); // update blackboard let db = bt.get_blackboard(); diff --git a/examples/src/async_drone/main.rs b/examples/src/async_drone/main.rs index 306649b..922de79 100644 --- a/examples/src/async_drone/main.rs +++ b/examples/src/async_drone/main.rs @@ -180,7 +180,7 @@ async fn drone_tick( } }, } - ); + ).unwrap(); } #[tokio::main] diff --git a/examples/src/boids/boid.rs b/examples/src/boids/boid.rs index 4bf09c7..d3075ee 100644 --- a/examples/src/boids/boid.rs +++ b/examples/src/boids/boid.rs @@ -172,5 +172,5 @@ pub fn game_tick(dt: f32, cursor: mint::Point2, boid: &mut Boid, other_boid }, } - }); + }).unwrap(); } diff --git a/examples/src/simple_npc_ai/main.rs b/examples/src/simple_npc_ai/main.rs index e891da9..4fd248a 100644 --- a/examples/src/simple_npc_ai/main.rs +++ b/examples/src/simple_npc_ai/main.rs @@ -58,7 +58,7 @@ fn game_tick(bt: &mut BT, state: &mut EnemyNPCState) - (Success, 0.0) } } - }); + }).unwrap(); // return status: status.0