From 8bf2afd341bd5d4bd7a7467dea2949c42bf35f13 Mon Sep 17 00:00:00 2001 From: Danil Alexeev Date: Fri, 6 Dec 2024 12:36:37 +0300 Subject: [PATCH] Fix `CollisionShape{2D,3D}.debug_color` inconsistencies --- doc/classes/CollisionShape2D.xml | 6 +-- doc/classes/CollisionShape3D.xml | 3 +- scene/2d/physics/collision_shape_2d.cpp | 30 ++++++++--- scene/2d/physics/collision_shape_2d.h | 13 ++++- scene/3d/physics/collision_shape_3d.cpp | 66 +++++++++++++++---------- scene/3d/physics/collision_shape_3d.h | 8 +-- scene/resources/3d/shape_3d.cpp | 4 ++ scene/resources/3d/shape_3d.h | 6 +++ 8 files changed, 92 insertions(+), 44 deletions(-) diff --git a/doc/classes/CollisionShape2D.xml b/doc/classes/CollisionShape2D.xml index dd04bf7b8202..e0ce05fa9c03 100644 --- a/doc/classes/CollisionShape2D.xml +++ b/doc/classes/CollisionShape2D.xml @@ -13,9 +13,9 @@ https://godotengine.org/asset-library/asset/2719 - - The collision shape debug color. - [b]Note:[/b] The default value is [member ProjectSettings.debug/shapes/collision/shape_color]. The [code]Color(0, 0, 0, 1)[/code] value documented here is a placeholder, and not the actual default debug color. + + The collision shape color that is displayed in the editor, or in the running project if [b]Debug > Visible Collision Shapes[/b] is checked at the top of the editor. + [b]Note:[/b] The default value is [member ProjectSettings.debug/shapes/collision/shape_color]. The [code]Color(0, 0, 0, 0)[/code] value documented here is a placeholder, and not the actual default debug color. A disabled collision shape has no effect in the world. This property should be changed with [method Object.set_deferred]. diff --git a/doc/classes/CollisionShape3D.xml b/doc/classes/CollisionShape3D.xml index 69a7dd2b3664..b64a34209c3f 100644 --- a/doc/classes/CollisionShape3D.xml +++ b/doc/classes/CollisionShape3D.xml @@ -30,7 +30,8 @@ - The collision shape color that is displayed in the editor, or in the running project if [b]Debug > Visible Collision Shapes[/b] is checked at the top of the editor. If this is reset to its default value of [code]Color(0, 0, 0, 0)[/code], the value of [member ProjectSettings.debug/shapes/collision/shape_color] will be used instead. + The collision shape color that is displayed in the editor, or in the running project if [b]Debug > Visible Collision Shapes[/b] is checked at the top of the editor. + [b]Note:[/b] The default value is [member ProjectSettings.debug/shapes/collision/shape_color]. The [code]Color(0, 0, 0, 0)[/code] value documented here is a placeholder, and not the actual default debug color. If [code]true[/code], when the shape is displayed, it will show a solid fill color in addition to its wireframe. diff --git a/scene/2d/physics/collision_shape_2d.cpp b/scene/2d/physics/collision_shape_2d.cpp index bdd0d06b5e2a..5a4c8b00731a 100644 --- a/scene/2d/physics/collision_shape_2d.cpp +++ b/scene/2d/physics/collision_shape_2d.cpp @@ -49,11 +49,6 @@ void CollisionShape2D::_update_in_shape_owner(bool p_xform_only) { collision_object->shape_owner_set_one_way_collision_margin(owner_id, one_way_collision_margin); } -Color CollisionShape2D::_get_default_debug_color() const { - SceneTree *st = SceneTree::get_singleton(); - return st ? st->get_debug_collisions_color() : Color(); -} - void CollisionShape2D::_notification(int p_what) { switch (p_what) { case NOTIFICATION_PARENTED: { @@ -232,7 +227,18 @@ real_t CollisionShape2D::get_one_way_collision_margin() const { return one_way_collision_margin; } +#ifdef DEBUG_ENABLED + +Color CollisionShape2D::_get_default_debug_color() const { + const SceneTree *st = SceneTree::get_singleton(); + return st ? st->get_debug_collisions_color() : Color(0.0, 0.0, 0.0, 0.0); +} + void CollisionShape2D::set_debug_color(const Color &p_color) { + if (debug_color == p_color) { + return; + } + debug_color = p_color; queue_redraw(); } @@ -266,6 +272,8 @@ void CollisionShape2D::_validate_property(PropertyInfo &p_property) const { } } +#endif // DEBUG_ENABLED + void CollisionShape2D::_bind_methods() { ClassDB::bind_method(D_METHOD("set_shape", "shape"), &CollisionShape2D::set_shape); ClassDB::bind_method(D_METHOD("get_shape"), &CollisionShape2D::get_shape); @@ -275,20 +283,26 @@ void CollisionShape2D::_bind_methods() { ClassDB::bind_method(D_METHOD("is_one_way_collision_enabled"), &CollisionShape2D::is_one_way_collision_enabled); ClassDB::bind_method(D_METHOD("set_one_way_collision_margin", "margin"), &CollisionShape2D::set_one_way_collision_margin); ClassDB::bind_method(D_METHOD("get_one_way_collision_margin"), &CollisionShape2D::get_one_way_collision_margin); - ClassDB::bind_method(D_METHOD("set_debug_color", "color"), &CollisionShape2D::set_debug_color); - ClassDB::bind_method(D_METHOD("get_debug_color"), &CollisionShape2D::get_debug_color); ADD_PROPERTY(PropertyInfo(Variant::OBJECT, "shape", PROPERTY_HINT_RESOURCE_TYPE, "Shape2D"), "set_shape", "get_shape"); ADD_PROPERTY(PropertyInfo(Variant::BOOL, "disabled"), "set_disabled", "is_disabled"); ADD_PROPERTY(PropertyInfo(Variant::BOOL, "one_way_collision"), "set_one_way_collision", "is_one_way_collision_enabled"); ADD_PROPERTY(PropertyInfo(Variant::FLOAT, "one_way_collision_margin", PROPERTY_HINT_RANGE, "0,128,0.1,suffix:px"), "set_one_way_collision_margin", "get_one_way_collision_margin"); + +#ifdef DEBUG_ENABLED + ClassDB::bind_method(D_METHOD("set_debug_color", "color"), &CollisionShape2D::set_debug_color); + ClassDB::bind_method(D_METHOD("get_debug_color"), &CollisionShape2D::get_debug_color); + ADD_PROPERTY(PropertyInfo(Variant::COLOR, "debug_color"), "set_debug_color", "get_debug_color"); // Default value depends on a project setting, override for doc generation purposes. - ADD_PROPERTY_DEFAULT("debug_color", Color()); + ADD_PROPERTY_DEFAULT("debug_color", Color(0.0, 0.0, 0.0, 0.0)); +#endif // DEBUG_ENABLED } CollisionShape2D::CollisionShape2D() { set_notify_local_transform(true); set_hide_clip_children(true); +#ifdef DEBUG_ENABLED debug_color = _get_default_debug_color(); +#endif // DEBUG_ENABLED } diff --git a/scene/2d/physics/collision_shape_2d.h b/scene/2d/physics/collision_shape_2d.h index e4f04e77e408..f8c8b26b7285 100644 --- a/scene/2d/physics/collision_shape_2d.h +++ b/scene/2d/physics/collision_shape_2d.h @@ -45,17 +45,26 @@ class CollisionShape2D : public Node2D { bool disabled = false; bool one_way_collision = false; real_t one_way_collision_margin = 1.0; - Color debug_color; void _shape_changed(); void _update_in_shape_owner(bool p_xform_only = false); + + // Not wrapped in `#ifdef DEBUG_ENABLED` as it is used for rendering. + Color debug_color = Color(0.0, 0.0, 0.0, 0.0); + +#ifdef DEBUG_ENABLED Color _get_default_debug_color() const; +#endif // DEBUG_ENABLED protected: void _notification(int p_what); + +#ifdef DEBUG_ENABLED bool _property_can_revert(const StringName &p_name) const; bool _property_get_revert(const StringName &p_name, Variant &r_property) const; void _validate_property(PropertyInfo &p_property) const; +#endif // DEBUG_ENABLED + static void _bind_methods(); public: @@ -77,8 +86,10 @@ class CollisionShape2D : public Node2D { void set_one_way_collision_margin(real_t p_margin); real_t get_one_way_collision_margin() const; +#ifdef DEBUG_ENABLED void set_debug_color(const Color &p_color); Color get_debug_color() const; +#endif // DEBUG_ENABLED PackedStringArray get_configuration_warnings() const override; diff --git a/scene/3d/physics/collision_shape_3d.cpp b/scene/3d/physics/collision_shape_3d.cpp index 362c61026be1..9f9e5f33b976 100644 --- a/scene/3d/physics/collision_shape_3d.cpp +++ b/scene/3d/physics/collision_shape_3d.cpp @@ -81,12 +81,6 @@ void CollisionShape3D::_update_in_shape_owner(bool p_xform_only) { void CollisionShape3D::_notification(int p_what) { switch (p_what) { case NOTIFICATION_PARENTED: { -#ifdef DEBUG_ENABLED - if (debug_color == get_placeholder_default_color()) { - debug_color = SceneTree::get_singleton()->get_debug_collisions_color(); - } -#endif // DEBUG_ENABLED - collision_object = Object::cast_to(get_parent()); if (collision_object) { owner_id = collision_object->create_shape_owner(this); @@ -189,7 +183,7 @@ void CollisionShape3D::_bind_methods() { ADD_PROPERTY(PropertyInfo(Variant::COLOR, "debug_color"), "set_debug_color", "get_debug_color"); // Default value depends on a project setting, override for doc generation purposes. - ADD_PROPERTY_DEFAULT("debug_color", get_placeholder_default_color()); + ADD_PROPERTY_DEFAULT("debug_color", Color(0.0, 0.0, 0.0, 0.0)); ADD_PROPERTY(PropertyInfo(Variant::BOOL, "debug_fill"), "set_enable_debug_fill", "get_enable_debug_fill"); #endif // DEBUG_ENABLED @@ -200,27 +194,27 @@ void CollisionShape3D::set_shape(const Ref &p_shape) { return; } if (shape.is_valid()) { - shape->disconnect_changed(callable_mp(this, &CollisionShape3D::shape_changed)); +#ifdef DEBUG_ENABLED + shape->disconnect_changed(callable_mp(this, &CollisionShape3D::_shape_changed)); +#endif // DEBUG_ENABLED shape->disconnect_changed(callable_mp((Node3D *)this, &Node3D::update_gizmos)); } shape = p_shape; if (shape.is_valid()) { #ifdef DEBUG_ENABLED - if (shape->get_debug_color() != get_placeholder_default_color()) { + if (shape->are_debug_properties_edited()) { set_debug_color(shape->get_debug_color()); set_debug_fill_enabled(shape->get_debug_fill()); - } else if (get_debug_color() != get_placeholder_default_color()) { - shape->set_debug_color(debug_color); - shape->set_debug_fill(debug_fill); } else { - set_debug_color(SceneTree::get_singleton()->get_debug_collisions_color()); - shape->set_debug_color(SceneTree::get_singleton()->get_debug_collisions_color()); + shape->set_debug_color(debug_color); shape->set_debug_fill(debug_fill); } #endif // DEBUG_ENABLED shape->connect_changed(callable_mp((Node3D *)this, &Node3D::update_gizmos)); - shape->connect_changed(callable_mp(this, &CollisionShape3D::shape_changed)); +#ifdef DEBUG_ENABLED + shape->connect_changed(callable_mp(this, &CollisionShape3D::_shape_changed)); +#endif // DEBUG_ENABLED } update_gizmos(); if (collision_object) { @@ -254,15 +248,21 @@ bool CollisionShape3D::is_disabled() const { } #ifdef DEBUG_ENABLED + +Color CollisionShape3D::_get_default_debug_color() const { + const SceneTree *st = SceneTree::get_singleton(); + return st ? st->get_debug_collisions_color() : Color(0.0, 0.0, 0.0, 0.0); +} + void CollisionShape3D::set_debug_color(const Color &p_color) { - if (p_color == get_placeholder_default_color()) { - debug_color = SceneTree::get_singleton()->get_debug_collisions_color(); - } else if (debug_color != p_color) { - debug_color = p_color; + if (debug_color == p_color) { + return; + } - if (shape.is_valid()) { - shape->set_debug_color(p_color); - } + debug_color = p_color; + + if (shape.is_valid()) { + shape->set_debug_color(p_color); } } @@ -295,27 +295,39 @@ bool CollisionShape3D::_property_can_revert(const StringName &p_name) const { bool CollisionShape3D::_property_get_revert(const StringName &p_name, Variant &r_property) const { if (p_name == "debug_color") { - r_property = SceneTree::get_singleton()->get_debug_collisions_color(); + r_property = _get_default_debug_color(); return true; } return false; } -#endif // DEBUG_ENABLED -void CollisionShape3D::shape_changed() { -#ifdef DEBUG_ENABLED +void CollisionShape3D::_validate_property(PropertyInfo &p_property) const { + if (p_property.name == "debug_color") { + if (debug_color == _get_default_debug_color()) { + p_property.usage = PROPERTY_USAGE_DEFAULT & ~PROPERTY_USAGE_STORAGE; + } else { + p_property.usage = PROPERTY_USAGE_DEFAULT; + } + } +} + +void CollisionShape3D::_shape_changed() { if (shape->get_debug_color() != debug_color) { set_debug_color(shape->get_debug_color()); } if (shape->get_debug_fill() != debug_fill) { set_debug_fill_enabled(shape->get_debug_fill()); } -#endif // DEBUG_ENABLED } +#endif // DEBUG_ENABLED + CollisionShape3D::CollisionShape3D() { //indicator = RenderingServer::get_singleton()->mesh_create(); set_notify_local_transform(true); +#ifdef DEBUG_ENABLED + debug_color = _get_default_debug_color(); +#endif // DEBUG_ENABLED } CollisionShape3D::~CollisionShape3D() { diff --git a/scene/3d/physics/collision_shape_3d.h b/scene/3d/physics/collision_shape_3d.h index 0eaecb9f6163..fb78462550f2 100644 --- a/scene/3d/physics/collision_shape_3d.h +++ b/scene/3d/physics/collision_shape_3d.h @@ -44,10 +44,11 @@ class CollisionShape3D : public Node3D { CollisionObject3D *collision_object = nullptr; #ifdef DEBUG_ENABLED - Color debug_color = get_placeholder_default_color(); + Color debug_color; bool debug_fill = true; - static const Color get_placeholder_default_color() { return Color(0.0, 0.0, 0.0, 0.0); } + Color _get_default_debug_color() const; + void _shape_changed(); #endif // DEBUG_ENABLED #ifndef DISABLE_DEPRECATED @@ -65,10 +66,9 @@ class CollisionShape3D : public Node3D { #ifdef DEBUG_ENABLED bool _property_can_revert(const StringName &p_name) const; bool _property_get_revert(const StringName &p_name, Variant &r_property) const; + void _validate_property(PropertyInfo &p_property) const; #endif // DEBUG_ENABLED - void shape_changed(); - public: void make_convex_from_siblings(); diff --git a/scene/resources/3d/shape_3d.cpp b/scene/resources/3d/shape_3d.cpp index a1ee7b85dd53..f2b14d314cb8 100644 --- a/scene/resources/3d/shape_3d.cpp +++ b/scene/resources/3d/shape_3d.cpp @@ -67,12 +67,14 @@ void Shape3D::set_margin(real_t p_margin) { } #ifdef DEBUG_ENABLED + void Shape3D::set_debug_color(const Color &p_color) { if (p_color == debug_color) { return; } debug_color = p_color; + debug_properties_edited = true; _update_shape(); } @@ -86,12 +88,14 @@ void Shape3D::set_debug_fill(bool p_fill) { } debug_fill = p_fill; + debug_properties_edited = true; _update_shape(); } bool Shape3D::get_debug_fill() const { return debug_fill; } + #endif // DEBUG_ENABLED Ref Shape3D::get_debug_mesh() { diff --git a/scene/resources/3d/shape_3d.h b/scene/resources/3d/shape_3d.h index e956f4c32282..d0ab4d96c385 100644 --- a/scene/resources/3d/shape_3d.h +++ b/scene/resources/3d/shape_3d.h @@ -47,8 +47,12 @@ class Shape3D : public Resource { Ref debug_mesh_cache; Ref collision_material; + // Not wrapped in `#ifdef DEBUG_ENABLED` as it is used for rendering. Color debug_color = Color(0.0, 0.0, 0.0, 0.0); bool debug_fill = true; +#ifdef DEBUG_ENABLED + bool debug_properties_edited = false; +#endif // DEBUG_ENABLED protected: static void _bind_methods(); @@ -83,6 +87,8 @@ class Shape3D : public Resource { void set_debug_fill(bool p_fill); bool get_debug_fill() const; + + _FORCE_INLINE_ bool are_debug_properties_edited() const { return debug_properties_edited; } #endif // DEBUG_ENABLED Shape3D();