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

[Bug]: Link release search filter includes widget types #505

Open
christian-byrne opened this issue Aug 18, 2024 · 1 comment
Open

[Bug]: Link release search filter includes widget types #505

christian-byrne opened this issue Aug 18, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@christian-byrne
Copy link
Collaborator

christian-byrne commented Aug 18, 2024

Frontend Version

1.2.26

Expected Behavior

In the search popover after empty link release, it will only show nodes compatible with the released link.

Actual Behavior

The filter also includes nodes that have matching widget types.

Steps to Reproduce

  1. Add a node that has a primitive output type (STRING, INT, etc.). Since there are non in core you need custom node. For example you can use ImpactWildcardProcessor which outputs STRING
  2. Drag a link from the STRING output slot and release on canvas
  3. The results will show many nodes that have STRING widget inputs, instead of just STRING slot inputs. Choosing one can print warning or throw error depending on if there are any slots at all.

Debug Logs

.

Browser Logs

index.html:4 ComfyUI Front-end version: 1.2.26
api.ts:67 
litegraphTypes.ts:63 Could not find slot with type STRING on node Save Image. This should never happen
    connectTo @ litegraphTypes.ts:63
    (anonymous) @ NodeSearchBoxPopover.vue:88
    addNode @ NodeSearchBoxPopover.vue:87
    callWithErrorHandling @ runtime-core.esm-bundler.js:195
    callWithAsyncErrorHandling @ runtime-core.esm-bundler.js:202
    emit @ runtime-core.esm-bundler.js:726
    createVNode.onOptionSelect._cache.<computed>._cache.<computed> @ NodeSearchBox.vue:136
    callWithErrorHandling @ runtime-core.esm-bundler.js:195
    callWithAsyncErrorHandling @ runtime-core.esm-bundler.js:202
    emit @ runtime-core.esm-bundler.js:726
    onOptionSelect @ index.mjs:515
    onClick @ index.mjs:1281
    callWithErrorHandling @ runtime-core.esm-bundler.js:195
    callWithAsyncErrorHandling @ runtime-core.esm-bundler.js:202
    invoker @ runtime-dom.esm-bundler.js:693

What browsers do you use to access the UI ?

Google Chrome

Other

I think it makes sense to keep the behavior but convert the widgets to input and then connect. Otherwise add a null check here:

const newNodeSlot = newNodeSlots.findIndex(

@huchenlei huchenlei added the bug Something isn't working label Aug 18, 2024
@christian-byrne
Copy link
Collaborator Author

#644 fixed the error that occurred from this.

In terms of the behavior, I'm not sure whether it's a good idea to automatically convert the primitive widgets to inputs when adding nodes from link-release. If not, then this issue can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants