From ac045a5b3a2643411486bb2ab3ac7b0d7221fe2a Mon Sep 17 00:00:00 2001 From: miguel Date: Fri, 17 Nov 2023 19:07:10 +0000 Subject: [PATCH] :bug: fix limiter node behaviour (#250) * :bug: make behavior of limiter consistent with other nodes * add note about limiter interruption behavior * reset limiters properly * clarify docs * tweak test timings * remove uncessecary warnings * fix timings * fix warning message on limiter --- addons/beehave/nodes/beehave_tree.gd | 10 +++--- addons/beehave/nodes/composites/composite.gd | 8 ----- addons/beehave/nodes/decorators/decorator.gd | 8 ----- addons/beehave/nodes/decorators/limiter.gd | 29 +++++++++++---- .../beehave/nodes/decorators/time_limiter.gd | 33 +++++++++++------ docs/manual/decorators.md | 8 +++-- test/nodes/decorators/limiter_test.gd | 19 ++++++++-- test/nodes/decorators/time_limiter_test.gd | 36 ++++++++++--------- 8 files changed, 94 insertions(+), 57 deletions(-) diff --git a/addons/beehave/nodes/beehave_tree.gd b/addons/beehave/nodes/beehave_tree.gd index d0f5b925..5950259f 100644 --- a/addons/beehave/nodes/beehave_tree.gd +++ b/addons/beehave/nodes/beehave_tree.gd @@ -74,11 +74,6 @@ func _ready() -> void: if Engine.is_editor_hint(): return - if self.get_child_count() > 0 and not self.get_child(0) is BeehaveNode: - push_warning("Beehave error: Root %s should have only one child of type BeehaveNode (NodePath: %s)" % [self.name, self.get_path()]) - disable() - return - if not blackboard: _internal_blackboard = Blackboard.new() add_child(_internal_blackboard, false, Node.INTERNAL_MODE_BACK) @@ -124,6 +119,8 @@ func _physics_process(delta: float) -> void: func tick() -> int: + if actor == null or get_child_count() == 0: + return FAILURE var child := self.get_child(0) if status != RUNNING: child.before_run(actor, blackboard) @@ -143,6 +140,9 @@ func tick() -> int: func _get_configuration_warnings() -> PackedStringArray: var warnings:PackedStringArray = [] + + if actor == null: + warnings.append("Configure target node on tree") if get_children().any(func(x): return not (x is BeehaveNode)): warnings.append("All children of this node should inherit from BeehaveNode class.") diff --git a/addons/beehave/nodes/composites/composite.gd b/addons/beehave/nodes/composites/composite.gd index 67e7afa6..4a8fa5a5 100644 --- a/addons/beehave/nodes/composites/composite.gd +++ b/addons/beehave/nodes/composites/composite.gd @@ -7,14 +7,6 @@ class_name Composite extends BeehaveNode var running_child: BeehaveNode = null -func _ready(): - if Engine.is_editor_hint(): - return - - if self.get_child_count() < 1: - push_warning("BehaviorTree Error: Composite %s should have at least one child (NodePath: %s)" % [self.name, self.get_path()]) - - func _get_configuration_warnings() -> PackedStringArray: var warnings: PackedStringArray = super._get_configuration_warnings() diff --git a/addons/beehave/nodes/decorators/decorator.gd b/addons/beehave/nodes/decorators/decorator.gd index 826fd4aa..3b934983 100644 --- a/addons/beehave/nodes/decorators/decorator.gd +++ b/addons/beehave/nodes/decorators/decorator.gd @@ -8,14 +8,6 @@ class_name Decorator extends BeehaveNode var running_child: BeehaveNode = null -func _ready(): - if Engine.is_editor_hint(): - return - - if self.get_child_count() != 1: - push_warning("Beehave Error: Decorator %s should have only one child (NodePath: %s)" % [self.name, self.get_path()]) - - func _get_configuration_warnings() -> PackedStringArray: var warnings: PackedStringArray = super._get_configuration_warnings() diff --git a/addons/beehave/nodes/decorators/limiter.gd b/addons/beehave/nodes/decorators/limiter.gd index 4b88f2bc..ae2695b2 100644 --- a/addons/beehave/nodes/decorators/limiter.gd +++ b/addons/beehave/nodes/decorators/limiter.gd @@ -2,19 +2,20 @@ @icon("../../icons/limiter.svg") class_name LimiterDecorator extends Decorator -## The limiter will execute its child `x` amount of times. When the number of +## The limiter will execute its `RUNNING` child `x` amount of times. When the number of ## maximum ticks is reached, it will return a `FAILURE` status code. +## The count resets the next time that a child is not `RUNNING` @onready var cache_key = 'limiter_%s' % self.get_instance_id() @export var max_count : float = 0 func tick(actor: Node, blackboard: Blackboard) -> int: - var child = self.get_child(0) - var current_count = blackboard.get_value(cache_key, 0, str(actor.get_instance_id())) + if not get_child_count() == 1: + return FAILURE - if current_count == 0: - child.before_run(actor, blackboard) + var child = get_child(0) + var current_count = blackboard.get_value(cache_key, 0, str(actor.get_instance_id())) if current_count < max_count: blackboard.set_value(cache_key, current_count + 1, str(actor.get_instance_id())) @@ -29,14 +30,30 @@ func tick(actor: Node, blackboard: Blackboard) -> int: if child is ActionLeaf and response == RUNNING: running_child = child blackboard.set_value("running_action", child, str(actor.get_instance_id())) - + + if response != RUNNING: + child.after_run(actor, blackboard) + return response else: + interrupt(actor, blackboard) child.after_run(actor, blackboard) return FAILURE + + +func before_run(actor: Node, blackboard: Blackboard) -> void: + blackboard.set_value(cache_key, 0, str(actor.get_instance_id())) + if get_child_count() > 0: + get_child(0).before_run(actor, blackboard) func get_class_name() -> Array[StringName]: var classes := super() classes.push_back(&"LimiterDecorator") return classes + + +func _get_configuration_warnings() -> PackedStringArray: + if not get_child_count() == 1: + return ["Requires exactly one child node"] + return [] diff --git a/addons/beehave/nodes/decorators/time_limiter.gd b/addons/beehave/nodes/decorators/time_limiter.gd index be4bddb3..b9fa93fd 100644 --- a/addons/beehave/nodes/decorators/time_limiter.gd +++ b/addons/beehave/nodes/decorators/time_limiter.gd @@ -2,20 +2,25 @@ @icon("../../icons/limiter.svg") class_name TimeLimiterDecorator extends Decorator -## The Time Limit Decorator will give its child a set amount of time to finish -## before interrupting it and return a `FAILURE` status code. The timer is reset -## every time before the node runs. +## The Time Limit Decorator will give its `RUNNING` child a set amount of time to finish +## before interrupting it and return a `FAILURE` status code. +## The timer resets the next time that a child is not `RUNNING` @export var wait_time: = 0.0 -var time_left: = 0.0 - -@onready var child: BeehaveNode = get_child(0) +@onready var cache_key = 'time_limiter_%s' % self.get_instance_id() func tick(actor: Node, blackboard: Blackboard) -> int: + if not get_child_count() == 1: + return FAILURE + + var child = self.get_child(0) + var time_left = blackboard.get_value(cache_key, 0.0, str(actor.get_instance_id())) + if time_left < wait_time: time_left += get_physics_process_delta_time() + blackboard.set_value(cache_key, time_left, str(actor.get_instance_id())) var response = child.tick(actor, blackboard) if can_send_message(blackboard): BeehaveDebuggerMessages.process_tick(child.get_instance_id(), response) @@ -28,20 +33,28 @@ func tick(actor: Node, blackboard: Blackboard) -> int: running_child = child if child is ActionLeaf: blackboard.set_value("running_action", child, str(actor.get_instance_id())) - + else: + child.after_run(actor, blackboard) return response else: - child.after_run(actor, blackboard) interrupt(actor, blackboard) + child.after_run(actor, blackboard) return FAILURE func before_run(actor: Node, blackboard: Blackboard) -> void: - time_left = 0.0 - child.before_run(actor, blackboard) + blackboard.set_value(cache_key, 0.0, str(actor.get_instance_id())) + if get_child_count() > 0: + get_child(0).before_run(actor, blackboard) func get_class_name() -> Array[StringName]: var classes := super() classes.push_back(&"TimeLimiterDecorator") return classes + + +func _get_configuration_warnings() -> PackedStringArray: + if not get_child_count() == 1: + return ["Requires exactly one child node"] + return [] diff --git a/docs/manual/decorators.md b/docs/manual/decorators.md index 5ef90cce..23472d97 100644 --- a/docs/manual/decorators.md +++ b/docs/manual/decorators.md @@ -17,11 +17,15 @@ An `Inverter` node reverses the outcome of its child node. It returns `FAILURE` **Example:** An NPC is patrolling an area and should change its path if it *doesn't* detect an enemy. ## Limiter -The `Limiter` node executes its child a specified number of times (x). When the maximum number of ticks is reached, it returns a `FAILURE` status code. This can be beneficial when you want to limit the number of times an action or condition is executed, such as limiting the number of attempts an NPC makes to perform a task. +The `Limiter` node executes its `RUNNING` child a specified number of times (x). When the maximum number of ticks is reached, it returns a `FAILURE` status code. The limiter resets its counter after its child returns either `SUCCESS` or `FAILURE`. + +This node can be beneficial when you want to limit the number of times an action or condition is executed, such as limiting the number of attempts an NPC makes to perform a task. Once a limiter reaches its maximum number of ticks, it will start interrupting its child on every tick. **Example:** An NPC tries to unlock a door with lockpicks but will give up after three attempts if unsuccessful. ## TimeLimiter -The `TimeLimiter` node only gives its child a set amount of time to finish. When the time is up, it interrupts its child and returns a `FAILURE` status code. This is useful when you want to limit the execution time of a long running action. +The `TimeLimiter` node only gives its `RUNNING` child a set amount of time to finish. When the time is up, it interrupts its child and returns a `FAILURE` status code. The time limiter resets its time after its child returns either `SUCCESS` or `FAILURE`. + +This note is useful when you want to limit the execution time of a long running action. Once a time limiter reaches its time limit, it will start interrupting its child on every tick. **Example:** A mob aggros and tries to chase you, the chase action will last a maximum of 10 seconds before being aborted if not complete. diff --git a/test/nodes/decorators/limiter_test.gd b/test/nodes/decorators/limiter_test.gd index 08a0b252..13439cfa 100644 --- a/test/nodes/decorators/limiter_test.gd +++ b/test/nodes/decorators/limiter_test.gd @@ -32,11 +32,25 @@ func before_test() -> void: func test_max_count(count: int, test_parameters: Array = [[2], [0]]) -> void: limiter.max_count = count - + action.status = BeehaveNode.RUNNING for i in range(count): - assert_that(tree.tick()).is_equal(BeehaveNode.SUCCESS) + assert_that(tree.tick()).is_equal(BeehaveNode.RUNNING) + assert_that(action.count).is_equal(count) assert_that(tree.tick()).is_equal(BeehaveNode.FAILURE) + # ensure it resets its child after it reached max count + assert_that(action.count).is_equal(0) + + +func test_interrupt_after_run() -> void: + action.status = BeehaveNode.RUNNING + limiter.max_count = 1 + tree.tick() + assert_that(limiter.running_child).is_equal(action) + action.status = BeehaveNode.FAILURE + tree.tick() + assert_that(action.count).is_equal(0) + assert_that(limiter.running_child).is_equal(null) func test_clear_running_child_after_run() -> void: @@ -46,4 +60,5 @@ func test_clear_running_child_after_run() -> void: assert_that(limiter.running_child).is_equal(action) action.status = BeehaveNode.SUCCESS tree.tick() + assert_that(action.count).is_equal(2) assert_that(limiter.running_child).is_equal(null) diff --git a/test/nodes/decorators/time_limiter_test.gd b/test/nodes/decorators/time_limiter_test.gd index 3320f2ab..04a06eb0 100644 --- a/test/nodes/decorators/time_limiter_test.gd +++ b/test/nodes/decorators/time_limiter_test.gd @@ -13,47 +13,51 @@ const __blackboard = "res://addons/beehave/blackboard.gd" var tree: BeehaveTree var action: ActionLeaf var time_limiter: TimeLimiterDecorator +var actor: Node2D +var blackboard: Blackboard +var runner:GdUnitSceneRunner func before_test() -> void: tree = auto_free(load(__tree).new()) + actor = auto_free(Node2D.new()) + blackboard = auto_free(load(__blackboard).new()) + + tree.actor = actor + tree.blackboard = blackboard action = auto_free(load(__action).new()) time_limiter = auto_free(load(__source).new()) - var actor = auto_free(Node2D.new()) - var blackboard = auto_free(load(__blackboard).new()) - + time_limiter.add_child(action) tree.add_child(time_limiter) - time_limiter.child = action - tree.actor = actor - tree.blackboard = blackboard + runner = scene_runner(tree) func test_return_failure_when_child_exceeds_time_limiter() -> void: - time_limiter.wait_time = 1.0 + time_limiter.wait_time = 0.1 action.status = BeehaveNode.RUNNING + await runner.simulate_frames(1, 10) assert_that(tree.tick()).is_equal(BeehaveNode.RUNNING) - time_limiter.time_left = 0.5 - assert_that(tree.tick()).is_equal(BeehaveNode.RUNNING) - time_limiter.time_left = 1.0 + await runner.simulate_frames(5, 100) assert_that(tree.tick()).is_equal(BeehaveNode.FAILURE) func test_reset_when_child_finishes() -> void: - time_limiter.wait_time = 1.0 + time_limiter.wait_time = 0.5 action.status = BeehaveNode.RUNNING + await runner.simulate_frames(1, 10) assert_that(tree.tick()).is_equal(BeehaveNode.RUNNING) - time_limiter.time_left = 0.5 + await runner.simulate_frames(5, 100) action.status = BeehaveNode.SUCCESS assert_that(tree.tick()).is_equal(BeehaveNode.SUCCESS) func test_clear_running_child_after_run() -> void: - time_limiter.wait_time = 1.0 + time_limiter.wait_time = 0.5 action.status = BeehaveNode.RUNNING - tree.tick() + await runner.simulate_frames(1, 50) assert_that(time_limiter.running_child).is_equal(action) action.status = BeehaveNode.SUCCESS - tree.tick() - assert_that(time_limiter.running_child).is_equal(null) + await runner.simulate_frames(1, 50) + assert_that(time_limiter.running_child).is_null()