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

If a switch input is renamed when it is already selected, the combo value doesn't update #46

Open
chrisgoringe opened this issue Nov 25, 2024 · 15 comments

Comments

@chrisgoringe
Copy link

I renamed the slot, but the combo doesn't change
fouth

as a result,

Screenshot 2024-11-25 112024

@Trung0246
Copy link
Owner

Looks like I forgot about it.

@chrisgoringe
Copy link
Author

chrisgoringe commented Nov 25, 2024 via email

@JorgeR81
Copy link

JorgeR81 commented Nov 25, 2024

Picked up in testing of https://github.com/chrisgoringe/cg-controller,
because this was the most requested custom node to support!

The Hub node also works great in the Controller !

They both complement each other, very well:

  • The Hub allows the Controller to add individual widgets, independently from their nodes ( like InvokeAI ).
    We just need to add the widgets to the Hub node. And then add the Hub to the Controller !

  • And the Controller allows to add Image nodes, next to the Hub, in the same Controller window / side panel.


There is also a visual issue in the Hub, where is text widget is not placed correctly ( #47 ).
But it still works correctly, when the Hub is added to the Controller, as you can see in the screenshot.


d1

@JorgeR81
Copy link

The only feature I'm missing, it's the ability to hide the Hub's controls, once it's configured, because they waste a lot of space.

But I'm not sure about the best way to do this:

Ideally the Controller would use the "native" Hub node features.

But in this case, I guess the Controller needs this feature more, because it has a limited amount of free space in the screen.

@JorgeR81
Copy link

JorgeR81 commented Nov 25, 2024

About the switch fix ( fad4ef5).

Now it updates, when I rename the ( first ) input.
But I can only rename the first input.
When try o rename other inputs, nothing happens, when click OK in the text pop-up.

When I press F12, I see the browser it's in debug mode.
It's Goggle Chrome.


log

@chrisgoringe
Copy link
Author

  • or @chrisgoringe adds a feature to hide the Hub buttons, in the Controller.

Ideally the Controller would use the "native" Hub node features.

But in this case, I guess the Controller needs this feature more, because it has a limited amount of free space in the screen.

This would be covered by the controller having the option to choose which widgets to display, right? Planned for v1.6...

@Trung0246
Copy link
Owner

Now it updates, when I rename the ( first ) input. But I can only rename the first input. When try o rename other inputs, nothing happens, when click OK in the text pop-up.

Hm weird this should works to my testing. What did you do?

@JorgeR81
Copy link

Hm weird this should works to my testing. What did you do?

I did some more testing.
It works in old workflows, when input names were changed already.
But it does not work, if the inputs have the original name, in a new switch.


test

@JorgeR81
Copy link

JorgeR81 commented Nov 25, 2024

This would be covered by the controller having the option to choose which widgets to display, right? Planned for v1.6...

I'm not sure.

Will the Controller rely on the upcoming "advanced widgets" ComfyUI feature, to hide the widgets ?
Comfy-Org/litegraph.js#250

And will this ComfyUI feature, support hiding the Hub's buttons ?

If I convert the Hub's widgets to inputs, they don't show up in the Controller.
But we still can see the buttons, which take half the space ( and will be useless without their widgets ).


wt

@chrisgoringe
Copy link
Author

This would be covered by the controller having the option to choose which widgets to display, right? Planned for v1.6...

I'm not sure.

Will the Controller rely on the upcoming "advanced widgets" ComfyUI feature, to hide the widgets ? Comfy-Org/litegraph.js#250

no

And will this ComfyUI feature support hiding the Hub's buttons ?

yes (buttons are widgets)

But maybe we should take this conversation over to chrisgoringe/cg-controller#143 since it's not about 0246's switch

@JorgeR81
Copy link

JorgeR81 commented Nov 28, 2024

I did some more testing.
It works in old workflows, when input names were changed already.
But it does not work, if the inputs have the original name, in a new switch.

@Trung0246, where you able replicate this ?

Perhaps it's better to revert the current fix, until you have a complete fix.
With the old bug, we have a workaround. We just need to select the option again.
But with the new bug, introduced by the fix, we can't rename new inputs.

@Trung0246
Copy link
Owner

Where you able replicate this ?

Unfortunately no. On my side it works correctly. I could add option to revert back to original behavior.

@JorgeR81
Copy link

JorgeR81 commented Nov 29, 2024

On my side it works correctly.

Maybe this is related to my configuration.
I have:

  • Windows 10 
  • Portable install 
  • pytorch version: 2.3.1+cu121

I could add option to revert back to original behavior.

Yes, that would work for me.
I don't mind the original behavior.

@JorgeR81
Copy link

JorgeR81 commented Dec 1, 2024

I could add option to revert back to original behavior.

I tried the new option.
The old behavior is working fine for me.

@JorgeR81
Copy link

JorgeR81 commented Dec 1, 2024

I've made some more tests with the new behavior.

I'm happy with the original behavior, but I will share my test results, in case it helps to get a better solution.

sw1


With the new behavior, I've found that I can change all the input names, if the switch has the same amount of outputs, like in the image below.

sw


If the switch has no outputs, I can't change the name of any input.

sw3


If the switch has 1 output, I can't change the name of the first input.

sw01


If the switch has 2 outputs, I can't change the name of the first and second outputs.

sw22


If the switch has 3 outputs, I can change the names of all 3 inputs.

sw


But if remove 1 output, I can't change the name of the last input ( test3 )

sw2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants