Page MenuHome
Differential D16870

Use ntreeContainsTree in more place
Needs ReviewPublic
Actions

Authored by Iliya Katueshenock (Moder) on Dec 27 2022, 12:36 PM.

Details

Reviewers
Hans Goudey (HooglyBoogly)
Jacques Lucke (JacquesLucke)
Summary

I tried to unify the code for checking the nodes' group for collision.
I've changed the naming conventions a bit, trying to make them context dependent and more general.


Also, in the future can be add:

  • Map cache to reduce recursion.
  • Text output of collision (inputs: NodeGroup, MyNodeGroup, output: NodeGroup->NodeGroup.001->NodeGroup.002->MyNodeGroup).

Diff Detail

Event Timeline

Iliya Katueshenock (Moder) requested review of this revision.Dec 27 2022, 12:36 PM
Iliya Katueshenock (Moder) created this revision.
Hans Goudey (HooglyBoogly) awarded a token.Dec 27 2022, 2:27 PM
Iliya Katueshenock (Moder) updated this revision to Diff 58914.Dec 27 2022, 6:00 PM
Iliya Katueshenock (Moder) retitled this revision from Cleanup: Use ntreeHasTree in more place to Cleanup: Use ntreeContainTree in more place.
Iliya Katueshenock (Moder) edited the summary of this revision. (Show Details)
Iliya Katueshenock (Moder) added reviewers: Hans Goudey (HooglyBoogly), Jacques Lucke (JacquesLucke).
Iliya Katueshenock (Moder) removed a subscriber: Hans Goudey (HooglyBoogly).
Iliya Katueshenock (Moder) updated this revision to Diff 58915.Dec 27 2022, 6:09 PM
Iliya Katueshenock (Moder) edited the summary of this revision. (Show Details)Dec 27 2022, 6:16 PM
Iliya Katueshenock (Moder) updated this revision to Diff 58922.Dec 28 2022, 11:08 AM
Iliya Katueshenock (Moder) edited the summary of this revision. (Show Details)
Iliya Katueshenock (Moder) edited the summary of this revision. (Show Details)
Iliya Katueshenock (Moder) updated this revision to Diff 59923.Wed, Jan 25, 8:11 PM
  • Merge master
  • Some naming and comment changes
Iliya Katueshenock (Moder) edited the summary of this revision. (Show Details)Wed, Jan 25, 8:11 PM
Iliya Katueshenock (Moder) retitled this revision from Cleanup: Use ntreeContainTree in more place to Use ntreeContainsTree in more place.
Iliya Katueshenock (Moder) set the repository for this revision to rB Blender.
Iliya Katueshenock (Moder) edited the summary of this revision. (Show Details)
Iliya Katueshenock (Moder) added a project: Python API.
Iliya Katueshenock (Moder) edited the summary of this revision. (Show Details)Wed, Jan 25, 8:15 PM
Hans Goudey (HooglyBoogly) requested changes to this revision.Wed, Jan 25, 11:32 PM

Looks good. Just two naming/tooltip suggestions.

source/blender/makesrna/intern/rna_nodetree.c
12725–12727

This comment can be moved to the function's description. Suggestion: Check if the node tree contains another. Used to avoid creating recursive node groups

12728

contains_in -> contains_group

This revision now requires changes to proceed.Wed, Jan 25, 11:32 PM
Iliya Katueshenock (Moder) added inline comments.Thu, Jan 26, 10:37 AM
source/blender/makesrna/intern/rna_nodetree.c
12728

A . contains_in . B mean, that searching in B group try to find A group.
but
A . contains_group . B mean, that B can containing in A group and this trying to confirm?

Hans Goudey (HooglyBoogly) added inline comments.Thu, Jan 26, 11:16 PM
source/blender/makesrna/intern/rna_nodetree.c
12728

I think contains_group is clearer. The function acts like a question-- "Does A contain B". A is the subject and B is the direct object. "contains_in" sounds like incorrect grammar IMO

Iliya Katueshenock (Moder) updated this revision to Diff 59984.Fri, Jan 27, 3:44 AM
Iliya Katueshenock (Moder) marked 2 inline comments as done.
Iliya Katueshenock (Moder) edited the summary of this revision. (Show Details)
  • Change py method name
Iliya Katueshenock (Moder) marked an inline comment as done.Fri, Jan 27, 3:44 AM
Hans Goudey (HooglyBoogly) accepted this revision.Mon, Feb 6, 4:35 AM
This revision is now accepted and ready to land.Mon, Feb 6, 4:35 AM
Hans Goudey (HooglyBoogly) added 1 blocking reviewer(s): Jacques Lucke (JacquesLucke).Tue, Feb 7, 12:11 AM
This revision now requires review to proceed.Tue, Feb 7, 12:11 AM

Revision Contents
Changeset List

PathSize
release/
scripts/
startup/
bl_ui/
11 lines
12 lines
source/
blender/
blenkernel/
7 lines
intern/
14 lines
editors/
space_node/
4 lines
20 lines
makesrna/
intern/
15 lines

Diff 59984

View Options

release/scripts/startup/bl_ui/node_add_menu.py

Loading...
View Options

release/scripts/startup/nodeitems_builtins.py

Loading...
View Options

source/blender/blenkernel/BKE_node.h

Loading...
View Options

source/blender/blenkernel/intern/node.cc

Loading...
View Options

source/blender/editors/space_node/node_edit.cc

Loading...
View Options

source/blender/editors/space_node/node_group.cc

Loading...
View Options

source/blender/makesrna/intern/rna_nodetree.c

Loading...