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

Scene Dock script button unnecessarily restrictive and breaks backward compatibility #87383

Closed
Naros opened this issue Jan 19, 2024 · 3 comments · Fixed by #88288
Closed

Scene Dock script button unnecessarily restrictive and breaks backward compatibility #87383

Naros opened this issue Jan 19, 2024 · 3 comments · Fixed by #88288

Comments

@Naros
Copy link
Contributor

Naros commented Jan 19, 2024

Tested versions

4.3.dev2

System information

Windows 11

Issue description

In Godot 4.2 and earlier, we relied on the EditorPlugin::_edit function call to open our Visual Scripting script attached to nodes within the scene in our GDExtension Plug-In.

In Godot 4.3, the change #84284 broke this functionality. The change unnecessarily restricts implementations to having to register a ScriptEditorBase implementation; however, for GDExtensions, there is currently not an exposed way to contribute these, and so this breaks our user's ability to interact with the SceneTree Dock and to quickly jump from a node to a visual script that is attached.

Steps to reproduce

  1. Use Godot 4.3.dev2
  2. Install Orchestrator Plugin https://github.com/Vahera/godot-orchestrator/releases (2.0.dev1)
  3. Create an orchestration script, attaching it to a scene node in any scene.
  4. Attempt to click on the script button next to the scene node, plugin does not react and show the graph window.

Minimal reproduction project (MRP)

N/A

@Mickeon
Copy link
Contributor

Mickeon commented Jan 19, 2024

I wonder if there's an elegant solution to fix this and keep the PR's quality-of-life change. Although, on the surface it sounds better to revert #84284...

@Naros
Copy link
Contributor Author

Naros commented Jan 19, 2024

I wonder if there's an elegant solution to fix this and keep the PR's quality-of-life change. Although, on the surface it sounds better to revert #84284...

The following idea works and restores the behavior, not sure if it would be acceptable though as it seems a bit hacky:

void SceneTreeDock::_script_open_request(const Ref<Script> &p_script) {
	const int plugin_count = EditorNode::get_singleton()->get_editor_data().get_editor_plugin_count();
	for (int i = 0; i < plugin_count; i++) {
		EditorPlugin* plugin = EditorNode::get_singleton()->get_editor_data().get_editor_plugin(i);
		ScriptEditorPlugin* script_editor_plugin = Object::cast_to<ScriptEditorPlugin>(plugin);
		if (!script_editor_plugin && plugin->handles(p_script.ptr())) {
			EditorNode::get_singleton()->edit_resource(p_script);
			return;
		}
	}
	if (ScriptEditor::get_singleton()->edit(p_script, true)) {
		EditorNode::get_singleton()->editor_select(EditorNode::EDITOR_SCRIPT);
	}
}

The ideal solution here would likely be to expose the bits to GDExtension necessary to register ScriptEditorBase functions in the same way that C++ modules can; however, at least for us, this would mean the code would be substantially different between 4.2 and 4.3 and frankly that seems a bit extreme in a minor release as a result of 3 lines of editor code changes.

@Mickeon
Copy link
Contributor

Mickeon commented Jan 22, 2024

The following idea works and restores the behavior, not sure if it would be acceptable though as it seems a bit hacky:

Hey. If it actually does fix it, it does not hurt to open a PR with it. It's a regression that does break compatibility to an extent, so it's fairly high priority. At worst it will not be merged as is, at best a better solution may be proposed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Very Bad
3 participants