Fix CollisionShape{2D,3D}.debug_color inconsistencies

dalexeev · dalexeev · commit 8bf2afd341bd · 2024-12-06T15:12:20.000+03:00
diff --git a/doc/classes/CollisionShape2D.xml b/doc/classes/CollisionShape2D.xml
@@ -13,9 +13,9 @@
 		<link title="2D Kinematic Character Demo">https://godotengine.org/asset-library/asset/2719</link>
 	</tutorials>
 	<members>
-		<member name="debug_color" type="Color" setter="set_debug_color" getter="get_debug_color" default="Color(0, 0, 0, 1)">
-			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.
+		<member name="debug_color" type="Color" setter="set_debug_color" getter="get_debug_color" default="Color(0, 0, 0, 0)">
+			The collision shape color that is displayed in the editor, or in the running project if [b]Debug &gt; 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.
 		</member>
 		<member name="disabled" type="bool" setter="set_disabled" getter="is_disabled" default="false" keywords="enabled">
 			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
@@ -30,7 +30,8 @@
 	</methods>
 	<members>
 		<member name="debug_color" type="Color" setter="set_debug_color" getter="get_debug_color" default="Color(0, 0, 0, 0)">
-			The collision shape color that is displayed in the editor, or in the running project if [b]Debug &gt; 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 &gt; 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.
 		</member>
 		<member name="debug_fill" type="bool" setter="set_enable_debug_fill" getter="get_enable_debug_fill" default="true">
 			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
@@ -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
@@ -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
@@ -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<CollisionObject3D>(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<Shape3D> &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
@@ -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
@@ -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<ArrayMesh> Shape3D::get_debug_mesh() {
diff --git a/scene/resources/3d/shape_3d.h b/scene/resources/3d/shape_3d.h
@@ -47,8 +47,12 @@ class Shape3D : public Resource {
 	Ref<ArrayMesh> debug_mesh_cache;
 	Ref<Material> 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();

Original file line number	Diff line number	Diff line change
`@@ -67,12 +67,14 @@ void Shape3D::set_margin(real_t p_margin) {`
`67`	`67`	`}`
`68`	`68`
`69`	`69`	`#ifdef DEBUG_ENABLED`
	`70`	`+`
`70`	`71`	`void Shape3D::set_debug_color(const Color &p_color) {`
`71`	`72`	`if (p_color == debug_color) {`
`72`	`73`	`return;`
`73`	`74`	`}`
`74`	`75`
`75`	`76`	`debug_color = p_color;`
	`77`	`+ debug_properties_edited = true;`
`76`	`78`	`_update_shape();`
`77`	`79`	`}`
`78`	`80`
`@@ -86,12 +88,14 @@ void Shape3D::set_debug_fill(bool p_fill) {`
`86`	`88`	`}`
`87`	`89`
`88`	`90`	`debug_fill = p_fill;`
	`91`	`+ debug_properties_edited = true;`
`89`	`92`	`_update_shape();`
`90`	`93`	`}`
`91`	`94`
`92`	`95`	`bool Shape3D::get_debug_fill() const {`
`93`	`96`	`return debug_fill;`
`94`	`97`	`}`
	`98`	`+`
`95`	`99`	`#endif // DEBUG_ENABLED`
`96`	`100`
`97`	`101`	`Ref<ArrayMesh> Shape3D::get_debug_mesh() {`