Closed Zylann closed 9 months ago
if I select the sprite and then click on the "Edit Texture" button, I get an error:
https://github.com/godotengine/godot/pull/73512#issuecomment-1435453732
Yet another note after seeing this suggestion https://github.com/godotengine/godot/issues/40166#issuecomment-1213573494:
If I provide true
as the last argument of inspect_object
, which the doc describes as:
If inspector_only is true, plugins will not attempt to edit object.
Then my plugin's UI gets closed, as EditorNode::push_item
still calls _edit_current
and then make_visible(false)
and edit(null)
. It didn't use to do that (in fact that was the point of it, seems?). This might have been a workaround since I saw other plugins inspect "sub-objects" to allow users to edit them via the inspector usin this boolean, but I had no luck trying that, I dont understand this option...
I searched how the VisualShaderEditorPlugin handles editing sub-resources inside nodes with the inspector, without getting bothered, and it uses InspectorDock::get_inspector_singleton()->edit(p_resource.ptr());
, which is not available to scripts.
I got a similar bug today, the engine sunddely crashes when I click on the TileMap node. =/
ERROR: The TileSetAtlasSource atlas has no tile at (2, 1). at: (scene\resources\tile_set.cpp:4515) ERROR: Condition "plugins_list.has(p_plugin)" is true. at: EditorPluginList::add_plugin (editor\editor_node.cpp:8152)
I am also getting plugins_list.has(p_plugin)
errors with a TileSet editor inspector plugin. I'm having difficulty pinpointing what's causing it because my code isn't doing anything at the time the error occurs. The error appears when a user opens the TileSet bottom editor. I have placed a persistent helper node under base_control
, but the plugin script doesn't return true from _can_handle()
until it reaches AtlasTileProxyObject
, which is after the error occurs.
In my case, the error is not associated with crashes or any other obvious problems.
Never mind! A new project with no plugins still prints this error when opening the TileSet editor, so it's coming from the engine itself.
editor/editor_node.cpp:8152 - Condition "plugins_list.has(p_plugin)" is true.
I'm seeing the same error after doing tile set/map for a moment. Using Godot v4.0.stable.official [92bee43ad]. No plugins added to the project.
For the record I still see this kind of errors in the editor once in a while.
Godot 4.1.1. No plugins, worked with TileMaps for a day learning the engine. This is all that shows up when starting up my project:
Godot Engine v4.1.1.stable.mono.official (c) 2007-present Juan Linietsky, Ariel Manzur & Godot Contributors.
modules/gltf/register_types.cpp:73 - Blend file import is enabled in the project settings, but no Blender path is configured in the editor settings. Blend files will not be imported.
--- Debug adapter server started ---
--- GDScript language server started ---
editor/editor_node.cpp:8195 - Condition "plugins_list.has(p_plugin)" is true.
editor/editor_node.cpp:8195 - Condition "plugins_list.has(p_plugin)" is true.
editor/editor_node.cpp:8195 - Condition "plugins_list.has(p_plugin)" is true.
editor/editor_node.cpp:8195 - Condition "plugins_list.has(p_plugin)" is true.
^ This is fixed in 4.2.
No longer crashes on master. I guess the issue is resolved? You can't handle the same object by two plugins at the same time, this is expected limitation.
Closing per the comment above. Please let me know if the issue is still reproduceable for you in 4.2. We'll probably need an updated MRP if it is.
You can't handle the same object by two plugins at the same time,
Sorry but this is not the same:
This plugin handles BOTH Sprite2D and Texture2D.
It's ONE plugin handling two TYPES of objects. But I guess if you also include the editor's core plugins, that also makes two plugins handling the same classes (which prevents any extensibility/custom sprites??)
But I guess it's also documented as not being supported. But there is no explanation about how else it should be done... this is quite a severe limitation.
I've been using that for several reasons:
make_visible
is litterally meant to do so, otherwise it's unclear to me how we're supposed to implement it).Cast2DEditorPlugin
, EditorTextureTooltipPlugin
, Polygon3DEditorPlugin
(indirectly), ScriptEditorPlugin
, ShaderEditorPlugin
, SpriteFramesEditorPlugin
, TilesEditorPlugin
(although maybe they arent edited at the same time, but it's also unclear what "same time" actually means)Condition "plugins_list.has(p_plugin)" is true.
keeps getting printed, seemingly causing no issues, but then creating noise in every bug report, annoying users, while also not providing any information about its real cause, not even which plugins, and it doesn't always print, so it's also hard to link it directly to a plugin handling more than one type.I'm still working on 4.1.3 where the error still prints, although I havent experienced crashes in my plugins (therefore I focused my limited time on more important things). Will eventually start testing 4.2, but regardless, handling the limitation is unclear at the moment.
I'm considering to stop implementing make_visible
entirely, but then I don't know how I can hide tools at the right moment without relying on make_visible
, handles
and edit
(which at this point have all become unreliable for my use case)
You can manually determine edited object by using selection_changed()
or edited_object_changed()
signals. This way you don't need to rely on handles()
, edit()
etc., so your editor will not conflict with other editors. However you also can't take advantage of overlay draw and input, not sure how these can be handled manually. But that's where having 2 plugins would cause conflicts, because they'd both try to draw and handle input.
Unfortunately, EditorSelection
is only about nodes. I have resources too. Also, how is edited_object_changed
behaving with sub-inspectors? It's ambiguous because they are technically open at the same time.
It's emitted only for the main inspector object.
In GDScript "addon" plugins, you can't have more than one EditorPlugin in your cfg.
IIRC there is a proposal for more plugins per addon. It shouldn't be difficult to implement. That's how TileMap issues were fixed. There is 2 separate handler plugins now, dependent on each other to make sure you can edit both without problems.
it's also unclear what "same time" actually means)
The editor has a few editing contexts - the scene tree, inspector and sub-inspectors. They can operate independently, i.e. call handles()
and edit()
each for different objects. "same time" means handling something by a single plugin in multiple contexts at once. If your plugin handles a node and this node has a sub-resource handled by the same plugin then it's not allowed. If you want to edit 2 objects at the same time using the same plugin, you could make handles()
return false
when one object is already edited.
I dont know how I'm supposed to refactor things
Hmm, maybe I could look into your plugin to see how things are implemented and give some advice.
My GDScript plugin isn't directly affected because it only handles nodes, however I have the issue in my voxel terrain module, which nests resources inside nodes at multiple levels.
If you want to have a look (although there is a lot going on):
For context, here is a screenshot showing one example of nesting:
Here showing the case of two of the plugins in my module: The terrain plugin, which preferably should keep its controls shown while the user tweaks the generator, and the generator plugin allowing to edit a graph, which itself contains "sub-objects" that use the inspector as property editor, almost always causing the error to print when alternating between nodes of the graph and the graph resource itself.
I rely on make_visible
, which means I ended up doing this in VoxelTerrainEditorPlugin
:
https://github.com/Zylann/godot_voxel/blob/c8baeb95cecc4e8f23ce1dfc3bec96846967d8a7/editor/terrain/voxel_terrain_editor_plugin.cpp#L139C54-L167
And this in VoxelGraphEditorPlugin
:
https://github.com/Zylann/godot_voxel/blob/c8baeb95cecc4e8f23ce1dfc3bec96846967d8a7/editor/graph/voxel_graph_editor_plugin.cpp#L58
There is of course more to it, notably the fact edit(null)
is called either way when I switch between two handled objects, which means I get streams of unnecessary make_visible(false); make_visible(true); ...
and some issues with saving. but these aren't directly related. Also very often when I deselect the terrain node to edit something completely unrelated, and select back the terrain, it also re-selects every nested inspector (sometimes even across restarts), causing further occurrences of the error.
Note: I'm intentionally not using any API that isn't available to GDExtensions. There are tricks the core editor does I can't rely on.
I did a quick patch with 2 changes: 0001-makeshift-patch.patch
handles()
does not return true if something is already editedThe code is hacky, but you get the idea. With this fix I am able to see the Terrain button and graph editor at the same time without errors, so I think it works. I don't know the plugin enough to test it properly.
Had a look at the patch, it really looks like a hack. Seeing stuff like this in those functions makes things confusing compared to what they are meant for. Notably handles
, which in theory should only depend on the passed object, and not some state of the plugin, which now results in race conditions.
The hack in make_visible
is now also a source of race conditions. Everytime there is a call_deferred
, it opens the door for more problems in the future.
I wouldn't have come up with this idea because it goes against what the methods are meant to do and seems to rather exploit Godot's implementation details (which I'm not much aware of, I dont often inspect the editor's core logic).
I feel like Godot should have cleaner ways to code things for contextes like this. It constantly happens for me and it's tiring to keep adding hacks because it feels like I'm loosing control and could break some random day. But not sure what would have to change... as I described earlier, the logic seems too restrictive, partially ill-formed or unclear (both in docs and error messages). Because even though the API could still assume only one "thing" is edited at a time, in practice it doesn't always exist alone in a void, and may be nested in related resources, nodes or objects, which may be part of a whole context with interacting UIs. The sole fact you can have a node selected in the scene tree and a different object edited in the inspector is an example of this, but it goes beyond that of course.
I gave your patch a try, and seems to work when I select a graph resource in a terrain.
Although I suppose the same hack must be added in other places as well: errors keep happening after I select nodes of the graph: clicking on a node uses the inspector to "edit" a custom object representing it (mostly because we can't have extra inspectors). Then clicking on the background is supposed to "edit" the graph resource itself in the inspector. But when the graph gets edited that way, the error occurs everytime editor\editor_node.cpp:8240 - Condition "plugins_list.has(p_plugin)" is true.
. I suppose that's because the graph editor plugin has to handle both graph resource and graph nodes to function properly but Godot doesnt like that (no idea why tho), but again I'm really confused about how else to cleanly handle that without messing up all the UIs that are supposed to be shown in this nested editing context.
Actually, that second case I just described is indirectly triggered by my own code, which simply needs to use the inspector to edit a specific "sub-object" of my graph resource, while keeping the graph editing context: https://github.com/Zylann/godot_voxel/blob/cee2a86af4e6e9d76398856c868a20cf27b7052f/editor/graph/voxel_graph_editor_plugin.cpp#L195
get_editor_interface()->inspect_object(*wrapper);
Therefore, there might be a simpler solution than your hack here.
I noticed this had a parameter p_inspector_only
, which the doc describes:
If inspector_only is true, plugins will not attempt to edit object.
Which sounds like it would solve this specific case, and I would be able to remove this sub-object class from handles
, getting rid of the error I had before.
But when I provide true
, it makes no difference... edit(null)
and make_visible(false)
are still called, so selecting a graph node now hides the whole graph editor (so plugins are NOT ignored), which defeats the whole point for me.
Though in this case I could workardound with this:
_ignore_edit_null = true;
_ignore_make_visible = true;
get_editor_interface()->inspect_object(*wrapper, String(), true);
_ignore_edit_null = false;
_ignore_make_visible = false;
Which luckily seems to be working because neither edit
nor make_visible
are deferred calls. (but of course using inspector prev/next arrows still break and hide the editor)
Can you check if this patch fixes your issue with inspector_only
?
patch.txt
Though probably more changes to handle sub-resources.
The limitation of one object per plugin/context can't be lifted, because it makes the editor more predictable when unediting objects. However, assuming my above patch is working (not tested), we could have something like is_editing()
method for EditorPlugin, which would help to determine whether to edit the object or just open it in the inspector.
FYI, these are all the changes I went with for working around the errors so far:
https://github.com/Zylann/godot_voxel/commit/f86ab5cab4db1dd8a5a64b5b0f55aefe24443750 https://github.com/Zylann/godot_voxel/commit/743984b3ae4bfa5c6acfb098486b8c2814d6b601
I noticed that completely removing the "side handling" for the terrain editor plugin actually didn't have harmful effects [anymore?] (at least in use cases I tested). I was quite curious because I thought selecting a sub-inspector would cause edit(null)
and make_visible(false)
called on the terrain plugin, but it didnt (which is good for me, but I don't know if that's by design in Godot or I'm accidentally exploiting a bug).
I tested your patch: VoxelGraphEditorPlugin
still receives make_visible(false)
and edit(null)
when selecting nodes of the graph. Although, that doesn't happen when I click on the background (which selects the graph itself). Note, the code I use to select the graph again is still using inspect_object
without specifying p_inspector_only
. Also, inspector prev/next arrows still do the hiding.
(which is good for me, but I don't know if that's by design in Godot or I'm accidentally exploiting a bug).
This is likely result of the editing context split, which happened in some 4.0 alpha IIRC. Scene tree and editor properties are separate editing contexts.
I tested your patch: VoxelGraphEditorPlugin still receives make_visible(false) and edit(null) when selecting nodes of the graph.
I'd have to look into it, but the module doesn't compile on master.
I'd have to look into it, but the module doesn't compile on master.
Should be fixed now
I tested the graph in the newest version. Clicking a graph node will open it in the inspector, with no error printed. Not sure what's the problem. The Terrain button in the toolbar is still visible, but VoxelTerrain is not in the inspector, because it's taken by graph node. That looks like expected behavior.
I tested the graph in the newest version. Clicking a graph node will open it in the inspector, with no error printed
Forgot to mention: that's because there is still a workaround to prevent the issue from being noticed.
See this: https://github.com/Zylann/godot_voxel/blob/7ebf64cd7e9d5e95a7c2e7e4f13010091c7e72eb/editor/graph/voxel_graph_editor_plugin.cpp#L81-L84
When I say that edit(null)
and make_visible(false)
are still called even with p_inspector_only=true
, I figure this out by enabling verbose output (-v
) in my build, so I can see whether the problem still exist or not with the debug print (then my workaround code makes it so it looks like it doesnt happen).
VoxelTerrain is not in the inspector, because it's taken by graph node. That looks like expected behavior.
That's expected.
When you click GraphNode, it will be edited in the inspector. Any plugin that uses inspector will lose its editing context and receive edit(null)
. If you want to keep graph editor and edit the graph node, you need to persist the bottom editor. Either permanently, like Shader Editor, or temporarily, using the recently added self-owning mechanism: #81523
Is it adressing both calls to edit(null)
and make_visible(false)
?
Unfortunately I don't think I can use the "self owning" thing yet. It's not documented, not exposed to extensions, and when I look at the changes in the theme editor, it uses APIs that are also not exposed. Also not sure what get_next_edited_object
means? Why wouldn't this just be passed to can_auto_hide
in order to react to what's being edited?
These two methods are called together. At least in the case that affects your plugin. Also the API is not exposed, because it was added to fix an internal editor bug. So far no user has expressed a need to have it available, because it's quite advanced and corner case.
@Zylann mentioned this error occurs when handling the same object by two different plugins, but I have the exact same error when handling subresources with just one plugin.
In my plugin I override _handles(object)
like so:
func _handles(object: Object) -> bool:
return object is Resource
But whenever I select a subresource in the inspector, I get:
editor/editor_node.cpp:8240 - Condition "plugins_list.has(p_plugin)" is true.
What's more, the subresource closes in the inspector, preventing me to edit it any further.
Well that error appears when you try to edit multiple objects by the same plugin within the same editor context, which is very likely to happen when your plugin handles any resource.
Godot version
Godot 4 rc2
System information
Windows 10 64 bits NVIDIA GeForce GTX 1060
Issue description
I was trying to reproduce a nasty error I started having since stuff got changed in some areas of editor plugins, but in RC2 instead of reproducing my issue, I ended up finding this crash. So I thought I'd report it...
I wrote a simple test plugin that adds a bottom panel with two buttons:
This plugin handles BOTH
Sprite2D
andTexture2D
.Steps to reproduce
After testing the plugin a bit, I turned it off, then clicked on the
Script
tab: Godot crashes. No information popup or logs, just shuts down.Minimal reproduction project
EditorPluginEditAnotherHandled.zip
This project has the plugin enabled by default.
Secondary note: I was told that one of the changes to EditorPlugin was that
_edit(null)
can be called when "there is no longer any selected object handled by this plugin" (seen in the doc). But if you look in the console while alternating the two buttons (which ask the editor to inspect two different objects handled by the plugin), you will notice thatedit(null)
occurs half of the time, I dont understand why.Third note: When testing this repro in master 9c960a8c2494eb826a557a7ffc96dd4547f9d31e, if I select the sprite and then click on the "Edit Texture" button, I get an error:
Which I dont understand either.