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 union link connection type support #5806

Merged
merged 3 commits into from
Dec 3, 2024
Merged

Add union link connection type support #5806

merged 3 commits into from
Dec 3, 2024

Conversation

huchenlei
Copy link
Collaborator

Requested by @Kosinkadink

Supports running workflow with union types, i.e. types separated with comma like T1,T2,.... The frontend code (litegraph) already supports the connection of it. It's the backend validation failing blocking this feature to be properly used.

Before:
image

After:
image

Reproduction workflow

UnionTest.json

Required custom nodes:

@huchenlei huchenlei added the Feature A new feature to add to ComfyUI. label Nov 27, 2024
@Kosinkadink
Copy link
Collaborator

Kosinkadink commented Nov 27, 2024

Did some testing by making an AnimateDiff-Evolved input accept 4 different inputs, and it works as expected when running:
image

One thing that these union type inputs can't do (for good reason) is take primitive node inputs. This means that there is no way to connect basic types like INT, FLOAT, STRING etc to them with vanilla ComfyUI nodes. This is outside the scope of this PR, but I think it's finally time for ComfyUI to have native nodes for what the primitive node tries to do. A bunch of custom node packs have their own nodes to do this, but this should not require any node packs to solve or deal with. Primitive nodes also don't show their previous value when disconnected, so they are just messy to work with for the most part.

@huchenlei
Copy link
Collaborator Author

Did some testing by making an AnimateDiff-Evolved input accept 4 different inputs, and it works as expected when running: image

One thing that these union type inputs can't do (for good reason) is take primitive node inputs. This means that there is no way to connect basic types like INT, FLOAT, STRING etc to them with vanilla ComfyUI nodes. This is outside the scope of this PR, but I think it's finally time for ComfyUI to have native nodes for what the primitive node tries to do. A bunch of custom node packs have their own nodes to do this, but this should not require any node packs to solve or deal with. Primitive nodes also don't show their previous value when disconnected, so they are just messy to work with for the most part.

Primitive node support requires changes in the frontend, as it has custom logic on type checking.

@Amorano
Copy link

Amorano commented Nov 27, 2024

Requested by @Kosinkadink

Supports running workflow with union types, i.e. types separated with comma like T1,T2,.... The frontend code (litegraph) already supports the connection of it. It's the backend validation failing blocking this feature to be properly used.

Thank you for adding this back in -- been waiting for months for this to regress =D

@comfyanonymous comfyanonymous merged commit 8d4e063 into master Dec 3, 2024
6 checks passed
@comfyanonymous comfyanonymous deleted the union_types branch December 3, 2024 10:46
huchenlei added a commit that referenced this pull request Dec 3, 2024
comfyanonymous pushed a commit that referenced this pull request Dec 3, 2024
huchenlei added a commit that referenced this pull request Dec 3, 2024
webfiltered added a commit to webfiltered/ComfyUI that referenced this pull request Dec 4, 2024
@webfiltered webfiltered mentioned this pull request Dec 4, 2024
comfyanonymous pushed a commit that referenced this pull request Dec 4, 2024
* Reapply "Add union link connection type support (#5806)" (#5889)

This reverts commit bf9a90a.

* Fix union type breaks existing type workarounds

* Add non-string test

* Add tests for hacks and non-string types

* Support python versions lower than 3.11
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