Prevent stack overflow when setting a shader global value

[Chaosus]

Should fix #68791. It's a regression of #59564, causes stack overflow (infinite recursion call of commit_action between UndoRedo/EditorUndoRedoManager classes). There is still a bit buggy cuz it generates two Set calls (instead of one) and pushes them to history, but this prevents crashing at least.

[adamscott]

Looks fine to me.

[KoBeWi]

I looked into the issue and I can't find what causes the recursion. There is a UndoRedo set on ShaderGlobalsEditorInterface, but where it's triggered? The "Set Shader Global Variable" action only sets ProjectSettings.

EDIT:
I think the cause of the issue is that block_update is not functional (due to unexpected set).

Adding

if (block_update) {
	return false;
}

at the beginning also fixes the issue.

[Chaosus]

@KoBeWi You definitely need to check it with a debugger:

if (block_update) {
	return false;
}

This works but this less elegant (and lead to the same result)

[KoBeWi]

Eh, something is fishy here. This fixes the immediate crash, but you can still crash if you mess with undo/redo. Although for whatever reason you can't normally undo setting shader global, you need to use History dock.

Here's stack trace for the double set that causes the recursion:

ShaderGlobalsEditorInterface::_set(const StringName & p_name, const Variant & p_value) Line 84 (c:\Users\Tomek\Desktop\Godot\godot\editor\shader_globals_editor.cpp:84)
ShaderGlobalsEditorInterface::_setv(const StringName & p_name, const Variant & p_property) Line 70 (c:\Users\Tomek\Desktop\Godot\godot\editor\shader_globals_editor.cpp:70)
Object::set(const StringName & p_name, const Variant & p_value, bool * r_valid) Line 293 (c:\Users\Tomek\Desktop\Godot\godot\core\object\object.cpp:293)
UndoRedo::_process_operation_list(List<UndoRedo::Operation,DefaultAllocator>::Element * E) Line 353 (c:\Users\Tomek\Desktop\Godot\godot\core\object\undo_redo.cpp:353)
UndoRedo::_redo(bool p_execute) Line 77 (c:\Users\Tomek\Desktop\Godot\godot\core\object\undo_redo.cpp:77)
UndoRedo::commit_action(bool p_execute) Line 298 (c:\Users\Tomek\Desktop\Godot\godot\core\object\undo_redo.cpp:298)
EditorUndoRedoManager::commit_action(bool p_execute) Line 235 (c:\Users\Tomek\Desktop\Godot\godot\editor\editor_undo_redo_manager.cpp:235)
ShaderGlobalsEditorInterface::_set(const StringName & p_name, const Variant & p_value) Line 119 (c:\Users\Tomek\Desktop\Godot\godot\editor\shader_globals_editor.cpp:119)
ShaderGlobalsEditorInterface::_setv(const StringName & p_name, const Variant & p_property) Line 70 (c:\Users\Tomek\Desktop\Godot\godot\editor\shader_globals_editor.cpp:70)
Object::set(const StringName & p_name, const Variant & p_value, bool * r_valid) Line 293 (c:\Users\Tomek\Desktop\Godot\godot\core\object\object.cpp:293)
UndoRedo::_process_operation_list(List<UndoRedo::Operation,DefaultAllocator>::Element * E) Line 353 (c:\Users\Tomek\Desktop\Godot\godot\core\object\undo_redo.cpp:353)
UndoRedo::_redo(bool p_execute) Line 77 (c:\Users\Tomek\Desktop\Godot\godot\core\object\undo_redo.cpp:77)
UndoRedo::commit_action(bool p_execute) Line 298 (c:\Users\Tomek\Desktop\Godot\godot\core\object\undo_redo.cpp:298)
EditorUndoRedoManager::commit_action(bool p_execute) Line 235 (c:\Users\Tomek\Desktop\Godot\godot\editor\editor_undo_redo_manager.cpp:235)
EditorInspector::_edit_set(const String & p_name, const Variant & p_value, bool p_refresh_all, const String & p_changed_field) Line 3644 (c:\Users\Tomek\Desktop\Godot\godot\editor\editor_inspector.cpp:3644)
EditorInspector::_property_changed(const String & p_path, const Variant & p_value, const String & p_name, bool p_changing, bool p_update_all) Line 3663 (c:\Users\Tomek\Desktop\Godot\godot\editor\editor_inspector.cpp:3663)
call_with_variant_args_helper<EditorInspector,String const &,Variant const &,String const &,bool,bool,0,1,2,3,4>(EditorInspector * p_instance, void(EditorInspector::*)(const String &, const Variant &, const String &, bool, bool) p_method, const Variant * * p_args, Callable::CallError & r_error, IndexSequence<0,1,2,3,4> __formal) Line 262 (c:\Users\Tomek\Desktop\Godot\godot\core\variant\binder_common.h:262)
call_with_variant_args<EditorInspector,String const &,Variant const &,String const &,bool,bool>(EditorInspector * p_instance, void(EditorInspector::*)(const String &, const Variant &, const String &, bool, bool) p_method, const Variant * * p_args, int p_argcount, Callable::CallError & r_error) Line 377 (c:\Users\Tomek\Desktop\Godot\godot\core\variant\binder_common.h:377)
CallableCustomMethodPointer<EditorInspector,String const &,Variant const &,String const &,bool,bool>::call(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 105 (c:\Users\Tomek\Desktop\Godot\godot\core\object\callable_method_pointer.h:105)
Callable::callp(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 51 (c:\Users\Tomek\Desktop\Godot\godot\core\variant\callable.cpp:51)
CallableCustomBind::call(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 100 (c:\Users\Tomek\Desktop\Godot\godot\core\variant\callable_bind.cpp:100)
Callable::callp(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 51 (c:\Users\Tomek\Desktop\Godot\godot\core\variant\callable.cpp:51)
Object::emit_signalp(const StringName & p_name, const Variant * * p_args, int p_argcount) Line 1047 (c:\Users\Tomek\Desktop\Godot\godot\core\object\object.cpp:1047)
EditorProperty::emit_changed(const StringName & p_property, const Variant & p_value, const StringName & p_field, bool p_changing) Line 120 (c:\Users\Tomek\Desktop\Godot\godot\editor\editor_inspector.cpp:120)
EditorPropertyCheck::_checkbox_pressed() Line 733 (c:\Users\Tomek\Desktop\Godot\godot\editor\editor_properties.cpp:733)
call_with_variant_args_helper<EditorPropertyCheck>(EditorPropertyCheck * p_instance, void(EditorPropertyCheck::*)() p_method, const Variant * * p_args, Callable::CallError & r_error, IndexSequence<> __formal) Line 267 (c:\Users\Tomek\Desktop\Godot\godot\core\variant\binder_common.h:267)
call_with_variant_args<EditorPropertyCheck>(EditorPropertyCheck * p_instance, void(EditorPropertyCheck::*)() p_method, const Variant * * p_args, int p_argcount, Callable::CallError & r_error) Line 377 (c:\Users\Tomek\Desktop\Godot\godot\core\variant\binder_common.h:377)
CallableCustomMethodPointer<EditorPropertyCheck>::call(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 105 (c:\Users\Tomek\Desktop\Godot\godot\core\object\callable_method_pointer.h:105)
Callable::callp(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 51 (c:\Users\Tomek\Desktop\Godot\godot\core\variant\callable.cpp:51)
Object::emit_signalp(const StringName & p_name, const Variant * * p_args, int p_argcount) Line 1047 (c:\Users\Tomek\Desktop\Godot\godot\core\object\object.cpp:1047)
Object::emit_signal<>(const StringName & p_name) Line 865 (c:\Users\Tomek\Desktop\Godot\godot\core\object\object.h:865)
BaseButton::_pressed() Line 139 (c:\Users\Tomek\Desktop\Godot\godot\scene\gui\base_button.cpp:139)
BaseButton::on_action_event(Ref<InputEvent> p_event) Line 174 (c:\Users\Tomek\Desktop\Godot\godot\scene\gui\base_button.cpp:174)
BaseButton::gui_input(const Ref<InputEvent> & p_event) Line 69 (c:\Users\Tomek\Desktop\Godot\godot\scene\gui\base_button.cpp:69)
Control::_call_gui_input(const Ref<InputEvent> & p_event) Line 1710 (c:\Users\Tomek\Desktop\Godot\godot\scene\gui\control.cpp:1710)
Viewport::_gui_call_input(Control * p_control, const Ref<InputEvent> & p_input) Line 1327 (c:\Users\Tomek\Desktop\Godot\godot\scene\main\viewport.cpp:1327)
Viewport::_gui_input_event(Ref<InputEvent> p_event) Line 1601 (c:\Users\Tomek\Desktop\Godot\godot\scene\main\viewport.cpp:1601)
Viewport::push_input(const Ref<InputEvent> & p_event, bool p_local_coords) Line 2783 (c:\Users\Tomek\Desktop\Godot\godot\scene\main\viewport.cpp:2783)
Window::_window_input(const Ref<InputEvent> & p_ev) Line 1090 (c:\Users\Tomek\Desktop\Godot\godot\scene\main\window.cpp:1090)
call_with_variant_args_helper<Window,Ref<InputEvent> const &,0>(Window * p_instance, void(Window::*)(const Ref<InputEvent> &) p_method, const Variant * * p_args, Callable::CallError & r_error, IndexSequence<0> __formal) Line 262 (c:\Users\Tomek\Desktop\Godot\godot\core\variant\binder_common.h:262)
call_with_variant_args<Window,Ref<InputEvent> const &>(Window * p_instance, void(Window::*)(const Ref<InputEvent> &) p_method, const Variant * * p_args, int p_argcount, Callable::CallError & r_error) Line 377 (c:\Users\Tomek\Desktop\Godot\godot\core\variant\binder_common.h:377)
CallableCustomMethodPointer<Window,Ref<InputEvent> const &>::call(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 105 (c:\Users\Tomek\Desktop\Godot\godot\core\object\callable_method_pointer.h:105)
Callable::callp(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 51 (c:\Users\Tomek\Desktop\Godot\godot\core\variant\callable.cpp:51)
DisplayServerWindows::_dispatch_input_event(const Ref<InputEvent> & p_event) Line 2152 (c:\Users\Tomek\Desktop\Godot\godot\platform\windows\display_server_windows.cpp:2152)
DisplayServerWindows::_dispatch_input_events(const Ref<InputEvent> & p_event) Line 2117 (c:\Users\Tomek\Desktop\Godot\godot\platform\windows\display_server_windows.cpp:2117)
Input::_parse_input_event_impl(const Ref<InputEvent> & p_event, bool p_is_emulated) Line 663 (c:\Users\Tomek\Desktop\Godot\godot\core\input\input.cpp:663)
Input::flush_buffered_events() Line 886 (c:\Users\Tomek\Desktop\Godot\godot\core\input\input.cpp:886)
DisplayServerWindows::process_events() Line 1846 (c:\Users\Tomek\Desktop\Godot\godot\platform\windows\display_server_windows.cpp:1846)
OS_Windows::run() Line 1011 (c:\Users\Tomek\Desktop\Godot\godot\platform\windows\os_windows.cpp:1011)
widechar_main(int argc, wchar_t * * argv) Line 181 (c:\Users\Tomek\Desktop\Godot\godot\platform\windows\godot_windows.cpp:181)
_main() Line 203 (c:\Users\Tomek\Desktop\Godot\godot\platform\windows\godot_windows.cpp:203)

Changing the property triggers inspector action which then callbacks to ShaderGlobalsEditorInterface and sets the value again. The reason for error is that another UndoRedo action is started while the previous one is being commited. I tested this recently and it would lead to the same result with the old UndoRedo system, so this code was borked from the beginning (or at least since someone messed up undo actions).

Anyway, right now it's impossible to edit the values at all, so the PR is probably ok for now.

[akien-mga]

Thanks!