Fix Navigation agent callback wild pointer crash

Fixes crash in sanitizer builds when callback agent or object are already freed.

(cherry picked from commit 194c1c44e0a20faa4463e3a41bb12cf93a71fc03)

doc/classes/Navigation2DServer.xml

@@ -40,12 +40,12 @@
 		<method name="agent_set_callback" qualifiers="const">
 			<return type="void" />
 			<argument index="0" name="agent" type="RID" />
-			<argument index="1" name="receiver" type="Object" />
+			<argument index="1" name="object_id" type="int" />
 			<argument index="2" name="method" type="String" />
 			<argument index="3" name="userdata" type="Variant" default="null" />
 			<description>
-				Callback called at the end of the RVO process. If a callback is created manually and the agent is placed on a navigation map it will calculate avoidance for the agent and dispatch the calculated [code]safe_velocity[/code] to the [code]receiver[/code] object with a signal to the chosen [code]method[/code] name.
-				[b]Note:[/b] Created callbacks are always processed independently of the SceneTree state as long as the agent is on a navigation map and not freed. To disable the dispatch of a callback from an agent use [method agent_set_callback] again with a [code]null[/code] object as the [code]receiver[/code].
+				Sets the callback [code]object_id[/code] and [code]method[/code] that gets called after each avoidance processing step for the [code]agent[/code]. The calculated [code]safe_velocity[/code] will be dispatched with a signal to the object just before the physics calculations.
+				[b]Note:[/b] Created callbacks are always processed independently of the SceneTree state as long as the agent is on a navigation map and not freed. To disable the dispatch of a callback from an agent use [method agent_set_callback] again with a [code]0[/code] ObjectID as the [code]object_id[/code].
 			</description>
 		</method>
 		<method name="agent_set_map" qualifiers="const">

doc/classes/NavigationServer.xml

@@ -40,12 +40,12 @@
 		<method name="agent_set_callback" qualifiers="const">
 			<return type="void" />
 			<argument index="0" name="agent" type="RID" />
-			<argument index="1" name="receiver" type="Object" />
+			<argument index="1" name="object_id" type="int" />
 			<argument index="2" name="method" type="String" />
 			<argument index="3" name="userdata" type="Variant" default="null" />
 			<description>
-				Callback called at the end of the RVO process. If a callback is created manually and the agent is placed on a navigation map it will calculate avoidance for the agent and dispatch the calculated [code]safe_velocity[/code] to the [code]receiver[/code] object with a signal to the chosen [code]method[/code] name.
-				[b]Note:[/b] Created callbacks are always processed independently of the SceneTree state as long as the agent is on a navigation map and not freed. To disable the dispatch of a callback from an agent use [method agent_set_callback] again with a [code]null[/code] object as the [code]receiver[/code].
+				Sets the callback [code]object_id[/code] and [code]method[/code] that gets called after each avoidance processing step for the [code]agent[/code]. The calculated [code]safe_velocity[/code] will be dispatched with a signal to the object just before the physics calculations.
+				[b]Note:[/b] Created callbacks are always processed independently of the SceneTree state as long as the agent is on a navigation map and not freed. To disable the dispatch of a callback from an agent use [method agent_set_callback] again with a [code]0[/code] ObjectID as the [code]object_id[/code].
 			</description>
 		</method>
 		<method name="agent_set_map" qualifiers="const">

modules/navigation/godot_navigation_server.cpp

@@ -537,14 +537,14 @@ bool GodotNavigationServer::agent_is_map_changed(RID p_agent) const {
 	return agent->is_map_changed();
 }
 
-COMMAND_4(agent_set_callback, RID, p_agent, Object *, p_receiver, StringName, p_method, Variant, p_udata) {
+COMMAND_4(agent_set_callback, RID, p_agent, ObjectID, p_object_id, StringName, p_method, Variant, p_udata) {
 	RvoAgent *agent = agent_owner.getornull(p_agent);
 	ERR_FAIL_COND(agent == nullptr);
 
-	agent->set_callback(p_receiver == nullptr ? 0 : p_receiver->get_instance_id(), p_method, p_udata);
+	agent->set_callback(p_object_id, p_method, p_udata);
 
 	if (agent->get_map()) {
-		if (p_receiver == nullptr) {
+		if (p_object_id == ObjectID()) {
 			agent->get_map()->remove_agent_as_controlled(agent);
 		} else {
 			agent->get_map()->set_agent_as_controlled(agent);

modules/navigation/godot_navigation_server.h

@@ -147,7 +147,7 @@ public:
 	COMMAND_2(agent_set_position, RID, p_agent, Vector3, p_position);
 	COMMAND_2(agent_set_ignore_y, RID, p_agent, bool, p_ignore);
 	virtual bool agent_is_map_changed(RID p_agent) const;
-	COMMAND_4_DEF(agent_set_callback, RID, p_agent, Object *, p_receiver, StringName, p_method, Variant, p_udata, Variant());
+	COMMAND_4_DEF(agent_set_callback, RID, p_agent, ObjectID, p_object_id, StringName, p_method, Variant, p_udata, Variant());
 
 	COMMAND_1(free, RID, p_object);
 

scene/2d/navigation_agent_2d.cpp

@@ -211,9 +211,9 @@ NavigationAgent2D::~NavigationAgent2D() {
 void NavigationAgent2D::set_avoidance_enabled(bool p_enabled) {
 	avoidance_enabled = p_enabled;
 	if (avoidance_enabled) {
-		Navigation2DServer::get_singleton()->agent_set_callback(agent, this, "_avoidance_done");
+		Navigation2DServer::get_singleton()->agent_set_callback(agent, get_instance_id(), "_avoidance_done");
 	} else {
-		Navigation2DServer::get_singleton()->agent_set_callback(agent, nullptr, "_avoidance_done");
+		Navigation2DServer::get_singleton()->agent_set_callback(agent, ObjectID(), "_avoidance_done");
 	}
 }
 
@@ -223,7 +223,7 @@ bool NavigationAgent2D::get_avoidance_enabled() const {
 
 void NavigationAgent2D::set_agent_parent(Node *p_agent_parent) {
 	// remove agent from any avoidance map before changing parent or there will be leftovers on the RVO map
-	Navigation2DServer::get_singleton()->agent_set_callback(agent, nullptr, "_avoidance_done");
+	Navigation2DServer::get_singleton()->agent_set_callback(agent, ObjectID(), "_avoidance_done");
 	if (Object::cast_to<Node2D>(p_agent_parent) != nullptr) {
 		// place agent on navigation map first or else the RVO agent callback creation fails silently later
 		agent_parent = Object::cast_to<Node2D>(p_agent_parent);

scene/3d/navigation_agent.cpp

@@ -214,9 +214,9 @@ NavigationAgent::~NavigationAgent() {
 void NavigationAgent::set_avoidance_enabled(bool p_enabled) {
 	avoidance_enabled = p_enabled;
 	if (avoidance_enabled) {
-		NavigationServer::get_singleton()->agent_set_callback(agent, this, "_avoidance_done");
+		NavigationServer::get_singleton()->agent_set_callback(agent, get_instance_id(), "_avoidance_done");
 	} else {
-		NavigationServer::get_singleton()->agent_set_callback(agent, nullptr, "_avoidance_done");
+		NavigationServer::get_singleton()->agent_set_callback(agent, ObjectID(), "_avoidance_done");
 	}
 }
 
@@ -245,7 +245,7 @@ Node *NavigationAgent::get_navigation_node() const {
 
 void NavigationAgent::set_agent_parent(Node *p_agent_parent) {
 	// remove agent from any avoidance map before changing parent or there will be leftovers on the RVO map
-	NavigationServer::get_singleton()->agent_set_callback(agent, nullptr, "_avoidance_done");
+	NavigationServer::get_singleton()->agent_set_callback(agent, ObjectID(), "_avoidance_done");
 	if (Object::cast_to<Spatial>(p_agent_parent) != nullptr) {
 		// place agent on navigation map first or else the RVO agent callback creation fails silently later
 		agent_parent = Object::cast_to<Spatial>(p_agent_parent);

servers/navigation_2d_server.cpp

@@ -146,10 +146,6 @@ Transform trf2_to_trf3(const Transform2D &d) {
 	return Transform(b, o);
 }
 
-Object *obj_to_obj(Object *d) {
-	return d;
-}
-
 StringName sn_to_sn(StringName &d) {
 	return d;
 }
@@ -158,6 +154,10 @@ Variant var_to_var(Variant &d) {
 	return d;
 }
 
+ObjectID id_to_id(const ObjectID &id) {
+	return id;
+}
+
 Ref<NavigationMesh> poly_to_mesh(Ref<NavigationPolygon> d) {
 	if (d.is_valid()) {
 		return d->get_mesh();
@@ -218,7 +218,7 @@ void Navigation2DServer::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("agent_set_target_velocity", "agent", "target_velocity"), &Navigation2DServer::agent_set_target_velocity);
 	ClassDB::bind_method(D_METHOD("agent_set_position", "agent", "position"), &Navigation2DServer::agent_set_position);
 	ClassDB::bind_method(D_METHOD("agent_is_map_changed", "agent"), &Navigation2DServer::agent_is_map_changed);
-	ClassDB::bind_method(D_METHOD("agent_set_callback", "agent", "receiver", "method", "userdata"), &Navigation2DServer::agent_set_callback, DEFVAL(Variant()));
+	ClassDB::bind_method(D_METHOD("agent_set_callback", "agent", "object_id", "method", "userdata"), &Navigation2DServer::agent_set_callback, DEFVAL(Variant()));
 
 	ClassDB::bind_method(D_METHOD("free_rid", "rid"), &Navigation2DServer::free);
 
@@ -319,6 +319,6 @@ void FORWARD_2_C(agent_set_ignore_y, RID, p_agent, bool, p_ignore, rid_to_rid, b
 
 bool FORWARD_1_C(agent_is_map_changed, RID, p_agent, rid_to_rid);
 
-void FORWARD_4_C(agent_set_callback, RID, p_agent, Object *, p_receiver, StringName, p_method, Variant, p_udata, rid_to_rid, obj_to_obj, sn_to_sn, var_to_var);
+void FORWARD_4_C(agent_set_callback, RID, p_agent, ObjectID, p_object_id, StringName, p_method, Variant, p_udata, rid_to_rid, id_to_id, sn_to_sn, var_to_var);
 
 void FORWARD_1_C(free, RID, p_object, rid_to_rid);

servers/navigation_2d_server.h

@@ -181,7 +181,7 @@ public:
 	virtual bool agent_is_map_changed(RID p_agent) const;
 
 	/// Callback called at the end of the RVO process
-	virtual void agent_set_callback(RID p_agent, Object *p_receiver, StringName p_method, Variant p_udata = Variant()) const;
+	virtual void agent_set_callback(RID p_agent, ObjectID p_object_id, StringName p_method, Variant p_udata = Variant()) const;
 
 	/// Destroy the `RID`
 	virtual void free(RID p_object) const;

servers/navigation_server.cpp

@@ -87,7 +87,7 @@ void NavigationServer::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("agent_set_target_velocity", "agent", "target_velocity"), &NavigationServer::agent_set_target_velocity);
 	ClassDB::bind_method(D_METHOD("agent_set_position", "agent", "position"), &NavigationServer::agent_set_position);
 	ClassDB::bind_method(D_METHOD("agent_is_map_changed", "agent"), &NavigationServer::agent_is_map_changed);
-	ClassDB::bind_method(D_METHOD("agent_set_callback", "agent", "receiver", "method", "userdata"), &NavigationServer::agent_set_callback, DEFVAL(Variant()));
+	ClassDB::bind_method(D_METHOD("agent_set_callback", "agent", "object_id", "method", "userdata"), &NavigationServer::agent_set_callback, DEFVAL(Variant()));
 
 	ClassDB::bind_method(D_METHOD("free_rid", "rid"), &NavigationServer::free);
 

servers/navigation_server.h

@@ -197,7 +197,7 @@ public:
 	virtual bool agent_is_map_changed(RID p_agent) const = 0;
 
 	/// Callback called at the end of the RVO process
-	virtual void agent_set_callback(RID p_agent, Object *p_receiver, StringName p_method, Variant p_udata = Variant()) const = 0;
+	virtual void agent_set_callback(RID p_agent, ObjectID p_object_id, StringName p_method, Variant p_udata = Variant()) const = 0;
 
 	/// Destroy the `RID`
 	virtual void free(RID p_object) const = 0;