Geometry Nodes: Add warning for tools and text in node editor header #110813

Hans Goudey · 2023-08-04T21:33:52+02:00

Hans Goudey commented

2023-08-04 21:33:52 +02:00

Add three bits of helping information to make setting up a node
group operator easier in some cases:

A "Mark as Asset" button for reusing existing groups
"Node group is not marked as tool" warning label
"Add asset to catalog to show in menus" help text

Add three bits of helping information to make setting up a node group operator easier in some cases: 1. A "Mark as Asset" button for reusing existing groups 2. "Node group is not marked as tool" warning label 3. "Add asset to catalog to show in menus" help text --- ![image](/attachments/76798b17-291a-4307-b192-ce95d12007f0) ![image](/attachments/d18611b9-45ab-470b-b6b6-57178ae114ae) ![image](/attachments/7153da07-7855-4dda-ad0e-74eadef3e853) ![image](/attachments/9ad8d71b-a8af-410b-82ac-4e2b2d7bc567)

image.png

15 KiB

image.png

16 KiB

image.png

11 KiB

image.png

16 KiB

Hans Goudey added 1 commit 2023-08-04 21:33:58 +02:00

62a365b976 Geometry Nodes: Add warning for tools and text in node editor header

Hans Goudey requested review from Dalai Felinto 2023-08-04 21:34:15 +02:00

~~Hans Goudey referenced this pull request 2023-08-04 21:38:21 +02:00~~

Node Tools Overview #101778

Dalai Felinto commented

2023-08-04 23:21:11 +02:00

I think we should add Asset to the second message “Node group is not marked as tool” to guide users as to where to fix the problem.

E.g., “Asset not marked as tool”, or enabled as tool.

I’m wondering if the big Mark as Asset button needs more context.

Cc @pablovazquez

I think we should add Asset to the second message “Node group is not marked as tool” to guide users as to where to fix the problem. E.g., “Asset not marked as tool”, or enabled as tool. I’m wondering if the big Mark as Asset button needs more context. Cc @pablovazquez

👍 1

Hans Goudey added 1 commit 2023-08-21 14:31:14 +02:00

9f8c6f4a4b Change "node group" to "asset"

Dalai Felinto added this to the Nodes & Physics project 2023-08-21 15:15:09 +02:00

Dalai Felinto commented

2023-08-21 16:19:17 +02:00

Warnings working well so far. Some issues:

When I move to a catalog, the warning doesn't get dismissed right away (missing notifier?).
I think there is a bug, where if I try to move the node group to the Hair catalog it allows me, but it doesn't show in the menu (and the warning is still there).
I think we could have a warning too for when no Mode/Geometry is selected. But it could be a more generic one: "Change asset options to support at least a mode and geometry".

Warnings working well so far. Some issues: * When I move to a catalog, the warning doesn't get dismissed right away (missing notifier?). * I think there is a bug, where if I try to move the node group to the Hair catalog it allows me, but it doesn't show in the menu (and the warning is still there). * I think we could have a warning too for when no Mode/Geometry is selected. But it could be a more generic one: "Change asset options to support at least a mode and geometry".

Dalai Felinto commented

2023-08-21 17:30:14 +02:00

I think there is a bug, where if I try to move the node group to the Hair catalog it allows me, but it doesn't show in the menu (and the warning is still there).

Example file for the bug I mentioned: catalog.blend

> I think there is a bug, where if I try to move the node group to the Hair catalog it allows me, but it doesn't show in the menu (and the warning is still there). Example file for the bug I mentioned: [catalog.blend](/attachments/03a34a50-d1b7-4038-943f-6925a6bdb063) ![image](/attachments/9bf0f114-4e26-4fc6-9899-4cadc8b0d7a2)

catalog.blend

102 KiB

image.png

304 KiB

Hans Goudey commented

2023-08-21 21:54:56 +02:00

Thanks for the comments! Should be resolved now.

if I try to move the node group to the Hair catalog it allows me, but it doesn't show in the menu

This is still a known todo, the actual internal tree for the menus has to be rebuild when asset catalogs change. I fixed the missing redraw though.

Thanks for the comments! Should be resolved now. >if I try to move the node group to the Hair catalog it allows me, but it doesn't show in the menu This is still a known todo, the actual internal tree for the menus has to be rebuild when asset catalogs change. I fixed the missing redraw though.

Hans Goudey added 3 commits 2023-08-21 21:55:00 +02:00

594a985834 Merge branch 'main' into node-group-tool-header-warnings

c8a8178e8b Missing redraw when asset changes catalog

buildbot/vexp-code-patch-coordinator Build done. Details

e4b7a296ee Add warning "Asset options must support at least one type and mode"

Dalai Felinto commented

2023-08-21 22:00:29 +02:00

@Harley did you try the file? Because even after reopening Blender the operator didn’t show up in a menu. I’ll try again tomorrow into see if I’m not missing anything.

And even if it requires reopening Blender, at least the warning should go away since it is no longer valid.

@blender-bot package

@Harley did you try the file? Because even after reopening Blender the operator didn’t show up in a menu. I’ll try again tomorrow into see if I’m not missing anything. And even if it requires reopening Blender, at least the warning should go away since it is no longer valid. @blender-bot package

Blender Bot commented

2023-08-21 22:00:32 +02:00

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR110813) when ready.

Hans Goudey added 2 commits 2023-08-21 22:21:20 +02:00

38ace52411 Merge branch 'main' into node-group-tool-header-warnings

7de8ab1fbd Fix "asset not in catalog" warning for essentials assets

Hans Goudey commented

2023-08-21 22:22:10 +02:00

And even if it requires reopening Blender, at least the warning should go away since it is no longer valid.

Ah, my bad, sorry. "catalog simple name" seems to be empty for the "Hair" catalog. Not sure why that happens, but checking for a zeroed catalog ID is probably a better way of doing this anyway. Fixed now for me with that file, thanks.

>And even if it requires reopening Blender, at least the warning should go away since it is no longer valid. Ah, my bad, sorry. "catalog simple name" seems to be empty for the "Hair" catalog. Not sure why that happens, but checking for a zeroed catalog ID is probably a better way of doing this anyway. Fixed now for me with that file, thanks.

Harley Acheson commented

2023-08-22 02:42:08 +02:00

@dfelinto - Harley did you try the file?

Yes, if I open up your "catalog.blend" I see what you show, with a warning that won't go away.

But, to be honest, I am only now trying to get my head around the intent here.

It feels odd to me to click on an icon of a stack of books to "clear asset". Is this just a placeholder icon? Similarly the "copy" icon seems wrong for "New Geometry Tool Group"

I'm not really a fan of warnings and error messages in the header like this. Have we explored all other possibilities for this? It creates a very wide minimum width and adds constraint on what can be added to the header later. And will create inconsistency in that we don't have the option of doing this in many other places.

Draw it on the field? This seems to be a status and that would fit with View3D statuses. Or add a footer that we force visible when needed?

Has @pablovazquez looked at this? I would hope users could be led through these processes in a more natural way. For "Asset not marked as tool", if doing so is optional then it should not need a strong warning. I have chosen a valid state so shouldn't have to suffer a permanent warning. If it is not a valid state then we should eliminate it.

"Asset options must support at least one type and mode" is also a bit muddled. First it might be a bit clearer if the Asset popover had a title of "Asset Options" so I would know what you are referring to. The "Mode" mentioned in the warning has a clear section in the popover. But the "Type" in the message and the popover are different things. The "one type" looks to want me to select from one of the two "Geometry" checkboxes. And there is a "Type" in the popover but that is to "mark as tool".

> @dfelinto - Harley did you try the file? Yes, if I open up your "catalog.blend" I see what you show, with a warning that won't go away. But, to be honest, I am only now trying to get my head around the intent here. It feels odd to me to click on an icon of a stack of books to "clear asset". Is this just a placeholder icon? Similarly the "copy" icon seems wrong for "New Geometry Tool Group" I'm not really a fan of warnings and error messages in the header like this. Have we explored all other possibilities for this? It creates a very wide minimum width and adds constraint on what can be added to the header later. And will create inconsistency in that we don't have the option of doing this in many other places. Draw it on the field? This seems to be a status and that would fit with View3D statuses. Or add a footer that we force visible when needed? Has @pablovazquez looked at this? I would hope users could be led through these processes in a more natural way. For "Asset not marked as tool", if doing so is **optional** then it should not need a strong warning. I have chosen a valid state so shouldn't have to suffer a permanent warning. If it is not a valid state then we should eliminate it. "Asset options must support at least one type and mode" is also a bit muddled. First it might be a bit clearer if the Asset popover had a title of "Asset Options" so I would know what you are referring to. The "Mode" mentioned in the warning has a clear section in the popover. But the "Type" in the message and the popover are different things. The "one type" looks to want me to select from one of the two "Geometry" checkboxes. And there is a "Type" in the popover but that is to "mark as tool".

Harley Acheson commented

2023-08-22 02:51:04 +02:00

My above comments were a bit ramble-y. Here are some gifs...

The action described as "Mark as Tool" is clicking a single item checkbox with a section heading of "Type".

The warning about selecting at least one "Type" and "Mode" has nothing to do with the "Type" but of "Geometry"...

My above comments were a bit ramble-y. Here are some gifs... The action described as "Mark as Tool" is clicking a single item checkbox with a section heading of "Type". ![MarkAsTool.gif](/attachments/0ff35863-8b18-41cf-87e0-6ef4162b0a08) The warning about selecting at least one "Type" and "Mode" has nothing to do with the "Type" but of "Geometry"... ![TypeAndMode.gif](/attachments/b2cf0e80-1b31-496c-a251-7ba1dfa77bfa)

TypeAndMode.gif

21 KiB

MarkAsTool.gif

24 KiB

Dalai Felinto commented

2023-08-22 10:36:44 +02:00

Hey Harley, I intended to tag Hans, not you sorry. I'm glad you stepped in though.

The type is indeed unrelated, and the warning should be about geometry, not type.

It feels odd to me to click on an icon of a stack of books to "clear asset". Is this just a placeholder icon? Similarly the "copy" icon seems wrong for "New Geometry Tool Group"

The stack of book has been the icons for assets since they were introduced.

For "Asset not marked as tool", if doing so is optional then it should not need a strong warning.

This is a permanent warning because you are in the Tool context, and should be a warning when NOT editing a tool. This warning can even take over other (future) buttons on the header, so I'm less concerned about it taking space.

That said I'm open for different proposals. I hope this can be considered non-blocking though (or that those other proposals can be presented today/tomorrow at most, so we can still have Node Tools this week).

For me, what bothers me the most is that you can run into a cascade of warnings that you need to solve in sequence. Although instructive, it can be an annoying process.

Hey Harley, I intended to tag Hans, not you sorry. I'm glad you stepped in though. The type is indeed unrelated, and the warning should be about geometry, not type. > It feels odd to me to click on an icon of a stack of books to "clear asset". Is this just a placeholder icon? Similarly the "copy" icon seems wrong for "New Geometry Tool Group" The stack of book has been the icons for assets since they were introduced. > For "Asset not marked as tool", if doing so is optional then it should not need a strong warning. This is a permanent warning because you are in the Tool context, and should be a warning when NOT editing a tool. This warning can even take over other (future) buttons on the header, so I'm less concerned about it taking space. That said I'm open for different proposals. I hope this can be considered non-blocking though (or that those other proposals can be presented today/tomorrow at most, so we can still have Node Tools this week). For me, what bothers me the most is that you can run into a cascade of warnings that you need to solve in sequence. Although instructive, it can be an annoying process.

Dalai Felinto commented

2023-08-22 10:55:09 +02:00

Another option is ot always leave the Mode/Geometry in a valid state. So if deselect all Geometries, we turn all the other Geometries ON.

Jacques Lucke commented

2023-09-11 15:11:31 +02:00

The warnings shouldn't be necessary anymore due to a redesign of how assets are setup.

Jacques Lucke closed this pull request

2023-09-11 15:11:32 +02:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Geometry Nodes: Add warning for tools and text in node editor header #110813

Pull request closed