Merge b5fe6bb77f into f90c48eb81

Fix re-parenting an agent interrupts its state machine

bt/tasks/bt_task.cpp

@@ -172,9 +172,8 @@ void BTTask::initialize(Node *p_agent, const Ref<Blackboard> &p_blackboard, Node
 		get_child(i)->initialize(p_agent, p_blackboard, p_scene_root);
 	}
 
-	if (!GDVIRTUAL_CALL(_setup)) {
 	_setup();
-	}
+	GDVIRTUAL_CALL(_setup);
 }
 
 Ref<BTTask> BTTask::clone() const {
@@ -235,9 +234,9 @@ BT::Status BTTask::execute(double p_delta) {
 				data.children.get(i)->abort();
 			}
 		}
-		if (!GDVIRTUAL_CALL(_enter)) {
+		// First native, then script.
 		_enter();
-		}
+		GDVIRTUAL_CALL(_enter);
 	} else {
 		data.elapsed += p_delta;
 	}
@@ -247,9 +246,9 @@ BT::Status BTTask::execute(double p_delta) {
 	}
 
 	if (data.status != RUNNING) {
-		if (!GDVIRTUAL_CALL(_exit)) {
+		// First script, then native.
+		GDVIRTUAL_CALL(_exit);
 		_exit();
-		}
 		data.elapsed = 0.0;
 	}
 	return data.status;
@@ -260,10 +259,10 @@ void BTTask::abort() {
 		get_child(i)->abort();
 	}
 	if (data.status == RUNNING) {
-		if (!GDVIRTUAL_CALL(_exit)) {
+		// First script, then native.
+		GDVIRTUAL_CALL(_exit);
 		_exit();
 	}
-	}
 	data.status = FRESH;
 	data.elapsed = 0.0;
 }

doc/source/classes/class_limbohsm.rst

@@ -45,27 +45,27 @@ Methods
 .. table::
    :widths: auto
 
-   +-------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+   +-------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
-   | |void|                              | :ref:`add_transition<class_LimboHSM_method_add_transition>`\ (\ from_state\: :ref:`LimboState<class_LimboState>`, to_state\: :ref:`LimboState<class_LimboState>`, event\: ``StringName``\ ) |
+   | |void|                              | :ref:`add_transition<class_LimboHSM_method_add_transition>`\ (\ from_state\: :ref:`LimboState<class_LimboState>`, to_state\: :ref:`LimboState<class_LimboState>`, event\: ``StringName``, guard\: ``Callable`` = Callable()\ ) |
-   +-------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+   +-------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
    | |void|                              | :ref:`change_active_state<class_LimboHSM_method_change_active_state>`\ (\ state\: :ref:`LimboState<class_LimboState>`\ )                                                                                                       |
-   +-------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+   +-------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
    | :ref:`LimboState<class_LimboState>` | :ref:`get_active_state<class_LimboHSM_method_get_active_state>`\ (\ ) |const|                                                                                                                                                  |
-   +-------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+   +-------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
    | :ref:`LimboState<class_LimboState>` | :ref:`get_leaf_state<class_LimboHSM_method_get_leaf_state>`\ (\ ) |const|                                                                                                                                                      |
-   +-------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+   +-------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
    | :ref:`LimboState<class_LimboState>` | :ref:`get_previous_active_state<class_LimboHSM_method_get_previous_active_state>`\ (\ ) |const|                                                                                                                                |
-   +-------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+   +-------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
    | ``bool``                            | :ref:`has_transition<class_LimboHSM_method_has_transition>`\ (\ from_state\: :ref:`LimboState<class_LimboState>`, event\: ``StringName``\ ) |const|                                                                            |
-   +-------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+   +-------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
    | |void|                              | :ref:`initialize<class_LimboHSM_method_initialize>`\ (\ agent\: ``Node``, parent_scope\: :ref:`Blackboard<class_Blackboard>` = null\ )                                                                                         |
-   +-------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+   +-------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
    | |void|                              | :ref:`remove_transition<class_LimboHSM_method_remove_transition>`\ (\ from_state\: :ref:`LimboState<class_LimboState>`, event\: ``StringName``\ )                                                                              |
-   +-------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+   +-------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
    | |void|                              | :ref:`set_active<class_LimboHSM_method_set_active>`\ (\ active\: ``bool``\ )                                                                                                                                                   |
-   +-------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+   +-------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
    | |void|                              | :ref:`update<class_LimboHSM_method_update>`\ (\ delta\: ``float``\ )                                                                                                                                                           |
-   +-------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+   +-------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
 
 .. rst-class:: classref-section-separator
 
@@ -191,9 +191,16 @@ Method Descriptions
 
 .. rst-class:: classref-method
 
-|void| **add_transition**\ (\ from_state\: :ref:`LimboState<class_LimboState>`, to_state\: :ref:`LimboState<class_LimboState>`, event\: ``StringName``\ ) :ref:`🔗<class_LimboHSM_method_add_transition>`
+|void| **add_transition**\ (\ from_state\: :ref:`LimboState<class_LimboState>`, to_state\: :ref:`LimboState<class_LimboState>`, event\: ``StringName``, guard\: ``Callable`` = Callable()\ ) :ref:`🔗<class_LimboHSM_method_add_transition>`
 
-Establishes a transition from one state to another when ``event`` is dispatched. Both ``from_state`` and ``to_state`` must be immediate children of this state.
+Establishes a transition from one state to another when ``event`` is dispatched. Both ``from_state`` and ``to_state`` must be immediate children of this **LimboHSM**.
+
+Optionally, a ``guard`` function can be specified, which must return a boolean value. If the guard function returns ``false``, the transition will not occur. The guard function is called immediately before the transition is considered. For a state-wide guard function, check out :ref:`LimboState.set_guard<class_LimboState_method_set_guard>`.
+
+::
+
+    func my_guard() -> bool:
+        return is_some_condition_met()
 
 .. rst-class:: classref-item-separator
 

doc_classes/LimboHSM.xml

@@ -14,8 +14,14 @@
 			<param index="0" name="from_state" type="LimboState" />
 			<param index="1" name="to_state" type="LimboState" />
 			<param index="2" name="event" type="StringName" />
+			<param index="3" name="guard" type="Callable" default="Callable()" />
 			<description>
-				Establishes a transition from one state to another when [param event] is dispatched. Both [param from_state] and [param to_state] must be immediate children of this state.
+				Establishes a transition from one state to another when [param event] is dispatched. Both [param from_state] and [param to_state] must be immediate children of this [LimboHSM].
+				Optionally, a [param guard] function can be specified, which must return a boolean value. If the guard function returns [code]false[/code], the transition will not occur. The guard function is called immediately before the transition is considered. For a state-wide guard function, check out [method LimboState.set_guard].
+				[codeblock]
+				func my_guard() -&gt; bool:
+				    return is_some_condition_met()
+				[/codeblock]
 			</description>
 		</method>
 		<method name="change_active_state">

hsm/limbo_hsm.cpp

@@ -100,7 +100,7 @@ void LimboHSM::update(double p_delta) {
 	}
 }
 
-void LimboHSM::add_transition(LimboState *p_from_state, LimboState *p_to_state, const StringName &p_event) {
+void LimboHSM::add_transition(LimboState *p_from_state, LimboState *p_to_state, const StringName &p_event, const Callable &p_guard) {
 	ERR_FAIL_COND_MSG(p_from_state != nullptr && p_from_state->get_parent() != this, "LimboHSM: Unable to add a transition from a state that is not an immediate child of mine.");
 	ERR_FAIL_COND_MSG(p_to_state == nullptr, "LimboHSM: Unable to add a transition to a null state.");
 	ERR_FAIL_COND_MSG(p_to_state->get_parent() != this, "LimboHSM: Unable to add a transition to a state that is not an immediate child of mine.");
@@ -108,8 +108,13 @@ void LimboHSM::add_transition(LimboState *p_from_state, LimboState *p_to_state,
 
 	TransitionKey key = Transition::make_key(p_from_state, p_event);
 	ERR_FAIL_COND_MSG(transitions.has(key), "LimboHSM: Unable to add another transition with the same event and origin.");
-	// Note: Explicit casting needed for GDExtension.
+	// Note: Explicit ObjectID casting needed for GDExtension.
-	transitions[key] = { p_from_state != nullptr ? ObjectID(p_from_state->get_instance_id()) : ObjectID(), ObjectID(p_to_state->get_instance_id()), p_event };
+	transitions[key] = {
+		p_from_state != nullptr ? ObjectID(p_from_state->get_instance_id()) : ObjectID(),
+		ObjectID(p_to_state->get_instance_id()),
+		p_event,
+		p_guard
+	};
 }
 
 void LimboHSM::remove_transition(LimboState *p_from_state, const StringName &p_event) {
@@ -166,13 +171,13 @@ bool LimboHSM::_dispatch(const StringName &p_event, const Variant &p_cargo) {
 
 		Transition transition;
 		_get_transition(active_state, p_event, transition);
-		if (transition.is_valid()) {
+		if (transition.is_valid() && transition.is_allowed()) {
 			to_state = Object::cast_to<LimboState>(ObjectDB::get_instance(transition.to_state));
 		}
 		if (to_state == nullptr) {
 			// Get ANYSTATE transition.
 			_get_transition(nullptr, p_event, transition);
-			if (transition.is_valid()) {
+			if (transition.is_valid() && transition.is_allowed()) {
 				to_state = Object::cast_to<LimboState>(ObjectDB::get_instance(transition.to_state));
 				if (to_state == active_state) {
 					// Transitions to self are not allowed with ANYSTATE.
@@ -260,22 +265,46 @@ void LimboHSM::_validate_property(PropertyInfo &p_property) const {
 	}
 }
 
+void LimboHSM::_exit_if_not_inside_tree() {
+	if (is_active() && !is_inside_tree()) {
+		_exit();
+	}
+}
+
 void LimboHSM::_notification(int p_what) {
 	switch (p_what) {
 		case NOTIFICATION_POST_ENTER_TREE: {
 			if (was_active && is_root()) {
 				// Re-activate the root HSM if it was previously active.
-				// Typically, this happens when the node is re-entered scene repeatedly (e.g., re-parenting, pooling).
+				// Typically, this happens when the node is re-entered scene repeatedly (such as with object pooling).
 				set_active(true);
 			}
 		} break;
 		case NOTIFICATION_EXIT_TREE: {
 			if (is_root()) {
-				// Remember active status for re-parenting and exit state machine
+				// Exit the state machine if the root HSM is no longer in the scene tree (except when being reparented).
-				// to release resources and signal connections if active.
+				// This ensures that resources and signal connections are released if active.
-				was_active = active;
+				was_active = is_active();
 				if (is_active()) {
+					// Check if the HSM node is being deleted.
+					bool is_being_deleted = false;
+					Node *node = this;
+					while (node) {
+						if (node->is_queued_for_deletion()) {
+							is_being_deleted = true;
+							break;
+						}
+						node = node->get_parent();
+					}
+
+					if (is_being_deleted) {
+						// Exit the state machine immediately if the HSM is being deleted.
 						_exit();
+					} else {
+						// Use deferred mode to prevent exiting during Node re-parenting.
+						// This allows the HSM to remain active when it (or one of its parents) is reparented.
+						callable_mp(this, &LimboHSM::_exit_if_not_inside_tree).call_deferred();
+					}
 				}
 			}
 		} break;
@@ -300,7 +329,7 @@ void LimboHSM::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("get_leaf_state"), &LimboHSM::get_leaf_state);
 	ClassDB::bind_method(D_METHOD("set_active", "active"), &LimboHSM::set_active);
 	ClassDB::bind_method(D_METHOD("update", "delta"), &LimboHSM::update);
-	ClassDB::bind_method(D_METHOD("add_transition", "from_state", "to_state", "event"), &LimboHSM::add_transition);
+	ClassDB::bind_method(D_METHOD("add_transition", "from_state", "to_state", "event", "guard"), &LimboHSM::add_transition, DEFVAL(Callable()));
 	ClassDB::bind_method(D_METHOD("remove_transition", "from_state", "event"), &LimboHSM::remove_transition);
 	ClassDB::bind_method(D_METHOD("has_transition", "from_state", "event"), &LimboHSM::has_transition);
 	ClassDB::bind_method(D_METHOD("anystate"), &LimboHSM::anystate);

hsm/limbo_hsm.h

@@ -39,9 +39,12 @@ private:
 		ObjectID from_state;
 		ObjectID to_state;
 		StringName event;
+		Callable guard;
 
 		inline bool is_valid() const { return to_state != ObjectID(); }
 
+		inline bool is_allowed() const { return guard.is_null() || guard.call(); }
+
 		static _FORCE_INLINE_ TransitionKey make_key(LimboState *p_from_state, const StringName &p_event) {
 			return TransitionKey(
 					p_from_state != nullptr ? uint64_t(p_from_state->get_instance_id()) : 0,
@@ -60,6 +63,7 @@ private:
 	HashMap<TransitionKey, Transition, TransitionKeyHasher> transitions;
 
 	void _get_transition(LimboState *p_from_state, const StringName &p_event, Transition &r_transition) const;
+	void _exit_if_not_inside_tree();
 
 protected:
 	static void _bind_methods();
@@ -93,7 +97,7 @@ public:
 
 	void update(double p_delta);
 
-	void add_transition(LimboState *p_from_state, LimboState *p_to_state, const StringName &p_event);
+	void add_transition(LimboState *p_from_state, LimboState *p_to_state, const StringName &p_event, const Callable &p_guard = Callable());
 	void remove_transition(LimboState *p_from_state, const StringName &p_event);
 	bool has_transition(LimboState *p_from_state, const StringName &p_event) const { return transitions.has(Transition::make_key(p_from_state, p_event)); }
 

tests/test_hsm.h

@@ -215,7 +215,7 @@ TEST_CASE("[Modules][LimboAI] HSM") {
 		CHECK(beta_updates->num_callbacks == 0);
 		CHECK(beta_exits->num_callbacks == 1); // * exited
 	}
-	SUBCASE("Test transition with guard") {
+	SUBCASE("Test transition with state-wide guard") {
 		Ref<TestGuard> guard = memnew(TestGuard);
 		state_beta->set_guard(callable_mp(guard.ptr(), &TestGuard::can_enter));
 
@@ -234,6 +234,25 @@ TEST_CASE("[Modules][LimboAI] HSM") {
 			CHECK(beta_entries->num_callbacks == 0);
 		}
 	}
+	SUBCASE("Test transition with transition-scoped guard") {
+		Ref<TestGuard> guard = memnew(TestGuard);
+		hsm->add_transition(state_alpha, state_beta, "guarded_transition", callable_mp(guard.ptr(), &TestGuard::can_enter));
+
+		SUBCASE("When entry is permitted") {
+			guard->permitted_to_enter = true;
+			hsm->dispatch("guarded_transition");
+			CHECK(hsm->get_active_state() == state_beta);
+			CHECK(alpha_exits->num_callbacks == 1);
+			CHECK(beta_entries->num_callbacks == 1);
+		}
+		SUBCASE("When entry is not permitted") {
+			guard->permitted_to_enter = false;
+			hsm->dispatch("guarded_transition");
+			CHECK(hsm->get_active_state() == state_alpha);
+			CHECK(alpha_exits->num_callbacks == 0);
+			CHECK(beta_entries->num_callbacks == 0);
+		}
+	}
 	SUBCASE("When there is no transition for given event") {
 		hsm->dispatch("not_found");
 		CHECK(alpha_exits->num_callbacks == 0);