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 'constant' data value input nodes #3899

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

teward
Copy link
Contributor

@teward teward commented Jun 27, 2024

There are numerous times in workflows where it is useful or advantageous to have individual data input nodes for primitive datatypes (INT, STRING (both multiline and not), FLOAT).

However, these nodes are NOT provided as part of ComfyUI. There are numerous external node packs which include primitive datatype nodes.

This pull request is designed to make these part of ComfyUI in its 'extras' group of nodes.


Note that my coding style for nodes is to design a "base" node so that the bare minimum of changes are necessary for the actual nodes themselves to function. In this case, the _CONSTANT_BASE node acts as a structure defining the various data types for components of the node that are accepted later. And make sure that things're implemented before running. As such, however, because we only have to override a few small bits (RETURN_TYPES, RETURN_NAMES, and INPUT_TYPES definitions), the rest of the node base makes sure everything else "just works" without having to duplicate code. That's the only reason it was used here, because of how I code nodes.

Revisions / changes are welcome if necessary, but I think adding these primitive "constant" nodes would be a positive improvement and inclusion (it's also not a major set of changes).

@mcmonkey4eva
Copy link
Collaborator

Are you aware of the Primitive node?

@teward
Copy link
Contributor Author

teward commented Jun 27, 2024

Are you aware of the Primitive node?

Yes, however usage for it is Unclear and is not as useful for new users without such documentation (hint: there is none). Unless there's more clear documentation on this node, it's not clear to other users of how to provide such static inputs/values to other components of nodes. Which is why I sort of "take" the primitive node and expand it to the core datatypes for ease of use for other users.

@Bocian-1
Copy link

Are you aware of the Primitive node?

Primitive nodes force additional widgets (control_after_generate and sometimes control_filter_list) which you can't remove or hide in group nodes. This makes them very cumbersome when you don't want those widgets. It's especially awful for group nodes, where you may have a bunch of them. If I converted some of my group nodes to use primitives instead of nodes like impact float, they'd be 2x+ as big and most widgets would at best be useless or at worst could completely break internal logic if you accidentally set some control_after_generate to anything other than fixed. And again, you can't hide those currently.

Personally, I'd prefer to just be able to hide these widgets in group nodes or have a second version of primitive that doesn't add them in the first place (would also make setting up group nodes less tedious as you wouldn't have to go through hiding everything each time), but as a last resort, this PR would also make me and others less dependent on custom nodes for this.

@asagi4
Copy link
Contributor

asagi4 commented Jun 28, 2024

Primitive nodes are annoying: They don't exist in API-serialized graphs and they can't be rerouted. They're kind of special snowflakes that have weird behaviour that differs from other nodes. Having constants serialized in the API format would make it easy to create "input nodes" in workflows that are decoupled from other parameters any specific node might need.

In addition to the nodes here, constant string nodes with dynamicPrompts set to false would also be useful.

@mcmonkey4eva mcmonkey4eva added the Feature A new feature to add to ComfyUI. label Jun 28, 2024
The comments on the original PR suggested it'd be nice to have nodes with dynamicInputs set to False for both of the String nodes.  As such, I've created a dedicated node, and set the other string constants to dynamicInputs = True.
@teward
Copy link
Contributor Author

teward commented Sep 24, 2024

Primitive nodes are annoying: They don't exist in API-serialized graphs and they can't be rerouted. They're kind of special snowflakes that have weird behaviour that differs from other nodes. Having constants serialized in the API format would make it easy to create "input nodes" in workflows that are decoupled from other parameters any specific node might need.

In addition to the nodes here, constant string nodes with dynamicPrompts set to false would also be useful.

I think the last commit I made addresses this request, adding two additional String nodes that are non-dynamicInput(s).

@teward
Copy link
Contributor Author

teward commented Sep 26, 2024

@comfyanonymous and others this PR has sat for a few months, any updates on acceptance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature A new feature to add to ComfyUI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants