From 194c1c44e0a20faa4463e3a41bb12cf93a71fc03 Mon Sep 17 00:00:00 2001 From: smix8 <52464204+smix8@users.noreply.github.com> Date: Mon, 5 Dec 2022 23:05:56 +0100 Subject: [PATCH] Fix Navigation agent callback wild pointer crash Fixes crash in sanitizer builds when callback agent or object are already freed. --- doc/classes/NavigationServer2D.xml | 6 +++--- doc/classes/NavigationServer3D.xml | 6 +++--- modules/navigation/godot_navigation_server.cpp | 6 +++--- modules/navigation/godot_navigation_server.h | 2 +- scene/2d/navigation_agent_2d.cpp | 6 +++--- scene/3d/navigation_agent_3d.cpp | 6 +++--- servers/navigation_server_2d.cpp | 12 ++++++------ servers/navigation_server_2d.h | 2 +- servers/navigation_server_3d.cpp | 2 +- servers/navigation_server_3d.h | 2 +- 10 files changed, 25 insertions(+), 25 deletions(-) diff --git a/doc/classes/NavigationServer2D.xml b/doc/classes/NavigationServer2D.xml index 981ab8a5e1d7..e25cac751445 100644 --- a/doc/classes/NavigationServer2D.xml +++ b/doc/classes/NavigationServer2D.xml @@ -40,12 +40,12 @@ - + - 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 [param receiver] object with a signal to the chosen [param method] 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 [param receiver]. + Sets the callback [param object_id] and [param method] that gets called after each avoidance processing step for the [param agent]. 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 [param object_id]. diff --git a/doc/classes/NavigationServer3D.xml b/doc/classes/NavigationServer3D.xml index 943aa03ef7e9..55360ca98c64 100644 --- a/doc/classes/NavigationServer3D.xml +++ b/doc/classes/NavigationServer3D.xml @@ -40,12 +40,12 @@ - + - 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 [param receiver] object with a signal to the chosen [param method] 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 [param receiver]. + Sets the callback [param object_id] and [param method] that gets called after each avoidance processing step for the [param agent]. 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 [param object_id]. diff --git a/modules/navigation/godot_navigation_server.cpp b/modules/navigation/godot_navigation_server.cpp index 8ca73a3adb49..934caa033a16 100644 --- a/modules/navigation/godot_navigation_server.cpp +++ b/modules/navigation/godot_navigation_server.cpp @@ -676,14 +676,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.get_or_null(p_agent); ERR_FAIL_COND(agent == nullptr); - agent->set_callback(p_receiver == nullptr ? ObjectID() : 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); diff --git a/modules/navigation/godot_navigation_server.h b/modules/navigation/godot_navigation_server.h index ab5e722d3561..4005e34dd52f 100644 --- a/modules/navigation/godot_navigation_server.h +++ b/modules/navigation/godot_navigation_server.h @@ -167,7 +167,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 override; - 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); diff --git a/scene/2d/navigation_agent_2d.cpp b/scene/2d/navigation_agent_2d.cpp index 1a62c9bb6caa..904b6564bdd5 100644 --- a/scene/2d/navigation_agent_2d.cpp +++ b/scene/2d/navigation_agent_2d.cpp @@ -197,9 +197,9 @@ NavigationAgent2D::~NavigationAgent2D() { void NavigationAgent2D::set_avoidance_enabled(bool p_enabled) { avoidance_enabled = p_enabled; if (avoidance_enabled) { - NavigationServer2D::get_singleton()->agent_set_callback(agent, this, "_avoidance_done"); + NavigationServer2D::get_singleton()->agent_set_callback(agent, get_instance_id(), "_avoidance_done"); } else { - NavigationServer2D::get_singleton()->agent_set_callback(agent, nullptr, "_avoidance_done"); + NavigationServer2D::get_singleton()->agent_set_callback(agent, ObjectID(), "_avoidance_done"); } } @@ -209,7 +209,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 - NavigationServer2D::get_singleton()->agent_set_callback(agent, nullptr, "_avoidance_done"); + NavigationServer2D::get_singleton()->agent_set_callback(agent, ObjectID(), "_avoidance_done"); if (Object::cast_to(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(p_agent_parent); diff --git a/scene/3d/navigation_agent_3d.cpp b/scene/3d/navigation_agent_3d.cpp index 36350d251eaf..e907b9f66f7e 100644 --- a/scene/3d/navigation_agent_3d.cpp +++ b/scene/3d/navigation_agent_3d.cpp @@ -204,9 +204,9 @@ NavigationAgent3D::~NavigationAgent3D() { void NavigationAgent3D::set_avoidance_enabled(bool p_enabled) { avoidance_enabled = p_enabled; if (avoidance_enabled) { - NavigationServer3D::get_singleton()->agent_set_callback(agent, this, "_avoidance_done"); + NavigationServer3D::get_singleton()->agent_set_callback(agent, get_instance_id(), "_avoidance_done"); } else { - NavigationServer3D::get_singleton()->agent_set_callback(agent, nullptr, "_avoidance_done"); + NavigationServer3D::get_singleton()->agent_set_callback(agent, ObjectID(), "_avoidance_done"); } } @@ -216,7 +216,7 @@ bool NavigationAgent3D::get_avoidance_enabled() const { void NavigationAgent3D::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 - NavigationServer3D::get_singleton()->agent_set_callback(agent, nullptr, "_avoidance_done"); + NavigationServer3D::get_singleton()->agent_set_callback(agent, ObjectID(), "_avoidance_done"); if (Object::cast_to(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(p_agent_parent); diff --git a/servers/navigation_server_2d.cpp b/servers/navigation_server_2d.cpp index 04e5d2f6a1be..30c84c310feb 100644 --- a/servers/navigation_server_2d.cpp +++ b/servers/navigation_server_2d.cpp @@ -140,10 +140,6 @@ static Transform3D trf2_to_trf3(const Transform2D &d) { return Transform3D(b, o); } -static Object *obj_to_obj(Object *d) { - return d; -} - static StringName sn_to_sn(const StringName &d) { return d; } @@ -152,6 +148,10 @@ static Variant var_to_var(const Variant &d) { return d; } +static ObjectID id_to_id(const ObjectID &id) { + return id; +} + static Ref poly_to_mesh(Ref d) { if (d.is_valid()) { return d->get_mesh(); @@ -289,7 +289,7 @@ void NavigationServer2D::_bind_methods() { ClassDB::bind_method(D_METHOD("agent_set_target_velocity", "agent", "target_velocity"), &NavigationServer2D::agent_set_target_velocity); ClassDB::bind_method(D_METHOD("agent_set_position", "agent", "position"), &NavigationServer2D::agent_set_position); ClassDB::bind_method(D_METHOD("agent_is_map_changed", "agent"), &NavigationServer2D::agent_is_map_changed); - ClassDB::bind_method(D_METHOD("agent_set_callback", "agent", "receiver", "method", "userdata"), &NavigationServer2D::agent_set_callback, DEFVAL(Variant())); + ClassDB::bind_method(D_METHOD("agent_set_callback", "agent", "object_id", "method", "userdata"), &NavigationServer2D::agent_set_callback, DEFVAL(Variant())); ClassDB::bind_method(D_METHOD("free_rid", "rid"), &NavigationServer2D::free); @@ -408,7 +408,7 @@ 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); diff --git a/servers/navigation_server_2d.h b/servers/navigation_server_2d.h index 54cfc6b14e31..0d3769304b4c 100644 --- a/servers/navigation_server_2d.h +++ b/servers/navigation_server_2d.h @@ -218,7 +218,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; virtual void query_path(const Ref &p_query_parameters, Ref p_query_result) const; diff --git a/servers/navigation_server_3d.cpp b/servers/navigation_server_3d.cpp index cab8816747f5..d739028f94eb 100644 --- a/servers/navigation_server_3d.cpp +++ b/servers/navigation_server_3d.cpp @@ -109,7 +109,7 @@ void NavigationServer3D::_bind_methods() { ClassDB::bind_method(D_METHOD("agent_set_target_velocity", "agent", "target_velocity"), &NavigationServer3D::agent_set_target_velocity); ClassDB::bind_method(D_METHOD("agent_set_position", "agent", "position"), &NavigationServer3D::agent_set_position); ClassDB::bind_method(D_METHOD("agent_is_map_changed", "agent"), &NavigationServer3D::agent_is_map_changed); - ClassDB::bind_method(D_METHOD("agent_set_callback", "agent", "receiver", "method", "userdata"), &NavigationServer3D::agent_set_callback, DEFVAL(Variant())); + ClassDB::bind_method(D_METHOD("agent_set_callback", "agent", "object_id", "method", "userdata"), &NavigationServer3D::agent_set_callback, DEFVAL(Variant())); ClassDB::bind_method(D_METHOD("free_rid", "rid"), &NavigationServer3D::free); diff --git a/servers/navigation_server_3d.h b/servers/navigation_server_3d.h index 0f537383a2db..9c49259fa0d0 100644 --- a/servers/navigation_server_3d.h +++ b/servers/navigation_server_3d.h @@ -233,7 +233,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;