Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EditorInspectorPlugin._parse_property gets called on all plugins when inspecting an array inspector, regardless of can_handle #71236

Closed
Zylann opened this issue Jan 11, 2023 · 5 comments · Fixed by #86467

Comments

@Zylann
Copy link
Contributor

Zylann commented Jan 11, 2023

Godot version

Godot 4 beta11

System information

Windows 10 64 bits NVIDIA GeForce GTX 1060

Issue description

I have an inspector plugin that only handles a custom object type, but found out that _parse_property gets called on it every time I open an array inspector. I did not expect that because in can_handle I return false already. But another problem is, there is no way to actually specify if an inspector plugin can handle non-object types.

Another of my inspector plugins does not fail, but instead gets triggered unwantedly: it replaces AABB properties with a custom one that works differently. But as a result, it also triggers in Variant array inspectors where items are AABBs.

I'm not sure if it is a design issue or a documentation issue, because if it is the latter, it means every plugin has to check if the object is valid in this method, regardless of can_handle (I don't know if other methods are in this case too?).

Call stack:

_err_print_error(const char * p_function, const char * p_file, int p_line, const char * p_error, const char * p_message, bool p_editor_notify, ErrorHandlerType p_type) Line 86 (core\error\error_macros.cpp:86)
_err_print_error(const char * p_function, const char * p_file, int p_line, const char * p_error, bool p_editor_notify, ErrorHandlerType p_type) Line 78 (core\error\error_macros.cpp:78)
zylann::voxel::VoxelInstanceLibraryMultiMeshItemInspectorPlugin::parse_property(Object * p_object, const Variant::Type p_type, const String & p_path, const PropertyHint p_hint, const String & p_hint_text, const unsigned int p_usage, const bool p_wide) Line 77 (modules\voxel\editor\instance_library\voxel_instance_library_multimesh_item_inspector_plugin.cpp:77)
EditorInspector::instantiate_property_editor(Object * p_object, const Variant::Type p_type, const String & p_path, const PropertyHint p_hint, const String & p_hint_text, const unsigned int p_usage, const bool p_wide) Line 2444 (editor\editor_inspector.cpp:2444)
EditorPropertyArray::update_property() Line 355 (editor\editor_properties_array_dict.cpp:355)
EditorPropertyArray::_edit_pressed() Line 547 (editor\editor_properties_array_dict.cpp:547)
call_with_variant_args_helper<EditorPropertyArray>(EditorPropertyArray * p_instance, void(EditorPropertyArray::*)() p_method, const Variant * * p_args, Callable::CallError & r_error, IndexSequence<> __formal) Line 267 (core\variant\binder_common.h:267)
call_with_variant_args<EditorPropertyArray>(EditorPropertyArray * p_instance, void(EditorPropertyArray::*)() p_method, const Variant * * p_args, int p_argcount, Callable::CallError & r_error) Line 377 (core\variant\binder_common.h:377)
CallableCustomMethodPointer<EditorPropertyArray>::call(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 105 (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 (core\variant\callable.cpp:51)
Object::emit_signalp(const StringName & p_name, const Variant * * p_args, int p_argcount) Line 1047 (core\object\object.cpp:1047)
Object::emit_signal<>(const StringName & p_name) Line 869 (core\object\object.h:869)
BaseButton::_pressed() Line 139 (scene\gui\base_button.cpp:139)
BaseButton::on_action_event(Ref<InputEvent> p_event) Line 174 (scene\gui\base_button.cpp:174)
BaseButton::gui_input(const Ref<InputEvent> & p_event) Line 69 (scene\gui\base_button.cpp:69)
Control::_call_gui_input(const Ref<InputEvent> & p_event) Line 1736 (scene\gui\control.cpp:1736)
Viewport::_gui_call_input(Control * p_control, const Ref<InputEvent> & p_input) Line 1333 (scene\main\viewport.cpp:1333)
Viewport::_gui_input_event(Ref<InputEvent> p_event) Line 1607 (scene\main\viewport.cpp:1607)
Viewport::push_input(const Ref<InputEvent> & p_event, bool p_local_coords) Line 2801 (scene\main\viewport.cpp:2801)
Window::_window_input(const Ref<InputEvent> & p_ev) Line 1331 (scene\main\window.cpp:1331)

I don't know if it's the only place, but this happens because of this:

prop = EditorInspector::instantiate_property_editor(nullptr, value_type, "", subtype_hint, subtype_hint_string, PROPERTY_USAGE_NONE);

Which then results in this:

EditorProperty *EditorInspector::instantiate_property_editor(Object *p_object, const Variant::Type p_type, const String &p_path, PropertyHint p_hint, const String &p_hint_text, const uint32_t p_usage, const bool p_wide) {
for (int i = inspector_plugin_count - 1; i >= 0; i--) {
inspector_plugins[i]->parse_property(p_object, p_type, p_path, p_hint, p_hint_text, p_usage, p_wide);

Specifically in array and dict sub-inspectors, can_handle is not even checked (and in fact it can't because it only receives objects) then parse_property is called with a null object on ALL inspector plugins. This does not happen for properties of objects.

Steps to reproduce

Make an EditorInspectorPlugin that implements _parse_property, for example:

@tool
extends EditorInspectorPlugin

func _can_handle(object):
	return object is Sprite2D:

func _parse_property(object, type, name, hint_type, hint_string, usage_flags, wide):
	if not (object is Sprite2D):
		push_error(str("Passed ", object, " to inspector plugin that only handles Sprite2D"))
	return false

Make an EditorPlugin to register it.
Enable the plugin.
Create a Line2D node.
In the inspector, add points to the Line2D node, then expand the points property.
Observe errors.

Minimal reproduction project

EditorInspectorPluginVariantType.zip

@Zylann Zylann changed the title EditorInspectorPlugin._parse_property gets called on all plugins when inspecting an array inspector, disregarding can_handle EditorInspectorPlugin._parse_property gets called on all plugins when inspecting an array inspector, regardless of can_handle Jan 11, 2023
@SneaK1ng
Copy link

Maybe should call can_handle() before calling parse_property()?

@Zylann
Copy link
Contributor Author

Zylann commented Feb 20, 2023

Just bumped again into a similar issue, but this time when clicking on a VisualShader Compare node. Apparently my plugin's parse_property is also called for properties of this object even though it does not handle that object...

@uguu-org
Copy link

I also ran into an unexpected call of _parse_property, not with arrays but with AnimationTree. I found that things were fine until I added an AnimationNodeTransition.

Attached is a reduced scene to reproduce the error.
inspector_test.zip

Click on AnimationTree to observe something like these in the output:

Ignored AnimationTree:<AnimationTree#1703608902859> (AnimationTree)
Unexpected call to handle AnimationTree:<AnimationTree#1703608902859>
[]

The first line says _can_handle is called, the next line says _parse_property is still called regardless, and the third line says the calling stack is empty.

@Zylann
Copy link
Contributor Author

Zylann commented Sep 5, 2023

Still hitting this in latest 4.1.1, my inspector plugin parse_property is called for instances of VisualShaderNodeColorFunc when I create a ColorFunc node in a Visual Shader, despite my plugin not handling that type. Everytime I create a node in VisualShader, actually.
The reason is similar to my first post, except there doesn't seem to be any array involved here.

@isaaccp
Copy link
Contributor

isaaccp commented Jan 28, 2024

Still happening at HEAD. I have a custom look up for StringNames in different namespaces and it's impossible to invoke the right one when they are in an Array due to this issue :/

Also, I thought I would work around this issue by returning false in _parse_property(), which should cause the inspector to keep the original property editor instead of replacing it, but it doesn't seem to work like that for Arrays either, I guess due to the same difference in how arrays are treated.

@akien-mga akien-mga added this to the 4.3 milestone Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants