This looks useful, it's usually much easier to control this than an equals operation with a range.

Should the selection be exclusive or inclusive? "Between" sounds exlusive. "In Range" is suggested as an alternative that's a bit more descriptive.

The implementation of the "between" and "not between" should not be duplicated.

We couldn't find the Python "between" function you mentioned. Also, I don't really think SQL should necessarily be considered a source for our UI design. I like the patch though.

Vyacheslav (hitrpr) awarded a token.Jan 14 2022, 12:23 AM

Address comments.
Change name from Between to In Range.
Use functions for comparisons.
Behaviour note, like Clamp Range it takes
into account the value of the min/max arguments
and swaps if required.

Harbormaster completed remote builds in B19978: Diff 47050.Jan 14 2022, 1:24 AM

Perhaps it could help to have an "Exclusive" checkbox or option to choose whether it is inclusive or exclusive?

Similar to "Equal or greater" being separated from "Greater"

Iliya Katueshenock (Moder) awarded a token.Feb 1 2022, 11:26 AM

Hans Goudey (HooglyBoogly) retitled this revision from Geometry Nodes: Add Between/Not Between operation modes to Compare node to Geometry Nodes: Add Between operations to Compare node.Feb 2 2022, 6:38 PM

Hans Goudey (HooglyBoogly) mentioned this in T95453: Geometry Nodes 3.2 Targets.Feb 2 2022, 7:59 PM

The code looks good to me. I think we can avoid the exclusive checkbox for now, to avoid adding options unless they're really necessary (it would also have a performance impact).

I think the socket labels are incorrect in this case (I would expect "Angle Min" and "Angle Max"

Also, when there's only one value, and the other two inputs are "Min" and "Max", what do you think about labeling the input "Value" rather than "A"?

Address comments with socket labels.
Add ui_class function for changing node header color.

Harbormaster completed remote builds in B20389: Diff 48053.Feb 7 2022, 7:16 PM

Hans Goudey (HooglyBoogly) added inline comments.Feb 11 2022, 7:36 PM

source/blender/nodes/function/nodes/node_fn_compare.cc
60–64	The situation with these identifiers is totally awful, but it probably makes sense to stick with the conventions from this file here
82	Could you commit this separately? (No need for a patch, since you've already implemented it for other nodes)
106–167	This looks like a huge mess, but I'm going to count on it being improved with separate refactors like multi-type sockets and moving the `is_available` checks to the socket declarations later.
225–230	Do you think there's a practical benefit for switching the min and max when they're reversed? I'm wondering if maybe that's not expected or necessary. It would be nice to avoid since there's a performance cost to the branching.