Page MenuHome

UI/Nodes: Improve feedback when adding node fails (e.g. on drag & drop)
ClosedPublic

Authored by Julian Eisel (Severin) on Feb 15 2021, 12:12 PM.

Details

Summary

This is especially useful when trying to add a node group instance, e.g. via
drag & drop from the Outliner or Asset Browser.
Previously this would just silently fail, with no information why. This is a
source of confusion, e.g. earlier, it took me a moment to realize I was
dragging a node group into itself, which failed of course.
Blender should always try to help the user with useful error messages.

Adds error messages like: "Nesting a node group inside of itself is not
allowed", "Not a compositor node tree".

Adds a disabled hint return argument to node and node tree polling functions.
On error the hint is reported, or could even be shown in advance (e.g. if
checked via an operator poll option).

Depends on D10405.

Diff Detail

Repository
rB Blender
Branch
temp-node-add-feedback (branched from master)
Build Status
Buildable 13041
Build 13041: arc lint + arc unit

Event Timeline

Julian Eisel (Severin) requested review of this revision.Feb 15 2021, 12:12 PM
Julian Eisel (Severin) created this revision.
Jacques Lucke (JacquesLucke) requested changes to this revision.Feb 16 2021, 2:42 PM
Jacques Lucke (JacquesLucke) added inline comments.
source/blender/blenkernel/BKE_node.h
290–291

Add comment about ownership of the string returned in distabled_hint. Also, should it be r_disabled_hint? Same below.
Is the implementation of poll responsible for setting this to null if there is no hint?

source/blender/blenkernel/intern/node.cc
1938–1939

Could also just call it disabled_hint instead of dummy_info everywhere?

source/blender/makesrna/intern/rna_nodetree.c
3158

shadows the variable in the outer scope

source/blender/nodes/function/node_function_util.cc
20–22

static

This revision now requires changes to proceed.Feb 16 2021, 2:42 PM
Julian Eisel (Severin) marked 4 inline comments as done.
  • Address review feedback

Besides the one comment, this looks good to me now.

source/blender/blenkernel/BKE_node.h
294

The "owned by caller" part does not sound quite right, given that the caller is not responsible for freeing it. If the callback sets this, it must point to a statically allocated string it seems.

This revision is now accepted and ready to land.Mar 2 2021, 12:50 PM