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 tab for custom nodes #92039

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Muller-Castro
Copy link
Contributor

Adds a new tab dedicated to displaying the nodes accessible by the editor through "class_name" so as not to mix with the built-in nodes.

To avoid affecting the logic of the dialog, I copied the custom nodes to the new tab and hid them from the main tab.

Example

@AdriaandeJongh
Copy link
Contributor

AdriaandeJongh commented May 17, 2024

Hmm, this may add some UX friction. What happens when you search for a custom node and the builtin node tab is open? And does this window remember which tab was opened last and reopen on the same tab? Will this pr add another click every other time i want to add a node?

Regardless, i always thought it was pretty clear which nodes were custom in the node list because they have the script name behind them. In terms of cleanliness, idk if this is worth the UX friction.

@MewPurPur
Copy link
Contributor

MewPurPur commented May 17, 2024

I think this can be good and it can help resolve some problems, but it should be a dropdown with checkboxes for which nodes you want to see (built-in, custom, editor...). There's a proposal for showing editor nodes, but they'd need to be disabled at first.

Nodes that are ancestors but don't fit into the checked category should be dimmed out and not selectable.

@timothyqiu
Copy link
Member

I think this categorization will help users who manually select nodes. But it becomes more cumbersome for users who use the search function to create nodes.

Maybe this can be not actual TabContainers, but two toggle buttons for filtering built-in and custom nodes. When searching anything, these two toggle buttons are disabled so it always search among all nodes.

@Muller-Castro
Copy link
Contributor Author

Muller-Castro commented May 17, 2024

With the toggle buttons (suggested by @timothyqiu):

Example.mp4

@bsil78
Copy link

bsil78 commented May 17, 2024

By the way, I think that "Filter" term alone is confusing most of the time. "Filter out" would be more precise about effect of toggle button IMHO.

@MewPurPur
Copy link
Contributor

That feels like the opposite of what it is, I'd just remove the line.

@AdriaandeJongh
Copy link
Contributor

I agree that 'filter' is confusing. I can see something like what @MewPurPur suggested: a dropdown named 'show' with checkboxes for which nodes you want to see (built-in, custom, editor).

On top of that, seeing that last video, I would also suggest that maybe if only custom nodes are shown, that their parent classes are not shown as well, because technically those are built-in nodes? Not 100% on that, though.

@Lazy-Rabbit-2001
Copy link

Lazy-Rabbit-2001 commented May 18, 2024

This may be a partial assistance in realization of godotengine/godot-proposals#8741, as the proposal mentioned the similar functionality of this one
And I think the things in this proposal may help you a bit
(plus I think what you did primarily is beneficial for achieving your goal of implementing this feature, with the proposal as your hint)

@Muller-Castro
Copy link
Contributor Author

There are many good ideas. So far we have:

a) dropdown with checkboxes
b) filter with toggle buttons
c) tab for custom nodes
d) add category tab from godotengine/godot-proposals#8741

Personally I think they're all great.

Let me know which one I should choose or if it's better to push an update and video of each one first. 👍

@Lazy-Rabbit-2001
Copy link

Lazy-Rabbit-2001 commented May 19, 2024

In adddition, (this may be a bit off-topic but it's worth thinking of) if you don't mind, you can expand in the future the field from merely custom nodes to both custom nodes and custom resourses, because I found that when you arrange create_dialog.cpp, the same change may happen to the dialoge of creatibg new resourse as well.

@bsil78
Copy link

bsil78 commented May 20, 2024

I think filtering OUT results options should be inside Matches window space in order to make it obvious that is is filtrring OUT results of search.

@bsil78
Copy link

bsil78 commented May 20, 2024

I think that, for UX reasons too, a simpler quick way to filter is preferable than a drop down list. Check boxes of what we want or not is preferable IMHO. I would put inside Matches window a label "Show only : " with 2 check boxes "Built-in Nodes", "Custom Nodes", checked by default when popup window opens.

I think this is good and complementary with godotengine/godot-proposals#8741 as Built-in and Custom are obvious default categories, not personnal ones.

[Edit: typos]

@Muller-Castro
Copy link
Contributor Author

Hello! Just passing by to say that I'm going to implement what @Lazy-Rabbit-2001 proposed since his/her proposal is more detailed and no objections were raised since the last update. I think I can finish it by tomorrow and upload the code and video.

If there are any other points that anyone would like to highlight (or change), please let me know.

@Muller-Castro
Copy link
Contributor Author

Implementation of godotengine/godot-proposals#8741 with a few changes:

  • A row just for the category controls.
  • Single click to switch tabs, otherwise I'd have to change TabBar code, which is very complex.
  • Click on the close button to remove the tab.
  • Built-in has a lock icon to make it clear that it can't close, that it can't remove nodes from itself and so that it's easily distinguishable among the other tabs.
  • Add node opens a checklist to select all tabs that will receive the selected node.
  • The system supports both built-in and class_name classes.
  • The same node can be distributed to several tabs.
  • I wasn't sure if it should copy the child nodes as well so only one node is copied.
Categories.mp4

All changes are stored in tabs.Node and tabs.Resource in .godot/editor/ in a JSON format like this (but without the indentation):

image

@Lazy-Rabbit-2001
Copy link

Lazy-Rabbit-2001 commented Jun 2, 2024

Implementation of godotengine/godot-proposals#8741 with a few changes:

  • A row just for the category controls.
  • Single click to switch tabs, otherwise I'd have to change TabBar code, which is very complex.
  • Click on the close button to remove the tab.
  • Built-in has a lock icon to make it clear that it can't close, that it can't remove nodes from itself and so that it's easily distinguishable among the other tabs.
  • Add node opens a checklist to select all tabs that will receive the selected node.
  • The system supports both built-in and class_name classes.
  • The same node can be distributed to several tabs.
  • I wasn't sure if it should copy the child nodes as well so only one node is copied.
Categories.mp4

All changes are stored in tabs.Node and tabs.Resource in .godot/editor/ in a JSON format like this (but without the indentation):

image

I don't think copying child without distinguishable marks is a good idea, as children classes may be categorized in other categories as well. In this case, I think you can check how the superseded proposal of that one handled this situation, which may contribute to your progress a lot

Edit: BTW, it's better to consider these:

  1. Add a drag-and-drop for nodes that should be categorized in other tabs. For example, if one wants to group AnimationPlayer node to a custom category named "animation", the AnimationPlayer should be draggable and when dragged to the tab, the tab will be automatically focused and the developer can release the LMB in the current tree item area to put the dragged into the current category.
  2. Make adding custom category more packed. I mean, you can simply place the "+" button literally at the right edge of the tabs row. When "+" is clicked, a small dialog will pop to input the name, with "OK" to confirm the name and "×" to close, which is similar to how a new scene is added at the top of the scene editing pannel

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

Successfully merging this pull request may close these issues.

None yet

8 participants