ParticleProcessMaterial cannot reset Orbital Velocity properties back to default

[golddotasksquestions]

Godot version

4.0 Beta4

System information

Win 10, Nvidea 765M

Issue description

https://user-images.githubusercontent.com/47016402/201218251-4bf64024-348f-418c-a29c-c5a2ea067593.mp4

Steps to reproduce

Change the values of the ParticleProcessMaterial and try to reset it to default by clicking the icon.

Minimal reproduction project

No response

[ChristianForeman]

Hello, I'd like to try to fix this bug as my first contribution to Godot if that's alright. May take me a few days/weeks to figure out 😅

[Calinou]

Hello, I'd like to try to fix this bug as my first contribution to Godot if that's alright. May take me a few days to figure out sweat_smile

Feel free to open a pull request for this :slightly_smiling_face:

[ChristianForeman]

Quick update: still working on this issue, it's taking me quite a bit of time to familiarize myself with the codebase and fix the issue, hope that's alright!

[akien-mga]

The issue is caused by a discrepancy between the default values for a new ParticleProcessMaterial instance, and the values used for a 2D one (with Disable Z flag enabled).

The same issue happens with other properties which are shown or hidden dynamically in _validate_property based on other properties - only the set which is included in the default state has a valid revert value, the rest is not known to the docs and the ClassDB function which retrieves default values for each property.

There are more affected properties in this class, and likely in other classes too (though this one is a particularly egregious example as it has a ton of dynamically shown and hidden properties).

As a proof of concept, completely bypassing _validate_property fixes the docs, and the revert buttons:

diff --git a/doc/classes/ParticleProcessMaterial.xml b/doc/classes/ParticleProcessMaterial.xml
index d046d52ed1..d25822ac80 100644
--- a/doc/classes/ParticleProcessMaterial.xml
+++ b/doc/classes/ParticleProcessMaterial.xml
@@ -115,10 +115,10 @@
        <member name="attractor_interaction_enabled" type="bool" setter="set_attractor_interaction_enabled" getter="is_attractor_interaction_enabled" default="true">
            True if the interaction with particle attractors is enabled.
        </member>
-       <member name="collision_bounce" type="float" setter="set_collision_bounce" getter="get_collision_bounce">
+       <member name="collision_bounce" type="float" setter="set_collision_bounce" getter="get_collision_bounce" default="0.0">
            The particles' bounciness. Values range from [code]0[/code] (no bounce) to [code]1[/code] (full bounciness). Only effective if [member collision_mode] is [constant COLLISION_RIGID].
        </member>
-       <member name="collision_friction" type="float" setter="set_collision_friction" getter="get_collision_friction">
+       <member name="collision_friction" type="float" setter="set_collision_friction" getter="get_collision_friction" default="0.0">
            The particles' friction. Values range from [code]0[/code] (frictionless) to [code]1[/code] (maximum friction). Only effective if [member collision_mode] is [constant COLLISION_RIGID].
        </member>
        <member name="collision_mode" type="int" setter="set_collision_mode" getter="get_collision_mode" enum="ParticleProcessMaterial.CollisionMode" default="0">
@@ -153,7 +153,7 @@
        <member name="direction" type="Vector3" setter="set_direction" getter="get_direction" default="Vector3(1, 0, 0)">
            Unit vector specifying the particles' emission direction.
        </member>
-       <member name="emission_box_extents" type="Vector3" setter="set_emission_box_extents" getter="get_emission_box_extents">
+       <member name="emission_box_extents" type="Vector3" setter="set_emission_box_extents" getter="get_emission_box_extents" default="Vector3(1, 1, 1)">
            The box's extents if [code]emission_shape[/code] is set to [constant EMISSION_SHAPE_BOX].
        </member>
        <member name="emission_color_texture" type="Texture2D" setter="set_emission_color_texture" getter="get_emission_color_texture">
@@ -163,28 +163,28 @@
        <member name="emission_normal_texture" type="Texture2D" setter="set_emission_normal_texture" getter="get_emission_normal_texture">
            Particle velocity and rotation will be set by sampling this texture at the same point as the [member emission_point_texture]. Used only in [constant EMISSION_SHAPE_DIRECTED_POINTS]. Can be created automatically from mesh or node by selecting "Create Emission Points from Mesh/Node" under the "Particles" tool in the toolbar.
        </member>
-       <member name="emission_point_count" type="int" setter="set_emission_point_count" getter="get_emission_point_count">
+       <member name="emission_point_count" type="int" setter="set_emission_point_count" getter="get_emission_point_count" default="1">
            The number of emission points if [code]emission_shape[/code] is set to [constant EMISSION_SHAPE_POINTS] or [constant EMISSION_SHAPE_DIRECTED_POINTS].
        </member>
        <member name="emission_point_texture" type="Texture2D" setter="set_emission_point_texture" getter="get_emission_point_texture">
            Particles will be emitted at positions determined by sampling this texture at a random position. Used with [constant EMISSION_SHAPE_POINTS] and [constant EMISSION_SHAPE_DIRECTED_POINTS]. Can be created automatically from mesh or node by selecting "Create Emission Points from Mesh/Node" under the "Particles" tool in the toolbar.
        </member>
-       <member name="emission_ring_axis" type="Vector3" setter="set_emission_ring_axis" getter="get_emission_ring_axis">
+       <member name="emission_ring_axis" type="Vector3" setter="set_emission_ring_axis" getter="get_emission_ring_axis" default="Vector3(0, 0, 1)">
            The axis of the ring when using the emitter [constant EMISSION_SHAPE_RING].
        </member>
-       <member name="emission_ring_height" type="float" setter="set_emission_ring_height" getter="get_emission_ring_height">
+       <member name="emission_ring_height" type="float" setter="set_emission_ring_height" getter="get_emission_ring_height" default="1.0">
            The height of the ring when using the emitter [constant EMISSION_SHAPE_RING].
        </member>
-       <member name="emission_ring_inner_radius" type="float" setter="set_emission_ring_inner_radius" getter="get_emission_ring_inner_radius">
+       <member name="emission_ring_inner_radius" type="float" setter="set_emission_ring_inner_radius" getter="get_emission_ring_inner_radius" default="0.0">
            The inner radius of the ring when using the emitter [constant EMISSION_SHAPE_RING].
        </member>
-       <member name="emission_ring_radius" type="float" setter="set_emission_ring_radius" getter="get_emission_ring_radius">
+       <member name="emission_ring_radius" type="float" setter="set_emission_ring_radius" getter="get_emission_ring_radius" default="1.0">
            The radius of the ring when using the emitter [constant EMISSION_SHAPE_RING].
        </member>
        <member name="emission_shape" type="int" setter="set_emission_shape" getter="get_emission_shape" enum="ParticleProcessMaterial.EmissionShape" default="0">
            Particles will be emitted inside this region. Use [enum EmissionShape] constants for values.
        </member>
-       <member name="emission_sphere_radius" type="float" setter="set_emission_sphere_radius" getter="get_emission_sphere_radius">
+       <member name="emission_sphere_radius" type="float" setter="set_emission_sphere_radius" getter="get_emission_sphere_radius" default="1.0">
            The sphere's radius if [code]emission_shape[/code] is set to [constant EMISSION_SHAPE_SPHERE].
        </member>
        <member name="flatness" type="float" setter="set_flatness" getter="get_flatness" default="0.0">
@@ -223,11 +223,11 @@
        <member name="orbit_velocity_curve" type="Texture2D" setter="set_param_texture" getter="get_param_texture">
            Each particle's orbital velocity will vary along this [CurveTexture].
        </member>
-       <member name="orbit_velocity_max" type="float" setter="set_param_max" getter="get_param_max">
+       <member name="orbit_velocity_max" type="float" setter="set_param_max" getter="get_param_max" default="0.0">
            Maximum orbital velocity applied to each particle. Makes the particles circle around origin. Specified in number of full rotations around origin per second.
            Only available when [member particle_flag_disable_z] is [code]true[/code].
        </member>
-       <member name="orbit_velocity_min" type="float" setter="set_param_min" getter="get_param_min">
+       <member name="orbit_velocity_min" type="float" setter="set_param_min" getter="get_param_min" default="0.0">
            Minimum equivalent of [member orbit_velocity_max].
        </member>
        <member name="particle_flag_align_y" type="bool" setter="set_particle_flag" getter="get_particle_flag" default="false">
@@ -260,13 +260,13 @@
        <member name="spread" type="float" setter="set_spread" getter="get_spread" default="45.0">
            Each particle's initial direction range from [code]+spread[/code] to [code]-spread[/code] degrees.
        </member>
-       <member name="sub_emitter_amount_at_collision" type="int" setter="set_sub_emitter_amount_at_collision" getter="get_sub_emitter_amount_at_collision">
+       <member name="sub_emitter_amount_at_collision" type="int" setter="set_sub_emitter_amount_at_collision" getter="get_sub_emitter_amount_at_collision" default="1">
            Sub particle amount on collision.
            Maximum amount set in the sub particles emitter.
        </member>
-       <member name="sub_emitter_amount_at_end" type="int" setter="set_sub_emitter_amount_at_end" getter="get_sub_emitter_amount_at_end">
+       <member name="sub_emitter_amount_at_end" type="int" setter="set_sub_emitter_amount_at_end" getter="get_sub_emitter_amount_at_end" default="1">
        </member>
-       <member name="sub_emitter_frequency" type="float" setter="set_sub_emitter_frequency" getter="get_sub_emitter_frequency">
+       <member name="sub_emitter_frequency" type="float" setter="set_sub_emitter_frequency" getter="get_sub_emitter_frequency" default="4.0">
        </member>
        <member name="sub_emitter_keep_velocity" type="bool" setter="set_sub_emitter_keep_velocity" getter="get_sub_emitter_keep_velocity" default="false">
        </member>
diff --git a/scene/resources/particle_process_material.cpp b/scene/resources/particle_process_material.cpp
index 7ae154ea1d..b6fc7276aa 100644
--- a/scene/resources/particle_process_material.cpp
+++ b/scene/resources/particle_process_material.cpp
@@ -1397,6 +1397,8 @@ RID ParticleProcessMaterial::get_shader_rid() const {
 }

 void ParticleProcessMaterial::_validate_property(PropertyInfo &p_property) const {
+   return;
+
    if (p_property.name == "emission_sphere_radius" && (emission_shape != EMISSION_SHAPE_SPHERE && emission_shape != EMISSION_SHAPE_SPHERE_SURFACE)) {
        p_property.usage = PROPERTY_USAGE_NONE;
    }

But it removes the functionality that properties which don't do anything in the current configuration should be hidden.

A proper fix would likely imply tweaking the relevant DocTools and ClassDB logic to make it skip _validate_property, though that has a significant impact and might uncover other problems.

A safer workaround is to add ADD_PROPERTY_DEFAULT("orbit_velocity_min", 0.0); etc. in the _bind_methods for each property which is currently hidden in the default state. But it's easy to miss some or to override some with a valid default, especially if the set of validated properties changes over time. And as mentioned a lot of other classes might have similar issues. So I'd suggest attempting my former suggestion first.

[YuriSizov]

Perhaps a better approach would be to set usage to storage only instead of none. We can also customize the revert behavior for these properties, but it would have no effect on the docs.