godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.17k stars 98 forks source link

Expose `CreateDialog` class to scripting #7564

Open Nophlock opened 1 year ago

Nophlock commented 1 year ago

Describe the project you are working on

System: Godot v4.1.1.stable - Windows 10.0.19045 - Vulkan (Forward+) - dedicated Radeon RX 570 Series (Advanced Micro Devices, Inc.; 31.0.14051.5006) - AMD Ryzen 5 5600X 6-Core Processor (12 Threads)

I'm working on a small plugin where the user can create new nodes through a button to the scene and then the nodes gets aligned according to the settings the user have made

Describe the problem or limitation you are having in your project

After a long search in the documentation and in the source code itself, I couldn't find a way to open the "Create New Node" Dialog from GDScript. It's actually hard to tell if this is a bug or in some way intended (maybe the CreateDialog-Class cant be used directly?) but it would be weird, since all the other editor related things are usable from the EditorPlugin-Class. I wanted to use this for a custom plugin I'm currently writing, where a button press would then trigger the dialog itself, which leaded me to this issue.

PS: I could be actually really blind and missed it which would be really stupid by me, but I would also be really glad about this :sweat_smile:

Describe the feature / enhancement and how it helps to overcome the problem or limitation

This is mainly a feature for plugins and other code parts which tries to enhance the editor in certain ways. It is specifically useful if a node should be handled differently compared to the normal way of adding an object to the scene (since the dialog would only return the node and not directly add it to the tree).

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Could work like this

@tool
extends EditorPlugin

func _ready():
    var dialog : CreateDialog = get_create_dialog()#maybe get_create_new_node_dialog is more fitting?
    var new_node = dialog.show_dialog()
    # do custom stuff with the new node like adding it to the scene tree or do some preparations before that

If this enhancement will not be used often, can it be worked around with a few lines of script?

I don't think so, but a workaround would be nice (which isn't to recode the entire dialog through GDScript :stuck_out_tongue:)

Is there a reason why this should be core and not an add-on in the asset library?

Everything related is already part of the core, so this should be in core aswell

YuriSizov commented 1 year ago

but it would be weird, since all the other editor related things are usable from the EditorPlugin

Barely anything is exposed to the API from the editor internals. We are very careful with what we expose based on the needs of plugin developers, because exposing everything would make it very hard to develop the editor itself because we'd need to preserve compatibility for the public API.

Nophlock commented 1 year ago

Hey, thanks for the reply.

Regarding code security, I can't really judge how this would affect the overall development. My general thought from looking at the code (without being an expert) was that almost everything seems to be already in place to do this feature (scene_tree_dock.cpp seems to be the only place where the CreateDialog is used). On the other hand, I can already access e.g. the ScriptCreateDialog, so my immediate conclusion was that I can also access the CreateDialog, which, contrary to my expectations, isn't the case. In general, I think there are a number of cases where it would make sense to have access to it from a plugin perspective, since it would allow manipulating the selected object before placing it in the scene, like placing the object at specific positions, or wrapping the object in an additional container that manages the selected node as a parent, or even creating a dialogue with additional node settings before instantiation for example.

YuriSizov commented 1 year ago

Regarding code security, I can't really judge how this would affect the overall development. My general thought from looking at the code (without being an expert) was that almost everything seems to be already in place to do this feature

I wasn't talking about code security. When working on the editor internal classes can be modified to fit the needs of the editor without much of a second thought. However, if the class is exposed to the public API, every change must be done in a compatible way. In other words, if we expose an internal class, every future change to it must keep the current public API the same.

This doesn't mean we can't expose it, I was just correcting your point about everything else being exposed already. This is not the case, and the above is the reason why it's not the case. We need to be very careful when deciding what to expose and put on the public API surface. Because doing so locks everything in.

dalexeev commented 1 year ago

I think we should rename the class if we decide to expose it. For example, to EditorNodeCreateDialog. Yes, ScriptCreateDialog doesn't use the Editor prefix, but it's most likely an unintentional exception.

KoBeWi commented 1 year ago

Technically CreateDialog is a generic object selector, it doesn't need Node as base type. The Create New Resource option also uses CreateDialog.

nlupugla commented 1 year ago

I'm in favor of exposing more editor dialogs, but the consensus seems to be to expose them via EditorInterface. For example, see: https://github.com/godotengine/godot/pull/81655

AnidemDex commented 1 year ago

While this proposal is solved, I made a reimplementation with GDScript

NodeSelector code ```gdscript class NodeSelector extends ConfirmationDialog: var editor_plugin:EditorPlugin var fake_tree:Tree var line_edit:LineEdit var node_path:NodePath func _recursive_create(ref_item:TreeItem, ref_node:Node, root_node:Node) -> void: ref_item.set_text(0, ref_node.name) ref_item.set_icon(0, get_theme_icon(ref_node.get_class(), "EditorIcons")) ref_item.set_metadata(0, root_node.get_path_to(ref_node)) for child in ref_node.get_children(): var item:TreeItem = ref_item.create_child() _recursive_create(item, child, root_node) func _notification(what: int) -> void: if what == NOTIFICATION_VISIBILITY_CHANGED: if not visible: return fake_tree.clear() fake_tree.deselect_all() line_edit.text = "" get_ok_button().disabled = true var scene_root:Node =\ editor_plugin.get_editor_interface().get_edited_scene_root() if not is_instance_valid(scene_root): return var root:TreeItem = fake_tree.create_item() root.set_text(0, scene_root.name) root.set_icon(0, get_theme_icon(scene_root.get_class(), "EditorIcons")) _recursive_create(root, scene_root, scene_root) func _fake_tree_item_selected() -> void: line_edit.text = fake_tree.get_selected().get_metadata(0) get_ok_button().disabled = line_edit.text.is_empty() node_path = NodePath(line_edit.text) func _fake_tree_item_activated() -> void: node_path = NodePath(fake_tree.get_selected().get_metadata(0)) confirmed.emit() hide() func _line_edit_text_changed(new_text: String) -> void: fake_tree.deselect_all() get_ok_button().disabled = new_text.is_empty() node_path = NodePath(new_text) func _init() -> void: name = "NodeSelector" title = "Node Selector" var vb := VBoxContainer.new() vb.size_flags_vertical = Control.SIZE_EXPAND_FILL vb.size_flags_horizontal = Control.SIZE_EXPAND_FILL add_child(vb) line_edit = LineEdit.new() line_edit.placeholder_text = "Input NodePath" line_edit.size_flags_horizontal = Control.SIZE_EXPAND_FILL line_edit.text_changed.connect(_line_edit_text_changed) vb.add_child(line_edit) register_text_enter(line_edit) fake_tree = Tree.new() fake_tree.size_flags_horizontal = Control.SIZE_EXPAND_FILL fake_tree.size_flags_vertical = Control.SIZE_EXPAND_FILL fake_tree.item_selected.connect(_fake_tree_item_selected) fake_tree.item_activated.connect(_fake_tree_item_activated) vb.add_child(fake_tree) ```

You will only need to create it and show it when you need it (even reuse it since is kinda "generic")

@tool
extends EditorPlugin

var node_selector := NodeSelector.new()

func _enter_tree() -> void:
  node_selector.editor_plugin = self
  node_selector.confirmed.connect(_on_node_selector_confirmed)
  EditorInterface.get_base_control_node().add_child(node_selector)
  node_selector.popup_clamped()

func _on_node_selector_confirmed() -> void:
  print(node_selector.node_path)

It was made for my specific use case (being able to select a node path in editor inspector or providing an arbitrary path) but I think it fits most of the cases where user needs a node selector. The implementation was taken from https://github.com/AnidemDex/Blockflow/blob/main/editor/inspector/inspector_tools.gd#L8C1-L78C26