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

Add a 'plugin' keyword similar to 'tool' #17592

Closed
RandomShaper opened this issue Mar 17, 2018 · 22 comments
Closed

Add a 'plugin' keyword similar to 'tool' #17592

RandomShaper opened this issue Mar 17, 2018 · 22 comments

Comments

@RandomShaper
Copy link
Member

The problem

While developing plugins you have to add the the tool keyword to every script. The problem is that if you open an scene file belonging to the plugin, it runs and if you save it, you are effectively saving a runtime state of the GUI (lists populated, controls enabled/disabled and so on). Because of that I'm having to remove/re-add tool many times.

I know there is Engine.editor_hint, but I think that allows only to check if your code is running on the editor (both for editing and a running plugin) instead of as part of a running project.

The proposal

Adding a new keyword (let's say plugin) that you can use instead of tool that enables the script only if it's part of a running plugin. In other words, these would be the semantics for all cases:

  • No keyword => Run script only if the project is actually running
  • plugin keyword => Run script only if it has been loaded as part of a running plugin
  • tool keyword => Run script everywhere

I think I can implement this myself if is makes enough sense and there's no a good way of achieving it currently.

@Zylann
Copy link
Contributor

Zylann commented Mar 17, 2018

I design my plugin's UI through the editor, and I thought of this as well. I likely avoided it by chance so far since my UIs are encapsulated and singleton-free. They basically do nothing if they aren't told to do so by their owner (which doesn't exist within the edited scene so the logic stays idle).
Eventually I have some states turning on as the defaults, but I don't care because they are not saved, and they get overrided as soon as the component receives the real data.

Initially I thought to have a way to check in scripts if the scene root was the edited scene root and then discard any logic, but that would be quite cumbersome in addition to the is_editor_hint()...

Run script only if it has been loaded as part of a running plugin

I guess this is known by the editor, if the scene gets instanced outside of an edited scene root?

I think this problem was never encountered before because so far, all editor UI was created by code... but if you think about it, every single C++ node in Godot is implemented in the same situation: they all run in all cases (there is no tool concept in C++). So with the same mindset, you could manage to use tool and still have no issues with plugin scenes. Basically, the problem can also be solved with scene/code design, even if the scene is constructed in a .tscn.

This mindset for example implies that you should not connect internal signals by code in _ready, but in the .tscn, otherwise you could save duplicate connections. It's a limitation, but once you know it it makes sense.

Do you have an example of an impossible/annoying situation?

@vnen
Copy link
Member

vnen commented Mar 25, 2018

I bumped into something like this too. I have to carefully not save the scene or delete everything from an ItemList that was populated by the script before saving. I also have issues with stuff that needs the plugin reference which is not available in design-time.

Signals aren't really a problem unless you connect them as persistent (which wouldn't make much sense anyway).

The alternative is not to use the tree callbacks (_ready, {enter,exit}_tree) to avoid anything being called in editor. But this doesn't look idiomatic to me, it's just a workaround. I add and remove stuff from the tree in many places and having to remember to call the initialization function every time is error-prone (especially because it's not need for game scenes).

I would be satisfied by something to editor_hint that says whether the scene is being edited or running in the editor. I like the plugin keyword though, it looks cleaner and avoids a workaround.

@RandomShaper
Copy link
Member Author

@Zylann, I agree that you can split responsibilities in such a way that you avoid the issue, if I have got your point. Probably doable in every situation, but for simple plugins you may prefer to mix UI and logic.

@RandomShaper
Copy link
Member Author

Now I'm wondering if it wouldn't be better to let plugin writers have something like tool # plugin in order to preserve compatibility.

@vnen
Copy link
Member

vnen commented Mar 26, 2018

I would just allow both plugin and tool for plugins. I imagine that some scripts might run both as plugins and as editor tools.

I'm still not sure if a new keyword is the best solution (I'm always wary of adding stuff to GDScript). I can't think of anything else, though.

@bojidar-bg
Copy link
Contributor

We might have it as tool(PLUGIN), similar to export(int, ENUM)

@TheGreatSage
Copy link

I think this is needed as well, I'm a frequent user of ReferenceRect when designing a GUI so I can visualize empty spaces. However, its based on the Engine.editor_hint so when placed in the gui running on a plugin the red box is still visible.

Having a plugin_hint equivalent that goes along with a plugin keyword would would help fix annoyances like this.

@Zylann
Copy link
Contributor

Zylann commented Apr 12, 2018

I'm now having this problem in a constraining situation.

I have a dialog window that contains a viewport, and at runtime, I assign its ViewportTexture to a TextureRect in the window. I also modify materials that are used to render that TextureRect too.
And now I end up saving a ViewportTexture inside the .tscn file, and the generated material too.

I cannot think of a workaround that works by design this time. The workaround I have currently just makes things a bit more complicated (having a boolean set externally, or create the TextureFrame at runtime so that it's not saved)

EDIT: I found a better workaround:

	if is_inside_tree() and get_tree().edited_scene_root:
		return

This way I can see the tool parts of my plugin that don't cause issues in the edited scene, and I can disable other parts.

EDIT: turns out it... does not work??
Of course it doesn't... because even though it's the edited scene root, the WHOLE editor is ONE SceneTree, it seems... so unless there is a way to identify the root of the ACTUAL edited scene, I'm blocked again.

I changed my workaround to be:

func is_in_edited_scene():
	# TODO https://github.com/godotengine/godot/issues/17592
	return is_inside_tree() and ((get_parent() is Control) == false)

This is obviously very fragile. I'd like a correct version of this to be available somehow :/

@willnationsdev
Copy link
Contributor

@Zylann The EditorInterface class has a get_edited_scene_root() method which sounds like it would give you exactly what you were looking for. No? If you emit a signal that connects to your EditorPlugin, then it can tell your node what's going on.

@Zylann
Copy link
Contributor

Zylann commented Jun 15, 2018

@willnationsdev but that's the problem. It's on EditorInterface, which I can't access from any node of my tool's scene when it is inside the edited scene...

@willnationsdev
Copy link
Contributor

Would you prefer that get_editor_interface be accessible to every Node, but return null for non-tool scripts? That'd be cool...

@Zylann
Copy link
Contributor

Zylann commented Jun 15, 2018

@willnationsdev probably not something that extreme...

the WHOLE editor is ONE SceneTree, it seems... so unless there is a way to identify the root of the ACTUAL edited scene, I'm blocked again.

What I would need would be just a way to check if my node inside an edited scene. edited_scene_root sounded like a very good candidate, but I'm not sure why I could not use it here... perhaps it needs to be contextual, without using get_tree() explicitely (which gives you the root of the editor, not the edited scene). Being contextual means it would looks upwards the parents instead of starting from the root.

@willnationsdev
Copy link
Contributor

@Zylann is this not part of a plugin? Why not ping the EditorPlugin via signal like I suggested? Is that not an option, or do you find it tedious or something?

@Zylann
Copy link
Contributor

Zylann commented Jun 15, 2018

@willnationsdev this is not an option because I can't access the EditorPlugin in the first place (and normally shouldn't). Also, giving access to it in the Node class isn't trivial because you would have to identify which plugin you want somehow. There might be a way by changing the handles function of my own plugin to handle every possible node so that my plugin can parse all nodes I select and test which ones are from my plugin and tell them to do stuff, but that's incredibly tedious and hacky for the given use case.

Edit:
Got this issue again and my workaround was again to check what is the parent of the node with if get_parent() is Viewport, but still fragile because it doesn't explicitely mean it's the edited scene root and I just exploit the fact my scene is never used as direct child of a viewport.

@ghost
Copy link

ghost commented Dec 10, 2018

Been using the following. So far haven't had issues with it since I call it in the root node scripts that need it, but maybe it has holes in it.

func scene_being_edited(scene):
	return scene == get_tree().edited_scene_root

@Zylann
Copy link
Contributor

Zylann commented Dec 10, 2018

OMG, for some reason I thought this was a boolean. That will improve my utility function to get that info, then :)

static func is_in_edited_scene(node):
	if not node.is_inside_tree():
		return false
	var edited_scene = node.get_tree().edited_scene_root
	if node == edited_scene:
		return true
	while node.get_parent() != null:
		node = node.get_parent()
		if node == edited_scene:
			return true
	return false

@ghost
Copy link

ghost commented Dec 10, 2018

is_a_parent_of() works in some of these situations too, and I've been able to avoid those while loops. If it's safe to do so you might even just write it like:

# Haven't tested this.  X)
static func is_in_edited_scene(node):
	var edited_scene = node.get_tree().edited_scene_root
	return node.is_inside_tree() and (node == edited_scene or edited_scene.is_a_parent_of(node))

@Zylann
Copy link
Contributor

Zylann commented Dec 10, 2018

Close :)
Tested this:

static func is_in_edited_scene(node):
	if not node.is_inside_tree():
		return false
	var edited_scene = node.get_tree().edited_scene_root
	if node == edited_scene:
		return true
	return edited_scene != null and edited_scene.is_a_parent_of(node)

Edit: had to also check if the edited_scene is null, that can happen on editor startup if your plugin is active.

@Zylann
Copy link
Contributor

Zylann commented Dec 14, 2018

I just read in #23651 (comment) there is a get_tree()->is_node_being_edited(), present in tool builds only, which is however not exposed to GDScript (also I wonder why it's a SceneTree method Oo)
Which is similarly implemented as:

bool SceneTree::is_node_being_edited(const Node *p_node) const {

	return Engine::get_singleton()->is_editor_hint() && edited_scene_root && (edited_scene_root->is_a_parent_of(p_node) || edited_scene_root == p_node);
}

@ghost
Copy link

ghost commented Dec 15, 2018

The wheel reinvented.

@Calinou
Copy link
Member

Calinou commented May 26, 2020

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.

The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.

If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!

@aftamat4ik
Copy link

Close :) Tested this:

static func is_in_edited_scene(node):
	if not node.is_inside_tree():
		return false
	var edited_scene = node.get_tree().edited_scene_root
	if node == edited_scene:
		return true
	return edited_scene != null and edited_scene.is_a_parent_of(node)

works for me. c# version:

if(SourceNode.IsInsideTree() == false)
    return false;

var EditedScene = SourceNode.GetTree().EditedSceneRoot;
if(SourceNode == EditedScene)
    return true;

return EditedScene != null && EditedScene.IsAncestorOf(SourceNode);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants