godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.16k stars 21.19k forks source link

Global shader sampler variables don't store their values #92178

Open matheusmdx opened 5 months ago

matheusmdx commented 5 months ago

Tested versions

Tested in: Godot v4.3.dev [40b4130c9] and v4.3.dev6.official [89850d553]

System information

Godot v4.3.dev (40b4130c9) - Windows 10.0.19045 - Vulkan (Forward+) - dedicated AMD Radeon RX 580 2048SP (Advanced Micro Devices, Inc.; 31.0.21912.14) - AMD Ryzen 5 3600 6-Core Processor (12 Threads)

Issue description

Godot don't store the data from any sampler variable type, after set the value in project settings restarting the editor make all the sampler variables go back to empty, the exception is if you use a resource already stored in your project directory, in this case the value will be preserved after the restart

https://github.com/godotengine/godot/assets/44306054/ae47ecba-60c8-42e5-b6ea-ff5119c74986

Steps to reproduce

Create a new global shader variable in project settings of any sampler type Create a new resource inside the variable Close project settings and restart the editor

When you open the global shaders again the variable will be empty

Minimal reproduction project (MRP)

No needed, any project can reproduce that.

akien-mga commented 5 months ago

Confirmed in latest master, the sampler* types (i.e. Texture resources) are not saved properly.

With these non-empty sampler globals: image

I only get this in project.godot after saving:

[shader_globals]

test_mat2={
"type": "mat2",
"value": PackedFloat32Array(2, 3, 1, 5, 0)
}
test_vec4={
"type": "vec4",
"value": Vector4(1, 2, 3, 4)
}
test_sampler2d={
"type": "sampler2D",
"value": ""
}
test_sampler3d={
"type": "sampler3D",
"value": ""
}

Same issue in 4.2.2.stable.

akien-mga commented 5 months ago

Adding this code seems to solve saving the resources:

diff --git a/editor/shader_globals_editor.cpp b/editor/shader_globals_editor.cpp
index bd9b80590e..cf56a97be9 100644
--- a/editor/shader_globals_editor.cpp
+++ b/editor/shader_globals_editor.cpp
@@ -81,7 +81,12 @@ class ShaderGlobalsEditorInterface : public Object {
        if (type >= RS::GLOBAL_VAR_TYPE_SAMPLER2D) {
            Ref<Resource> res = p_value;
            if (res.is_valid()) {
-               gv["value"] = res->get_path();
+               // Save path if saved resource, otherwise the resource object itself.
+               if (!res->get_path().is_empty()) {
+                   gv["value"] = res->get_path();
+               } else {
+                   gv["value"] = res;
+               }
            } else {
                gv["value"] = "";
            }

The project.godot then looks like this:

[shader_globals]

test_mat2={
"type": "mat2",
"value": PackedFloat32Array(2, 3, 1, 5, 0)
}
test_vec4={
"type": "vec4",
"value": Vector4(1, 2, 3, 4)
}
test_sampler2d={
"type": "sampler2D",
"value": Object(CurveTexture,"resource_local_to_scene":false,"resource_name":"","width":512,"texture_mode":0,"curve":Object(Curve,"resource_local_to_scene":false,"resource_name":"","min_value":0.0,"max_value":1.0,"bake_resolution":100,"_data":[Vector2(0.271574, 0.705882), 0.0, 0.0, 0, 0, Vector2(0.748731, 0.0382353), 0.0, 0.0, 0, 0],"point_count":2,"script":null)
,"script":null)

}
test_sampler3d={
"type": "sampler3D",
"value": Object(NoiseTexture3D,"resource_local_to_scene":false,"resource_name":"","width":64,"height":64,"depth":64,"invert":false,"seamless":false,"seamless_blend_skirt":0.1,"normalize":true,"color_ramp":null,"noise":Object(FastNoiseLite,"resource_local_to_scene":false,"resource_name":"","noise_type":1,"seed":0,"frequency":0.01,"offset":Vector3(0, 0, 0),"fractal_type":1,"fractal_octaves":5,"fractal_lacunarity":2.0,"fractal_gain":0.5,"fractal_weighted_strength":0.0,"fractal_ping_pong_strength":2.0,"cellular_distance_function":0,"cellular_jitter":1.0,"cellular_return_type":1,"domain_warp_enabled":false,"domain_warp_type":0,"domain_warp_amplitude":30.0,"domain_warp_frequency":0.05,"domain_warp_fractal_type":1,"domain_warp_fractal_octaves":5,"domain_warp_fractal_lacunarity":6.0,"domain_warp_fractal_gain":0.5,"script":null)
,"script":null)

}
test_sampler2d_ext={
"type": "sampler2D",
"value": "res://new_noise_texture_2d.tres"
}

But that file can't be loaded again by Godot, it errors with:

ERROR: Cannot get class 'CurveTexture'.
   at: _instantiate_internal (./core/object/class_db.cpp:503)
ERROR: Error parsing /home/akien/testgdscript/project.godot at line 28: Can't instantiate Object() of type: CurveTexture File might be corrupted.
   at: _load_settings_text (core/config/project_settings.cpp:777)
ERROR: Couldn't load file '/home/akien/testgdscript/project.godot', error code 43.
   at: _load_settings_text_or_binary (core/config/project_settings.cpp:811)

And I don't really understand why. ClassDB.instantiate("CurveTexture") in GDScript works just fine. And project.godot already supports loading InputEvent objects for the input mappings.

jsjtxietian commented 5 months ago

And I don't really understand why. ClassDB.instantiate("CurveTexture") in GDScript works just fine.

Because of timing, see normally ClassDB's class should have about 1300+ class:

image

but when loading the CurveTexture, it only has some core classes in it ( in register_core_types, including Input )

image

Classes like CurveTexture are registered in register_scene_types, which is later than core